[U-Boot] [PATCH v4 04/13] arm: Add start_call_board_init_r() to each start.S

[Simon Glass]

Hi Albert,

On Tue, Feb 21, 2012 at 12:02 PM, Simon Glass <sjg at chromium.org> wrote:
> Hi Albert,
>
> On Tue, Feb 21, 2012 at 11:32 AM, Albert ARIBAUD
> <albert.u.boot at aribaud.net> wrote:
>> Hi Simon,
>>
>> Le 21/02/2012 02:32, Simon Glass a écrit :
>>
>>> We don't want this in a common file, or at least not yet, so add
>>> this function to every start.S individually. The existing code tacked
>>> on the end of a long relocation function and does not suit our needs
>>> since it doesn't allow the address of board_init_r() to be passed in
>>> and cannot be called from C since it expects values in registers r4
>>> and above.
>>
>>
>> It is not really an addition, as every start.S file involved already has
>> this sequence at the end of its relocate_code routine.
>>
>>
>>> Signed-off-by: Simon Glass<sjg at chromium.org>
>>> ---
>>> Changes in v4:
>>> - Put start_call_board_init_r() into each start.S, sadly
>>> - Split out start_call_board_init_r() addition into new patch
>>>
>>>  arch/arm/cpu/arm1136/start.S   |   19 +++++++++++++++++++
>>>  arch/arm/cpu/arm1176/start.S   |   19 +++++++++++++++++++
>>>  arch/arm/cpu/arm720t/start.S   |   19 +++++++++++++++++++
>>>  arch/arm/cpu/arm920t/start.S   |   19 +++++++++++++++++++
>>>  arch/arm/cpu/arm925t/start.S   |   19 +++++++++++++++++++
>>>  arch/arm/cpu/arm926ejs/start.S |   19 +++++++++++++++++++
>>>  arch/arm/cpu/arm946es/start.S  |   19 +++++++++++++++++++
>>>  arch/arm/cpu/arm_intcm/start.S |   19 +++++++++++++++++++
>>>  arch/arm/cpu/armv7/start.S     |   19 +++++++++++++++++++
>>>  arch/arm/cpu/ixp/start.S       |   19 +++++++++++++++++++
>>>  arch/arm/cpu/lh7a40x/start.S   |   19 +++++++++++++++++++
>>>  arch/arm/cpu/pxa/start.S       |   20 ++++++++++++++++++++
>>>  arch/arm/cpu/s3c44b0/start.S   |   19 +++++++++++++++++++
>>>  arch/arm/cpu/sa1100/start.S    |   19 +++++++++++++++++++
>>>  14 files changed, 267 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S
>>> index 28e059e..e74d5f9 100644
>>> --- a/arch/arm/cpu/arm1136/start.S
>>> +++ b/arch/arm/cpu/arm1136/start.S
>>> @@ -295,6 +295,25 @@ _board_init_r_ofs:
>>>        .word board_init_r - _start
>>>  #endif
>>>
>>> +/**
>>
>>
>> Do we agree to Doxygen annotations in U-Boot?
>
> I hope so, it makes things easy to read for those with editors that support it.
>
>>
>>
>>> + * Jump to board_init_r with a new stack pointer
>>> + *
>>> + * @param gd   Pointer to global data
>>> + * @param dest_addr    Destination address from global data
>>> + * @param func Address of board_init_r function (relocated)
>>> + * @param sp   New stack pointer
>>> + */
>>> +.globl start_call_board_init_r
>>> +start_call_board_init_r:
>>
>>
>> Why "start_call_board_init_r"? "start_" may mislead readers into thinking
>> we're starting something there and finishing it elsewhere, and
>> "call_board_init_r" doesn't tell we're switching stacks etc.
>>
>> I suggest "pivot_to_board_init_r", in analogy with Linux 'pivot_root'. Other
>> suggestions are welcome.
>
> OK I will go with pivot_to_board_init_r.

I have updated two of the patches to v5 to deal with this change.

>
>>
>> Better yet, we could avoid this new function altogether and make start.S
>> much cleaner by rewriting the startup sequence as:
>>
>> 1. start.S sets up the initial stack
>> 2. start.S calls C function board_init_f
>> 3. board_init_f allocates initial GD and stores final stack, final GD,
>>   and final base address within initial GD, then returns normally
>> 4. start.S sets up the final stack and final GD
>> 4. start.S calls C function relocate_code using relevant GD fields
>>   as its arguments
>> 5. relocate_code returns normally
>> 6. start.S calls either nand_boot or board_init_r()
>>
>> This way all the logic would be in start.S and board_init_f(),
>> relocate_code() and board_init_r() would be simple and independent
>> functions.
>
> Please see the generic relocation effort which I think is a better
> place to put this. If you like I can revisit this once we have that
> sorted out.

Sorry I meant the generic board init series. Once we have generic
board init and generic relocation then everything becomes common. Then
it might be easier to change the function signature of board_init_f()
or board_init_r() if we want to.

One more point - as Graeme points out it would be nice to turn on the
dcache earlier. Another thing to look at under generic board init I
think.

>
>>

Regards,
Simon