Hi Tom,
On 21 December 2014 at 05:30, Tom Rini trini@ti.com wrote:
On Fri, Dec 19, 2014 at 08:32:35AM -0700, Simon Glass wrote:
Hi Tom, Albert,
On 19 December 2014 at 07:40, Tom Rini trini@ti.com wrote:
On Thu, Dec 18, 2014 at 05:21:21PM -0700, Simon Glass wrote:
This is an attempt to tidy up the early SPL code in an attempt to pave the way for driver model in SPL:
- Avoid setting up SDRAM before board_init_f()
- Avoid touching global_data before board_init_f()
- Allow board_init_f() to set up a new stack (seems that the SRAM stack
is not large enough on these boards)
This needs more work but it does boot on Beaglebone Black.
Signed-off-by: Simon Glass sjg@chromium.org
arch/arm/cpu/armv7/am33xx/board.c | 60 ++++++++++++++++++++++++++------------ arch/arm/cpu/armv7/lowlevel_init.S | 4 --- arch/arm/include/asm/spl.h | 3 ++ arch/arm/lib/crt0.S | 9 ++++++ include/configs/ti_armv7_common.h | 5 ++-- 5 files changed, 56 insertions(+), 25 deletions(-)
This takes things in the wrong direction I think. Since omap3/4/5 have the same problem we're going to have to duplicate a bunch of this code. But we can do omap_save_boot_params a bit later I'm pretty sure we can shove it into spl_board_init() in arch/arm/cpu/armv7/omap-common/boot-common.c and I'm going to do my best to do that today and test it on at least a few boards.
I don't have a lot of background on SPL stuff as I only know one implementation in detail. So these comments may be a bit off.
There seem to be two drivers causing this oddness:
- The need to save boot params before global_data is available. I
wonder if it is possible to avoid overwriting the boot params, and save them later, in board_init_f()? If not, then I don't think the global_data structure should be used. A static local variable in the data section, with just a few fields in it, could be used instead. That avoids the temptation to thing that we are creating a global_data structure before crt0.S does it officially. If the data had just been stored into the data section, without messing with global_data, then I don't think we would have this problem.
This is actually covered today, really. We do "save_boot_params" first thing and that function must save a magic register value from ROM (or whatever) to a "stable" magic location in memory.
Ah OK. So long as it doesn't save to global_data we are fine. It could just have a little structure somewhere in the data segment.
- Need for more stack that can be fitted into SRAM. I think the only
sensible option here is to change the stack after board_init_f(). As Albert says this should be done in crt0.S (in fact that's where I put my code). Forcing the dram init to before board_init_f() in SPL seems broken to me.
This is also covered. The flow is cpu_init_crit -> lowlevel_init (set stack to CONFIG_SYS_INIT_SP_ADDR) -> s_init (init DDR) -> _main (set stack to CONFIG_SPL_STACK, can be in DDR).
Well s_init is a board-specific thing. What I mean is that crt0.S should set up an initial stack - there should be no stack before that. Then board_init_f() uses the initial stack, sets up DRAM, then we move to a second larger stack before board_init_r() is called.
I think we should try to have the same flow as U-Boot proper:
start.S lowlevel_init (no stack, no global_data, no dram) - can only use 'data' section to write stuff crt0.S (sets up stack and global_data, no dram) board_init_f (sets up dram) relocate stack if required (but not code) board_init_r (running with full stack in dram)
I suppose since old school PowerPC needed i2c to read spd eeproms it's possible this ordering could work still today where there's a good bit of stuff neeed in order to bring DDR on some ARM platforms.
(Note I am talking just about SPL here, as above)
I suppose what I am saying is that there is no point in setting up DRAM before board_init_f() in SPL. There is no global_data available, nor is there any console, so any code that happens before board_init_f() should be absolutely essential for getting to board_init_f(). I can't think of much that fits in that category, maybe some clock things?
Regards, Simon