Re: [PATCH] iio: commom: st_sensors: ensure proper DMA alignment

["Nuno Sá" <noname.nuno@gmail.com>]

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@kernel.org> wrote:
> 
> > From: Nuno Sa <nuno.sa@analog.com>
> > 
> > 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@kernel.org/
> > 
> > Fixes: e031d5f558f1 ("iio:st_sensors: remove buffer allocation at each
> > buffer enable")
> > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > ---
> >  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á