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

[Jonathan Cameron]

On Mon, 9 Feb 2026 18:38:31 -0300
Jonathan Santos <jonath4nns@xxxxxxxxx> wrote:

> On 02/07, Jonathan Cameron wrote:
> > On Sat, 31 Jan 2026 22:35:20 -0300
> > Jonathan Santos <Jonathan.Santos@xxxxxxxxxx> 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?
I don't have a strong view either way as the trade offs of
using continuous type modes for oneshot and not tend to be rather driver
and usecase specific.  So I'll leave that decision to the driver author
and anyone else who cares about a particular part.

Fix wise you probably want the minor change of adding the reinit before
the pulse as that gives us something small to backport, even if you then
rip the code out again.

Thanks,

Jonathan

> 
> > Jonathan
> >   
> > > 
> > > Fixes: a5f8c7da3dbe ("iio: adc: Add AD7768-1 ADC basic support")
> > > Signed-off-by: Jonathan Santos <Jonathan.Santos@xxxxxxxxxx>
> > > ---
> > >  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)