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

[Jonathan Santos <jonath4nns@gmail.com>]

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á