Re: [PATCH 4/4] iio: adc: ad4691: add SPI offload support

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 05 Mar 2026 14:23:30 +0200
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:

> From: Radu Sabau <radu.sabau@analog.com>
> 
> Add SPI offload support to the AD4691 family driver to enable
> DMA-based RX stream acquisition. When an SPI offload is available,
> the driver switches to a pre-built SPI message with 32-bit transfers
> (4-byte frames aligned to DMA width) and registers a periodic
> offload trigger for autonomous, CPU-independent sampling.
> 
> The offload path implements its own buffer setup ops
> (ad4691_offload_buffer_postenable/predisable) that enable the
> offload trigger and wire the DMAengine buffer, while the existing
> software triggered buffer path is retained as a fallback for
> non-offload configurations.
> 
> Offload channel specs use a 32-bit storage/repeat with a 16-bit
> shift to extract ADC data from the MSBytes of each DMA word,
> matching the wire format in Manual Mode where SDO outputs ADC data
> directly without a command echo.
> 
> Kconfig gains a dependency on IIO_BUFFER_DMAENGINE.
> 
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>

Just a few really quick comments as I'm out of time for today.

J

> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> index ab48f336e46c..7ec0a2555a4b 100644
> --- a/drivers/iio/adc/ad4691.c
> +++ b/drivers/iio/adc/ad4691.c
> @@ -8,6 +8,7 @@
>  #include <linux/clk.h>
>  #include <linux/delay.h>
>  #include <linux/device.h>
> +#include <linux/dmaengine.h>
>  #include <linux/err.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/hrtimer.h>
> @@ -21,11 +22,15 @@
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/spi/spi.h>
> +#include <linux/spi/offload/consumer.h>
> +#include <linux/spi/offload/provider.h>

This is a provider and a consumer?

>  #include <linux/util_macros.h>
>  #include <linux/units.h>
>  #include <linux/unaligned.h>

> @@ -719,7 +871,9 @@ static int ad4691_set_sampling_freq(struct iio_dev *indio_dev, unsigned int freq
>  		 * count. The exact period is refined at buffer enable time when
>  		 * the active channel count is known.
>  		 */
> -		period_ns = ad4691_cnv_burst_period_ns(st, st->chip->num_channels);
> +		period_ns = ad4691_cnv_burst_period_ns(st,
> +							st->chip->num_channels,
Check for reformatting like occurred for 1st two lines here and try and
tweak earlier patches to reduce the churn.
> +							false);

> +
> +static int ad4691_offload_buffer_predisable(struct iio_dev *indio_dev)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	struct spi_offload_trigger *trigger;
> +	int ret = 0, tmp;
> +
> +	trigger = (st->adc_mode == AD4691_MANUAL_MODE) ?
> +		  st->offload_trigger_periodic : st->offload_trigger;
> +
> +	spi_offload_trigger_disable(st->offload, trigger);
> +	spi_unoptimize_message(&st->offload_msg);
> +
> +	/* Stop conversions and reset sequencer state (not needed for MANUAL_MODE) */
> +	if (st->adc_mode != AD4691_MANUAL_MODE) {
> +		tmp = ad4691_sampling_enable(st, false);
> +		if (!ret)
> +			ret = tmp;
> +
> +		tmp = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
> +				   AD4691_SEQ_ALL_CHANNELS_OFF);
> +		if (!ret)

If that failed, all bets are off.  May be better to just return the error.

> +			ret = tmp;
> +
> +		tmp = regmap_write(st->regmap, AD4691_STATE_RESET_REG,
> +				   AD4691_STATE_RESET_ALL);
> +		if (!ret)
> +			ret = tmp;
> +	}
> +
> +	return ret;
> +}

>  static irqreturn_t ad4691_irq(int irq, void *private)
>  {
>  	struct iio_dev *indio_dev = private;
> @@ -1353,10 +1802,17 @@ static void ad4691_setup_channels(struct iio_dev *indio_dev,
>  				  struct ad4691_state *st)
>  {
>  	if (st->adc_mode == AD4691_MANUAL_MODE) {
> -		if (st->chip->num_channels == 8)
> -			indio_dev->channels = ad4693_manual_channels;
> -		else
> -			indio_dev->channels = ad4691_manual_channels;
> +		if (st->offload) {

Add more pointers to channel arrays to the chip_info structures and just look them
up from there.

> +			if (st->chip->num_channels == 8)
> +				indio_dev->channels = ad4693_manual_offload_channels;
> +			else
> +				indio_dev->channels = ad4691_manual_offload_channels;
> +		} else {
> +			if (st->chip->num_channels == 8)
> +				indio_dev->channels = ad4693_manual_channels;
> +			else
> +				indio_dev->channels = ad4691_manual_channels;
> +		}