On 06/03 07:17, Bin Meng wrote: > Hi Andrew, > > On Wed, Jun 3, 2015 at 4:05 AM, Andrew Bradford > <[email protected]> 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 <[email protected]> > >> > >> --- > >> > >> 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 <[email protected]> Thanks! -Andrew _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
