[PATCH v2 0/9] x86: Fixes for chromebook_link64 and chromebook_samus_tpl

These boards have various problems which prevent them from booting. In the case of link there was a recent regression with PCI probing added to SPL. For samus_tpl it needs a few tweaks to get things back up and running.
This series resolves all known issues.
For v2, the 'Avoid starting up PCI automatically in SPL' patch has been replaced with a global_data flag.
Changes in v2: - Add new patch to allow marking driver model as dead - Reword commit message for clarity and to fix typo - Add new patch to mark driver model as dead when disabling CAR
Simon Glass (9): dm: core: Allow marking driver model as dead x86: broadwell: Show the memory delay x86: Add some log categories x86: samus_tpl: Correct text base and alloc sizes x86: spl: Change the condition for copying U-Boot to RAM x86: broadwell: Avoid initing the CPU twice x86: broadwell: Set up MTRRs x86: dm: Mark driver model as dead when disabling CAR x86: doc: Update the list of supported Chromebooks
arch/x86/cpu/broadwell/cpu.c | 10 +++++----- arch/x86/cpu/broadwell/sdram.c | 2 ++ arch/x86/cpu/intel_common/mrc.c | 18 +++++++++++++++++- arch/x86/dts/chromebook_samus.dts | 1 + arch/x86/lib/init_helpers.c | 7 +++---- arch/x86/lib/spl.c | 5 ++++- arch/x86/lib/tpl.c | 2 ++ common/spl/spl.c | 2 +- configs/chromebook_samus_tpl_defconfig | 4 +++- doc/arch/x86.rst | 5 +++-- include/asm-generic/global_data.h | 5 +++++ 11 files changed, 46 insertions(+), 15 deletions(-)

On x86 devices we use CAR (Cache-As-RAM) to hold the malloc() region in SPL, since SDRAM is not set up yet. This means that driver model stores its tables in this region.
When preparing to jump from SPL to U-Boot proper, we must disable CAR, so that the CPU can uses the caches normally. This means that driver model tables become inaccessible. From there until we jump to U-Boot proper, we must avoid using driver model.
This is only a problem on boards which operate this way, for example chromebook_link64
Add a flag to indicate that driver model is dead and should not be used. It can be used in SPL to avoid hanging the machine.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Add new patch to allow marking driver model as dead
common/spl/spl.c | 2 +- include/asm-generic/global_data.h | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index 0062f3f45d9a..045a5e89625d 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -800,7 +800,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2) IS_ENABLED(CONFIG_SPL_ATF)) dram_init_banksize();
- if (CONFIG_IS_ENABLED(PCI)) { + if (CONFIG_IS_ENABLED(PCI) && !(gd->flags & GD_FLG_DM_DEAD)) { ret = pci_init(); if (ret) puts(SPL_TPL_PROMPT "Cannot initialize PCI\n"); diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index 8fc205ded1a3..5b11cb17ac03 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -667,6 +667,11 @@ enum gd_flags { * @GD_FLG_OF_TAG_MIGRATE: Device tree has old u-boot,dm- tags */ GD_FLG_OF_TAG_MIGRATE = 0x200000, + /** + * @GD_FLG_DM_DEAD: Driver model is not accessible. This can be set when + * the memory used to holds its tables has been mapped out. + */ + GD_FLG_DM_DEAD = 0x400000, };
#endif /* __ASSEMBLY__ */

On Thu, Sep 7, 2023 at 11:58 PM Simon Glass sjg@chromium.org wrote:
On x86 devices we use CAR (Cache-As-RAM) to hold the malloc() region in SPL, since SDRAM is not set up yet. This means that driver model stores its tables in this region.
When preparing to jump from SPL to U-Boot proper, we must disable CAR, so that the CPU can uses the caches normally. This means that driver model tables become inaccessible. From there until we jump to U-Boot proper, we must avoid using driver model.
This is only a problem on boards which operate this way, for example chromebook_link64
Add a flag to indicate that driver model is dead and should not be used. It can be used in SPL to avoid hanging the machine.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Add new patch to allow marking driver model as dead
common/spl/spl.c | 2 +- include/asm-generic/global_data.h | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Samus only takes 7 seconds but it is long enough to think it has hung. Add a message about what it is doing, similar to the approach on coral.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
(no changes since v1)
arch/x86/cpu/intel_common/mrc.c | 18 +++++++++++++++++- arch/x86/dts/chromebook_samus.dts | 1 + 2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/intel_common/mrc.c b/arch/x86/cpu/intel_common/mrc.c index 56cc253831aa..ff959d1bd8d8 100644 --- a/arch/x86/cpu/intel_common/mrc.c +++ b/arch/x86/cpu/intel_common/mrc.c @@ -9,6 +9,7 @@ #include <dm.h> #include <init.h> #include <log.h> +#include <spl.h> #include <syscon.h> #include <asm/cpu.h> #include <asm/global_data.h> @@ -251,13 +252,28 @@ static int sdram_initialise(struct udevice *dev, struct udevice *me_dev, int mrc_common_init(struct udevice *dev, void *pei_data, bool use_asm_linkage) { struct udevice *me_dev; - int ret; + int ret, delay;
ret = syscon_get_by_driver_data(X86_SYSCON_ME, &me_dev); if (ret) return ret;
+ delay = dev_read_u32_default(dev, "fspm,training-delay", 0); + if (spl_phase() == PHASE_SPL) { + if (delay) + printf("SDRAM training (%d seconds)...", delay); + else + log_debug("SDRAM init..."); + } else { + if (delay) + printf("(%d seconds)...", delay); + } + ret = sdram_initialise(dev, me_dev, pei_data, use_asm_linkage); + if (delay) + printf("done\n"); + else + log_debug("done\n"); if (ret) return ret; quick_ram_check(); diff --git a/arch/x86/dts/chromebook_samus.dts b/arch/x86/dts/chromebook_samus.dts index 96705ceed074..ddff277046a4 100644 --- a/arch/x86/dts/chromebook_samus.dts +++ b/arch/x86/dts/chromebook_samus.dts @@ -266,6 +266,7 @@ board-id-gpios = <&gpio_c 5 0>, <&gpio_c 4 0>, <&gpio_c 3 0>, <&gpio_c 1 0>; bootph-all; + fspm,training-delay = <7>; spd { #address-cells = <1>; #size-cells = <0>;

Add some missing log categories to a few files.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
(no changes since v1)
arch/x86/cpu/broadwell/sdram.c | 2 ++ arch/x86/lib/tpl.c | 2 ++ 2 files changed, 4 insertions(+)
diff --git a/arch/x86/cpu/broadwell/sdram.c b/arch/x86/cpu/broadwell/sdram.c index f477d513efce..d30ebee021ea 100644 --- a/arch/x86/cpu/broadwell/sdram.c +++ b/arch/x86/cpu/broadwell/sdram.c @@ -5,6 +5,8 @@ * From coreboot src/soc/intel/broadwell/romstage/raminit.c */
+#define LOG_CATEGORY UCLASS_RAM + #include <common.h> #include <dm.h> #include <init.h> diff --git a/arch/x86/lib/tpl.c b/arch/x86/lib/tpl.c index 18b05b2f6720..273e9c8e1ca1 100644 --- a/arch/x86/lib/tpl.c +++ b/arch/x86/lib/tpl.c @@ -3,6 +3,8 @@ * Copyright (c) 2018 Google, Inc */
+#define LOG_CATEGORY LOGC_BOOT + #include <common.h> #include <debug_uart.h> #include <dm.h>

Make sure that CONFIG_X86_OFFSET_U_BOOT is the same as CONFIG_TEXT_BASE as it is changing CONFIG_X86_OFFSET_U_BOOT
Samus boots into U-Boot proper in flash, not RAM. Also expand the SPL malloc() size a little, to avoid an error.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v2: - Reword commit message for clarity and to fix typo
configs/chromebook_samus_tpl_defconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/configs/chromebook_samus_tpl_defconfig b/configs/chromebook_samus_tpl_defconfig index 33ada9fe4f93..9dd29401ef12 100644 --- a/configs/chromebook_samus_tpl_defconfig +++ b/configs/chromebook_samus_tpl_defconfig @@ -1,6 +1,8 @@ CONFIG_X86=y CONFIG_TEXT_BASE=0xffed0000 CONFIG_SYS_MALLOC_F_LEN=0x2000 +CONFIG_TPL_SYS_MALLOC_F_LEN=0x2000 +CONFIG_SPL_SYS_MALLOC_F_LEN=0x3000 CONFIG_NR_DRAM_BANKS=8 CONFIG_ENV_SIZE=0x1000 CONFIG_ENV_OFFSET=0x3F8000 @@ -19,7 +21,7 @@ CONFIG_HAVE_MRC=y CONFIG_HAVE_REFCODE=y CONFIG_SMP=y CONFIG_HAVE_VGA_BIOS=y -CONFIG_X86_OFFSET_U_BOOT=0xffee0000 +CONFIG_X86_OFFSET_U_BOOT=0xffed0000 CONFIG_SYS_MONITOR_BASE=0xFFED0000 CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y

Make this depend on whether the address matches the offset, rather than a particular board build. For samus_tpl we don't need to copy, for example.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
(no changes since v1)
arch/x86/lib/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c index 58fa572b71ae..335dacf47fd0 100644 --- a/arch/x86/lib/spl.c +++ b/arch/x86/lib/spl.c @@ -258,7 +258,7 @@ static int spl_board_load_image(struct spl_image_info *spl_image, spl_image->os = IH_OS_U_BOOT; spl_image->name = "U-Boot";
- if (!IS_ENABLED(CONFIG_SYS_COREBOOT)) { + if (spl_image->load_addr != spl_get_image_pos()) { /* Copy U-Boot from ROM */ memcpy((void *)spl_image->load_addr, (void *)spl_get_image_pos(), spl_get_image_size());

When TPL has already set up the CPU, don't do it again. This existing code actually has this backwards, so fix it.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
(no changes since v1)
arch/x86/cpu/broadwell/cpu.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/cpu/broadwell/cpu.c b/arch/x86/cpu/broadwell/cpu.c index 560b1f7893f6..cbd4a3b67973 100644 --- a/arch/x86/cpu/broadwell/cpu.c +++ b/arch/x86/cpu/broadwell/cpu.c @@ -11,6 +11,7 @@ #include <event.h> #include <init.h> #include <log.h> +#include <spl.h> #include <asm/cpu.h> #include <asm/cpu_x86.h> #include <asm/cpu_common.h> @@ -67,12 +68,11 @@ int arch_cpu_init(void) { post_code(POST_CPU_INIT);
-#ifdef CONFIG_TPL /* Do a mini-init if TPL has already done the full init */ - return x86_cpu_reinit_f(); -#else - return x86_cpu_init_f(); -#endif + if (IS_ENABLED(CONFIG_TPL) && spl_phase() != PHASE_TPL) + return x86_cpu_reinit_f(); + else + return x86_cpu_init_f(); }
int checkcpu(void)

The current condition does not handle the samus_tpl case where it sets up the RAM in SPL but needs to commit the MTRRs in U-Boot proper.
Add another case to handle this and update the comment.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/x86/lib/init_helpers.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/lib/init_helpers.c b/arch/x86/lib/init_helpers.c index 60a2707dcf1b..bf0c921577d1 100644 --- a/arch/x86/lib/init_helpers.c +++ b/arch/x86/lib/init_helpers.c @@ -15,7 +15,8 @@ DECLARE_GLOBAL_DATA_PTR; int init_cache_f_r(void) { bool do_mtrr = CONFIG_IS_ENABLED(X86_32BIT_INIT) || - IS_ENABLED(CONFIG_FSP_VERSION2); + IS_ENABLED(CONFIG_FSP_VERSION2) || + (IS_ENABLED(CONFIG_TPL) && IS_ENABLED(CONFIG_HAVE_MRC)); int ret;
/* @@ -23,11 +24,9 @@ int init_cache_f_r(void) * * booting from slimbootloader - MTRRs are already set up * booting with FSPv1 - MTRRs are already set up - * booting with FSPv2 - MTRRs must be set here + * booting with FSPv2 or MRC - MTRRs must be set here * booting from coreboot - in this case there is no SPL, so we set up * the MTRRs here - * Note: if there is an SPL, then it has already set up MTRRs so we - * don't need to do that here */ do_mtrr &= !IS_ENABLED(CONFIG_FSP_VERSION1) && !IS_ENABLED(CONFIG_SYS_SLIMBOOTLOADER);

On Thu, Sep 7, 2023 at 11:58 PM Simon Glass sjg@chromium.org wrote:
The current condition does not handle the samus_tpl case where it sets up the RAM in SPL but needs to commit the MTRRs in U-Boot proper.
Add another case to handle this and update the comment.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
arch/x86/lib/init_helpers.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

When turning off CAR, set the flag to make sure that nothing tries to use driver model in SPL before jumping to U-Bot proper, since its tables are in CAR.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Add new patch to mark driver model as dead when disabling CAR
arch/x86/lib/spl.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c index 335dacf47fd0..c15f11f8cdf4 100644 --- a/arch/x86/lib/spl.c +++ b/arch/x86/lib/spl.c @@ -230,6 +230,9 @@ void board_init_f_r(void) mtrr_commit(false); init_cache(); gd->flags &= ~GD_FLG_SERIAL_READY; + + /* make sure driver model is not accessed from now on */ + gd->flags |= GD_FLG_DM_DEAD; debug("cache status %d\n", dcache_status()); board_init_r(gd, 0); }

On Thu, Sep 7, 2023 at 11:58 PM Simon Glass sjg@chromium.org wrote:
When turning off CAR, set the flag to make sure that nothing tries to use driver model in SPL before jumping to U-Bot proper, since its tables are in CAR.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Add new patch to mark driver model as dead when disabling CAR
arch/x86/lib/spl.c | 3 +++ 1 file changed, 3 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

One is missing, so add it. Also mention the SoC used in each.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
(no changes since v1)
doc/arch/x86.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/doc/arch/x86.rst b/doc/arch/x86.rst index 725a1ae58639..89d3e7ba0e4b 100644 --- a/doc/arch/x86.rst +++ b/doc/arch/x86.rst @@ -25,12 +25,13 @@ are supported: - Bayley Bay CRB - Cherry Hill CRB - Congatec QEVAL 2.0 & conga-QA3/E3845 + - Coral (Apollo Lake - Chromebook 2017) - Cougar Canyon 2 CRB - Crown Bay CRB - Galileo - - Link (Chromebook Pixel) + - Link (Ivy Bridge - Chromebook Pixel) - Minnowboard MAX - - Samus (Chromebook Pixel 2015) + - Samus (Broadwell - Chromebook Pixel 2015) - QEMU x86 (32-bit & 64-bit)
As for loading an OS, U-Boot supports directly booting a 32-bit or 64-bit

On Thu, Sep 7, 2023 at 11:58 PM Simon Glass sjg@chromium.org wrote:
These boards have various problems which prevent them from booting. In the case of link there was a recent regression with PCI probing added to SPL. For samus_tpl it needs a few tweaks to get things back up and running.
This series resolves all known issues.
For v2, the 'Avoid starting up PCI automatically in SPL' patch has been replaced with a global_data flag.
Changes in v2:
- Add new patch to allow marking driver model as dead
- Reword commit message for clarity and to fix typo
- Add new patch to mark driver model as dead when disabling CAR
Simon Glass (9): dm: core: Allow marking driver model as dead x86: broadwell: Show the memory delay x86: Add some log categories x86: samus_tpl: Correct text base and alloc sizes x86: spl: Change the condition for copying U-Boot to RAM x86: broadwell: Avoid initing the CPU twice x86: broadwell: Set up MTRRs x86: dm: Mark driver model as dead when disabling CAR x86: doc: Update the list of supported Chromebooks
series applied to u-boot-x86/next, thanks!
participants (2)
-
Bin Meng
-
Simon Glass