RE: [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC

[Miclaus, Antoniu]

> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> Sent: Monday, March 16, 2026 4:42 PM
> To: Miclaus, Antoniu <Antoniu.Miclaus@xxxxxxxxxx>
> Cc: Jonathan Cameron <jic23@xxxxxxxxxx>; Lars-Peter Clausen
> <lars@xxxxxxxxxx>; Hennerich, Michael <Michael.Hennerich@xxxxxxxxxx>;
> David Lechner <dlechner@xxxxxxxxxxxx>; Sa, Nuno <Nuno.Sa@xxxxxxxxxx>;
> Andy Shevchenko <andy@xxxxxxxxxx>; Rob Herring <robh@xxxxxxxxxx>;
> Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>; Conor Dooley
> <conor+dt@xxxxxxxxxx>; Olivier Moysan <olivier.moysan@xxxxxxxxxxx>; linux-
> iio@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v6 4/4] iio: adc: ad4080: add support for AD4880 dual-
> channel ADC
> 
> [External]
> 
> On Mon, Mar 16, 2026 at 12:31:09PM +0000, Miclaus, Antoniu wrote:
> > > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> > > Sent: Monday, March 16, 2026 11:57 AM
> > > On Sat, Mar 14, 2026 at 12:00:22PM +0000, Jonathan Cameron wrote:
> > > > On Fri, 13 Mar 2026 16:22:53 +0200
> > > > Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:
> > > > > On Fri, Mar 13, 2026 at 01:58:53PM +0200, Antoniu Miclaus wrote:
> 
> ...
> 
> > > > > - What is special about channel 0?
> > > >
> > > > Nothing.
> > >
> > > Then why code does explicit access to regmap channel 0?
> > > We should have regmap[ch] in all cases in the code.
> > >
> >   There are three places that use channel 0 explicitly, none of which
> >   imply channel 0 is functionally special from a hardware perspective:
> >
> >   1. ad4080_reg_access() - the debugfs reg_access callback has no
> >   channel concept, it's a single (reg, val) interface. We have to
> >   pick one regmap, channel 0 is the default choice. I can improve
> >   the comment to make this clearer.
> 
> Then it's simply wrong. You allow only one channel to be printed. The debugfs
> has to print two channels, no?

The IIO debugfs_reg_access callback signature is fixed by the framework -
it provides (reg, writeval, readval) with no channel parameter.

For v7 I can drop debugfs_reg_access. But doesn't hurt if we have at least half
access to the debugfs for it.
> 
> >   2. ad4080_properties_parse() - uses regmap_get_device(st->regmap[0])
> >   solely to obtain the struct device * for reading DT properties.
> >   The device tree properties live on the parent SPI node, which is
> >   channel 0's device. This isn't "channel 0 is special", it's just
> >   "DT properties belong to the primary SPI device."
> 
> Can we simply pass the struct device to that function?
Yes, can do that in v7 if you think it is absolutely necessary.
> 
> >   3. devm_iio_backend_request_buffer() - requests the buffer from
> >   st->back[0] because all channel data is interleaved into a
> >   single stream (there's an inline comment). Only one buffer is needed.
> 
> But this is not regmap related, is it?
> 
Correct, it's not - I should not have grouped it with the regmap points.

> >   All register configuration (setup, filter, decimation) already uses
> >   regmap[ch] throughout.
> >
> > > > > - Is it okay to communicate with different channels simultaneously?
> > > >
> > > > Yes. They are entirely parallel bits of silicon. Own state machines
> > > > and everything.
> > > > The configuration registers section of the datasheet says:
> > > > "Each channel has it's own independent configuration memory
> > > > accessible through it's separate configuration SPI interface."
> > > >
> > > > >   Wouldn't be a nasty race with HW IO?
> > > >
> > > > Nope. You are talking to different devices (more or less).
> > >
> > > If it's a twins in the package, why do we have a special handling and not
> just
> > > describing two independent devices in the DT/fw?
> >
> > Because they are not fully independent - they share:
> > - Power supplies and voltage reference
> > - The CNV clock (conversion trigger)
> 
> Okay, then why not having a core part and a glue driver that registers as many
> devices as you wish and provides just a common stuff?
> 
Because the AD4880 is not two independent ADCs sharing a package - it is
a single device with a single interleaved data output. Splitting into
separate IIO devices would make synchronized dual-channel capture
impossible from userspace, which is the primary use case for this part.

The shared resources (supplies, CNV clock, interleaved data stream) are
not just "glue" - they define the device's operating model. The
per-channel SPI interfaces exist only for register configuration; the
actual data path is a single stream handled entirely by the backend.

> We have similar (to some extend) cases with SPI/I²C where
> drivers/platform/x86/serial-multi-instantiate.c services as "MFD" for that
> type of busses.
> 
> > - A single interleaved data output stream
> 
> How does it work in non-racy way?
> 

The data path has no software involvement at runtime. The CNV clock
 triggers both channels to sample simultaneously, and the device outputs
 the conversion results as a single interleaved bitstream on the data
 lane(s). The FPGA backend captures this stream directly - no SPI
 register reads are involved in the data path. The only SPI traffic is
 for configuration, and each channel has its own independent SPI
 interface and regmap, so there is no shared bus contention.

> > Describing them as two independent DT nodes would mean duplicating
> > all the shared resources, and more importantly, the data interface
> > is a single interleaved stream feeding into one IIO buffer. Having
> > two separate IIO devices would make synchronized capture impossible
> > from userspace.
> >
> > This is exactly the use case spi_new_ancillary_device() was designed
> > for - a multi-die device sharing a bus with separate chip selects for
> > configuration but common data/clock/power infrastructure.
> 
> See above.
> 
> > > TO me is either something special about channel 0, then we have to
> > > synchronise
> > > accesses, or there is no point to have this patch at all, just make devices to
> > > be the same under the hood and describe as independent pair.
> 
> --
> With Best Regards,
> Andy Shevchenko
>