| Message ID | 20240117-adis-improv-v1-1-7f90e9fad200@analog.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: imu: adis: misc fixes/improvements | expand |
On Wed, 17 Jan 2024 14:10:49 +0100 Nuno Sa <nuno.sa@analog.com> wrote: > Aligning the buffer to the L1 cache is not sufficient in some platforms > as they might have larger cacheline sizes for caches after L1 and thus, > we can't guarantee DMA safety. > > That was the whole reason to introduce IIO_DMA_MINALIGN in [1]. Do the same > for the sigma_delta ADCs. > > [1]: https://lore.kernel.org/linux-iio/20220508175712.647246-2-jic23@kernel.org/ > > Fixes: ccd2b52f4ac6 ("staging:iio: Add common ADIS library") > Signed-off-by: Nuno Sa <nuno.sa@analog.com> Guess I didn't look in the main headers :( Anyhow good to clean this straggler up. I'll apply it to the fixes-togreg branch of iio.git and mark it for stable. I 'think' the definition of IIO_DMA_MINALIGN long got picked up by stable. Jonathan > --- > include/linux/iio/imu/adis.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h > index dc9ea299e088..8898966bc0f0 100644 > --- a/include/linux/iio/imu/adis.h > +++ b/include/linux/iio/imu/adis.h > @@ -11,6 +11,7 @@ > > #include <linux/spi/spi.h> > #include <linux/interrupt.h> > +#include <linux/iio/iio.h> > #include <linux/iio/types.h> > > #define ADIS_WRITE_REG(reg) ((0x80 | (reg))) > @@ -131,7 +132,7 @@ struct adis { > unsigned long irq_flag; > void *buffer; > > - u8 tx[10] ____cacheline_aligned; > + u8 tx[10] __aligned(IIO_DMA_MINALIGN); > u8 rx[4]; > }; > >
On Sun, 2024-01-21 at 16:12 +0000, Jonathan Cameron wrote: > On Wed, 17 Jan 2024 14:10:49 +0100 > Nuno Sa <nuno.sa@analog.com> wrote: > > > Aligning the buffer to the L1 cache is not sufficient in some platforms > > as they might have larger cacheline sizes for caches after L1 and thus, > > we can't guarantee DMA safety. > > > > That was the whole reason to introduce IIO_DMA_MINALIGN in [1]. Do the same > > for the sigma_delta ADCs. > > > > [1]: https://lore.kernel.org/linux-iio/20220508175712.647246-2-jic23@kernel.org/ > > > > Fixes: ccd2b52f4ac6 ("staging:iio: Add common ADIS library") > > Signed-off-by: Nuno Sa <nuno.sa@analog.com> > Guess I didn't look in the main headers :( > Not many users anyways. Doing a git grep shows: git grep "____cacheline_aligned" include/linux/iio/ include/linux/iio/adc/ad_sigma_delta.h:102: uint8_t tx_buf[4] ____cacheline_aligned; include/linux/iio/common/st_sensors.h:261: char buffer_data[ST_SENSORS_MAX_BUFFER_SIZE] ____cacheline_aligned; include/linux/iio/imu/adis.h:134: u8 tx[10]____cacheline_aligned; So we are only missing the st header. I can send patch for it later today. - Nuno Sá
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h index dc9ea299e088..8898966bc0f0 100644 --- a/include/linux/iio/imu/adis.h +++ b/include/linux/iio/imu/adis.h @@ -11,6 +11,7 @@ #include <linux/spi/spi.h> #include <linux/interrupt.h> +#include <linux/iio/iio.h> #include <linux/iio/types.h> #define ADIS_WRITE_REG(reg) ((0x80 | (reg))) @@ -131,7 +132,7 @@ struct adis { unsigned long irq_flag; void *buffer; - u8 tx[10] ____cacheline_aligned; + u8 tx[10] __aligned(IIO_DMA_MINALIGN); u8 rx[4]; };
Aligning the buffer to the L1 cache is not sufficient in some platforms as they might have larger cacheline sizes for caches after L1 and thus, we can't guarantee DMA safety. That was the whole reason to introduce IIO_DMA_MINALIGN in [1]. Do the same for the sigma_delta ADCs. [1]: https://lore.kernel.org/linux-iio/20220508175712.647246-2-jic23@kernel.org/ Fixes: ccd2b52f4ac6 ("staging:iio: Add common ADIS library") Signed-off-by: Nuno Sa <nuno.sa@analog.com> --- include/linux/iio/imu/adis.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)