diff mbox series

[v3,2/3] ltc2471: ltc2461/ltc2463 compatible strings

Message ID 20200727135834.84093-3-darius.berghe@analog.com (mailing list archive)
State New, archived
Headers show
Series [v3,1/3] ltc2471: add of_match_table for existing devices | expand

Commit Message

Berghe, Darius July 27, 2020, 1:58 p.m. UTC 
Add compatible strings for these devices in the existing ltc2471
driver.

Signed-off-by: Darius Berghe <darius.berghe@analog.com>
---
 drivers/iio/adc/ltc2471.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

Comments

Jonathan Cameron July 27, 2020, 5:26 p.m. UTC | #1 
On Mon, 27 Jul 2020 16:58:33 +0300
Darius Berghe <darius.berghe@analog.com> wrote:

> Add compatible strings for these devices in the existing ltc2471
> driver.
> 
> Signed-off-by: Darius Berghe <darius.berghe@analog.com>

Hi Darius,

A few additional minor comments from me.

> ---
>  drivers/iio/adc/ltc2471.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/ltc2471.c b/drivers/iio/adc/ltc2471.c
> index e1c4e966524d..8c57203b1fe4 100644
> --- a/drivers/iio/adc/ltc2471.c
> +++ b/drivers/iio/adc/ltc2471.c
> @@ -1,5 +1,7 @@
>  /*
> - * Driver for Linear Technology LTC2471 and LTC2473 voltage monitors
> + * Driver for Linear Technology LTC2461, LTC2463, LTC2471 and LTC2473 voltage
> + * monitors.
> + * The LTC2463 is identical to the 2461, but reports a differential signal.
>   * The LTC2473 is identical to the 2471, but reports a differential signal.
>   *
>   * Copyright (C) 2017 Topic Embedded Products
> @@ -17,8 +19,10 @@
>  #include <linux/mod_devicetable.h>
>  
>  enum ltc2471_chips {
> +	ltc2461,
> +	ltc2463,
>  	ltc2471,
> -	ltc2473,
> +	ltc2473

Why drop the comma?  We've just added two new devices. Seems possible there
may be more in the future!

>  };
>  
>  struct ltc2471_data {
> @@ -122,7 +126,7 @@ static int ltc2471_i2c_probe(struct i2c_client *client,
>  	indio_dev->name = id->name;
>  	indio_dev->info = &ltc2471_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> -	if (id->driver_data == ltc2473)
> +	if (id->driver_data == ltc2473 || id->driver_data == ltc2463)
If the only use of driver_data is going to be this check, then just set it
to 2473 for the 2463 and 2473.  It's not uncommon to do this when we have
a bunch of devices that look the same to software.

>  		indio_dev->channels = ltc2473_channel;
>  	else
>  		indio_dev->channels = ltc2471_channel;
> @@ -139,6 +143,8 @@ static int ltc2471_i2c_probe(struct i2c_client *client,
>  }
>  
>  static const struct i2c_device_id ltc2471_i2c_id[] = {
> +	{ "ltc2461", ltc2461 },
> +	{ "ltc2463", ltc2463 },
>  	{ "ltc2471", ltc2471 },
>  	{ "ltc2473", ltc2473 },
>  	{}
> @@ -146,6 +152,8 @@ static const struct i2c_device_id ltc2471_i2c_id[] = {
>  MODULE_DEVICE_TABLE(i2c, ltc2471_i2c_id);
>  
>  static const struct of_device_id ltc2471_of_match[] = {
> +	{ .compatible = "adi,ltc2461" },
> +	{ .compatible = "adi,ltc2463" },
>  	{ .compatible = "adi,ltc2471" },
>  	{ .compatible = "adi,ltc2473" },
>  	{}
> @@ -163,6 +171,6 @@ static struct i2c_driver ltc2471_i2c_driver = {
>  
>  module_i2c_driver(ltc2471_i2c_driver);
>  
> -MODULE_DESCRIPTION("LTC2471/LTC2473 ADC driver");
> +MODULE_DESCRIPTION("LTC2461/LTC2463/LTC2471/LTC2473 ADC driver");
>  MODULE_AUTHOR("Topic Embedded Products");
>  MODULE_LICENSE("GPL v2");
Berghe, Darius July 28, 2020, 6:24 a.m. UTC | #2 
Hi Jonathan,

Thanks for review, comments inline.

> -----Original Message-----
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Sent: Monday, July 27, 2020 8:27 PM
> To: Berghe, Darius <Darius.Berghe@analog.com>
> Cc: linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; jic23@kernel.org; robh@kernel.org
> Subject: Re: [PATCH v3 2/3] ltc2471: ltc2461/ltc2463 compatible strings
> 
> [External]
> 
> On Mon, 27 Jul 2020 16:58:33 +0300
> Darius Berghe <darius.berghe@analog.com> wrote:
> 
> > Add compatible strings for these devices in the existing ltc2471
> > driver.
> >
> > Signed-off-by: Darius Berghe <darius.berghe@analog.com>
> 
> Hi Darius,
> 
> A few additional minor comments from me.
> 
> > ---
> >  drivers/iio/adc/ltc2471.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ltc2471.c b/drivers/iio/adc/ltc2471.c
> > index e1c4e966524d..8c57203b1fe4 100644
> > --- a/drivers/iio/adc/ltc2471.c
> > +++ b/drivers/iio/adc/ltc2471.c
> > @@ -1,5 +1,7 @@
> >  /*
> > - * Driver for Linear Technology LTC2471 and LTC2473 voltage monitors
> > + * Driver for Linear Technology LTC2461, LTC2463, LTC2471 and LTC2473
> > + voltage
> > + * monitors.
> > + * The LTC2463 is identical to the 2461, but reports a differential signal.
> >   * The LTC2473 is identical to the 2471, but reports a differential signal.
> >   *
> >   * Copyright (C) 2017 Topic Embedded Products @@ -17,8 +19,10 @@
> > #include <linux/mod_devicetable.h>
> >
> >  enum ltc2471_chips {
> > +	ltc2461,
> > +	ltc2463,
> >  	ltc2471,
> > -	ltc2473,
> > +	ltc2473
> 
> Why drop the comma?  We've just added two new devices. Seems possible
> there may be more in the future!
> 

Ok, will leave it there in v4.

> >  };
> >
> >  struct ltc2471_data {
> > @@ -122,7 +126,7 @@ static int ltc2471_i2c_probe(struct i2c_client
> *client,
> >  	indio_dev->name = id->name;
> >  	indio_dev->info = &ltc2471_info;
> >  	indio_dev->modes = INDIO_DIRECT_MODE;
> > -	if (id->driver_data == ltc2473)
> > +	if (id->driver_data == ltc2473 || id->driver_data == ltc2463)
> If the only use of driver_data is going to be this check, then just set it to 2473
> for the 2463 and 2473.  It's not uncommon to do this when we have a bunch
> of devices that look the same to software.

Yes the chips are similar but there is at least one feature which requires this level 
of distinguishing them: the sampling rate (60sps for ltc2461/3 and selectable 208/833sps 
for ltc2471/3). It's not used anywhere for now but I can see it being implemented
as standard IIO dev attribute sampling frequency.

> 
> >  		indio_dev->channels = ltc2473_channel;
> >  	else
> >  		indio_dev->channels = ltc2471_channel; @@ -139,6 +143,8
> @@ static
> > int ltc2471_i2c_probe(struct i2c_client *client,  }
> >
> >  static const struct i2c_device_id ltc2471_i2c_id[] = {
> > +	{ "ltc2461", ltc2461 },
> > +	{ "ltc2463", ltc2463 },
> >  	{ "ltc2471", ltc2471 },
> >  	{ "ltc2473", ltc2473 },
> >  	{}
> > @@ -146,6 +152,8 @@ static const struct i2c_device_id ltc2471_i2c_id[]
> > = {  MODULE_DEVICE_TABLE(i2c, ltc2471_i2c_id);
> >
> >  static const struct of_device_id ltc2471_of_match[] = {
> > +	{ .compatible = "adi,ltc2461" },
> > +	{ .compatible = "adi,ltc2463" },
> >  	{ .compatible = "adi,ltc2471" },
> >  	{ .compatible = "adi,ltc2473" },
> >  	{}
> > @@ -163,6 +171,6 @@ static struct i2c_driver ltc2471_i2c_driver = {
> >
> >  module_i2c_driver(ltc2471_i2c_driver);
> >
> > -MODULE_DESCRIPTION("LTC2471/LTC2473 ADC driver");
> > +MODULE_DESCRIPTION("LTC2461/LTC2463/LTC2471/LTC2473 ADC
> driver");
> >  MODULE_AUTHOR("Topic Embedded Products");  MODULE_LICENSE("GPL
> v2");
Jonathan Cameron Aug. 1, 2020, 4:05 p.m. UTC | #3 
On Tue, 28 Jul 2020 06:24:03 +0000
"Berghe, Darius" <Darius.Berghe@analog.com> wrote:

> Hi Jonathan,
> 
> Thanks for review, comments inline.
> 
> > -----Original Message-----
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Sent: Monday, July 27, 2020 8:27 PM
> > To: Berghe, Darius <Darius.Berghe@analog.com>
> > Cc: linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org;
> > devicetree@vger.kernel.org; jic23@kernel.org; robh@kernel.org
> > Subject: Re: [PATCH v3 2/3] ltc2471: ltc2461/ltc2463 compatible strings
> > 
> > [External]
> > 
> > On Mon, 27 Jul 2020 16:58:33 +0300
> > Darius Berghe <darius.berghe@analog.com> wrote:
> >   
> > > Add compatible strings for these devices in the existing ltc2471
> > > driver.
> > >
> > > Signed-off-by: Darius Berghe <darius.berghe@analog.com>  
> > 
> > Hi Darius,
> > 
> > A few additional minor comments from me.
> >   
> > > ---
> > >  drivers/iio/adc/ltc2471.c | 16 ++++++++++++----
> > >  1 file changed, 12 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/iio/adc/ltc2471.c b/drivers/iio/adc/ltc2471.c
> > > index e1c4e966524d..8c57203b1fe4 100644
> > > --- a/drivers/iio/adc/ltc2471.c
> > > +++ b/drivers/iio/adc/ltc2471.c
> > > @@ -1,5 +1,7 @@
> > >  /*
> > > - * Driver for Linear Technology LTC2471 and LTC2473 voltage monitors
> > > + * Driver for Linear Technology LTC2461, LTC2463, LTC2471 and LTC2473
> > > + voltage
> > > + * monitors.
> > > + * The LTC2463 is identical to the 2461, but reports a differential signal.
> > >   * The LTC2473 is identical to the 2471, but reports a differential signal.
> > >   *
> > >   * Copyright (C) 2017 Topic Embedded Products @@ -17,8 +19,10 @@
> > > #include <linux/mod_devicetable.h>
> > >
> > >  enum ltc2471_chips {
> > > +	ltc2461,
> > > +	ltc2463,
> > >  	ltc2471,
> > > -	ltc2473,
> > > +	ltc2473  
> > 
> > Why drop the comma?  We've just added two new devices. Seems possible
> > there may be more in the future!
> >   
> 
> Ok, will leave it there in v4.
> 
> > >  };
> > >
> > >  struct ltc2471_data {
> > > @@ -122,7 +126,7 @@ static int ltc2471_i2c_probe(struct i2c_client  
> > *client,  
> > >  	indio_dev->name = id->name;
> > >  	indio_dev->info = &ltc2471_info;
> > >  	indio_dev->modes = INDIO_DIRECT_MODE;
> > > -	if (id->driver_data == ltc2473)
> > > +	if (id->driver_data == ltc2473 || id->driver_data == ltc2463)  
> > If the only use of driver_data is going to be this check, then just set it to 2473
> > for the 2463 and 2473.  It's not uncommon to do this when we have a bunch
> > of devices that look the same to software.  
> 
> Yes the chips are similar but there is at least one feature which requires this level 
> of distinguishing them: the sampling rate (60sps for ltc2461/3 and selectable 208/833sps 
> for ltc2471/3). It's not used anywhere for now but I can see it being implemented
> as standard IIO dev attribute sampling frequency.

Fair enough.  Perhaps a note in the patch description so no one asks the question
on the next version?

> 
> >   
> > >  		indio_dev->channels = ltc2473_channel;
> > >  	else
> > >  		indio_dev->channels = ltc2471_channel; @@ -139,6 +143,8  
> > @@ static  
> > > int ltc2471_i2c_probe(struct i2c_client *client,  }
> > >
> > >  static const struct i2c_device_id ltc2471_i2c_id[] = {
> > > +	{ "ltc2461", ltc2461 },
> > > +	{ "ltc2463", ltc2463 },
> > >  	{ "ltc2471", ltc2471 },
> > >  	{ "ltc2473", ltc2473 },
> > >  	{}
> > > @@ -146,6 +152,8 @@ static const struct i2c_device_id ltc2471_i2c_id[]
> > > = {  MODULE_DEVICE_TABLE(i2c, ltc2471_i2c_id);
> > >
> > >  static const struct of_device_id ltc2471_of_match[] = {
> > > +	{ .compatible = "adi,ltc2461" },
> > > +	{ .compatible = "adi,ltc2463" },
> > >  	{ .compatible = "adi,ltc2471" },
> > >  	{ .compatible = "adi,ltc2473" },
> > >  	{}
> > > @@ -163,6 +171,6 @@ static struct i2c_driver ltc2471_i2c_driver = {
> > >
> > >  module_i2c_driver(ltc2471_i2c_driver);
> > >
> > > -MODULE_DESCRIPTION("LTC2471/LTC2473 ADC driver");
> > > +MODULE_DESCRIPTION("LTC2461/LTC2463/LTC2471/LTC2473 ADC  
> > driver");  
> > >  MODULE_AUTHOR("Topic Embedded Products");  MODULE_LICENSE("GPL  
> > v2");  
>
diff mbox series

Patch

diff --git a/drivers/iio/adc/ltc2471.c b/drivers/iio/adc/ltc2471.c
index e1c4e966524d..8c57203b1fe4 100644
--- a/drivers/iio/adc/ltc2471.c
+++ b/drivers/iio/adc/ltc2471.c
@@ -1,5 +1,7 @@ 
 /*
- * Driver for Linear Technology LTC2471 and LTC2473 voltage monitors
+ * Driver for Linear Technology LTC2461, LTC2463, LTC2471 and LTC2473 voltage
+ * monitors.
+ * The LTC2463 is identical to the 2461, but reports a differential signal.
  * The LTC2473 is identical to the 2471, but reports a differential signal.
  *
  * Copyright (C) 2017 Topic Embedded Products
@@ -17,8 +19,10 @@ 
 #include <linux/mod_devicetable.h>
 
 enum ltc2471_chips {
+	ltc2461,
+	ltc2463,
 	ltc2471,
-	ltc2473,
+	ltc2473
 };
 
 struct ltc2471_data {
@@ -122,7 +126,7 @@  static int ltc2471_i2c_probe(struct i2c_client *client,
 	indio_dev->name = id->name;
 	indio_dev->info = &ltc2471_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-	if (id->driver_data == ltc2473)
+	if (id->driver_data == ltc2473 || id->driver_data == ltc2463)
 		indio_dev->channels = ltc2473_channel;
 	else
 		indio_dev->channels = ltc2471_channel;
@@ -139,6 +143,8 @@  static int ltc2471_i2c_probe(struct i2c_client *client,
 }
 
 static const struct i2c_device_id ltc2471_i2c_id[] = {
+	{ "ltc2461", ltc2461 },
+	{ "ltc2463", ltc2463 },
 	{ "ltc2471", ltc2471 },
 	{ "ltc2473", ltc2473 },
 	{}
@@ -146,6 +152,8 @@  static const struct i2c_device_id ltc2471_i2c_id[] = {
 MODULE_DEVICE_TABLE(i2c, ltc2471_i2c_id);
 
 static const struct of_device_id ltc2471_of_match[] = {
+	{ .compatible = "adi,ltc2461" },
+	{ .compatible = "adi,ltc2463" },
 	{ .compatible = "adi,ltc2471" },
 	{ .compatible = "adi,ltc2473" },
 	{}
@@ -163,6 +171,6 @@  static struct i2c_driver ltc2471_i2c_driver = {
 
 module_i2c_driver(ltc2471_i2c_driver);
 
-MODULE_DESCRIPTION("LTC2471/LTC2473 ADC driver");
+MODULE_DESCRIPTION("LTC2461/LTC2463/LTC2471/LTC2473 ADC driver");
 MODULE_AUTHOR("Topic Embedded Products");
 MODULE_LICENSE("GPL v2");