Re: [PATCH v6 10/17] iio: adc: ad7944: don't use storagebits for sizing

[Nuno Sá]

On Wed, 2024-12-11 at 14:54 -0600, David Lechner wrote:
> Replace use of storagebits with realbits for determining the number of
> bytes needed for SPI transfers.
> 
> When adding SPI offload support, storagebits will no longer be
> guaranteed to be the "best fit" for 16-bit chips so we can no longer
> rely on storagebits being the correct size expected by the SPI
> framework. Instead, derive the correct size from realbits since it will
> always be correct even when SPI offloads are used.
> 
> Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx>
> ---

Reviewed-vy: Nuno Sa <nuno.sa@xxxxxxxxxx>

> v6 changes: none
> 
> v5 changes: none
> 
> v4 changes: new patch in v4
> ---
>  drivers/iio/adc/ad7944.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7944.c b/drivers/iio/adc/ad7944.c
> index
> a5aea4e9f1a7bdd8ca10f9f4580ad3216ddcdfcb..6d1202bd55a013b092ff803f2065fd128dfa
> 9bdd 100644
> --- a/drivers/iio/adc/ad7944.c
> +++ b/drivers/iio/adc/ad7944.c
> @@ -98,6 +98,9 @@ struct ad7944_chip_info {
>  	const struct iio_chan_spec channels[2];
>  };
>  
> +/* get number of bytes for SPI xfer */
> +#define AD7944_SPI_BYTES(scan_type) ((scan_type).realbits > 16 ? 4 : 2)
> +
>  /*
>   * AD7944_DEFINE_CHIP_INFO - Define a chip info structure for a specific chip
>   * @_name: The name of the chip
> @@ -164,7 +167,7 @@ static int ad7944_3wire_cs_mode_init_msg(struct device
> *dev, struct ad7944_adc *
>  
>  	/* Then we can read the data during the acquisition phase */
>  	xfers[2].rx_buf = &adc->sample.raw;
> -	xfers[2].len = BITS_TO_BYTES(chan->scan_type.storagebits);
> +	xfers[2].len = AD7944_SPI_BYTES(chan->scan_type);
>  	xfers[2].bits_per_word = chan->scan_type.realbits;
>  
>  	spi_message_init_with_transfers(&adc->msg, xfers, 3);
> @@ -193,7 +196,7 @@ static int ad7944_4wire_mode_init_msg(struct device *dev,
> struct ad7944_adc *adc
>  	xfers[0].delay.unit = SPI_DELAY_UNIT_NSECS;
>  
>  	xfers[1].rx_buf = &adc->sample.raw;
> -	xfers[1].len = BITS_TO_BYTES(chan->scan_type.storagebits);
> +	xfers[1].len = AD7944_SPI_BYTES(chan->scan_type);
>  	xfers[1].bits_per_word = chan->scan_type.realbits;
>  
>  	spi_message_init_with_transfers(&adc->msg, xfers, 2);
> @@ -228,7 +231,7 @@ static int ad7944_chain_mode_init_msg(struct device *dev,
> struct ad7944_adc *adc
>  	xfers[0].delay.unit = SPI_DELAY_UNIT_NSECS;
>  
>  	xfers[1].rx_buf = adc->chain_mode_buf;
> -	xfers[1].len = BITS_TO_BYTES(chan->scan_type.storagebits) *
> n_chain_dev;
> +	xfers[1].len = AD7944_SPI_BYTES(chan->scan_type) * n_chain_dev;
>  	xfers[1].bits_per_word = chan->scan_type.realbits;
>  
>  	spi_message_init_with_transfers(&adc->msg, xfers, 2);
> @@ -274,12 +277,12 @@ static int ad7944_single_conversion(struct ad7944_adc
> *adc,
>  		return ret;
>  
>  	if (adc->spi_mode == AD7944_SPI_MODE_CHAIN) {
> -		if (chan->scan_type.storagebits > 16)
> +		if (chan->scan_type.realbits > 16)
>  			*val = ((u32 *)adc->chain_mode_buf)[chan-
> >scan_index];
>  		else
>  			*val = ((u16 *)adc->chain_mode_buf)[chan-
> >scan_index];
>  	} else {
> -		if (chan->scan_type.storagebits > 16)
> +		if (chan->scan_type.realbits > 16)
>  			*val = adc->sample.raw.u32;
>  		else
>  			*val = adc->sample.raw.u16;
> @@ -409,8 +412,7 @@ static int ad7944_chain_mode_alloc(struct device *dev,
>  	/* 1 word for each voltage channel + aligned u64 for timestamp */
>  
>  	chain_mode_buf_size = ALIGN(n_chain_dev *
> -		BITS_TO_BYTES(chan[0].scan_type.storagebits), sizeof(u64))
> -		+ sizeof(u64);
> +		AD7944_SPI_BYTES(chan[0].scan_type), sizeof(u64)) +
> sizeof(u64);
>  	buf = devm_kzalloc(dev, chain_mode_buf_size, GFP_KERNEL);
>  	if (!buf)
>  		return -ENOMEM;
>