Re: [PATCH] 2c: i801: Improve handling of chip-specific feature definitions

[Jarkko Nikula <jarkko.nikula@linux.intel.com>]

On 11/18/21 12:09 PM, Jean Delvare wrote:
> Hi Heiner,
> 
> On Mon, 08 Nov 2021 21:10:12 +0100, Heiner Kallweit wrote:
>> Reduce source code and code size by defining the chip features
>> statically.
> 
> While I don't like the PCI_DEVICE_DATA macro implementation (for it
> breaks grepping for PCI defines), I generally enjoy more data and less
> code. So I am fine with this change.
> 
> Jarkko, you are typically the one adding support for new devices to
> this driver so this change will affect you. Are you OK with that change?
> 
I think it makes code more readable and less error prone when adding 
support for new devices and merging with other upstream changes. I 
remember one such accident:

fd4b204a0971 ("i2c: i801: Bring back Block Process Call support for 
certain platforms")

>> +#define DEF_FEATURES	(FEATURE_BLOCK_PROC | FEATURE_I2C_BLOCK_READ	| \
> 
> Not a good name ("default" isn't descriptive) and not consistent
> either. I suggest "FEATURES_82801EB" instead, as this is the first
> chipset which supported all these features. And you can make the
> definitions of FEATURES_82801DB and FEATURES_82801EB consistent
> (spacing/alignment).
> 
How about calling default as FEATURES_ICH5 and 82801DB as FEATURES_ICH4? 
That makes easier to follow comments like "/* ICH4 and later */" in the 
code.

Jarkko