Devicetree
 help / color / mirror / Atom feed
From: "Kurt Borja" <kuurtb@gmail.com>
To: "Jonathan Cameron" <jic23@kernel.org>,
	"David Lechner" <dlechner@baylibre.com>
Cc: "Kurt Borja" <kuurtb@gmail.com>, "Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 7/7] iio: adc: Add ti-ads1263-adc2 driver
Date: Tue, 30 Jun 2026 12:57:08 -0500	[thread overview]
Message-ID: <DJMKTP1FHTIY.161J5V5PQF62O@gmail.com> (raw)
In-Reply-To: <20260630020052.4ab593f4@jic23-huawei>

On Mon Jun 29, 2026 at 8:00 PM -05, Jonathan Cameron wrote:
> On Mon, 29 Jun 2026 11:38:26 -0500
> David Lechner <dlechner@baylibre.com> wrote:
>
>> On 6/28/26 3:08 PM, Kurt Borja wrote:
>> > On Sun Jun 28, 2026 at 12:22 PM -05, David Lechner wrote:  
>> >> On 6/28/26 12:36 AM, Kurt Borja wrote:  
>> >>> The TI ADS1263 embeds a second 24-bit delta-sigma ADC (ADC2) with its
>> >>> own input mux, reference, gain and sample-rate selection.
>> >>>
>> >>> Model ADC2 as a separate IIO device on the auxiliary bus: the ti-ads1262
>> >>> SPI driver instantiates the auxiliary device and exports a small set of
>> >>> TI_ADS1262-namespaced helpers for the conversion and register accesses
>> >>> that must go through the shared bus. ADC2 channels are derived from the
>> >>> parent's configured channels.
>> >>>  
>> >> Can these just be additional channels in the main iio device rather
>> >> than a separate iio device?  
>> > 
>> > I guess we can do it, but wouldn't it be quite a mess? I think doing it
>> > that way adds a lot of complexity: channel naming, available scan masks
>> > (because both ADCs can be sampled at the same time), optimized software
>> > sequencing would only work in ADC1 channels, ADC2 doesn't have a DRDY
>> > IRQ, etc.  
>> 
>> Channel naming is easy, e.g. just add 100 to channel and channel2 for
>> ADC1 and 200 for ADC2.

It wouldn't be the end of the world, but it would be confusing without
some documentation or without relying on channel labels.

>> 
>> And if ADC2 is mostly for diagnostics, do we really care about trying
>> to optimize it?

Oh I meant the ADC1 optimization, the ADC2 has no optimization
considerations.

>> 
>> > 
>> > IMO separating both drivers makes everything simpler, easier to
>> > understand and easier to maintain in the future.
>> >   
>> 
>> Sure, I don't have any strong objection to doing this way. We just
>> usually try to avoid multiple IIO devices for a single chip. Although
>> one of the exceptions to this is when a chip has independent cores.
>> I guess this fits that description, although it is a little muddled due
>> to sharing the same input pins, sensor bias, IDACs and probably a few
>> other things - i.e. doing buffered reads on both cores at the same time
>> requires a static IDAC output to avoid issues.

Yes, the IDAC stuff is the most annoying detail. The datasheet claims:

	All input configurations (channel select, IDAC, level-shift,
	sensor bias) are available to ADC2.

and is technically true, but IDAC configuration is not independent so
it's really up to the driver to make it "available" to ADC2.

My approach for now is to not support the IDACs in the ADC2. This makes
sense for me because I can set a static IDAC output on ADC1 and sample
ADC2 on the side.

I can support it later though, shouldn't be too hard. When ADC1 is
sampling just one channel we can -EBUSY, when there's more than one
channel reads are already synchronized so we can just switch the IDAC
without much trouble.

>
> There are (I think) still some gaps in the multibuffer support.
> Ideally we'd have long ago solved those, but without that (I think we
> can't support different triggers for example) I'm not sure we could do
> this as a single device if we support buffered capture.
>
> The question that comes to mind is do the usecases for this 'debug'
> ADC need that support?  If it were just sysfs then a single device
> would be easy to do.  On the other side, splitting it is up later
> isn't something we can easily retrofit.  There is precedence for

IMO the ability to buffer read ADC2 at a much lower trigger rate is not
_absolutely necessary_ but it's still very confortable. And although the
code simplification is not that HUGE, it's still significant to me.

> multiple devices like this for sensor hubs (also driven by the lack
> of complete multibuffer support).  That doesn't hit all the isseus
> here though as I can't immediately think of a case where properties
> set for one iio_dev effect another (I didn't check though!)

All channel properties are independent, except for the IDAC stuff
described above. As for the Sensor Bias (burn-out current stuff), if we
end up only having sysfs sampling (_burnoutraw) then it's not really a
problem.

>
> To me the multidevice support is fine - particularly if it's optional
> - so the sub driver can be skipped if someone doesn't care about
> this extra ADC.
>
> Thanks,
>
> Jonathan

-- 
Thanks,
 ~ Kurt

      reply	other threads:[~2026-06-30 17:57 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-28  5:36 [PATCH v2 0/7] iio: adc: Add TI ADS126X ADC family support Kurt Borja
2026-06-28  5:36 ` [PATCH v2 1/7] dt-bindings: iio: adc: Add TI ADS126x ADC family Kurt Borja
2026-06-28 15:45   ` David Lechner
2026-06-28 19:12     ` Kurt Borja
2026-06-29 14:21       ` David Lechner
2026-06-29 16:27         ` Kurt Borja
2026-06-29 16:43           ` David Lechner
2026-06-30 17:14             ` Kurt Borja
2026-06-30 18:38               ` David Lechner
2026-07-01  0:28                 ` Jonathan Cameron
2026-06-28  5:36 ` [PATCH v2 2/7] iio: adc: Add ti-ads1262 driver Kurt Borja
2026-06-28 17:15   ` David Lechner
2026-06-28 20:00     ` Kurt Borja
2026-06-29 14:38       ` David Lechner
2026-06-30  0:32         ` Jonathan Cameron
2026-06-30 17:17           ` Kurt Borja
2026-06-30  0:28     ` Jonathan Cameron
2026-06-30  0:43   ` Jonathan Cameron
2026-06-28  5:36 ` [PATCH v2 3/7] iio: adc: ti-ads1262: Add channel filter support Kurt Borja
2026-06-28  5:36 ` [PATCH v2 4/7] iio: adc: ti-ads1262: Add excitation current support Kurt Borja
2026-06-30  0:47   ` Jonathan Cameron
2026-06-30 17:18     ` Kurt Borja
2026-06-28  5:36 ` [PATCH v2 5/7] iio: adc: ti-ads1262: Add conversion delay support Kurt Borja
2026-06-30  0:50   ` Jonathan Cameron
2026-06-30 17:23     ` Kurt Borja
2026-06-30 18:44       ` David Lechner
2026-07-01  0:20         ` Jonathan Cameron
2026-06-28  5:36 ` [PATCH v2 6/7] iio: adc: ti-ads1262: Add buffer and trigger support Kurt Borja
2026-06-30  0:54   ` Jonathan Cameron
2026-06-28  5:36 ` [PATCH v2 7/7] iio: adc: Add ti-ads1263-adc2 driver Kurt Borja
2026-06-28 17:22   ` David Lechner
2026-06-28 20:08     ` Kurt Borja
2026-06-29 16:38       ` David Lechner
2026-06-30  1:00         ` Jonathan Cameron
2026-06-30 17:57           ` Kurt Borja [this message]

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=DJMKTP1FHTIY.161J5V5PQF62O@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 \
    /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