Skip to contentSkip to conversation

tegra20: add assembly wrapper for lowlevel_init()

PATCH New Allen Martin 3 participants 3 messages Aug 8, 2012

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 tegra20: add assembly wrapper for lowlevel_init()

lowlevel_init() is called before stack is initialized, so it's not safe to call directly into C code. Copy this wrapper from omap that saves off the ip register and sets up a temporary stack. This fixes a hang using CodeSourcery toolchain.

---
 arch/arm/cpu/armv7/tegra20/Makefile        |    5 +--
 arch/arm/cpu/armv7/tegra20/lowlevel_init.S |   48 ++++++++++++++++++++++++++++
 arch/arm/cpu/tegra20-common/ap20.c         |    2 +-
 3 files changed, 52 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/tegra20/lowlevel_init.S
arch/arm/cpu/armv7/tegra20/Makefile +3 -2
index 5f4035d..2a22f1a 100644
@@ -29,10 +29,11 @@ LIB	=  $(obj)lib$(SOC).o
 
 COBJS-$(CONFIG_USB_EHCI_TEGRA) += usb.o
 COBJS-$(CONFIG_CMD_ENTERRCM) += cmd_enterrcm.o
+SOBJS	+= lowlevel_init.o
 
 COBJS	:= $(COBJS-y)
-SRCS	:= $(COBJS:.o=.c)
-OBJS	:= $(addprefix $(obj),$(COBJS))
+SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
+OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS))
 
 all:	 $(obj).depend $(LIB)
 
arch/arm/cpu/armv7/tegra20/lowlevel_init.S new file +48 -0

48 lines not on this page · raw message

arch/arm/cpu/tegra20-common/ap20.c +1 -1
index 2d4705a..00588da 100644
@@ -114,7 +114,7 @@ static void init_pmc_scratch(void)
 	writel(odmdata, &pmc->pmc_scratch20);
 }
 
-void lowlevel_init(void)
+void s_init(void)
 {
 	/* Init PMC scratch memory */
 	init_pmc_scratch();
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/1 tegra20: add assembly wrapper for lowlevel_init()

Dear Allen Martin,

Quoted 4 lines · In message <[email protected]> you wrote:

lowlevel_init() is called before stack is initialized, so it's not safe to call directly into C code. Copy this wrapper from omap that saves off the ip register and sets up a temporary stack. This fixes a hang using CodeSourcery toolchain.

...

arch/arm/cpu/armv7/tegra20/lowlevel_init.S line 41

Quoted 9 lines
arch/arm/cpu/armv7/tegra20/lowlevel_init.S new file +9 -0
+	/*
+	 * Setup a temporary stack
+	 */
+	ldr	sp, =LOW_LEVEL_SRAM_STACK
+
+	/*
+	 * Save the old lr(passed in ip) and the current lr to stack
+	 */
+	push	{ip, lr}

Are you sure this provides a valid stack frame? I think there are more requirements to that...

BTW: this LOW_LEVEL_SRAM_STACK is really ugly; It is completely undocumented, and breaks naming conventions (it should be CONFIG_SYS_... instead). I know that this was not introduced by this patch, but it should be fixed ASAP - it's mostly TI and Tegra that use that...

Best regards,

Wolfgang Denk

Signature · 6 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
What is tolerance? -- it is the consequence of humanity. We  are  all
formed  of frailty and error; let us pardon reciprocally each other's
folly -- that is the first law of nature.                  - Voltaire
#2 lore raw

PATCH 1/1 tegra20: add assembly wrapper for lowlevel_init()

Quoted 22 lines · Wolfgang Denk

Dear Allen Martin,

Quoted 4 lines · In message <[email protected]> you wrote:

lowlevel_init() is called before stack is initialized, so it's not safe to call directly into C code. Copy this wrapper from omap that saves off the ip register and sets up a temporary stack. This fixes a hang using CodeSourcery toolchain.

...

arch/arm/cpu/armv7/tegra20/lowlevel_init.S line 41

Quoted 9 lines
arch/arm/cpu/armv7/tegra20/lowlevel_init.S new file +9 -0
+	/*
+	 * Setup a temporary stack
+	 */
+	ldr	sp, =LOW_LEVEL_SRAM_STACK
+
+	/*
+	 * Save the old lr(passed in ip) and the current lr to stack
+	 */
+	push	{ip, lr}

Are you sure this provides a valid stack frame? I think there are more requirements to that...

Quoted 5 lines

BTW: this LOW_LEVEL_SRAM_STACK is really ugly; It is completely undocumented, and breaks naming conventions (it should be CONFIG_SYS_... instead). I know that this was not introduced by this patch, but it should be fixed ASAP - it's mostly TI and Tegra that use that...

So I've been pondering this a little too. TI and now Tegra are the ones that use it, but based on the bug Allen ran into it seems like any of the armv7 platforms could hit it and perhaps we should make this more generic and live in arch/arm/cpu/armv7/lowlevel_init.S or be part of start.S.

Signature · 1 line
Tom