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

On 8/2/26 6:38 AM, Abdelnasser Hussein wrote:
> The SPI core may use DMA for transfers. Using a stack-allocated
> buffer for DMA is unsafe and can trigger faults when VMAP_STACK is
> enabled, since the stack is not guaranteed to be DMA-accessible.
> 
> Move the transfer buffer from the stack into the ad7816_chip_info
> structure so it has a stable lifetime suitable for DMA transfers.
> Mark the buffer with ____cacheline_aligned to ensure proper alignment
> for DMA operations.

Should also mention fixing the "wrong" sizeof() use in the spi_read()
call. It was the correct size, but the wrong variable was referenced.

> 

Probably deserves a Fixes: tag.

> Signed-off-by: Abdelnasser Hussein <[email protected]>
> ---
>  drivers/staging/iio/adc/ad7816.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
> index 0e32a2295990..fcaadebff0f4 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -50,6 +50,7 @@ struct ad7816_chip_info {
>  	u8  oti_data[AD7816_CS_MAX + 1];
>  	u8  channel_id;	/* 0 always be temperature */
>  	u8  mode;
> +	__be16 rx_buf ____cacheline_aligned;

In IIO, we have a special macro for this instead of `____cacheline_aligned`. 

__aligned(IIO_DMA_MINALIGN);

>  };
>  
>  enum ad7816_type {
> @@ -65,7 +66,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;
>  
>  	gpiod_set_value(chip->rdwr_pin, 1);
>  	gpiod_set_value(chip->rdwr_pin, 0);
> @@ -91,14 +91,12 @@ 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;
>  }
>