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

[Tom Rini]

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.

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

-- 
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/80998132/attachment.pgp>