Re: [PATCH] iio: adc: ad7606: fix raw read for 18-bit chips

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 02 May 2025 10:04:30 -0500
David Lechner <dlechner@baylibre.com> wrote:

> Fix 18-bit raw read for 18-bit chips by applying a mask to the value
> we receive from the SPI controller.
> 
> SPI controllers either return 1, 2 or 4 bytes per word depending on the
> bits_per_word. For 16-bit chips, there was no problem since they raw
> data fit exactly in the 2 bytes received from the SPI controller. But
> now that we have 18-bit chips and we are using bits_per_word = 18, we
> cannot assume that the extra bits in the 32-bit word are always zero.
> In fact, with the AXI SPI Engine controller, these bits are not always
> zero which caused the raw values to read 10s of 1000s of volts instead
> of the correct value. Therefore, we need to mask the value we receive
> from the SPI controller to ensure that only the 18 bits of real data
> are used.
> 
> Fixes: f3838e934dff ("iio: adc: ad7606: add support for AD7606C-{16,18} parts")
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
> Jonathan, I based this on the fixes-togreg branch since I assume it will
> go that route, but beware that there is a trivial merge conflict with
> https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=togreg&id=3b5b55ca940733b7b533fe3137a307ceacb4fb90
> as that patch modifies an adjacent line.
Applied to the fixes-togreg branch of iio.git. Let's see if this causes
much trouble in next. Given it's just adjacent lines hopefully won't be a
problem.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/ad7606.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> index 703556eb7257ea0647135c4b268a8ead93115c6f..8ed65a35b4862315431a4852453639b4e056178e 100644
> --- a/drivers/iio/adc/ad7606.c
> +++ b/drivers/iio/adc/ad7606.c
> @@ -727,17 +727,16 @@ static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch,
>  		goto error_ret;
>  
>  	chan = &indio_dev->channels[ch + 1];
> -	if (chan->scan_type.sign == 'u') {
> -		if (realbits > 16)
> -			*val = st->data.buf32[ch];
> -		else
> -			*val = st->data.buf16[ch];
> -	} else {
> -		if (realbits > 16)
> -			*val = sign_extend32(st->data.buf32[ch], realbits - 1);
> -		else
> -			*val = sign_extend32(st->data.buf16[ch], realbits - 1);
> -	}
> +
> +	if (realbits > 16)
> +		*val = st->data.buf32[ch];
> +	else
> +		*val = st->data.buf16[ch];
> +
> +	*val &= GENMASK(realbits - 1, 0);
> +
> +	if (chan->scan_type.sign == 's')
> +		*val = sign_extend32(*val, realbits - 1);
>  
>  error_ret:
>  	if (!st->gpio_convst) {
> 
> ---
> base-commit: 609bc31eca06c7408e6860d8b46311ebe45c1fef
> change-id: 20250502-iio-adc-ad7606-fix-raw-read-for-18-bit-chips-ed035c488eca
> 
> Best regards,