Re: [PATCH] counter: fix privdata alignment

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

On Thu, 2024-02-08 at 13:34 -0500, William Breathitt Gray wrote:
> 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?
> 
> 
Hi William,

Yeah, maybe. I can spin a new version with that... To be sure, you mean (in the
comment) private -> privdata, right?

Also realized a typo in the commit message:

"Aligning to the L1 cache does guarantee..." - Obviously, I meant "does
*not*...".

Thanks!
- Nuno Sá