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: 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.

  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.

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