Re: [PATCH 4/9] iio: adc: rzt2h: claim direct mode on single reads

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Unsafe 1-jiffy hardware timeout causes spurious failures.
- [High] Late interrupt triggers spurious completion for subsequent reads on timeout.
--

commit 804ca0ac39c62632fae66702cb8c967efd8b7dee
Author: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>

iio: adc: rzt2h: claim direct mode on single reads
    
Claim direct mode around the single read to prevent it from conflicting
with an ongoing buffered read.

> diff --git a/drivers/iio/adc/rzt2h_adc.c b/drivers/iio/adc/rzt2h_adc.c
> index 995308a419f12..95bcebdc02cb3 100644
> --- a/drivers/iio/adc/rzt2h_adc.c
> +++ b/drivers/iio/adc/rzt2h_adc.c
> @@ -158,8 +158,13 @@ static int rzt2h_adc_read_raw(struct iio_dev *indio_dev,
>  	struct rzt2h_adc *adc = iio_priv(indio_dev);
>  
>  	switch (mask) {
> -	case IIO_CHAN_INFO_RAW:
> +	case IIO_CHAN_INFO_RAW: {
> +		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> +		if (IIO_DEV_ACQUIRE_FAILED(claim))
> +			return -EBUSY;
> +
>  		return rzt2h_adc_read_single(adc, chan->channel, val);

[Severity: High]
This isn't a bug introduced by this patch, but this is a pre-existing issue
in rzt2h_adc_read_single(). Can the 1-jiffy timeout expire instantaneously
and cause spurious failures?

Looking at rzt2h_adc_read_single():

	ret = wait_for_completion_timeout(&adc->completion, usecs_to_jiffies(1));
	if (!ret) {
		ret = -ETIMEDOUT;
		goto disable;
	}

A 1-jiffy timeout can expire almost instantaneously if the next timer tick
occurs immediately after the call. Even though the conversion takes ~0.32us,
could it be outraced by the timer tick, causing the driver to incorrectly assume
a hardware failure and abort the read?

[Severity: High]
This is also a pre-existing issue, but does a timeout in rzt2h_adc_read_single()
leave a pending interrupt that could corrupt subsequent reads?

If a timeout occurs, the cleanup path calls rzt2h_adc_stop() to disable device
interrupts, but it does not synchronize with the IRQ controller. A pending or
concurrently running ISR will unconditionally signal completion:

static irqreturn_t rzt2h_adc_isr(int irq, void *private)
{
	struct rzt2h_adc *adc = private;

	complete(&adc->completion);

	return IRQ_HANDLED;
}

If this late ISR executes immediately after a subsequent read calls
reinit_completion(), could the new read falsely complete instantly and return
stale or garbage data from the ADC before the hardware actually finishes?

> +	}
>  	case IIO_CHAN_INFO_SCALE:
>  		*val = 1800;
>  		*val2 = 12;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828145943.2077589-1-cosmin-gabriel.tanislav.xa@renesas.com?part=4