Subject: Re: [PATCH 4/11] Add platform_has_feature()

From: Nathan Fontenot <[email-masked]>
Date: Tue, 19 Mar 2013 13:03:50 -0500

On 03/14/2013 03:56 AM, Paul Mackerras wrote:
> On Fri, Mar 08, 2013 at 10:02:31PM -0600, Nathan Fontenot wrote:
>> The firmware_has_feature() function makes it easy to check for supported
>> features of the hardware. There is not corresponding function to check for
>> features supported by the client architecture.
> 
> Actually, firmware_has_feature checks for supported features of the
> hypervisor, or in a sense the platform, rather than hardware.

Ahh, thanks for clarifying that for me. I'll update the description.

> 
>> This patch adds a platform_has_feature() function to check features selected
>> by firmware and reported via the device tree 'ibm,architecture-vec5'
>> property. As part of this the #defines used for the architecture vector are
>> moved to prom.h and re-defined such that the vector 5 options have the vector
>> index and the feature bits encoded into them. This allows for callers of
>> platform_has_feature() to pass in a single pre-defined value.
> 
> One other comment below...
> 
>>  /* PCIe/MSI support.  Without MSI full PCIe is not supported */
>>  #ifdef CONFIG_PCI_MSI
>> -#define OV5_MSI			0x01	/* PCIe/MSI support */
>> +#define OV5_MSI			0x0201	/* PCIe/MSI support */
>>  #else
>> -#define OV5_MSI			0x00
>> +#define OV5_MSI			0x0200
>>  #endif /* CONFIG_PCI_MSI */
> 
> The #ifdef was done this way in order to control what ended up in the
> option vector we pass to the platform firmware.  For checking what the
> platform supports, wouldn't we want OV5_MSI to be 0x0201 always?
> Similarly for OV5_CMO, OV5_XCMO, etc.?

Yes, you're correct. I will update this.

-- 
-Nathan