[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aYEoCscolCfvasJ9@JSANTO12-L01.ad.analog.com>
Date: Mon, 2 Feb 2026 19:41:14 -0300
From: Jonathan Santos <jonath4nns@...il.com>
To: Nuno Sá <noname.nuno@...il.com>
Cc: Jonathan Santos <Jonathan.Santos@...log.com>, linux-iio@...r.kernel.org,
linux-kernel@...r.kernel.org, Michael.Hennerich@...log.com,
lars@...afoo.de, jic23@...nel.org, dlechner@...libre.com,
nuno.sa@...log.com, andy@...nel.org
Subject: Re: [PATCH 2/3] iio: adc: ad7768-1: prevent one-shot mode with
wideband filter
On 02/04, Nuno Sá wrote:
> On Sat, 2026-01-31 at 22:35 -0300, Jonathan Santos wrote:
> > The AD7768-1 datasheet specifies that the wideband low ripple FIR filter
> > is only available in continuous conversion mode and should not be used
> > with one-shot mode to avoid malfunction and incorrect data.
> >
> > Add filter type checks in ad7768_scan_direct() to skip one-shot mode
> > switching when wideband filter is configured.
> >
> > Signed-off-by: Jonathan Santos <Jonathan.Santos@...log.com>
> > ---
> > drivers/iio/adc/ad7768-1.c | 25 +++++++++++++++----------
> > 1 file changed, 15 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> > index 8d39b71703ae..374614ea97ac 100644
> > --- a/drivers/iio/adc/ad7768-1.c
> > +++ b/drivers/iio/adc/ad7768-1.c
> > @@ -463,14 +463,17 @@ static int ad7768_scan_direct(struct iio_dev *indio_dev)
> > struct ad7768_state *st = iio_priv(indio_dev);
> > int readval, ret;
> >
> > - ret = ad7768_set_mode(st, AD7768_ONE_SHOT);
> > - if (ret < 0)
> > - return ret;
> > + /* Wideband filter is not available in One-Shot conversion mode */
> > + if (st->filter_type != AD7768_FILTER_WIDEBAND) {
> > + 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;
> > + /* 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);
> >
> > @@ -496,9 +499,11 @@ static int ad7768_scan_direct(struct iio_dev *indio_dev)
> > * Any SPI configuration of the AD7768-1 can only be
> > * performed in continuous conversion mode.
> > */
> > - ret = ad7768_set_mode(st, AD7768_CONTINUOUS);
> > - if (ret < 0)
> > - return ret;
> > + if (st->filter_type != AD7768_FILTER_WIDEBAND) {
> > + ret = ad7768_set_mode(st, AD7768_CONTINUOUS);
> > + if (ret < 0)
> > + return ret;
> > + }
>
>
> So the idea is to still have continuous mode running and get the latest sample after
> we do reinit_completion()? If that's the case, why do we have the one shot logic? Does
> it bring that much added value? Asking because I'm just wondering we could just let
> it in continuous mode all the time.
The concern on having continuous conversion mode all the time was that
sometimes, especially at higher sampling rates, a new conversion (DRDY)
occurs while the controller is still reading the ADC_DATA register.
I was initially worried the data register might be refreshed mid-transfer.
However, after reviewing the datasheet more carefully, that does not
appear to be the case. As the ADAQ7768-1 datasheet on page 76 states:
"The register driving DOUT resets by the last SCLK rising edge in
an SPI read frame, or a rising CS edge, whichever occurs first.
-> The number of SCLKs required to reset the SPI interface
depends on the read configuration. The 16th rising SCLK edge
resets the SPI interface for a normal read operation and up to
40 SCLKs when reading back ADC conversion data, plus the
status and CRC headers."
Then I guess we could just remove the one-shot mode.
>
> This also sounds like a fix so maybe a Fixes tag?
>
> - Nuno Sá
Powered by blists - more mailing lists