Re: [PATCH v3 2/2] staging: iio: ad7816: avoid DMA from stack in spi_read

[Jonathan Cameron <jic23@kernel.org>]

On Mon,  3 Aug 2026 16:43:46 +0300
Abdelnasser Hussein <abdelnasserhussein11@gmail.com> wrote:

> The SPI transfer buffer is allocated on the stack, which is unsafe when
> the SPI core performs DMA transfers. With VMAP_STACK enabled, this can
> lead to DMA mapping failures because the stack is not guaranteed to be
> DMA-accessible.

Hmm. That's a new one for me. I was just thinking it was the cache coherency
problems that meant we shouldn't DMA to/from the stack because we can't
guarantee what is going to be in the cacheline.  Looking briefly into
it it seems this is another good reason not to do it.

> 
> Move the buffer into struct ad7816_chip_info to provide storage with an
> appropriate lifetime for DMA, align it with

Why is lifetime relevant here?  The on stack data was fine lifetime wise.

> __aligned(IIO_DMA_MINALIGN), and update the spi_read() sizeof() argument
> to reference the relocated buffer.
> 
> Fixes: 7924425db04a ("staging: iio: adc: new driver for AD7816 devices")
> 
No blank lines in commit block. Sometimes I just fix these up when picking
patches up, but sometimes I get grumpy and bounce them back to submitter
to fix up in a new version.

> Signed-off-by: Abdelnasser Hussein <abdelnasserhussein11@gmail.com>
> ---
>  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 b5a0c2871e00..c58a6bf77020 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -51,6 +51,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 {
> @@ -66,7 +67,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;
>  
>  	mutex_lock(&chip->lock);
>  
> @@ -95,7 +95,7 @@ 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");
>  		mutex_unlock(&chip->lock);
> @@ -103,7 +103,7 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
>  		return ret;
>  	}
>  
> -	*data = be16_to_cpu(buf);
> +	*data = be16_to_cpu(chip->rx_buf);
>  	mutex_unlock(&chip->lock);
>  	return ret;
>  }