[U-Boot] [PATCH v6 4/6] arm: spl: Allow board_init_r() to run with a larger stack

Tom Rini trini at konsulko.com
Wed Mar 4 16:10:21 CET 2015


On Wed, Mar 04, 2015 at 08:38:33AM -0500, Tom Rini wrote:
> On Tue, Mar 03, 2015 at 08:23:23PM -0700, Simon Glass wrote:
> > Hi Tom,
> > 
> > On 3 March 2015 at 13:14, Tom Rini <trini at konsulko.com> wrote:
> > > On Tue, Mar 03, 2015 at 12:04:16PM -0700, Simon Glass wrote:
> > >> Hi Tom,
> > >>
> > >> On 3 March 2015 at 10:49, Tom Rini <trini at konsulko.com> wrote:
> > >> > On Tue, Mar 03, 2015 at 08:03:00AM -0700, Simon Glass wrote:
> > >> >
> > >> >> At present SPL uses a single stack, either CONFIG_SPL_STACK or
> > >> >> CONFIG_SYS_INIT_SP_ADDR. Since some SPL features (such as MMC and
> > >> >> environment) require a lot of stack, some boards set CONFIG_SPL_STACK to
> > >> >> point into SDRAM. They then set up SDRAM very early, before board_init_f(),
> > >> >> so that the larger stack can be used.
> > >> >>
> > >> >> This is an abuse of lowlevel_init(). That function should only be used for
> > >> >> essential start-up code which cannot be delayed. An example of a valid use is
> > >> >> when only part of the SPL code is visible/executable, and the SoC must be set
> > >> >> up so that board_init_f() can be reached. It should not be used for SDRAM
> > >> >> init, console init, etc.
> > >> >>
> > >> >> Add a CONFIG_SPL_STACK_R option, which allows the stack to be moved to a new
> > >> >> address before board_init_r() is called in SPL.
> > >> >>
> > >> >> The expected SPL flow (for CONFIG_SPL_FRAMEWORK) is documented in the README.
> > >> >>
> > >> >> Signed-off-by: Simon Glass <sjg at chromium.org>
> > >> >> For version 1:
> > >> >> Acked-by: Albert ARIBAUD <albert.u.boot at aribaud.net>
> > >> >> Reviewed-by: Stefan Roese <sr at denx.de>
> > >> >> Tested-by: Bo Shen <voice.shen at atmel.com>
> > >> >> Acked-by: Bo Shen <voice.shen at atmel.com>
> > >> >> Acked-by: Heiko Schocher <hs at denx.de>
> > >> >> Tested-by: Heiko Schocher <hs at denx.de>
> > >> >> ---
> > >> >>
> > >> >> Changes in v6: None
> > >> >> Changes in v5:
> > >> >> - Rebase to master
> > >> >>
> > >> >> Changes in v4:
> > >> >> - Adjust README to mention that lowlevel_init() should have no stack
> > >> >>
> > >> >> Changes in v3: None
> > >> >> Changes in v2:
> > >> >> - Move docs to top-level README file and expand them to cover U-Boot proper
> > >> >> - Add Kconfig settings
> > >> >>
> > >> >>  Kconfig             | 18 ++++++++++++++
> > >> >>  README              | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >> >>  arch/arm/lib/crt0.S | 13 +++++++---
> > >> >>  common/spl/spl.c    | 35 +++++++++++++++++++++++++++
> > >> >>  4 files changed, 132 insertions(+), 3 deletions(-)
> > >> > [snip]
> > >> >> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
> > >> >> index 22df3e5..7939ced 100644
> > >> >> --- a/arch/arm/lib/crt0.S
> > >> >> +++ b/arch/arm/lib/crt0.S
> > >> >> @@ -113,7 +113,14 @@ here:
> > >> >>  /* Set up final (full) environment */
> > >> >>
> > >> >>       bl      c_runtime_cpu_setup     /* we still call old routine here */
> > >> >> -
> > >> >> +#endif
> > >> >> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_FRAMEWORK)
> > >> >> +# ifdef CONFIG_SPL_BUILD
> > >> >> +     /* Use a DRAM stack for the rest of SPL, if requested */
> > >> >> +     bl      spl_relocate_stack_gd
> > >> >> +     cmp     r0, #0
> > >> >> +     movne   sp, r0
> > >> >> +# endif
> > >> >>       ldr     r0, =__bss_start        /* this is auto-relocated! */
> > >> >>       ldr     r1, =__bss_end          /* this is auto-relocated! */
> > >> >>
> > >> >> @@ -124,9 +131,10 @@ clbss_l:cmp      r0, r1                  /* while not at end of BSS */
> > >> >>       addlo   r0, r0, #4              /* move to next */
> > >> >>       blo     clbss_l
> > >> >>
> > >> >> +#if ! defined(CONFIG_SPL_BUILD)
> > >> >>       bl coloured_LED_init
> > >> >>       bl red_led_on
> > >> >> -
> > >> >> +#endif
> > >> >>       /* call board_init_r(gd_t *id, ulong dest_addr) */
> > >> >>       mov     r0, r9                  /* gd_t */
> > >> >>       ldr     r1, [r9, #GD_RELOCADDR] /* dest_addr */
> > >> >> @@ -134,7 +142,6 @@ clbss_l:cmp       r0, r1                  /* while not at end of BSS */
> > >> >>       ldr     pc, =board_init_r       /* this is auto-relocated! */
> > >> >>
> > >> >>       /* we should not return here. */
> > >> >> -
> > >> >>  #endif
> > >> >>
> > >> >>  ENDPROC(_main)
> > >> >> diff --git a/common/spl/spl.c b/common/spl/spl.c
> > >> >> index ded0f30..cd75bbc 100644
> > >> >> --- a/common/spl/spl.c
> > >> >> +++ b/common/spl/spl.c
> > >> >> @@ -281,3 +281,38 @@ void preloader_console_init(void)
> > >> >>       spl_display_print();
> > >> >>  #endif
> > >> >>  }
> > >> >> +
> > >> >> +/**
> > >> >> + * spl_relocate_stack_gd() - Relocate stack ready for board_init_r() execution
> > >> >> + *
> > >> >> + * Sometimes board_init_f() runs with a stack in SRAM but we want to use SDRAM
> > >> >> + * for the main board_init_r() execution. This is typically because we need
> > >> >> + * more stack space for things like the MMC sub-system.
> > >> >> + *
> > >> >> + * This function calculates the stack position, copies the global_data into
> > >> >> + * place and returns the new stack position. The caller is responsible for
> > >> >> + * setting up the sp register.
> > >> >> + *
> > >> >> + * @return new stack location, or 0 to use the same stack
> > >> >> + */
> > >> >> +ulong spl_relocate_stack_gd(void)
> > >> >> +{
> > >> >> +#ifdef CONFIG_SPL_STACK_R
> > >> >> +     gd_t *new_gd;
> > >> >> +     ulong ptr;
> > >> >> +
> > >> >> +     /* Get stack position: use 8-byte alignment for ABI compliance */
> > >> >> +     ptr = CONFIG_SPL_STACK_R - sizeof(gd_t);
> > >> >> +     ptr &= ~7;
> > >> >> +     new_gd = (gd_t *)ptr;
> > >> >> +     memcpy(new_gd, (void *)gd, sizeof(gd_t));
> > >> >> +     gd = new_gd;
> > >> >> +
> > >> >> +     /* Clear the BSS. */
> > >> >> +     memset(__bss_start, 0, __bss_end - __bss_start);
> > >> >> +
> > >> >> +     return ptr;
> > >> >> +#else
> > >> >> +     return 0;
> > >> >> +#endif
> > >> >> +}
> > >> >
> > >> > All of this _does_ move gd into where CONFIG_SPL_STACK_R points.  It
> > >> > does _not_ move the stack itself into where CONFIG_SPL_STACK_R points so
> > >> > the big use case (am335x_boneblack for example where
> > >> > CONFIG_SPL_ENV_SUPPORT is set) doesn't work and blows up as we overflow
> > >> > the area for stack.
> > >>
> > >> OK I'll have to test more. What sort of problem should i see?
> > >
> > > We hang on entering env_reloc_spec() which is what puts some big sized
> > > variables on the stack.
> > >
> > >> The return value from spl_relocate_stack_gd() should be shoved into
> > >> sp. Does that not happen?
> > >
> > > It doesn't appear to be.  I hadn't broken out the BBB with JTAG tho.
> > 
> > I printed out the stack in board_init_r() and it looks right: 32MB
> > above the base of SDRAM.
> > 
> > Also I don't seem to get a crash in SPL. How do I make that happen? If
> > I define CONFIG_SPL_USBETH_SUPPORT I get an error about the .data
> > section not fitin in .sram for u-boot-spl.
> 
> With v5 at least, am335x_boneblack_config running on a BBB does it.
> I'll try v6 this morning and see what's going on..

With your v6 as-is, things are working.  So I'll go track down what's
hopefully some problem with what I was doing from here, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150304/569875bf/attachment.sig>


More information about the U-Boot mailing list