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

[Jonathan Cameron <jic23@kernel.org>]

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.

Jonathan

>  };
> 
> ---
> base-commit: f9c0358aadcba16d04d139a5412b413eeee87afe
> change-id: 20240122-dev_dma_safety_stm-c7016673f4ec
> --
> 
> Thanks!
> - Nuno Sá
>