[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aYpT1zES5la/8TTb@JSANTO12-L01.ad.analog.com>
Date: Mon, 9 Feb 2026 18:38:31 -0300
From: Jonathan Santos <jonath4nns@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Jonathan Santos <Jonathan.Santos@...log.com>, linux-iio@...r.kernel.org,
linux-kernel@...r.kernel.org, Michael.Hennerich@...log.com,
lars@...afoo.de, dlechner@...libre.com, nuno.sa@...log.com,
andy@...nel.org
Subject: Re: [PATCH 1/3] iio: adc: ad7768-1: fix one-shot mode data
acquisition
On 02/07, Jonathan Cameron wrote:
> On Sat, 31 Jan 2026 22:35:20 -0300
> Jonathan Santos <Jonathan.Santos@...log.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@...log.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)
Powered by blists - more mailing lists