| Message ID | 1344448286-14144-1-git-send-email-amartin@nvidia.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
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
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.
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();
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