On Mon, 4 Nov 2024 11:19:04 +0100
Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxx> 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@xxxxxxxxxx>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxx/
>
> b4 ignored Nuno's Reviewed-by tag with
>
> NOTE: some trailers ignored due to from/email mismatches:
> ! Trailer: Reviewed-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
> Msg From: Nuno Sá <noname.nuno@xxxxxxxxx>
>
> 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
