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

[David Lechner <dlechner@baylibre.com>]

On 5/17/26 7:25 AM, Jonathan Cameron wrote:
> 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
> 
This is what I came up with (totally untested).

Since timestamp can never be set in scan_mask/active_scan_mask, it should
be safe to exclude it from masklength without breaking existing code.

I didn't check all callers of masklength/iio_get_masklength() though.

---
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 9d66510a1d49..17f539fc23e2 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -2300,8 +2300,10 @@ int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
 	if (channels) {
 		int ml = 0;
 
-		for (i = 0; i < indio_dev->num_channels; i++)
-			ml = max(ml, channels[i].scan_index + 1);
+		for (i = 0; i < indio_dev->num_channels; i++) {
+			if (channels[i].type != IIO_TIMESTAMP)
+				ml = max(ml, channels[i].scan_index + 1);
+		}
 		ACCESS_PRIVATE(indio_dev, masklength) = ml;
 	}