Re: [PATCH v4 2/2] staging: iio: adc: ad7816: Use DMA-safe buffer for SPI read

[Jonathan Cameron]

On Sun,  6 Sep 2026 14:40:49 +0300
Abdelnasser Hussein <abdelnasserhussein11@xxxxxxxxx> wrote:

> Stack buffers are not guaranteed to be cache-coherent and must not
> be used with SPI reads, which can trigger issues with VMAP_STACK.

This needs a rewrite and to show more understanding of what is going
on.  What it says write now implies that some how buffers on the heap
are magically cache coherent and ones on the stack are not

I'd suggest watching the talk Wolfram Sang gave at ELCE a few years
back on this topic as a starting point.

> Fix this by using a dedicated DMA-safe rx_buf aligned with
> IIO_DMA_MINALIGN. Also, correct the sizeof() argument from *data
> to chip->rx_buf to match the buffer size.

They are the same size.  I'm not saying the fix is wrong but calling
it correct is misleading.

> 
> Fixes: 7024425db64a ("staging: iio: adc: new driver for AD7816 devices")
> Signed-off-by: Abdelnasser Hussein <abdelnasserhussein11@xxxxxxxxx>
> ---
>  drivers/staging/iio/adc/ad7816.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
> index ab80b3a889bb..e14ae805f076 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -52,6 +52,8 @@ struct ad7816_chip_info {
>  	u8  oti_data[AD7816_CS_MAX + 1];
>  	u8  channel_id;	/* 0 always be temperature */
>  	u8  mode;
> +	struct mutex lock; /* protect device state during SPI transfers */
> +	__be16 rx_buf __aligned(IIO_DMA_MINALIGN);
>  };
>  
>  enum ad7816_type {
> @@ -94,13 +96,13 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
>  
>  	gpiod_set_value(chip->rdwr_pin, 0);
>  	gpiod_set_value(chip->rdwr_pin, 1);
> -	ret = spi_read(spi_dev, &buf, sizeof(*data));
> +	ret = spi_read(spi_dev, &chip->rx_buf, sizeof(chip->rx_buf));
>  	if (ret < 0) {
>  		dev_err(&spi_dev->dev, "SPI data read error\n");
>  		return ret;
>  	}
>  
> -	*data = be16_to_cpu(buf);
> +	*data = be16_to_cpu(chip->rx_buf);
>  
>  	return ret;
>  }