From mboxrd@z Thu Jan 1 00:00:00 1970 From: Aneesh V Date: Fri, 22 Jun 2012 15:11:36 -0700 Subject: [U-Boot] [PATCH] arm: enable unaligned access on ARMv7 In-Reply-To: <1340365637.1381.38.camel@tellur> References: <1338918451-10126-1-git-send-email-dev@lynxeye.de> <4FCE52FB.3020501@wwwdotorg.org> <1338923180.1377.2.camel@tellur> <20120622111501.21e1fdcc@aari01-12> <1340357816.1381.21.camel@tellur> <20120622131654.2c694a82@aari01-12> <1340365637.1381.38.camel@tellur> Message-ID: <4FE4ED98.2030103@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de +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/ best regards, Aneesh