All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers
@ 2020-04-19 15:01 jic23
  2020-04-19 15:02 ` [PATCH 1/7] iio: light: bh1780: use mod_devicetable.h and drop of_match_ptr macro jic23
                   ` (7 more replies)
  0 siblings, 8 replies; 22+ messages in thread
From: jic23 @ 2020-04-19 15:01 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

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

Hi All,

Given we keep having to explain to people that of_match_ptr is less
than ideal now we have the option of ACPI DSDT using PRP0001 and
the compatible, it seems sensible to reduce the number of instances
that people might copy for a new driver.

Added theoretical benefit is that we can probe all these drivers from
appropriate DSDT (though I doubt anyone will).

I'm sending this first set out to see if anyone has strong views against
doing this for at least the simple drivers that have no other device
tree dependence.  Obviously more work would be needed to remove
use of of_match_ptr from IIO completely.

Light sensors picked as a starting point as they tend to be simple.

I may do follow ups in larger blocks to avoid so many small patches
(or indeed flatten these into one when applying)

Thanks

Jonathan

Jonathan Cameron (7):
  iio: light: bh1780: use mod_devicetable.h and drop of_match_ptr macro
  iio: light: cm32181: Add mod_devicetable.h and remove of_match_ptr
  iio: light: cm3232: Add mod_devicetable.h include and drop
    of_match_ptr
  iio: light: gp2ap020a00f: Swap of.h for mod_devicetable.h + drop
    of_match_ptr
  iio: light: opt3001: Add mod_devicetable.h and drop use of
    of_match_ptr
  iio: light: st_uvis25: Add mod_devicetable.h and drop of_match_ptr
  iio: light: vl6180: swap of.h for mod_devicetable.h and drop
    of_match_ptr

 drivers/iio/light/bh1780.c        | 6 ++----
 drivers/iio/light/cm32181.c       | 3 ++-
 drivers/iio/light/cm3232.c        | 3 ++-
 drivers/iio/light/gp2ap020a00f.c  | 6 ++----
 drivers/iio/light/opt3001.c       | 3 ++-
 drivers/iio/light/st_uvis25_i2c.c | 3 ++-
 drivers/iio/light/st_uvis25_spi.c | 3 ++-
 drivers/iio/light/vl6180.c        | 2 +-
 8 files changed, 15 insertions(+), 14 deletions(-)

-- 
2.26.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/7] iio: light: bh1780: use mod_devicetable.h and drop of_match_ptr macro
  2020-04-19 15:01 [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers jic23
@ 2020-04-19 15:02 ` jic23
  2020-04-20  6:05   ` Ardelean, Alexandru
  2020-04-19 15:02 ` [PATCH 2/7] iio: light: cm32181: Add mod_devicetable.h and remove of_match_ptr jic23
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: jic23 @ 2020-04-19 15:02 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

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

Whilst this enables ACPI binding or the device via PRP0001 the
primary aim is to remove potential for these two things to be
cut and paste into new drivers.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/bh1780.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/light/bh1780.c b/drivers/iio/light/bh1780.c
index a8361006dcd9..03f2d8d123c4 100644
--- a/drivers/iio/light/bh1780.c
+++ b/drivers/iio/light/bh1780.c
@@ -13,7 +13,7 @@
 #include <linux/platform_device.h>
 #include <linux/delay.h>
 #include <linux/module.h>
-#include <linux/of.h>
+#include <linux/mod_devicetable.h>
 #include <linux/pm_runtime.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
@@ -273,13 +273,11 @@ static const struct i2c_device_id bh1780_id[] = {
 
 MODULE_DEVICE_TABLE(i2c, bh1780_id);
 
-#ifdef CONFIG_OF
 static const struct of_device_id of_bh1780_match[] = {
 	{ .compatible = "rohm,bh1780gli", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, of_bh1780_match);
-#endif
 
 static struct i2c_driver bh1780_driver = {
 	.probe		= bh1780_probe,
@@ -288,7 +286,7 @@ static struct i2c_driver bh1780_driver = {
 	.driver = {
 		.name = "bh1780",
 		.pm = &bh1780_dev_pm_ops,
-		.of_match_table = of_match_ptr(of_bh1780_match),
+		.of_match_table = of_bh1780_match,
 	},
 };
 
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/7] iio: light: cm32181: Add mod_devicetable.h and remove of_match_ptr
  2020-04-19 15:01 [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers jic23
  2020-04-19 15:02 ` [PATCH 1/7] iio: light: bh1780: use mod_devicetable.h and drop of_match_ptr macro jic23
@ 2020-04-19 15:02 ` jic23
  2020-04-20  6:05   ` Ardelean, Alexandru
  2020-04-19 15:02 ` [PATCH 3/7] iio: light: cm3232: Add mod_devicetable.h include and drop of_match_ptr jic23
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: jic23 @ 2020-04-19 15:02 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

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

Enables probing via the ACPI PRP0001 route but more is mosty about
removing examples of this that might get copied into new drivers.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/cm32181.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
index 5f4fb5674fa0..73c48f46220c 100644
--- a/drivers/iio/light/cm32181.c
+++ b/drivers/iio/light/cm32181.c
@@ -9,6 +9,7 @@
 #include <linux/i2c.h>
 #include <linux/mutex.h>
 #include <linux/module.h>
+#include <linux/mod_devicetable.h>
 #include <linux/interrupt.h>
 #include <linux/regulator/consumer.h>
 #include <linux/iio/iio.h>
@@ -354,7 +355,7 @@ MODULE_DEVICE_TABLE(of, cm32181_of_match);
 static struct i2c_driver cm32181_driver = {
 	.driver = {
 		.name	= "cm32181",
-		.of_match_table = of_match_ptr(cm32181_of_match),
+		.of_match_table = cm32181_of_match,
 	},
 	.id_table       = cm32181_id,
 	.probe		= cm32181_probe,
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/7] iio: light: cm3232: Add mod_devicetable.h include and drop of_match_ptr
  2020-04-19 15:01 [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers jic23
  2020-04-19 15:02 ` [PATCH 1/7] iio: light: bh1780: use mod_devicetable.h and drop of_match_ptr macro jic23
  2020-04-19 15:02 ` [PATCH 2/7] iio: light: cm32181: Add mod_devicetable.h and remove of_match_ptr jic23
@ 2020-04-19 15:02 ` jic23
  2020-04-20  6:05   ` Ardelean, Alexandru
  2020-04-19 15:02 ` [PATCH 4/7] iio: light: gp2ap020a00f: Swap of.h for mod_devicetable.h + " jic23
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: jic23 @ 2020-04-19 15:02 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

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

Enables ACPI probing via PRP0001 and removes an example that might
be cut and paste to a new driver.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/cm3232.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/light/cm3232.c b/drivers/iio/light/cm3232.c
index cd3cfb7d02bd..867200825686 100644
--- a/drivers/iio/light/cm3232.c
+++ b/drivers/iio/light/cm3232.c
@@ -10,6 +10,7 @@
 
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/mod_devicetable.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/init.h>
@@ -418,7 +419,7 @@ MODULE_DEVICE_TABLE(of, cm3232_of_match);
 static struct i2c_driver cm3232_driver = {
 	.driver = {
 		.name	= "cm3232",
-		.of_match_table = of_match_ptr(cm3232_of_match),
+		.of_match_table = cm3232_of_match,
 #ifdef CONFIG_PM_SLEEP
 		.pm	= &cm3232_pm_ops,
 #endif
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/7] iio: light: gp2ap020a00f: Swap of.h for mod_devicetable.h + drop of_match_ptr
  2020-04-19 15:01 [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers jic23
                   ` (2 preceding siblings ...)
  2020-04-19 15:02 ` [PATCH 3/7] iio: light: cm3232: Add mod_devicetable.h include and drop of_match_ptr jic23
@ 2020-04-19 15:02 ` jic23
  2020-04-20  6:06   ` Ardelean, Alexandru
  2020-04-19 15:02 ` [PATCH 5/7] iio: light: opt3001: Add mod_devicetable.h and drop use of of_match_ptr jic23
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: jic23 @ 2020-04-19 15:02 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

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

Also drops ifdef protections for CONFIG_OF.

Enables probing via ACPI PRP0001 and removes an example that might be
cut and paste into new drivers.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/gp2ap020a00f.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index 7fbbce0d4bc7..070d4cd0cf54 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -38,8 +38,8 @@
 #include <linux/irq.h>
 #include <linux/irq_work.h>
 #include <linux/module.h>
+#include <linux/mod_devicetable.h>
 #include <linux/mutex.h>
-#include <linux/of.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
@@ -1617,18 +1617,16 @@ static const struct i2c_device_id gp2ap020a00f_id[] = {
 
 MODULE_DEVICE_TABLE(i2c, gp2ap020a00f_id);
 
-#ifdef CONFIG_OF
 static const struct of_device_id gp2ap020a00f_of_match[] = {
 	{ .compatible = "sharp,gp2ap020a00f" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, gp2ap020a00f_of_match);
-#endif
 
 static struct i2c_driver gp2ap020a00f_driver = {
 	.driver = {
 		.name	= GP2A_I2C_NAME,
-		.of_match_table = of_match_ptr(gp2ap020a00f_of_match),
+		.of_match_table = gp2ap020a00f_of_match,
 	},
 	.probe		= gp2ap020a00f_probe,
 	.remove		= gp2ap020a00f_remove,
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 5/7] iio: light: opt3001: Add mod_devicetable.h and drop use of of_match_ptr
  2020-04-19 15:01 [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers jic23
                   ` (3 preceding siblings ...)
  2020-04-19 15:02 ` [PATCH 4/7] iio: light: gp2ap020a00f: Swap of.h for mod_devicetable.h + " jic23
@ 2020-04-19 15:02 ` jic23
  2020-04-20  6:06   ` Ardelean, Alexandru
  2020-04-19 15:02 ` [PATCH 6/7] iio: light: st_uvis25: Add mod_devicetable.h and drop of_match_ptr jic23
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: jic23 @ 2020-04-19 15:02 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

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

Enables probing via ACPI PRP0001 but mostly about removing examples
that might be copied to new drivers.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/opt3001.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/light/opt3001.c b/drivers/iio/light/opt3001.c
index 92004a2563ea..82abfa57b59c 100644
--- a/drivers/iio/light/opt3001.c
+++ b/drivers/iio/light/opt3001.c
@@ -16,6 +16,7 @@
 #include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/mod_devicetable.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
 #include <linux/types.h>
@@ -844,7 +845,7 @@ static struct i2c_driver opt3001_driver = {
 
 	.driver = {
 		.name = "opt3001",
-		.of_match_table = of_match_ptr(opt3001_of_match),
+		.of_match_table = opt3001_of_match,
 	},
 };
 
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 6/7] iio: light: st_uvis25: Add mod_devicetable.h and drop of_match_ptr
  2020-04-19 15:01 [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers jic23
                   ` (4 preceding siblings ...)
  2020-04-19 15:02 ` [PATCH 5/7] iio: light: opt3001: Add mod_devicetable.h and drop use of of_match_ptr jic23
@ 2020-04-19 15:02 ` jic23
  2020-04-20  6:08   ` Ardelean, Alexandru
  2020-04-19 15:02 ` [PATCH 7/7] iio: light: vl6180: swap of.h for " jic23
  2020-04-20  6:04 ` [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers Ardelean, Alexandru
  7 siblings, 1 reply; 22+ messages in thread
From: jic23 @ 2020-04-19 15:02 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

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

Enables probing via ACPI PRP0001 and removes an example that we don't
want people to cut and paste into new drivers.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/st_uvis25_i2c.c | 3 ++-
 drivers/iio/light/st_uvis25_spi.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/light/st_uvis25_i2c.c b/drivers/iio/light/st_uvis25_i2c.c
index 400724069d19..98cd49eefe45 100644
--- a/drivers/iio/light/st_uvis25_i2c.c
+++ b/drivers/iio/light/st_uvis25_i2c.c
@@ -9,6 +9,7 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/mod_devicetable.h>
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/regmap.h>
@@ -55,7 +56,7 @@ static struct i2c_driver st_uvis25_driver = {
 	.driver = {
 		.name = "st_uvis25_i2c",
 		.pm = &st_uvis25_pm_ops,
-		.of_match_table = of_match_ptr(st_uvis25_i2c_of_match),
+		.of_match_table = st_uvis25_i2c_of_match,
 	},
 	.probe = st_uvis25_i2c_probe,
 	.id_table = st_uvis25_i2c_id_table,
diff --git a/drivers/iio/light/st_uvis25_spi.c b/drivers/iio/light/st_uvis25_spi.c
index cd3761a3ee3f..af9d94d12787 100644
--- a/drivers/iio/light/st_uvis25_spi.c
+++ b/drivers/iio/light/st_uvis25_spi.c
@@ -9,6 +9,7 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/mod_devicetable.h>
 #include <linux/spi/spi.h>
 #include <linux/slab.h>
 #include <linux/regmap.h>
@@ -55,7 +56,7 @@ static struct spi_driver st_uvis25_driver = {
 	.driver = {
 		.name = "st_uvis25_spi",
 		.pm = &st_uvis25_pm_ops,
-		.of_match_table = of_match_ptr(st_uvis25_spi_of_match),
+		.of_match_table = st_uvis25_spi_of_match,
 	},
 	.probe = st_uvis25_spi_probe,
 	.id_table = st_uvis25_spi_id_table,
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 7/7] iio: light: vl6180: swap of.h for mod_devicetable.h and drop of_match_ptr
  2020-04-19 15:01 [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers jic23
                   ` (5 preceding siblings ...)
  2020-04-19 15:02 ` [PATCH 6/7] iio: light: st_uvis25: Add mod_devicetable.h and drop of_match_ptr jic23
@ 2020-04-19 15:02 ` jic23
  2020-04-20  6:19   ` Ardelean, Alexandru
  2020-04-20  6:04 ` [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers Ardelean, Alexandru
  7 siblings, 1 reply; 22+ messages in thread
From: jic23 @ 2020-04-19 15:02 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

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

Enables probing via ACPI PRP0001 route and removes an example of
an approach we no longer want people to copy.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/light/vl6180.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
index d9533a76b8f6..7e67d7b3bfb6 100644
--- a/drivers/iio/light/vl6180.c
+++ b/drivers/iio/light/vl6180.c
@@ -537,7 +537,7 @@ MODULE_DEVICE_TABLE(i2c, vl6180_id);
 static struct i2c_driver vl6180_driver = {
 	.driver = {
 		.name   = VL6180_DRV_NAME,
-		.of_match_table = of_match_ptr(vl6180_of_match),
+		.of_match_table = vl6180_of_match,
 	},
 	.probe  = vl6180_probe,
 	.id_table = vl6180_id,
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers
  2020-04-19 15:01 [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers jic23
                   ` (6 preceding siblings ...)
  2020-04-19 15:02 ` [PATCH 7/7] iio: light: vl6180: swap of.h for " jic23
@ 2020-04-20  6:04 ` Ardelean, Alexandru
  2020-04-20  6:22   ` Ardelean, Alexandru
  7 siblings, 1 reply; 22+ messages in thread
From: Ardelean, Alexandru @ 2020-04-20  6:04 UTC (permalink / raw)
  To: jic23, linux-iio; +Cc: Jonathan.Cameron

On Sun, 2020-04-19 at 16:01 +0100, jic23@kernel.org wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Hi All,
> 
> Given we keep having to explain to people that of_match_ptr is less
> than ideal now we have the option of ACPI DSDT using PRP0001 and
> the compatible, it seems sensible to reduce the number of instances
> that people might copy for a new driver.
> 
> Added theoretical benefit is that we can probe all these drivers from
> appropriate DSDT (though I doubt anyone will).
> 
> I'm sending this first set out to see if anyone has strong views against
> doing this for at least the simple drivers that have no other device
> tree dependence.  Obviously more work would be needed to remove
> use of of_match_ptr from IIO completely.
> 
> Light sensors picked as a starting point as they tend to be simple.
> 
> I may do follow ups in larger blocks to avoid so many small patches
> (or indeed flatten these into one when applying)

fwiw: i was also planning to do a sweep of these;
well, tbh, the main intent was to target ADI drivers first and do a bigger
conversion for them to make the slightly friendlier with ACPI; 

aside from this, i'm also noticing some bad patterns being copied from older
drivers, when asking new people to write Linux drivers;
i did not make a list, probably should have;
one is 'mlock' [still] being copied; and accessing other internal fields;
but the internal fields accessing requires a bit of a cleanup in the form of
privatizing the fields somehow;


> 
> Thanks
> 
> Jonathan
> 
> Jonathan Cameron (7):
>   iio: light: bh1780: use mod_devicetable.h and drop of_match_ptr macro
>   iio: light: cm32181: Add mod_devicetable.h and remove of_match_ptr
>   iio: light: cm3232: Add mod_devicetable.h include and drop
>     of_match_ptr
>   iio: light: gp2ap020a00f: Swap of.h for mod_devicetable.h + drop
>     of_match_ptr
>   iio: light: opt3001: Add mod_devicetable.h and drop use of
>     of_match_ptr
>   iio: light: st_uvis25: Add mod_devicetable.h and drop of_match_ptr
>   iio: light: vl6180: swap of.h for mod_devicetable.h and drop
>     of_match_ptr
> 
>  drivers/iio/light/bh1780.c        | 6 ++----
>  drivers/iio/light/cm32181.c       | 3 ++-
>  drivers/iio/light/cm3232.c        | 3 ++-
>  drivers/iio/light/gp2ap020a00f.c  | 6 ++----
>  drivers/iio/light/opt3001.c       | 3 ++-
>  drivers/iio/light/st_uvis25_i2c.c | 3 ++-
>  drivers/iio/light/st_uvis25_spi.c | 3 ++-
>  drivers/iio/light/vl6180.c        | 2 +-
>  8 files changed, 15 insertions(+), 14 deletions(-)
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/7] iio: light: bh1780: use mod_devicetable.h and drop of_match_ptr macro
  2020-04-19 15:02 ` [PATCH 1/7] iio: light: bh1780: use mod_devicetable.h and drop of_match_ptr macro jic23
@ 2020-04-20  6:05   ` Ardelean, Alexandru
  0 siblings, 0 replies; 22+ messages in thread
From: Ardelean, Alexandru @ 2020-04-20  6:05 UTC (permalink / raw)
  To: jic23, linux-iio; +Cc: Jonathan.Cameron

On Sun, 2020-04-19 at 16:02 +0100, jic23@kernel.org wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Whilst this enables ACPI binding or the device via PRP0001 the
> primary aim is to remove potential for these two things to be
> cut and paste into new drivers.

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/light/bh1780.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/light/bh1780.c b/drivers/iio/light/bh1780.c
> index a8361006dcd9..03f2d8d123c4 100644
> --- a/drivers/iio/light/bh1780.c
> +++ b/drivers/iio/light/bh1780.c
> @@ -13,7 +13,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/delay.h>
>  #include <linux/module.h>
> -#include <linux/of.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> @@ -273,13 +273,11 @@ static const struct i2c_device_id bh1780_id[] = {
>  
>  MODULE_DEVICE_TABLE(i2c, bh1780_id);
>  
> -#ifdef CONFIG_OF
>  static const struct of_device_id of_bh1780_match[] = {
>  	{ .compatible = "rohm,bh1780gli", },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, of_bh1780_match);
> -#endif
>  
>  static struct i2c_driver bh1780_driver = {
>  	.probe		= bh1780_probe,
> @@ -288,7 +286,7 @@ static struct i2c_driver bh1780_driver = {
>  	.driver = {
>  		.name = "bh1780",
>  		.pm = &bh1780_dev_pm_ops,
> -		.of_match_table = of_match_ptr(of_bh1780_match),
> +		.of_match_table = of_bh1780_match,
>  	},
>  };
>  

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/7] iio: light: cm32181: Add mod_devicetable.h and remove of_match_ptr
  2020-04-19 15:02 ` [PATCH 2/7] iio: light: cm32181: Add mod_devicetable.h and remove of_match_ptr jic23
@ 2020-04-20  6:05   ` Ardelean, Alexandru
  0 siblings, 0 replies; 22+ messages in thread
From: Ardelean, Alexandru @ 2020-04-20  6:05 UTC (permalink / raw)
  To: jic23, linux-iio; +Cc: Jonathan.Cameron

On Sun, 2020-04-19 at 16:02 +0100, jic23@kernel.org wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Enables probing via the ACPI PRP0001 route but more is mosty about
> removing examples of this that might get copied into new drivers.
> 

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/light/cm32181.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
> index 5f4fb5674fa0..73c48f46220c 100644
> --- a/drivers/iio/light/cm32181.c
> +++ b/drivers/iio/light/cm32181.c
> @@ -9,6 +9,7 @@
>  #include <linux/i2c.h>
>  #include <linux/mutex.h>
>  #include <linux/module.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/interrupt.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/iio/iio.h>
> @@ -354,7 +355,7 @@ MODULE_DEVICE_TABLE(of, cm32181_of_match);
>  static struct i2c_driver cm32181_driver = {
>  	.driver = {
>  		.name	= "cm32181",
> -		.of_match_table = of_match_ptr(cm32181_of_match),
> +		.of_match_table = cm32181_of_match,
>  	},
>  	.id_table       = cm32181_id,
>  	.probe		= cm32181_probe,

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/7] iio: light: cm3232: Add mod_devicetable.h include and drop of_match_ptr
  2020-04-19 15:02 ` [PATCH 3/7] iio: light: cm3232: Add mod_devicetable.h include and drop of_match_ptr jic23
@ 2020-04-20  6:05   ` Ardelean, Alexandru
  0 siblings, 0 replies; 22+ messages in thread
From: Ardelean, Alexandru @ 2020-04-20  6:05 UTC (permalink / raw)
  To: jic23, linux-iio; +Cc: Jonathan.Cameron

On Sun, 2020-04-19 at 16:02 +0100, jic23@kernel.org wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Enables ACPI probing via PRP0001 and removes an example that might
> be cut and paste to a new driver.
> 

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/light/cm3232.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/cm3232.c b/drivers/iio/light/cm3232.c
> index cd3cfb7d02bd..867200825686 100644
> --- a/drivers/iio/light/cm3232.c
> +++ b/drivers/iio/light/cm3232.c
> @@ -10,6 +10,7 @@
>  
>  #include <linux/i2c.h>
>  #include <linux/module.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  #include <linux/init.h>
> @@ -418,7 +419,7 @@ MODULE_DEVICE_TABLE(of, cm3232_of_match);
>  static struct i2c_driver cm3232_driver = {
>  	.driver = {
>  		.name	= "cm3232",
> -		.of_match_table = of_match_ptr(cm3232_of_match),
> +		.of_match_table = cm3232_of_match,
>  #ifdef CONFIG_PM_SLEEP
>  		.pm	= &cm3232_pm_ops,
>  #endif

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 4/7] iio: light: gp2ap020a00f: Swap of.h for mod_devicetable.h + drop of_match_ptr
  2020-04-19 15:02 ` [PATCH 4/7] iio: light: gp2ap020a00f: Swap of.h for mod_devicetable.h + " jic23
@ 2020-04-20  6:06   ` Ardelean, Alexandru
  0 siblings, 0 replies; 22+ messages in thread
From: Ardelean, Alexandru @ 2020-04-20  6:06 UTC (permalink / raw)
  To: jic23, linux-iio; +Cc: Jonathan.Cameron

On Sun, 2020-04-19 at 16:02 +0100, jic23@kernel.org wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Also drops ifdef protections for CONFIG_OF.
> 
> Enables probing via ACPI PRP0001 and removes an example that might be
> cut and paste into new drivers.
> 

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/light/gp2ap020a00f.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/light/gp2ap020a00f.c
> b/drivers/iio/light/gp2ap020a00f.c
> index 7fbbce0d4bc7..070d4cd0cf54 100644
> --- a/drivers/iio/light/gp2ap020a00f.c
> +++ b/drivers/iio/light/gp2ap020a00f.c
> @@ -38,8 +38,8 @@
>  #include <linux/irq.h>
>  #include <linux/irq_work.h>
>  #include <linux/module.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/mutex.h>
> -#include <linux/of.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
> @@ -1617,18 +1617,16 @@ static const struct i2c_device_id gp2ap020a00f_id[] =
> {
>  
>  MODULE_DEVICE_TABLE(i2c, gp2ap020a00f_id);
>  
> -#ifdef CONFIG_OF
>  static const struct of_device_id gp2ap020a00f_of_match[] = {
>  	{ .compatible = "sharp,gp2ap020a00f" },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, gp2ap020a00f_of_match);
> -#endif
>  
>  static struct i2c_driver gp2ap020a00f_driver = {
>  	.driver = {
>  		.name	= GP2A_I2C_NAME,
> -		.of_match_table = of_match_ptr(gp2ap020a00f_of_match),
> +		.of_match_table = gp2ap020a00f_of_match,
>  	},
>  	.probe		= gp2ap020a00f_probe,
>  	.remove		= gp2ap020a00f_remove,

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/7] iio: light: opt3001: Add mod_devicetable.h and drop use of of_match_ptr
  2020-04-19 15:02 ` [PATCH 5/7] iio: light: opt3001: Add mod_devicetable.h and drop use of of_match_ptr jic23
@ 2020-04-20  6:06   ` Ardelean, Alexandru
  0 siblings, 0 replies; 22+ messages in thread
From: Ardelean, Alexandru @ 2020-04-20  6:06 UTC (permalink / raw)
  To: jic23, linux-iio; +Cc: Jonathan.Cameron

On Sun, 2020-04-19 at 16:02 +0100, jic23@kernel.org wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Enables probing via ACPI PRP0001 but mostly about removing examples
> that might be copied to new drivers.
> 

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/light/opt3001.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/opt3001.c b/drivers/iio/light/opt3001.c
> index 92004a2563ea..82abfa57b59c 100644
> --- a/drivers/iio/light/opt3001.c
> +++ b/drivers/iio/light/opt3001.c
> @@ -16,6 +16,7 @@
>  #include <linux/irq.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/mutex.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
> @@ -844,7 +845,7 @@ static struct i2c_driver opt3001_driver = {
>  
>  	.driver = {
>  		.name = "opt3001",
> -		.of_match_table = of_match_ptr(opt3001_of_match),
> +		.of_match_table = opt3001_of_match,
>  	},
>  };
>  

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 6/7] iio: light: st_uvis25: Add mod_devicetable.h and drop of_match_ptr
  2020-04-19 15:02 ` [PATCH 6/7] iio: light: st_uvis25: Add mod_devicetable.h and drop of_match_ptr jic23
@ 2020-04-20  6:08   ` Ardelean, Alexandru
  0 siblings, 0 replies; 22+ messages in thread
From: Ardelean, Alexandru @ 2020-04-20  6:08 UTC (permalink / raw)
  To: jic23, linux-iio; +Cc: Jonathan.Cameron

On Sun, 2020-04-19 at 16:02 +0100, jic23@kernel.org wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Enables probing via ACPI PRP0001 and removes an example that we don't
> want people to cut and paste into new drivers.
> 

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/light/st_uvis25_i2c.c | 3 ++-
>  drivers/iio/light/st_uvis25_spi.c | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/light/st_uvis25_i2c.c
> b/drivers/iio/light/st_uvis25_i2c.c
> index 400724069d19..98cd49eefe45 100644
> --- a/drivers/iio/light/st_uvis25_i2c.c
> +++ b/drivers/iio/light/st_uvis25_i2c.c
> @@ -9,6 +9,7 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/i2c.h>
>  #include <linux/slab.h>
>  #include <linux/regmap.h>
> @@ -55,7 +56,7 @@ static struct i2c_driver st_uvis25_driver = {
>  	.driver = {
>  		.name = "st_uvis25_i2c",
>  		.pm = &st_uvis25_pm_ops,
> -		.of_match_table = of_match_ptr(st_uvis25_i2c_of_match),
> +		.of_match_table = st_uvis25_i2c_of_match,
>  	},
>  	.probe = st_uvis25_i2c_probe,
>  	.id_table = st_uvis25_i2c_id_table,
> diff --git a/drivers/iio/light/st_uvis25_spi.c
> b/drivers/iio/light/st_uvis25_spi.c
> index cd3761a3ee3f..af9d94d12787 100644
> --- a/drivers/iio/light/st_uvis25_spi.c
> +++ b/drivers/iio/light/st_uvis25_spi.c
> @@ -9,6 +9,7 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/spi/spi.h>
>  #include <linux/slab.h>
>  #include <linux/regmap.h>
> @@ -55,7 +56,7 @@ static struct spi_driver st_uvis25_driver = {
>  	.driver = {
>  		.name = "st_uvis25_spi",
>  		.pm = &st_uvis25_pm_ops,
> -		.of_match_table = of_match_ptr(st_uvis25_spi_of_match),
> +		.of_match_table = st_uvis25_spi_of_match,
>  	},
>  	.probe = st_uvis25_spi_probe,
>  	.id_table = st_uvis25_spi_id_table,

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 7/7] iio: light: vl6180: swap of.h for mod_devicetable.h and drop of_match_ptr
  2020-04-19 15:02 ` [PATCH 7/7] iio: light: vl6180: swap of.h for " jic23
@ 2020-04-20  6:19   ` Ardelean, Alexandru
  2020-04-20 15:45     ` Jonathan Cameron
  0 siblings, 1 reply; 22+ messages in thread
From: Ardelean, Alexandru @ 2020-04-20  6:19 UTC (permalink / raw)
  To: jic23, linux-iio; +Cc: Jonathan.Cameron

On Sun, 2020-04-19 at 16:02 +0100, jic23@kernel.org wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Enables probing via ACPI PRP0001 route and removes an example of
> an approach we no longer want people to copy.
> 

This doesn't include 'linux/mod_devicetable.h'.
I'm wondering now if it is needed at all.
Should we remove it from the rest?

With that addressed:

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/light/vl6180.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
> index d9533a76b8f6..7e67d7b3bfb6 100644
> --- a/drivers/iio/light/vl6180.c
> +++ b/drivers/iio/light/vl6180.c
> @@ -537,7 +537,7 @@ MODULE_DEVICE_TABLE(i2c, vl6180_id);
>  static struct i2c_driver vl6180_driver = {
>  	.driver = {
>  		.name   = VL6180_DRV_NAME,
> -		.of_match_table = of_match_ptr(vl6180_of_match),
> +		.of_match_table = vl6180_of_match,
>  	},
>  	.probe  = vl6180_probe,
>  	.id_table = vl6180_id,

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers
  2020-04-20  6:04 ` [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers Ardelean, Alexandru
@ 2020-04-20  6:22   ` Ardelean, Alexandru
  2020-04-20 15:48     ` Jonathan Cameron
  0 siblings, 1 reply; 22+ messages in thread
From: Ardelean, Alexandru @ 2020-04-20  6:22 UTC (permalink / raw)
  To: jic23, linux-iio; +Cc: Jonathan.Cameron

On Mon, 2020-04-20 at 06:04 +0000, Ardelean, Alexandru wrote:
> [External]
> 
> On Sun, 2020-04-19 at 16:01 +0100, jic23@kernel.org wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > Hi All,
> > 
> > Given we keep having to explain to people that of_match_ptr is less
> > than ideal now we have the option of ACPI DSDT using PRP0001 and
> > the compatible, it seems sensible to reduce the number of instances
> > that people might copy for a new driver.
> > 
> > Added theoretical benefit is that we can probe all these drivers from
> > appropriate DSDT (though I doubt anyone will).
> > 
> > I'm sending this first set out to see if anyone has strong views against
> > doing this for at least the simple drivers that have no other device
> > tree dependence.  Obviously more work would be needed to remove
> > use of of_match_ptr from IIO completely.
> > 
> > Light sensors picked as a starting point as they tend to be simple.
> > 
> > I may do follow ups in larger blocks to avoid so many small patches
> > (or indeed flatten these into one when applying)
> 
> fwiw: i was also planning to do a sweep of these;
> well, tbh, the main intent was to target ADI drivers first and do a bigger
> conversion for them to make the slightly friendlier with ACPI; 
> 
> aside from this, i'm also noticing some bad patterns being copied from older
> drivers, when asking new people to write Linux drivers;
> i did not make a list, probably should have;
> one is 'mlock' [still] being copied; and accessing other internal fields;
> but the internal fields accessing requires a bit of a cleanup in the form of
> privatizing the fields somehow;
> 

One thing I noticed in the series.
No idea if it is needed or not; a build would tell:
   Is '#include <linux/mod_devicetable.h>' required for this change?
Most patches add it, but I don't feel it is needed; I could be wrong though.

What I noticed is that all 'linux/of.h' , 'linux/of_device.h' &
'linux/of_platform.h' include it.

> 
> > Thanks
> > 
> > Jonathan
> > 
> > Jonathan Cameron (7):
> >   iio: light: bh1780: use mod_devicetable.h and drop of_match_ptr macro
> >   iio: light: cm32181: Add mod_devicetable.h and remove of_match_ptr
> >   iio: light: cm3232: Add mod_devicetable.h include and drop
> >     of_match_ptr
> >   iio: light: gp2ap020a00f: Swap of.h for mod_devicetable.h + drop
> >     of_match_ptr
> >   iio: light: opt3001: Add mod_devicetable.h and drop use of
> >     of_match_ptr
> >   iio: light: st_uvis25: Add mod_devicetable.h and drop of_match_ptr
> >   iio: light: vl6180: swap of.h for mod_devicetable.h and drop
> >     of_match_ptr
> > 
> >  drivers/iio/light/bh1780.c        | 6 ++----
> >  drivers/iio/light/cm32181.c       | 3 ++-
> >  drivers/iio/light/cm3232.c        | 3 ++-
> >  drivers/iio/light/gp2ap020a00f.c  | 6 ++----
> >  drivers/iio/light/opt3001.c       | 3 ++-
> >  drivers/iio/light/st_uvis25_i2c.c | 3 ++-
> >  drivers/iio/light/st_uvis25_spi.c | 3 ++-
> >  drivers/iio/light/vl6180.c        | 2 +-
> >  8 files changed, 15 insertions(+), 14 deletions(-)
> > 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 7/7] iio: light: vl6180: swap of.h for mod_devicetable.h and drop of_match_ptr
  2020-04-20  6:19   ` Ardelean, Alexandru
@ 2020-04-20 15:45     ` Jonathan Cameron
  2020-04-21  6:58       ` Ardelean, Alexandru
  0 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2020-04-20 15:45 UTC (permalink / raw)
  To: Ardelean, Alexandru; +Cc: jic23, linux-iio

On Mon, 20 Apr 2020 06:19:17 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> On Sun, 2020-04-19 at 16:02 +0100, jic23@kernel.org wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > Enables probing via ACPI PRP0001 route and removes an example of
> > an approach we no longer want people to copy.
> >   
> 
> This doesn't include 'linux/mod_devicetable.h'.
> I'm wondering now if it is needed at all.
> Should we remove it from the rest?
> 
Oops.

So this is one of those classics.  mod_devicetable is included by the spi
and i2c headers but there is no actual 'need' for them to do so. The
could (I think) get away with an appropriate forwards definition.

This is in contrast to the drivers that need to know what that structure
looks like (as does the i2c core device tree code etc).

So should we include it or not?  I'm generally of the view that we should
for resilience but others favour minimal includes.

Jonathan


> With that addressed:
> 
> Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >  drivers/iio/light/vl6180.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
> > index d9533a76b8f6..7e67d7b3bfb6 100644
> > --- a/drivers/iio/light/vl6180.c
> > +++ b/drivers/iio/light/vl6180.c
> > @@ -537,7 +537,7 @@ MODULE_DEVICE_TABLE(i2c, vl6180_id);
> >  static struct i2c_driver vl6180_driver = {
> >  	.driver = {
> >  		.name   = VL6180_DRV_NAME,
> > -		.of_match_table = of_match_ptr(vl6180_of_match),
> > +		.of_match_table = vl6180_of_match,
> >  	},
> >  	.probe  = vl6180_probe,
> >  	.id_table = vl6180_id,  



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers
  2020-04-20  6:22   ` Ardelean, Alexandru
@ 2020-04-20 15:48     ` Jonathan Cameron
  2020-04-21  0:50       ` Andy Shevchenko
  0 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2020-04-20 15:48 UTC (permalink / raw)
  To: Ardelean, Alexandru; +Cc: jic23, linux-iio

On Mon, 20 Apr 2020 06:22:09 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> On Mon, 2020-04-20 at 06:04 +0000, Ardelean, Alexandru wrote:
> > [External]
> > 
> > On Sun, 2020-04-19 at 16:01 +0100, jic23@kernel.org wrote:  
> > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > 
> > > Hi All,
> > > 
> > > Given we keep having to explain to people that of_match_ptr is less
> > > than ideal now we have the option of ACPI DSDT using PRP0001 and
> > > the compatible, it seems sensible to reduce the number of instances
> > > that people might copy for a new driver.
> > > 
> > > Added theoretical benefit is that we can probe all these drivers from
> > > appropriate DSDT (though I doubt anyone will).
> > > 
> > > I'm sending this first set out to see if anyone has strong views against
> > > doing this for at least the simple drivers that have no other device
> > > tree dependence.  Obviously more work would be needed to remove
> > > use of of_match_ptr from IIO completely.
> > > 
> > > Light sensors picked as a starting point as they tend to be simple.
> > > 
> > > I may do follow ups in larger blocks to avoid so many small patches
> > > (or indeed flatten these into one when applying)  
> > 
> > fwiw: i was also planning to do a sweep of these;
> > well, tbh, the main intent was to target ADI drivers first and do a bigger
> > conversion for them to make the slightly friendlier with ACPI; 
> > 
> > aside from this, i'm also noticing some bad patterns being copied from older
> > drivers, when asking new people to write Linux drivers;
> > i did not make a list, probably should have;
> > one is 'mlock' [still] being copied; and accessing other internal fields;
> > but the internal fields accessing requires a bit of a cleanup in the form of
> > privatizing the fields somehow;
> >   
> 
> One thing I noticed in the series.
> No idea if it is needed or not; a build would tell:
>    Is '#include <linux/mod_devicetable.h>' required for this change?
> Most patches add it, but I don't feel it is needed; I could be wrong though.

I addressed this in reply to patch 7.  It's mainly obtained via
i2c.h and spi.h in these drivers.  They don't have any particular need
to include it as they could deal with an opaque pointer.

However, seems unlikely that'll get tidied up any time soon and
debatable whether there is any point in doing so.

> 
> What I noticed is that all 'linux/of.h' , 'linux/of_device.h' &
> 'linux/of_platform.h' include it.

True, but we shouldn't include any of them unless we have reason to do
so. They bring baggage we don't need for these drivers.

Jonathan


> 
> >   
> > > Thanks
> > > 
> > > Jonathan
> > > 
> > > Jonathan Cameron (7):
> > >   iio: light: bh1780: use mod_devicetable.h and drop of_match_ptr macro
> > >   iio: light: cm32181: Add mod_devicetable.h and remove of_match_ptr
> > >   iio: light: cm3232: Add mod_devicetable.h include and drop
> > >     of_match_ptr
> > >   iio: light: gp2ap020a00f: Swap of.h for mod_devicetable.h + drop
> > >     of_match_ptr
> > >   iio: light: opt3001: Add mod_devicetable.h and drop use of
> > >     of_match_ptr
> > >   iio: light: st_uvis25: Add mod_devicetable.h and drop of_match_ptr
> > >   iio: light: vl6180: swap of.h for mod_devicetable.h and drop
> > >     of_match_ptr
> > > 
> > >  drivers/iio/light/bh1780.c        | 6 ++----
> > >  drivers/iio/light/cm32181.c       | 3 ++-
> > >  drivers/iio/light/cm3232.c        | 3 ++-
> > >  drivers/iio/light/gp2ap020a00f.c  | 6 ++----
> > >  drivers/iio/light/opt3001.c       | 3 ++-
> > >  drivers/iio/light/st_uvis25_i2c.c | 3 ++-
> > >  drivers/iio/light/st_uvis25_spi.c | 3 ++-
> > >  drivers/iio/light/vl6180.c        | 2 +-
> > >  8 files changed, 15 insertions(+), 14 deletions(-)
> > >   



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers
  2020-04-20 15:48     ` Jonathan Cameron
@ 2020-04-21  0:50       ` Andy Shevchenko
  2020-04-25 14:49         ` Jonathan Cameron
  0 siblings, 1 reply; 22+ messages in thread
From: Andy Shevchenko @ 2020-04-21  0:50 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Ardelean, Alexandru, jic23, linux-iio

On Mon, Apr 20, 2020 at 6:49 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
> On Mon, 20 Apr 2020 06:22:09 +0000
> "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:
> > On Mon, 2020-04-20 at 06:04 +0000, Ardelean, Alexandru wrote:
> > > On Sun, 2020-04-19 at 16:01 +0100, jic23@kernel.org wrote:

> > One thing I noticed in the series.
> > No idea if it is needed or not; a build would tell:
> >    Is '#include <linux/mod_devicetable.h>' required for this change?
> > Most patches add it, but I don't feel it is needed; I could be wrong though.
>
> I addressed this in reply to patch 7.  It's mainly obtained via
> i2c.h and spi.h in these drivers.  They don't have any particular need
> to include it as they could deal with an opaque pointer.
>
> However, seems unlikely that'll get tidied up any time soon and
> debatable whether there is any point in doing so.

I would use the simple rule, we include header if we have direct user of it.

> > What I noticed is that all 'linux/of.h' , 'linux/of_device.h' &
> > 'linux/of_platform.h' include it.
>
> True, but we shouldn't include any of them unless we have reason to do
> so. They bring baggage we don't need for these drivers.


--
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 7/7] iio: light: vl6180: swap of.h for mod_devicetable.h and drop of_match_ptr
  2020-04-20 15:45     ` Jonathan Cameron
@ 2020-04-21  6:58       ` Ardelean, Alexandru
  0 siblings, 0 replies; 22+ messages in thread
From: Ardelean, Alexandru @ 2020-04-21  6:58 UTC (permalink / raw)
  To: Jonathan.Cameron; +Cc: jic23, linux-iio

On Mon, 2020-04-20 at 16:45 +0100, Jonathan Cameron wrote:
> On Mon, 20 Apr 2020 06:19:17 +0000
> "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:
> 
> > On Sun, 2020-04-19 at 16:02 +0100, jic23@kernel.org wrote:
> > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > 
> > > Enables probing via ACPI PRP0001 route and removes an example of
> > > an approach we no longer want people to copy.
> > >   
> > 
> > This doesn't include 'linux/mod_devicetable.h'.
> > I'm wondering now if it is needed at all.
> > Should we remove it from the rest?
> > 
> Oops.
> 
> So this is one of those classics.  mod_devicetable is included by the spi
> and i2c headers but there is no actual 'need' for them to do so. The
> could (I think) get away with an appropriate forwards definition.
> 
> This is in contrast to the drivers that need to know what that structure
> looks like (as does the i2c core device tree code etc).
> 
> So should we include it or not?  I'm generally of the view that we should
> for resilience but others favour minimal includes.

I'm a bit in the middle here between minimalism & resilience/more-explicit-
inclusions.

No strong opinions about it.
I've been through 1-2 inclusion-hell fixing; still no strong opinion.
Maybe I need a few more.

I usually leave it to someone else's preference.

> 
> Jonathan
> 
> 
> > With that addressed:
> > 
> > Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > 
> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > ---
> > >  drivers/iio/light/vl6180.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
> > > index d9533a76b8f6..7e67d7b3bfb6 100644
> > > --- a/drivers/iio/light/vl6180.c
> > > +++ b/drivers/iio/light/vl6180.c
> > > @@ -537,7 +537,7 @@ MODULE_DEVICE_TABLE(i2c, vl6180_id);
> > >  static struct i2c_driver vl6180_driver = {
> > >  	.driver = {
> > >  		.name   = VL6180_DRV_NAME,
> > > -		.of_match_table = of_match_ptr(vl6180_of_match),
> > > +		.of_match_table = vl6180_of_match,
> > >  	},
> > >  	.probe  = vl6180_probe,
> > >  	.id_table = vl6180_id,  
> 
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers
  2020-04-21  0:50       ` Andy Shevchenko
@ 2020-04-25 14:49         ` Jonathan Cameron
  0 siblings, 0 replies; 22+ messages in thread
From: Jonathan Cameron @ 2020-04-25 14:49 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Jonathan Cameron, Ardelean, Alexandru, linux-iio

On Tue, 21 Apr 2020 03:50:57 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Mon, Apr 20, 2020 at 6:49 PM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> > On Mon, 20 Apr 2020 06:22:09 +0000
> > "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:  
> > > On Mon, 2020-04-20 at 06:04 +0000, Ardelean, Alexandru wrote:  
> > > > On Sun, 2020-04-19 at 16:01 +0100, jic23@kernel.org wrote:  
> 
> > > One thing I noticed in the series.
> > > No idea if it is needed or not; a build would tell:
> > >    Is '#include <linux/mod_devicetable.h>' required for this change?
> > > Most patches add it, but I don't feel it is needed; I could be wrong though.  
> >
> > I addressed this in reply to patch 7.  It's mainly obtained via
> > i2c.h and spi.h in these drivers.  They don't have any particular need
> > to include it as they could deal with an opaque pointer.
> >
> > However, seems unlikely that'll get tidied up any time soon and
> > debatable whether there is any point in doing so.  
> 
> I would use the simple rule, we include header if we have direct user of it.
Sensible and easy to understand basis to make the decision. Let's go
with that (though I don't want to see lots of isolated 'fixes' for
'missing' headers!)

> 
> > > What I noticed is that all 'linux/of.h' , 'linux/of_device.h' &
> > > 'linux/of_platform.h' include it.  
> >
> > True, but we shouldn't include any of them unless we have reason to do
> > so. They bring baggage we don't need for these drivers.  
All applied with patch 7 amended to add the include.

Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with them.

Thanks,

Jonathan

> 
> 
> --
> With Best Regards,
> Andy Shevchenko


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2020-04-25 14:49 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-19 15:01 [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers jic23
2020-04-19 15:02 ` [PATCH 1/7] iio: light: bh1780: use mod_devicetable.h and drop of_match_ptr macro jic23
2020-04-20  6:05   ` Ardelean, Alexandru
2020-04-19 15:02 ` [PATCH 2/7] iio: light: cm32181: Add mod_devicetable.h and remove of_match_ptr jic23
2020-04-20  6:05   ` Ardelean, Alexandru
2020-04-19 15:02 ` [PATCH 3/7] iio: light: cm3232: Add mod_devicetable.h include and drop of_match_ptr jic23
2020-04-20  6:05   ` Ardelean, Alexandru
2020-04-19 15:02 ` [PATCH 4/7] iio: light: gp2ap020a00f: Swap of.h for mod_devicetable.h + " jic23
2020-04-20  6:06   ` Ardelean, Alexandru
2020-04-19 15:02 ` [PATCH 5/7] iio: light: opt3001: Add mod_devicetable.h and drop use of of_match_ptr jic23
2020-04-20  6:06   ` Ardelean, Alexandru
2020-04-19 15:02 ` [PATCH 6/7] iio: light: st_uvis25: Add mod_devicetable.h and drop of_match_ptr jic23
2020-04-20  6:08   ` Ardelean, Alexandru
2020-04-19 15:02 ` [PATCH 7/7] iio: light: vl6180: swap of.h for " jic23
2020-04-20  6:19   ` Ardelean, Alexandru
2020-04-20 15:45     ` Jonathan Cameron
2020-04-21  6:58       ` Ardelean, Alexandru
2020-04-20  6:04 ` [PATCH 0/7] iio: light: clean out of_match_ptr and tidy headers Ardelean, Alexandru
2020-04-20  6:22   ` Ardelean, Alexandru
2020-04-20 15:48     ` Jonathan Cameron
2020-04-21  0:50       ` Andy Shevchenko
2020-04-25 14:49         ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.