Re: [PATCH 1/3] iio: adc: ad7768-1: fix one-shot mode data acquisition

[Jonathan Santos <jonath4nns@gmail.com>]

On 02/07, Jonathan Cameron wrote:
> On Sat, 31 Jan 2026 22:35:20 -0300
> Jonathan Santos <Jonathan.Santos@analog.com> wrote:
> 
> > According to the datasheet, one-shot mode requires a SYNC_IN pulse to
> > trigger a new sample conversion. In the current implementation, No sync
> > pulse was sent after switching to one-shot mode and reinit_completion()
> > was called before mode switching, creating a race condition where spurious
> > interrupts during mode change could trigger completion prematurely.
> > 
> > Fix by sending a sync pulse after configuring one-shot mode and moving
> > reinit_completion() after the pulse to ensure it only waits for the
> > actual conversion completion.
> That smells like a race...
> 
> What stops us taking a long break between ending that sync pulse and
> the reinit_completition() such that we have initialized if before the
> complete()?
> 

Yes, you are right, we have a race here. The inital problem was having 
the ad7768_set_mode() before the reinit_completion() because, almost 
everytime, a new conversion is done before setting the one-shot mode, 
so the complete() happens before the expected trigger (that never
happened because ad7768_send_sync_pulse() was not present).

> You always have to do it before what ever ultimately makes the complete()
> happen. Have you seen an sign of the spurious interrupts or is that
> more a conjecture of what might happen? 
> 

Agreed, I can put reinit_completion() before sending the sync pulse to
avoid this situation. But maybe it is better just to remove the one-shot
mode as discussed in the patch 2/3. What do you think?

> Jonathan
> 
> > 
> > Fixes: a5f8c7da3dbe ("iio: adc: Add AD7768-1 ADC basic support")
> > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> > ---
> >  drivers/iio/adc/ad7768-1.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> > index fcd8aea7152e..8d39b71703ae 100644
> > --- a/drivers/iio/adc/ad7768-1.c
> > +++ b/drivers/iio/adc/ad7768-1.c
> > @@ -463,12 +463,17 @@ static int ad7768_scan_direct(struct iio_dev *indio_dev)
> >  	struct ad7768_state *st = iio_priv(indio_dev);
> >  	int readval, ret;
> >  
> > -	reinit_completion(&st->completion);
> > -
> >  	ret = ad7768_set_mode(st, AD7768_ONE_SHOT);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > +	/* One-shot mode requires a SYNC pulse to generate a new sample */
> > +	ret = ad7768_send_sync_pulse(st);
> > +	if (ret)
> > +		return ret;
> > +
> > +	reinit_completion(&st->completion);
> > +
> >  	ret = wait_for_completion_timeout(&st->completion,
> >  					  msecs_to_jiffies(1000));
> >  	if (!ret)