| Message ID | 1360336339-10703-13-git-send-email-sjg@chromium.org |
|---|---|
| State | Deferred, archived |
| Delegated to: | Tom Rini |
| Headers | show |
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@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. > 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? -Scott
+Albert Hi Scott, On Tue, Feb 12, 2013 at 12:29 PM, Scott Wood <scottwood@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@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
Hi Scott, On Tue, Feb 12, 2013 at 2:34 PM, Scott Wood <scottwood@freescale.com> wrote: > On 02/12/2013 04:23:21 PM, Simon Glass wrote: >> >> +Albert >> >> Hi Scott, >> >> On Tue, Feb 12, 2013 at 12:29 PM, Scott Wood <scottwood@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@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. > > > It's ARM-specific terminology ("exception" is more typical than "abort" > elsewhere). Why 32 words, BTW? Why is it larger in SPL versus non-SPL? These are good questions but I don't know the answers. I guess the abort stack may need enough space to panic()? I am copying code from the various arch/xxx/board.c files in order to merge them. Regards, Simon > > -Scott
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 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 setup_baud_rate, display_new_sp, reloc_fdt,
This adds secondary program loader support to the generic board. Signed-off-by: Simon Glass <sjg@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(+)