[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAF7HswPByuLXZuHPdfvcXAZRSZkH0BYyy+PjbD7g3Sk97r+UvQ@mail.gmail.com>
Date: Wed, 24 Dec 2025 13:06:25 +0800
From: Kyle Hsieh <kylehsieh1995@...il.com>
To: David Lechner <dlechner@...libre.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>,
Jonathan Cameron <jic23@...nel.org>, Nuno Sá <nuno.sa@...log.com>,
Andy Shevchenko <andy@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Liam Beguin <liambeguin@...il.com>, linux-iio@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] iio: adc: ltc2309: add support for ltc2305
On Wed, Dec 24, 2025 at 2:38 AM David Lechner <dlechner@...libre.com> wrote:
>
> On 12/23/25 3:12 AM, Kyle Hsieh wrote:
> > Add support for the 2-channel LTC2305 ADC in the existing LTC2309 driver.
> > The LTC2305 and LTC2309 share similar features: both are 12-bit,
> > low-noise, low-power SAR ADCs with an I2C interface.
> > The main difference is the number of channels: LTC2305 has 2 channels,
> > while LTC2309 has 8 channels.
> >
> > Signed-off-by: Kyle Hsieh <kylehsieh1995@...il.com>
> > ---
> > drivers/iio/adc/ltc2309.c | 50 +++++++++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 46 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ltc2309.c b/drivers/iio/adc/ltc2309.c
> > index 5f0d947d0615..7a611ddd00d2 100644
> > --- a/drivers/iio/adc/ltc2309.c
> > +++ b/drivers/iio/adc/ltc2309.c
> > @@ -1,9 +1,11 @@
> > // SPDX-License-Identifier: GPL-2.0
> > /*
> > * The LTC2309 is an 8-Channel, 12-Bit SAR ADC with an I2C Interface.
> > + * The LTC2305 is a 2-Channel, 12-Bit SAR ADC with an I2C Interface.
>
> Would be more conventional to list things in low to high order like in the
> dt-bindings. (same applies throughout this patch)
>
I will reorder descriptions and definitions to list devices in
low-to-high order.
> > *
> > * Datasheet:
> > * https://www.analog.com/media/en/technical-documentation/data-sheets/2309fd.pdf
> > + * https://www.analog.com/media/en/technical-documentation/data-sheets/23015fb.pdf
> > *
> > * Copyright (c) 2023, Liam Beguin <liambeguin@...il.com>
> > */
> > @@ -60,6 +62,13 @@ enum ltc2309_channels {
> > LTC2309_CH7,
> > };
> >
> > +enum ltc2305_channels {
> > + LTC2305_CH0_CH1 = 0,
>
> Initializer is not strictly needed. I guess it is consistent with the
> existing code though, so OK.
>
> > + LTC2305_CH1_CH0,
> > + LTC2305_CH0,
> > + LTC2305_CH1,
> > +};
> > +
> > #define LTC2309_CHAN(_chan, _addr) { \
> > .type = IIO_VOLTAGE, \
> > .indexed = 1, \
> > @@ -99,6 +108,31 @@ static const struct iio_chan_spec ltc2309_channels[] = {
> > LTC2309_DIFF_CHAN(7, 6, LTC2309_CH7_CH6),
> > };
> >
> > +static const struct iio_chan_spec ltc2305_channels[] = {
> > + LTC2309_CHAN(0, LTC2305_CH0),
> > + LTC2309_CHAN(1, LTC2305_CH1),
> > + LTC2309_DIFF_CHAN(0, 1, LTC2305_CH0_CH1),
> > + LTC2309_DIFF_CHAN(1, 0, LTC2305_CH1_CH0),
> > +};
> > +
> > +struct ltc230x_chip_info {
>
> We avoid putting "x" in names like this because the pattern breaks too often.
> Just use the main driver prefix of ltc2309.
>
Got it, I will replace this 'ltc203x' with 'ltc2309'.
> > + const char *name;
> > + const struct iio_chan_spec *channels;
> > + int num_channels;
> > +};
> > +
> > +static const struct ltc230x_chip_info ltc2309_chip_info = {
> > + .name = "ltc2309",
> > + .channels = ltc2309_channels,
> > + .num_channels = ARRAY_SIZE(ltc2309_channels),
> > +};
> > +
> > +static const struct ltc230x_chip_info ltc2305_chip_info = {
> > + .name = "ltc2305",
> > + .channels = ltc2305_channels,
> > + .num_channels = ARRAY_SIZE(ltc2305_channels),
> > +};
> > +
> > static int ltc2309_read_raw_channel(struct ltc2309 *ltc2309,
> > unsigned long address, int *val)
> > {
> > @@ -158,6 +192,8 @@ static const struct iio_info ltc2309_info = {
> >
> > static int ltc2309_probe(struct i2c_client *client)
> > {
> > + const struct of_device_id *match;
>
> Unused?
>
I will remove in v2.
> > + const struct ltc230x_chip_info *chip_info;
> > struct iio_dev *indio_dev;
> > struct ltc2309 *ltc2309;
> > int ret;
> > @@ -167,13 +203,17 @@ static int ltc2309_probe(struct i2c_client *client)
> > return -ENOMEM;
> >
> > ltc2309 = iio_priv(indio_dev);
> > + chip_info = device_get_match_data(&client->dev);
>
> Why not i2c_get_match_data()?
>
I will switch from device_get_match_data() to i2c_get_match_data() in v2
> > + if (!chip_info)
> > + return -EINVAL;
> > +
> > ltc2309->dev = &indio_dev->dev;
> > ltc2309->client = client;
> >
> > - indio_dev->name = "ltc2309";
> > + indio_dev->name = chip_info->name;
> > indio_dev->modes = INDIO_DIRECT_MODE;
> > - indio_dev->channels = ltc2309_channels;
> > - indio_dev->num_channels = ARRAY_SIZE(ltc2309_channels);
> > + indio_dev->channels = chip_info->channels;
> > + indio_dev->num_channels = chip_info->num_channels;
> > indio_dev->info = <c2309_info;
> >
> > ret = devm_regulator_get_enable_read_voltage(&client->dev, "vref");
> > @@ -189,13 +229,15 @@ static int ltc2309_probe(struct i2c_client *client)
> > }
> >
> > static const struct of_device_id ltc2309_of_match[] = {
> > - { .compatible = "lltc,ltc2309" },
> > + { .compatible = "lltc,ltc2309", .data = <c2309_chip_info },
> > + { .compatible = "lltc,ltc2305", .data = <c2305_chip_info },
> > { }
> > };
> > MODULE_DEVICE_TABLE(of, ltc2309_of_match);
> >
> > static const struct i2c_device_id ltc2309_id[] = {
> > { "ltc2309" },
> > + { "ltc2305" },
>
> Also need to add chip_infos here.
>
Thanks reminder, I will add.
> > { }
> > };
> > MODULE_DEVICE_TABLE(i2c, ltc2309_id);
> >
>
Powered by blists - more mailing lists