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

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

>
> Amicalement,
> --
> Albert.

Regards,
Simon