| Message ID | 20210506034332.752263-1-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] iio: bme680_i2c: Remove ACPI support | expand |
Hi, On 5/6/21 5:43 AM, Guenter Roeck 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@intel.com> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com> > Cc: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > v2: Instead of making bme680_acpi_match conditional, > remove ACPI support entirely since the ACPI ID is > not valid. Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > > drivers/iio/chemical/bme680_i2c.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/drivers/iio/chemical/bme680_i2c.c b/drivers/iio/chemical/bme680_i2c.c > index 29c0dfa4702b..74cf89c82c0a 100644 > --- a/drivers/iio/chemical/bme680_i2c.c > +++ b/drivers/iio/chemical/bme680_i2c.c > @@ -11,7 +11,6 @@ > * Note: SDO pin cannot be left floating otherwise I2C address > * will be undefined. > */ > -#include <linux/acpi.h> > #include <linux/i2c.h> > #include <linux/module.h> > #include <linux/regmap.h> > @@ -42,12 +41,6 @@ static const struct i2c_device_id bme680_i2c_id[] = { > }; > MODULE_DEVICE_TABLE(i2c, bme680_i2c_id); > > -static const struct acpi_device_id bme680_acpi_match[] = { > - {"BME0680", 0}, > - {}, > -}; > -MODULE_DEVICE_TABLE(acpi, bme680_acpi_match); > - > static const struct of_device_id bme680_of_i2c_match[] = { > { .compatible = "bosch,bme680", }, > {}, > @@ -57,7 +50,6 @@ MODULE_DEVICE_TABLE(of, bme680_of_i2c_match); > static struct i2c_driver bme680_i2c_driver = { > .driver = { > .name = "bme680_i2c", > - .acpi_match_table = ACPI_PTR(bme680_acpi_match), > .of_match_table = bme680_of_i2c_match, > }, > .probe = bme680_i2c_probe, >
On Thu, May 6, 2021 at 6:43 AM Guenter Roeck <linux@roeck-us.net> 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. Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> with the SPI part amended in the same way. Thanks! > Reported-by: kernel test robot <lkp@intel.com> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com> > Cc: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > v2: Instead of making bme680_acpi_match conditional, > remove ACPI support entirely since the ACPI ID is > not valid. > > drivers/iio/chemical/bme680_i2c.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/drivers/iio/chemical/bme680_i2c.c b/drivers/iio/chemical/bme680_i2c.c > index 29c0dfa4702b..74cf89c82c0a 100644 > --- a/drivers/iio/chemical/bme680_i2c.c > +++ b/drivers/iio/chemical/bme680_i2c.c > @@ -11,7 +11,6 @@ > * Note: SDO pin cannot be left floating otherwise I2C address > * will be undefined. > */ > -#include <linux/acpi.h> > #include <linux/i2c.h> > #include <linux/module.h> > #include <linux/regmap.h> > @@ -42,12 +41,6 @@ static const struct i2c_device_id bme680_i2c_id[] = { > }; > MODULE_DEVICE_TABLE(i2c, bme680_i2c_id); > > -static const struct acpi_device_id bme680_acpi_match[] = { > - {"BME0680", 0}, > - {}, > -}; > -MODULE_DEVICE_TABLE(acpi, bme680_acpi_match); > - > static const struct of_device_id bme680_of_i2c_match[] = { > { .compatible = "bosch,bme680", }, > {}, > @@ -57,7 +50,6 @@ MODULE_DEVICE_TABLE(of, bme680_of_i2c_match); > static struct i2c_driver bme680_i2c_driver = { > .driver = { > .name = "bme680_i2c", > - .acpi_match_table = ACPI_PTR(bme680_acpi_match), > .of_match_table = bme680_of_i2c_match, > }, > .probe = bme680_i2c_probe, > -- > 2.25.1 >
On Thu, May 06, 2021 at 12:28:40PM +0300, Andy Shevchenko wrote: > On Thu, May 6, 2021 at 6:43 AM Guenter Roeck <linux@roeck-us.net> 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. > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > with the SPI part amended in the same way. > Right. I just sent a patch doing that. Oddly enough 0-day didn't complain about that one to me, nor about many other drivers with the same problem. No idea how it decides if and when to make noise. Is there a way to determine invalid ACPI IDs ? I could write a coccinelle script to remove the code automatically. Thanks, Guenter
Hi, On 5/6/21 3:37 PM, Guenter Roeck wrote: > On Thu, May 06, 2021 at 12:28:40PM +0300, Andy Shevchenko wrote: >> On Thu, May 6, 2021 at 6:43 AM Guenter Roeck <linux@roeck-us.net> 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. >> >> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> >> >> with the SPI part amended in the same way. >> > Right. I just sent a patch doing that. Oddly enough 0-day didn't complain > about that one to me, nor about many other drivers with the same problem. > No idea how it decides if and when to make noise. > > Is there a way to determine invalid ACPI IDs ? No, unfortunately not. There is a format which ACPI IDs are supposed to follow, but some "out in the wild" API ids don't follow this; and many fake (made up) ACPI ids do follow it... We (mostly Andy and me) are not even 100% sure this one is a fake ACPI ID, but we do pretty strongly believe that it is. Regards, Hans
On Thu, May 06, 2021 at 03:42:08PM +0200, Hans de Goede wrote: > Hi, > > On 5/6/21 3:37 PM, Guenter Roeck wrote: > > On Thu, May 06, 2021 at 12:28:40PM +0300, Andy Shevchenko wrote: > >> On Thu, May 6, 2021 at 6:43 AM Guenter Roeck <linux@roeck-us.net> 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. > >> > >> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > >> > >> with the SPI part amended in the same way. > >> > > Right. I just sent a patch doing that. Oddly enough 0-day didn't complain > > about that one to me, nor about many other drivers with the same problem. > > No idea how it decides if and when to make noise. > > > > Is there a way to determine invalid ACPI IDs ? > > No, unfortunately not. There is a format which ACPI IDs are > supposed to follow, but some "out in the wild" API ids don't > follow this; and many fake (made up) ACPI ids do follow it... > > We (mostly Andy and me) are not even 100% sure this one is > a fake ACPI ID, but we do pretty strongly believe that it is. > What a mess :-( Guenter
On Thu, May 6, 2021 at 4:37 PM Guenter Roeck <linux@roeck-us.net> wrote: > On Thu, May 06, 2021 at 12:28:40PM +0300, Andy Shevchenko wrote: > > On Thu, May 6, 2021 at 6:43 AM Guenter Roeck <linux@roeck-us.net> 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. > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > > > with the SPI part amended in the same way. > > > Right. I just sent a patch doing that. Oddly enough 0-day didn't complain > about that one to me, nor about many other drivers with the same problem. > No idea how it decides if and when to make noise. randconfig I believe. > Is there a way to determine invalid ACPI IDs ? I could write a coccinelle > script to remove the code automatically. As Hans said... My understanding that most of the fake IDs come into life due to: - people apply similar rules to them as they knew about OF case (and certain maintainers blindly allowed that) - people in big companies need to quickly prototype something without giving a crap about ACPI specification and / or process The last part (I believe the smallest one) is vendors who heard about ACPI, but haven't enough knowledge about the process.
On Thu, May 6, 2021 at 4:50 PM Guenter Roeck <linux@roeck-us.net> wrote: > On Thu, May 06, 2021 at 03:42:08PM +0200, Hans de Goede wrote: > > On 5/6/21 3:37 PM, Guenter Roeck wrote: ... > > We (mostly Andy and me) are not even 100% sure this one is > > a fake ACPI ID, but we do pretty strongly believe that it is. > > > > What a mess :-( What we can do is a checkpatch-alike check for vendor ID to be registered in [1] and issue a warning if not. At least it alerts maintainers. Joe, do you think it is doable or we should have a separate tool for that? (Because I have no clue how checkpatch cohabits with internet connection, otherwise the problem with synchronisation of that registry might be a problem) [1]: https://uefi.org/PNP_ACPI_Registry
On Thu, 2021-05-06 at 17:31 +0300, Andy Shevchenko wrote: > On Thu, May 6, 2021 at 4:50 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On Thu, May 06, 2021 at 03:42:08PM +0200, Hans de Goede wrote: > > > On 5/6/21 3:37 PM, Guenter Roeck wrote: > > ... > > > > We (mostly Andy and me) are not even 100% sure this one is > > > a fake ACPI ID, but we do pretty strongly believe that it is. > > > > > > > What a mess :-( > > What we can do is a checkpatch-alike check for vendor ID to be > registered in [1] and issue a warning if not. At least it alerts > maintainers. Joe, do you think it is doable or we should have a > separate tool for that? (Because I have no clue how checkpatch > cohabits with internet connection, otherwise the problem with > synchronisation of that registry might be a problem) Perhaps best to have a separate scriptable tool and if necessary have checkpatch use it like the spdxcheck block. scripts/checkpatch.pl-sub is_SPDX_License_valid { scripts/checkpatch.pl- my ($license) = @_; scripts/checkpatch.pl- scripts/checkpatch.pl: return 1 if (!$tree || which("python") eq "" || !(-e "$root/scripts/spdxcheck.py") || !(-e "$gitroot")); scripts/checkpatch.pl- scripts/checkpatch.pl- my $root_path = abs_path($root); scripts/checkpatch.pl: my $status = `cd "$root_path"; echo "$license" | python scripts/spdxcheck.py -`; scripts/checkpatch.pl- return 0 if ($status ne ""); scripts/checkpatch.pl- return 1; scripts/checkpatch.pl-} [] scripts/checkpatch.pl- } elsif ($rawline =~ /(SPDX-License-Identifier: .*)/) { scripts/checkpatch.pl- my $spdx_license = $1; scripts/checkpatch.pl: if (!is_SPDX_License_valid($spdx_license)) { scripts/checkpatch.pl- WARN("SPDX_LICENSE_TAG", scripts/checkpatch.pl- "'$spdx_license' is not supported in LICENSES/...\n" . $herecurr); scripts/checkpatch.pl- }
On Wed, 5 May 2021 20:43:32 -0700 Guenter Roeck <linux@roeck-us.net> 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@intel.com> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com> > Cc: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> 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. Jonathan > --- > v2: Instead of making bme680_acpi_match conditional, > remove ACPI support entirely since the ACPI ID is > not valid. > > drivers/iio/chemical/bme680_i2c.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/drivers/iio/chemical/bme680_i2c.c b/drivers/iio/chemical/bme680_i2c.c > index 29c0dfa4702b..74cf89c82c0a 100644 > --- a/drivers/iio/chemical/bme680_i2c.c > +++ b/drivers/iio/chemical/bme680_i2c.c > @@ -11,7 +11,6 @@ > * Note: SDO pin cannot be left floating otherwise I2C address > * will be undefined. > */ > -#include <linux/acpi.h> > #include <linux/i2c.h> > #include <linux/module.h> > #include <linux/regmap.h> > @@ -42,12 +41,6 @@ static const struct i2c_device_id bme680_i2c_id[] = { > }; > MODULE_DEVICE_TABLE(i2c, bme680_i2c_id); > > -static const struct acpi_device_id bme680_acpi_match[] = { > - {"BME0680", 0}, > - {}, > -}; > -MODULE_DEVICE_TABLE(acpi, bme680_acpi_match); > - > static const struct of_device_id bme680_of_i2c_match[] = { > { .compatible = "bosch,bme680", }, > {}, > @@ -57,7 +50,6 @@ MODULE_DEVICE_TABLE(of, bme680_of_i2c_match); > static struct i2c_driver bme680_i2c_driver = { > .driver = { > .name = "bme680_i2c", > - .acpi_match_table = ACPI_PTR(bme680_acpi_match), > .of_match_table = bme680_of_i2c_match, > }, > .probe = bme680_i2c_probe,
On 5/7/21 2:31 AM, Jonathan Cameron wrote: > On Wed, 5 May 2021 20:43:32 -0700 > Guenter Roeck <linux@roeck-us.net> 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@intel.com> >> Cc: Andy Shevchenko <andy.shevchenko@gmail.com> >> Cc: Hans de Goede <hdegoede@redhat.com> >> Signed-off-by: Guenter Roeck <linux@roeck-us.net> > > 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. > Ok, no problem. I'll keep that in mind if I hit any others. Thanks, Guenter
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@roeck-us.net> 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@intel.com> > > Cc: Andy Shevchenko <andy.shevchenko@gmail.com> > > Cc: Hans de Goede <hdegoede@redhat.com> > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > > 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. Andy, Hans, does that look about right ? 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) [ and how to auto-detect those - any idea ? ] If you like I'll be happy to send you the coccinelle script I wrote to play with. Guenter --- drivers/iio/light/stk3310.c STK3310: match (prefix) against STK (SANTAK CORP.) STK3311: match (prefix) against STK (SANTAK CORP.) STK3335: match (prefix) against STK (SANTAK CORP.) drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c INVN6000: match (ACPI ID) against INVN (Invensense, Inc) drivers/iio/imu/fxos8700_i2c.c FXOS8700: No match drivers/iio/dac/ad5592r.c ADS5592: match (prefix) against ADS (Analog Devices Inc) drivers/iio/accel/stk8312.c STK8312: match (prefix) against STK (SANTAK CORP.) drivers/iio/light/us5182d.c USD5182: match (prefix) against USD (U.S. Digital Corporation) drivers/iio/imu/fxos8700_spi.c FXOS8700: No match drivers/iio/dac/ad5593r.c ADS5593: match (prefix) against ADS (Analog Devices Inc) drivers/iio/accel/stk8ba50.c STK8BA50: match (prefix) against STK (SANTAK CORP.) drivers/iio/accel/bma220_spi.c BMA0220: No match drivers/iio/magnetometer/ak8975.c AK8975: No match AK8963: No match INVN6500: match (ACPI ID) against INVN (Invensense, Inc) AK009911: No match AK09911: No match AKM9911: match (prefix) against AKM (Asahi Kasei Microsystems Company Ltd) AK09912: No match drivers/iio/imu/bmi160/bmi160_i2c.c BMI0160: match (prefix) against BMI (Benson Medical Instruments Company) drivers/iio/chemical/bme680_i2c.c BME0680: No match drivers/iio/accel/mxc6255.c MXC6225: No match MXC6255: No match drivers/iio/accel/da280.c MIRAACC: match (prefix) against MIR (Miro Computer Prod.) drivers/iio/proximity/sx9310.c STH9310: match (prefix) against STH (Semtech Corporation) STH9311: match (prefix) against STH (Semtech Corporation) drivers/iio/accel/bmc150-accel-i2c.c BSBA0150: No match BMC150A: No match BMI055A: match (prefix) against BMI (Benson Medical Instruments Company) BMA0255: No match BMA250E: No match BMA222: No match BMA222E: No match BMA0280: No match BOSC0200: match (ACPI ID) against BOSC (Robert Bosch GmbH) drivers/iio/light/rpr0521.c RPR0521: No match drivers/iio/humidity/hts221_i2c.c SMO9100: match (prefix) against SMO (STMicroelectronics) drivers/iio/adc/ti-adc108s102.c INT3495: match (prefix) against INT (Interphase Corporation) drivers/iio/accel/kxcjk-1013.c KXCJ1013: No match KXCJ1008: No match KXCJ9000: No match KIOX0008: match (ACPI ID) against KIOX (Kionix, Inc.) KIOX0009: match (ACPI ID) against KIOX (Kionix, Inc.) KIOX000A: match (ACPI ID) against KIOX (Kionix, Inc.) KIOX010A: match (ACPI ID) against KIOX (Kionix, Inc.) KIOX020A: match (ACPI ID) against KIOX (Kionix, Inc.) KXTJ1009: No match KXJ2109: No match SMO8500: match (prefix) against SMO (STMicroelectronics) drivers/iio/magnetometer/mmc35240.c MMC35240: No match drivers/iio/light/apds9960.c MSHW0184: match (ACPI ID) against MSHW (Microsoft Corporation) drivers/iio/accel/bmc150-accel-spi.c BSBA0150: No match BMC150A: No match BMI055A: match (prefix) against BMI (Benson Medical Instruments Company) BMA0255: No match BMA250E: No match BMA222: No match BMA222E: No match BMA0280: No match drivers/iio/proximity/sx9500.c SSX9500: No match SASX9500: match (prefix) against SAS (Stores Automated Systems Inc) drivers/iio/imu/bmi160/bmi160_spi.c BMI0160: match (prefix) against BMI (Benson Medical Instruments Company) drivers/iio/chemical/bme680_spi.c BME0680: No match drivers/iio/accel/mxc4005.c MXC4005: No match MXC6655: No match drivers/iio/pressure/hp206c.c HOP206C: No match 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/accel/mma9551.c MMA9551: match (prefix) against MMA (Micromedia AG) drivers/iio/potentiometer/max5487.c MAX5487: match (prefix) against MAX (Rogen Tech Distribution Inc) MAX5488: match (prefix) against MAX (Rogen Tech Distribution Inc) MAX5489: match (prefix) against MAX (Rogen Tech Distribution Inc) drivers/iio/light/jsa1212.c JSA1212: No match drivers/iio/imu/kmx61.c KMX61021: No match drivers/iio/pressure/st_pressure_i2c.c SNO9210: match (prefix) against SNO (SINOSUN TECHNOLOGY CO., LTD) drivers/iio/light/pa12203001.c TXCPA122: No match drivers/iio/light/cm32181.c CPLM3218: match (ACPI ID) against CPLM (Capella Microsystems Inc.) drivers/iio/humidity/am2315.c AOS2315: No match drivers/iio/accel/st_accel_i2c.c SMO8840: match (prefix) against SMO (STMicroelectronics) SMO8A90: match (prefix) against SMO (STMicroelectronics) drivers/iio/accel/mma7660.c MMA7660: match (prefix) against MMA (Micromedia AG) drivers/iio/magnetometer/bmc150_magn_spi.c BMC150B: No match BMC156B: No match BMM150B: No match drivers/iio/light/max44000.c MAX44000: match (prefix) against MAX (Rogen Tech Distribution Inc) drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c INVN6500: match (ACPI ID) against INVN (Invensense, Inc) drivers/iio/gyro/bmg160_i2c.c BMG0160: No match BMI055B: match (prefix) against BMI (Benson Medical Instruments Company) BMI088B: match (prefix) against BMI (Benson Medical Instruments Company) drivers/iio/adc/ti-adc128s052.c AANT1280: match (ACPI ID) against AANT (AAEON Technology Inc.) drivers/iio/accel/mma9553.c MMA9553: match (prefix) against MMA (Micromedia AG) drivers/iio/magnetometer/bmc150_magn_i2c.c BMC150B: No match BMC156B: No match BMM150B: No match drivers/iio/light/ltr501.c LTER0501: No match LTER0559: No match LTER0301: No match
Hi, On 5/8/21 9:48 AM, Andy Shevchenko wrote: > > > On Saturday, May 8, 2021, Guenter Roeck <linux@roeck-us.net <mailto:linux@roeck-us.net>> 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@roeck-us.net <mailto:linux@roeck-us.net>> 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@intel.com <mailto:lkp@intel.com>> > > > Cc: Andy Shevchenko <andy.shevchenko@gmail.com <mailto:andy.shevchenko@gmail.com>> > > > Cc: Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>> > > > Signed-off-by: Guenter Roeck <linux@roeck-us.net <mailto:linux@roeck-us.net>> > > > > 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 :| Regards, Hans
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@roeck-us.net <mailto:linux@roeck-us.net>> 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@roeck-us.net <mailto:linux@roeck-us.net>> 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@intel.com <mailto:lkp@intel.com>> >> > > Cc: Andy Shevchenko <andy.shevchenko@gmail.com <mailto:andy.shevchenko@gmail.com>> >> > > Cc: Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>> >> > > Signed-off-by: Guenter Roeck <linux@roeck-us.net <mailto:linux@roeck-us.net>> >> > >> > 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
On 5/8/21 12:48 AM, Andy Shevchenko wrote: [ ... ] > > If you like I'll be happy to send you the coccinelle script I wrote > to play with. > > > Please share (maybe even thru GH gist or so?) > https://github.com/groeck/coccinelle-patches, subdirectory 'acpi'. With "MODE=patch", it "only" touches 258 files in the kernel, and 43 files in drivers/iio. From the other comments it looks like we would need another csv based match table, something like ID, "valid" | "invalid" to override the results from the published ID tables. Guenter
On Wed, 5 May 2021 20:43:32 -0700 Guenter Roeck <linux@roeck-us.net> 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@intel.com> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com> > Cc: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> Message tweaked to reflect PRP0001 route which should work just fine with ACPI and this driver. Jonathan > --- > v2: Instead of making bme680_acpi_match conditional, > remove ACPI support entirely since the ACPI ID is > not valid. > > drivers/iio/chemical/bme680_i2c.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/drivers/iio/chemical/bme680_i2c.c b/drivers/iio/chemical/bme680_i2c.c > index 29c0dfa4702b..74cf89c82c0a 100644 > --- a/drivers/iio/chemical/bme680_i2c.c > +++ b/drivers/iio/chemical/bme680_i2c.c > @@ -11,7 +11,6 @@ > * Note: SDO pin cannot be left floating otherwise I2C address > * will be undefined. > */ > -#include <linux/acpi.h> > #include <linux/i2c.h> > #include <linux/module.h> > #include <linux/regmap.h> > @@ -42,12 +41,6 @@ static const struct i2c_device_id bme680_i2c_id[] = { > }; > MODULE_DEVICE_TABLE(i2c, bme680_i2c_id); > > -static const struct acpi_device_id bme680_acpi_match[] = { > - {"BME0680", 0}, > - {}, > -}; > -MODULE_DEVICE_TABLE(acpi, bme680_acpi_match); > - > static const struct of_device_id bme680_of_i2c_match[] = { > { .compatible = "bosch,bme680", }, > {}, > @@ -57,7 +50,6 @@ MODULE_DEVICE_TABLE(of, bme680_of_i2c_match); > static struct i2c_driver bme680_i2c_driver = { > .driver = { > .name = "bme680_i2c", > - .acpi_match_table = ACPI_PTR(bme680_acpi_match), > .of_match_table = bme680_of_i2c_match, > }, > .probe = bme680_i2c_probe,
diff --git a/drivers/iio/chemical/bme680_i2c.c b/drivers/iio/chemical/bme680_i2c.c index 29c0dfa4702b..74cf89c82c0a 100644 --- a/drivers/iio/chemical/bme680_i2c.c +++ b/drivers/iio/chemical/bme680_i2c.c @@ -11,7 +11,6 @@ * Note: SDO pin cannot be left floating otherwise I2C address * will be undefined. */ -#include <linux/acpi.h> #include <linux/i2c.h> #include <linux/module.h> #include <linux/regmap.h> @@ -42,12 +41,6 @@ static const struct i2c_device_id bme680_i2c_id[] = { }; MODULE_DEVICE_TABLE(i2c, bme680_i2c_id); -static const struct acpi_device_id bme680_acpi_match[] = { - {"BME0680", 0}, - {}, -}; -MODULE_DEVICE_TABLE(acpi, bme680_acpi_match); - static const struct of_device_id bme680_of_i2c_match[] = { { .compatible = "bosch,bme680", }, {}, @@ -57,7 +50,6 @@ MODULE_DEVICE_TABLE(of, bme680_of_i2c_match); static struct i2c_driver bme680_i2c_driver = { .driver = { .name = "bme680_i2c", - .acpi_match_table = ACPI_PTR(bme680_acpi_match), .of_match_table = bme680_of_i2c_match, }, .probe = bme680_i2c_probe,
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@intel.com> Cc: Andy Shevchenko <andy.shevchenko@gmail.com> Cc: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- v2: Instead of making bme680_acpi_match conditional, remove ACPI support entirely since the ACPI ID is not valid. drivers/iio/chemical/bme680_i2c.c | 8 -------- 1 file changed, 8 deletions(-)