Hi Simon, On Tue, Feb 4, 2020 at 1:15 AM Simon Glass <s...@chromium.org> wrote: > > Hi Bin, > > On Mon, 3 Feb 2020 at 10:10, Bin Meng <bmeng...@gmail.com> wrote: > > > > Hi Simon, > > > > On Tue, Feb 4, 2020 at 12:20 AM Simon Glass <s...@chromium.org> wrote: > > > > > > Hi Bin, > > > > > > On Mon, 3 Feb 2020 at 04:05, Bin Meng <bmeng...@gmail.com> wrote: > > > > > > > > Hi Simon, > > > > > > > > On Sun, Dec 22, 2019 at 12:13 AM Simon Glass <s...@chromium.org> wrote: > > > > > > > > > > When U-Boot is no the first-stage bootloader much of this code is not > > > > > needed and can break booting. Add checks for this to the FSP code. > > > > > > > > > > Rather than checking for the amount of available SDRAM, just use 1GB > > > > > in > > > > > this situation, which should be safe. Using 2GB may run into a memory > > > > > hole on some SoCs. > > > > > > > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > > > > --- > > > > > > > > > > arch/x86/lib/fsp/fsp_dram.c | 8 ++++++++ > > > > > arch/x86/lib/fsp/fsp_graphics.c | 3 +++ > > > > > arch/x86/lib/fsp2/fsp_dram.c | 10 ++++++++++ > > > > > arch/x86/lib/fsp2/fsp_init.c | 2 +- > > > > > 4 files changed, 22 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/arch/x86/lib/fsp/fsp_dram.c b/arch/x86/lib/fsp/fsp_dram.c > > > > > index 9ce0ddf0d3..15e82de2fe 100644 > > > > > --- a/arch/x86/lib/fsp/fsp_dram.c > > > > > +++ b/arch/x86/lib/fsp/fsp_dram.c > > > > > @@ -44,6 +44,14 @@ int dram_init_banksize(void) > > > > > phys_addr_t low_end; > > > > > uint bank; > > > > > > > > > > + if (!ll_boot_init()) { > > > > > + gd->bd->bi_dram[0].start = 0; > > > > > + gd->bd->bi_dram[0].size = gd->ram_size; > > > > > + > > > > > + mtrr_add_request(MTRR_TYPE_WRBACK, 0, gd->ram_size); > > > > > + return 0; > > > > > + } > > > > > + > > > > > > > > I wonder why this change is needed. When booting from other > > > > bootloader, this dram_init_banksize() will not be called, no? > > > > > > How about this: > > > > > > It is useful to be able to boot the same x86 image on a device with or > > > without a first-stage bootloader. For example, with chromebook_coral, it > > > is helpful for testing to be able to boot the same U-Boot (complete with > > > FSP) on bare metal and from coreboot. It allows checking of things like > > > CPU speed, comparing registers, ACPI tables and the like. > > > > > > The idea is to change ll_boot_init() to false, and rebuild without > > > changing > > > anything else. > > > > This sounds like for a debugging purpose, instead of a supported > > configuration. So when we boot from previous bootloader, we really > > should use its configuration, for the coreboot case, > > coreboot-x86_defconfig should be used, and we can compare the > > registers, ACPI tables in that configuration, instead of "hacking" > > current U-Boot codes like in this series. > > Well the problem is then that we cannot boot the same image with or > without coreboot. Also there are various of device-tree things in > coral that we need for the laptop to work properly. My idea is to > detect coreboot and set ll_boot_init() dynamically if needed.
If so, why should we keep coreboot_defconfig for U-Boot as a supported target? I am more interested in what you proposed here: "detect coreboot and set ll_boot_init() dynamically if needed." The name ll_boot_init() does not sound great. It's written like a function call but actually it is a macro for true/false. Can we do something in the gd->flags to indicate we are booting from previous stage bootloaders instead of relying on ll_boot_init()? Regards, Bin