linux-kernel - Re: [PATCH 01/15] iio: adc: ad4030: Fix _scale for when oversampling is enabled

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: <20250830194308.32def0e5@jic23-huawei>
Date: Sat, 30 Aug 2025 19:43:08 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Marcelo Schmitt <marcelo.schmitt@...log.com>
Cc: <linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 <linux-doc@...r.kernel.org>, <devicetree@...r.kernel.org>,
 <linux-spi@...r.kernel.org>, <Michael.Hennerich@...log.com>,
 <nuno.sa@...log.com>, <eblanc@...libre.com>, <dlechner@...libre.com>,
 <andy@...nel.org>, <corbet@....net>, <robh@...nel.org>,
 <krzk+dt@...nel.org>, <conor+dt@...nel.org>, <broonie@...nel.org>,
 <Jonathan.Cameron@...wei.com>, <andriy.shevchenko@...ux.intel.com>,
 <ahaslam@...libre.com>, <marcelo.schmitt1@...il.com>
Subject: Re: [PATCH 01/15] iio: adc: ad4030: Fix _scale for when
 oversampling is enabled

On Fri, 29 Aug 2025 21:40:24 -0300
Marcelo Schmitt <marcelo.schmitt@...log.com> wrote:

> Previously, the AD4030 driver was using the number of scan realbits for the
> voltage channel to derive the scale to millivolts. Though, when sample
> averaging is enabled (oversampling_ratio > 1), the number of scan realbits
> for the channel is set to 30 and doesn't match the amount of conversion
> precision bits. Due to that, the calculated channel scale did not correctly
> scale raw sample data to millivolt units in those cases. Use chip specific
> precision bits to derive the correct channel _scale on every and all
> channel configuration.
> 
> Fixes: dc78e71d7c15 ("iio: adc: ad4030: remove some duplicate code")
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@...log.com>

Hi Marcelo

I was assuming that when this said 'averaging' it actually meant
summing (there is a note about using the upper precision bits to get the same
scaling which is what we'd expect it were simply summing over X samples).

So given that we don't divide back down to get the original scaling I'm
not following how this works.

E.g. If we 'averaged' just 2 values of 3 then we'd go from a value of 3 to
one of 6.  Therefore I'd expect the scale to halve as each lsb represents
half the voltage it did when we weren't averaging those 2 samples.

I think that is what we'd see with the current code, so my reasoning is
clearly wrong, but why?

Jonathan

> ---
> This was probalby buggy since 
> commit 949abd1ca5a4 ("iio: adc: ad4030: add averaging support")
> but I decided to set the fixes tag with dc78e71d7c15 because this patch will
> not apply cleanly over 949abd1ca5a4.
> 
>  drivers/iio/adc/ad4030.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
> index 1bc2f9a22470..82784593f976 100644
> --- a/drivers/iio/adc/ad4030.c
> +++ b/drivers/iio/adc/ad4030.c
> @@ -394,7 +394,14 @@ static int ad4030_get_chan_scale(struct iio_dev *indio_dev,
>  	else
>  		*val = st->vref_uv / MILLI;
>  
> -	*val2 = scan_type->realbits;
> +	/*
> +	 * Even though the sample data comes in a 30-bit chunk when the ADC
> +	 * is averaging samples, the conversion precision is still 16-bit or
> +	 * 24-bit depending on the device. Thus, instead of scan_type->realbits,
> +	 * use chip specific precision bits to derive the correct scale to mV.
> +	 */
> +	*val2 = scan_type->realbits == 30 ? st->chip->precision_bits
> +					  : scan_type->realbits;
>  
>  	return IIO_VAL_FRACTIONAL_LOG2;
>  }