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

[Jonathan Cameron]

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.

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;