Hi Simon,
On Sun, 18 Jan 2015 11:55:36 -0700 Simon Glass sjg@chromium.org 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 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 this series:
I still have an opinion that global_data itself is a nightmare rather than a useful stuff.
@@ -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
It is not clear to me why this change is related to a larger stack.
}
+/**
- 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
I guess CONFIG_SPL_STACK_R has the type, "hex", not "bool".
In terms of Kconfig's way, the "ifdef CONFIG_SPL_STACK_R" is the abuse of the #ifdef conditonal.
When Kconfig was introduced, I might have mentioned "Add a new CONFIG to Kconfig rather than headers when you introduce a new feature and document the usage in Kconfig".
Most of people do not stick to that, so I think you can excuse here.
I assume you (or somebody else) will implement it correctly when it is moved to Kconfig. (I should be easy.)
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 +--------------------
Is this flow ARM-specific? It looks like the following description is very generic althogh I do not familiar with the other architectures..
+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
I do not mean I object against this patch. From here, this is beyond the scope of this patch. Just comments about the current SPL boot flow that, I think, is not very nice.
One of the most important tasks of a boot loader is DRAM initialization. This is sometimes problematic, so the printf debug (or any other early UART feature) would be really helpful. If DRAM init fails, I want some error messages on the console rather than the silent die.
In the main U-Boot boot flow (common/board_f.c), the initcalls are invoked in this order: initf_dm, serial_init, console_init_f, dram_init
It is nice the UART is available in dram_init().
On the other hand, what is happening in SPL is like this: [1] You are introducing CONFIG_SPL_DM [2] I assume the legacy drivers will be all dropped including UART of SPL [3] The DM scan (dm_init_and_scan) is called in board_init_r() [4] As you mentioned above in the README, DRAM should be setup in board_init_f()
Both [3] and [4] together make it difficult the UART debug of dram_init().
I guess [3] is just a temporary workaround in order to introduce DM into SPL and we will have to re-design the SPL boot flow someday.
Perhaps what will happen next might be to reuse common/board_f.c for SPL. (i.e. Generic Board for SPL, CONFIG_SPL_GENERIC_BOARD?)
Roughly, what we want to do in SPL is all included in common/board_f.c
Moreover, if SPL is enabled, we can skip common/board_f.c in the main U-boot image. Most of the initializations have already been done in SPL. We do not have to do board_init_f() twice.
Best Regards Masahiro Yamada