Hi Bin,
On Tue, 26 Nov 2019 at 01:36, Bin Meng <bmeng...@gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, Nov 25, 2019 at 12:11 PM Simon Glass <s...@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 <s...@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).
>
> > + * 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,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
