On Sat, 2024-01-27 at 15:56 +0000, Jonathan Cameron wrote: > On Mon, 22 Jan 2024 15:15:41 +0100 > Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@xxxxxxxxxx> wrote: > > > From: Nuno Sa <nuno.sa@xxxxxxxxxx> > > > > 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 st_sensors common buffer. > > > > [1]: > > https://lore.kernel.org/linux-iio/20220508175712.647246-2-jic23@xxxxxxxxxx/ > > > > Fixes: e031d5f558f1 ("iio:st_sensors: remove buffer allocation at each > > buffer enable") > > Signed-off-by: Nuno Sa <nuno.sa@xxxxxxxxxx> > > --- > > include/linux/iio/common/st_sensors.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/linux/iio/common/st_sensors.h > > b/include/linux/iio/common/st_sensors.h > > index 607c3a89a647..a02652cf4862 100644 > > --- a/include/linux/iio/common/st_sensors.h > > +++ b/include/linux/iio/common/st_sensors.h > > @@ -258,7 +258,7 @@ struct st_sensor_data { > > bool hw_irq_trigger; > > s64 hw_timestamp; > > > > - char buffer_data[ST_SENSORS_MAX_BUFFER_SIZE] ____cacheline_aligned; > > + char buffer_data[ST_SENSORS_MAX_BUFFER_SIZE] > > __aligned(IIO_DMA_MINALIGN); > > > > struct mutex odr_lock; > Hi Nuno. > > This is another problem. There should be nothing after a DMA safe buffer > embedded > in a structure like we do here. We rely on C structure padding to ensure the > rest of the __aligned(IIO_DMA_MINALIGN) region is unused and that doesn't work > if the buffer isn't the last element. > > My guess is we are safe to just reorder this before the buffer. > Nuno, do you mind spinning a v2 that does that as well as the size change. > Hi Jonathan, Somehow I failed to see your reply and was so focused on just having the proper alignment that completely forgot the mutex after the buffer is obviously problematic. Good catch! Will spin a v2. - Nuno Sá
