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

[Heiko Schocher]

Hello Simon,

Am 19.01.2015 07:46, schrieb 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(-)
[...]
>> 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!

Tested on an sam9260 based board (not mainlined yet) with 4k sram
for SPL and relocating stack to RAM with CONFIG_SPL_STACK_R

:-D

Acked-by: Heiko Schocher <hs at denx.de>
Tested-by: Heiko Schocher <hs at denx.de>

Thanks!

bye,
Heiko
BTW: before your patch SPL size was 3392 bytes, with your patch
size shrunk to 3227 bytes :-)
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany