[U-Boot] [PATCH v5 12/23] Add spl load feature

[Simon Glass]

+Albert

Hi Scott,

On Tue, Feb 12, 2013 at 12:29 PM, Scott Wood <scottwood at freescale.com> wrote:
> On 02/08/2013 09:12:08 AM, Simon Glass wrote:
>>
>> This adds secondary program loader support to the generic board.
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>> Changes in v5: None
>> Changes in v4: None
>> Changes in v3: None
>> Changes in v2: None
>>
>>  common/board_f.c | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/common/board_f.c b/common/board_f.c
>> index aa10f4b..3a8036f 100644
>> --- a/common/board_f.c
>> +++ b/common/board_f.c
>> @@ -309,6 +309,7 @@ static int reserve_uboot(void)
>>         return 0;
>>  }
>>
>> +#ifndef CONFIG_SPL_BUILD
>>  /* reserve memory for malloc() area */
>>  static int reserve_malloc(void)
>>  {
>> @@ -328,6 +329,7 @@ static int reserve_board(void)
>>                         sizeof(bd_t), gd->dest_addr_sp);
>>         return 0;
>>  }
>> +#endif
>>
>>  static int setup_machine(void)
>>  {
>> @@ -365,6 +367,7 @@ static int reserve_fdt(void)
>>         return 0;
>>  }
>>
>> +#ifndef CONFIG_SPL_BUILD
>>  static int reserve_stacks(void)
>>  {
>>         /* setup stack pointer for exceptions */
>> @@ -384,6 +387,17 @@ static int reserve_stacks(void)
>>
>>         return 0;
>>  }
>> +#endif
>> +
>> +#ifdef CONFIG_SPL_BUILD
>> +static int reserve_stacks_spl(void)
>> +{
>> +       /* Why not -= ? */
>> +       gd->dest_addr_sp += 128;        /* leave 32 words for abort-stack
>> */
>> +       gd->irq_sp = gd->dest_addr_sp;
>> +       return 0;
>> +}
>> +#endif
>
>
> "abort-stack" doesn't sound very generic, and that "why not" question should
> probably be answered.

I'm not sure what you mean by the first comment. For the second, I
agree. It looks wrong to me. I copied Albert who might know.

>
>
>>  static int display_new_sp(void)
>>  {
>> @@ -524,12 +538,18 @@ static init_fnc_t init_sequence_f[] = {
>>         reserve_lcd,
>>  #endif
>>         reserve_uboot,
>> +#ifndef CONFIG_SPL_BUILD
>>         reserve_malloc,
>>         reserve_board,
>> +#endif
>>         setup_machine,
>>         reserve_global_data,
>>         reserve_fdt,
>> +#ifdef CONFIG_SPL_BUILD
>> +       reserve_stacks_spl,
>> +#else
>>         reserve_stacks,
>> +#endif
>
>
> Why not just put the ifdef inside reserve_stacks()?  Or at least give them
> the same name?

OK I will do that.

>
> -Scott

Regards,
Simon