| 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 |
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 = <c2471_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");
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 = <c2471_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");
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 = <c2471_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 --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 = <c2471_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");
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(-)