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

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Wed, 6 Aug 2025 10:03:43 +0300
Matti Vaittinen <mazziesaccount@gmail.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@gmail.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);