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

David Lechner
2026-04-10T21:00:47Z

On 4/9/26 10:28 AM, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@analog.com>
>
> 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.
>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
> drivers/iio/adc/Kconfig | 2 +
> drivers/iio/adc/ad4691.c | 398 ++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 395 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index d498f16c0816..fdc6565933c5 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -143,8 +143,10 @@ config AD4691
> tristate "Analog Devices AD4691 Family ADC Driver"
> depends on SPI
> select IIO_BUFFER
> + select IIO_BUFFER_DMAENGINE
> select IIO_TRIGGERED_BUFFER
> select REGMAP
> + select SPI_OFFLOAD
> help
> Say yes here to build support for Analog Devices AD4691 Family MuxSAR
> SPI analog to digital converters (ADC).
> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> index 3e5caa0972eb..839ea7f44c78 100644
> --- a/drivers/iio/adc/ad4691.c
> +++ b/drivers/iio/adc/ad4691.c
> @@ -22,6 +22,8 @@
> #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>
> #include <linux/units.h>
> #include <linux/unaligned.h>
>
> @@ -43,6 +45,11 @@
>
> #define AD4691_CNV_DUTY_CYCLE_NS 380
> #define AD4691_CNV_HIGH_TIME_NS 430
> +/*
> + * Conservative default for the manual offload periodic trigger. Low enough
> + * to work safely out of the box across all OSR and channel count combinations.
> + */
> +#define AD4691_OFFLOAD_INITIAL_TRIGGER_HZ (100 * HZ_PER_KHZ)
>
> #define AD4691_SPI_CONFIG_A_REG 0x000
> #define AD4691_SW_RESET (BIT(7) | BIT(0))
> @@ -95,6 +102,8 @@
> #define AD4691_ACC_IN(n) (0x252 + (3 * (n)))
> #define AD4691_ACC_STS_DATA(n) (0x283 + (4 * (n)))
>
> +#define AD4691_OFFLOAD_BITS_PER_WORD 16

This is just the same as realbits in scan info. So could use that
directly instead.

> +
> static const char * const ad4691_supplies[] = { "avdd", "vio" };
>
> enum ad4691_ref_ctrl {
> @@ -114,6 +123,7 @@ struct ad4691_chip_info {
> const char *name;
> unsigned int max_rate;
> const struct ad4691_channel_info *sw_info;
> + const struct ad4691_channel_info *offload_info;
> };
>
> #define AD4691_CHANNEL(ch) \
> @@ -177,6 +187,18 @@ static const struct ad4691_channel_info ad4693_sw_info = {
> .num_channels = ARRAY_SIZE(ad4693_channels),
> };
>
> +static const struct ad4691_channel_info ad4691_offload_info = {
> + .channels = ad4691_channels,
> + /* No soft timestamp; num_channels caps access to 16. */
> + .num_channels = 16,

`ARRAY_SIZE(ad4691_channels) - 1` would make sense too.

> +};
> +
> +static const struct ad4691_channel_info ad4693_offload_info = {
> + .channels = ad4693_channels,
> + /* No soft timestamp; num_channels caps access to 8. */
> + .num_channels = 8,
> +};
> +
> /*
> * Internal oscillator frequency table. Index is the OSC_FREQ_REG[3:0] value.
> * Index 0 (1 MHz) is only valid for AD4692/AD4694; AD4691/AD4693 support
> @@ -207,24 +229,36 @@ static const struct ad4691_chip_info ad4691_chip_info = {
> .name = "ad4691",
> .max_rate = 500 * HZ_PER_KHZ,
> .sw_info = &ad4691_sw_info,
> + .offload_info = &ad4691_offload_info,
> };
>
> static const struct ad4691_chip_info ad4692_chip_info = {
> .name = "ad4692",
> .max_rate = 1 * HZ_PER_MHZ,
> .sw_info = &ad4691_sw_info,
> + .offload_info = &ad4691_offload_info,
> };
>
> static const struct ad4691_chip_info ad4693_chip_info = {
> .name = "ad4693",
> .max_rate = 500 * HZ_PER_KHZ,
> .sw_info = &ad4693_sw_info,
> + .offload_info = &ad4693_offload_info,
> };
>
> static const struct ad4691_chip_info ad4694_chip_info = {
> .name = "ad4694",
> .max_rate = 1 * HZ_PER_MHZ,
> .sw_info = &ad4693_sw_info,
> + .offload_info = &ad4693_offload_info,
> +};
> +
> +struct ad4691_offload_state {
> + struct spi_offload *spi;

I would call this "offload" or "instance". "spi" is usally the SPI
device handle.

> + struct spi_offload_trigger *trigger;
> + u64 trigger_hz;
> + u8 tx_cmd[17][2];
> + u8 tx_reset[4];
> };
>

...

> +
> +static int ad4691_cnv_burst_offload_buffer_predisable(struct iio_dev *indio_dev)
> +{
> + struct ad4691_state *st = iio_priv(indio_dev);
> + struct ad4691_offload_state *offload = st->offload;
> + int ret;
> +
> + spi_offload_trigger_disable(offload->spi, offload->trigger);
> +
> + ret = ad4691_sampling_enable(st, false);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
> + AD4691_SEQ_ALL_CHANNELS_OFF);

Why this extra step? We don't have it when unwinding in the
error path of the postenable function.

> + if (ret)
> + return ret;
> +
> + spi_unoptimize_message(&st->scan_msg);
> +
> + return ad4691_exit_conversion_mode(st);
> +}
> +
> +static const struct iio_buffer_setup_ops ad4691_cnv_burst_offload_buffer_setup_ops = {
> + .postenable = &ad4691_cnv_burst_offload_buffer_postenable,
> + .predisable = &ad4691_cnv_burst_offload_buffer_predisable,
> +};
> +
> static ssize_t sampling_frequency_show(struct device *dev,
> struct device_attribute *attr,
> char *buf)