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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Tue, Mar 10, 2026 at 04:32:25PM +0200, 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 Clock Mode and Manual Mode support offload, but use different
> trigger mechanisms:
> 
> CNV Clock Mode: the SPI Engine is triggered by the ADC's DATA_READY
> signal on GP0. 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 GP0/DATA_READY output via
> devm_spi_offload_trigger_register() so the offload framework can
> match the '#trigger-source-cells' phandle from the device tree and
> automatically fire the SPI Engine DMA transfer at end-of-conversion.
> The pre-built SPI message reads all active channels from the AVG_IN
> accumulator registers (2-byte address + 2-byte data per channel,
> one 4-byte transfer each) followed by a state reset word to re-arm
> the accumulator for the next cycle.
> 
> 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 4-byte 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 32-bit frames (bits_per_word=32, len=4) for
> DMA word alignment. In Manual Mode the 4-byte DMA word layout is
> [dummy(8), data_hi(8), data_lo(8), extra(8)]; the channel scan type
> storagebits=32, shift=8, realbits=16 correctly extracts the 16-bit
> ADC result from the middle two bytes.
> 
> Kconfig gains a dependency on IIO_BUFFER_DMAENGINE.

...

> +			st->sampling_period = ns_to_ktime(DIV_ROUND_CLOSEST_ULL
> +				(NSEC_PER_SEC, freq));

Bad indentation.

...

> +static int ad4691_offload_buffer_postenable(struct iio_dev *indio_dev)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	struct spi_offload_trigger_config config = { };
> +	struct spi_offload_trigger *trigger;
> +	struct spi_transfer *xfer = st->offload_xfer;
> +	int ret, num_xfers = 0;
> +	int active_chans[16];
> +	unsigned int bit;
> +	int n_active = 0;
> +	int i;
> +
> +	memset(xfer, 0, sizeof(st->offload_xfer));
> +
> +	/* Collect active channels in scan order */
> +	iio_for_each_active_channel(indio_dev, bit)
> +		active_chans[n_active++] = bit;
> +
> +	ret = ad4691_enter_conversion_mode(st);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * MANUAL_MODE uses a periodic (PWM) trigger and reads directly from
> +	 * the ADC. CNV_CLOCK_MODE uses the DATA_READY trigger and reads from
> +	 * accumulators.
> +	 */
> +	if (st->adc_mode == AD4691_MANUAL_MODE) {
> +		config.type = SPI_OFFLOAD_TRIGGER_PERIODIC;
> +		config.periodic.frequency_hz = st->offload_trigger_hz;
> +		trigger = st->offload_trigger_periodic;
> +		if (!trigger)
> +			return -EINVAL;
> +	} else {
> +		ret = regmap_write(st->regmap, AD4691_STATE_RESET_REG,
> +				   AD4691_STATE_RESET_ALL);
> +		if (ret)
> +			return ret;

> +		/* Configure accumulator masks - 0 = enabled, 1 = masked */
> +		ret = regmap_write(st->regmap, AD4691_ACC_MASK1_REG,
> +				   ~(*indio_dev->active_scan_mask) & 0xFF);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_write(st->regmap, AD4691_ACC_MASK2_REG,
> +				   ~(*indio_dev->active_scan_mask >> 8) & 0xFF);
> +		if (ret)
> +			return ret;

Why bulk write can't be used?

> +		/* Configure sequencer with active channels */
> +		ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
> +				   *indio_dev->active_scan_mask);
> +		if (ret)
> +			return ret;
> +
> +		iio_for_each_active_channel(indio_dev, bit) {
> +			ret = regmap_write(st->regmap, AD4691_ACC_COUNT_LIMIT(bit),
> +					   AD4691_ACC_COUNT_VAL);
> +			if (ret)
> +				return ret;
> +		}
> +
> +		config.type = SPI_OFFLOAD_TRIGGER_DATA_READY;
> +		trigger = st->offload_trigger;
> +	}
> +
> +	if (st->adc_mode == AD4691_MANUAL_MODE) {
> +		/*
> +		 * Manual mode with CNV tied to CS: Each CS toggle triggers a
> +		 * conversion AND reads the previous conversion result (pipeline).
> +		 */
> +		for (i = 0; i < n_active; i++) {
> +			st->offload_tx_cmd[num_xfers] = AD4691_ADC_CHAN(active_chans[i]) << 24;
> +			xfer[num_xfers].tx_buf = &st->offload_tx_cmd[num_xfers];
> +			xfer[num_xfers].len = 4;
> +			xfer[num_xfers].bits_per_word = 32;
> +			xfer[num_xfers].speed_hz = st->spi->max_speed_hz;
> +			xfer[num_xfers].cs_change = 1;
> +			xfer[num_xfers].cs_change_delay.value = 1000;
> +			xfer[num_xfers].cs_change_delay.unit = SPI_DELAY_UNIT_NSECS;
> +			/* First transfer RX is garbage - don't capture it */
> +			if (num_xfers)
> +				xfer[num_xfers].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
> +			num_xfers++;
> +		}
> +
> +		/* Final NOOP to flush pipeline and get last channel's data */
> +		st->offload_tx_cmd[num_xfers] = AD4691_NOOP << 24;
> +		xfer[num_xfers].tx_buf = &st->offload_tx_cmd[num_xfers];
> +		xfer[num_xfers].len = 4;
> +		xfer[num_xfers].bits_per_word = 32;
> +		xfer[num_xfers].speed_hz = st->spi->max_speed_hz;
> +		xfer[num_xfers].cs_change = 0;
> +		xfer[num_xfers].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
> +		num_xfers++;
> +	} else {
> +		/*
> +		 * CNV_CLOCK_MODE: single transfer per channel (2-byte cmd +
> +		 * 2-byte data = 4 bytes, one 32-bit SPI Engine DMA word).
> +		 * AVG_IN registers are used; RX layout: [cmd_hi, cmd_lo, d_hi, d_lo]
> +		 */
> +		for (i = 0; i < n_active; i++) {
> +			unsigned int reg;
> +			int ch = active_chans[i];
> +
> +			reg = AD4691_AVG_IN(ch);
> +			st->offload_tx_cmd[ch] =
> +				((reg >> 8) | 0x80) << 24 |
> +				(reg & 0xFF) << 16;

Use proper put_unaligned() and friends.

> +			xfer[num_xfers].tx_buf = &st->offload_tx_cmd[ch];
> +			xfer[num_xfers].len = 4;
> +			xfer[num_xfers].bits_per_word = 32;
> +			xfer[num_xfers].speed_hz = st->spi->max_speed_hz;
> +			xfer[num_xfers].offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
> +			xfer[num_xfers].cs_change = 1;
> +			num_xfers++;
> +		}
> +
> +		/*
> +		 * State reset: clear accumulator so DATA_READY can fire again.
> +		 * With bits_per_word=32, SPI engine transmits MSB first.
> +		 */
> +		st->offload_tx_reset = ((AD4691_STATE_RESET_REG >> 8) << 24) |
> +				       ((AD4691_STATE_RESET_REG & 0xFF) << 16) |
> +				       (0x01 << 8);

Ditto.

> +
> +		xfer[num_xfers].tx_buf = &st->offload_tx_reset;
> +		xfer[num_xfers].len = 4;
> +		xfer[num_xfers].bits_per_word = 32;
> +		xfer[num_xfers].speed_hz = st->spi->max_speed_hz;
> +		xfer[num_xfers].cs_change = 0;
> +		num_xfers++;
> +	}
> +
> +	if (num_xfers == 0)
> +		return -EINVAL;
> +
> +	/*
> +	 * For MANUAL_MODE, validate that the trigger frequency is low enough
> +	 * for all SPI transfers to complete. Each transfer is 32 bits.
> +	 * Add 50% margin for CS setup/hold and other overhead.
> +	 */
> +	if (st->adc_mode == AD4691_MANUAL_MODE) {
> +		u64 min_period_ns;
> +		u64 trigger_period_ns;
> +
> +		/* Time for all transfers in nanoseconds, with 50% overhead margin */
> +		min_period_ns = div64_u64((u64)num_xfers * AD4691_OFFLOAD_BITS_PER_WORD *
> +					  NSEC_PER_SEC * 3,
> +					  st->spi->max_speed_hz * 2);
> +
> +		trigger_period_ns = div64_u64(NSEC_PER_SEC, st->offload_trigger_hz);

Why 64-bit division? The dividend is 32-bit value.

> +		if (trigger_period_ns < min_period_ns)
> +			return -EINVAL;
> +	}
> +
> +	spi_message_init_with_transfers(&st->offload_msg, xfer, num_xfers);
> +	st->offload_msg.offload = st->offload;
> +
> +	ret = spi_optimize_message(st->spi, &st->offload_msg);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * For CNV_CLOCK_MODE, start conversions before enabling the trigger.
> +	 * If the trigger is enabled first, the SPI engine blocks waiting for
> +	 * DATA_READY, and any subsequent SPI write times out.
> +	 *
> +	 * MANUAL_MODE: CNV is tied to CS; conversion starts with each transfer.
> +	 */
> +	if (st->adc_mode == AD4691_CNV_CLOCK_MODE) {
> +		ret = ad4691_sampling_enable(st, true);
> +		if (ret)
> +			goto err_unoptimize_message;
> +	}
> +
> +	ret = spi_offload_trigger_enable(st->offload, trigger, &config);
> +	if (ret)
> +		goto err_sampling_disable;
> +
> +	return 0;
> +
> +err_sampling_disable:
> +	if (st->adc_mode == AD4691_CNV_CLOCK_MODE)
> +		ad4691_sampling_enable(st, false);
> +err_unoptimize_message:
> +	spi_unoptimize_message(&st->offload_msg);
> +	return ret;
> +}

...

> +	st->offload = devm_spi_offload_get(dev, spi, &ad4691_offload_config);

	ret = PTR_ERR_OR_ZERO(...); ?

> +	if (IS_ERR(st->offload)) {
> +		if (PTR_ERR(st->offload) != -ENODEV)
> +			return dev_err_probe(dev, PTR_ERR(st->offload),
> +					     "failed to get SPI offload\n");
> +		st->offload = NULL;
> +	}

	if (ret == -ENODEV)
		->offload = NULL;
	else if (ret)
		return dev_err_probe(...);

-- 
With Best Regards,
Andy Shevchenko