Hi Simon, -----"Simon Glass" <[email protected]> schrieb: ----- > Betreff: [PATCH 6/6] x86: fsp: Support a warning message when DRAM init is > slow > > With DDR4, Intel SOCs take quite a long time to init their memory. During > this time, if the user is watching, it looks like SPL has hung. Add a > message in this case. > > This works by adding a return code to fspm_update_config() that indicates > whether MRC data was found and a new property to the device tree. > > Also add one more debug message while starting. > > Signed-off-by: Simon Glass <[email protected]> > --- > > arch/x86/cpu/apollolake/fsp_m.c | 12 ++++++++-- > arch/x86/dts/chromebook_coral.dts | 1 + > arch/x86/include/asm/fsp2/fsp_internal.h | 3 ++- > arch/x86/lib/fsp2/fsp_meminit.c | 24 +++++++++++++++---- > .../fsp/fsp2/apollolake/fsp-m.txt | 3 +++ > 5 files changed, 35 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/cpu/apollolake/fsp_m.c b/arch/x86/cpu/apollolake/fsp_m.c > index 1301100cd5..65461d85b8 100644 > --- a/arch/x86/cpu/apollolake/fsp_m.c > +++ b/arch/x86/cpu/apollolake/fsp_m.c > @@ -16,10 +16,14 @@ int fspm_update_config(struct udevice *dev, struct > fspm_upd *upd) > { > struct fsp_m_config *cfg = &upd->config; > struct fspm_arch_upd *arch = &upd->arch; > + int cache_ret = 0; > ofnode node; > + int ret; > > arch->nvs_buffer_ptr = NULL; > - prepare_mrc_cache(upd); > + cache_ret = prepare_mrc_cache(upd); > + if (cache_ret && cache_ret != -ENOENT) > + return log_msg_ret("mrc", cache_ret); > arch->stack_base = (void *)0xfef96000; > arch->boot_loader_tolum_size = 0; > arch->boot_mode = FSP_BOOT_WITH_FULL_CONFIGURATION; > @@ -28,7 +32,11 @@ int fspm_update_config(struct udevice *dev, struct > fspm_upd *upd) > if (!ofnode_valid(node)) > return log_msg_ret("fsp-m settings", -ENOENT); > > - return fsp_m_update_config_from_dtb(node, cfg); > + ret = fsp_m_update_config_from_dtb(node, cfg); > + if (ret) > + return log_msg_ret("dtb", cache_ret); > + > + return cache_ret; > } > > /* > diff --git a/arch/x86/dts/chromebook_coral.dts > b/arch/x86/dts/chromebook_coral.dts > index dea35b73a0..aad12f2c4d 100644 > --- a/arch/x86/dts/chromebook_coral.dts > +++ b/arch/x86/dts/chromebook_coral.dts > @@ -117,6 +117,7 @@ > reg = <0x00000000 0 0 0 0>; > compatible = "intel,apl-hostbridge"; > pciex-region-size = <0x10000000>; > + fspm,training-delay = <21>; > /* > * Parameters used by the FSP-S binary blob. This is > * really unfortunate since these parameters mostly > diff --git a/arch/x86/include/asm/fsp2/fsp_internal.h > b/arch/x86/include/asm/fsp2/fsp_internal.h > index f751fbf961..b4a4fbbd84 100644 > --- a/arch/x86/include/asm/fsp2/fsp_internal.h > +++ b/arch/x86/include/asm/fsp2/fsp_internal.h > @@ -57,7 +57,8 @@ int arch_fsps_preinit(void); > * > * @dev: Hostbridge device containing config > * @upd: Config data to fill in > - * @return 0 if OK, -ve on error > + * @return 0 if OK, -ENOENT if OK but no MRC-cache data was found, other -ve > on > + * error > */ > int fspm_update_config(struct udevice *dev, struct fspm_upd *upd); > > diff --git a/arch/x86/lib/fsp2/fsp_meminit.c b/arch/x86/lib/fsp2/fsp_meminit.c > index faf9c29aef..ce0b0aff76 100644 > --- a/arch/x86/lib/fsp2/fsp_meminit.c > +++ b/arch/x86/lib/fsp2/fsp_meminit.c > @@ -9,6 +9,7 @@ > #include <common.h> > #include <binman.h> > #include <bootstage.h> > +#include <dm.h> > #include <log.h> > #include <asm/mrccache.h> > #include <asm/fsp/fsp_infoheader.h> > @@ -63,8 +64,10 @@ int fsp_memory_init(bool s3wake, bool use_spi_flash) > struct fsp_header *hdr; > struct hob_header *hob; > struct udevice *dev; > + int delay; > int ret; > > + log_debug("Locating FSP\n"); > ret = fsp_locate_fsp(FSP_M, &entry, use_spi_flash, &dev, &hdr, NULL); > if (ret) > return log_msg_ret("locate FSP", ret); > @@ -76,21 +79,32 @@ int fsp_memory_init(bool s3wake, bool use_spi_flash) > return log_msg_ret("Bad UPD signature", -EPERM); > memcpy(&upd, fsp_upd, sizeof(upd)); > > + delay = dev_read_u32_default(dev, "fspm,training-delay", 0); > ret = fspm_update_config(dev, &upd); > - if (ret) > - return log_msg_ret("Could not setup config", ret); > - > - debug("SDRAM init..."); > + if (ret) { > + if (ret != -ENOENT) > + return log_msg_ret("Could not setup config", ret); > + } else { > + delay = 0; > + } > + > + if (delay) > + printf("SDRAM training (%d seconds)...", delay); > + else > + log_debug("SDRAM init..."); > bootstage_start(BOOTSTAGE_ID_ACCUM_FSP_M, "fsp-m"); > func = (fsp_memory_init_func)(hdr->img_base + hdr->fsp_mem_init); > ret = func(&upd, &hob); > bootstage_accum(BOOTSTAGE_ID_ACCUM_FSP_M); > cpu_reinit_fpu(); > + if (delay) > + printf("done\n"); > + else > + log_debug("done\n"); > if (ret) > return log_msg_ret("SDRAM init fail\n", ret); > > gd->arch.hob_list = hob; > - debug("done\n"); > > ret = fspm_done(dev); > if (ret) > diff --git a/doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-m.txt > b/doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-m.txt > index 647a0862d4..67407fa935 100644 > --- a/doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-m.txt > +++ b/doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-m.txt > @@ -17,6 +17,9 @@ values of the FSP-M are used. > [2] https://github.com/IntelFsp/FSP/tree/master/ApolloLakeFspBinPkg/Docs > > Optional properties: > +- fspm,training-delay: Time taken to train DDR memory if there is no cached > MRC > + data, in seconds. This is used to show a message if possible. For Intel > APL > + this is typicaly 21 seconds.
1) The required time is not APL specific, I think it depends on the memory configuration. I have tested it on an APL board with just 1 GB RAM, and there it only takes ~6 seconds. How about rewording it as follows: "As an example, for Chromebook Coral this is typically 21 seconds." 2) Typo: typically > - fspm,serial-debug-port-address: Debug Serial Port Base address > - fspm,serial-debug-port-type: Debug Serial Port Type > 0: NONE > -- > 2.27.0.rc0.183.gde8f92d652-goog Reviewed-by: Wolfgang Wallner <[email protected]> Tested-by: Wolfgang Wallner <[email protected]> Tested on a custom Apollo Lake board
