diff mbox series

iio: health: max30100: use generic property handler

Message ID 5e6afe0d.1c69fb81.25912.f2eb@mx.google.com (mailing list archive)
State Superseded
Headers show
Series iio: health: max30100: use generic property handler | expand

Commit Message

Rohit Sarkar March 13, 2020, 3:29 a.m. UTC
Instead of of_property_read_xxx use device_property_read_xxx as it is
compatible with ACPI too as opposed to only device tree.

Signed-off-by: Rohit Sarkar <rohitsarkar5398@gmail.com>
---
 drivers/iio/health/max30100.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Jonathan Cameron March 15, 2020, 12:50 p.m. UTC | #1
On Fri, 13 Mar 2020 08:59:13 +0530
Rohit Sarkar <rohitsarkar5398@gmail.com> wrote:

> Instead of of_property_read_xxx use device_property_read_xxx as it is
> compatible with ACPI too as opposed to only device tree.
> 
> Signed-off-by: Rohit Sarkar <rohitsarkar5398@gmail.com>

Look at how this driver is probing from DT.  There is another thing you need
to do to make it possible to use PRP001 ACPI based bindings.

(check what Andy Shevchenko did in a similar patch)

I'm being deliberately vague as useful for you to understand what is going
on here for yourself :)

Also, make sure to check you have cc'd everyone relevant.  Here you are missing
the driver author. +cc Matt
Jonathan

> ---
>  drivers/iio/health/max30100.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/health/max30100.c b/drivers/iio/health/max30100.c
> index 84010501762d..8249c6b36818 100644
> --- a/drivers/iio/health/max30100.c
> +++ b/drivers/iio/health/max30100.c
> @@ -16,7 +16,7 @@
>  #include <linux/irq.h>
>  #include <linux/i2c.h>
>  #include <linux/mutex.h>
> -#include <linux/of.h>
> +#include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
> @@ -267,11 +267,10 @@ static int max30100_get_current_idx(unsigned int val, int *reg)
>  static int max30100_led_init(struct max30100_data *data)
>  {
>  	struct device *dev = &data->client->dev;
> -	struct device_node *np = dev->of_node;
>  	unsigned int val[2];
>  	int reg, ret;
>  
> -	ret = of_property_read_u32_array(np, "maxim,led-current-microamp",
> +	ret = device_property_read_u32_array(dev, "maxim,led-current-microamp",
>  					(unsigned int *) &val, 2);
>  	if (ret) {
>  		/* Default to 24 mA RED LED, 50 mA IR LED */
Rohit Sarkar March 15, 2020, 3:27 p.m. UTC | #2
On Sun, Mar 15, 2020 at 12:50:39PM +0000, Jonathan Cameron wrote:
> On Fri, 13 Mar 2020 08:59:13 +0530
> Rohit Sarkar <rohitsarkar5398@gmail.com> wrote:
> 
> > Instead of of_property_read_xxx use device_property_read_xxx as it is
> > compatible with ACPI too as opposed to only device tree.
> > 
> > Signed-off-by: Rohit Sarkar <rohitsarkar5398@gmail.com>
> 
> Look at how this driver is probing from DT.  There is another thing you need
> to do to make it possible to use PRP001 ACPI based bindings.
> 
> (check what Andy Shevchenko did in a similar patch)
> 
> I'm being deliberately vague as useful for you to understand what is going
> on here for yourself :)
> 
> Also, make sure to check you have cc'd everyone relevant.  Here you are missing
> the driver author. +cc Matt
> Jonathan

Have sent out a v2 with the required correction, although I still dont completely
understand how this ACPI magic works. In the process of going over 
https://www.kernel.org/doc/Documentation/acpi/enumeration.txt get a
better picture.

Thanks,
Rohit
Matt Ranostay March 16, 2020, 8:08 a.m. UTC | #3
On Sun, Mar 15, 2020 at 5:50 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Fri, 13 Mar 2020 08:59:13 +0530
> Rohit Sarkar <rohitsarkar5398@gmail.com> wrote:
>
> > Instead of of_property_read_xxx use device_property_read_xxx as it is
> > compatible with ACPI too as opposed to only device tree.
> >
> > Signed-off-by: Rohit Sarkar <rohitsarkar5398@gmail.com>
>
> Look at how this driver is probing from DT.  There is another thing you need
> to do to make it possible to use PRP001 ACPI based bindings.
>
> (check what Andy Shevchenko did in a similar patch)
>
> I'm being deliberately vague as useful for you to understand what is going
> on here for yourself :)
>
> Also, make sure to check you have cc'd everyone relevant.  Here you are missing
> the driver author. +cc Matt
> Jonathan
>

Highly doubt anyone is using this in an ACPI setting. Am I missing
something though?

- Matt

> > ---
> >  drivers/iio/health/max30100.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iio/health/max30100.c b/drivers/iio/health/max30100.c
> > index 84010501762d..8249c6b36818 100644
> > --- a/drivers/iio/health/max30100.c
> > +++ b/drivers/iio/health/max30100.c
> > @@ -16,7 +16,7 @@
> >  #include <linux/irq.h>
> >  #include <linux/i2c.h>
> >  #include <linux/mutex.h>
> > -#include <linux/of.h>
> > +#include <linux/property.h>
> >  #include <linux/regmap.h>
> >  #include <linux/iio/iio.h>
> >  #include <linux/iio/buffer.h>
> > @@ -267,11 +267,10 @@ static int max30100_get_current_idx(unsigned int val, int *reg)
> >  static int max30100_led_init(struct max30100_data *data)
> >  {
> >       struct device *dev = &data->client->dev;
> > -     struct device_node *np = dev->of_node;
> >       unsigned int val[2];
> >       int reg, ret;
> >
> > -     ret = of_property_read_u32_array(np, "maxim,led-current-microamp",
> > +     ret = device_property_read_u32_array(dev, "maxim,led-current-microamp",
> >                                       (unsigned int *) &val, 2);
> >       if (ret) {
> >               /* Default to 24 mA RED LED, 50 mA IR LED */
>
Jonathan Cameron March 16, 2020, 11:56 a.m. UTC | #4
On Mon, 16 Mar 2020 01:08:20 -0700
Matt Ranostay <matt.ranostay@konsulko.com> wrote:

> On Sun, Mar 15, 2020 at 5:50 AM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Fri, 13 Mar 2020 08:59:13 +0530
> > Rohit Sarkar <rohitsarkar5398@gmail.com> wrote:
> >  
> > > Instead of of_property_read_xxx use device_property_read_xxx as it is
> > > compatible with ACPI too as opposed to only device tree.
> > >
> > > Signed-off-by: Rohit Sarkar <rohitsarkar5398@gmail.com>  
> >
> > Look at how this driver is probing from DT.  There is another thing you need
> > to do to make it possible to use PRP001 ACPI based bindings.
> >
> > (check what Andy Shevchenko did in a similar patch)
> >
> > I'm being deliberately vague as useful for you to understand what is going
> > on here for yourself :)
> >
> > Also, make sure to check you have cc'd everyone relevant.  Here you are missing
> > the driver author. +cc Matt
> > Jonathan
> >  
> 
> Highly doubt anyone is using this in an ACPI setting. Am I missing
> something though?

The 'new' wonder that is the ACPI ID PRP001.  It lets you instantiate
a device with a DT binding but using a chunk of DSDT.

Where it comes in handy is if you happen to have an X86 machine that you
are using to test and do it by overriding the DSDT (easy to do) to add
whatever controller you are using and the device.  Basically it is the
ACPI equivalent of hacking the dt file for a board.

Very convenient for testing device drivers.

Jonathan

> 
> - Matt
> 
> > > ---
> > >  drivers/iio/health/max30100.c | 5 ++---
> > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/iio/health/max30100.c b/drivers/iio/health/max30100.c
> > > index 84010501762d..8249c6b36818 100644
> > > --- a/drivers/iio/health/max30100.c
> > > +++ b/drivers/iio/health/max30100.c
> > > @@ -16,7 +16,7 @@
> > >  #include <linux/irq.h>
> > >  #include <linux/i2c.h>
> > >  #include <linux/mutex.h>
> > > -#include <linux/of.h>
> > > +#include <linux/property.h>
> > >  #include <linux/regmap.h>
> > >  #include <linux/iio/iio.h>
> > >  #include <linux/iio/buffer.h>
> > > @@ -267,11 +267,10 @@ static int max30100_get_current_idx(unsigned int val, int *reg)
> > >  static int max30100_led_init(struct max30100_data *data)
> > >  {
> > >       struct device *dev = &data->client->dev;
> > > -     struct device_node *np = dev->of_node;
> > >       unsigned int val[2];
> > >       int reg, ret;
> > >
> > > -     ret = of_property_read_u32_array(np, "maxim,led-current-microamp",
> > > +     ret = device_property_read_u32_array(dev, "maxim,led-current-microamp",
> > >                                       (unsigned int *) &val, 2);
> > >       if (ret) {
> > >               /* Default to 24 mA RED LED, 50 mA IR LED */  
> >
diff mbox series

Patch

diff --git a/drivers/iio/health/max30100.c b/drivers/iio/health/max30100.c
index 84010501762d..8249c6b36818 100644
--- a/drivers/iio/health/max30100.c
+++ b/drivers/iio/health/max30100.c
@@ -16,7 +16,7 @@ 
 #include <linux/irq.h>
 #include <linux/i2c.h>
 #include <linux/mutex.h>
-#include <linux/of.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
@@ -267,11 +267,10 @@  static int max30100_get_current_idx(unsigned int val, int *reg)
 static int max30100_led_init(struct max30100_data *data)
 {
 	struct device *dev = &data->client->dev;
-	struct device_node *np = dev->of_node;
 	unsigned int val[2];
 	int reg, ret;
 
-	ret = of_property_read_u32_array(np, "maxim,led-current-microamp",
+	ret = device_property_read_u32_array(dev, "maxim,led-current-microamp",
 					(unsigned int *) &val, 2);
 	if (ret) {
 		/* Default to 24 mA RED LED, 50 mA IR LED */