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

[Tom Rini <trini@ti.com>]

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

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

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

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141221/d2a0e173/attachment.pgp>