Re: [PATCH v11 3/6] iio: adc: ad4691: add triggered buffer support

[Jonathan Cameron <jic23@kernel.org>]

On Sat, 16 May 2026 12:32:51 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 5/15/26 8:31 AM, Radu Sabau via B4 Relay wrote:
> > From: Radu Sabau <radu.sabau@analog.com>
> > 
> > Add buffered capture support using the IIO triggered buffer framework.
> > 
> > CNV Burst Mode: the GP pin identified by interrupt-names in the device
> > tree is configured as DATA_READY output. The IRQ handler stops
> > conversions and fires the IIO trigger; the trigger handler executes a
> > pre-built SPI message that reads all active channels from the AVG_IN
> > accumulator registers and then resets accumulator state and restarts
> > conversions for the next cycle.
> > 
> > Manual Mode: CNV is tied to SPI CS so each transfer simultaneously
> > reads the previous result and starts the next conversion (pipelined
> > N+1 scheme). At preenable time a pre-built, optimised SPI message of
> > N+1 transfers is constructed (N channel reads plus one NOOP to drain
> > the pipeline). The trigger handler executes the message in a single
> > spi_sync() call and collects the results. An external trigger (e.g.
> > iio-trig-hrtimer) is required to drive the trigger at the desired
> > sample rate.
> > 
> > Both modes share the same trigger handler and push a complete scan —
> > one big-endian 16-bit (__be16) slot per active channel, densely packed
> > in scan_index order, followed by a timestamp.
> > 
> > The CNV Burst Mode sampling frequency (PWM period) is exposed as a
> > buffer-level attribute via IIO_DEVICE_ATTR.
> > 
> > Signed-off-by: Radu Sabau <radu.sabau@analog.com>

> > +
> > +static int ad4691_manual_buffer_preenable(struct iio_dev *indio_dev)
> > +{
> > +	struct ad4691_state *st = iio_priv(indio_dev);
> > +	unsigned int k, i;
> > +	int ret;
> > +
> > +	memset(st->scan_xfers, 0, sizeof(st->scan_xfers));
> > +	memset(st->scan_tx, 0, sizeof(st->scan_tx));
> > +
> > +	spi_message_init(&st->scan_msg);
> > +
> > +	k = 0;
> > +	iio_for_each_active_channel(indio_dev, i) {
> > +		if (i >= indio_dev->num_channels - 1)
> > +			break; /* skip soft timestamp */  
> 
> I don't think timestamp gets set in the scan mask. It is handled separately.

FWIW that is a sashiko false postive (I believe anyway!)
If we do hit this please shout as we have a core bug.

If anyone has time to look at how hard it would be to tweak
iio_for_each_active_channel to skip a last element timestamp that
would be great.

I think that iterates one too far which is what sashiko is tripping over.

I'm only keen to fix that if we can make it low cost and hid it entirely
from drivers.

Jonathan

> 
> > +		/*
> > +		 * Channel-select command occupies the first (high) byte of the
> > +		 * 16-bit DIN frame; the second byte is a don't-care zero pad.
> > +		 * put_unaligned_be16() writes [cmd, 0x00] in memory so the
> > +		 * SPI controller sends the command byte first on the wire.
> > +		 */
> > +		put_unaligned_be16((u16)(AD4691_ADC_CHAN(i) << 8), &st->scan_tx[k]);
> > +		st->scan_xfers[k].tx_buf = &st->scan_tx[k];
> > +		/*
> > +		 * The pipeline means xfer[0] receives the residual from the
> > +		 * previous sequence, not a valid sample. Discard it (rx_buf=NULL)
> > +		 * to avoid aliasing vals[0] across two concurrent DMA mappings.
> > +		 * xfer[1] (or the NOOP when only one channel is active) writes
> > +		 * the real ch[0] result to vals[0]. Subsequent transfers write
> > +		 * into vals[k-1] so each result lands at the next dense slot.
> > +		 */
> > +		st->scan_xfers[k].rx_buf = (k == 0) ? NULL : &st->vals[k - 1];
> > +		st->scan_xfers[k].len = sizeof(st->scan_tx[k]);
> > +		st->scan_xfers[k].cs_change = 1;
> > +		st->scan_xfers[k].cs_change_delay.value = AD4691_CNV_HIGH_TIME_NS;
> > +		st->scan_xfers[k].cs_change_delay.unit = SPI_DELAY_UNIT_NSECS;
> > +		spi_message_add_tail(&st->scan_xfers[k], &st->scan_msg);
> > +		k++;
> > +	}
> > +
> > +	/* Final NOOP transfer retrieves the last channel's result. */
> > +	st->scan_xfers[k].tx_buf = &st->scan_tx[k]; /* scan_tx[k] == 0 == NOOP */
> > +	st->scan_xfers[k].rx_buf = &st->vals[k - 1];
> > +	st->scan_xfers[k].len = sizeof(st->scan_tx[k]);
> > +	spi_message_add_tail(&st->scan_xfers[k], &st->scan_msg);
> > +
> > +	ret = spi_optimize_message(st->spi, &st->scan_msg);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = ad4691_enter_conversion_mode(st);
> > +	if (ret) {
> > +		spi_unoptimize_message(&st->scan_msg);
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}