| Message ID | 1395257399-359545-63-git-send-email-arnd@arndb.de |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 03/19/2014 01:29 PM, Arnd Bergmann wrote: > In a combined ARMv6/v7 kernel, we cannot use the > movt/movw instructions to load an immediate, as they > are not valid on ARMv6. > > This changes the file to use an indirect load instead, > as lots of other implementations do. Hmmm. This code is guaranteed to only execute on Tegra (well, perhaps someone can turn on the wrong debug option and run it on non-Tegra, but then it's guaranteed not to work since the HW it touches doesn't exist). As such, the code ought to be able to use ARMv7 instructions. As a fix for similar issues in assembly code in arch/arm/mach-tegra/*.S, Makefile there does: asflags-y += -march=armv7-a (I think you added that? Yes, in 408e713545ca "ARM: tegra: build assembly files with -march=armv7-a") Shouldn't we use the same fix in this case too? -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 19 March 2014 13:40:06 Stephen Warren wrote: > Hmmm. This code is guaranteed to only execute on Tegra (well, perhaps > someone can turn on the wrong debug option and run it on non-Tegra, but > then it's guaranteed not to work since the HW it touches doesn't exist). > As such, the code ought to be able to use ARMv7 instructions. > > As a fix for similar issues in assembly code in arch/arm/mach-tegra/*.S, > Makefile there does: > > asflags-y += -march=armv7-a > > (I think you added that? Yes, in 408e713545ca "ARM: tegra: build > assembly files with -march=armv7-a") > > Shouldn't we use the same fix in this case too? That was my first idea, but I couldn't come up with a nice way to do this for arch/arm/kernel/debug.S, which #includes the specific implementation. I'd rather not put lots of per-platform hacks into arch/arm/kernel/Makefile. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/19/2014 01:51 PM, Arnd Bergmann wrote: > On Wednesday 19 March 2014 13:40:06 Stephen Warren wrote: >> Hmmm. This code is guaranteed to only execute on Tegra (well, perhaps >> someone can turn on the wrong debug option and run it on non-Tegra, but >> then it's guaranteed not to work since the HW it touches doesn't exist). >> As such, the code ought to be able to use ARMv7 instructions. >> >> As a fix for similar issues in assembly code in arch/arm/mach-tegra/*.S, >> Makefile there does: >> >> asflags-y += -march=armv7-a >> >> (I think you added that? Yes, in 408e713545ca "ARM: tegra: build >> assembly files with -march=armv7-a") >> >> Shouldn't we use the same fix in this case too? > > That was my first idea, but I couldn't come up with a nice way to do this > for arch/arm/kernel/debug.S, which #includes the specific implementation. > > I'd rather not put lots of per-platform hacks into arch/arm/kernel/Makefile. Oh, I guess the fact it's include makes it a bit more painful. You could deal with it in Kconfig reasonably easily by having each of DEBUG_*_UART select a config option indicating how to compile code that includes DEBUG_LL_INCLUDE, and then have the Makefiles set up asflags based on those. Still, that would be a lot of work and this patch is simpler. Tested-by: Stephen Warren <swarren@nvidia.com> Acked-by: Stephen Warren <swarren@nvidia.com> -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 19 March 2014, Stephen Warren wrote: > On 03/19/2014 01:51 PM, Arnd Bergmann wrote: > > On Wednesday 19 March 2014 13:40:06 Stephen Warren wrote: > >> Hmmm. This code is guaranteed to only execute on Tegra (well, perhaps > >> someone can turn on the wrong debug option and run it on non-Tegra, but > >> then it's guaranteed not to work since the HW it touches doesn't exist). > >> As such, the code ought to be able to use ARMv7 instructions. > >> > >> As a fix for similar issues in assembly code in arch/arm/mach-tegra/*.S, > >> Makefile there does: > >> > >> asflags-y += -march=armv7-a > >> > >> (I think you added that? Yes, in 408e713545ca "ARM: tegra: build > >> assembly files with -march=armv7-a") > >> > >> Shouldn't we use the same fix in this case too? > > > > That was my first idea, but I couldn't come up with a nice way to do this > > for arch/arm/kernel/debug.S, which #includes the specific implementation. > > > > I'd rather not put lots of per-platform hacks into arch/arm/kernel/Makefile. > > Oh, I guess the fact it's include makes it a bit more painful. > > You could deal with it in Kconfig reasonably easily by having each of > DEBUG_*_UART select a config option indicating how to compile code that > includes DEBUG_LL_INCLUDE, and then have the Makefiles set up asflags > based on those. Still, that would be a lot of work and this patch is > simpler. > > Tested-by: Stephen Warren <swarren@nvidia.com> > Acked-by: Stephen Warren <swarren@nvidia.com> Ok, thanks for testing! I'm always cautious aobut my own assembly skills, so I'm glad this worked. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/include/debug/tegra.S b/arch/arm/include/debug/tegra.S index f98763f..3bc8059 100644 --- a/arch/arm/include/debug/tegra.S +++ b/arch/arm/include/debug/tegra.S @@ -53,8 +53,7 @@ #define checkuart(rp, rv, lhu, bit, uart) \ /* Load address of CLK_RST register */ \ - movw rp, #TEGRA_CLK_RST_DEVICES_##lhu & 0xffff ; \ - movt rp, #TEGRA_CLK_RST_DEVICES_##lhu >> 16 ; \ + ldr rp, =TEGRA_CLK_RST_DEVICES_##lhu ; \ /* Load value from CLK_RST register */ \ ldr rp, [rp, #0] ; \ /* Test UART's reset bit */ \ @@ -62,8 +61,7 @@ /* If set, can't use UART; jump to save no UART */ \ bne 90f ; \ /* Load address of CLK_OUT_ENB register */ \ - movw rp, #TEGRA_CLK_OUT_ENB_##lhu & 0xffff ; \ - movt rp, #TEGRA_CLK_OUT_ENB_##lhu >> 16 ; \ + ldr rp, =TEGRA_CLK_OUT_ENB_##lhu ; \ /* Load value from CLK_OUT_ENB register */ \ ldr rp, [rp, #0] ; \ /* Test UART's clock enable bit */ \ @@ -71,8 +69,7 @@ /* If clear, can't use UART; jump to save no UART */ \ beq 90f ; \ /* Passed all tests, load address of UART registers */ \ - movw rp, #TEGRA_UART##uart##_BASE & 0xffff ; \ - movt rp, #TEGRA_UART##uart##_BASE >> 16 ; \ + ldr rp, =TEGRA_UART##uart##_BASE ; \ /* Jump to save UART address */ \ b 91f @@ -90,15 +87,16 @@ #ifdef CONFIG_TEGRA_DEBUG_UART_AUTO_ODMDATA /* Check ODMDATA */ -10: movw \rp, #TEGRA_PMC_SCRATCH20 & 0xffff - movt \rp, #TEGRA_PMC_SCRATCH20 >> 16 +10: ldr \rp, =TEGRA_PMC_SCRATCH20 ldr \rp, [\rp, #0] @ Load PMC_SCRATCH20 - ubfx \rv, \rp, #18, #2 @ 19:18 are console type + lsr \rv, \rp, #18 @ 19:18 are console type + and \rv, \rv, #3 cmp \rv, #2 @ 2 and 3 mean DCC, UART beq 11f @ some boards swap the meaning cmp \rv, #3 @ so accept either bne 90f -11: ubfx \rv, \rp, #15, #3 @ 17:15 are UART ID +11: lsr \rv, \rp, #15 @ 17:15 are UART ID + and \rv, #7 cmp \rv, #0 @ UART 0? beq 20f cmp \rv, #1 @ UART 1?
In a combined ARMv6/v7 kernel, we cannot use the movt/movw instructions to load an immediate, as they are not valid on ARMv6. This changes the file to use an indirect load instead, as lots of other implementations do. Signed-off-by: Arnd Bergmann <arnd@arndb.de> Cc: Stephen Warren <swarren@wwwdotorg.org> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: linux-tegra@vger.kernel.org --- arch/arm/include/debug/tegra.S | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)