On Sun, 27 Nov 2022 18:48:44 +0200 Lauri Kasanen <cand@xxxxxxx> wrote: > On Sun, 27 Nov 2022 14:41:14 +0000 > Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > > From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> > > > > The use of ____cacheline_aligned to ensure a buffer is DMA safe only > > enforces the start of the buffer alignment. In this case, sufficient > > alignment is already ensured by the use of kzalloc(). > > ____cacheline_aligned does not ensure that no other members of the > > structure are placed in the same cacheline after the end of the > > buffer marked. Thus to ensure a DMA safe buffer it must be at the end > > of the structure. > > This move is unnecessary, because the cacheline is 16 bytes and the > buffer is 64 bytes. Ah. That particular option hadn't occurred to me (and I'd failed to notice how big the buffer is :( ). The marking isn't needed at all then as the allocation is already guaranteed to be sufficiently aligned. However, maybe that is a bit subtle and having some sort of marking is useful. Curious question though, why is the buffer so big? Each struct joydata is 8 bytes I think, but the driver only accesses 4 of them. Is the hardware putting garbage into the remaining 2 cachelines or is there something subtle going on? Or given my earlier success, maybe I'm misreading the code entirely. Jonathan > > > Whilst here switch from ____cacheline_aligned to > > __aligned(ARCH_KMALLOC_MINALIGN) as on some architectures, with variable > > sized cacheline lines across their cache hierarchy, require this > > greater alignment guarantee for DMA safety. Make this change throughout > > the driver as it reduces need for a reader to know about the particular > > architecture. > > This change looks ok. > > - Lauri
