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

[Andy Shevchenko]

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?

>   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?

>   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?

>   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?

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?

> 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