[U-Boot] [PATCH] usb: do explicit unaligned accesses

[Marek Vasut <marex@denx.de>]

Dear Albert ARIBAUD,

> Hi Marek,
> 
> On Sat, 1 Sep 2012 16:34:09 +0200, Marek Vasut <marex@denx.de> wrote:
> > Dear Albert ARIBAUD,
> > 
> > > Hi Marek,
> > > 
> > > On Sat, 1 Sep 2012 00:16:43 +0200, Marek Vasut <marex@denx.de>
> > > 
> > > wrote:
> > > > Dear Albert ARIBAUD,
> > > > 
> > > > > I think you are talking about lumping small-sized accesses
> > > > > together into a bigger access possibly aligned.
> > > > 
> > > > This is exactly what I mean.
> > > > 
> > > > > If I am correct, then I don't
> > > > > think this is related to misaligned accesses.
> > > > 
> > > > Why won't it be? Such access can in the end turn out to be
> > > > aligned, therefore leveraging all the penalty.
> > > 
> > > I have not expressed myself clearly. Yes, access lumping is related
> > > to access alignment. What I meant is: disallowing misaligned native
> > > accesses will not prevent access lumping. Misalignment restrictions
> > > do indeed restrict how such lumpings will happen, but it does not
> > > prevent lumping per se.
> > > 
> > > One place where lumping and misalignement prevention did clash was
> > > raised in the previous discussion: a 7+1 bytes function-local char
> > > array was allocated on a non-aligned address (which is possible and
> > > normal because it is a char) and was initialized with some content.
> > > The compiler lumped the initialization as two misaligned 32-byte
> > > native accesses, despite misaligned native accesses being forbidden
> > > by compiler command line options. This was a compiler bug.
> > 
> > But that'd mean that instead of fixing a compiler, we'd be doing a
> > workaround in our code?
> 
> Not exactly.
> 
> First, in this instance, a fix to the compiler has been at least
> requested, if not already applied (I would need to check this). The fix
> causes the compiler to still generate misaligned 32-bit accesses *if*
> misaligned native accesses are allowed, and to use only allowed
> accesses otherwise.

But then again, this is compiler bug we exposed, no need to hide it. I'm firm on 
this one.

> Second, I do not ask U-Boot contributors to mark code as explicitly
> unaligned when the misaligned access is caused by a compiler or
> code error; I ask them to mark code as unaligned when the misaligned
> access is *unavoidable* because the HW or some standard imposes it.

I see, I'm starting to see your point. Maybe because I've missed the previous 
discussion.

> Here, the specification from which the USB struc is derived imposes a
> misaligned field. This, rather than any compiler bug, makes the
> misaligned access *unavoidable*. And because it is, I ask that it be
> marked so, by the explicit use of unaligned accessors.

Still, this is unaligned only on ARM, not on maybe some other arches, right?

> > > > > If I am not correct, can
> > > > > you please detail what you meant?
> > > > > 
> > > > > > Besides, right now, the code is much more readable. So I
> > > > > > really don't like adding some strange macros to force crazy
> > > > > > aligned access if the compiler can do it for us and can do it
> > > > > > better.
> > > > > 
> > > > > I personally would let the compiler do it too, but I prefer it
> > > > > to be clearly indicated to the reader of the code when an
> > > > > access is known to be misaligned.
> > > > 
> > > > I'd enable enable the Alignment trapping in the CPU and die on an
> > > > unaligned access at runtime -- to indicate the user that he should
> > > > fix his bloody compiler.
> > > 
> > > ... or fix his bloody structure, or fix his bloody f...ixing pointer
> > > arithmetic, or... but I do agree with the trapping, and that's my
> > > plan.
> > > 
> > > However other architectures may need, or choose, another stance on
> > > alignments, and it is best if they don't have to discover intended
> > > misaligned accesses the hard way.
> > 
> > Yet still, in such case, valid compiler has to generate valid
> > workaround code.
> 
> Yes. However, letting the compiler generate workarounds may end up
> letting it generate workarounds for misaligned accesses caused by errors
> or bugs also. Marking the code explicitly helps telling which is which
> too.

Does this work across architectures too? Like, on arm it's misaligned, on intel 
it isnt.

> > > Thus my opinion that any misaligned
> > > access which cannot be fixed should not be sliently left for the
> > > compiler to handle, but should (also) be clearly marked as such, if
> > > only for humans to notice.
> > 
> > I can't say I agree here ... since it's a really ad-hoc solution. I
> > can't say I see any real benefit other than that it's hiding possible
> > compiler bugs :-(
> 
> Here it is barely an ad hoc solution, as the alternative would be
> fixing the hardware or worse, spec (can someone tell us where this
> misaligned struct field originates from exactly, hw or USB spec?)

http://www.intel.com/technology/usb/download/ehci-r10.pdf I think you're looking 
around 3.6 .

> > Best regards,
> > Marek Vasut
> 
> Amicalement,