* Re: [PATCH 2/3] dt-bindings: iio: adc: lltc,ltc2497: add LTC2305 support
[not found] ` <20260320140819.191700-3-carlosjr.jones@analog.com>
@ 2026-03-20 17:27 ` Conor Dooley
2026-03-20 17:27 ` Conor Dooley
0 siblings, 1 reply; 13+ messages in thread
From: Conor Dooley @ 2026-03-20 17:27 UTC (permalink / raw)
To: Carlos Jones Jr
Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, David Lechner,
Michael Hennerich, Liam Beguin, Nuno Sá, Andy Shevchenko,
Tobias Sperling, Jorge Marques, linux-iio, devicetree,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1903 bytes --]
On Fri, Mar 20, 2026 at 10:08:18PM +0800, Carlos Jones Jr wrote:
> Add support for the LTC2305 2-channel, 12-bit ADC to the existing
> LTC2497 device tree bindings.
> The LTC2305 is compatible with the
> LTC2309 driver implementation.
Clearly this is not true, given the driver patches in the series.
pw-bot: changes-requested
Cheers,
Conor.
>
> Signed-off-by: Carlos Jones Jr <carlosjr.jones@analog.com>
> ---
> .../devicetree/bindings/iio/adc/lltc,ltc2497.yaml | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml
> index 5cc6a9684077..b246d492950e 100644
> --- a/Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml
> @@ -4,13 +4,19 @@
> $id: http://devicetree.org/schemas/iio/adc/lltc,ltc2497.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> -title: Linear Technology / Analog Devices LTC2497 and LTC2309 ADC
> +title: Linear Technology / Analog Devices LTC2497 and similar ADCs
>
> maintainers:
> - Michael Hennerich <michael.hennerich@analog.com>
> - Liam Beguin <liambeguin@gmail.com>
>
> description: |
> + LTC2305:
> + low noise, low power, 2-channel, 12-bit successive approximation ADC with an
> + I2C compatible serial interface.
> +
> + https://www.analog.com/media/en/technical-documentation/data-sheets/2305fa.pdf
> +
> LTC2309:
> low noise, low power, 8-channel, 12-bit successive approximation ADC with an
> I2C compatible serial interface.
> @@ -28,6 +34,7 @@ description: |
> properties:
> compatible:
> enum:
> + - lltc,ltc2305
> - lltc,ltc2309
> - lltc,ltc2497
> - lltc,ltc2499
> --
> 2.43.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] dt-bindings: iio: adc: lltc,ltc2497: add LTC2305 support
2026-03-20 17:27 ` [PATCH 2/3] dt-bindings: iio: adc: lltc,ltc2497: add LTC2305 support Conor Dooley
@ 2026-03-20 17:27 ` Conor Dooley
2026-03-23 10:39 ` Jones, Carlos jr
0 siblings, 1 reply; 13+ messages in thread
From: Conor Dooley @ 2026-03-20 17:27 UTC (permalink / raw)
To: Carlos Jones Jr
Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, David Lechner,
Michael Hennerich, Liam Beguin, Nuno Sá, Andy Shevchenko,
Tobias Sperling, Jorge Marques, linux-iio, devicetree,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2153 bytes --]
On Fri, Mar 20, 2026 at 05:27:11PM +0000, Conor Dooley wrote:
> On Fri, Mar 20, 2026 at 10:08:18PM +0800, Carlos Jones Jr wrote:
> > Add support for the LTC2305 2-channel, 12-bit ADC to the existing
> > LTC2497 device tree bindings.
>
> > The LTC2305 is compatible with the
> > LTC2309 driver implementation.
>
> Clearly this is not true, given the driver patches in the series.
Additionally, this is a binding so talk about hardware not drivers.
> pw-bot: changes-requested
>
> Cheers,
> Conor.
>
> >
> > Signed-off-by: Carlos Jones Jr <carlosjr.jones@analog.com>
> > ---
> > .../devicetree/bindings/iio/adc/lltc,ltc2497.yaml | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml
> > index 5cc6a9684077..b246d492950e 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2497.yaml
> > @@ -4,13 +4,19 @@
> > $id: http://devicetree.org/schemas/iio/adc/lltc,ltc2497.yaml#
> > $schema: http://devicetree.org/meta-schemas/core.yaml#
> >
> > -title: Linear Technology / Analog Devices LTC2497 and LTC2309 ADC
> > +title: Linear Technology / Analog Devices LTC2497 and similar ADCs
> >
> > maintainers:
> > - Michael Hennerich <michael.hennerich@analog.com>
> > - Liam Beguin <liambeguin@gmail.com>
> >
> > description: |
> > + LTC2305:
> > + low noise, low power, 2-channel, 12-bit successive approximation ADC with an
> > + I2C compatible serial interface.
> > +
> > + https://www.analog.com/media/en/technical-documentation/data-sheets/2305fa.pdf
> > +
> > LTC2309:
> > low noise, low power, 8-channel, 12-bit successive approximation ADC with an
> > I2C compatible serial interface.
> > @@ -28,6 +34,7 @@ description: |
> > properties:
> > compatible:
> > enum:
> > + - lltc,ltc2305
> > - lltc,ltc2309
> > - lltc,ltc2497
> > - lltc,ltc2499
> > --
> > 2.43.0
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] iio: adc: ltc2309: introduce chip_info structure
[not found] ` <20260320140819.191700-2-carlosjr.jones@analog.com>
@ 2026-03-21 12:51 ` Jonathan Cameron
2026-03-23 10:39 ` Jones, Carlos jr
[not found] ` <ab1dNoC6dg4X-Yml@ashevche-desk.local>
1 sibling, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2026-03-21 12:51 UTC (permalink / raw)
To: Carlos Jones Jr
Cc: Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, David Lechner, Michael Hennerich, Liam Beguin,
Nuno Sá, Andy Shevchenko, Tobias Sperling, Jorge Marques,
linux-iio, devicetree, linux-kernel
On Fri, 20 Mar 2026 22:08:17 +0800
Carlos Jones Jr <carlosjr.jones@analog.com> wrote:
> This is a preparatory patch that introduces a chip_info structure
> to the LTC2309 driver to facilitate adding support for additional
> chip variants with different channel configurations and timing
> requirements.
>
> The chip_info structure contains chip-specific data including
> the channel specifications, number of channels, and read delay
> timing. This change does not modify the existing LTC2309
> functionality.
>
> Signed-off-by: Carlos Jones Jr <carlosjr.jones@analog.com>
Hi Carlos,
Firstly welcome to IIO!
A few comments inline. Some overlap with Andy's review.
Jonathan
> ---
> drivers/iio/adc/ltc2309.c | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/ltc2309.c b/drivers/iio/adc/ltc2309.c
> index 5f0d947d0615..4ea25873398c 100644
> --- a/drivers/iio/adc/ltc2309.c
> +++ b/drivers/iio/adc/ltc2309.c
> @@ -8,6 +8,7 @@
> * Copyright (c) 2023, Liam Beguin <liambeguin@gmail.com>
> */
> #include <linux/bitfield.h>
> +#include <linux/delay.h>
> #include <linux/i2c.h>
> #include <linux/iio/iio.h>
> #include <linux/kernel.h>
> @@ -26,18 +27,26 @@
> #define LTC2309_DIN_UNI BIT(3)
> #define LTC2309_DIN_SLEEP BIT(2)
>
> +struct ltc2309_chip_info {
> + const struct iio_chan_spec *channels;
We now have __counted_by_ptr so you can use that marking
to make it explicit that num_channels is telling us how
many elements channels has.
> + unsigned int num_channels;
> + unsigned int read_delay_us;
> +};
> +
> /**
> * struct ltc2309 - internal device data structure
> * @dev: Device reference
> * @client: I2C reference
> * @lock: Lock to serialize data access
> * @vref_mv: Internal voltage reference
> + * @chip_info: Chip-specific configuration data
See below. Maybe more appropriate to copy the read_delay rather
than keeping pointer to full structure around.
> */
> struct ltc2309 {
> struct device *dev;
> struct i2c_client *client;
> struct mutex lock; /* serialize data access */
> int vref_mv;
> + const struct ltc2309_chip_info *chip_info;
> };
>
> /* Order matches expected channel address, See datasheet Table 1. */
> @@ -117,6 +126,10 @@ static int ltc2309_read_raw_channel(struct ltc2309 *ltc2309,
> return ret;
> }
>
> + if (ltc2309->chip_info->read_delay_us)
> + usleep_range(ltc2309->chip_info->read_delay_us,
> + ltc2309->chip_info->read_delay_us * 2);
Andy covered this. fsleep() provides standard tolerance on
usleeps if we don't care about precise timing (and given it's a sleep
we never get precise timing anyway!)
> +
> ret = i2c_master_recv(ltc2309->client, (char *)&buf, 2);
> if (ret < 0) {
> dev_err(ltc2309->dev, "i2c read failed: %pe\n", ERR_PTR(ret));
> @@ -156,6 +169,12 @@ static const struct iio_info ltc2309_info = {
> .read_raw = ltc2309_read_raw,
> };
>
> +static const struct ltc2309_chip_info ltc2309_chip_info = {
> + .channels = ltc2309_channels,
> + .num_channels = ARRAY_SIZE(ltc2309_channels),
> + .read_delay_us = 0,
> +};
> +
> static int ltc2309_probe(struct i2c_client *client)
> {
> struct iio_dev *indio_dev;
> @@ -169,11 +188,12 @@ static int ltc2309_probe(struct i2c_client *client)
> ltc2309 = iio_priv(indio_dev);
> ltc2309->dev = &indio_dev->dev;
> ltc2309->client = client;
> + ltc2309->chip_info = <c2309_chip_info;
Given only the read_delay_us is used after probe, I'd add a variable for
that and copy just that value over. If you have other changes that
are coming in the near future that will add more fields to the structure
that are needed after probe, then fine to leave it as you have it
(but add a mention in the commit message).
>
> indio_dev->name = "ltc2309";
Given we try to present the actual device name in sysfs, I'd expect to see the
name coming from the chip_info structure as well.
> indio_dev->modes = INDIO_DIRECT_MODE;
> - indio_dev->channels = ltc2309_channels;
> - indio_dev->num_channels = ARRAY_SIZE(ltc2309_channels);
> + indio_dev->channels = ltc2309->chip_info->channels;
> + indio_dev->num_channels = ltc2309->chip_info->num_channels;
> indio_dev->info = <c2309_info;
>
> ret = devm_regulator_get_enable_read_voltage(&client->dev, "vref");
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] iio: adc: ltc2309: add support for LTC2305
[not found] ` <ab1eQ55b_8JiyAGA@ashevche-desk.local>
@ 2026-03-21 12:54 ` Jonathan Cameron
2026-03-23 10:39 ` Jones, Carlos jr
2026-03-23 10:39 ` Jones, Carlos jr
1 sibling, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2026-03-21 12:54 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Carlos Jones Jr, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, David Lechner,
Michael Hennerich, Liam Beguin, Nuno Sá, Andy Shevchenko,
Tobias Sperling, Jorge Marques, linux-iio, devicetree,
linux-kernel
On Fri, 20 Mar 2026 16:48:35 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Fri, Mar 20, 2026 at 10:08:19PM +0800, Carlos Jones Jr wrote:
> > The LTC2305 is a 2-channel, 12-bit, fast ADC with an I2C interface,
> > compatible with the LTC2309 (which has 8 channels).
> >
> > This patch adds support for the LTC2305 by using the chip_info
> > structure to handle the different channel configurations between the
> > two variants. The LTC2305 exposes 2 single-ended channels and 2
> > differential combinations.
> >
> > The LTC2305 requires a 1.6μs delay between I2C write and read
> > operations, which is implemented using chip-specific timing to avoid
> > affecting existing LTC2309 functionality.
>
> ...
>
> > config LTC2309
> > - tristate "Linear Technology LTC2309 ADC driver"
> > + tristate "Linear Technology LTC2309 and similar ADC driver"
> > depends on I2C
> > help
> > - Say yes here to build support for Linear Technology LTC2309, a low
> > - noise, low power, 8-channel, 12-bit SAR ADC
> > + Say yes here to build support for Linear Technology LTC2309 and
> > + similar low noise, low power SAR ADCs.
>
> No, in Kconfig help text (and possibly title above) we have to be crystal
> clear for user what IPs (chips, SoCs, et cetera) are being supported by
> the driver. There is no go for 'and similar'.
One follow up comment is format the resulting description as a list
as it reduces churn in the long run. e.g. how AD7173 does it.
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 1/3] iio: adc: ltc2309: introduce chip_info structure
[not found] ` <ab1dNoC6dg4X-Yml@ashevche-desk.local>
@ 2026-03-23 10:38 ` Jones, Carlos jr
2026-03-23 11:06 ` Jones, Carlos jr
0 siblings, 1 reply; 13+ messages in thread
From: Jones, Carlos jr @ 2026-03-23 10:38 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, David Lechner,
Hennerich, Michael, Liam Beguin, Sa, Nuno, Andy Shevchenko,
Tobias Sperling, Marques, Jorge, linux-iio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
> On Fri, Mar 20, 2026 at 10:08:17PM +0800, Carlos Jones Jr wrote:
> > This is a preparatory patch that introduces a chip_info structure to
> > the LTC2309 driver to facilitate adding support for additional chip
> > variants with different channel configurations and timing
> > requirements.
> >
> > The chip_info structure contains chip-specific data including the
> > channel specifications, number of channels, and read delay timing.
> > This change does not modify the existing LTC2309 functionality.
>
> ...
>
> > struct ltc2309 {
> > struct device *dev;
> > struct i2c_client *client;
> > struct mutex lock; /* serialize data access */
> > int vref_mv;
> > + const struct ltc2309_chip_info *chip_info;
> > };
>
> Have you checked the layout with `pahole` tool? Does it agree with your
> choice?
>
> ...
Will revert addition of const struct ltc2309_chip_info *chip_info since it is
indeed, wasteful to carry the pointer to the full structure when only the
read delay is used after probe.
> > + .read_delay_us = 0,
>
> Unneeded.
>
Will convert to variable to copy this value over instead. Thanks.
>
> > + if (ltc2309->chip_info->read_delay_us)
> > + usleep_range(ltc2309->chip_info->read_delay_us,
> > + ltc2309->chip_info->read_delay_us * 2);
>
> fsleep()
>
> ...
>
> > +static const struct ltc2309_chip_info ltc2309_chip_info = {
> > + .channels = ltc2309_channels,
> > + .num_channels = ARRAY_SIZE(ltc2309_channels),
>
> Perhaps you also want to add (currently missing?) array_size.h.
>
> > +};
>
> --
> With Best Regards,
> Andy Shevchenko
>
Noted and thanks for both fsleep and array_size.h
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 3/3] iio: adc: ltc2309: add support for LTC2305
[not found] ` <ab1eQ55b_8JiyAGA@ashevche-desk.local>
2026-03-21 12:54 ` [PATCH 3/3] iio: adc: ltc2309: add support for LTC2305 Jonathan Cameron
@ 2026-03-23 10:39 ` Jones, Carlos jr
1 sibling, 0 replies; 13+ messages in thread
From: Jones, Carlos jr @ 2026-03-23 10:39 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, David Lechner,
Hennerich, Michael, Liam Beguin, Sa, Nuno, Andy Shevchenko,
Tobias Sperling, Marques, Jorge, linux-iio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
> > - ltc2309->chip_info = <c2309_chip_info;
> > + ltc2309->chip_info = i2c_get_match_data(client);
>
> Strictly speaking this change with the associated ID table changes should go in
> a separate patch.
>
> ...
>
Will do. Thanks.
> > 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 },
>
> Keep it ordered by the value of compatible string.
>
> > { }
> > };
>
> ...
>
> > static const struct i2c_device_id ltc2309_id[] = {
> > - { "ltc2309" },
> > + { "ltc2309", (kernel_ulong_t)<c2309_chip_info },
> > + { "ltc2305", (kernel_ulong_t)<c2305_chip_info },
> > { }
>
> In the similar way as above.
>
> > };
>
> --
> With Best Regards,
> Andy Shevchenko
>
Noted and thanks on the value order.
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 0/3] Add support for LTC2305
[not found] ` <ab1eqIXaav4sVucJ@ashevche-desk.local>
@ 2026-03-23 10:39 ` Jones, Carlos jr
0 siblings, 0 replies; 13+ messages in thread
From: Jones, Carlos jr @ 2026-03-23 10:39 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, David Lechner,
Hennerich, Michael, Liam Beguin, Sa, Nuno, Andy Shevchenko,
Tobias Sperling, Marques, Jorge, linux-iio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
> On Fri, Mar 20, 2026 at 10:08:16PM +0800, Carlos Jones Jr wrote:
> > The LTC2305 is a 2-channel, 12-bit, fast ADC with an I2C interface,
> > compatible with the LTC2309 (which has 8 channels).
> >
> > This patch adds support for the LTC2305 by introducing a chip_info
> > structure to handle the different channel configurations between the
> > two variants. The LTC2305 exposes 2 single-ended channels and 2
> > differential combinations.
> >
> > Also updates the device tree bindings to include the lltc,ltc2305
> > compatible string and documents it in the Kconfig.
>
> There is nothing bad with the series, but hey, ADI, again telling you, deploy the
> internal Wiki and put there most of the typical problems with the code
> submitted by ADI! This series could be done just in one round, if you do this
> and prevent wasting reviewers' time!
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
Thanks for your patience, Andy. We'll do better.
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 2/3] dt-bindings: iio: adc: lltc,ltc2497: add LTC2305 support
2026-03-20 17:27 ` Conor Dooley
@ 2026-03-23 10:39 ` Jones, Carlos jr
0 siblings, 0 replies; 13+ messages in thread
From: Jones, Carlos jr @ 2026-03-23 10:39 UTC (permalink / raw)
To: Conor Dooley
Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, David Lechner,
Hennerich, Michael, Liam Beguin, Sa, Nuno, Andy Shevchenko,
Tobias Sperling, Marques, Jorge, linux-iio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
> On Fri, Mar 20, 2026 at 05:27:11PM +0000, Conor Dooley wrote:
> > On Fri, Mar 20, 2026 at 10:08:18PM +0800, Carlos Jones Jr wrote:
> > > Add support for the LTC2305 2-channel, 12-bit ADC to the existing
> > > LTC2497 device tree bindings.
> >
> > > The LTC2305 is compatible with the
> > > LTC2309 driver implementation.
> >
> > Clearly this is not true, given the driver patches in the series.
>
> Additionally, this is a binding so talk about hardware not drivers.
>
Will revise misleading claim on compatibility and focus text on the
hardware. Thank you.
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 1/3] iio: adc: ltc2309: introduce chip_info structure
2026-03-21 12:51 ` [PATCH 1/3] iio: adc: ltc2309: introduce chip_info structure Jonathan Cameron
@ 2026-03-23 10:39 ` Jones, Carlos jr
2026-03-23 11:00 ` Jones, Carlos jr
0 siblings, 1 reply; 13+ messages in thread
From: Jones, Carlos jr @ 2026-03-23 10:39 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, David Lechner, Hennerich, Michael, Liam Beguin,
Sa, Nuno, Andy Shevchenko, Tobias Sperling, Marques, Jorge,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
> On Fri, 20 Mar 2026 22:08:17 +0800
> Carlos Jones Jr <carlosjr.jones@analog.com> wrote:
>
> > This is a preparatory patch that introduces a chip_info structure to
> > the LTC2309 driver to facilitate adding support for additional chip
> > variants with different channel configurations and timing
> > requirements.
> >
> > The chip_info structure contains chip-specific data including the
> > channel specifications, number of channels, and read delay timing.
> > This change does not modify the existing LTC2309 functionality.
> >
> > Signed-off-by: Carlos Jones Jr <carlosjr.jones@analog.com>
> Hi Carlos,
>
> Firstly welcome to IIO!
>
> A few comments inline. Some overlap with Andy's review.
>
> Jonathan
>
Thank you for the warm welcome, Jonathan.
> > ---
> > drivers/iio/adc/ltc2309.c | 24 ++++++++++++++++++++++--
> > 1 file changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ltc2309.c b/drivers/iio/adc/ltc2309.c
> > index 5f0d947d0615..4ea25873398c 100644
> > --- a/drivers/iio/adc/ltc2309.c
> > +++ b/drivers/iio/adc/ltc2309.c
> > @@ -8,6 +8,7 @@
> > * Copyright (c) 2023, Liam Beguin <liambeguin@gmail.com>
> > */
> > #include <linux/bitfield.h>
> > +#include <linux/delay.h>
> > #include <linux/i2c.h>
> > #include <linux/iio/iio.h>
> > #include <linux/kernel.h>
> > @@ -26,18 +27,26 @@
> > #define LTC2309_DIN_UNI BIT(3)
> > #define LTC2309_DIN_SLEEP BIT(2)
> >
> > +struct ltc2309_chip_info {
> > + const struct iio_chan_spec *channels;
>
> We now have __counted_by_ptr so you can use that marking to make it
> explicit that num_channels is telling us how many elements channels has.
>
Thanks for the instruction, I will apply it to the code.
>
> > */
> > struct ltc2309 {
> > struct device *dev;
> > struct i2c_client *client;
> > struct mutex lock; /* serialize data access */
> > int vref_mv;
> > + const struct ltc2309_chip_info *chip_info;
> > };
> >
> > /* Order matches expected channel address, See datasheet Table 1. */
> > @@ -117,6 +126,10 @@ static int ltc2309_read_raw_channel(struct
> ltc2309 *ltc2309,
> > return ret;
> > }
> >
> > + if (ltc2309->chip_info->read_delay_us)
> > + usleep_range(ltc2309->chip_info->read_delay_us,
> > + ltc2309->chip_info->read_delay_us * 2);
>
> Andy covered this. fsleep() provides standard tolerance on usleeps if we don't
> care about precise timing (and given it's a sleep we never get precise timing
> anyway!)
>
Noted and thanks.
> > + unsigned int num_channels;
> > + unsigned int read_delay_us;
> > +};
> > +
> > /**
> > * struct ltc2309 - internal device data structure
> > * @dev: Device reference
> > * @client: I2C reference
> > * @lock: Lock to serialize data access
> > * @vref_mv: Internal voltage reference
> > + * @chip_info: Chip-specific configuration data
> See below. Maybe more appropriate to copy the read_delay rather than
> keeping pointer to full structure around.
>
> > +
> > ret = i2c_master_recv(ltc2309->client, (char *)&buf, 2);
> > if (ret < 0) {
> > dev_err(ltc2309->dev, "i2c read failed: %pe\n", ERR_PTR(ret));
> @@
> > -156,6 +169,12 @@ static const struct iio_info ltc2309_info = {
> > .read_raw = ltc2309_read_raw,
> > };
> >
> > +static const struct ltc2309_chip_info ltc2309_chip_info = {
> > + .channels = ltc2309_channels,
> > + .num_channels = ARRAY_SIZE(ltc2309_channels),
> > + .read_delay_us = 0,
> > +};
> > +
> > static int ltc2309_probe(struct i2c_client *client) {
> > struct iio_dev *indio_dev;
> > @@ -169,11 +188,12 @@ static int ltc2309_probe(struct i2c_client *client)
> > ltc2309 = iio_priv(indio_dev);
> > ltc2309->dev = &indio_dev->dev;
> > ltc2309->client = client;
> > + ltc2309->chip_info = <c2309_chip_info;
>
> Given only the read_delay_us is used after probe, I'd add a variable for that
> and copy just that value over. If you have other changes that are coming in
> the near future that will add more fields to the structure that are needed after
> probe, then fine to leave it as you have it (but add a mention in the commit
> message).
>
As, I have no visibility into the other similar devices that may require more
fields to the structure, let me revert the structure changes and use a local
variable to deal with the read_delay_us instead. Thanks.
> >
> > indio_dev->name = "ltc2309";
>
> Given we try to present the actual device name in sysfs, I'd expect to see the
> name coming from the chip_info structure as well.
>
Will do, thank you.
> > indio_dev->modes = INDIO_DIRECT_MODE;
> > - indio_dev->channels = ltc2309_channels;
> > - indio_dev->num_channels = ARRAY_SIZE(ltc2309_channels);
> > + indio_dev->channels = ltc2309->chip_info->channels;
> > + indio_dev->num_channels = ltc2309->chip_info->num_channels;
> > indio_dev->info = <c2309_info;
> >
> > ret = devm_regulator_get_enable_read_voltage(&client->dev, "vref");
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 3/3] iio: adc: ltc2309: add support for LTC2305
2026-03-21 12:54 ` [PATCH 3/3] iio: adc: ltc2309: add support for LTC2305 Jonathan Cameron
@ 2026-03-23 10:39 ` Jones, Carlos jr
2026-03-23 11:06 ` Andy Shevchenko
0 siblings, 1 reply; 13+ messages in thread
From: Jones, Carlos jr @ 2026-03-23 10:39 UTC (permalink / raw)
To: Jonathan Cameron, Andy Shevchenko
Cc: Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, David Lechner, Hennerich, Michael, Liam Beguin,
Sa, Nuno, Andy Shevchenko, Tobias Sperling, Marques, Jorge,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
> On Fri, 20 Mar 2026 16:48:35 +0200
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>
> > On Fri, Mar 20, 2026 at 10:08:19PM +0800, Carlos Jones Jr wrote:
> > > The LTC2305 is a 2-channel, 12-bit, fast ADC with an I2C interface,
> > > compatible with the LTC2309 (which has 8 channels).
> > >
> > > This patch adds support for the LTC2305 by using the chip_info
> > > structure to handle the different channel configurations between the
> > > two variants. The LTC2305 exposes 2 single-ended channels and 2
> > > differential combinations.
> > >
> > > The LTC2305 requires a 1.6μs delay between I2C write and read
> > > operations, which is implemented using chip-specific timing to avoid
> > > affecting existing LTC2309 functionality.
> >
> > ...
> >
> > > config LTC2309
> > > - tristate "Linear Technology LTC2309 ADC driver"
> > > + tristate "Linear Technology LTC2309 and similar ADC driver"
> > > depends on I2C
> > > help
> > > - Say yes here to build support for Linear Technology LTC2309, a low
> > > - noise, low power, 8-channel, 12-bit SAR ADC
> > > + Say yes here to build support for Linear Technology LTC2309 and
> > > + similar low noise, low power SAR ADCs.
> >
> > No, in Kconfig help text (and possibly title above) we have to be
> > crystal clear for user what IPs (chips, SoCs, et cetera) are being
> > supported by the driver. There is no go for 'and similar'.
>
> One follow up comment is format the resulting description as a list as it
> reduces churn in the long run. e.g. how AD7173 does it.
>
I was informed about the 'and similar' context in the Kconfig to scale
the driver smoothly but I mistakenly applied it to the help text as well
instead of the title only. Will revise accordingly. BTW, keeping the 'and
similar' context in the title since there were 9 instances of it in Kconfig.
Thank you.
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 1/3] iio: adc: ltc2309: introduce chip_info structure
2026-03-23 10:39 ` Jones, Carlos jr
@ 2026-03-23 11:00 ` Jones, Carlos jr
0 siblings, 0 replies; 13+ messages in thread
From: Jones, Carlos jr @ 2026-03-23 11:00 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, David Lechner, Hennerich, Michael, Liam Beguin,
Sa, Nuno, Andy Shevchenko, Tobias Sperling, Marques, Jorge,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
> > > + unsigned int num_channels;
> > > + unsigned int read_delay_us;
> > > +};
> > > +
> > > /**
> > > * struct ltc2309 - internal device data structure
> > > * @dev: Device reference
> > > * @client: I2C reference
> > > * @lock: Lock to serialize data access
> > > * @vref_mv: Internal voltage reference
> > > + * @chip_info: Chip-specific configuration data
> > See below. Maybe more appropriate to copy the read_delay rather than
> > keeping pointer to full structure around.
> >
> > > +
> > > ret = i2c_master_recv(ltc2309->client, (char *)&buf, 2);
> > > if (ret < 0) {
> > > dev_err(ltc2309->dev, "i2c read failed: %pe\n", ERR_PTR(ret));
> > @@
> > > -156,6 +169,12 @@ static const struct iio_info ltc2309_info = {
> > > .read_raw = ltc2309_read_raw,
> > > };
> > >
> > > +static const struct ltc2309_chip_info ltc2309_chip_info = {
> > > + .channels = ltc2309_channels,
> > > + .num_channels = ARRAY_SIZE(ltc2309_channels),
> > > + .read_delay_us = 0,
> > > +};
> > > +
> > > static int ltc2309_probe(struct i2c_client *client) {
> > > struct iio_dev *indio_dev;
> > > @@ -169,11 +188,12 @@ static int ltc2309_probe(struct i2c_client
> *client)
> > > ltc2309 = iio_priv(indio_dev);
> > > ltc2309->dev = &indio_dev->dev;
> > > ltc2309->client = client;
> > > + ltc2309->chip_info = <c2309_chip_info;
> >
> > Given only the read_delay_us is used after probe, I'd add a variable
> > for that and copy just that value over. If you have other changes
> > that are coming in the near future that will add more fields to the
> > structure that are needed after probe, then fine to leave it as you
> > have it (but add a mention in the commit message).
> >
>
> As, I have no visibility into the other similar devices that may require more
> fields to the structure, let me revert the structure changes and use a local
> variable to deal with the read_delay_us instead. Thanks.
>
Not a local variable, I meant replace the chip info with the read_delay_us
instead.
struct ltc2309 {
struct device *dev;
struct i2c_client *client;
struct mutex lock; /* serialize data access */
int vref_mv;
unsigned int read_delay_us;
};
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] iio: adc: ltc2309: add support for LTC2305
2026-03-23 10:39 ` Jones, Carlos jr
@ 2026-03-23 11:06 ` Andy Shevchenko
0 siblings, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2026-03-23 11:06 UTC (permalink / raw)
To: Jones, Carlos jr
Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, David Lechner,
Hennerich, Michael, Liam Beguin, Sa, Nuno, Andy Shevchenko,
Tobias Sperling, Marques, Jorge, linux-iio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
On Mon, Mar 23, 2026 at 10:39:30AM +0000, Jones, Carlos jr wrote:
> > On Fri, 20 Mar 2026 16:48:35 +0200
> > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
...
> > > > config LTC2309
> > > > - tristate "Linear Technology LTC2309 ADC driver"
> > > > + tristate "Linear Technology LTC2309 and similar ADC driver"
> > > > depends on I2C
> > > > help
> > > > - Say yes here to build support for Linear Technology LTC2309, a low
> > > > - noise, low power, 8-channel, 12-bit SAR ADC
> > > > + Say yes here to build support for Linear Technology LTC2309 and
> > > > + similar low noise, low power SAR ADCs.
> > >
> > > No, in Kconfig help text (and possibly title above) we have to be
> > > crystal clear for user what IPs (chips, SoCs, et cetera) are being
> > > supported by the driver. There is no go for 'and similar'.
> >
> > One follow up comment is format the resulting description as a list as it
> > reduces churn in the long run. e.g. how AD7173 does it.
>
> I was informed about the 'and similar' context in the Kconfig to scale
> the driver smoothly but I mistakenly applied it to the help text as well
> instead of the title only. Will revise accordingly. BTW, keeping the 'and
> similar' context in the title since there were 9 instances of it in Kconfig.
OK, works for me!
*Just always put yourself on the user's position who is not familiar with
the coding and may not even know about programming.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 1/3] iio: adc: ltc2309: introduce chip_info structure
2026-03-23 10:38 ` Jones, Carlos jr
@ 2026-03-23 11:06 ` Jones, Carlos jr
0 siblings, 0 replies; 13+ messages in thread
From: Jones, Carlos jr @ 2026-03-23 11:06 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, David Lechner,
Hennerich, Michael, Liam Beguin, Sa, Nuno, Andy Shevchenko,
Tobias Sperling, Marques, Jorge, linux-iio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
>
> Will revert addition of const struct ltc2309_chip_info *chip_info since it is
> indeed, wasteful to carry the pointer to the full structure when only the read
> delay is used after probe.
>
> > > + .read_delay_us = 0,
> >
> > Unneeded.
> >
>
> Will convert to variable to copy this value over instead. Thanks.
>
Sorry, I meant, I will remove this unneeded setting since it is redundant.
(global struct members are zero-initialized already)
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-03-23 11:07 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260320140819.191700-1-carlosjr.jones@analog.com>
[not found] ` <20260320140819.191700-3-carlosjr.jones@analog.com>
2026-03-20 17:27 ` [PATCH 2/3] dt-bindings: iio: adc: lltc,ltc2497: add LTC2305 support Conor Dooley
2026-03-20 17:27 ` Conor Dooley
2026-03-23 10:39 ` Jones, Carlos jr
[not found] ` <20260320140819.191700-2-carlosjr.jones@analog.com>
2026-03-21 12:51 ` [PATCH 1/3] iio: adc: ltc2309: introduce chip_info structure Jonathan Cameron
2026-03-23 10:39 ` Jones, Carlos jr
2026-03-23 11:00 ` Jones, Carlos jr
[not found] ` <ab1dNoC6dg4X-Yml@ashevche-desk.local>
2026-03-23 10:38 ` Jones, Carlos jr
2026-03-23 11:06 ` Jones, Carlos jr
[not found] ` <20260320140819.191700-4-carlosjr.jones@analog.com>
[not found] ` <ab1eQ55b_8JiyAGA@ashevche-desk.local>
2026-03-21 12:54 ` [PATCH 3/3] iio: adc: ltc2309: add support for LTC2305 Jonathan Cameron
2026-03-23 10:39 ` Jones, Carlos jr
2026-03-23 11:06 ` Andy Shevchenko
2026-03-23 10:39 ` Jones, Carlos jr
[not found] ` <ab1eqIXaav4sVucJ@ashevche-desk.local>
2026-03-23 10:39 ` [PATCH 0/3] Add " Jones, Carlos jr
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox