[PATCH v2 2/2] iio: adc: ltc2309: add support for ltc2305

Kyle Hsieh posted 2 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v2 2/2] iio: adc: ltc2309: add support for ltc2305
Posted by Kyle Hsieh 1 month, 2 weeks ago
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@gmail.com>
---
 drivers/iio/adc/ltc2309.c | 51 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 46 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/ltc2309.c b/drivers/iio/adc/ltc2309.c
index 5f0d947d0615..0cf9bcae36c8 100644
--- a/drivers/iio/adc/ltc2309.c
+++ b/drivers/iio/adc/ltc2309.c
@@ -1,8 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
+ * The LTC2305 is a  2-Channel, 12-Bit SAR ADC with an I2C Interface.
  * The LTC2309 is an 8-Channel, 12-Bit SAR ADC with an I2C Interface.
  *
  * Datasheet:
+ * https://www.analog.com/media/en/technical-documentation/data-sheets/23015fb.pdf
  * https://www.analog.com/media/en/technical-documentation/data-sheets/2309fd.pdf
  *
  * Copyright (c) 2023, Liam Beguin <liambeguin@gmail.com>
@@ -41,6 +43,13 @@ struct ltc2309 {
 };
 
 /* Order matches expected channel address, See datasheet Table 1. */
+enum ltc2305_channels {
+	LTC2305_CH0_CH1 = 0,
+	LTC2305_CH1_CH0,
+	LTC2305_CH0,
+	LTC2305_CH1,
+};
+
 enum ltc2309_channels {
 	LTC2309_CH0_CH1 = 0,
 	LTC2309_CH2_CH3,
@@ -80,6 +89,13 @@ enum ltc2309_channels {
 	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
 }
 
+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),
+};
+
 static const struct iio_chan_spec ltc2309_channels[] = {
 	LTC2309_CHAN(0, LTC2309_CH0),
 	LTC2309_CHAN(1, LTC2309_CH1),
@@ -99,6 +115,24 @@ static const struct iio_chan_spec ltc2309_channels[] = {
 	LTC2309_DIFF_CHAN(7, 6, LTC2309_CH7_CH6),
 };
 
+struct ltc2309_chip_info {
+	const char *name;
+	const struct iio_chan_spec *channels;
+	int num_channels;
+};
+
+static const struct ltc2309_chip_info ltc2305_chip_info = {
+	.name = "ltc2305",
+	.channels = ltc2305_channels,
+	.num_channels = ARRAY_SIZE(ltc2305_channels),
+};
+
+static const struct ltc2309_chip_info ltc2309_chip_info = {
+	.name = "ltc2309",
+	.channels = ltc2309_channels,
+	.num_channels = ARRAY_SIZE(ltc2309_channels),
+};
+
 static int ltc2309_read_raw_channel(struct ltc2309 *ltc2309,
 				    unsigned long address, int *val)
 {
@@ -158,6 +192,7 @@ static const struct iio_info ltc2309_info = {
 
 static int ltc2309_probe(struct i2c_client *client)
 {
+	const struct ltc2309_chip_info *chip_info;
 	struct iio_dev *indio_dev;
 	struct ltc2309 *ltc2309;
 	int ret;
@@ -167,13 +202,17 @@ static int ltc2309_probe(struct i2c_client *client)
 		return -ENOMEM;
 
 	ltc2309 = iio_priv(indio_dev);
+	chip_info = i2c_get_match_data(client);
+	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 = &ltc2309_info;
 
 	ret = devm_regulator_get_enable_read_voltage(&client->dev, "vref");
@@ -189,13 +228,15 @@ static int ltc2309_probe(struct i2c_client *client)
 }
 
 static const struct of_device_id ltc2309_of_match[] = {
-	{ .compatible = "lltc,ltc2309" },
+	{ .compatible = "lltc,ltc2305", .data = &ltc2305_chip_info },
+	{ .compatible = "lltc,ltc2309", .data = &ltc2309_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ltc2309_of_match);
 
 static const struct i2c_device_id ltc2309_id[] = {
-	{ "ltc2309" },
+	{ "ltc2305", (kernel_ulong_t)&ltc2305_chip_info },
+	{ "ltc2309", (kernel_ulong_t)&ltc2309_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ltc2309_id);

-- 
2.34.1
Re: [PATCH v2 2/2] iio: adc: ltc2309: add support for ltc2305
Posted by Jonathan Cameron 1 month, 1 week ago
On Wed, 24 Dec 2025 13:37:15 +0800
Kyle Hsieh <kylehsieh1995@gmail.com> 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@gmail.com>
Hi Kyle

This is a fairly small patch, so don't bother doing it this time, but
for future reference, if you are doing a refactor to enable something new
split it into a refactor patch (which makes no operational changes) and
a new stuff patch. Here first of those patches would introduce the chip_info
structure but only for existing supported devices.  That can be reviewed
easily to make sure there are not functional changes.  The second patch then
adds the entries for the new device (which can be checked against the datasheet).

When it is very small, in the interests of expediency we sometimes don't
worry too much about the ideal formation of patches.

In line I mention that the ltc2301 would be very easy to add as well if you
want to do so.  Otherwise looks good to me. I'll leave it on list a little
while though before applying.

> ---
>  drivers/iio/adc/ltc2309.c | 51 ++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 46 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/ltc2309.c b/drivers/iio/adc/ltc2309.c
> index 5f0d947d0615..0cf9bcae36c8 100644
> --- a/drivers/iio/adc/ltc2309.c
> +++ b/drivers/iio/adc/ltc2309.c
> @@ -1,8 +1,10 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> + * The LTC2305 is a  2-Channel, 12-Bit SAR ADC with an I2C Interface.
>   * The LTC2309 is an 8-Channel, 12-Bit SAR ADC with an I2C Interface.
>   *
>   * Datasheet:
> + * https://www.analog.com/media/en/technical-documentation/data-sheets/23015fb.pdf

If you wanted to, it should be trivial to also support the 2301 (I looked given the
odd datasheet file name!)  For families of parts it is common to add support based
on only have access to a small subset.


>   * https://www.analog.com/media/en/technical-documentation/data-sheets/2309fd.pdf
Re: [PATCH v2 2/2] iio: adc: ltc2309: add support for ltc2305
Posted by Kyle Hsieh 1 month, 1 week ago
On Sun, Dec 28, 2025 at 2:18 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Wed, 24 Dec 2025 13:37:15 +0800
> Kyle Hsieh <kylehsieh1995@gmail.com> 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@gmail.com>
> Hi Kyle
>
> This is a fairly small patch, so don't bother doing it this time, but
> for future reference, if you are doing a refactor to enable something new
> split it into a refactor patch (which makes no operational changes) and
> a new stuff patch. Here first of those patches would introduce the chip_info
> structure but only for existing supported devices.  That can be reviewed
> easily to make sure there are not functional changes.  The second patch then
> adds the entries for the new device (which can be checked against the datasheet).
>
> When it is very small, in the interests of expediency we sometimes don't
> worry too much about the ideal formation of patches.
>
> In line I mention that the ltc2301 would be very easy to add as well if you
> want to do so.  Otherwise looks good to me. I'll leave it on list a little
> while though before applying.
Hi Jonathan,

Thanks for the review and feedback.
I appreciate the suggestions regarding patch splitting for future submissions,
and the tip about LTC2301 support.
I will  keep that in mind for future work.

Best regards,
Kyle
>
> > ---
> >  drivers/iio/adc/ltc2309.c | 51 ++++++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 46 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ltc2309.c b/drivers/iio/adc/ltc2309.c
> > index 5f0d947d0615..0cf9bcae36c8 100644
> > --- a/drivers/iio/adc/ltc2309.c
> > +++ b/drivers/iio/adc/ltc2309.c
> > @@ -1,8 +1,10 @@
> >  // SPDX-License-Identifier: GPL-2.0
> >  /*
> > + * The LTC2305 is a  2-Channel, 12-Bit SAR ADC with an I2C Interface.
> >   * The LTC2309 is an 8-Channel, 12-Bit SAR ADC with an I2C Interface.
> >   *
> >   * Datasheet:
> > + * https://www.analog.com/media/en/technical-documentation/data-sheets/23015fb.pdf
>
> If you wanted to, it should be trivial to also support the 2301 (I looked given the
> odd datasheet file name!)  For families of parts it is common to add support based
> on only have access to a small subset.
>
>
> >   * https://www.analog.com/media/en/technical-documentation/data-sheets/2309fd.pdf
Re: [PATCH v2 2/2] iio: adc: ltc2309: add support for ltc2305
Posted by Andy Shevchenko 1 month, 1 week ago
On Wed, Dec 24, 2025 at 01:37:15PM +0800, 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.

>  /* Order matches expected channel address, See datasheet Table 1. */
> +enum ltc2305_channels {
> +	LTC2305_CH0_CH1 = 0,
> +	LTC2305_CH1_CH0,
> +	LTC2305_CH0,
> +	LTC2305_CH1,

When it's hardware defined, assign all of them explicitly. Otherwise drop the
unneeded 0 which is guaranteed by the C standard.

> +};

...

> +	chip_info = i2c_get_match_data(client);

> +	if (!chip_info)
> +		return -EINVAL;

I consider this check redundant. There is shouldn't be a production code that
works nicely when there is a clear mistake in it (absence of the mandatory
static initialiser). The author of the change should have been testing this
and hence it will Oops the kernel, which means that the initial code is b0rken.
So, drop the dead check.

-- 
With Best Regards,
Andy Shevchenko