[U-Boot] [PATCH v5 045/101] x86: fsp: Add FSP2 base support
Bin Meng
bmeng.cn at gmail.com
Wed Nov 27 06:11:07 UTC 2019
Hi Simon,
On Wed, Nov 27, 2019 at 1:08 AM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Bin,
>
> On Tue, 26 Nov 2019 at 01:36, Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Mon, Nov 25, 2019 at 12:11 PM Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Add support for some important configuration options and FSP memory init.
> > > The memory init uses swizzle tables from the device tree.
> > >
> > > Support for the FSP_S binary is also included.
> > >
> > > Bootstage timing is used for both FSP_M and FSP_S and memory-mapped SPI
> > > reads.
> > >
> > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > ---
> > >
> > > Changes in v5:
> > > - Drop SAFETY_MARGIN
> > >
> > > Changes in v4:
> > > - Add a LOG_CATEGORY for silicon init
> > > - Drop duplicate VBT file CONFIG
> > > - Enable HAVE_VBT for FSP2 also
> > > - Explain the 'twisty headers' comment
> > > - Fix FSP_M reference to refer to FSP_S in commit message
> > > - Fix comment on fsp_silicon_init()
> > > - Rename arch_fsp_s_preinit() to arch_fsps_preinit()
> > > - Rename get_coreboot_fsp() and add comments
> > > - Switch over to use pinctrl for pad init/config
> > > - Use lower-case pinctrl in arch_cpu_init_dm()
> > >
> > > Changes in v3:
> > > - Add a proper implementation of fsp_notify
> > > - Add an fsp: tag
> > > - Add bootstage timing for memory-mapped reads
> > > - Add fsp_locate_fsp to locate an fsp component
> > > - Add fspm_done() hook
> > > - Add support for FSP-S component and VBT
> > > - Simplify types for fsp_locate_fsp()
> > > - Switch mmap to use SPI instead of SPI flash
> > >
> > > Changes in v2: None
> > >
> > > arch/x86/Kconfig | 54 ++++++-
> > > arch/x86/include/asm/fsp2/fsp_api.h | 63 ++++++++
> > > arch/x86/include/asm/fsp2/fsp_internal.h | 97 +++++++++++++
> > > arch/x86/lib/fsp2/Makefile | 10 ++
> > > arch/x86/lib/fsp2/fsp_common.c | 13 ++
> > > arch/x86/lib/fsp2/fsp_dram.c | 77 ++++++++++
> > > arch/x86/lib/fsp2/fsp_init.c | 174 +++++++++++++++++++++++
> > > arch/x86/lib/fsp2/fsp_meminit.c | 97 +++++++++++++
> > > arch/x86/lib/fsp2/fsp_silicon_init.c | 54 +++++++
> > > arch/x86/lib/fsp2/fsp_support.c | 131 +++++++++++++++++
> > > include/bootstage.h | 3 +
> > > 11 files changed, 770 insertions(+), 3 deletions(-)
> > > create mode 100644 arch/x86/include/asm/fsp2/fsp_api.h
> > > create mode 100644 arch/x86/include/asm/fsp2/fsp_internal.h
> > > create mode 100644 arch/x86/lib/fsp2/Makefile
> > > create mode 100644 arch/x86/lib/fsp2/fsp_common.c
> > > create mode 100644 arch/x86/lib/fsp2/fsp_dram.c
> > > create mode 100644 arch/x86/lib/fsp2/fsp_init.c
> > > create mode 100644 arch/x86/lib/fsp2/fsp_meminit.c
> > > create mode 100644 arch/x86/lib/fsp2/fsp_silicon_init.c
> > > create mode 100644 arch/x86/lib/fsp2/fsp_support.c
> > >
> > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > index 17a6fe6d3d..6bac5d5fe8 100644
> > > --- a/arch/x86/Kconfig
> > > +++ b/arch/x86/Kconfig
> > > @@ -326,7 +326,7 @@ config X86_RAMTEST
> > >
> > > config FLASH_DESCRIPTOR_FILE
> > > string "Flash descriptor binary filename"
> > > - depends on HAVE_INTEL_ME
> > > + depends on HAVE_INTEL_ME || FSP_VERSION2
> > > default "descriptor.bin"
> > > help
> > > The filename of the file to use as flash descriptor in the
> > > @@ -411,6 +411,54 @@ config FSP_ADDR
> > > The default base address of 0xfffc0000 indicates that the binary must
> > > be located at offset 0xc0000 from the beginning of a 1MB flash device.
> > >
> > > +if FSP_VERSION2
> > > +
> > > +config FSP_FILE_T
> > > + string "Firmware-Support-Package binary filename (Temp RAM)"
> > > + default "fsp_t.bin"
> > > + help
> > > + The filename of the file to use for the temporary-RAM init phase from
> > > + the Firmware-Support-Package binary. Put this in the board directory.
> >
> > nits: Firmware Support Package (drop -)
>
> OK...the hyphens are actually correct I think, but perhaps make it
> hard to search for.
>
> [..]
>
> > > config VIDEO_FSP
> > > bool "Enable FSP framebuffer driver support"
> > > - depends on HAVE_VBT && DM_VIDEO
> > > + depends on (HAVE_VBT || FSP_VERSION2) && DM_VIDEO
> >
> > I think the original logic already satisfies the dependency
> > requirement, that we don't need explicitly list FSP_VERSION2 here.
>
> Yes, now that I don't need to add a new Kconfig I can drop this.
>
> [..]
>
> > > diff --git a/arch/x86/lib/fsp2/fsp_init.c b/arch/x86/lib/fsp2/fsp_init.c
> > > new file mode 100644
> > > index 0000000000..bcc385e876
> > > --- /dev/null
> > > +++ b/arch/x86/lib/fsp2/fsp_init.c
> > > @@ -0,0 +1,174 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright 2019 Google LLC
> > > + */
> > > +
> > > +#include <common.h>
> > > +#include <binman.h>
> > > +#include <binman_sym.h>
> > > +#include <cbfs.h>
> > > +#include <dm.h>
> > > +#include <init.h>
> > > +#include <spi.h>
> > > +#include <spl.h>
> > > +#include <spi_flash.h>
> > > +#include <asm/intel_pinctrl.h>
> > > +#include <dm/uclass-internal.h>
> > > +#include <asm/fsp2/fsp_internal.h>
> > > +
> > > +int arch_cpu_init_dm(void)
> > > +{
> > > + struct udevice *dev;
> > > + ofnode node;
> > > + int ret;
> > > +
> > > + /* Make sure pads are set up early in U-Boot */
> > > + if (spl_phase() != PHASE_BOARD_F)
> > > + return 0;
> > > +
> > > + /* Probe all pinctrl devices to set up the pads */
> > > + ret = uclass_first_device_err(UCLASS_PINCTRL, &dev);
> > > + if (ret)
> > > + return log_msg_ret("no fsp pinctrl", ret);
> > > + node = ofnode_path("fsp");
> > > + if (!ofnode_valid(node))
> > > + return log_msg_ret("no fsp params", -EINVAL);
> > > + ret = pinctrl_config_pads_for_node(dev, node);
> > > + if (ret)
> > > + return log_msg_ret("pad config", ret);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +#if !defined(CONFIG_TPL_BUILD)
> > > +binman_sym_declare(ulong, intel_fsp_m, image_pos);
> > > +binman_sym_declare(ulong, intel_fsp_m, size);
> > > +
> > > +/**
> > > + * get_cbfs_fsp() - Obtain the FSP by looking up in CBFS
> > > + *
> > > + * This looks up an FSP in a CBFS. It is used mostly for testing, when booting
> > > + * U-Boot from a hybrid image containing coreboot as the first-stage bootloader.
> > > + *
> > > + * @type; Type to look up (only FSP_M supported at present)
> > > + * @map_base: Base memory address for mapped SPI
> > > + * @entry: Returns an entry containing the position of the FSP image
> > > + */
> > > +static int get_cbfs_fsp(enum fsp_type_t type, ulong map_base,
> > > + struct binman_entry *entry)
> > > +{
> > > + /*
> > > + * Use a hard-coded position of CBFS in the ROM for now. It would be
> > > + * possible to read the position using the FMAP in the ROM, but since
> > > + * this code is only used for development, it doesn't seem worth it.
> >
> > I don't understand why this code is only used for development.
>
> Because we don't use CBFS in U-Boot normally. This is just a way to
> meld coreboot and U-Boot for development purposes. I think it is
> useful enough to have in here and perhaps one day become more generic
> (perhaps when we do another embedded platform).
>
My understanding is that U-Boot boots directly without coreboot's help
on Apollo Lake in this series. If this is used for development, what
benefit do we get?
Perhaps could you please describe it with more details about the
"hybrid" mode, ie: mixed coreboot and U-Boot. It's not as simple as
using U-Boot as the coreboot payload, I assume. In any case, the
hardcoded number won't work for other platform, I think.
> >
> > > + * Use the 'cbfstool <image> layout' command to get these values, e.g.:
> > > + * 'COREBOOT' (CBFS, size 1814528, offset 2117632)
> >
> > Does file_cbfs_find() and its friends in fs/cbfs.c help?
>
> That is for finding a CBFS file in a CBFS. We need to actually find
> the CBFS and there are unfortunately several of them in the image.
>
> The only really way would be to hard-code the section name and use the
> FMAP to find it. But we don't really use FMAP either, so that seems
> like a lot of code to add.
>
> [..]
Regards,
Bin
More information about the U-Boot
mailing list