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

On Sun, 2 Aug 2026 10:31:29 -0500
David Lechner <[email protected]> wrote:

> 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`. 

We've had a couple of these recently.  ____cacheline_aligned is simply
wrong and I'm curious where that is coming from?  That's the performance
hint cache line size, typically that of l1 and l2. In some systems other
caches before the incoherent SPI controllers have larger cacheline sizes
and we have to align to those.

The correct option if not using the IIO one is __aligned(ARCH_DMA_MINALIGN)

For historical reasons IIO has it's own version of that which predates
all architectures providing ARCH_DMA_MINALIGN.

> 
> __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;
> >  }
> >    
> 
>