From: "Kurt Borja" <kuurtb@gmail.com>
To: "David Lechner" <dlechner@baylibre.com>,
"Kurt Borja" <kuurtb@gmail.com>,
"Jonathan Cameron" <jic23@kernel.org>,
"Nuno Sá" <nuno.sa@analog.com>,
"Andy Shevchenko" <andy@kernel.org>,
"Rob Herring" <robh@kernel.org>,
"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
"Conor Dooley" <conor+dt@kernel.org>
Cc: "Nguyen Minh Tien" <zizuzacker@gmail.com>,
<linux-iio@vger.kernel.org>, <devicetree@vger.kernel.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 0/4] iio: adc: new ti-ads112c14 driver
Date: Tue, 16 Jun 2026 12:26:58 -0500 [thread overview]
Message-ID: <DJANEYYA4QTA.1JBN2L78PNXDD@gmail.com> (raw)
In-Reply-To: <d3270250-ae18-4c0f-a0fe-e0fdabfce046@baylibre.com>
On Tue Jun 16, 2026 at 10:21 AM -05, David Lechner wrote:
> On 6/15/26 7:18 PM, Kurt Borja wrote:
>> Hi David,
>>
>> On Mon Jun 15, 2026 at 4:59 PM -05, David Lechner (TI) wrote:
>>> This adds support for TI ADS112C14 and ADS122C14 ADC chips.
>>>
>>> The closest thing we've seen to this in the kernel already is ads124s08.
>>> However, that has a completely different register map and the DT
>>> bindings are incomplete and the driver is extremely basic. So I've just
>>> started from scratch here.
>>>
>>> We've also had a similar submission recently for ADS1220 [1]. That chip
>>> is in a similar situation to ads124s08 in that it has a different
>>> register map (but the submitted DT bindings are better than the ones for
>>> ads124s08, even if still a bit incomplete). And literally as I was
>>> writing the previous sentence, another series [2] was sent for yet
>>> another similar family of chips (ADS1262). That one is even more complex
>>> in the feature set than the ones I am working on. I was going to polish
>>> up the driver a bit more before submitting it, but now it seems more
>>> urgent to coordinate with the other two series to align on how we would
>>> like to handle all of these.
>>>
>>> [1]: https://lore.kernel.org/linux-iio/20260610151342.44274-1-zizuzacker@gmail.com/
>>> [2]: https://lore.kernel.org/linux-iio/20260612-ads126x-v1-0-894c788d03ed@gmail.com/
>>>
>>> All of these chips have in common that they are designed for use with
>>> RTDs and thermocouples and so they look very similar to each other in
>>> terms of wiring and feature set, even if the register maps are
>>> different. They are in the gray area where we could either keep them
>>> separate because they are just different enough, or we could do like
>>> we've done before with ad_sigma_delta and have a bit of an abstraction
>>> layer for the register differences and otherwise try to share as much
>>> code as possible. Normally, I would lean towards keeping them separate,
>>> but in this case, I'm considering trying to share code because the
>>> devicetree bindings for the inputs is complex and is going to be mostly
>>> the same across all of these chips.
>>
>> The channel configuration is indeed very similar for the three chips.
>> All three have IDAC, BOC and VREF configurations.
>
> Hmm... I forgot to include the burnout current in the DT bindings. Following
> the channel = "conditions for measurement" pattern that I have set out here
> I guess that would mean that we would need to have the same inputs twice
> when using the burnout. One "channel" would be the one used to do a "precision"
> measurement and the other would be the one to do open/short circuit detection.
>
>
> i2c {
> #address-cells = <1>;
> #size-cells = <0>;
>
> adc@40 {
> compatible = "ti,ads112c14";
> reg = <0x40>;
>
> avdd-supply = <&avdd>;
> dvdd-supply = <&dvdd>;
>
> refp-supply = <&avdd>;
>
> #address-cells = <1>;
> #size-cells = <0>;
>
> channel@0 {
> reg = <0>;
> diff-channels = <1>, <2>;
> excitation-channels = <0>, <3>;
> excitation-current-microamp = <500>;
> current-chopping;
> ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
> label = "rtd-precision";
> };
>
> channel@1 {
> reg = <0>;
> diff-channels = <1>, <2>;
> excitation-channels = <0>, <3>;
> excitation-current-microamp = <500>;
> burnout-current-nanoamp = <1000>;
> ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
> label = "rtd-diagnostic";
> };
This would mean we wouldn't be able to use iio_chan_spec .channel and
.channel2 to describe inputs because of duplicate sysfs attributes, no?
> };
> };
>
>>
>>>
>>> If we decide to go the route of sharing code, we could still merge this
>>> series as-is and then do the refactoring to add the abstraction layer in
>>> a follow-up series that also adds support for the first of the other
>>> chips.
>>
>> Do you have a proposal of how such an abstraction would look like? I do
>> like the idea of abstracting the firmware parsing, scales and shared
>> calculations.
>
> As mentioned, it would look a bit like the ad_sigma_delta ADC driver.
> Basically, each chip (family with same registers) would still have it's
> own driver that include the chip-specific const info structs. In this
> case, these would also include function pointers for all of the functions
> that need to access registers. And there will be lots of supports_... bool
> flags for the shared code to use.
>
>>
>>>
>>> This series includes just basic support for reading single measurements
>>> from the ADC and gain selection via the scale attribute. I plan to
>>> follow this up with additional series to add support for buffered reads,
>>> filtering/oversampling configuration, event support, gpio controller
>>> support and perhaps a few other things that are slipping my mind right
>>> now.
>>>
>>> The most interesting part about this (that I alluded to above) is the
>>> way channels are handled. These are multipling ADCs with differential
>>> and single-ended inputs. But what sets them apart from other similar
>>> chips is that since they are designed for use with RTDs, there can also
>>> be a current output required to excite the RTD and this current output
>>> might be different for different channels. So the way I conceptualized
>>> the channels is that the devicetree specifies the conditions needed
>>> to take a particular measurement rather than being purely a physical
>>> channel.
>>>
>>> This makes things more flexible, but does make the driver a bit more
>>> complex. For example, knowing when the current output needs to be
>>> enabled or disabled. For now, I have chosen a lazy-enable where they
>>> are not turned on until the first measurement is taken that requires
>>> them, but then they stay on until another measurement is taken that
>>> doesn't require them. This can lead to some oddness with the diagnostic
>>> channels that may be measuring something that indirectly requires the
>>> current output (i.e. the external reference voltage when it is connected
>>> to a resistor rather than a power supply). This means you need to take
>>> a measurement that requires the current output to be enabled before the
>>> diagnostic channels will give accurate readings.
>>
>> This is the same approach I took around the BOC, it feels kinda hacky
>> but it makes sense. Just an idea I thought about just now: What if we
>> have an additional write-only "_enable" sysfs attribute for these
>> channels?
>
> I would not want to make a write-only attribute, we always want to be
> able to read back what the current state is.
Yeah, I don't know why I said WO. Reading would be fine too.
>
> Do you mean an _enable for just the BOC? I think I would do it like I
> suggested above instead.
No, no just the BOC. The BOC, IDAC and rest of side effects. Thinking
about it some more, it would be a bit redundant but clearer if proper
documentation is provided.
>
>>
>> The approach I took for the IDAC was to have a single configuration that
>> it's enabled for all channels. This makes some sense in my device when
>> thinking about optimal software sequencial reads, because of the
>> register layout, but I also see the value in having per-channel IDAC
>> configuration. I think I will take your approach, so we have the same
>> channel configuration around this.
>
> There are only two IDACs but on chips with 8 AIN pins we could have two
> RTDs wired up and if both require 2 IDACs, then we can't have a fixed
> setting for the IDACs. This is why I made it per-channel.
I'll make it per-channel too.
>
>>
>> Have you thought about how to implement the BOC? In the ADS1262 the
>> feature can be found "Sensor Bias". What I did was add per channel DT
>> properties for this too.
>
> See above.
>
>>
>> Another question. When you implement power management in the future,
>> will you enable autosuspend? IDAC currents will be lost if autosuspend
>> is enabled. Is this acceptable? In my case I did enable autosuspend, but
>> I have some doubts about this.
>
> I don't like to implement power management unless I have an application
> that actually requires it. Otherwise, I consider it premature optimization.
> It is difficult to know if we would be implementing it in a way that is
> actually useful for a real-world use case. These chips seem like they
> would mostly be used in an industrial setting, not in battery powered
> applications, so I'm not sure anyone is going to worry about saving a few
> milliamps.
Makes sense. I can probably drop the autosuspend in that case.
>
>>
>>>
>>> I have also pushed a branch to [3] that contains the start of some
>>> documentation for this driver that can give some more insight into how
>>> the implementation works. It still needs some work and also documents
>>> some things that haven't been implemented yet, so I haven't included it
>>> in this series yet.
>>>
>>> [3]: https://github.com/dlech/linux/blob/b4/iio-adc-ti-ads122c14/Documentation/iio/ads112c14.rst
>>>
>>> Signed-off-by: David Lechner <dlechner@baylibre.com>
>>> ---
>>> David Lechner (TI) (4):
>>> dt-bindings: iio: adc: add ti,ads122c14
>>> iio: adc: add ti-ads112c14 driver
>>> iio: adc: ti-ads112c14: implement gain on internal short SYS_MON channel
>>> iio: adc: ti-ads112c14: add measurement channel support
>>>
>>> .../devicetree/bindings/iio/adc/ti,ads112c14.yaml | 224 +++++
>>> MAINTAINERS | 8 +
>>> drivers/iio/adc/Kconfig | 12 +
>>> drivers/iio/adc/Makefile | 1 +
>>> drivers/iio/adc/ti-ads112c14.c | 1053 ++++++++++++++++++++
>>> include/dt-bindings/iio/adc/ti,ads112c14.h | 11 +
>>> 6 files changed, 1309 insertions(+)
>>> ---
>>> base-commit: ec039126b7fac4e3af35ebccaa7c6f9b6875ba81
>>> change-id: 20260514-iio-adc-ti-ads122c14-d0b92479334e
>>>
>>> Best regards,
>>> --
>>> David Lechner (TI) <dlechner@baylibre.com>
>>
--
Thanks,
~ Kurt
next prev parent reply other threads:[~2026-06-16 17:27 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-15 21:59 [PATCH 0/4] iio: adc: new ti-ads112c14 driver David Lechner (TI)
2026-06-15 21:59 ` [PATCH 1/4] dt-bindings: iio: adc: add ti,ads122c14 David Lechner (TI)
2026-06-15 22:10 ` sashiko-bot
2026-06-16 0:26 ` Kurt Borja
2026-06-16 15:22 ` David Lechner
2026-06-16 17:31 ` Kurt Borja
2026-06-16 16:07 ` Conor Dooley
2026-06-16 19:54 ` David Lechner
2026-06-16 20:50 ` Conor Dooley
2026-06-16 21:04 ` David Lechner
2026-06-15 22:00 ` [PATCH 2/4] iio: adc: add ti-ads112c14 driver David Lechner (TI)
2026-06-15 22:11 ` sashiko-bot
2026-06-16 7:32 ` Andy Shevchenko
2026-06-16 15:38 ` David Lechner
2026-06-15 22:00 ` [PATCH 3/4] iio: adc: ti-ads112c14: implement gain on internal short SYS_MON channel David Lechner (TI)
2026-06-15 22:14 ` sashiko-bot
2026-06-16 7:58 ` Andy Shevchenko
2026-06-16 10:03 ` Nuno Sá
2026-06-15 22:00 ` [PATCH 4/4] iio: adc: ti-ads112c14: add measurement channel support David Lechner (TI)
2026-06-15 22:13 ` sashiko-bot
2026-06-16 8:36 ` Andy Shevchenko
2026-06-16 15:55 ` David Lechner
2026-06-16 15:30 ` David Lechner
2026-06-16 0:18 ` [PATCH 0/4] iio: adc: new ti-ads112c14 driver Kurt Borja
2026-06-16 15:21 ` David Lechner
2026-06-16 17:26 ` Kurt Borja [this message]
2026-06-16 18:16 ` David Lechner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DJANEYYA4QTA.1JBN2L78PNXDD@gmail.com \
--to=kuurtb@gmail.com \
--cc=andy@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dlechner@baylibre.com \
--cc=jic23@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nuno.sa@analog.com \
--cc=robh@kernel.org \
--cc=zizuzacker@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox