linux-kernel - Re: [PATCH] iio: adc: ad7124: Disable all channels at probe time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241109152438.0135f0c2@jic23-huawei>
Date: Sat, 9 Nov 2024 15:24:38 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Uwe Kleine-König <u.kleine-koenig@...libre.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich
 <Michael.Hennerich@...log.com>, Dumitru Ceclan <dumitru.ceclan@...log.com>,
 Nuno Sa <nuno.sa@...log.com>, Guillaume Ranquet <granquet@...libre.com>,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: adc: ad7124: Disable all channels at probe time

On Mon,  4 Nov 2024 11:19:04 +0100
Uwe Kleine-König <u.kleine-koenig@...libre.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@...log.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@...libre.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@...log.com>
> 	     Msg From: Nuno Sá <noname.nuno@...il.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