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

[Simon Glass]

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.

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

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.

Regards,
Simon