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

Jonathan Cameron
2026-05-05T14:08:13Z

On Thu, 30 Apr 2026 13:16:45 +0300
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> 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 u16 slot per channel at its scan_index position, followed by a
> timestamp — to the IIO buffer via iio_push_to_buffers_with_ts().
>
> 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>
Another Sashiko found issue inline.

Also it calls out that you have no validate_trigger which might mean
another trigger is used. Moving the irq enable to the trigger_reenable
should solve that.

> +static ssize_t sampling_frequency_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct ad4691_state *st = iio_priv(indio_dev);
> +
> + return sysfs_emit(buf, "%lu\n", NSEC_PER_SEC / st->cnv_period_ns);
> +}
> +
> +static ssize_t sampling_frequency_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct ad4691_state *st = iio_priv(indio_dev);
> + int freq, ret;
> +
> + ret = kstrtoint(buf, 10, &freq);
I missed this but as Sashiko points out this could read in a negative
frequency. Given that's clearly silly kstrtouint()

> + if (ret)
> + return ret;
> +
> + IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> + if (IIO_DEV_ACQUIRE_FAILED(claim))
> + return -EBUSY;
> +
> +
> + ret = ad4691_set_pwm_freq(st, freq);
> + if (ret)
> + return ret;
> +
> + return len;
> +}
> +
> +static IIO_DEVICE_ATTR_RW(sampling_frequency, 0);
> +
> +static const struct iio_dev_attr *ad4691_buffer_attrs[] = {
> + &iio_dev_attr_sampling_frequency,
> + NULL
> +};