RFC: am35xx: Rearrange SPL on am35xx

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 <[email protected]>

---

 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(-)

Quoted 11 lines · Simon Glass

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 <[email protected]>

Quoted 8 lines

---

 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.

Hi Tom, Albert,

Quoted 22 lines · Tom Rini

Quoted 11 lines · Simon Glass

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 <[email protected]>

Quoted 8 lines

---

 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(-)

Quoted 6 lines

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:

1. 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.

2. 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.

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)

Quoted 37 lines · Simon Glass

Hi Tom, Albert,

Quoted 22 lines · Tom Rini

Quoted 11 lines · Simon Glass

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 <[email protected]>

Quoted 8 lines

---

 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(-)

Quoted 6 lines

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:

Quoted 9 lines

1. 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.

Quoted 6 lines

2. 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).

Quoted 10 lines

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.

Hi Tom,

Quoted 48 lines · Tom Rini

Quoted 37 lines · Simon Glass

Hi Tom, Albert,

Quoted 22 lines · Tom Rini

Quoted 11 lines · Simon Glass

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 <[email protected]>

Quoted 8 lines

---

 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(-)

Quoted 6 lines

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:

Quoted 9 lines

1. 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.

Quoted 3 lines

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.

Quoted 7 lines

Quoted 6 lines

2. 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.

Quoted 3 lines

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.

Quoted 10 lines

Quoted 9 lines

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)

Quoted 3 lines

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?

Quoted 65 lines · Simon Glass

Hi Tom,

Quoted 48 lines · Tom Rini

Quoted 37 lines · Simon Glass

Hi Tom, Albert,

Quoted 22 lines · Tom Rini

Quoted 11 lines · Simon Glass

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 <[email protected]>

Quoted 8 lines

---

 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(-)

Quoted 6 lines

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:

Quoted 9 lines

1. 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.

Quoted 3 lines

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.

Quoted 7 lines

Quoted 6 lines

2. 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.

Quoted 8 lines

Quoted 3 lines

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'd like to see someone re-jigger all of the code here. It could work but most armv7 SoCs have a lot of things going on in s_init and I'm not sure how much of it we can delay all that much. We can (and do) use an initial stack pointing elsewhere so maybe we could delay relocating the main stack.

Quoted 17 lines

Quoted 11 lines

Quoted 10 lines

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)

Quoted 3 lines

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)

Quoted 6 lines

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?

Clock and pinmux. It could work...