diff mbox series

[7/7] iio: light: vl6180: swap of.h for mod_devicetable.h and drop of_match_ptr

Message ID 20200419150206.43033-8-jic23@kernel.org (mailing list archive)
State New, archived
Headers show
Series iio: light: clean out of_match_ptr and tidy headers | expand

Commit Message

Jonathan Cameron April 19, 2020, 3:02 p.m. UTC
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(-)

Comments

Alexandru Ardelean April 20, 2020, 6:19 a.m. UTC | #1
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,
Jonathan Cameron April 20, 2020, 3:45 p.m. UTC | #2
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,
Alexandru Ardelean April 21, 2020, 6:58 a.m. UTC | #3
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,  
> 
>
diff mbox series

Patch

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,