Re: [PATCH v2] iio: bme680_i2c: Remove ACPI support

[Hans de Goede]

Hi again,

On 5/8/21 11:41 AM, Hans de Goede wrote:
> Hi,
> 
> On 5/8/21 9:48 AM, Andy Shevchenko wrote:
>>
>>
>> On Saturday, May 8, 2021, Guenter Roeck <linux@xxxxxxxxxxxx <mailto:linux@xxxxxxxxxxxx>> wrote:
>>
>>     On Fri, May 07, 2021 at 10:31:54AM +0100, Jonathan Cameron wrote:
>>     > On Wed,  5 May 2021 20:43:32 -0700
>>     > Guenter Roeck <linux@xxxxxxxxxxxx <mailto:linux@xxxxxxxxxxxx>> wrote:
>>     >
>>     > > With CONFIG_ACPI=n and -Werror, 0-day reports:
>>     > >
>>     > > drivers/iio/chemical/bme680_i2c.c:46:36: error:
>>     > >     'bme680_acpi_match' defined but not used
>>     > >
>>     > > Apparently BME0680 is not a valid ACPI ID. Remove it and with it
>>     > > ACPI support from the bme680_i2c driver.
>>     > >
>>     > > Reported-by: kernel test robot <lkp@xxxxxxxxx <mailto:lkp@xxxxxxxxx>>
>>     > > Cc: Andy Shevchenko <andy.shevchenko@xxxxxxxxx <mailto:andy.shevchenko@xxxxxxxxx>>
>>     > > Cc: Hans de Goede <hdegoede@xxxxxxxxxx <mailto:hdegoede@xxxxxxxxxx>>
>>     > > Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx <mailto:linux@xxxxxxxxxxxx>>
>>     >
>>     > A note for these is that I'll change the patch titles when applying.
>>     > We aren't removing ACPI support from the drivers, we are simply
>>     > removing the ACPI ID table entries.  For most of these PRP0001 magic
>>     > will work just fine with the OF table.  That's probably the
>>     > right way for small companies etc to use these in products without
>>     > having to jump through the hoops of getting an ACPI ID.
>>     >
>>
>>     Below is what Coccinelle tells me about ACPI IDs in drivers/iio.
>>     The script (tries) to do a prefix match of all ACPI IDs it finds against
>>     the PNP and ACPI ID databases from https://uefi.org/PNP_ACPI_Registry <https://uefi.org/PNP_ACPI_Registry>.
>>
>>     Andy, Hans, does that look about right ?
>>
>>
>>
>> The result looks nice for the first step!
>>  
>>
>>
>>     Next question is what to do with the mismatches and with false
>>     negatives such as:
>>
>>     drivers/iio/accel/stk8312.c
>>       STK8312: match (prefix) against STK (SANTAK CORP.)
>>     drivers/iio/light/isl29018.c
>>       ISL29018: match (prefix) against ISL (Isolation Systems)
>>       ISL29023: match (prefix) against ISL (Isolation Systems)
>>       ISL29035: match (prefix) against ISL (Isolation Systems)
>>     drivers/iio/gyro/bmg160_i2c.c
>>
>>
>>  
>>
>>       BMI055B: match (prefix) against BMI (Benson Medical Instruments Company)
>>       BMI088B: match (prefix) against BMI (Benson Medical Instruments Company)
>>
>>
>> These I think the real ones from the existing devices.
> 
> No that is wrong, these are Bosch sensors, so the BOSC0200 entry for
> the companion accelerometer is the only entry using the official company
> prefix. At least "BMA" ("BMA250E") and "BSG" ("BSG1160", "BSG2150") are
> also know to be used as prefixes for ACPI HIDs which are in active
> use for Bosch sensors :|
> 
> And Benson Medical Instruments Company has nothing to do with these
> sensors :|

p.s.

And there also is the Lenovo Yoga 300 11IBR convertible laptop which has
2 Bosch accelerometers, 1 in the display and 1 in the base/keyboard
described in a single ACPI "Device (ACC1) {}" ACPI fwnode (so not 1 fwnode
per sensor), and this single node to describe 2 separate I2C connected
ICs has a ACPI HID of "DUAL250E" (*), how is that for sticking to the HID
naming spec ?

I really have the feeling that the naming spec is not worth much more
then the paper it is written on, it is really really easy to find a ton
of examples out in the field which don't adhere to it.

Regards,

Hans



*) I recently got my hands on one of these and I still need to add support
for this to the kernel