From: "Berghe, Darius" <Darius.Berghe@analog.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: "linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"jic23@kernel.org" <jic23@kernel.org>,
"robh@kernel.org" <robh@kernel.org>
Subject: RE: [PATCH v3 2/3] ltc2471: ltc2461/ltc2463 compatible strings
Date: Tue, 28 Jul 2020 06:24:03 +0000 [thread overview]
Message-ID: <MWHPR03MB319956D095B2715835D0BF1A96730@MWHPR03MB3199.namprd03.prod.outlook.com> (raw)
In-Reply-To: <20200727182647.00002e3c@huawei.com>
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");
next prev parent reply other threads:[~2020-07-28 6:24 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-27 13:58 iio:adc:ltc2371: add support for ltc2361/ltc2363 Darius Berghe
2020-07-27 13:58 ` [PATCH v3 1/3] ltc2471: add of_match_table for existing devices Darius Berghe
2020-07-27 13:58 ` [PATCH v3 2/3] ltc2471: ltc2461/ltc2463 compatible strings Darius Berghe
2020-07-27 17:26 ` Jonathan Cameron
2020-07-28 6:24 ` Berghe, Darius [this message]
2020-08-01 16:05 ` Jonathan Cameron
2020-07-27 13:58 ` [PATCH v3 3/3] ltc2471 driver yaml Darius Berghe
2020-07-27 17:24 ` Jonathan Cameron
2020-07-27 18:15 ` Rob Herring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MWHPR03MB319956D095B2715835D0BF1A96730@MWHPR03MB3199.namprd03.prod.outlook.com \
--to=darius.berghe@analog.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=devicetree@vger.kernel.org \
--cc=jic23@kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).