[PATCH 2/3] iio: adc: ad7768-1: prevent one-shot mode with wideband filter

Jonathan Santos posted 3 patches 1 week ago
[PATCH 2/3] iio: adc: ad7768-1: prevent one-shot mode with wideband filter
Posted by Jonathan Santos 1 week ago
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@analog.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;
+	}
 
 	return readval;
 }
-- 
2.34.1
Re: [PATCH 2/3] iio: adc: ad7768-1: prevent one-shot mode with wideband filter
Posted by Nuno Sá 3 days, 22 hours ago
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@analog.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.

This also sounds like a fix so maybe a Fixes tag?

- Nuno Sá
Re: [PATCH 2/3] iio: adc: ad7768-1: prevent one-shot mode with wideband filter
Posted by Jonathan Santos 5 days, 16 hours ago
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@analog.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á