[U-Boot] [PATCH v2] arm: spl: Allow board_init_r() to run with a larger stack

[Tom Rini]

On Mon, Feb 02, 2015 at 12:30:59PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On 2 February 2015 at 11:57, Tom Rini <trini at ti.com> wrote:
> > On Mon, Feb 02, 2015 at 11:16:16AM -0700, Simon Glass wrote:
> >> Hi Albert,
> >>
> >> On 31 January 2015 at 15:36, Albert ARIBAUD <albert.u.boot at aribaud.net> wrote:
> >> > Hello Simon,
> >> >
> >> > On Mon, 26 Jan 2015 08:34:21 -0700, Simon Glass <sjg at chromium.org>
> >> > wrote:
> >> >> At present SPL uses a single stack, either CONFIG_SPL_STACK or
> >> >> CONFIG_SYS_INIT_SP_ADDR. Since some SPL features (such as MMC and
> >> >> environment) require a lot of stack, some boards set CONFIG_SPL_STACK to
> >> >> point into SDRAM. They then set up SDRAM very early, before board_init_f(),
> >> >> so that the larger stack can be used.
> >> >>
> >> >> This is an abuse of lowlevel_init(). That function should only be used for
> >> >> essential start-up code which cannot be delayed. An example of a valid use is
> >> >> when only part of the SPL code is visible/executable, and the SoC must be set
> >> >> up so that board_init_f() can be reached. It should not be used for SDRAM
> >> >> init, console init, etc.
> >> >>
> >> >> Add a CONFIG_SPL_STACK_R option, which allows the stack to be moved to a new
> >> >> address before board_init_r() is called in SPL.
> >> >>
> >> >> The expected SPL flow (for CONFIG_SPL_FRAMEWORK) is now:
> >> >>
> >> >> Execution starts with start.S. Two main functions can be provided by the
> >> >> board implementation. The purpose and limitations of each is described below.
> >> >> After that, the common board_init_r() is called to perform the SPL task.
> >> >>
> >> >> lowlevel_init():
> >> >>       - purpose: essential init to permit execution to reach board_init_f()
> >> >>       - no global_data, but there is a stack
> >> >
> >> > lowlevel_init() does not have a stack for all ARM CPUs, e.g. for
> >> > arm926ejs it has none; and I don't see any code in this patch that
> >> > provides one. If we need lowlevel_init to have a stack, then this stack
> >> > should be set up in crt0.S and lowlevel_init called from there (which
> >> > I would be fine with, btw).
> >
> > Keep in mind that today in crt0.S we basically setup stack and call
> > board_init_f().  I'm not sure, but maybe it would be, useful to have a
> > pre-board_init_f hook?  But maybe it's more globally useful rather than
> > an ARM thing.
> 
> I feel it would be nice to do this from crt0.S rather than start.S,
> since it is common code. Any time we have funny things in individual
> start.S files it creates tension.

Well, what I'm thinking and kind of recalling is that the normal flow
(not just on ARM) is ... stuff, _main which sets up for board_init_f().
What we don't quite have is a consistent first function in the
board_init_f func table.  Why not add one there?

> >> OK, so I should adjust this comment I think. Is that the only action?
> >>
> >>        - no global_data, but may be a stack (e.g. for armv7)
> >
> > Looking at all of this code recently, for trying to consolidate things,
> > I think we need to figure out the right answer for how to deal with
> > things like perhaps FEL too and be clear about what we do and don't
> > want.  The pre-armv7 arches seem rather consistent but armv7 went off on
> > its own thing for a while and is slowly cycling back now.
> >
> > I'm not sure, but what to do with FEL will help guide us, that we might
> > be able to say lowlevel_init does not have stack, even on armv7.  If we
> > can shove a hook early into board_init_r() and can delay those things
> > that we do early today.
> 
> Declaring that lowlevel_init() may not have a stack would be one way
> to stop people adding more and more junk there :-)

Right.  And it feels like it might be doable too.

> I am going to take a close look at FEL. I feel that FEL is a good test
> of the flexibility of the early phase: start.S, lowlevel_init /
> board_init_f() - particularly for SPL.

Yeah, I really want to see what we need to do to make FEL work and fit
well too.

-- 
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/20150202/0c4935c3/attachment.sig>