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

[Jonathan Cameron]

On Sat, 12 Sep 2026 16:25:17 +0300
Abdelnasser Hussein <abdelnasserhussein11@xxxxxxxxx> wrote:

> Stack buffers are unsafe for DMA with VMAP_STACK enabled because
> they are not physically contiguous and can share cache lines. This
> causes data corruption.

This one has come up a few times as a false or at least misleading
explanation.  It really doesn't have anything to do with VMAP_STACK
as that only a most deals with pages and cachelines which matter here
are much smaller.

Any stack variable is a problem simply because we don't control the
data layout and pretty much anything else can end up in the same
cache line. 

> 
> Fix this by using a dedicated rx_buf aligned with IIO_DMA_MINALIGN
> in the device state structure to ensure cache coherency. This also
> corrects the size argument in spi_read() to match the new buffer.

Make sure you understand why this alignment marking works to ensure
the buffer is in a cache line on it's own.

However, don't do this for a single use small buffer.
Use spi_write_then_read() (with 0 sized write) but also make
sure you understand why that works.

Jonathan


> 
> 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, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
> index acf26c5a267a..2fd2d6796603 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -53,6 +53,7 @@ struct ad7816_chip_info {
>  	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 {
> @@ -68,7 +69,6 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
>  {
>  	struct spi_device *spi_dev = chip->spi_dev;
>  	int ret;
> -	__be16 buf;
>  
>  	guard(mutex)(&chip->lock);
>  
> @@ -96,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;
>  }