linux-kernel - Re: [PATCH 4/8] iio: adc: ad7476: Use correct channel for bit info

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: <20250806160413.00005a75@huawei.com>
Date: Wed, 6 Aug 2025 16:04:13 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Matti Vaittinen <mazziesaccount@...il.com>
CC: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>, Lars-Peter Clausen
	<lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>, Jonathan
 Cameron <jic23@...nel.org>, David Lechner <dlechner@...libre.com>, Nuno
 Sá <nuno.sa@...log.com>, Andy Shevchenko
	<andy@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
	<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Liam Girdwood
	<lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
	<linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/8] iio: adc: ad7476: Use correct channel for bit info

On Wed, 6 Aug 2025 10:03:43 +0300
Matti Vaittinen <mazziesaccount@...il.com> wrote:

> The ad7476 supports ADCs which use separate GPIO for starting the
> conversion. For such devices, the driver uses different channel
> information if the GPIO is found. The bit information is still always
> used from the original (non 'convstart') channels.
> 
> This has not been causing problems because the bit information for the
> 'convstart' -channel and the 'normal' -channel is identical. It,
> however, will cause issues if an IC has different characteristics for an
> 'convstart' -channel and regular channel. Furthermore, this will cause
> problems if a device always requires the convstart GPIO and thus only
> defines the convstart channel.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@...il.com>
> ---
> It appears that the _only_ difference between the 'convstart' -channel
> and the 'normal' channel is a lack of the 'raw-read' support. I might
> prefer seeing the _same_ channel information being used for 'convstart'
> and 'normal' channels, just setting the IIO_CHAN_INFO_RAW -bit when the
> CONVSTART GPIO is found. This would allow getting rid of the 'convstart'
> -channel spec altogeher. Having only one channel info spec would also
> help the code-reader to understand that the driver really provides only
> one data channel to the users. Currently a quick reader may assume the
> driver for some reason provides both the 'convstart' and the 'normal'
> channels.
> 
> Adding the IIO_CHAN_INFO_RAW when CONVSTART GPIO is obtained would
> however require the channel information structs to be mutable - which may
> be seen as a "no, no" by some. Hence this minimally intrusive patch.
If you duplicate them before updating that is probably fine, just keep the
ones in the chip info static const. 
> ---
>  drivers/iio/adc/ad7476.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> index 7b6d36999afc..fc701267358e 100644
> --- a/drivers/iio/adc/ad7476.c
> +++ b/drivers/iio/adc/ad7476.c
> @@ -121,8 +121,8 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
>  
>  		if (ret < 0)
>  			return ret;
> -		*val = (ret >> st->chip_info->channel[0].scan_type.shift) &
> -			GENMASK(st->chip_info->channel[0].scan_type.realbits - 1, 0);
> +		*val = (ret >> chan->scan_type.shift) &
> +			GENMASK(chan->scan_type.realbits - 1, 0);
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_SCALE:
>  		*val = st->scale_mv;
> @@ -345,7 +345,7 @@ static int ad7476_probe(struct spi_device *spi)
>  	/* Setup default message */
>  
>  	st->xfer.rx_buf = &st->data;
> -	st->xfer.len = st->chip_info->channel[0].scan_type.storagebits / 8;
> +	st->xfer.len = indio_dev->channels[0].scan_type.storagebits / 8;
>  
>  	spi_message_init(&st->msg);
>  	spi_message_add_tail(&st->xfer, &st->msg);