Re: [PATCH v6 6/8] iio: imu: adis16607: Add FIFO-based buffered capture

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] iio: imu: adis16607: initial fifo_watermark in adis16607_probe()
  is overwritten on buffer enable
--

commit 432b63f4ff217d947afef093d1237f760013e881
Author: Radu Sabau <radu.sabau@analog.com>

iio: imu: adis16607: Add FIFO-based buffered capture

This commit adds hardware FIFO buffered capture support to the adis16607
driver using a kfifo buffer, a level-triggered threaded interrupt handler,
and configurable watermark attributes.

> diff --git a/drivers/iio/imu/adis16607.c b/drivers/iio/imu/adis16607.c
> index f5dc37c54191e..377e7c46f860b 100644
> --- a/drivers/iio/imu/adis16607.c
> +++ b/drivers/iio/imu/adis16607.c
[ ... ]
> @@ -228,6 +262,333 @@ static const struct adis_ops adis16607_ops = {
[ ... ]
> +static ssize_t hwfifo_watermark_show(struct device *dev,
> +				     struct device_attribute *attr,
> +				     char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct adis16607 *st = iio_priv(indio_dev);
> +
> +	return sysfs_emit(buf, "%u\n", st->fifo_watermark);
> +}
[ ... ]
> +static int adis16607_set_watermark(struct iio_dev *indio_dev, unsigned int val)
> +{
> +	struct adis16607 *st = iio_priv(indio_dev);
> +
> +	val = clamp(val, 1, ADIS16607_MAX_FIFO_WM);
> +	st->fifo_watermark = val;
> +
> +	return 0;
> +}
[ ... ]
> @@ -792,6 +1183,8 @@ static int adis16607_probe(struct spi_device *spi)
>  	indio_dev->info = &adis16607_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
> +	st->fifo_watermark = ADIS16607_MAX_FIFO_WM;

[Severity: Low]
Does initializing st->fifo_watermark to ADIS16607_MAX_FIFO_WM take effect
when the buffer is enabled?

devm_iio_kfifo_buffer_setup_ext() initializes buffer->watermark to 1 via
iio_kfifo_allocate() -> iio_buffer_init(), and iio_enable_buffers() calls
hwfifo_set_watermark() with that value before postenable() runs. As a
result, hwfifo_watermark reports 38 in sysfs before the first capture while
buffer/watermark is 1, and enabling the buffer immediately overwrites
st->fifo_watermark with 1 before adis16607_buffer_postenable() programs
ADIS16607_REG_USER_FIFO_CFG.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-adis16607-v6-0-3f678719942a@analog.com?part=6