diff mbox series

[v2,2/5] iio:adc:ti-adc081c: Drop ACPI ids that seem very unlikely to be official.

Message ID 20200721171444.825099-3-jic23@kernel.org (mailing list archive)
State New, archived
Headers show
Series iio:adc more of_match_ptr and similar removal | expand

Commit Message

Jonathan Cameron July 21, 2020, 5:14 p.m. UTC 
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

We have no known users of these in the wild.
it seems very unlikely these are real IDS having the form ADCXXXX
as that ID is owned by Achnor Datacomm not TI.

Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v1->v2
New patch 
 drivers/iio/adc/ti-adc081c.c | 24 +-----------------------
 1 file changed, 1 insertion(+), 23 deletions(-)

Comments

Andy Shevchenko July 21, 2020, 6:30 p.m. UTC | #1 
On Tue, Jul 21, 2020 at 8:16 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> We have no known users of these in the wild.
> it seems very unlikely these are real IDS having the form ADCXXXX

IDS -> IDs

> as that ID is owned by Achnor Datacomm not TI.

After addressing above typo fix,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

>
> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> v1->v2
> New patch
>  drivers/iio/adc/ti-adc081c.c | 24 +-----------------------
>  1 file changed, 1 insertion(+), 23 deletions(-)
>
> diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c
> index 9426f70a8005..8bc04cfae465 100644
> --- a/drivers/iio/adc/ti-adc081c.c
> +++ b/drivers/iio/adc/ti-adc081c.c
> @@ -19,7 +19,6 @@
>  #include <linux/i2c.h>
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
> -#include <linux/acpi.h>
>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
> @@ -153,17 +152,7 @@ static int adc081c_probe(struct i2c_client *client,
>         if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
>                 return -EOPNOTSUPP;
>
> -       if (ACPI_COMPANION(&client->dev)) {
> -               const struct acpi_device_id *ad_id;
> -
> -               ad_id = acpi_match_device(client->dev.driver->acpi_match_table,
> -                                         &client->dev);
> -               if (!ad_id)
> -                       return -ENODEV;
> -               model = &adcxx1c_models[ad_id->driver_data];
> -       } else {
> -               model = &adcxx1c_models[id->driver_data];
> -       }
> +       model = &adcxx1c_models[id->driver_data];
>
>         iio = devm_iio_device_alloc(&client->dev, sizeof(*adc));
>         if (!iio)
> @@ -238,21 +227,10 @@ static const struct of_device_id adc081c_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, adc081c_of_match);
>
> -#ifdef CONFIG_ACPI
> -static const struct acpi_device_id adc081c_acpi_match[] = {
> -       { "ADC081C", ADC081C },
> -       { "ADC101C", ADC101C },
> -       { "ADC121C", ADC121C },
> -       { }
> -};
> -MODULE_DEVICE_TABLE(acpi, adc081c_acpi_match);
> -#endif
> -
>  static struct i2c_driver adc081c_driver = {
>         .driver = {
>                 .name = "adc081c",
>                 .of_match_table = adc081c_of_match,
> -               .acpi_match_table = ACPI_PTR(adc081c_acpi_match),
>         },
>         .probe = adc081c_probe,
>         .remove = adc081c_remove,
> --
> 2.27.0
>
Jonathan Cameron Aug. 9, 2020, 2:09 p.m. UTC | #2 
On Tue, 21 Jul 2020 21:30:08 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, Jul 21, 2020 at 8:16 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > We have no known users of these in the wild.
> > it seems very unlikely these are real IDS having the form ADCXXXX  
> 
> IDS -> IDs
> 
> > as that ID is owned by Achnor Datacomm not TI.  
> 
> After addressing above typo fix,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Applied.

Thanks,

Jonathan

> 
> >
> > Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> > v1->v2
> > New patch
> >  drivers/iio/adc/ti-adc081c.c | 24 +-----------------------
> >  1 file changed, 1 insertion(+), 23 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c
> > index 9426f70a8005..8bc04cfae465 100644
> > --- a/drivers/iio/adc/ti-adc081c.c
> > +++ b/drivers/iio/adc/ti-adc081c.c
> > @@ -19,7 +19,6 @@
> >  #include <linux/i2c.h>
> >  #include <linux/module.h>
> >  #include <linux/mod_devicetable.h>
> > -#include <linux/acpi.h>
> >
> >  #include <linux/iio/iio.h>
> >  #include <linux/iio/buffer.h>
> > @@ -153,17 +152,7 @@ static int adc081c_probe(struct i2c_client *client,
> >         if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
> >                 return -EOPNOTSUPP;
> >
> > -       if (ACPI_COMPANION(&client->dev)) {
> > -               const struct acpi_device_id *ad_id;
> > -
> > -               ad_id = acpi_match_device(client->dev.driver->acpi_match_table,
> > -                                         &client->dev);
> > -               if (!ad_id)
> > -                       return -ENODEV;
> > -               model = &adcxx1c_models[ad_id->driver_data];
> > -       } else {
> > -               model = &adcxx1c_models[id->driver_data];
> > -       }
> > +       model = &adcxx1c_models[id->driver_data];
> >
> >         iio = devm_iio_device_alloc(&client->dev, sizeof(*adc));
> >         if (!iio)
> > @@ -238,21 +227,10 @@ static const struct of_device_id adc081c_of_match[] = {
> >  };
> >  MODULE_DEVICE_TABLE(of, adc081c_of_match);
> >
> > -#ifdef CONFIG_ACPI
> > -static const struct acpi_device_id adc081c_acpi_match[] = {
> > -       { "ADC081C", ADC081C },
> > -       { "ADC101C", ADC101C },
> > -       { "ADC121C", ADC121C },
> > -       { }
> > -};
> > -MODULE_DEVICE_TABLE(acpi, adc081c_acpi_match);
> > -#endif
> > -
> >  static struct i2c_driver adc081c_driver = {
> >         .driver = {
> >                 .name = "adc081c",
> >                 .of_match_table = adc081c_of_match,
> > -               .acpi_match_table = ACPI_PTR(adc081c_acpi_match),
> >         },
> >         .probe = adc081c_probe,
> >         .remove = adc081c_remove,
> > --
> > 2.27.0
> >  
> 
>
Jonathan Cameron Sept. 2, 2021, 9:23 a.m. UTC | #3 
On Tue, 21 Jul 2020 18:14:41 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> We have no known users of these in the wild.
> it seems very unlikely these are real IDS having the form ADCXXXX
> as that ID is owned by Achnor Datacomm not TI.
> 
> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Kunyang reports that ACPI ID ADC081C is out in the wild on some
AAEON boards.

I'll send out a partial revert soon to bring back that ID with appropriate
comments on where it used.

Key thing in future for these sorts of IDs is we need clarity
on who is using them before agreeing to take them. In this particular
my undestanding is the driver came first and as it worked the AAEON
firmware team used the ID.

Thanks,

Jonathan


> ---
> v1->v2
> New patch 
>  drivers/iio/adc/ti-adc081c.c | 24 +-----------------------
>  1 file changed, 1 insertion(+), 23 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c
> index 9426f70a8005..8bc04cfae465 100644
> --- a/drivers/iio/adc/ti-adc081c.c
> +++ b/drivers/iio/adc/ti-adc081c.c
> @@ -19,7 +19,6 @@
>  #include <linux/i2c.h>
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
> -#include <linux/acpi.h>
>  
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
> @@ -153,17 +152,7 @@ static int adc081c_probe(struct i2c_client *client,
>  	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
>  		return -EOPNOTSUPP;
>  
> -	if (ACPI_COMPANION(&client->dev)) {
> -		const struct acpi_device_id *ad_id;
> -
> -		ad_id = acpi_match_device(client->dev.driver->acpi_match_table,
> -					  &client->dev);
> -		if (!ad_id)
> -			return -ENODEV;
> -		model = &adcxx1c_models[ad_id->driver_data];
> -	} else {
> -		model = &adcxx1c_models[id->driver_data];
> -	}
> +	model = &adcxx1c_models[id->driver_data];
>  
>  	iio = devm_iio_device_alloc(&client->dev, sizeof(*adc));
>  	if (!iio)
> @@ -238,21 +227,10 @@ static const struct of_device_id adc081c_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, adc081c_of_match);
>  
> -#ifdef CONFIG_ACPI
> -static const struct acpi_device_id adc081c_acpi_match[] = {
> -	{ "ADC081C", ADC081C },
> -	{ "ADC101C", ADC101C },
> -	{ "ADC121C", ADC121C },
> -	{ }
> -};
> -MODULE_DEVICE_TABLE(acpi, adc081c_acpi_match);
> -#endif
> -
>  static struct i2c_driver adc081c_driver = {
>  	.driver = {
>  		.name = "adc081c",
>  		.of_match_table = adc081c_of_match,
> -		.acpi_match_table = ACPI_PTR(adc081c_acpi_match),
>  	},
>  	.probe = adc081c_probe,
>  	.remove = adc081c_remove,
Andy Shevchenko Sept. 2, 2021, 10:03 a.m. UTC | #4 
On Thu, Sep 2, 2021 at 12:23 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
> On Tue, 21 Jul 2020 18:14:41 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
>
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > We have no known users of these in the wild.
> > it seems very unlikely these are real IDS having the form ADCXXXX
> > as that ID is owned by Achnor Datacomm not TI.
> >
> > Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Kunyang reports that ACPI ID ADC081C is out in the wild on some
> AAEON boards.

Can AAEON clarify about all IDs in the kernel they have been (ab)using
so far? Then we probably need some comments. Also I would like to be
assured by AAEON that they won't make such mistakes in the future.

> I'll send out a partial revert soon to bring back that ID with appropriate
> comments on where it was used.

Yes, please.

> Key thing in future for these sorts of IDs is we need clarity
> on who is using them before agreeing to take them. In this particular
> my undestanding is the driver came first and as it worked the AAEON
> firmware team used the ID.

Yes, because it's definitely not an officially registered one.
diff mbox series

Patch

diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c
index 9426f70a8005..8bc04cfae465 100644
--- a/drivers/iio/adc/ti-adc081c.c
+++ b/drivers/iio/adc/ti-adc081c.c
@@ -19,7 +19,6 @@ 
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
-#include <linux/acpi.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
@@ -153,17 +152,7 @@  static int adc081c_probe(struct i2c_client *client,
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
 		return -EOPNOTSUPP;
 
-	if (ACPI_COMPANION(&client->dev)) {
-		const struct acpi_device_id *ad_id;
-
-		ad_id = acpi_match_device(client->dev.driver->acpi_match_table,
-					  &client->dev);
-		if (!ad_id)
-			return -ENODEV;
-		model = &adcxx1c_models[ad_id->driver_data];
-	} else {
-		model = &adcxx1c_models[id->driver_data];
-	}
+	model = &adcxx1c_models[id->driver_data];
 
 	iio = devm_iio_device_alloc(&client->dev, sizeof(*adc));
 	if (!iio)
@@ -238,21 +227,10 @@  static const struct of_device_id adc081c_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, adc081c_of_match);
 
-#ifdef CONFIG_ACPI
-static const struct acpi_device_id adc081c_acpi_match[] = {
-	{ "ADC081C", ADC081C },
-	{ "ADC101C", ADC101C },
-	{ "ADC121C", ADC121C },
-	{ }
-};
-MODULE_DEVICE_TABLE(acpi, adc081c_acpi_match);
-#endif
-
 static struct i2c_driver adc081c_driver = {
 	.driver = {
 		.name = "adc081c",
 		.of_match_table = adc081c_of_match,
-		.acpi_match_table = ACPI_PTR(adc081c_acpi_match),
 	},
 	.probe = adc081c_probe,
 	.remove = adc081c_remove,