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

[Miclaus, Antoniu]

> -----Original Message-----
> From: Jonathan Cameron <jic23@xxxxxxxxxx>
> Sent: Sunday, March 22, 2026 7:39 PM
> To: Miclaus, Antoniu <Antoniu.Miclaus@xxxxxxxxxx>
> Cc: Lars-Peter Clausen <lars@xxxxxxxxxx>; Hennerich, Michael
> <Michael.Hennerich@xxxxxxxxxx>; David Lechner <dlechner@xxxxxxxxxxxx>;
> Sa, Nuno <Nuno.Sa@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 v7 4/4] iio: adc: ad4080: add support for AD4880 dual-
> channel ADC
> 
> [External]
> 
> On Sun, 22 Mar 2026 16:26:58 +0000
> "Miclaus, Antoniu" <Antoniu.Miclaus@xxxxxxxxxx> wrote:
> 
> > > -----Original Message-----
> > > From: Jonathan Cameron <jic23@xxxxxxxxxx>
> > > Sent: Saturday, March 21, 2026 2:18 PM
> > > To: Miclaus, Antoniu <Antoniu.Miclaus@xxxxxxxxxx>
> > > Cc: Lars-Peter Clausen <lars@xxxxxxxxxx>; Hennerich, Michael
> > > <Michael.Hennerich@xxxxxxxxxx>; David Lechner
> <dlechner@xxxxxxxxxxxx>;
> > > Sa, Nuno <Nuno.Sa@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 v7 4/4] iio: adc: ad4080: add support for AD4880
> dual-
> > > channel ADC
> > >
> > > [External]
> > >
> > > On Sat, 21 Mar 2026 12:01:54 +0200
> > > Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx> wrote:
> > >
> > > > Add support for the AD4880, a dual-channel 20-bit 40MSPS SAR ADC
> with
> > > > integrated fully differential amplifiers (FDA).
> > > >
> > > > The AD4880 has two independent ADC channels, each with its own SPI
> > > > configuration interface. The driver uses spi_new_ancillary_device() to
> > > > create an additional SPI device for the second channel, allowing both
> > > > channels to share the same SPI bus with different chip selects.
> > >
> > > Silly question - can we be sure that they both are on the same SPI bus?
> > > I think it's reasonable to assume no one would burn pins to wire the
> > > control interfaces up to separate busses. I'm not even sure how we'd
> > > do a binding if they were on separate busses.
> > >
> > > Otherwise, a follow on from the 'is it one backend or two' question
> > > on the binding.
> > >
> > > That long discussion between you and Andy has me looking at this a little
> > > more closely.
> >
> >   Yes, both channels are on the same SPI bus with two chip
> >   selects — one per internal ADC die.
> 
> Can you give a reference for this?  The datasheet I'm looking at
> has separate cs, sclk, sdi and sdo per channel.  So it would be
> a board thing only that puts them on the same SPI bus.

The AD4880 datasheet shows both topologies — Figure 70 with fully independent SPI interfaces
and Figure 71 with shared SCLK/SDI and separate chip selects.

The config SPI is only used for register access — the actual data capture happens over LVDS, where
both channels are interleaved into one stream no matter how the config SPI is wired. Since the
driver treats this as a single IIO device with one buffer (matching the interleaved stream), the
shared-bus model with spi_new_ancillary_device() fits naturally IMO — one probe, one device, one IIO
instance.

> 
> >
> >   For the backend question — as explained in my reply to the
> >   binding patch, the FPGA uses two separate axi_ad408x IP
> >   instances. The buffer is requested from back[0] because
> >   the packer output feeds the DMA through backend A's clock
> >   domain.
> 
> Ok. So given the packer part makes this really one FPGA IP
> with two 'backend interfaces'.
> 
> Lets be clear about that in the comments etc.
> 
Sure, can do that in v8.
> Jonathan
> 
> >
> > > Jonathan
> > >
> > > >
> > > > Reviewed-by: David Lechner <dlechner@xxxxxxxxxxxx>
> > > > Reviewed-by: Nuno Sá <nuno.sa@xxxxxxxxxx>
> > > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx>
> > > > ---
> > > > Changes in v7:
> > > >   - Drop debugfs_reg_access for dual-channel AD4880 variant
> > > >   - Pass struct device * to ad4080_properties_parse() instead of
> > > >     using regmap_get_device(st->regmap[0])
> > > >
> > > >  drivers/iio/adc/ad4080.c | 231 ++++++++++++++++++++++++++++++-
> ----
> > > ----
> > > >  1 file changed, 181 insertions(+), 50 deletions(-)
> > > >
> > > > diff --git a/drivers/iio/adc/ad4080.c b/drivers/iio/adc/ad4080.c
> > > > index 7cf3b6ed7940..8767eef418e9 100644
> > > > --- a/drivers/iio/adc/ad4080.c
> > > > +++ b/drivers/iio/adc/ad4080.c
> > >
> > > > @@ -632,9 +752,10 @@ static int ad4080_probe(struct spi_device *spi)
> > > >  	indio_dev->name = st->info->name;
> > > >  	indio_dev->channels = st->info->channels;
> > > >  	indio_dev->num_channels = st->info->num_channels;
> > > > -	indio_dev->info = &ad4080_iio_info;
> > > > +	indio_dev->info = st->info->num_channels > 1 ?
> > > > +			  &ad4880_iio_info : &ad4080_iio_info;
> > > >
> > > > -	ret = ad4080_properties_parse(st);
> > > > +	ret = ad4080_properties_parse(st, dev);
> > > >  	if (ret)
> > > >  		return ret;
> > > >
> > > > @@ -644,15 +765,23 @@ static int ad4080_probe(struct spi_device
> *spi)
> > > >
> > > >  	st->clk_rate = clk_get_rate(clk);
> > > >
> > > > -	st->back = devm_iio_backend_get(dev, NULL);
> > > > -	if (IS_ERR(st->back))
> > > > -		return PTR_ERR(st->back);
> > > > +	/* Get backends for all channels */
> > > > +	for (unsigned int ch = 0; ch < st->info->num_channels; ch++) {
> > > > +		st->back[ch] = devm_iio_backend_get_by_index(dev, ch);
> > > > +		if (IS_ERR(st->back[ch]))
> > > > +			return PTR_ERR(st->back[ch]);
> > > >
> > > > -	ret = devm_iio_backend_request_buffer(dev, st->back, indio_dev);
> > > > -	if (ret)
> > > > -		return ret;
> > > > +		ret = devm_iio_backend_enable(dev, st->back[ch]);
> > > > +		if (ret)
> > > > +			return ret;
> > > > +	}
> > > >
> > > > -	ret = devm_iio_backend_enable(dev, st->back);
> > > > +	/*
> > > > +	 * Request buffer from the first backend only. For multi-channel
> > > > +	 * devices (e.g., AD4880), all backends share a single IIO buffer
> > > > +	 * as data from all ADC channels is interleaved into one stream.
> > > > +	 */
> > > > +	ret = devm_iio_backend_request_buffer(dev, st->back[0], indio_dev);
> > >
> > > So this is the interleaving bit.  Follows on from my question on the binding
> > > and whether it is appropriate to represent it as two separate backends
> > > vs a single one.  With a single one we'd need to make the control interfaces
> > > take a parameter to say which 'front end' we were configuring - though it
> > > kind of maps to channels in the particular case and we already have
> > > a parameter for that.
> > >
> > > The other option might be to make the dt-binding take a phandle + index to
> > > say this backend, with this front end interface.
> > >
> > > >  	if (ret)
> > > >  		return ret;
> >