Re: [PATCH v3 4/6] iio: imu: adis16607: Add FIFO-based buffered/triggered capture

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] iio: imu: adis16607: interrupt storm livelock on SPI read failure in trigger handler
--

commit 37b208fdae15ca27441d6faa6832234ed11de06d
Author: Radu Sabau <radu.sabau@analog.com>

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

This commit adds buffered capture via the device's hardware FIFO, which
is the only capture mode this driver supports. It handles the specific
command and payload format for FIFO data directly rather than using generic
adislib helpers.

> diff --git a/drivers/iio/imu/adis16607.c b/drivers/iio/imu/adis16607.c
> index 1d471f59f043d..dac1466454156 100644
> --- a/drivers/iio/imu/adis16607.c
> +++ b/drivers/iio/imu/adis16607.c
[ ... ]
> +static irqreturn_t adis16607_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct adis16607 *st = iio_priv(indio_dev);
> +	u8 fifo_sample[ADIS16607_FIFO_DATA_LEN];
> +	u16 diag_stat, word_cnt, sample_cnt, threshold;
> +	s64 timestamp;
> +	int ret, i;
> +
> +	mutex_lock(&st->adis.state_lock);
> +
> +	/* Check if FIFO threshold is met */
> +	ret = __adis_read_reg_16(&st->adis, ADIS16607_REG_DIAG_STAT, &diag_stat);
> +	if (ret)
> +		goto unlock;
[ ... ]
> +unlock:
> +	mutex_unlock(&st->adis.state_lock);
> +	iio_trigger_notify_done(indio_dev->trig);
> +	return IRQ_HANDLED;
> +}

[Severity: High]
Does this code risk an infinite interrupt storm livelock if the SPI read fails?

If __adis_read_reg_16() fails to read ADIS16607_REG_DIAG_STAT, the handler
jumps to the unlock label and returns IRQ_HANDLED.

Because the register read failed, the hardware interrupt status is not cleared
and the DEV_NOK pin remains asserted. Returning IRQ_HANDLED informs the kernel
IRQ subsystem that the interrupt was successfully serviced, which bypasses
the spurious interrupt detector.

When the threaded handler completes and unmasks the IRQ, won't it immediately
re-trigger and loop forever?

Should the handler return IRQ_NONE on unrecoverable bus errors so the kernel
can eventually disable the screaming interrupt?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-adis16607-v3-0-f972d4efee85@analog.com?part=4