From: "Kurt Borja" <kuurtb@gmail.com>
To: "David Lechner (TI)" <dlechner@baylibre.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: "Kurt Borja" <kuurtb@gmail.com>,
"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: Mon, 15 Jun 2026 19:18:08 -0500 [thread overview]
Message-ID: <DJA1J8D91ESA.2XU7OCVKN7LXU@gmail.com> (raw)
In-Reply-To: <20260615-iio-adc-ti-ads122c14-v1-0-e6bdadf7cb2b@baylibre.com>
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.
>
> 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.
>
> 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?
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.
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.
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 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 0:18 UTC|newest]
Thread overview: 20+ 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-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-15 22:00 ` [PATCH 2/4] iio: adc: add ti-ads112c14 driver David Lechner (TI)
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-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-16 8:36 ` Andy Shevchenko
2026-06-16 15:55 ` David Lechner
2026-06-16 15:30 ` David Lechner
2026-06-16 0:18 ` Kurt Borja [this message]
2026-06-16 15:21 ` [PATCH 0/4] iio: adc: new ti-ads112c14 driver David Lechner
2026-06-16 17:26 ` Kurt Borja
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=DJA1J8D91ESA.2XU7OCVKN7LXU@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