Patchwork [U-Boot] tegra20: add assembly wrapper for lowlevel_init()

login
register
mail settings
Submitter Allen Martin
Date Aug. 8, 2012, 5:51 p.m.
Message ID <1344448286-14144-1-git-send-email-amartin@nvidia.com>
Download mbox | patch
Permalink /patch/175946/
State Changes Requested
Delegated to: Tom Rini
Headers show

Comments

Allen Martin - Aug. 8, 2012, 5:51 p.m.
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.

Signed-off-by: Allen Martin <amartin@nvidia.com>
---
 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
Wolfgang Denk - Aug. 8, 2012, 7:44 p.m.
Dear Allen Martin,

In message <1344448286-14144-1-git-send-email-amartin@nvidia.com> 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.
...
> +	/*
> +	 * 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
Tom Rini - Aug. 8, 2012, 7:48 p.m.
On 08/08/2012 12:44 PM, Wolfgang Denk wrote:
> Dear Allen Martin,
> 
> In message <1344448286-14144-1-git-send-email-amartin@nvidia.com> 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.
> ...
>> +	/*
>> +	 * 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...

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.

Patch

diff --git a/arch/arm/cpu/armv7/tegra20/Makefile b/arch/arm/cpu/armv7/tegra20/Makefile
index 5f4035d..2a22f1a 100644
--- a/arch/arm/cpu/armv7/tegra20/Makefile
+++ b/arch/arm/cpu/armv7/tegra20/Makefile
@@ -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)
 
diff --git a/arch/arm/cpu/armv7/tegra20/lowlevel_init.S b/arch/arm/cpu/armv7/tegra20/lowlevel_init.S
new file mode 100644
index 0000000..7976dfb
--- /dev/null
+++ b/arch/arm/cpu/armv7/tegra20/lowlevel_init.S
@@ -0,0 +1,48 @@ 
+/*
+ * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * Derived from code that is:
+ *
+ * (C) Copyright 2010
+ * Texas Instruments, <www.ti.com>
+ *  Aneesh V	<aneesh@ti.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <asm/arch/tegra20.h>
+#include <linux/linkage.h>
+
+ENTRY(lowlevel_init)
+	/*
+	 * 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}
+
+	/*
+	 * go setup pll, mux, memory
+	 */
+	bl	s_init
+	pop	{ip, pc}
+ENDPROC(lowlevel_init)
diff --git a/arch/arm/cpu/tegra20-common/ap20.c b/arch/arm/cpu/tegra20-common/ap20.c
index 2d4705a..00588da 100644
--- a/arch/arm/cpu/tegra20-common/ap20.c
+++ b/arch/arm/cpu/tegra20-common/ap20.c
@@ -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();