[U-Boot] [RFC PATCH 2/2] x86: fsp: Move FspInitEntry call to board_init_f()
Andrew Bradford
andrew at bradfordembedded.com
Wed Jun 3 14:59:27 CEST 2015
On 06/03 07:17, Bin Meng wrote:
> Hi Andrew,
>
> On Wed, Jun 3, 2015 at 4:05 AM, Andrew Bradford
> <andrew at bradfordembedded.com> wrote:
> > Hi Bin,
> >
> > On 06/01 20:31, Bin Meng wrote:
> >> The call to FspInitEntry is done in arch/x86/lib/fsp/fsp_car.S so far.
> >> It worked pretty well but looks not that good. Apart from doing too
> >> much work than just enabling CAR, it cannot read the configuration
> >> data from device tree at that time. Now we want to move it a little
> >> bit later as part of init_sequence_f[] being called by board_init_f().
> >> This way it looks and works better in the U-Boot initialization path.
> >>
> >> Due to FSP's design, after calling FspInitEntry it will not return to
> >> its caller, instead it jumps to a continuation function which is given
> >> by bootloader with a new stack in system memory. The original stack in
> >> the CAR is gone, but its content is perserved by FSP and described by
> >> a bootloader temporary memory HOB. Technically we can recover anything
> >> we had before in the previous stack, but that is way too complicated.
> >> To make life much easier, in the FSP continuation routine we just
> >> simply call fsp_init_done() and jump back to car_init_ret() to redo
> >> the whole board_init_f() initialization, but this time with a non-zero
> >> HOB list pointer saved in U-Boot's global data so that we can bypass
> >> the FspInitEntry for the second time.
> >>
> >> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> >>
> >> ---
> >>
> >> arch/x86/cpu/start.S | 6 +++++-
> >> arch/x86/include/asm/u-boot-x86.h | 4 ++++
> >> arch/x86/lib/fsp/fsp_car.S | 26 +++++---------------------
> >> arch/x86/lib/fsp/fsp_common.c | 8 ++++++++
> >> common/board_f.c | 3 +++
> >> 5 files changed, 25 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/arch/x86/cpu/start.S b/arch/x86/cpu/start.S
> >> index 2e5f9da..00e585e 100644
> >> --- a/arch/x86/cpu/start.S
> >> +++ b/arch/x86/cpu/start.S
> >> @@ -116,12 +116,16 @@ car_init_ret:
> >> rep stosb
> >>
> >> #ifdef CONFIG_HAVE_FSP
> >> + test %esi, %esi
> >> + jz skip_hob
> >> +
> >> /* Store HOB list */
> >> movl %esp, %edx
> >> addl $GD_HOB_LIST, %edx
> >> movl %esi, (%edx)
> >> -#endif
> >>
> >> +skip_hob:
> >> +#endif
> >> /* Setup first parameter to setup_gdt, pointer to global_data */
> >> movl %esp, %eax
> >>
> >> diff --git a/arch/x86/include/asm/u-boot-x86.h b/arch/x86/include/asm/u-boot-x86.h
> >> index faa5182..9ee4104 100644
> >> --- a/arch/x86/include/asm/u-boot-x86.h
> >> +++ b/arch/x86/include/asm/u-boot-x86.h
> >> @@ -54,6 +54,10 @@ u32 isa_map_rom(u32 bus_addr, int size);
> >> /* arch/x86/lib/... */
> >> int video_bios_init(void);
> >>
> >> +#ifdef CONFIG_HAVE_FSP
> >> +int x86_fsp_init(void);
> >> +#endif
> >> +
> >> void board_init_f_r_trampoline(ulong) __attribute__ ((noreturn));
> >> void board_init_f_r(void) __attribute__ ((noreturn));
> >>
> >> diff --git a/arch/x86/lib/fsp/fsp_car.S b/arch/x86/lib/fsp/fsp_car.S
> >> index 5e09568..afbf3f9 100644
> >> --- a/arch/x86/lib/fsp/fsp_car.S
> >> +++ b/arch/x86/lib/fsp/fsp_car.S
> >> @@ -56,28 +56,10 @@ temp_ram_init_ret:
> >>
> >> /* stack grows down from top of CAR */
> >> movl %edx, %esp
> >> + subl $4, %esp
> >>
> >> - /*
> >> - * TODO:
> >> - *
> >> - * According to FSP architecture spec, the fsp_init() will not return
> >> - * to its caller, instead it requires the bootloader to provide a
> >> - * so-called continuation function to pass into the FSP as a parameter
> >> - * of fsp_init, and fsp_init() will call that continuation function
> >> - * directly.
> >> - *
> >> - * The call to fsp_init() may need to be moved out of the car_init()
> >> - * to cpu_init_f() with the help of some inline assembly codes.
> >> - * Note there is another issue that fsp_init() will setup another stack
> >> - * using the fsp_init parameter stack_top after DRAM is initialized,
> >> - * which means any data on the previous stack (on the CAR) gets lost
> >> - * (ie: U-Boot global_data). FSP is supposed to support such scenario,
> >> - * however it does not work. This should be revisited in the future.
> >> - */
> >> - movl $CONFIG_FSP_TEMP_RAM_ADDR, %eax
> >> - xorl %edx, %edx
> >> - xorl %ecx, %ecx
> >> - call fsp_init
> >> + xor %esi, %esi
> >> + jmp car_init_done
> >>
> >> .global fsp_init_done
> >> fsp_init_done:
> >> @@ -86,6 +68,8 @@ fsp_init_done:
> >> * Save eax to esi temporarily.
> >> */
> >> movl %eax, %esi
> >> +
> >> +car_init_done:
> >> /*
> >> * Re-initialize the ebp (BIST) to zero, as we already reach here
> >> * which means we passed BIST testing before.
> >> diff --git a/arch/x86/lib/fsp/fsp_common.c b/arch/x86/lib/fsp/fsp_common.c
> >> index 001494d..5b25632 100644
> >> --- a/arch/x86/lib/fsp/fsp_common.c
> >> +++ b/arch/x86/lib/fsp/fsp_common.c
> >> @@ -46,3 +46,11 @@ void board_final_cleanup(void)
> >>
> >> return;
> >> }
> >> +
> >> +int x86_fsp_init(void)
> >> +{
> >> + if (!gd->arch.hob_list)
> >> + fsp_init(CONFIG_FSP_TEMP_RAM_ADDR, BOOT_FULL_CONFIG, NULL);
> >> +
> >> + return 0;
> >> +}
> >> diff --git a/common/board_f.c b/common/board_f.c
> >> index fbbad1b..21be26f 100644
> >> --- a/common/board_f.c
> >> +++ b/common/board_f.c
> >> @@ -753,6 +753,9 @@ static init_fnc_t init_sequence_f[] = {
> >> #ifdef CONFIG_OF_CONTROL
> >> fdtdec_setup,
> >> #endif
> >> +#if defined(CONFIG_X86) && defined(CONFIG_HAVE_FSP)
> >> + x86_fsp_init,
> >> +#endif
> >> #ifdef CONFIG_TRACE
> >> trace_early_init,
> >> #endif
> >
> > Overall, I like the idea of these two patches!
> >
> > In my quick testing on my Valley Island, I now appear to have access to
> > the gd and device tree blob prior to calling the fsp to setup SDRAM.
>
> Yes, that's expected. Besides the device tree access, are you able to
> boot the Valley Island successfully with these two patches?
Yes! :)
I'm able to boot and to extract data from the device tree prior to
calling into the FSP. So with a patchset like this one that you've
proposed I could then implement moving the fsp_config vpd data into
device tree to much more easily support additional baytrail boards.
Tested-by: Andrew Bradford <andrew.bradford at kodakalaris.com>
Thanks!
-Andrew
More information about the U-Boot
mailing list