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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <59ef14bb-0576-4660-8de1-be9cdd1a400d@gmail.com>
Date: Sun, 9 Nov 2025 14:15:57 +0530
From: Ajith Anandhan <ajithanandhan0406@...il.com>
To: David Lechner <dlechner@...libre.com>, linux-iio@...r.kernel.org
Cc: jic23@...nel.org, nuno.sa@...log.com, andy@...nel.org, robh@...nel.org,
 krzk+dt@...nel.org, conor+dt@...nel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 2/3] iio: adc: Add support for TI ADS1120

On 11/7/25 9:48 PM, David Lechner wrote:
> On 11/7/25 9:50 AM, Ajith Anandhan wrote:
>> On 10/31/25 2:43 AM, David Lechner wrote:
>>> On 10/30/25 11:34 AM, Ajith Anandhan wrote:
>>>> Add driver for the Texas Instruments ADS1120, a precision 16-bit
>>>> analog-to-digital converter with an SPI interface.
>>>>
> One note about the review process. Any suggestions you agree with, you
> don't need to reply to specifically. You can trim out those parts in
> your reply. It saves time for those reading the replies.
>
>>>> +struct ads1120_state {
>>>> +    struct spi_device    *spi;
>>>> +    struct mutex        lock;
>>>> +    /*
>>>> +     * Used to correctly align data.
>>>> +     * Ensure natural alignment for potential future timestamp support.
>>>> +     */
>>>> +    u8 data[4] __aligned(IIO_DMA_MINALIGN);
>>>> +
>>>> +    u8 config[4];
>>>> +    int current_channel;
>>>> +    int gain;
>>> Since inputs are multiplexed, we can make this gain a per-channel value, no?
>> Yes we can, However i want  to keep the initial version simple so would it be
>>
>> fine to support per-channel gain configurationin upcoming patches?
> Absolutely. I really appreciate splitting things up like that as it makes
> it much easier to review.
>
>>> It also sounds like that certain mux input have to have the PGA bypassed
>>> which means they are limited to only 3 gain values. So these would have
>>> a different scale_available attribute.
>>   Since, I'm gonna enable all the 15 channels. I believe we have to have all
>>
>> gains(for differential channels). Correct me if i'm wrong.
> Yes, that is how I understood the datasheet. Differential channels have all
> gains. Single-ended channels and diagnostic channels only get the low gains
> (1, 2, 4).
>
>
>>>> +static int ads1120_write_reg(struct ads1120_state *st, u8 reg, u8 value)
>>>> +{
>>>> +    u8 buf[2];
>>>> +
>>>> +    if (reg > ADS1120_REG_CONFIG3)
>>>> +        return -EINVAL;
>>>> +
>>>> +    buf[0] = ADS1120_CMD_WREG | (reg << 2);
>>>> +    buf[1] = value;
>>>> +
>>>> +    return spi_write(st->spi, buf, 2);
>>>> +}
>>> Can we use the regmap framework instead of writing our own?
>> I’d like to keep the first version simple so i will add the regmap support in the
>>
>> later patch since the single ended has less spi transaction to handle.
> It would be less churn to implement the regmap right away. Typically
> we try to avoid churn if we can help it. So this would be an exception
> to the general suggestion that splitting things up is better.


Got it, I’ll add regmap support and address all review comments in the v2
patch series.

BR,

Ajith.