diff mbox series

[v2] iio: bme680_i2c: Remove ACPI support

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

Commit Message

Guenter Roeck May 6, 2021, 3:43 a.m. UTC
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(-)

Comments

Hans de Goede May 6, 2021, 8:29 a.m. UTC | #1
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,
>
Andy Shevchenko May 6, 2021, 9:28 a.m. UTC | #2
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
>
Guenter Roeck May 6, 2021, 1:37 p.m. UTC | #3
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
Hans de Goede May 6, 2021, 1:42 p.m. UTC | #4
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
Guenter Roeck May 6, 2021, 1:50 p.m. UTC | #5
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
Andy Shevchenko May 6, 2021, 2:27 p.m. UTC | #6
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.
Andy Shevchenko May 6, 2021, 2:31 p.m. UTC | #7
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
Joe Perches May 6, 2021, 2:37 p.m. UTC | #8
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-                                  }
Jonathan Cameron May 7, 2021, 9:31 a.m. UTC | #9
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,
Guenter Roeck May 7, 2021, 1:34 p.m. UTC | #10
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
Guenter Roeck May 8, 2021, 3:09 a.m. UTC | #11
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
Hans de Goede May 8, 2021, 9:41 a.m. UTC | #12
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
Hans de Goede May 8, 2021, 9:47 a.m. UTC | #13
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
Guenter Roeck May 8, 2021, 3:24 p.m. UTC | #14
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
Jonathan Cameron May 8, 2021, 3:51 p.m. UTC | #15
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 mbox series

Patch

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,