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

[Jonathan Cameron]

On Tue, 17 Mar 2026 12:31:06 +0200
Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:

> On Tue, Mar 17, 2026 at 09:54:13AM +0000, Miclaus, Antoniu wrote:
> > > On Mon, Mar 16, 2026 at 03:09:18PM +0000, Miclaus, Antoniu wrote:  
> > > > > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> > > > > Sent: Monday, March 16, 2026 4:42 PM
> > > > > 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:  
> 
> ...
> 
> > > > > > > > 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.  
> > > 
> > > Sounds to me like you need, probably, a virtual device for that.
> > > Maybe even on IIO level. Do we expect more devices like this to
> > > be enabled in the future (or maybe already in tree, but lacking this
> > > feature)?  
> > 
> >  The AD4880 is a fairly unique part - having separate SPI config
> >  interfaces per channel with a shared interleaved data output is not
> >  a common pattern, and the chances of another device like this being
> >  upstreamed are low. Given that Jonathan has already reviewed and
> >  approved a previous version of this series, and the patch has
> >  collected multiple Reviewed-by tags, I'd prefer to keep the current
> >  approach.  
> 
> This...
> 
> > > > The shared resources (supplies, CNV clock, interleaved data stream) are
> > > > not just "glue" - they define the device's operating model.  
> > > 
> > > Sure, like any other resource for MFD (HW speaking).  
> > 
> > MFD models a single device exposing multiple functionally distinct
> > sub-devices. The AD4880 channels are not distinct sub-devices - they share a
> > single interleaved data stream, and splitting them would break
> > synchronized capture.  
> 
> ...and this are important pieces of information. Can you add a summary
> to v7 commit message explaining on the chosen approach?
> 
> > > > 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.  

In this whole discussion this is the one element that I think maybe needs
some additional clarification.

From the datasheet it looks to me like there are actually separate data paths
per channel. There are separate SPI / LVDS data lines.

Its a decision in the backend design / FPGA to fuse them. That is logical
because as you've noted the cnv clock is shared. Or is it?  In the initial
diagram on the datasheet first page it is shown as shared, but there are actually
separate CNV_chA+- and CNV_chB+. It's a draft datasheet, so if you can feedback
to the author it would be good to make it more consistent!

It would be possible to build a similar setup with multiple discrete
ADCs connected to a single backend - I just don't think we've seen that done
yet. 

Have I understood the above correctly?

My initial thoughts if we did would be a virtual / multiplexer
IIO device front end that was a consumer of the two separate IIO ADC device
instances (and probably had a path to their shared backend) + those two
instances would use the same backend. Thus the front end device could
provide the buffer interfaces. Without some prototyping I'm not sure how
we'd pull that together.

Note that I'm not suggesting this is the time to work out how to solve
that setup!

To my mind, this driver is effectively performing role of that front end and
the individual ADC instance drivers all in one. I think it's reasonable
to assume no one is going to buy this chip to use the channels independently
and if they do we can later work out how to make that work without this
initial path getting much in the way.


> > > 
> > > Okay, so it's in a way more complex (like a camera sensor in terms of
> > > data/configuration paths) device. It's now even more looking that the
> > > current approach is a quick hack rather than a solution to make this
> > > properly fit Linux device model.  
> > 
> > The driver uses spi_new_ancillary_device() for the config path and
> >  the IIO backend for the data path - both existing kernel
> >  infrastructure used as intended. No custom abstractions are added.  
> 
> Also would be nice to have something from this.
> 
> So, I think for v7 we need transform this discussion to a summary that covers
> all aspects:
> 
> - why MFD box can't be applied here
> - why regmaps are independent
> - how does it look with data and configuration paths
> - the device is unique and we do not expect more to come (or very little chances)
> - et cetera (or what I forgot to mention)

Agreed on this list.

thanks,

Jonathan


> 
> > > > > > 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.  
> > > 
> > > This... It doesn't fit the data path as far as I read from the above.  
> > 
> > Right, spi_new_ancillary_device() covers only the config path.
> > The data path is handled entirely by the IIO backend with no SPI
> > involvement at runtime.
> >   
> > > > > 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.  
>