[PATCH v3 5/12] Update firmware_has_feature() to check architecture bits

[Stephen Rothwell]

Hi Nathan,

On Mon, 22 Apr 2013 13:38:47 -0500 Nathan Fontenot <nfont at linux.vnet.ibm.com> wrote:
>
> -/* Option vector 5: PAPR/OF options supported */
> -#define OV5_LPAR		0x80	/* logical partitioning supported */
> -#define OV5_SPLPAR		0x40	/* shared-processor LPAR supported */
> +/* Option vector 5: PAPR/OF options supported
> + * Thses bits are also used for the platform_has_feature() call so
      ^^^^^
typo

> + * we encode the vector index in the define and use the OV5_FEAT()
> + * and OV5_INDX() macros to extract the desired information.
> + */
> +#define OV5_FEAT(x)	((x) & 0xff)
> +#define OV5_INDX(x)	((x) >> 8)
> +#define OV5_LPAR		0x0280	/* logical partitioning supported */
> +#define OV5_SPLPAR		0x0240	/* shared-processor LPAR supported */

Wouldn't it be clearer to say

#define OV5_LPAR	(OV5_INDX(0x2) | OV5_FEAT(0x80))

etc?

> @@ -145,6 +141,7 @@
>   * followed by # option vectors - 1, followed by the option vectors.
>   */
>  extern unsigned char ibm_architecture_vec[];
> +bool platform_has_feature(unsigned int);

"extern", please (if nothing else, for consistency).

> +static __initdata struct vec5_fw_feature
> +vec5_fw_features_table[FIRMWARE_MAX_FEATURES] = {

Why make this array FIRMWARE_MAX_FEATURES (63) long?  You could just
restrict the for loop below to ARRAY_SIZE(vec5_fw_features_table).

> +	{FW_FEATURE_TYPE1_AFFINITY,	OV5_TYPE1_AFFINITY},
> +};
> +
> +void __init fw_vec5_feature_init(const char *vec5, unsigned long len)
> +{
> +	unsigned int index, feat;
> +	int i;
> +
> +	pr_debug(" -> fw_vec5_feature_init()\n");
> +
> +	for (i = 0; i < FIRMWARE_MAX_FEATURES; i++) {
> +		if (!vec5_fw_features_table[i].feature)
> +			continue;

And this test could go away.

I realise that you have just copied the existing code, but you should not
do that blindly.  Maybe you could even add an (earlier) patch that fixes
the existing code.
-- 
Cheers,
Stephen Rothwell                    sfr at canb.auug.org.au
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20130423/516aabc7/attachment.sig>