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

Andy Shevchenko
2026-05-04T08:10:14Z

On Thu, Apr 30, 2026 at 01:16:46PM +0300, Radu Sabau via B4 Relay wrote:

> Add SPI offload support to enable DMA-based, CPU-independent data
> acquisition using the SPI Engine offload framework.
>
> When an SPI offload is available (devm_spi_offload_get() succeeds),
> the driver registers a DMA engine IIO buffer and uses dedicated buffer
> setup operations. If no offload is available the existing software
> triggered buffer path is used unchanged.
>
> Both CNV Burst Mode and Manual Mode support offload, but use different
> trigger mechanisms:
>
> CNV Burst Mode: the SPI Engine is triggered by the ADC's DATA_READY
> signal on the GP pin specified by the trigger-source consumer reference
> in the device tree (one cell = GP pin number 0-3). For this mode the
> driver acts as both an SPI offload consumer (DMA RX stream, message
> optimization) and a trigger source provider: it registers the
> GP/DATA_READY output via devm_spi_offload_trigger_register() so the
> offload framework can match the '#trigger-source-cells' phandle and
> automatically fire the SPI Engine DMA transfer at end-of-conversion.
>
> Manual Mode: the SPI Engine is triggered by a periodic trigger at
> the configured sampling frequency. The pre-built SPI message uses
> the pipelined CNV-on-CS protocol: N+1 16-bit transfers are issued
> for N active channels (the first result is discarded as garbage from
> the pipeline flush) and the remaining N results are captured by DMA.
>
> All offload transfers use 16-bit frames (bits_per_word=16, len=2).
> The channel scan_type (storagebits=16, shift=0, IIO_BE) is shared
> between the software triggered-buffer and offload paths; no separate
> scan_type or channel array is needed for the offload case. The
> ad4691_manual_channels[] array introduced in the triggered-buffer
> commit is reused here: it hides the IIO_CHAN_INFO_OVERSAMPLING_RATIO
> attribute, which is not applicable in Manual Mode.

> Kconfig gains a dependency on IIO_BUFFER_DMAENGINE.

Unneeded detail in the commit message. It's visible from the patch.
You probably wanted to explain "why?" it's required.

...

> #include <linux/regulator/consumer.h>
> #include <linux/reset.h>
> #include <linux/spi/spi.h>
> +#include <linux/spi/offload/consumer.h>
> +#include <linux/spi/offload/provider.h>

You missed at some point types.h (in this patch or earlier).

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

...

> - /* max 16 + 1 NOOP (manual) or 2*16 + 2 (CNV burst). */
> + /*
> + * Non-offload: max 16 + 1 NOOP (manual) or 2*16 + 2 (CNV burst).
> + * Offload reuses this array — both paths are mutually exclusive.
> + */

Can you make sure this will be in the end result from the beginning, id est
in the previous change put it as

/*
* Non-offload: max 16 + 1 NOOP (manual) or 2*16 + 2 (CNV burst).
* Offload is not supported.
*/

OR

/*
* Non-offload: max 16 + 1 NOOP (manual) or 2*16 + 2 (CNV burst).
*/

OR

/*
* max 16 + 1 NOOP (manual) or 2*16 + 2 (CNV burst).
*/

This will minimize the churn.

> struct spi_transfer scan_xfers[34];

...

> +static bool ad4691_offload_trigger_match(struct spi_offload_trigger *trigger,
> + enum spi_offload_trigger_type type,
> + u64 *args, u32 nargs)
> +{
> + return type == SPI_OFFLOAD_TRIGGER_DATA_READY &&
> + nargs == 1 && args[0] <= 3;

Seems there is a room in the previous line for more.

> +}

...

> - ret = regmap_write(st->regmap, AD4691_ACC_MASK_REG,
> - ~bitmap_read(indio_dev->active_scan_mask, 0,
> - iio_get_masklength(indio_dev)) & GENMASK(15, 0));
> + acc_mask = ~bitmap_read(indio_dev->active_scan_mask, 0,
> + iio_get_masklength(indio_dev)) & GENMASK(15, 0);
> + ret = regmap_write(st->regmap, AD4691_ACC_MASK_REG, acc_mask);

Make it like this in the previous patch.

> if (ret)
> goto err_unoptimize;

...

> + offload = devm_kzalloc(dev, sizeof(*offload), GFP_KERNEL);

Do you have device/devres.h already included (either explicitly or via device.h
or platform_device.h)?

> + if (!offload)
> + return -ENOMEM;

--
With Best Regards,
Andy Shevchenko