Re: [PATCH] counter: fix privdata alignment

[William Breathitt Gray <william.gray@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 1849 bytes --]

On Mon, Feb 05, 2024 at 04:58:14PM +0100, Nuno Sa via B4 Relay wrote:
> From: Nuno Sa <nuno.sa@analog.com>
> 
> Aligning to the L1 cache does guarantee the same alignment as kmallocing
> an object [1]. Furthermore, in some platforms, that alignment is not
> sufficient for DMA safety (in case someone wants to have a DMA safe
> buffer in privdata) [2].
> 
> Sometime ago, we had the same fixes in IIO.
> 
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/devres.c#n35
> [2]: https://lore.kernel.org/linux-iio/20220508175712.647246-2-jic23@kernel.org/
> 
> Fixes: c18e2760308e ("counter: Provide alternative counter registration functions")
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> ---
> William, if you prefer, we can do something like in IIO and add a
> specific COUNTER_DMA_MINALIGN define
> ---
>  drivers/counter/counter-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/counter/counter-core.c b/drivers/counter/counter-core.c
> index 09c77afb33ca..073bf6b67a57 100644
> --- a/drivers/counter/counter-core.c
> +++ b/drivers/counter/counter-core.c
> @@ -34,7 +34,7 @@ struct counter_device_allochelper {
>  	 * This is cache line aligned to ensure private data behaves like if it
>  	 * were kmalloced separately.
>  	 */
> -	unsigned long privdata[] ____cacheline_aligned;
> +	unsigned long privdata[] __aligned(ARCH_DMA_MINALIGN);
>  };
>  
>  static void counter_device_release(struct device *dev)
> 
> ---
> base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
> change-id: 20240205-counter-align-fix-3faebfb572af
> --
> 
> Thanks!
> - Nuno Sá

Hi Nunon,

This change sounds reasonable, but should the comment block above
privdata be updated to reflect the change?

Sincerely,

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]