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

[Heiko Schocher]

Hello Simon,

added Bo Shen to cc, as he currently try to set BSS (and stack) into
SDRAM for at91 based boards ... Bo, could you try this aproach?

Am 18.01.2015 19:55, schrieb Simon Glass:
> 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 now:
>
> Execution starts with start.S. Two main functions can be provided by the
> board implementation. The purpose and limitations of each is described below.
> After that, the common board_init_r() is called to perform the SPL task.
>
> lowlevel_init():
> 	- purpose: essential init to permit execution to reach board_init_f()
> 	- no global_data, but there is a stack
> 	- must not set up SDRAM or use console
> 	- must only do the bare minimum to allow execution to continue to
> 		board_init_f()
> 	- this is almost never needed
> 	- return normally from this function
>
> board_init_f():
> 	- purpose: set up the machine ready for running board_init_r():
> 		i.e. SDRAM and serial UART
> 	- global_data is available
> 	- preloader_console_init() can be called here in extremis
> 	- stack is in SRAM
> 	- should set up SDRAM, and anything needed to make the UART work
> 	- these is no need to clear BSS, it will be done by crt0.S
> 	- must return normally from this function (don't call board_init_r()
> 		directly)
>
> Here the BSS is cleared. If CONFIG_SPL_STACK_R is defined, then at this point
> the stack and global_data are relocated to below that address.
>
> board_init_r():
> 	- purpose: main execution, common code
> 	- global_data is available
> 	- SDRAM is available
> 	- stack is optionally in SDRAM, if CONFIG_SPL_STACK_R is defined and
> 		points into SDRAM
> 	- preloader_console_init() can be called here - typically this is
> 		done by defining CONFIG_SPL_BOARD_INIT and then supplying a
> 		spl_board_init() function containing this call
> 	- loads U-Boot or (in falcon mode) Linux
>
> Note: This patch is intended to apply over the top of Tom's SPL changes and

Sound good! Can you point me, which patches you mean? Thanks!

> this series:
>
> https://patchwork.ozlabs.org/patch/423785/
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
>   arch/arm/lib/crt0.S | 13 ++++++++++---
>   common/spl/spl.c    | 35 +++++++++++++++++++++++++++++++++++
>   doc/README.SPL      | 42 ++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 87 insertions(+), 3 deletions(-)

Hmm... I miss here "arch/arm/lib/spl.c" where "clearing BSS"
should get removed ... or?

> 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 1826c47..78bb279 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -276,3 +276,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
> +}

I try this patch out on a sam9260 board with 4k sram for SPL (if I have the
hw) ... looking into my current objdump with the approach from Bo,
see patch [1], I do not find "memcpy" in it ... I currently have a
SPL size (untested, compiled code only) currently from 3392 bytes, which
perfectly fits in 4k ... adding a memcpy in code, I get a size of 3464 bytes
fits also in 4k ... with space for some init stack in sram  ... so, I try
your patch ASAP, thanks!

bye,
Heiko
[1] http://patchwork.ozlabs.org/patch/429667/

> diff --git a/doc/README.SPL b/doc/README.SPL
> index 3ba313c..327d3e2 100644
> --- a/doc/README.SPL
> +++ b/doc/README.SPL
> @@ -95,3 +95,45 @@ cflow will spit out a number of warnings as it does not parse
>   the config files and picks functions based on #ifdef.  Parsing the '.i'
>   files instead introduces another set of headaches.  These warnings are
>   not usually important to understanding the flow, however.
> +
> +
> +ARM SPL Control Flow
> +--------------------
> +
> +Execution starts with start.S. Two main functions can be provided by the
> +board implementation. The purpose and limitations of each is described below.
> +After that, the common board_init_r() is called to perform the SPL task.
> +
> +lowlevel_init():
> +	- purpose: essential init to permit execution to reach board_init_f()
> +	- no global_data, but there is a stack
> +	- must not set up SDRAM or use console
> +	- must only do the bare minimum to allow execution to continue to
> +		board_init_f()
> +	- this is almost never needed
> +	- return normally from this function
> +
> +board_init_f():
> +	- purpose: set up the machine ready for running board_init_r():
> +		i.e. SDRAM and serial UART
> +	- global_data is available
> +	- preloader_console_init() can be called here in extremis
> +	- stack is in SRAM
> +	- should set up SDRAM, and anything needed to make the UART work
> +	- these is no need to clear BSS, it will be done by crt0.S
> +	- must return normally from this function (don't call board_init_r()
> +		directly)
> +
> +Here the BSS is cleared. If CONFIG_SPL_STACK_R is defined, then at this point
> +the stack and global_data are relocated to below that address.
> +
> +board_init_r():
> +	- purpose: main execution, common code
> +	- global_data is available
> +	- SDRAM is available
> +	- stack is optionally in SDRAM, if CONFIG_SPL_STACK_R is defined and
> +		points into SDRAM
> +	- preloader_console_init() can be called here - typically this is
> +		done by defining CONFIG_SPL_BOARD_INIT and then supplying a
> +		spl_board_init() function containing this call
> +	- loads U-Boot or (in falcon mode) Linux
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany