Linux Documentation
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Marcelo Schmitt <marcelo.schmitt@analog.com>
Cc: <linux-iio@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-doc@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-spi@vger.kernel.org>, <Michael.Hennerich@analog.com>,
	<nuno.sa@analog.com>, <eblanc@baylibre.com>,
	<dlechner@baylibre.com>, <andy@kernel.org>, <corbet@lwn.net>,
	<robh@kernel.org>, <krzk+dt@kernel.org>, <conor+dt@kernel.org>,
	<broonie@kernel.org>, <Jonathan.Cameron@huawei.com>,
	<andriy.shevchenko@linux.intel.com>, <ahaslam@baylibre.com>,
	<marcelo.schmitt1@gmail.com>
Subject: Re: [PATCH 01/15] iio: adc: ad4030: Fix _scale for when oversampling is enabled
Date: Sat, 30 Aug 2025 19:43:08 +0100	[thread overview]
Message-ID: <20250830194308.32def0e5@jic23-huawei> (raw)
In-Reply-To: <f9d40e8deb3d17a41b4fd6ecc1b8ac92275f0078.1756511030.git.marcelo.schmitt@analog.com>

On Fri, 29 Aug 2025 21:40:24 -0300
Marcelo Schmitt <marcelo.schmitt@analog.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@analog.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;
>  }


  parent reply	other threads:[~2025-08-30 18:43 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-30  0:39 [PATCH 00/15] Add SPI offload support to AD4030 Marcelo Schmitt
2025-08-30  0:40 ` [PATCH 01/15] iio: adc: ad4030: Fix _scale for when oversampling is enabled Marcelo Schmitt
2025-08-30  5:00   ` Andy Shevchenko
2025-08-30 18:43   ` Jonathan Cameron [this message]
2025-08-30 18:48     ` David Lechner
2025-09-02 13:18     ` Marcelo Schmitt
2025-08-30  0:40 ` [PATCH 02/15] dt-bindings: iio: adc: adi,ad4030: Reference spi-peripheral-props Marcelo Schmitt
2025-08-30  0:41 ` [PATCH 03/15] Documentation: iio: ad4030: Add double PWM SPI offload doc Marcelo Schmitt
2025-08-30 16:49   ` David Lechner
2025-08-30  0:41 ` [PATCH 04/15] dt-bindings: iio: adc: adi,ad4030: Add PWM Marcelo Schmitt
2025-08-30  0:42 ` [PATCH 05/15] spi: offload: types: add offset parameter Marcelo Schmitt
2025-08-30  5:01   ` Andy Shevchenko
2025-08-30  0:42 ` [PATCH 06/15] spi: spi-offload-trigger-pwm: Use duty offset Marcelo Schmitt
2025-08-30  5:02   ` Andy Shevchenko
2025-08-30 16:41   ` David Lechner
2025-08-30  0:42 ` [PATCH 07/15] iio: adc: ad4030: Add SPI offload support Marcelo Schmitt
2025-08-30  7:36   ` Andy Shevchenko
2025-08-30 12:08   ` kernel test robot
2025-08-30 19:11   ` Jonathan Cameron
2025-08-30 20:14   ` David Lechner
2025-09-02 14:52     ` Marcelo Schmitt
2025-08-30  0:43 ` [PATCH 08/15] dt-bindings: iio: adc: adi,ad4030: Add 4-lane per channel bus width option Marcelo Schmitt
2025-08-30 17:01   ` David Lechner
2025-08-30  0:43 ` [PATCH 09/15] iio: adc: ad4030: Support multiple data lanes per channel Marcelo Schmitt
2025-08-30  7:38   ` Andy Shevchenko
2025-08-30 17:19   ` David Lechner
2025-08-30  0:43 ` [PATCH 10/15] dt-bindings: iio: adc: adi,ad4030: Add adi,clock-mode Marcelo Schmitt
2025-08-30 18:02   ` David Lechner
2025-08-30  0:44 ` [PATCH 11/15] iio: adc: ad4030: Add clock mode option parse and setup Marcelo Schmitt
2025-08-30  7:42   ` Andy Shevchenko
2025-08-30  0:44 ` [PATCH 12/15] dt-bindings: iio: adc: adi,ad4030: Add adi,dual-data-rate Marcelo Schmitt
2025-08-30 17:27   ` David Lechner
2025-08-30  0:45 ` [PATCH 13/15] iio: adc: ad4030: Enable dual data rate Marcelo Schmitt
2025-08-30  7:46   ` Andy Shevchenko
2025-08-30 17:33   ` David Lechner
2025-08-30  0:45 ` [PATCH 14/15] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224 Marcelo Schmitt
2025-08-30 18:45   ` David Lechner
2025-08-30  0:45 ` [PATCH 15/15] iio: adc: ad4030: Add support for " Marcelo Schmitt
2025-08-30  7:57   ` Andy Shevchenko
2025-09-02 15:22     ` Marcelo Schmitt
2025-08-30 19:17   ` David Lechner
2025-09-01 11:47   ` Dan Carpenter
2025-08-30  2:48 ` [PATCH 00/15] Add SPI offload support to AD4030 Marcelo Schmitt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250830194308.32def0e5@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=ahaslam@baylibre.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=andy@kernel.org \
    --cc=broonie@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=corbet@lwn.net \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=eblanc@baylibre.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=marcelo.schmitt1@gmail.com \
    --cc=marcelo.schmitt@analog.com \
    --cc=nuno.sa@analog.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox