[PATCH v3 00/17] x86: Various fixes for chromebooks

This adds some fixes for x86-based Chromebook builds which have picked up a few problems recently.
With this, chromebook_link/64, chromebook_samus and chromebook_coral work correctly.
Changes in v3: - Fix 'intend' typo - Fix 'returns' typo - Fix 'somtimes' typo - Fix 'propblem' typo - Squash in patch 'Set up LPC only after relocation' - Fix 'build' typo - Fix 'boot' typo - Drop change to memset() - Fix invalid commit IDs obtained from another branch - Fix 'require' typo - Add more detail about why it might be hanging - Fix 'call' typo
Changes in v2: - Add new patch to set up LPC only after relocation - Add new patch to tidy up address for loading U-Boot from SPL - Drop patch "x86: Add on to existing MTRRs in SPL" - Add various patches to resolve problems with chromebook_link64
Simon Glass (17): dm: Emit the arch_cpu_init_dm() even only before relocation binman: Support writing symbols for ucode etypes sf: Guard against zero erasesize sf: Rename spi-nor-tiny functions x86: ivybridge: Ensure LPC is available for GPIO base x86: samus: Drop EFI_LOADER x86: Support debug UART in 64-bit mode x86: Tidy up availability of string functions x86: mrc: Correct SPL debug message x86: spl: Show debugging for BSS x86: Tidy up address for loading U-Boot from SPL x86: Return mtrr_add_request() to its old purpose x86: spl: Avoid using init_cache_f_r() from SPL spl: Commit MTRRs only in board_init_f_r() x86: Simplify cpu_jump_to_64bit_uboot() x86: samus: Don't include audio and SATA in TPL x86: samus: Adjust TPL start and pre-reloc memory size
arch/arm/mach-imx/imx8/cpu.c | 2 +- arch/arm/mach-imx/imx8m/soc.c | 2 +- arch/arm/mach-imx/imx8ulp/soc.c | 2 +- arch/arm/mach-imx/imx9/soc.c | 2 +- arch/arm/mach-omap2/am33xx/board.c | 2 +- arch/arm/mach-omap2/hwinit-common.c | 2 +- arch/mips/mach-pic32/cpu.c | 2 +- arch/nios2/cpu/cpu.c | 2 +- arch/riscv/cpu/cpu.c | 2 +- arch/x86/cpu/baytrail/cpu.c | 2 +- arch/x86/cpu/broadwell/Makefile | 4 +-- arch/x86/cpu/broadwell/cpu.c | 2 +- arch/x86/cpu/i386/cpu.c | 32 +++---------------- arch/x86/cpu/ivybridge/bd82x6x.c | 17 +++++----- arch/x86/cpu/ivybridge/cpu.c | 2 +- arch/x86/cpu/mtrr.c | 6 +++- arch/x86/cpu/quark/quark.c | 2 +- arch/x86/cpu/x86_64/cpu.c | 7 ++++ arch/x86/include/asm/string.h | 6 +++- arch/x86/lib/Makefile | 4 ++- arch/x86/lib/fsp2/fsp_init.c | 2 +- arch/x86/lib/mrccache.c | 2 +- arch/x86/lib/spl.c | 19 ++++------- configs/chromebook_samus_defconfig | 1 + configs/chromebook_samus_tpl_defconfig | 4 +-- doc/develop/event.rst | 6 ++-- drivers/core/root.c | 4 +-- drivers/cpu/microblaze_cpu.c | 2 +- drivers/mtd/spi/sf_probe.c | 3 +- drivers/mtd/spi/spi-nor-tiny.c | 16 +++++----- drivers/sysreset/sysreset_x86.c | 9 ++++-- include/event.h | 2 +- .../binman/etype/u_boot_spl_with_ucode_ptr.py | 2 +- .../binman/etype/u_boot_tpl_with_ucode_ptr.py | 2 +- tools/binman/etype/u_boot_with_ucode_ptr.py | 4 +-- 35 files changed, 89 insertions(+), 91 deletions(-)

The original function was only called once, before relocation. The new one is called again after relocation. This was not the intent of the original call. Fix this by renaming and updating the calling logic.
With this, chromebook_link64 makes it through SPL.
Fixes: 7fe32b3442f ("event: Convert arch_cpu_init_dm() to") Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events") Reviewed-by: Bin Meng bmeng.cn@gmail.com
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: - Fix 'intend' typo
arch/arm/mach-imx/imx8/cpu.c | 2 +- arch/arm/mach-imx/imx8m/soc.c | 2 +- arch/arm/mach-imx/imx8ulp/soc.c | 2 +- arch/arm/mach-imx/imx9/soc.c | 2 +- arch/arm/mach-omap2/am33xx/board.c | 2 +- arch/arm/mach-omap2/hwinit-common.c | 2 +- arch/mips/mach-pic32/cpu.c | 2 +- arch/nios2/cpu/cpu.c | 2 +- arch/riscv/cpu/cpu.c | 2 +- arch/x86/cpu/baytrail/cpu.c | 2 +- arch/x86/cpu/broadwell/cpu.c | 2 +- arch/x86/cpu/ivybridge/cpu.c | 2 +- arch/x86/cpu/quark/quark.c | 2 +- arch/x86/lib/fsp2/fsp_init.c | 2 +- doc/develop/event.rst | 6 +++--- drivers/core/root.c | 4 ++-- drivers/cpu/microblaze_cpu.c | 2 +- include/event.h | 2 +- 18 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/arch/arm/mach-imx/imx8/cpu.c b/arch/arm/mach-imx/imx8/cpu.c index be1f4edded1..99772f68c32 100644 --- a/arch/arm/mach-imx/imx8/cpu.c +++ b/arch/arm/mach-imx/imx8/cpu.c @@ -89,7 +89,7 @@ static int imx8_init_mu(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, imx8_init_mu); +EVENT_SPY(EVT_DM_POST_INIT_F, imx8_init_mu);
#if defined(CONFIG_ARCH_MISC_INIT) int arch_misc_init(void) diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index 4705e6c1192..5a4f8358c9f 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -549,7 +549,7 @@ static int imx8m_check_clock(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, imx8m_check_clock); +EVENT_SPY(EVT_DM_POST_INIT_F, imx8m_check_clock);
static void imx8m_setup_snvs(void) { diff --git a/arch/arm/mach-imx/imx8ulp/soc.c b/arch/arm/mach-imx/imx8ulp/soc.c index 8424332f429..81eae02b6a8 100644 --- a/arch/arm/mach-imx/imx8ulp/soc.c +++ b/arch/arm/mach-imx/imx8ulp/soc.c @@ -808,7 +808,7 @@ static int imx8ulp_evt_dm_post_init(void *ctx, struct event *event) { return imx8ulp_dm_post_init(); } -EVENT_SPY(EVT_DM_POST_INIT, imx8ulp_evt_dm_post_init); +EVENT_SPY(EVT_DM_POST_INIT_F, imx8ulp_evt_dm_post_init);
#if defined(CONFIG_SPL_BUILD) __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) diff --git a/arch/arm/mach-imx/imx9/soc.c b/arch/arm/mach-imx/imx9/soc.c index a16e22ea6bb..252663a9eec 100644 --- a/arch/arm/mach-imx/imx9/soc.c +++ b/arch/arm/mach-imx/imx9/soc.c @@ -262,7 +262,7 @@ int imx9_probe_mu(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, imx9_probe_mu); +EVENT_SPY(EVT_DM_POST_INIT_F, imx9_probe_mu);
int timer_init(void) { diff --git a/arch/arm/mach-omap2/am33xx/board.c b/arch/arm/mach-omap2/am33xx/board.c index a52d04d85c8..ecc0a592e99 100644 --- a/arch/arm/mach-omap2/am33xx/board.c +++ b/arch/arm/mach-omap2/am33xx/board.c @@ -535,4 +535,4 @@ static int am33xx_dm_post_init(void *ctx, struct event *event) #endif return 0; } -EVENT_SPY(EVT_DM_POST_INIT, am33xx_dm_post_init); +EVENT_SPY(EVT_DM_POST_INIT_F, am33xx_dm_post_init); diff --git a/arch/arm/mach-omap2/hwinit-common.c b/arch/arm/mach-omap2/hwinit-common.c index c4a8eabc3eb..771533394bc 100644 --- a/arch/arm/mach-omap2/hwinit-common.c +++ b/arch/arm/mach-omap2/hwinit-common.c @@ -246,7 +246,7 @@ static int omap2_system_init(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, omap2_system_init); +EVENT_SPY(EVT_DM_POST_INIT_F, omap2_system_init);
/* * Routine: wait_for_command_complete diff --git a/arch/mips/mach-pic32/cpu.c b/arch/mips/mach-pic32/cpu.c index de449e3c6a2..ec3c2505313 100644 --- a/arch/mips/mach-pic32/cpu.c +++ b/arch/mips/mach-pic32/cpu.c @@ -102,7 +102,7 @@ static int pic32_flash_prefetch(void *ctx, struct event *event) prefetch_init(); return 0; } -EVENT_SPY(EVT_DM_POST_INIT, pic32_flash_prefetch); +EVENT_SPY(EVT_DM_POST_INIT_F, pic32_flash_prefetch);
/* Un-gate DDR2 modules (gated by default) */ static void ddr2_pmd_ungate(void) diff --git a/arch/nios2/cpu/cpu.c b/arch/nios2/cpu/cpu.c index 85544503a5e..da167f4b29e 100644 --- a/arch/nios2/cpu/cpu.c +++ b/arch/nios2/cpu/cpu.c @@ -80,7 +80,7 @@ static int nios_cpu_setup(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, nios_cpu_setup); +EVENT_SPY(EVT_DM_POST_INIT_F, nios_cpu_setup);
static int altera_nios2_get_desc(const struct udevice *dev, char *buf, int size) diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c index e1ed4ec01d0..ecfb1fb08c4 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -145,7 +145,7 @@ int riscv_cpu_setup(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, riscv_cpu_setup); +EVENT_SPY(EVT_DM_POST_INIT_F, riscv_cpu_setup);
int arch_early_init_r(void) { diff --git a/arch/x86/cpu/baytrail/cpu.c b/arch/x86/cpu/baytrail/cpu.c index 4fb6a485542..4a7b4f617f8 100644 --- a/arch/x86/cpu/baytrail/cpu.c +++ b/arch/x86/cpu/baytrail/cpu.c @@ -64,7 +64,7 @@ static int baytrail_uart_init(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, baytrail_uart_init); +EVENT_SPY(EVT_DM_POST_INIT_F, baytrail_uart_init);
static void set_max_freq(void) { diff --git a/arch/x86/cpu/broadwell/cpu.c b/arch/x86/cpu/broadwell/cpu.c index 7877961451a..f30aebfe4c6 100644 --- a/arch/x86/cpu/broadwell/cpu.c +++ b/arch/x86/cpu/broadwell/cpu.c @@ -40,7 +40,7 @@ static int broadwell_init_cpu(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, broadwell_init_cpu); +EVENT_SPY(EVT_DM_POST_INIT_F, broadwell_init_cpu);
void set_max_freq(void) { diff --git a/arch/x86/cpu/ivybridge/cpu.c b/arch/x86/cpu/ivybridge/cpu.c index cffc5d5b1d8..c988d7ff477 100644 --- a/arch/x86/cpu/ivybridge/cpu.c +++ b/arch/x86/cpu/ivybridge/cpu.c @@ -86,7 +86,7 @@ static int ivybridge_cpu_init(void *ctx, struct event *ev)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, ivybridge_cpu_init); +EVENT_SPY(EVT_DM_POST_INIT_F, ivybridge_cpu_init);
#define PCH_EHCI0_TEMP_BAR0 0xe8000000 #define PCH_EHCI1_TEMP_BAR0 0xe8000400 diff --git a/arch/x86/cpu/quark/quark.c b/arch/x86/cpu/quark/quark.c index 0a1fbb34d40..1be8e38cdf4 100644 --- a/arch/x86/cpu/quark/quark.c +++ b/arch/x86/cpu/quark/quark.c @@ -263,7 +263,7 @@ static int quark_init_pcie(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, quark_init_pcie); +EVENT_SPY(EVT_DM_POST_INIT_F, quark_init_pcie);
int checkcpu(void) { diff --git a/arch/x86/lib/fsp2/fsp_init.c b/arch/x86/lib/fsp2/fsp_init.c index b15926e8247..afec7d08d67 100644 --- a/arch/x86/lib/fsp2/fsp_init.c +++ b/arch/x86/lib/fsp2/fsp_init.c @@ -42,7 +42,7 @@ int fsp_setup_pinctrl(void *ctx, struct event *event)
return ret; } -EVENT_SPY(EVT_DM_POST_INIT, fsp_setup_pinctrl); +EVENT_SPY(EVT_DM_POST_INIT_F, fsp_setup_pinctrl);
#if !defined(CONFIG_TPL_BUILD) binman_sym_declare(ulong, intel_fsp_m, image_pos); diff --git a/doc/develop/event.rst b/doc/develop/event.rst index 4ff59348371..4c34fffc63b 100644 --- a/doc/develop/event.rst +++ b/doc/develop/event.rst @@ -11,7 +11,7 @@ block device is probed. Rather than using weak functions and direct calls across subsystemss, it is often easier to use an event.
-An event consists of a type (e.g. EVT_DM_POST_INIT) and some optional data, +An event consists of a type (e.g. EVT_DM_POST_INIT_F) and some optional data, in `union event_data`. An event spy can be creasted to watch for events of a particular type. When the event is created, it is sent to each spy in turn.
@@ -26,9 +26,9 @@ To declare a spy, use something like this:: /* do something */ return 0; } - EVENT_SPY(EVT_DM_POST_INIT, snow_setup_cpus); + EVENT_SPY(EVT_DM_POST_INIT_F, snow_setup_cpus);
-Your function is called when EVT_DM_POST_INIT is emitted, i.e. after driver +Your function is called when EVT_DM_POST_INIT_F is emitted, i.e. after driver model is inited (in SPL, or in U-Boot proper before and after relocation).
diff --git a/drivers/core/root.c b/drivers/core/root.c index c4fb48548bb..6775fb0b657 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -436,8 +436,8 @@ int dm_init_and_scan(bool pre_reloc_only) return ret; } } - if (CONFIG_IS_ENABLED(DM_EVENT)) { - ret = event_notify_null(EVT_DM_POST_INIT); + if (CONFIG_IS_ENABLED(DM_EVENT) && !(gd->flags & GD_FLG_RELOC)) { + ret = event_notify_null(EVT_DM_POST_INIT_F); if (ret) return log_msg_ret("ev", ret); } diff --git a/drivers/cpu/microblaze_cpu.c b/drivers/cpu/microblaze_cpu.c index b9d07928223..c97a89fbd5c 100644 --- a/drivers/cpu/microblaze_cpu.c +++ b/drivers/cpu/microblaze_cpu.c @@ -29,7 +29,7 @@ static int microblaze_cpu_probe_all(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, microblaze_cpu_probe_all); +EVENT_SPY(EVT_DM_POST_INIT_F, microblaze_cpu_probe_all);
static void microblaze_set_cpuinfo_pvr(struct microblaze_cpuinfo *ci) { diff --git a/include/event.h b/include/event.h index e4580b68350..fe41080fa63 100644 --- a/include/event.h +++ b/include/event.h @@ -22,7 +22,7 @@ enum event_t { EVT_TEST,
/* Events related to driver model */ - EVT_DM_POST_INIT, + EVT_DM_POST_INIT_F, EVT_DM_PRE_PROBE, EVT_DM_POST_PROBE, EVT_DM_PRE_REMOVE,

On 05.05.23 00:50, Simon Glass wrote:
The original function was only called once, before relocation. The new one is called again after relocation. This was not the intent of the original call. Fix this by renaming and updating the calling logic.
With this, chromebook_link64 makes it through SPL.
Fixes: 7fe32b3442f ("event: Convert arch_cpu_init_dm() to") Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events") Reviewed-by: Bin Meng bmeng.cn@gmail.com
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Fix 'intend' typo
arch/arm/mach-imx/imx8/cpu.c | 2 +- arch/arm/mach-imx/imx8m/soc.c | 2 +- arch/arm/mach-imx/imx8ulp/soc.c | 2 +- arch/arm/mach-imx/imx9/soc.c | 2 +- arch/arm/mach-omap2/am33xx/board.c | 2 +- arch/arm/mach-omap2/hwinit-common.c | 2 +- arch/mips/mach-pic32/cpu.c | 2 +- arch/nios2/cpu/cpu.c | 2 +- arch/riscv/cpu/cpu.c | 2 +- arch/x86/cpu/baytrail/cpu.c | 2 +- arch/x86/cpu/broadwell/cpu.c | 2 +- arch/x86/cpu/ivybridge/cpu.c | 2 +- arch/x86/cpu/quark/quark.c | 2 +- arch/x86/lib/fsp2/fsp_init.c | 2 +- doc/develop/event.rst | 6 +++--- drivers/core/root.c | 4 ++-- drivers/cpu/microblaze_cpu.c | 2 +- include/event.h | 2 +- 18 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/arch/arm/mach-imx/imx8/cpu.c b/arch/arm/mach-imx/imx8/cpu.c index be1f4edded1..99772f68c32 100644 --- a/arch/arm/mach-imx/imx8/cpu.c +++ b/arch/arm/mach-imx/imx8/cpu.c @@ -89,7 +89,7 @@ static int imx8_init_mu(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, imx8_init_mu); +EVENT_SPY(EVT_DM_POST_INIT_F, imx8_init_mu);
#if defined(CONFIG_ARCH_MISC_INIT) int arch_misc_init(void) diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index 4705e6c1192..5a4f8358c9f 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -549,7 +549,7 @@ static int imx8m_check_clock(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, imx8m_check_clock); +EVENT_SPY(EVT_DM_POST_INIT_F, imx8m_check_clock);
static void imx8m_setup_snvs(void) { diff --git a/arch/arm/mach-imx/imx8ulp/soc.c b/arch/arm/mach-imx/imx8ulp/soc.c index 8424332f429..81eae02b6a8 100644 --- a/arch/arm/mach-imx/imx8ulp/soc.c +++ b/arch/arm/mach-imx/imx8ulp/soc.c @@ -808,7 +808,7 @@ static int imx8ulp_evt_dm_post_init(void *ctx, struct event *event) { return imx8ulp_dm_post_init(); } -EVENT_SPY(EVT_DM_POST_INIT, imx8ulp_evt_dm_post_init); +EVENT_SPY(EVT_DM_POST_INIT_F, imx8ulp_evt_dm_post_init);
#if defined(CONFIG_SPL_BUILD) __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) diff --git a/arch/arm/mach-imx/imx9/soc.c b/arch/arm/mach-imx/imx9/soc.c index a16e22ea6bb..252663a9eec 100644 --- a/arch/arm/mach-imx/imx9/soc.c +++ b/arch/arm/mach-imx/imx9/soc.c @@ -262,7 +262,7 @@ int imx9_probe_mu(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, imx9_probe_mu); +EVENT_SPY(EVT_DM_POST_INIT_F, imx9_probe_mu);
int timer_init(void) { diff --git a/arch/arm/mach-omap2/am33xx/board.c b/arch/arm/mach-omap2/am33xx/board.c index a52d04d85c8..ecc0a592e99 100644 --- a/arch/arm/mach-omap2/am33xx/board.c +++ b/arch/arm/mach-omap2/am33xx/board.c @@ -535,4 +535,4 @@ static int am33xx_dm_post_init(void *ctx, struct event *event) #endif return 0; } -EVENT_SPY(EVT_DM_POST_INIT, am33xx_dm_post_init); +EVENT_SPY(EVT_DM_POST_INIT_F, am33xx_dm_post_init); diff --git a/arch/arm/mach-omap2/hwinit-common.c b/arch/arm/mach-omap2/hwinit-common.c index c4a8eabc3eb..771533394bc 100644 --- a/arch/arm/mach-omap2/hwinit-common.c +++ b/arch/arm/mach-omap2/hwinit-common.c @@ -246,7 +246,7 @@ static int omap2_system_init(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, omap2_system_init); +EVENT_SPY(EVT_DM_POST_INIT_F, omap2_system_init);
/*
- Routine: wait_for_command_complete
diff --git a/arch/mips/mach-pic32/cpu.c b/arch/mips/mach-pic32/cpu.c index de449e3c6a2..ec3c2505313 100644 --- a/arch/mips/mach-pic32/cpu.c +++ b/arch/mips/mach-pic32/cpu.c @@ -102,7 +102,7 @@ static int pic32_flash_prefetch(void *ctx, struct event *event) prefetch_init(); return 0; } -EVENT_SPY(EVT_DM_POST_INIT, pic32_flash_prefetch); +EVENT_SPY(EVT_DM_POST_INIT_F, pic32_flash_prefetch);
/* Un-gate DDR2 modules (gated by default) */ static void ddr2_pmd_ungate(void) diff --git a/arch/nios2/cpu/cpu.c b/arch/nios2/cpu/cpu.c index 85544503a5e..da167f4b29e 100644 --- a/arch/nios2/cpu/cpu.c +++ b/arch/nios2/cpu/cpu.c @@ -80,7 +80,7 @@ static int nios_cpu_setup(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, nios_cpu_setup); +EVENT_SPY(EVT_DM_POST_INIT_F, nios_cpu_setup);
static int altera_nios2_get_desc(const struct udevice *dev, char *buf, int size) diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c index e1ed4ec01d0..ecfb1fb08c4 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -145,7 +145,7 @@ int riscv_cpu_setup(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, riscv_cpu_setup); +EVENT_SPY(EVT_DM_POST_INIT_F, riscv_cpu_setup);
int arch_early_init_r(void) { diff --git a/arch/x86/cpu/baytrail/cpu.c b/arch/x86/cpu/baytrail/cpu.c index 4fb6a485542..4a7b4f617f8 100644 --- a/arch/x86/cpu/baytrail/cpu.c +++ b/arch/x86/cpu/baytrail/cpu.c @@ -64,7 +64,7 @@ static int baytrail_uart_init(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, baytrail_uart_init); +EVENT_SPY(EVT_DM_POST_INIT_F, baytrail_uart_init);
static void set_max_freq(void) { diff --git a/arch/x86/cpu/broadwell/cpu.c b/arch/x86/cpu/broadwell/cpu.c index 7877961451a..f30aebfe4c6 100644 --- a/arch/x86/cpu/broadwell/cpu.c +++ b/arch/x86/cpu/broadwell/cpu.c @@ -40,7 +40,7 @@ static int broadwell_init_cpu(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, broadwell_init_cpu); +EVENT_SPY(EVT_DM_POST_INIT_F, broadwell_init_cpu);
void set_max_freq(void) { diff --git a/arch/x86/cpu/ivybridge/cpu.c b/arch/x86/cpu/ivybridge/cpu.c index cffc5d5b1d8..c988d7ff477 100644 --- a/arch/x86/cpu/ivybridge/cpu.c +++ b/arch/x86/cpu/ivybridge/cpu.c @@ -86,7 +86,7 @@ static int ivybridge_cpu_init(void *ctx, struct event *ev)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, ivybridge_cpu_init); +EVENT_SPY(EVT_DM_POST_INIT_F, ivybridge_cpu_init);
#define PCH_EHCI0_TEMP_BAR0 0xe8000000 #define PCH_EHCI1_TEMP_BAR0 0xe8000400 diff --git a/arch/x86/cpu/quark/quark.c b/arch/x86/cpu/quark/quark.c index 0a1fbb34d40..1be8e38cdf4 100644 --- a/arch/x86/cpu/quark/quark.c +++ b/arch/x86/cpu/quark/quark.c @@ -263,7 +263,7 @@ static int quark_init_pcie(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, quark_init_pcie); +EVENT_SPY(EVT_DM_POST_INIT_F, quark_init_pcie);
int checkcpu(void) { diff --git a/arch/x86/lib/fsp2/fsp_init.c b/arch/x86/lib/fsp2/fsp_init.c index b15926e8247..afec7d08d67 100644 --- a/arch/x86/lib/fsp2/fsp_init.c +++ b/arch/x86/lib/fsp2/fsp_init.c @@ -42,7 +42,7 @@ int fsp_setup_pinctrl(void *ctx, struct event *event)
return ret; } -EVENT_SPY(EVT_DM_POST_INIT, fsp_setup_pinctrl); +EVENT_SPY(EVT_DM_POST_INIT_F, fsp_setup_pinctrl);
#if !defined(CONFIG_TPL_BUILD) binman_sym_declare(ulong, intel_fsp_m, image_pos); diff --git a/doc/develop/event.rst b/doc/develop/event.rst index 4ff59348371..4c34fffc63b 100644 --- a/doc/develop/event.rst +++ b/doc/develop/event.rst @@ -11,7 +11,7 @@ block device is probed. Rather than using weak functions and direct calls across subsystemss, it is often easier to use an event.
-An event consists of a type (e.g. EVT_DM_POST_INIT) and some optional data, +An event consists of a type (e.g. EVT_DM_POST_INIT_F) and some optional data, in `union event_data`. An event spy can be creasted to watch for events of a particular type. When the event is created, it is sent to each spy in turn.
@@ -26,9 +26,9 @@ To declare a spy, use something like this:: /* do something */ return 0; }
- EVENT_SPY(EVT_DM_POST_INIT, snow_setup_cpus);
- EVENT_SPY(EVT_DM_POST_INIT_F, snow_setup_cpus);
-Your function is called when EVT_DM_POST_INIT is emitted, i.e. after driver +Your function is called when EVT_DM_POST_INIT_F is emitted, i.e. after driver model is inited (in SPL, or in U-Boot proper before and after relocation).
diff --git a/drivers/core/root.c b/drivers/core/root.c index c4fb48548bb..6775fb0b657 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -436,8 +436,8 @@ int dm_init_and_scan(bool pre_reloc_only) return ret; } }
- if (CONFIG_IS_ENABLED(DM_EVENT)) {
ret = event_notify_null(EVT_DM_POST_INIT);
- if (CONFIG_IS_ENABLED(DM_EVENT) && !(gd->flags & GD_FLG_RELOC)) {
if (ret) return log_msg_ret("ev", ret); }ret = event_notify_null(EVT_DM_POST_INIT_F);
diff --git a/drivers/cpu/microblaze_cpu.c b/drivers/cpu/microblaze_cpu.c index b9d07928223..c97a89fbd5c 100644 --- a/drivers/cpu/microblaze_cpu.c +++ b/drivers/cpu/microblaze_cpu.c @@ -29,7 +29,7 @@ static int microblaze_cpu_probe_all(void *ctx, struct event *event)
return 0; } -EVENT_SPY(EVT_DM_POST_INIT, microblaze_cpu_probe_all); +EVENT_SPY(EVT_DM_POST_INIT_F, microblaze_cpu_probe_all);
static void microblaze_set_cpuinfo_pvr(struct microblaze_cpuinfo *ci) { diff --git a/include/event.h b/include/event.h index e4580b68350..fe41080fa63 100644 --- a/include/event.h +++ b/include/event.h @@ -22,7 +22,7 @@ enum event_t { EVT_TEST,
/* Events related to driver model */
- EVT_DM_POST_INIT,
- EVT_DM_POST_INIT_F, EVT_DM_PRE_PROBE, EVT_DM_POST_PROBE, EVT_DM_PRE_REMOVE,
This broke the VisonFive2:
... U-Boot 2023.07-rc2-00058-g55171aedda8 (Jun 04 2023 - 14:00:57 +0200)
CPU: rv64imafdc_zba_zbb Model: StarFive VisionFive 2 v1.3B DRAM: 8 GiB initcall sequence 00000000fffe0b10 failed at call 00000000402160bc (err=-19) ### ERROR ### Please RESET the board ###
Already known?
Jan

Hi Jan,
On Sun, 4 Jun 2023 at 13:05, Jan Kiszka jan.kiszka@siemens.com wrote:
On 05.05.23 00:50, Simon Glass wrote:
The original function was only called once, before relocation. The new one is called again after relocation. This was not the intent of the original call. Fix this by renaming and updating the calling logic.
With this, chromebook_link64 makes it through SPL.
Fixes: 7fe32b3442f ("event: Convert arch_cpu_init_dm() to") Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events") Reviewed-by: Bin Meng bmeng.cn@gmail.com
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Fix 'intend' typo
arch/arm/mach-imx/imx8/cpu.c | 2 +- arch/arm/mach-imx/imx8m/soc.c | 2 +- arch/arm/mach-imx/imx8ulp/soc.c | 2 +- arch/arm/mach-imx/imx9/soc.c | 2 +- arch/arm/mach-omap2/am33xx/board.c | 2 +- arch/arm/mach-omap2/hwinit-common.c | 2 +- arch/mips/mach-pic32/cpu.c | 2 +- arch/nios2/cpu/cpu.c | 2 +- arch/riscv/cpu/cpu.c | 2 +- arch/x86/cpu/baytrail/cpu.c | 2 +- arch/x86/cpu/broadwell/cpu.c | 2 +- arch/x86/cpu/ivybridge/cpu.c | 2 +- arch/x86/cpu/quark/quark.c | 2 +- arch/x86/lib/fsp2/fsp_init.c | 2 +- doc/develop/event.rst | 6 +++--- drivers/core/root.c | 4 ++-- drivers/cpu/microblaze_cpu.c | 2 +- include/event.h | 2 +- 18 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/arch/arm/mach-imx/imx8/cpu.c b/arch/arm/mach-imx/imx8/cpu.c index be1f4edded1..99772f68c32 100644 --- a/arch/arm/mach-imx/imx8/cpu.c +++ b/arch/arm/mach-imx/imx8/cpu.c @@ -89,7 +89,7 @@ static int imx8_init_mu(void *ctx, struct event *event)
return 0;
} -EVENT_SPY(EVT_DM_POST_INIT, imx8_init_mu); +EVENT_SPY(EVT_DM_POST_INIT_F, imx8_init_mu);
#if defined(CONFIG_ARCH_MISC_INIT) int arch_misc_init(void) diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index 4705e6c1192..5a4f8358c9f 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -549,7 +549,7 @@ static int imx8m_check_clock(void *ctx, struct event *event)
return 0;
} -EVENT_SPY(EVT_DM_POST_INIT, imx8m_check_clock); +EVENT_SPY(EVT_DM_POST_INIT_F, imx8m_check_clock);
static void imx8m_setup_snvs(void) { diff --git a/arch/arm/mach-imx/imx8ulp/soc.c b/arch/arm/mach-imx/imx8ulp/soc.c index 8424332f429..81eae02b6a8 100644 --- a/arch/arm/mach-imx/imx8ulp/soc.c +++ b/arch/arm/mach-imx/imx8ulp/soc.c @@ -808,7 +808,7 @@ static int imx8ulp_evt_dm_post_init(void *ctx, struct event *event) { return imx8ulp_dm_post_init(); } -EVENT_SPY(EVT_DM_POST_INIT, imx8ulp_evt_dm_post_init); +EVENT_SPY(EVT_DM_POST_INIT_F, imx8ulp_evt_dm_post_init);
#if defined(CONFIG_SPL_BUILD) __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) diff --git a/arch/arm/mach-imx/imx9/soc.c b/arch/arm/mach-imx/imx9/soc.c index a16e22ea6bb..252663a9eec 100644 --- a/arch/arm/mach-imx/imx9/soc.c +++ b/arch/arm/mach-imx/imx9/soc.c @@ -262,7 +262,7 @@ int imx9_probe_mu(void *ctx, struct event *event)
return 0;
} -EVENT_SPY(EVT_DM_POST_INIT, imx9_probe_mu); +EVENT_SPY(EVT_DM_POST_INIT_F, imx9_probe_mu);
int timer_init(void) { diff --git a/arch/arm/mach-omap2/am33xx/board.c b/arch/arm/mach-omap2/am33xx/board.c index a52d04d85c8..ecc0a592e99 100644 --- a/arch/arm/mach-omap2/am33xx/board.c +++ b/arch/arm/mach-omap2/am33xx/board.c @@ -535,4 +535,4 @@ static int am33xx_dm_post_init(void *ctx, struct event *event) #endif return 0; } -EVENT_SPY(EVT_DM_POST_INIT, am33xx_dm_post_init); +EVENT_SPY(EVT_DM_POST_INIT_F, am33xx_dm_post_init); diff --git a/arch/arm/mach-omap2/hwinit-common.c b/arch/arm/mach-omap2/hwinit-common.c index c4a8eabc3eb..771533394bc 100644 --- a/arch/arm/mach-omap2/hwinit-common.c +++ b/arch/arm/mach-omap2/hwinit-common.c @@ -246,7 +246,7 @@ static int omap2_system_init(void *ctx, struct event *event)
return 0;
} -EVENT_SPY(EVT_DM_POST_INIT, omap2_system_init); +EVENT_SPY(EVT_DM_POST_INIT_F, omap2_system_init);
/*
- Routine: wait_for_command_complete
diff --git a/arch/mips/mach-pic32/cpu.c b/arch/mips/mach-pic32/cpu.c index de449e3c6a2..ec3c2505313 100644 --- a/arch/mips/mach-pic32/cpu.c +++ b/arch/mips/mach-pic32/cpu.c @@ -102,7 +102,7 @@ static int pic32_flash_prefetch(void *ctx, struct event *event) prefetch_init(); return 0; } -EVENT_SPY(EVT_DM_POST_INIT, pic32_flash_prefetch); +EVENT_SPY(EVT_DM_POST_INIT_F, pic32_flash_prefetch);
/* Un-gate DDR2 modules (gated by default) */ static void ddr2_pmd_ungate(void) diff --git a/arch/nios2/cpu/cpu.c b/arch/nios2/cpu/cpu.c index 85544503a5e..da167f4b29e 100644 --- a/arch/nios2/cpu/cpu.c +++ b/arch/nios2/cpu/cpu.c @@ -80,7 +80,7 @@ static int nios_cpu_setup(void *ctx, struct event *event)
return 0;
} -EVENT_SPY(EVT_DM_POST_INIT, nios_cpu_setup); +EVENT_SPY(EVT_DM_POST_INIT_F, nios_cpu_setup);
static int altera_nios2_get_desc(const struct udevice *dev, char *buf, int size) diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c index e1ed4ec01d0..ecfb1fb08c4 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -145,7 +145,7 @@ int riscv_cpu_setup(void *ctx, struct event *event)
return 0;
} -EVENT_SPY(EVT_DM_POST_INIT, riscv_cpu_setup); +EVENT_SPY(EVT_DM_POST_INIT_F, riscv_cpu_setup);
int arch_early_init_r(void) { diff --git a/arch/x86/cpu/baytrail/cpu.c b/arch/x86/cpu/baytrail/cpu.c index 4fb6a485542..4a7b4f617f8 100644 --- a/arch/x86/cpu/baytrail/cpu.c +++ b/arch/x86/cpu/baytrail/cpu.c @@ -64,7 +64,7 @@ static int baytrail_uart_init(void *ctx, struct event *event)
return 0;
} -EVENT_SPY(EVT_DM_POST_INIT, baytrail_uart_init); +EVENT_SPY(EVT_DM_POST_INIT_F, baytrail_uart_init);
static void set_max_freq(void) { diff --git a/arch/x86/cpu/broadwell/cpu.c b/arch/x86/cpu/broadwell/cpu.c index 7877961451a..f30aebfe4c6 100644 --- a/arch/x86/cpu/broadwell/cpu.c +++ b/arch/x86/cpu/broadwell/cpu.c @@ -40,7 +40,7 @@ static int broadwell_init_cpu(void *ctx, struct event *event)
return 0;
} -EVENT_SPY(EVT_DM_POST_INIT, broadwell_init_cpu); +EVENT_SPY(EVT_DM_POST_INIT_F, broadwell_init_cpu);
void set_max_freq(void) { diff --git a/arch/x86/cpu/ivybridge/cpu.c b/arch/x86/cpu/ivybridge/cpu.c index cffc5d5b1d8..c988d7ff477 100644 --- a/arch/x86/cpu/ivybridge/cpu.c +++ b/arch/x86/cpu/ivybridge/cpu.c @@ -86,7 +86,7 @@ static int ivybridge_cpu_init(void *ctx, struct event *ev)
return 0;
} -EVENT_SPY(EVT_DM_POST_INIT, ivybridge_cpu_init); +EVENT_SPY(EVT_DM_POST_INIT_F, ivybridge_cpu_init);
#define PCH_EHCI0_TEMP_BAR0 0xe8000000 #define PCH_EHCI1_TEMP_BAR0 0xe8000400 diff --git a/arch/x86/cpu/quark/quark.c b/arch/x86/cpu/quark/quark.c index 0a1fbb34d40..1be8e38cdf4 100644 --- a/arch/x86/cpu/quark/quark.c +++ b/arch/x86/cpu/quark/quark.c @@ -263,7 +263,7 @@ static int quark_init_pcie(void *ctx, struct event *event)
return 0;
} -EVENT_SPY(EVT_DM_POST_INIT, quark_init_pcie); +EVENT_SPY(EVT_DM_POST_INIT_F, quark_init_pcie);
int checkcpu(void) { diff --git a/arch/x86/lib/fsp2/fsp_init.c b/arch/x86/lib/fsp2/fsp_init.c index b15926e8247..afec7d08d67 100644 --- a/arch/x86/lib/fsp2/fsp_init.c +++ b/arch/x86/lib/fsp2/fsp_init.c @@ -42,7 +42,7 @@ int fsp_setup_pinctrl(void *ctx, struct event *event)
return ret;
} -EVENT_SPY(EVT_DM_POST_INIT, fsp_setup_pinctrl); +EVENT_SPY(EVT_DM_POST_INIT_F, fsp_setup_pinctrl);
#if !defined(CONFIG_TPL_BUILD) binman_sym_declare(ulong, intel_fsp_m, image_pos); diff --git a/doc/develop/event.rst b/doc/develop/event.rst index 4ff59348371..4c34fffc63b 100644 --- a/doc/develop/event.rst +++ b/doc/develop/event.rst @@ -11,7 +11,7 @@ block device is probed. Rather than using weak functions and direct calls across subsystemss, it is often easier to use an event.
-An event consists of a type (e.g. EVT_DM_POST_INIT) and some optional data, +An event consists of a type (e.g. EVT_DM_POST_INIT_F) and some optional data, in `union event_data`. An event spy can be creasted to watch for events of a particular type. When the event is created, it is sent to each spy in turn.
@@ -26,9 +26,9 @@ To declare a spy, use something like this:: /* do something */ return 0; }
- EVENT_SPY(EVT_DM_POST_INIT, snow_setup_cpus);
- EVENT_SPY(EVT_DM_POST_INIT_F, snow_setup_cpus);
-Your function is called when EVT_DM_POST_INIT is emitted, i.e. after driver +Your function is called when EVT_DM_POST_INIT_F is emitted, i.e. after driver model is inited (in SPL, or in U-Boot proper before and after relocation).
diff --git a/drivers/core/root.c b/drivers/core/root.c index c4fb48548bb..6775fb0b657 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -436,8 +436,8 @@ int dm_init_and_scan(bool pre_reloc_only) return ret; } }
if (CONFIG_IS_ENABLED(DM_EVENT)) {
ret = event_notify_null(EVT_DM_POST_INIT);
if (CONFIG_IS_ENABLED(DM_EVENT) && !(gd->flags & GD_FLG_RELOC)) {
ret = event_notify_null(EVT_DM_POST_INIT_F); if (ret) return log_msg_ret("ev", ret); }
diff --git a/drivers/cpu/microblaze_cpu.c b/drivers/cpu/microblaze_cpu.c index b9d07928223..c97a89fbd5c 100644 --- a/drivers/cpu/microblaze_cpu.c +++ b/drivers/cpu/microblaze_cpu.c @@ -29,7 +29,7 @@ static int microblaze_cpu_probe_all(void *ctx, struct event *event)
return 0;
} -EVENT_SPY(EVT_DM_POST_INIT, microblaze_cpu_probe_all); +EVENT_SPY(EVT_DM_POST_INIT_F, microblaze_cpu_probe_all);
static void microblaze_set_cpuinfo_pvr(struct microblaze_cpuinfo *ci) { diff --git a/include/event.h b/include/event.h index e4580b68350..fe41080fa63 100644 --- a/include/event.h +++ b/include/event.h @@ -22,7 +22,7 @@ enum event_t { EVT_TEST,
/* Events related to driver model */
EVT_DM_POST_INIT,
EVT_DM_POST_INIT_F, EVT_DM_PRE_PROBE, EVT_DM_POST_PROBE, EVT_DM_PRE_REMOVE,
This broke the VisonFive2:
... U-Boot 2023.07-rc2-00058-g55171aedda8 (Jun 04 2023 - 14:00:57 +0200)
CPU: rv64imafdc_zba_zbb Model: StarFive VisionFive 2 v1.3B DRAM: 8 GiB initcall sequence 00000000fffe0b10 failed at call 00000000402160bc (err=-19) ### ERROR ### Please RESET the board ###
Already known?
Yes there was a thread about it. I am not sure why, though. The change dropped calling this function after relocation, but it is still called before relocation.
Actually, perhaps: - before relocation the CPU devices are not bound (no bootph-pre-ram tags) - after relocation it is no-longer called - so it breaks?
The fix could be to add a new EVT_DM_POST_INIT_R event?
Regards, Simon

Allow symbol writing in these cases so that U-Boot can find the position and size of U-Boot at runtime.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
(no changes since v1)
tools/binman/etype/u_boot_spl_with_ucode_ptr.py | 2 +- tools/binman/etype/u_boot_tpl_with_ucode_ptr.py | 2 +- tools/binman/etype/u_boot_with_ucode_ptr.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/binman/etype/u_boot_spl_with_ucode_ptr.py b/tools/binman/etype/u_boot_spl_with_ucode_ptr.py index 72739a5eb67..18b99b00f4a 100644 --- a/tools/binman/etype/u_boot_spl_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_spl_with_ucode_ptr.py @@ -18,7 +18,7 @@ class Entry_u_boot_spl_with_ucode_ptr(Entry_u_boot_with_ucode_ptr): process. """ def __init__(self, section, etype, node): - super().__init__(section, etype, node) + super().__init__(section, etype, node, auto_write_symbols=True) self.elf_fname = 'spl/u-boot-spl'
def GetDefaultFilename(self): diff --git a/tools/binman/etype/u_boot_tpl_with_ucode_ptr.py b/tools/binman/etype/u_boot_tpl_with_ucode_ptr.py index 86f9578b714..f8cc22011ce 100644 --- a/tools/binman/etype/u_boot_tpl_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_tpl_with_ucode_ptr.py @@ -20,7 +20,7 @@ class Entry_u_boot_tpl_with_ucode_ptr(Entry_u_boot_with_ucode_ptr): process. """ def __init__(self, section, etype, node): - super().__init__(section, etype, node) + super().__init__(section, etype, node, auto_write_symbols=True) self.elf_fname = 'tpl/u-boot-tpl'
def GetDefaultFilename(self): diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py index 41731fd0e13..aab27ac8ee7 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -28,8 +28,8 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): microcode, to allow early x86 boot code to find it without doing anything complicated. Otherwise it is the same as the u-boot entry. """ - def __init__(self, section, etype, node): - super().__init__(section, etype, node) + def __init__(self, section, etype, node, auto_write_symbols=False): + super().__init__(section, etype, node, auto_write_symbols) self.elf_fname = 'u-boot' self.target_offset = None

With tiny SPI flash the erasesize is 0 which can cause a divide-by-zero error. Check for this and return a proper error instead.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: - Fix 'returns' typo
drivers/mtd/spi/sf_probe.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index e192f97efdc..de6516f1065 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -189,7 +189,8 @@ static int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len) struct mtd_info *mtd = &flash->mtd; struct erase_info instr;
- if (offset % mtd->erasesize || len % mtd->erasesize) { + if (!mtd->erasesize || + (offset % mtd->erasesize || len % mtd->erasesize)) { debug("SF: Erase offset/length not multiple of erase size\n"); return -EINVAL; }

The 'tiny' SPI nor functions have the same name as their big brothers, which can be confusing. Use different names so it is clear which version is in the image.
Signed-off-by: Simon Glass sjg@chromium.org Acked-by: Vignesh Raghavendra vigneshr@ti.com Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
(no changes since v1)
drivers/mtd/spi/spi-nor-tiny.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/spi/spi-nor-tiny.c b/drivers/mtd/spi/spi-nor-tiny.c index 68152ce3b4b..7aa24e129f9 100644 --- a/drivers/mtd/spi/spi-nor-tiny.c +++ b/drivers/mtd/spi/spi-nor-tiny.c @@ -361,7 +361,7 @@ static int spi_nor_wait_till_ready(struct spi_nor *nor) * Erase an address range on the nor chip. The address range may extend * one or more erase sectors. Return an error is there is a problem erasing. */ -static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) +static int spi_nor_erase_tiny(struct mtd_info *mtd, struct erase_info *instr) { return -ENOTSUPP; } @@ -390,8 +390,8 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor) return ERR_PTR(-EMEDIUMTYPE); }
-static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len, - size_t *retlen, u_char *buf) +static int spi_nor_read_tiny(struct mtd_info *mtd, loff_t from, size_t len, + size_t *retlen, u_char *buf) { struct spi_nor *nor = mtd_to_spi_nor(mtd); int ret; @@ -426,8 +426,8 @@ read_err: * FLASH_PAGESIZE chunks. The address range may be any size provided * it is within the physical boundaries. */ -static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len, - size_t *retlen, const u_char *buf) +static int spi_nor_write_tiny(struct mtd_info *mtd, loff_t to, size_t len, + size_t *retlen, const u_char *buf) { return -ENOTSUPP; } @@ -741,9 +741,9 @@ int spi_nor_scan(struct spi_nor *nor) mtd->writesize = 1; mtd->flags = MTD_CAP_NORFLASH; mtd->size = info->sector_size * info->n_sectors; - mtd->_erase = spi_nor_erase; - mtd->_read = spi_nor_read; - mtd->_write = spi_nor_write; + mtd->_erase = spi_nor_erase_tiny; + mtd->_read = spi_nor_read_tiny; + mtd->_write = spi_nor_write_tiny;
nor->size = mtd->size;

The bd82x6x_get_gpio_base() does not work if the LPC is not set up. Probe it early to avoid this problem.
In chromebook_link64 this problem shows up as an inability to read the GPIO straps for the memory type.
Also, probing LPC can cause PCI enumeration to take place, which significantly increases pre-relocation memory usage. LPC is sometimes enabled directly by SPL.
Adjust the logic to probe the LPC only after relocation. This allows chromebook_link64 to start up without a much larger CONFIG_SYS_MALLOC_F_LEN value.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: - Fix 'somtimes' typo - Fix 'propblem' typo - Squash in patch 'Set up LPC only after relocation'
Changes in v2: - Add new patch to set up LPC only after relocation
arch/x86/cpu/ivybridge/bd82x6x.c | 17 +++++++++-------- drivers/sysreset/sysreset_x86.c | 9 +++++++-- 2 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/arch/x86/cpu/ivybridge/bd82x6x.c b/arch/x86/cpu/ivybridge/bd82x6x.c index 89312a86349..417290f559e 100644 --- a/arch/x86/cpu/ivybridge/bd82x6x.c +++ b/arch/x86/cpu/ivybridge/bd82x6x.c @@ -31,7 +31,6 @@ DECLARE_GLOBAL_DATA_PTR; #define RCBA_AUDIO_CONFIG_HDA BIT(31) #define RCBA_AUDIO_CONFIG_MASK 0xfe
-#ifndef CONFIG_HAVE_FSP static int pch_revision_id = -1; static int pch_type = -1;
@@ -162,15 +161,19 @@ void pch_iobp_update(struct udevice *dev, u32 address, u32 andvalue,
static int bd82x6x_probe(struct udevice *dev) { - if (!(gd->flags & GD_FLG_RELOC)) - return 0; + /* make sure the LPC is inited since it provides the gpio base */ + uclass_first_device(UCLASS_LPC, &dev); + + if (!IS_ENABLED(CONFIG_HAVE_FSP)) { + if (!(gd->flags & GD_FLG_RELOC)) + return 0;
- /* Cause the SATA device to do its init */ - uclass_first_device(UCLASS_AHCI, &dev); + /* Cause the SATA device to do its init */ + uclass_first_device(UCLASS_AHCI, &dev); + }
return 0; } -#endif /* CONFIG_HAVE_FSP */
static int bd82x6x_pch_get_spi_base(struct udevice *dev, ulong *sbasep) { @@ -269,8 +272,6 @@ U_BOOT_DRIVER(bd82x6x_drv) = { .name = "bd82x6x", .id = UCLASS_PCH, .of_match = bd82x6x_ids, -#ifndef CONFIG_HAVE_FSP .probe = bd82x6x_probe, -#endif .ops = &bd82x6x_pch_ops, }; diff --git a/drivers/sysreset/sysreset_x86.c b/drivers/sysreset/sysreset_x86.c index 8042f3994fe..4936fdb76c7 100644 --- a/drivers/sysreset/sysreset_x86.c +++ b/drivers/sysreset/sysreset_x86.c @@ -129,8 +129,13 @@ static int x86_sysreset_probe(struct udevice *dev) { struct x86_sysreset_plat *plat = dev_get_plat(dev);
- /* Locate the PCH if there is one. It isn't essential */ - uclass_first_device(UCLASS_PCH, &plat->pch); + /* + * Locate the PCH if there is one. It isn't essential. Avoid this before + * relocation as we shouldn't need reset then and it needs a lot of + * memory for PCI enumeration. + */ + if (gd->flags & GD_FLG_RELOC) + uclass_first_device(UCLASS_PCH, &plat->pch);
return 0; }

This adds a lot of code so that it cannot be built with the binary blobs. It is not used on this board. Drop it.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: - Fix 'build' typo
configs/chromebook_samus_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/chromebook_samus_defconfig b/configs/chromebook_samus_defconfig index b933a2352e3..0d20891d2bc 100644 --- a/configs/chromebook_samus_defconfig +++ b/configs/chromebook_samus_defconfig @@ -84,3 +84,4 @@ CONFIG_FRAMEBUFFER_SET_VESA_MODE=y CONFIG_FRAMEBUFFER_VESA_MODE_11A=y CONFIG_TPM=y # CONFIG_GZIP is not set +# CONFIG_EFI_LOADER is not set

The debug UART is already set up in SPL, so there is no need to do anything here. We must provide the (empty) function though.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
(no changes since v1)
arch/x86/cpu/x86_64/cpu.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/x86/cpu/x86_64/cpu.c b/arch/x86/cpu/x86_64/cpu.c index 6a387612916..d1c3873dd6a 100644 --- a/arch/x86/cpu/x86_64/cpu.c +++ b/arch/x86/cpu/x86_64/cpu.c @@ -50,3 +50,10 @@ int x86_cpu_init_f(void) { return 0; } + +#ifdef CONFIG_DEBUG_UART_BOARD_INIT +void board_debug_uart_init(void) +{ + /* this was already done in SPL */ +} +#endif

For now, just enable the fast-but-large string functions in 32-boot U-Boot proper only. Avoid using them in SPL. We cannot use then in 64-bit builds since we only have 32-bit assembly.
Reviewed-by: Bin Meng bmeng.cn@gmail.com Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: - Fix 'boot' typo
arch/x86/include/asm/string.h | 6 +++++- arch/x86/lib/Makefile | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/string.h b/arch/x86/include/asm/string.h index c15b264a5c0..5c49b0f009b 100644 --- a/arch/x86/include/asm/string.h +++ b/arch/x86/include/asm/string.h @@ -14,7 +14,11 @@ extern char *strrchr(const char *s, int c); #undef __HAVE_ARCH_STRCHR extern char *strchr(const char *s, int c);
-#ifdef CONFIG_X86_64 +/* + * Our assembly routines do not work on in 64-bit mode and we don't do a lot of + * copying in SPL, so code size is more important there. + */ +#if defined(CONFIG_SPL_BUILD) || !IS_ENABLED(CONFIG_X86_32BIT_INIT)
#undef __HAVE_ARCH_MEMCPY extern void *memcpy(void *, const void *, __kernel_size_t); diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile index a6f22441474..b0612ae6dd5 100644 --- a/arch/x86/lib/Makefile +++ b/arch/x86/lib/Makefile @@ -10,7 +10,9 @@ obj-y += bios.o obj-y += bios_asm.o obj-y += bios_interrupts.o endif -obj-y += string.o +endif +ifndef CONFIG_SPL_BUILD +obj-$(CONFIG_X86_32BIT_INIT) += string.o endif ifndef CONFIG_SPL_BUILD obj-$(CONFIG_CMD_BOOTM) += bootm.o

SPL printf() does not normally support %#x so just use %x instead. Hex is expected in U-Boot anyway.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
(no changes since v1)
arch/x86/lib/mrccache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/lib/mrccache.c b/arch/x86/lib/mrccache.c index 38632e513fc..2f6f6880003 100644 --- a/arch/x86/lib/mrccache.c +++ b/arch/x86/lib/mrccache.c @@ -303,7 +303,7 @@ static int mrccache_save_type(enum mrc_type_t type) mrc = &gd->arch.mrc[type]; if (!mrc->len) return 0; - log_debug("Saving %#x bytes of MRC output data type %d to SPI flash\n", + log_debug("Saving %x bytes of MRC output data type %d to SPI flash\n", mrc->len, type); ret = mrccache_get_region(type, &sf, &entry); if (ret)

Show the area of memory cleared for BSS, when debugging is enabled.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: - Drop change to memset()
arch/x86/lib/spl.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c index bdf57ef7b5b..5e47ffa7db7 100644 --- a/arch/x86/lib/spl.c +++ b/arch/x86/lib/spl.c @@ -117,6 +117,8 @@ static int x86_spl_init(void) }
#ifndef CONFIG_SYS_COREBOOT + debug("BSS clear from %lx to %lx len %lx\n", (ulong)&__bss_start, + (ulong)&__bss_end, (ulong)&__bss_end - (ulong)&__bss_start); memset(&__bss_start, 0, (ulong)&__bss_end - (ulong)&__bss_start); # ifndef CONFIG_TPL

On Fri, May 5, 2023 at 6:51 AM Simon Glass sjg@chromium.org wrote:
Show the area of memory cleared for BSS, when debugging is enabled.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Drop change to memset()
arch/x86/lib/spl.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Use the binman symbols for this, to avoid hard-coding the value. We could use CONFIG_X86_OFFSET_U_BOOT for the address, but it seems better to obtain the offset and size through the same mechanism.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
(no changes since v2)
Changes in v2: - Add new patch to tidy up address for loading U-Boot from SPL
arch/x86/lib/spl.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c index 5e47ffa7db7..479889aec6f 100644 --- a/arch/x86/lib/spl.c +++ b/arch/x86/lib/spl.c @@ -217,16 +217,9 @@ static int spl_board_load_image(struct spl_image_info *spl_image, spl_image->name = "U-Boot";
if (!IS_ENABLED(CONFIG_SYS_COREBOOT)) { - /* - * Copy U-Boot from ROM - * TODO(sjg@chromium.org): Figure out a way to get the text base - * correctly here, and in the device-tree binman definition. - * - * Also consider using FIT so we get the correct image length - * and parameters. - */ - memcpy((char *)spl_image->load_addr, (char *)0xfff00000, - 0x100000); + /* Copy U-Boot from ROM */ + memcpy((void *)spl_image->load_addr, + (void *)spl_get_image_pos(), spl_get_image_size()); }
debug("Loading to %lx\n", spl_image->load_addr);

This function used to be for adding a list of requests to be actioned on relocation. Revert it back to this purpose, to avoid problems with boards which need control of their MTRRs (i.e. those which don't use FSP).
The mtrr_set_next_var() function is available when the next free variable-MTRR must be set, so this can be used instead.
Signed-off-by: Simon Glass sjg@chromium.org Fixes: 3bcd6cf89ef ("x86: mtrr: Skip MSRs that were already programmed..") Fixes: 596bd0589ad ("x86: mtrr: Do not clear the unused ones..") ---
Changes in v3: - Fix invalid commit IDs obtained from another branch
arch/x86/cpu/mtrr.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c index e69dfb552b1..c174dd9b3ad 100644 --- a/arch/x86/cpu/mtrr.c +++ b/arch/x86/cpu/mtrr.c @@ -156,8 +156,12 @@ int mtrr_commit(bool do_caches) debug("open done\n"); qsort(req, gd->arch.mtrr_req_count, sizeof(*req), h_comp_mtrr); for (i = 0; i < gd->arch.mtrr_req_count; i++, req++) - mtrr_set_next_var(req->type, req->start, req->size); + set_var_mtrr(i, req->type, req->start, req->size);
+ /* Clear the ones that are unused */ + debug("clear\n"); + for (; i < mtrr_get_var_count(); i++) + wrmsrl(MTRR_PHYS_MASK_MSR(i), 0); debug("close\n"); mtrr_close(&state, do_caches); debug("mtrr done\n");

Hi Simon,
On Fri, May 5, 2023 at 6:51 AM Simon Glass sjg@chromium.org wrote:
This function used to be for adding a list of requests to be actioned on relocation. Revert it back to this purpose, to avoid problems with boards which need control of their MTRRs (i.e. those which don't use FSP).
The mtrr_set_next_var() function is available when the next free variable-MTRR must be set, so this can be used instead.
Signed-off-by: Simon Glass sjg@chromium.org Fixes: 3bcd6cf89ef ("x86: mtrr: Skip MSRs that were already programmed..") Fixes: 596bd0589ad ("x86: mtrr: Do not clear the unused ones..")
Changes in v3:
- Fix invalid commit IDs obtained from another branch
arch/x86/cpu/mtrr.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c index e69dfb552b1..c174dd9b3ad 100644 --- a/arch/x86/cpu/mtrr.c +++ b/arch/x86/cpu/mtrr.c @@ -156,8 +156,12 @@ int mtrr_commit(bool do_caches) debug("open done\n"); qsort(req, gd->arch.mtrr_req_count, sizeof(*req), h_comp_mtrr); for (i = 0; i < gd->arch.mtrr_req_count; i++, req++)
mtrr_set_next_var(req->type, req->start, req->size);
set_var_mtrr(i, req->type, req->start, req->size);
This change actually reverts 3bcd6cf89ef ("x86: mtrr: Skip MSRs that were already programmed.."), but as 3bcd6cf89ef commit message says:
At present mtrr_commit() programs the MTRR MSRs starting from index 0, which may overwrite MSRs that were already programmed by previous boot stage or FSP.
So this change will cause regression.
/* Clear the ones that are unused */
debug("clear\n");
for (; i < mtrr_get_var_count(); i++)
wrmsrl(MTRR_PHYS_MASK_MSR(i), 0);
This change actually reverts 596bd0589ad ("x86: mtrr: Do not clear the unused ones.."), which will also create regression unfortunately..
debug("close\n"); mtrr_close(&state, do_caches); debug("mtrr done\n");
--
Regards, Bin

On Mon, May 8, 2023 at 10:48 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Fri, May 5, 2023 at 6:51 AM Simon Glass sjg@chromium.org wrote:
This function used to be for adding a list of requests to be actioned on relocation. Revert it back to this purpose, to avoid problems with boards which need control of their MTRRs (i.e. those which don't use FSP).
The mtrr_set_next_var() function is available when the next free variable-MTRR must be set, so this can be used instead.
Signed-off-by: Simon Glass sjg@chromium.org Fixes: 3bcd6cf89ef ("x86: mtrr: Skip MSRs that were already programmed..") Fixes: 596bd0589ad ("x86: mtrr: Do not clear the unused ones..")
Changes in v3:
- Fix invalid commit IDs obtained from another branch
arch/x86/cpu/mtrr.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c index e69dfb552b1..c174dd9b3ad 100644 --- a/arch/x86/cpu/mtrr.c +++ b/arch/x86/cpu/mtrr.c @@ -156,8 +156,12 @@ int mtrr_commit(bool do_caches) debug("open done\n"); qsort(req, gd->arch.mtrr_req_count, sizeof(*req), h_comp_mtrr); for (i = 0; i < gd->arch.mtrr_req_count; i++, req++)
mtrr_set_next_var(req->type, req->start, req->size);
set_var_mtrr(i, req->type, req->start, req->size);
This change actually reverts 3bcd6cf89ef ("x86: mtrr: Skip MSRs that were already programmed.."), but as 3bcd6cf89ef commit message says:
At present mtrr_commit() programs the MTRR MSRs starting from index 0, which may overwrite MSRs that were already programmed by previous boot stage or FSP.
So this change will cause regression.
/* Clear the ones that are unused */
debug("clear\n");
for (; i < mtrr_get_var_count(); i++)
wrmsrl(MTRR_PHYS_MASK_MSR(i), 0);
This change actually reverts 596bd0589ad ("x86: mtrr: Do not clear the unused ones.."), which will also create regression unfortunately..
debug("close\n"); mtrr_close(&state, do_caches); debug("mtrr done\n");
--
The regression mentioned above will cause Intel Crown Bay fail to boot. See https://lore.kernel.org/u-boot/20210731084529.7524-1-bmeng.cn@gmail.com/
Regards, Bin

Hi Bin,
On Mon, 8 May 2023 at 08:51, Bin Meng bmeng.cn@gmail.com wrote:
On Mon, May 8, 2023 at 10:48 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Fri, May 5, 2023 at 6:51 AM Simon Glass sjg@chromium.org wrote:
This function used to be for adding a list of requests to be actioned on relocation. Revert it back to this purpose, to avoid problems with boards which need control of their MTRRs (i.e. those which don't use FSP).
The mtrr_set_next_var() function is available when the next free variable-MTRR must be set, so this can be used instead.
Signed-off-by: Simon Glass sjg@chromium.org Fixes: 3bcd6cf89ef ("x86: mtrr: Skip MSRs that were already programmed..") Fixes: 596bd0589ad ("x86: mtrr: Do not clear the unused ones..")
Changes in v3:
- Fix invalid commit IDs obtained from another branch
arch/x86/cpu/mtrr.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c index e69dfb552b1..c174dd9b3ad 100644 --- a/arch/x86/cpu/mtrr.c +++ b/arch/x86/cpu/mtrr.c @@ -156,8 +156,12 @@ int mtrr_commit(bool do_caches) debug("open done\n"); qsort(req, gd->arch.mtrr_req_count, sizeof(*req), h_comp_mtrr); for (i = 0; i < gd->arch.mtrr_req_count; i++, req++)
mtrr_set_next_var(req->type, req->start, req->size);
set_var_mtrr(i, req->type, req->start, req->size);
This change actually reverts 3bcd6cf89ef ("x86: mtrr: Skip MSRs that were already programmed.."), but as 3bcd6cf89ef commit message says:
At present mtrr_commit() programs the MTRR MSRs starting from index 0, which may overwrite MSRs that were already programmed by previous boot stage or FSP.
So this change will cause regression.
/* Clear the ones that are unused */
debug("clear\n");
for (; i < mtrr_get_var_count(); i++)
wrmsrl(MTRR_PHYS_MASK_MSR(i), 0);
This change actually reverts 596bd0589ad ("x86: mtrr: Do not clear the unused ones.."), which will also create regression unfortunately..
debug("close\n"); mtrr_close(&state, do_caches); debug("mtrr done\n");
--
The regression mentioned above will cause Intel Crown Bay fail to boot. See https://lore.kernel.org/u-boot/20210731084529.7524-1-bmeng.cn@gmail.com/
Can that board perhaps use the other functoin to set MTRRs? It is mtrr_set_next_var()
The change in the API has broken two of the Chromebooks which is why I am trying to go back to the way it was. Does this board set up the MTRRs in FSP? If so, perhaps we need a Kconfig for that? It is not what the newer FSPs do, but perhaps we could use that behaviour for FSPv1?
Regards, Simon

Hi Simon,
On Tue, May 9, 2023 at 11:08 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Mon, 8 May 2023 at 08:51, Bin Meng bmeng.cn@gmail.com wrote:
On Mon, May 8, 2023 at 10:48 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Fri, May 5, 2023 at 6:51 AM Simon Glass sjg@chromium.org wrote:
This function used to be for adding a list of requests to be actioned on relocation. Revert it back to this purpose, to avoid problems with boards which need control of their MTRRs (i.e. those which don't use FSP).
The mtrr_set_next_var() function is available when the next free variable-MTRR must be set, so this can be used instead.
Signed-off-by: Simon Glass sjg@chromium.org Fixes: 3bcd6cf89ef ("x86: mtrr: Skip MSRs that were already programmed..") Fixes: 596bd0589ad ("x86: mtrr: Do not clear the unused ones..")
Changes in v3:
- Fix invalid commit IDs obtained from another branch
arch/x86/cpu/mtrr.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c index e69dfb552b1..c174dd9b3ad 100644 --- a/arch/x86/cpu/mtrr.c +++ b/arch/x86/cpu/mtrr.c @@ -156,8 +156,12 @@ int mtrr_commit(bool do_caches) debug("open done\n"); qsort(req, gd->arch.mtrr_req_count, sizeof(*req), h_comp_mtrr); for (i = 0; i < gd->arch.mtrr_req_count; i++, req++)
mtrr_set_next_var(req->type, req->start, req->size);
set_var_mtrr(i, req->type, req->start, req->size);
This change actually reverts 3bcd6cf89ef ("x86: mtrr: Skip MSRs that were already programmed.."), but as 3bcd6cf89ef commit message says:
At present mtrr_commit() programs the MTRR MSRs starting from index 0, which may overwrite MSRs that were already programmed by previous boot stage or FSP.
So this change will cause regression.
/* Clear the ones that are unused */
debug("clear\n");
for (; i < mtrr_get_var_count(); i++)
wrmsrl(MTRR_PHYS_MASK_MSR(i), 0);
This change actually reverts 596bd0589ad ("x86: mtrr: Do not clear the unused ones.."), which will also create regression unfortunately..
debug("close\n"); mtrr_close(&state, do_caches); debug("mtrr done\n");
--
The regression mentioned above will cause Intel Crown Bay fail to boot. See https://lore.kernel.org/u-boot/20210731084529.7524-1-bmeng.cn@gmail.com/
Can that board perhaps use the other functoin to set MTRRs? It is mtrr_set_next_var()
mtrr_commit() is called by the following common places:
- arch/x86/lib/init_helpers.c::init_cache_f_r() - arch/x86/lib/spl.c::board_init_f_r() - arch/x86/lib/fsp/fsp_graphics.c::fsp_video_probe() - drivers/video/vesa.c::vesa_video_probe()
The change in the API has broken two of the Chromebooks which is why I am trying to go back to the way it was. Does this board set up the MTRRs in FSP? If so, perhaps we need a Kconfig for that? It is not
Yes, FSPv1 sets up the MTRRs.
what the newer FSPs do, but perhaps we could use that behaviour for FSPv1?
This mtrr_commit() API is overloaded. On some places it is called with caller's intention to initialize MTRRs completely from scratch but on some other places it is called with caller's intention to initialize the next available MTRR. We should make this API usage clear. I will see if I can make a patch.
Regards, Bin

This function is used by U-Boot proper. It does not set up MTRRs when SPL is enabled, but we do want this done when it is called from SPL. In fact it is confusing to use the same function from SPL, since there are quite a few conditions there.
All init_cache_f_r() really does is commit the MTRRs and set up the cache. Do this in the SPL's version of this function instead.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: - Fix 'require' typo
arch/x86/lib/spl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c index 479889aec6f..61eb026c862 100644 --- a/arch/x86/lib/spl.c +++ b/arch/x86/lib/spl.c @@ -186,7 +186,8 @@ void board_init_f(ulong flags)
void board_init_f_r(void) { - init_cache_f_r(); + mtrr_commit(false); + init_cache(); gd->flags &= ~GD_FLG_SERIAL_READY; debug("cache status %d\n", dcache_status()); board_init_r(gd, 0);

We don't need to commit the SPI-flash MTRR change immediately, since it is now done in the board_init_f_r(). Also this causes chromebook_link64 to hang, presumably since we are still running from CAR (Cache-as-RAM) in SPL. Coral handles this OK, perhaps since it is running from a different memory area, but it has no effect on Coral anyway.
Drop the extra mtrr_commit() in the SPL implementation.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: - Add more detail about why it might be hanging
arch/x86/lib/spl.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c index 61eb026c862..ca1645f9d68 100644 --- a/arch/x86/lib/spl.c +++ b/arch/x86/lib/spl.c @@ -147,7 +147,6 @@ static int x86_spl_init(void) debug("%s: SPI cache setup failed (err=%d)\n", __func__, ret); return ret; } - mtrr_commit(true); # else ret = syscon_get_by_driver_data(X86_SYSCON_PUNIT, &punit); if (ret)

On Fri, May 5, 2023 at 6:51 AM Simon Glass sjg@chromium.org wrote:
We don't need to commit the SPI-flash MTRR change immediately, since it is now done in the board_init_f_r(). Also this causes chromebook_link64 to hang, presumably since we are still running from CAR (Cache-as-RAM) in SPL. Coral handles this OK, perhaps since it is running from a different memory area, but it has no effect on Coral anyway.
Drop the extra mtrr_commit() in the SPL implementation.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Add more detail about why it might be hanging
arch/x86/lib/spl.c | 1 - 1 file changed, 1 deletion(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

This copies the cpu_call64() function to memory address and then jumps to it. This seems to work correctly even when called from SPL, which is running from SPI flash.
Drop the copy as it is not needed.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: - Fix 'call' typo
arch/x86/cpu/i386/cpu.c | 32 +++++--------------------------- 1 file changed, 5 insertions(+), 27 deletions(-)
diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c index c7f6c5a013e..91cd5d7c9e4 100644 --- a/arch/x86/cpu/i386/cpu.c +++ b/arch/x86/cpu/i386/cpu.c @@ -572,6 +572,7 @@ int cpu_has_64bit(void) has_long_mode(); }
+/* Base address for page tables used for 64-bit mode */ #define PAGETABLE_BASE 0x80000 #define PAGETABLE_SIZE (6 * 4096)
@@ -614,43 +615,20 @@ int cpu_jump_to_64bit(ulong setup_base, ulong target) }
/* - * Jump from SPL to U-Boot + * cpu_jump_to_64bit_uboot() - Jump from SPL to U-Boot * - * This function is work-in-progress with many issues to resolve. - * - * It works by setting up several regions: - * ptr - a place to put the code that jumps into 64-bit mode - * gdt - a place to put the global descriptor table - * pgtable - a place to put the page tables - * - * The cpu_call64() code is copied from ROM and then manually patched so that - * it has the correct GDT address in RAM. U-Boot is copied from ROM into - * its pre-relocation address. Then we jump to the cpu_call64() code in RAM, - * which changes to 64-bit mode and starts U-Boot. + * It works by setting up page tables and calling the code to enter 64-bit long + * mode */ int cpu_jump_to_64bit_uboot(ulong target) { - typedef void (*func_t)(ulong pgtable, ulong setup_base, ulong target); uint32_t *pgtable; - func_t func; - char *ptr;
pgtable = (uint32_t *)PAGETABLE_BASE; - build_pagetable(pgtable);
- extern long call64_stub_size; - ptr = malloc(call64_stub_size); - if (!ptr) { - printf("Failed to allocate the cpu_call64 stub\n"); - return -ENOMEM; - } - memcpy(ptr, cpu_call64, call64_stub_size); - - func = (func_t)ptr; - /* Jump to U-Boot */ - func((ulong)pgtable, 0, (ulong)target); + cpu_call64(PAGETABLE_BASE, 0, (ulong)target);
return -EFAULT; }

These are not used in TPL so disable the drivers to save space.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
(no changes since v1)
arch/x86/cpu/broadwell/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/cpu/broadwell/Makefile b/arch/x86/cpu/broadwell/Makefile index 52d56c65be8..3e1f76d6118 100644 --- a/arch/x86/cpu/broadwell/Makefile +++ b/arch/x86/cpu/broadwell/Makefile @@ -2,7 +2,6 @@ # # Copyright (c) 2016 Google, Inc
-obj-y += adsp.o obj-$(CONFIG_$(SPL_TPL_)X86_16BIT_INIT) += cpu.o obj-$(CONFIG_$(SPL_TPL_)X86_32BIT_INIT) += cpu_full.o
@@ -14,6 +13,8 @@ obj-y += refcode.o endif ifndef CONFIG_SPL_BUILD # obj-y += cpu_from_spl.o +obj-y += adsp.o +obj-y += sata.o endif endif
@@ -29,5 +30,4 @@ obj-y += pch.o obj-y += pinctrl_broadwell.o obj-y += power_state.o obj-$(CONFIG_$(SPL_TPL_)X86_32BIT_INIT) += refcode.o -obj-y += sata.o obj-$(CONFIG_$(SPL_TPL_)X86_32BIT_INIT) += sdram.o

Move the TPL up a little to make room for the refcode binary blob. Also increase the pre-relocation memory to make space for recent additions.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
(no changes since v2)
Changes in v2: - Drop patch "x86: Add on to existing MTRRs in SPL" - Add various patches to resolve problems with chromebook_link64
configs/chromebook_samus_tpl_defconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/configs/chromebook_samus_tpl_defconfig b/configs/chromebook_samus_tpl_defconfig index 337768b45fd..4cfaf4bc5c7 100644 --- a/configs/chromebook_samus_tpl_defconfig +++ b/configs/chromebook_samus_tpl_defconfig @@ -1,6 +1,6 @@ CONFIG_X86=y CONFIG_TEXT_BASE=0xffed0000 -CONFIG_SYS_MALLOC_F_LEN=0x1a00 +CONFIG_SYS_MALLOC_F_LEN=0x2000 CONFIG_NR_DRAM_BANKS=8 CONFIG_ENV_SIZE=0x1000 CONFIG_ENV_OFFSET=0x3F8000 @@ -8,7 +8,7 @@ CONFIG_ENV_SECT_SIZE=0x1000 CONFIG_SPL_DM_SPI=y CONFIG_DEFAULT_DEVICE_TREE="chromebook_samus" CONFIG_SPL_TEXT_BASE=0xffe70000 -CONFIG_TPL_TEXT_BASE=0xfffd8000 +CONFIG_TPL_TEXT_BASE=0xfffd8100 CONFIG_DEBUG_UART_BASE=0x3f8 CONFIG_DEBUG_UART_CLOCK=1843200 CONFIG_DEBUG_UART_BOARD_INIT=y

Hi Simon,
On Fri, May 5, 2023 at 6:51 AM Simon Glass sjg@chromium.org wrote:
This adds some fixes for x86-based Chromebook builds which have picked up a few problems recently.
With this, chromebook_link/64, chromebook_samus and chromebook_coral work correctly.
Changes in v3:
- Fix 'intend' typo
- Fix 'returns' typo
- Fix 'somtimes' typo
- Fix 'propblem' typo
- Squash in patch 'Set up LPC only after relocation'
- Fix 'build' typo
- Fix 'boot' typo
- Drop change to memset()
- Fix invalid commit IDs obtained from another branch
- Fix 'require' typo
- Add more detail about why it might be hanging
- Fix 'call' typo
Changes in v2:
- Add new patch to set up LPC only after relocation
- Add new patch to tidy up address for loading U-Boot from SPL
- Drop patch "x86: Add on to existing MTRRs in SPL"
- Add various patches to resolve problems with chromebook_link64
I dropped patch #12, "x86: Return mtrr_add_request() to its old purpose" as it introduced a regression.
The rest of the patches are applied to u-boot-x86, thanks!
Regards, Bin
participants (3)
-
Bin Meng
-
Jan Kiszka
-
Simon Glass