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

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 22 Mar 2026 16:26:58 +0000
"Miclaus, Antoniu" <Antoniu.Miclaus@analog.com> wrote:

> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Saturday, March 21, 2026 2:18 PM
> > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>
> > Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael
> > <Michael.Hennerich@analog.com>; David Lechner <dlechner@baylibre.com>;
> > Sa, Nuno <Nuno.Sa@analog.com>; Rob Herring <robh@kernel.org>; Krzysztof
> > Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>;
> > Olivier Moysan <olivier.moysan@foss.st.com>; linux-iio@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> > 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@analog.com> 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.
 
> 
>   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.

Jonathan

> 
> > Jonathan
> >   
> > >
> > > Reviewed-by: David Lechner <dlechner@baylibre.com>
> > > Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > ---
> > > 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;  
>