HI Bin,
On Tue, 26 Nov 2019 at 23:11, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Nov 27, 2019 at 1:08 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Tue, 26 Nov 2019 at 01:36, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Nov 25, 2019 at 12:11 PM Simon Glass sjg@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@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"helpThe filename of the file to use for the temporary-RAM init phase fromthe 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_VIDEOI 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?
Yes that's right...the benefit is that we can run coreboot's earlier setup code as a way of verifying that U-Boot does the right thing.
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.
The hard-coded values only work for a particular platform, which is why I have the instructions on how to use them. You can modify coreboot to jump to U-Boot from various different stages, and validate that everything is working correctly.
I could remove this code if that seems better, but I think it is useful perhaps for others.
* 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