On Wed, 4 May 2022 13:00:58 +0000
"Sa, Nuno" <Nuno.Sa@xxxxxxxxxx> wrote:
> Hi Jonathan,
>
> > From: Jonathan Cameron <jic23@xxxxxxxxxx>
> > Sent: Tuesday, May 3, 2022 10:58 AM
> > To: linux-iio@xxxxxxxxxxxxxxx
> > Cc: Akinobu Mita <akinobu.mita@xxxxxxxxx>; Alexandru Lazar
> > <alazar@xxxxxxxxxxxxx>; Tachici, Alexandru
> > <Alexandru.Tachici@xxxxxxxxxx>; Miclaus, Antoniu
> > <Antoniu.Miclaus@xxxxxxxxxx>; Charles-Antoine Couret <charles-
> > antoine.couret@xxxxxxxxxxxxx>; Tanislav, Cosmin
> > <Cosmin.Tanislav@xxxxxxxxxx>; Cristian Pop
> > <cristian.pop@xxxxxxxxxx>; David Lechner <david@xxxxxxxxxxxxxx>;
> > Ivan Mikhaylov <i.mikhaylov@xxxxxxxxx>; Jacopo Mondi
> > <jacopo+renesas@xxxxxxxxxx>; Jean-Baptiste Maneyrol
> > <jmaneyrol@xxxxxxxxxxxxxx>; Lars-Peter Clausen
> > <lars@xxxxxxxxxx>; Marcelo Schmitt <marcelo.schmitt1@xxxxxxxxx>;
> > Mårten Lindahl <martenli@xxxxxxxx>; Matt Ranostay
> > <mranostay@xxxxxxxxx>; Hennerich, Michael
> > <Michael.Hennerich@xxxxxxxxxx>; Michael Welling
> > <mwelling@xxxxxxxx>; Mugilraj Dhavachelvan
> > <dmugil2000@xxxxxxxxx>; Navin Sankar Velliangiri
> > <navin@xxxxxxxxxxx>; Sa, Nuno <Nuno.Sa@xxxxxxxxxx>; Paul
> > Cercueil <paul@xxxxxxxxxxxxxxx>; Phil Reid
> > <preid@xxxxxxxxxxxxxxxxx>; Puranjay Mohan
> > <puranjay12@xxxxxxxxx>; Ricardo Ribalda <ribalda@xxxxxxxxxx>;
> > Robert Jones <rjones@xxxxxxxxxxxxx>; Rui Miguel Silva
> > <rui.silva@xxxxxxxxxx>; Sean Nyekjaer <sean.nyekjaer@xxxxxxxxx>;
> > Tomas Melin <tomas.melin@xxxxxxxxxxx>; Tomislav Denis
> > <tomislav.denis@xxxxxxx>; Uwe Kleine-König <u.kleine-
> > koenig@xxxxxxxxxxxxxx>; Jonathan Cameron
> > <Jonathan.Cameron@xxxxxxxxxx>; catalin.marinas@xxxxxxx
> > Subject: [PATCH 00/92] IIO: Fix alignment of buffers for DMA
> >
> > [External]
> >
> > From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> >
> > Many years ago, IIO started using ____cacheline_aligned to ensure
> > that buffers that might be used for DMA were not sharing a cacheline
> > with other data that might lead to DMA safety issues.
> >
> > As it turns out, that was fine at the time, but not based on the
> > correct alignment requirement (though I believe it was a conservative
> > choice at the time). Note that on many architectures this was
> > introducing
> > unecessary padding. We should have been using
> > ARCH_KMALLOC_MINALIGN
> > as other subsystems such as crypto have done for a long time.
> >
> > Patch 1 discription contains more information but in short, there are
> > ARM64 SoCs out their that have a larger cachline size for caches
> > beyond
> > L1. In many cases they maintain coherency for all DMA devices
> > attached
> > and so this isn't a problem, but there are exceptions that do not.
> >
> > So, this is a rather large patch set and just covers those drivers
> > that are in the last kernel release and in drivers/iio.
> >
> > Many of these drivers are somewhat old so I haven't specifically
> > cc'd anyone so will be relying on those kind enough to sanity check
> > patches on drivers that are beyond their own.
> >
> > Given there is ongoing discussion around reducing the alignment
> > requirements where possible, I've adopted the existing IIO_ALIGN
> > define througout. That way we have a single point to update if
> > that becomes relevant in future.
> >
>
> Nice to see this in... Since we are here, I guess in a couple of patches where we have:
>
> struct {
> ...
> u8 rx[] __aligned();
> u8 tx[] __aligned();
> };
>
> we could make it such that only the first member is aligned. But bah, I get that this
> is already a huge patchset to diverge from it. Anyhow,
Yup. I thought about tidying those up but decided it would just make
things harder to explain so could be done as a follow up.
I should have said so in the cover letter though!
I particularly like the case where (a long time ago) I added
a question on whether there was a need for separate alignment
and left that question in the final code. Oops.
>
> Acked-by: Nuno Sá <nuno.sa@xxxxxxxxxx>
Thanks for looking through all these.
We haven't yet concluded on the questions of naming and possible
additional define for __aligned(XXX) but if we do make such a change
it should be purely mechanical hence I'll keep all the tags people
have given and not bother them again :)
Thanks,
Jonathan
