Re: [RFC PATCH 2/3] iio: adc: Add support for TI ADS1120

Jonathan Cameron
2025-11-09T14:02:21Z

On Fri, 7 Nov 2025 20:10:15 +0530
Ajith Anandhan <ajithanandhan0406@gmail.com> wrote:

> On 10/30/25 11:24 PM, Jonathan Cameron wrote:
> > On Thu, 30 Oct 2025 22:04:10 +0530
> > Ajith Anandhan <ajithanandhan0406@gmail.com> wrote:
> >
> >> Add driver for the Texas Instruments ADS1120, a precision 16-bit
> >> analog-to-digital converter with an SPI interface.
> >>
> >> The driver provides:
> >> - 4 single-ended voltage input channels
> >> - Programmable gain amplifier (1 to 128)
> >> - Configurable data rates (20 to 1000 SPS)
> >> - Single-shot conversion mode
> >>
> >> Link: https://www.ti.com/lit/gpn/ads1120
> > Datasheet:
> >
> >> Signed-off-by: Ajith Anandhan <ajithanandhan0406@gmail.com>
> > Hi Ajith
> >
> > Various comments inline. Mostly superficial stuff but the DMA safety
> > of SPI buffers needs fixing. There is a useful talk from this given
> > by Wolfram Sang if you want to understand more about this
> > https://www.youtube.com/watch?v=JDwaMClvV-s
> >
> > Thanks,
> >
> > Jonathan

Hi Ajith,

A small process thing around efficiency.

Crop your reply to only include things where you are answering questions
or wish the discussion to focus. If you accept changes, just put that stuff
in the change log for the next version.
Save a lot of scrolling and makes it a lot less likely important stuff
will be lost in the noise!

> >> +static int ads1120_read_measurement(struct ads1120_state *st, int channel,
> >> + int *val)
> >> +{
> >> + int ret;
> >> +
> >> + ret = ads1120_set_channel(st, channel);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + /* Start single-shot conversion */
> > This all seems fairly standard so not sure what your RFC question was
> > looking for feedback on wrt to how you did single conversions?
>
> I was indeed concerned about using the polling(adding wait) method to
> read adc values.
>
> That's the reason i have asked it in the cover letter.
Ok. A bit more detail next time on what you want feedback on will
help focus things.

>
> >
> >> + ret = ads1120_write_cmd(st, ADS1120_CMD_START);
> >> + if (ret)

Thanks,

Jonathan