When during a measurement two channels are enabled, two measurements are
done that are reported sequencially in the DATA register. As the code
triggered by reading one of the sysfs properties expects that only one
channel is enabled it only reads the first data set which might or might
not belong to the intended channel.

To prevent this situation disable all channels during probe. This fixes
a problem in practise because the reset default for channel 0 is
enabled. So all measurements before the first measurement on channel 0
(which disables channel 0 at the end) might report wrong values.

Fixes: 7b8d045e497a ("iio: adc: ad7124: allow more than 8 channels")
Reviewed-by: Nuno Sa <nuno.sa@analog.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
Hello,

this patch was part of a series before. The remaining patches are still
under discussion. As this is a fix orthogonal to the other patches of
the series (apart from the other relevant change there also being
necessary to make the ad7124 work for me) it IMHO makes sense to apply
this one already now. There are machines that don't suffer from the
other issue (i.e. the device irq becoming pending by spi traffic), so
this fix is also valuable stand alone. It's IMHO good enough to go in
before v6.12.

The previous submission is available at
https://lore.kernel.org/linux-iio/20241028160748.489596-10-u.kleine-koenig@baylibre.com/

b4 ignored Nuno's Reviewed-by tag with

	NOTE: some trailers ignored due to from/email mismatches:
	    ! Trailer: Reviewed-by: Nuno Sa <nuno.sa@analog.com>
	     Msg From: Nuno Sá <noname.nuno@gmail.com>

I wonder if other maintainers use b4 apply's -S by default, because I
often run into this issue but don't see others mentioning that.
I added the tag here anyhow.

 drivers/iio/adc/ad7124.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index a5d91933f505..749304d38415 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -917,6 +917,9 @@ static int ad7124_setup(struct ad7124_state *st)
 		 * set all channels to this default value.
 		 */
 		ad7124_set_channel_odr(st, i, 10);
+
+		/* Disable all channels to prevent unintended conversions. */
+		ad_sd_write_reg(&st->sd, AD7124_CHANNEL(i), 2, 0);
 	}
 
 	ret = ad_sd_write_reg(&st->sd, AD7124_ADC_CONTROL, 2, st->adc_control);

base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc
-- 
2.45.2

On Mon,  4 Nov 2024 11:19:04 +0100
Uwe Kleine-König <u.kleine-koenig@baylibre.com> wrote:

> When during a measurement two channels are enabled, two measurements are
> done that are reported sequencially in the DATA register. As the code
> triggered by reading one of the sysfs properties expects that only one
> channel is enabled it only reads the first data set which might or might
> not belong to the intended channel.
> 
> To prevent this situation disable all channels during probe. This fixes
> a problem in practise because the reset default for channel 0 is
> enabled. So all measurements before the first measurement on channel 0
> (which disables channel 0 at the end) might report wrong values.
> 
> Fixes: 7b8d045e497a ("iio: adc: ad7124: allow more than 8 channels")
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> Hello,
> 
> this patch was part of a series before. The remaining patches are still
> under discussion. As this is a fix orthogonal to the other patches of
> the series (apart from the other relevant change there also being
> necessary to make the ad7124 work for me) it IMHO makes sense to apply
> this one already now. There are machines that don't suffer from the
> other issue (i.e. the device irq becoming pending by spi traffic), so
> this fix is also valuable stand alone. It's IMHO good enough to go in
> before v6.12.
> 
> The previous submission is available at
> https://lore.kernel.org/linux-iio/20241028160748.489596-10-u.kleine-koenig@baylibre.com/
> 
> b4 ignored Nuno's Reviewed-by tag with
> 
> 	NOTE: some trailers ignored due to from/email mismatches:
> 	    ! Trailer: Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> 	     Msg From: Nuno Sá <noname.nuno@gmail.com>
> 
> I wonder if other maintainers use b4 apply's -S by default, because I
> often run into this issue but don't see others mentioning that.
I only use -S when Nuno has given a tag.  It tends to be a bit too generous
on applying tags in general.

Thanks for picking it up here.

Sadly this has probably missed 6.12, but I have queued it up as a fix
for early next cycle and marked it for stable.

Thanks,

Jonathan

> I added the tag here anyhow.
> 
>  drivers/iio/adc/ad7124.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
> index a5d91933f505..749304d38415 100644
> --- a/drivers/iio/adc/ad7124.c
> +++ b/drivers/iio/adc/ad7124.c
> @@ -917,6 +917,9 @@ static int ad7124_setup(struct ad7124_state *st)
>  		 * set all channels to this default value.
>  		 */
>  		ad7124_set_channel_odr(st, i, 10);
> +
> +		/* Disable all channels to prevent unintended conversions. */
> +		ad_sd_write_reg(&st->sd, AD7124_CHANNEL(i), 2, 0);
>  	}
>  
>  	ret = ad_sd_write_reg(&st->sd, AD7124_ADC_CONTROL, 2, st->adc_control);
> 
> base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc