Message ID | 1371780797-17248-1-git-send-email-sr@denx.de |
---|---|
State | Superseded |
Delegated to: | Tom Rini |
Headers | show |
Hi Stefan, On Fri, 21 Jun 2013 04:13:17 +0200, Stefan Roese <sr@denx.de> wrote: > Fix a problem with a re-assignment of r8 in the SPL version. > > This patch now moves the call to s_init() to a later stage, right before > calling board_init_f(). And makes sure that r8 is correctly initialized > before s_init() is called. r8 now is only written in crt0.S. > > This error was detected on the SPL port for the Compulab CM-T35 board > (OMAP3530). > > Signed-off-by: Stefan Roese <sr@denx.de> > Cc: Tom Rini <trini@ti.com> > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> > --- > v2: > - Change handling/initializing of r8 as suggested by Albert. > It should only be written in crt0.S. > > Tom, while working on this version one question came up: > Is lowlevel_init() (file arch/arm/cpu/armv7/omap3/lowlevel_init.S) > needed any more? It calls cpy_clk_code() to copy some clk init > code into SRAM. But I fail to see if and where this code is really > executed from SRAM. Maybe I missed something. Perhaps you could > shed some light into this. > > Thanks, Stefan > > arch/arm/cpu/armv7/omap3/board.c | 2 -- > arch/arm/cpu/armv7/omap3/lowlevel_init.S | 3 +-- > arch/arm/lib/crt0.S | 7 ++++++- > 3 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c > index b72fadc..8f41dcd 100644 > --- a/arch/arm/cpu/armv7/omap3/board.c > +++ b/arch/arm/cpu/armv7/omap3/board.c > @@ -256,8 +256,6 @@ void s_init(void) > #endif > > #ifdef CONFIG_SPL_BUILD > - gd = &gdata; > - > preloader_console_init(); > > timer_init(); > diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S b/arch/arm/cpu/armv7/omap3/lowlevel_init.S > index eacfef8..8539093 100644 > --- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S > +++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S > @@ -226,8 +226,7 @@ ENTRY(lowlevel_init) > #endif /* NAND Boot */ > mov lr, ip /* restore link reg */ > ldr ip, [sp] /* restore save ip */ > - /* tail-call s_init to setup pll, mux, memory */ > - b s_init > + mov pc, lr > > ENDPROC(lowlevel_init) > > diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S > index a5bffb8..0f8d9f5 100644 > --- a/arch/arm/lib/crt0.S > +++ b/arch/arm/lib/crt0.S > @@ -85,8 +85,13 @@ ENTRY(_main) > bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ Actually, this... > sub sp, #GD_SIZE /* allocate one GD above SP */ > bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ ... should also be moved under the #else clause below -- no need to eat up valuable stack space when GD is in gdata. > - mov r8, sp /* GD is above SP */ Also, this... > mov r0, #0 ... should remain just before the call to board_init_f which needs it. Otherwise you run the (non-null) risk that r0 be non-zero on return from s_init and then this non-zero value be passed to board_init_f. > +#if defined(CONFIG_SPL_BUILD) > + ldr r8, =gdata /* SPL assigns r8 directly to &gdata */ > + bl s_init /* s_init() needs GD to be setup */ > +#else > + mov r8, sp /* GD is above SP */ > +#endif > bl board_init_f > > #if ! defined(CONFIG_SPL_BUILD) Amicalement,
diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c index b72fadc..8f41dcd 100644 --- a/arch/arm/cpu/armv7/omap3/board.c +++ b/arch/arm/cpu/armv7/omap3/board.c @@ -256,8 +256,6 @@ void s_init(void) #endif #ifdef CONFIG_SPL_BUILD - gd = &gdata; - preloader_console_init(); timer_init(); diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S b/arch/arm/cpu/armv7/omap3/lowlevel_init.S index eacfef8..8539093 100644 --- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S +++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S @@ -226,8 +226,7 @@ ENTRY(lowlevel_init) #endif /* NAND Boot */ mov lr, ip /* restore link reg */ ldr ip, [sp] /* restore save ip */ - /* tail-call s_init to setup pll, mux, memory */ - b s_init + mov pc, lr ENDPROC(lowlevel_init) diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index a5bffb8..0f8d9f5 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -85,8 +85,13 @@ ENTRY(_main) bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ sub sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ - mov r8, sp /* GD is above SP */ mov r0, #0 +#if defined(CONFIG_SPL_BUILD) + ldr r8, =gdata /* SPL assigns r8 directly to &gdata */ + bl s_init /* s_init() needs GD to be setup */ +#else + mov r8, sp /* GD is above SP */ +#endif bl board_init_f #if ! defined(CONFIG_SPL_BUILD)
Fix a problem with a re-assignment of r8 in the SPL version. This patch now moves the call to s_init() to a later stage, right before calling board_init_f(). And makes sure that r8 is correctly initialized before s_init() is called. r8 now is only written in crt0.S. This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530). Signed-off-by: Stefan Roese <sr@denx.de> Cc: Tom Rini <trini@ti.com> Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> --- v2: - Change handling/initializing of r8 as suggested by Albert. It should only be written in crt0.S. Tom, while working on this version one question came up: Is lowlevel_init() (file arch/arm/cpu/armv7/omap3/lowlevel_init.S) needed any more? It calls cpy_clk_code() to copy some clk init code into SRAM. But I fail to see if and where this code is really executed from SRAM. Maybe I missed something. Perhaps you could shed some light into this. Thanks, Stefan arch/arm/cpu/armv7/omap3/board.c | 2 -- arch/arm/cpu/armv7/omap3/lowlevel_init.S | 3 +-- arch/arm/lib/crt0.S | 7 ++++++- 3 files changed, 7 insertions(+), 5 deletions(-)