Re: [PATCH v2 1/6] iio: adc: ti-ads1298: fix the data read path

[Stephan Costa Barros]

On Sun, Oct 25, 2026 at 10:14:39PM +0200, Andy Shevchenko wrote:
> > +	/* Only changes while no transfer is in flight */
> > +	scoped_guard(spinlock_irqsave, &priv->irq_busy_lock)
> > +		buffered = priv->buffered;
>
> Strictly speaking you need to assign buffered to make any compiler not being
> confused by the scoped_guard(), which cases IIRC we used to have the past.

OK, I'll initialise it.

> > +	ret = ads1298_write_cmd(priv, ADS1298_CMD_START);
> > +	if (ret) {
> > +		scoped_guard(spinlock_irqsave, &priv->irq_busy_lock)
> > +			priv->buffered = false;
> > +	}
> > +
> > +	return ret;
>
> Before write_cmd is called the buffered might have been triggered. Is it okay?

Yes, and on purpose. By postenable the core has set up the scan mask and
the buffer, so a sample arriving before START is pushed like any other.
The chip does not convert before START anyway, unless its START pin is
tied high. The other order is the problem: a DRDY between START and
setting the flag would be taken as a single-shot read.

> > +	scoped_guard(spinlock_irqsave, &priv->irq_busy_lock)
> > +		priv->buffered = false;
> > +
> > +	return ret ?: ret2;
> >  }
>
> The code lacks of comments on why in all error cases (either first one, or
> second, or both are triggered) it's fine to continue. And what will be the
> meaning of the returned value in all three cases?

The core goes on with the teardown whatever predisable returns
(iio_disable_buffers() says so), so the driver must stop using the scan
in every case. The return value only reaches whoever wrote to
buffer/enable, and it is the first error.

If STOP fails, the chip may keep converting, but once the flag is clear
the completions leave the scan alone. In v2 the flag was cleared only
after the wait, which was wrong, v3 clears it right after STOP. If the 
wait times out, a completion that already saw the flag set could still 
be pushing. The SPI core completes every message, so I'm inclined to 
wait without a limit there, as on unbind, and return only the STOP error.

I'll put that in a comment. Jonathan asked to rework this part, so the
code may change, but the explanation stays.

Thanks,
Stephan