[PATCH v2 0/5] iio:adc more of_match_ptr and similar removal

[PATCH v2 0/5] iio:adc more of_match_ptr and similar removal

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Remaining set from v1 + one additional patch from comments in that
series.

Changes since V1.

* Most patches now merged this is cleaning up the leftovers.
* Rephrase commit messages for cases where there is an ACPI ID.
* Add a patch to drop the unlikely explicit ACPI support in adc081c.
* Avoid the casting away of const in axp20x by keeping it const throughout.
* bcm_adc add a kconfig protection to limit driver to being built when
  CONFIG_OF enabled, or COMPILE_TEST for build tests.
  
Continuation of slow process to try and remove use of of_match_ptr
to supress asignment of the of_device_id table when !CONFIG_OF.

Usual argument that it prevents ACPI being used with these drivers
via PRP0001 in DSDT.  Perhaps more usefully we are cutting down on the
number of places it can be cut and paste from into new drivers.

This is just the low hanging fruit.  I'm not yet sure if it makes
sense to expend the effort to use generic firmware properties etc
for some of the remaining drivers as it is unlikely they'll ever
be used with anything other than device tree. There are 4 current
ADC drivers in this more complex category.

Jonathan Cameron (5):
  iio:adc:axp20x: Convert from OF to generic fw / device properties
  iio:adc:ti-adc081c: Drop ACPI ids that seem very unlikely to be
    official.
  iio:adc:ti-adc108s102: Drop CONFIG_OF and of_match_ptr protections
  iio:adc:ti-adc128s052: drop of_match_ptr protection
  iio:adc:bcm_iproc: Drop of_match_ptr protection and switch to
    mod_devicetable.h

 drivers/iio/adc/Kconfig         |  2 +-
 drivers/iio/adc/axp20x_adc.c    | 14 +++++++-------
 drivers/iio/adc/bcm_iproc_adc.c |  4 ++--
 drivers/iio/adc/ti-adc081c.c    | 24 +-----------------------
 drivers/iio/adc/ti-adc108s102.c |  5 ++---
 drivers/iio/adc/ti-adc128s052.c |  3 ++-
 6 files changed, 15 insertions(+), 37 deletions(-)

-- 
2.27.0

[PATCH v2 1/5] iio:adc:axp20x: Convert from OF to generic fw / device properties

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Whilst fairly unlikely anyone will ever use this driver with anything
other than DT, we are trying to move IIO over to the generic interfaces
where easy to do so.

In this case this involved moving to generic check on presence
of fw_node, generic device_get_match_data and dropping the of_match_ptr
protection.  Also relevant header changes to have property.h and
mod_devicetable.h only.

Also drop the casting away of a const in favour of retaining
the const throughout.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Quentin Schulz <quentin.schulz@bootlin.com>
---
v1->v2

* Avoid the casting away of const by keeping it const throughout.

drivers/iio/adc/axp20x_adc.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
index 798ff2d89691..3e0c0233b431 100644
--- a/drivers/iio/adc/axp20x_adc.c
+++ b/drivers/iio/adc/axp20x_adc.c
@@ -9,10 +9,10 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_device.h>
+#include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/thermal.h>
 
@@ -67,7 +67,7 @@ struct axp_data;
 
 struct axp20x_adc_iio {
 	struct regmap		*regmap;
-	struct axp_data		*data;
+	const struct axp_data	*data;
 };
 
 enum axp20x_adc_channel_v {
@@ -670,15 +670,15 @@ static int axp20x_probe(struct platform_device *pdev)
 	info->regmap = axp20x_dev->regmap;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	if (!pdev->dev.of_node) {
+	if (!dev_fwnode(&pdev->dev)) {
 		const struct platform_device_id *id;
 
 		id = platform_get_device_id(pdev);
-		info->data = (struct axp_data *)id->driver_data;
+		info->data = (const struct axp_data *)id->driver_data;
 	} else {
 		struct device *dev = &pdev->dev;
 
-		info->data = (struct axp_data *)of_device_get_match_data(dev);
+		info->data = device_get_match_data(dev);
 	}
 
 	indio_dev->name = platform_get_device_id(pdev)->name;
@@ -742,7 +742,7 @@ static int axp20x_remove(struct platform_device *pdev)
 static struct platform_driver axp20x_adc_driver = {
 	.driver = {
 		.name = "axp20x-adc",
-		.of_match_table = of_match_ptr(axp20x_adc_of_match),
+		.of_match_table = axp20x_adc_of_match,
 	},
 	.id_table = axp20x_adc_id_match,
 	.probe = axp20x_probe,
-- 
2.27.0

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

[Jonathan Cameron]

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(-)

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

[PATCH v2 3/5] iio:adc:ti-adc108s102: Drop CONFIG_OF and of_match_ptr protections

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

I'm trying to clean this (now) anti-pattern out of IIO to avoid
cut and paste into new drivers.

Also add an include of mod_devicetable.h as the driver directly uses
struct of_device_id which is defined in there.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
---

v1->v2
* Drop reference to PRP0001 etc in this one as it has valid ACPI
  IDs.

drivers/iio/adc/ti-adc108s102.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ti-adc108s102.c b/drivers/iio/adc/ti-adc108s102.c
index 9b9b27415c93..183b2245e89b 100644
--- a/drivers/iio/adc/ti-adc108s102.c
+++ b/drivers/iio/adc/ti-adc108s102.c
@@ -20,6 +20,7 @@
 #include <linux/iio/trigger_consumer.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
+#include <linux/mod_devicetable.h>
 #include <linux/property.h>
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
@@ -299,13 +300,11 @@ static int adc108s102_remove(struct spi_device *spi)
 	return 0;
 }
 
-#ifdef CONFIG_OF
 static const struct of_device_id adc108s102_of_match[] = {
 	{ .compatible = "ti,adc108s102" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, adc108s102_of_match);
-#endif
 
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id adc108s102_acpi_ids[] = {
@@ -324,7 +323,7 @@ MODULE_DEVICE_TABLE(spi, adc108s102_id);
 static struct spi_driver adc108s102_driver = {
 	.driver = {
 		.name   = "adc108s102",
-		.of_match_table = of_match_ptr(adc108s102_of_match),
+		.of_match_table = adc108s102_of_match,
 		.acpi_match_table = ACPI_PTR(adc108s102_acpi_ids),
 	},
 	.probe		= adc108s102_probe,
-- 
2.27.0

[PATCH v2 4/5] iio:adc:ti-adc128s052: drop of_match_ptr protection

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

There is no real advantage in having these protections and
for parts that do not have an explicit ACPI ID, it prevents the
use of PRP0001. I'm trying to clear this out of IIO in general
to avoid copying in new drivers.

Include mod_devicetable.h as we are using of_device_id in here so
including that header is best practice.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Angelo Compagnucci <angelo.compagnucci@gmail.com>
---
 v1->v2
 * Mention that some parts of valid IDs, so we are interesting in
   enabling PRP0001 option for those that don't.
   
 drivers/iio/adc/ti-adc128s052.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
index e86f55ce093f..3143f35a6509 100644
--- a/drivers/iio/adc/ti-adc128s052.c
+++ b/drivers/iio/adc/ti-adc128s052.c
@@ -13,6 +13,7 @@
 #include <linux/err.h>
 #include <linux/spi/spi.h>
 #include <linux/module.h>
+#include <linux/mod_devicetable.h>
 #include <linux/iio/iio.h>
 #include <linux/property.h>
 #include <linux/regulator/consumer.h>
@@ -220,7 +221,7 @@ MODULE_DEVICE_TABLE(acpi, adc128_acpi_match);
 static struct spi_driver adc128_driver = {
 	.driver = {
 		.name = "adc128s052",
-		.of_match_table = of_match_ptr(adc128_of_match),
+		.of_match_table = adc128_of_match,
 		.acpi_match_table = ACPI_PTR(adc128_acpi_match),
 	},
 	.probe = adc128_probe,
-- 
2.27.0

[PATCH v2 5/5] iio:adc:bcm_iproc: Drop of_match_ptr protection and switch to mod_devicetable.h

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This driver cannot be instantiated from ACPI due to it's use of
syscon_regmap_lookup_by_phandle() but in the interests of clearing
this anti pattern out of IIO, let us switch to an explicit
check in kconfig and remove the protections on the of_match_table
The switch of header is because we only use of_device_id
in here and that is defined in mod_devicetable.h not of.h.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Raveendra Padasalagi <raveendra.padasalagi@broadcom.com>
---
 * Use Kconfig change to make it explicit you can build the driver
   without OF, but it won't do anything terribly useful.
 drivers/iio/adc/Kconfig         | 2 +-
 drivers/iio/adc/bcm_iproc_adc.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 66d9cc073157..f495d01a79b9 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -340,7 +340,7 @@ config AXP288_ADC
 
 config BCM_IPROC_ADC
 	tristate "Broadcom IPROC ADC driver"
-	depends on ARCH_BCM_IPROC || COMPILE_TEST
+	depends on (ARCH_BCM_IPROC && OF) || COMPILE_TEST
 	depends on MFD_SYSCON
 	default ARCH_BCM_CYGNUS
 	help
diff --git a/drivers/iio/adc/bcm_iproc_adc.c b/drivers/iio/adc/bcm_iproc_adc.c
index 936da32faa9d..44e1e53ada72 100644
--- a/drivers/iio/adc/bcm_iproc_adc.c
+++ b/drivers/iio/adc/bcm_iproc_adc.c
@@ -4,7 +4,7 @@
  */
 
 #include <linux/module.h>
-#include <linux/of.h>
+#include <linux/mod_devicetable.h>
 #include <linux/io.h>
 #include <linux/clk.h>
 #include <linux/mfd/syscon.h>
@@ -617,7 +617,7 @@ static struct platform_driver iproc_adc_driver = {
 	.remove	= iproc_adc_remove,
 	.driver	= {
 		.name	= "iproc-static-adc",
-		.of_match_table = of_match_ptr(iproc_adc_of_match),
+		.of_match_table = iproc_adc_of_match,
 	},
 };
 module_platform_driver(iproc_adc_driver);
-- 
2.27.0

Re: [PATCH v2 1/5] iio:adc:axp20x: Convert from OF to generic fw / device properties

[Andy Shevchenko]

On Tue, Jul 21, 2020 at 8:16 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Whilst fairly unlikely anyone will ever use this driver with anything
> other than DT, we are trying to move IIO over to the generic interfaces
> where easy to do so.
>
> In this case this involved moving to generic check on presence
> of fw_node, generic device_get_match_data and dropping the of_match_ptr

Please refer to the
- fwnode
- function()

as established practice in the documentation (and thus commit messages).

> protection.  Also relevant header changes to have property.h and
> mod_devicetable.h only.
>
> Also drop the casting away of a const in favour of retaining
> the const throughout.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Quentin Schulz <quentin.schulz@bootlin.com>
> ---
> v1->v2
>
> * Avoid the casting away of const by keeping it const throughout.
>
> drivers/iio/adc/axp20x_adc.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
> index 798ff2d89691..3e0c0233b431 100644
> --- a/drivers/iio/adc/axp20x_adc.c
> +++ b/drivers/iio/adc/axp20x_adc.c
> @@ -9,10 +9,10 @@
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> -#include <linux/of.h>
> -#include <linux/of_device.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/thermal.h>
>
> @@ -67,7 +67,7 @@ struct axp_data;
>
>  struct axp20x_adc_iio {
>         struct regmap           *regmap;
> -       struct axp_data         *data;
> +       const struct axp_data   *data;
>  };
>
>  enum axp20x_adc_channel_v {
> @@ -670,15 +670,15 @@ static int axp20x_probe(struct platform_device *pdev)
>         info->regmap = axp20x_dev->regmap;
>         indio_dev->modes = INDIO_DIRECT_MODE;
>
> -       if (!pdev->dev.of_node) {

> +       if (!dev_fwnode(&pdev->dev)) {

After the below change I think you may use positive conditional.

>                 const struct platform_device_id *id;
>
>                 id = platform_get_device_id(pdev);
> -               info->data = (struct axp_data *)id->driver_data;
> +               info->data = (const struct axp_data *)id->driver_data;
>         } else {
>                 struct device *dev = &pdev->dev;

This is probably good to have for the entire ->probe(). Or drop it
here completely.

> -               info->data = (struct axp_data *)of_device_get_match_data(dev);
> +               info->data = device_get_match_data(dev);
>         }
>
>         indio_dev->name = platform_get_device_id(pdev)->name;
> @@ -742,7 +742,7 @@ static int axp20x_remove(struct platform_device *pdev)
>  static struct platform_driver axp20x_adc_driver = {
>         .driver = {
>                 .name = "axp20x-adc",
> -               .of_match_table = of_match_ptr(axp20x_adc_of_match),
> +               .of_match_table = axp20x_adc_of_match,
>         },
>         .id_table = axp20x_adc_id_match,
>         .probe = axp20x_probe,
> --
> 2.27.0
>


-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

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
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/5] iio:adc:ti-adc108s102: Drop CONFIG_OF and of_match_ptr protections

[Andy Shevchenko]

On Tue, Jul 21, 2020 at 8:16 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> I'm trying to clean this (now) anti-pattern out of IIO to avoid
> cut and paste into new drivers.
>
> Also add an include of mod_devicetable.h as the driver directly uses
> struct of_device_id which is defined in there.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>
> v1->v2
> * Drop reference to PRP0001 etc in this one as it has valid ACPI
>   IDs.
>
> drivers/iio/adc/ti-adc108s102.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ti-adc108s102.c b/drivers/iio/adc/ti-adc108s102.c
> index 9b9b27415c93..183b2245e89b 100644
> --- a/drivers/iio/adc/ti-adc108s102.c
> +++ b/drivers/iio/adc/ti-adc108s102.c
> @@ -20,6 +20,7 @@
>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/interrupt.h>
>  #include <linux/module.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/property.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/spi/spi.h>
> @@ -299,13 +300,11 @@ static int adc108s102_remove(struct spi_device *spi)
>         return 0;
>  }
>
> -#ifdef CONFIG_OF
>  static const struct of_device_id adc108s102_of_match[] = {
>         { .compatible = "ti,adc108s102" },
>         { }
>  };
>  MODULE_DEVICE_TABLE(of, adc108s102_of_match);
> -#endif
>
>  #ifdef CONFIG_ACPI
>  static const struct acpi_device_id adc108s102_acpi_ids[] = {
> @@ -324,7 +323,7 @@ MODULE_DEVICE_TABLE(spi, adc108s102_id);
>  static struct spi_driver adc108s102_driver = {
>         .driver = {
>                 .name   = "adc108s102",
> -               .of_match_table = of_match_ptr(adc108s102_of_match),
> +               .of_match_table = adc108s102_of_match,
>                 .acpi_match_table = ACPI_PTR(adc108s102_acpi_ids),
>         },
>         .probe          = adc108s102_probe,
> --
> 2.27.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 4/5] iio:adc:ti-adc128s052: drop of_match_ptr protection

[Andy Shevchenko]

On Tue, Jul 21, 2020 at 8:16 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> There is no real advantage in having these protections and
> for parts that do not have an explicit ACPI ID, it prevents the
> use of PRP0001. I'm trying to clear this out of IIO in general
> to avoid copying in new drivers.
>
> Include mod_devicetable.h as we are using of_device_id in here so
> including that header is best practice.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Angelo Compagnucci <angelo.compagnucci@gmail.com>
> ---
>  v1->v2
>  * Mention that some parts of valid IDs, so we are interesting in
>    enabling PRP0001 option for those that don't.
>
>  drivers/iio/adc/ti-adc128s052.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
> index e86f55ce093f..3143f35a6509 100644
> --- a/drivers/iio/adc/ti-adc128s052.c
> +++ b/drivers/iio/adc/ti-adc128s052.c
> @@ -13,6 +13,7 @@
>  #include <linux/err.h>
>  #include <linux/spi/spi.h>
>  #include <linux/module.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/iio/iio.h>
>  #include <linux/property.h>
>  #include <linux/regulator/consumer.h>
> @@ -220,7 +221,7 @@ MODULE_DEVICE_TABLE(acpi, adc128_acpi_match);
>  static struct spi_driver adc128_driver = {
>         .driver = {
>                 .name = "adc128s052",
> -               .of_match_table = of_match_ptr(adc128_of_match),
> +               .of_match_table = adc128_of_match,
>                 .acpi_match_table = ACPI_PTR(adc128_acpi_match),
>         },
>         .probe = adc128_probe,
> --
> 2.27.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 5/5] iio:adc:bcm_iproc: Drop of_match_ptr protection and switch to mod_devicetable.h

[Andy Shevchenko]

On Tue, Jul 21, 2020 at 8:16 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> This driver cannot be instantiated from ACPI due to it's use of
> syscon_regmap_lookup_by_phandle() but in the interests of clearing
> this anti pattern out of IIO, let us switch to an explicit
> check in kconfig and remove the protections on the of_match_table

Kconfig

> The switch of header is because we only use of_device_id
> in here and that is defined in mod_devicetable.h not of.h.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Raveendra Padasalagi <raveendra.padasalagi@broadcom.com>
> ---
>  * Use Kconfig change to make it explicit you can build the driver
>    without OF, but it won't do anything terribly useful.
>  drivers/iio/adc/Kconfig         | 2 +-
>  drivers/iio/adc/bcm_iproc_adc.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 66d9cc073157..f495d01a79b9 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -340,7 +340,7 @@ config AXP288_ADC
>
>  config BCM_IPROC_ADC
>         tristate "Broadcom IPROC ADC driver"
> -       depends on ARCH_BCM_IPROC || COMPILE_TEST
> +       depends on (ARCH_BCM_IPROC && OF) || COMPILE_TEST
>         depends on MFD_SYSCON
>         default ARCH_BCM_CYGNUS
>         help
> diff --git a/drivers/iio/adc/bcm_iproc_adc.c b/drivers/iio/adc/bcm_iproc_adc.c
> index 936da32faa9d..44e1e53ada72 100644
> --- a/drivers/iio/adc/bcm_iproc_adc.c
> +++ b/drivers/iio/adc/bcm_iproc_adc.c
> @@ -4,7 +4,7 @@
>   */
>
>  #include <linux/module.h>
> -#include <linux/of.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/io.h>
>  #include <linux/clk.h>
>  #include <linux/mfd/syscon.h>
> @@ -617,7 +617,7 @@ static struct platform_driver iproc_adc_driver = {
>         .remove = iproc_adc_remove,
>         .driver = {
>                 .name   = "iproc-static-adc",
> -               .of_match_table = of_match_ptr(iproc_adc_of_match),
> +               .of_match_table = iproc_adc_of_match,
>         },
>  };
>  module_platform_driver(iproc_adc_driver);
> --
> 2.27.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 5/5] iio:adc:bcm_iproc: Drop of_match_ptr protection and switch to mod_devicetable.h

[Jonathan Cameron]

On Tue, 21 Jul 2020 21:34:48 +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>
> >
> > This driver cannot be instantiated from ACPI due to it's use of
> > syscon_regmap_lookup_by_phandle() but in the interests of clearing
> > this anti pattern out of IIO, let us switch to an explicit
> > check in kconfig and remove the protections on the of_match_table  
> 
> Kconfig
> 
> > The switch of header is because we only use of_device_id
> > in here and that is defined in mod_devicetable.h not of.h.  
> 
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to poke at it.

Thanks,

Jonathan

> 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Raveendra Padasalagi <raveendra.padasalagi@broadcom.com>
> > ---
> >  * Use Kconfig change to make it explicit you can build the driver
> >    without OF, but it won't do anything terribly useful.
> >  drivers/iio/adc/Kconfig         | 2 +-
> >  drivers/iio/adc/bcm_iproc_adc.c | 4 ++--
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 66d9cc073157..f495d01a79b9 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -340,7 +340,7 @@ config AXP288_ADC
> >
> >  config BCM_IPROC_ADC
> >         tristate "Broadcom IPROC ADC driver"
> > -       depends on ARCH_BCM_IPROC || COMPILE_TEST
> > +       depends on (ARCH_BCM_IPROC && OF) || COMPILE_TEST
> >         depends on MFD_SYSCON
> >         default ARCH_BCM_CYGNUS
> >         help
> > diff --git a/drivers/iio/adc/bcm_iproc_adc.c b/drivers/iio/adc/bcm_iproc_adc.c
> > index 936da32faa9d..44e1e53ada72 100644
> > --- a/drivers/iio/adc/bcm_iproc_adc.c
> > +++ b/drivers/iio/adc/bcm_iproc_adc.c
> > @@ -4,7 +4,7 @@
> >   */
> >
> >  #include <linux/module.h>
> > -#include <linux/of.h>
> > +#include <linux/mod_devicetable.h>
> >  #include <linux/io.h>
> >  #include <linux/clk.h>
> >  #include <linux/mfd/syscon.h>
> > @@ -617,7 +617,7 @@ static struct platform_driver iproc_adc_driver = {
> >         .remove = iproc_adc_remove,
> >         .driver = {
> >                 .name   = "iproc-static-adc",
> > -               .of_match_table = of_match_ptr(iproc_adc_of_match),
> > +               .of_match_table = iproc_adc_of_match,
> >         },
> >  };
> >  module_platform_driver(iproc_adc_driver);
> > --
> > 2.27.0
> >  
> 
> 

Re: [PATCH v2 4/5] iio:adc:ti-adc128s052: drop of_match_ptr protection

[Jonathan Cameron]

On Tue, 21 Jul 2020 21:32:03 +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>
> >
> > There is no real advantage in having these protections and
> > for parts that do not have an explicit ACPI ID, it prevents the
> > use of PRP0001. I'm trying to clear this out of IIO in general
> > to avoid copying in new drivers.
> >
> > Include mod_devicetable.h as we are using of_device_id in here so
> > including that header is best practice.  
> 
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Applied,

Thanks,

Jonathan

> 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Angelo Compagnucci <angelo.compagnucci@gmail.com>
> > ---
> >  v1->v2
> >  * Mention that some parts of valid IDs, so we are interesting in
> >    enabling PRP0001 option for those that don't.
> >
> >  drivers/iio/adc/ti-adc128s052.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
> > index e86f55ce093f..3143f35a6509 100644
> > --- a/drivers/iio/adc/ti-adc128s052.c
> > +++ b/drivers/iio/adc/ti-adc128s052.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/err.h>
> >  #include <linux/spi/spi.h>
> >  #include <linux/module.h>
> > +#include <linux/mod_devicetable.h>
> >  #include <linux/iio/iio.h>
> >  #include <linux/property.h>
> >  #include <linux/regulator/consumer.h>
> > @@ -220,7 +221,7 @@ MODULE_DEVICE_TABLE(acpi, adc128_acpi_match);
> >  static struct spi_driver adc128_driver = {
> >         .driver = {
> >                 .name = "adc128s052",
> > -               .of_match_table = of_match_ptr(adc128_of_match),
> > +               .of_match_table = adc128_of_match,
> >                 .acpi_match_table = ACPI_PTR(adc128_acpi_match),
> >         },
> >         .probe = adc128_probe,
> > --
> > 2.27.0
> >  
> 
> 

Re: [PATCH v2 3/5] iio:adc:ti-adc108s102: Drop CONFIG_OF and of_match_ptr protections

[Jonathan Cameron]

On Tue, 21 Jul 2020 21:31:36 +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>
> >
> > I'm trying to clean this (now) anti-pattern out of IIO to avoid
> > cut and paste into new drivers.
> >
> > Also add an include of mod_devicetable.h as the driver directly uses
> > struct of_device_id which is defined in there.  
> 
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Applied,

Thanks,

J
> 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Jan Kiszka <jan.kiszka@siemens.com>
> > ---
> >
> > v1->v2
> > * Drop reference to PRP0001 etc in this one as it has valid ACPI
> >   IDs.
> >
> > drivers/iio/adc/ti-adc108s102.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ti-adc108s102.c b/drivers/iio/adc/ti-adc108s102.c
> > index 9b9b27415c93..183b2245e89b 100644
> > --- a/drivers/iio/adc/ti-adc108s102.c
> > +++ b/drivers/iio/adc/ti-adc108s102.c
> > @@ -20,6 +20,7 @@
> >  #include <linux/iio/trigger_consumer.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/module.h>
> > +#include <linux/mod_devicetable.h>
> >  #include <linux/property.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/spi/spi.h>
> > @@ -299,13 +300,11 @@ static int adc108s102_remove(struct spi_device *spi)
> >         return 0;
> >  }
> >
> > -#ifdef CONFIG_OF
> >  static const struct of_device_id adc108s102_of_match[] = {
> >         { .compatible = "ti,adc108s102" },
> >         { }
> >  };
> >  MODULE_DEVICE_TABLE(of, adc108s102_of_match);
> > -#endif
> >
> >  #ifdef CONFIG_ACPI
> >  static const struct acpi_device_id adc108s102_acpi_ids[] = {
> > @@ -324,7 +323,7 @@ MODULE_DEVICE_TABLE(spi, adc108s102_id);
> >  static struct spi_driver adc108s102_driver = {
> >         .driver = {
> >                 .name   = "adc108s102",
> > -               .of_match_table = of_match_ptr(adc108s102_of_match),
> > +               .of_match_table = adc108s102_of_match,
> >                 .acpi_match_table = ACPI_PTR(adc108s102_acpi_ids),
> >         },
> >         .probe          = adc108s102_probe,
> > --
> > 2.27.0
> >  
> 
> 

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

[Jonathan Cameron]

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
> >  
> 
> 

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

[Jonathan Cameron]

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,

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

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko