[U-Boot] [PATCH] RFC: am35xx: Rearrange SPL on am35xx

[Simon Glass]

Hi Tom,

On 21 December 2014 at 12:29, Tom Rini <trini at ti.com> wrote:
> On Sun, Dec 21, 2014 at 11:53:21AM -0700, Simon Glass wrote:
>> Hi Tom,
>>
>> On 21 December 2014 at 05:30, Tom Rini <trini at 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 at 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 at 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:
>> >>
>> >> 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.
>>
>> 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.
>>
>> >
>> >> 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).
>>
>> 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.

Yes I've had a bit of a look around. I feel that s_init() should be
for very limited purposes and is being abused. There is a tendency for
everything to just keep moving earlier!

>
>> >> 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?
>
> Clock and pinmux.  It could work...

Now that you have omap sorted there are only a few that use gdata - I
think iMX, sunxi and zynq. I'm having a bit of a look at sunxi.

Regards,
Simon