On 06/22/2012 03:11 PM, Aneesh V wrote:
+Tom
Hi Lucas,
On 06/22/2012 04:47 AM, Lucas Stach wrote:
Hi Albert,
Am Freitag, den 22.06.2012, 13:16 +0200 schrieb Albert ARIBAUD:
Hi Lucas,
Linux in particular does reinitialize this state and I expect any
reasonable OS to do so.
Then what is the point of enabling it on U-Boot? Does it fix some
issue whereby some mis-aligned piece of data cannot be properly
aligned?
Yes, it fixes U-Boot USB on Tegra, when built with a recent toolchain.
Fixing the alignment of some of the structures in the USB code should
also be done, but this is a whole lot more invasive and requires some
more thought, as the discussion about this on LKML shows. The issue
doesn't show for older toolchains, as they by default emit code to
work around unaligned accesses.
This patch fixes all unaligned issues, that may appear with recent
toolchains. We avoid having to instruct the toolchain to work around
unaligned accesses and gain better performance in cases where it is
needed.
I am not too happy with enabling a lax behavior only to avoid an
issue which apparently is diagnosed and could / should be fixed at
its root. Can you point me to the relevant LKML thread
so that I get the details and understand what prevents fixing this by
'simply' aligning the USB structures?
I'm with you that the issue for this particular fault that I ran into
should be fixed at it's root and I will do so as soon as I have enough
time to do so, i.e. within the next three weeks.
You can find a thread about this here:
https://lkml.org/lkml/2011/4/27/278
The problem here is that simply removing the packed attribute is not the
right thing to do for structures that are used for accessing hardware
registers. I have to read a bit more of the USB code to come up with the
right thing to do for every structure there.
But apart from this, we certainly have situations where we have
unaligned accesses that are justified and could not be removed.
Activating the aligned access hardware check is crippling a feature of
the ARMv7 architecture that's apparently useful enough that all recent
toolchains default to using it and not using compiler side workarounds.
Furthermore setting the check bit doesn't even deactivate unaligned
access (there is also a bit for this, which is forced to 1 by all v7
implementations), but just traps the processor in case you care about
such unaligned accesses. Linux for example only sets this check bit if
you choose to install a trap handler for this.
I cannot see how enabling a hardware feature can be seen as allowing of
lax behaviour. As some of the USB structs are used to access hardware
registers, we can not align every struct there. Our options are either:
1. instruct the toolchain to insert workaround code or
2. allow v7 hardware to do the unaligned access on it's own
My comment about fixing the USB code applies only to part of the structs
used there as some of them generate _unnecessary_ unaligned accesses,
the issue that all unaligned accesses fail with current U-Boot built
with a recent toolchain has to be fixed either way and is exactly what
this patch does.
I think this issue was discussed before here(I haven't gone through all
the details of your problem, but it looks similar). And I think Tom
fixed this by wrapping the problematic accesses with get/set_unaligned().
Please look at this thread, especially starting from my post reporting
the OMAP4 regression:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/113347/
BTW, I agree that enabling un-aligned access is not a bad idea.
br,
Aneeesh
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
