Messages in this thread | | | Subject | Re: [PATCH] iio: ad_sigma_delta: Don't put SPI transfer buffer on the stack | From | Lars-Peter Clausen <> | Date | Thu, 12 Nov 2020 11:52:42 +0100 |
| |
On 11/12/20 11:14 AM, Alexandru Ardelean wrote: > On Thu, Nov 12, 2020 at 11:55 AM Lars-Peter Clausen <lars@metafoo.de> wrote: >> On 11/12/20 10:10 AM, Alexandru Ardelean wrote: >>> From: Lars-Peter Clausen <lars@metafoo.de> >>> >>> Use a heap allocated memory for the SPI transfer buffer. Using stack memory >>> can corrupt stack memory when using DMA on some systems. >>> >>> This change adds 4 bytes at the end of the current DMA buffer, which will >>> be used by the trigger handler. >>> This is required because the first 4 bytes are reserved for register data. >>> >>> Fixes: af3008485ea03 ("iio:adc: Add common code for ADI Sigma Delta devices") >>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> >>> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> >>> --- >>> drivers/iio/adc/ad_sigma_delta.c | 4 ++-- >>> include/linux/iio/adc/ad_sigma_delta.h | 2 +- >>> 2 files changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c >>> index 86039e9ecaca..33297f26508a 100644 >>> --- a/drivers/iio/adc/ad_sigma_delta.c >>> +++ b/drivers/iio/adc/ad_sigma_delta.c >>> @@ -395,11 +395,11 @@ static irqreturn_t ad_sd_trigger_handler(int irq, void *p) >>> struct iio_poll_func *pf = p; >>> struct iio_dev *indio_dev = pf->indio_dev; >>> struct ad_sigma_delta *sigma_delta = iio_device_get_drvdata(indio_dev); >>> + uint8_t *data = &sigma_delta->data[4]; >>> unsigned int reg_size; >>> unsigned int data_reg; >>> - uint8_t data[16]; >>> >>> - memset(data, 0x00, 16); >>> + memset(data, 0x00, 4); >> Younger me didn't know what he was doing, this is wrong. We need the >> extra space for the padding and timestamp. >> >> We also can't put the beginning of the buffer at an 4 byte offset since >> it needs to be 8 byte aligned for the timestamp. > I'll correct this. > I was re-spinning this out of some old patches and discussions on this > that I have. > So, then this becomes 24 bytes? Or 16? > > Something like: > uint8_t data[24] ____cacheline_aligned; > > uint8_t *data = &sigma_delta->data[8];
Yes.
| |