[PATCH 0/8] 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.
Simon Glass (8): 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 spl: x86: Avoid starting up PCI automatically in SPL 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 | 2 +- arch/x86/lib/tpl.c | 2 ++ common/spl/spl.c | 2 +- configs/chromebook_samus_tpl_defconfig | 4 +++- doc/arch/x86.rst | 5 +++-- 10 files changed, 38 insertions(+), 15 deletions(-)
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 ---
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 56cc253831a..ff959d1bd8d 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 96705ceed07..ddff277046a 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>;
On Thu, Aug 24, 2023 at 2:47 AM Simon Glass sjg@chromium.org wrote:
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
arch/x86/cpu/intel_common/mrc.c | 18 +++++++++++++++++- arch/x86/dts/chromebook_samus.dts | 1 + 2 files changed, 18 insertions(+), 1 deletion(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Add some missing log categories to a few files.
Signed-off-by: Simon Glass sjg@chromium.org ---
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 f477d513efc..d30ebee021e 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 18b05b2f672..273e9c8e1ca 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>
On Thu, Aug 24, 2023 at 2:47 AM Simon Glass sjg@chromium.org wrote:
Add some missing log categories to a few files.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/cpu/broadwell/sdram.c | 2 ++ arch/x86/lib/tpl.c | 2 ++ 2 files changed, 4 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Make sure that CONFIG_TEXT_BASE is the same as CONFIG_X86_OFFSET_U_BOOT since this boards boots into U-Boot proper in flash, not RAM. Also exapnd the SPL malloc() size a little, to avoid an error.
Signed-off-by: Simon Glass sjg@chromium.org ---
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 4cfaf4bc5c7..6fefe2ddc25 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
Hi Simon,
On Thu, Aug 24, 2023 at 2:48 AM Simon Glass sjg@chromium.org wrote:
Make sure that CONFIG_TEXT_BASE is the same as CONFIG_X86_OFFSET_U_BOOT
The commit message should say:
Make sure that CONFIG_X86_OFFSET_U_BOOT is the same as CONFIG_TEXT_BASE as it is changing CONFIG_X86_OFFSET_U_BOOT.
since this boards boots into U-Boot proper in flash, not RAM. Also exapnd
s/boards/board
typo: expand
the SPL malloc() size a little, to avoid an error.
Signed-off-by: Simon Glass sjg@chromium.org
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 4cfaf4bc5c7..6fefe2ddc25 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 --
Otherwise, Reviewed-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin
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 ---
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 58fa572b71a..335dacf47fd 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());
On Thu, Aug 24, 2023 at 2:48 AM Simon Glass sjg@chromium.org wrote:
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
arch/x86/lib/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
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 ---
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 f30aebfe4c6..70faa1ca474 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)
On Thu, Aug 24, 2023 at 2:48 AM Simon Glass sjg@chromium.org wrote:
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
arch/x86/cpu/broadwell/cpu.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
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 ---
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 60a2707dcf1..bf0c921577d 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);
Hi Simon,
On Thu, Aug 24, 2023 at 2:48 AM 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
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 60a2707dcf1..bf0c921577d 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));
So this combination is only for Samus TPL case, not a generic one, right?
I hate this as it's easy to break. Maybe we should be calling an API from the board specific codes to determine whether we should do the MTRR programming. Thoughts?
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);--
Regards, Bin
Hi Bin,
On Mon, 4 Sept 2023 at 03:22, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Thu, Aug 24, 2023 at 2:48 AM 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
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 60a2707dcf1..bf0c921577d 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));So this combination is only for Samus TPL case, not a generic one, right?
It is certainly targeted at samus TPL, but in a way it is applicable generically. When TPL is in use, we do the memory init in SPL and pass the RAM into to U-Boot proper, so it sets up the MTRRs. This is the same as the only other TPL board (coral). So samus and coral are in common in this sense...with the different being that samus uses MRC but coral usese FSP2.
I hate this as it's easy to break. Maybe we should be calling an API from the board specific codes to determine whether we should do the MTRR programming. Thoughts?
I am not sure how we would do that. I did suggest a special Kconfig a while back, so that boards can overwrite the setting if needed. But in any case we would want a sensible default for that Kconfig.
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);--
At least until we figure out something better, this seems clear enough.
Regards, Simon
For x86 platforms, PCI is core to their operation and is managed in arch-specific code. Each platform has its own way of doing this. For TPL and some SPL implementations, the full driver model PCI is not used.
A recent change enabled full PCI in TPL/SPL for all boards. This breaks some x86 boards, so adjust it to skip that for x86.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/spl/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index 0062f3f45d9..13d7b1a742f 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) && !IS_ENABLED(CONFIG_X86)) { ret = pci_init(); if (ret) puts(SPL_TPL_PROMPT "Cannot initialize PCI\n");
On 8/23/23 20:47, Simon Glass wrote:
For x86 platforms, PCI is core to their operation and is managed in arch-specific code. Each platform has its own way of doing this. For TPL and some SPL implementations, the full driver model PCI is not used.
A recent change enabled full PCI in TPL/SPL for all boards. This breaks some x86 boards, so adjust it to skip that for x86.
Could you, please, give some more detail?
* Which boards are broken? * Don't these boards have a pci_init() function? * In what way does calling pci_init() lead to a failure?
It would be preferable to have all architectures and boards use the same high level API. Excluding x86 here looks more like a (necessary) hack than like a target state.
Best regards
Heinrich
Signed-off-by: Simon Glass sjg@chromium.org
common/spl/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index 0062f3f45d9..13d7b1a742f 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) && !IS_ENABLED(CONFIG_X86)) { ret = pci_init(); if (ret) puts(SPL_TPL_PROMPT "Cannot initialize PCI\n");
Hi Heinrich,
On Wed, 23 Aug 2023 at 14:19, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 8/23/23 20:47, Simon Glass wrote:
For x86 platforms, PCI is core to their operation and is managed in arch-specific code. Each platform has its own way of doing this. For TPL and some SPL implementations, the full driver model PCI is not used.
A recent change enabled full PCI in TPL/SPL for all boards. This breaks some x86 boards, so adjust it to skip that for x86.
Could you, please, give some more detail?
- Which boards are broken?
For example, chromebook_samus and chromebook_samus_tpl
- Don't these boards have a pci_init() function?
Yes, the same one you are calling.
- In what way does calling pci_init() lead to a failure?
It probes and sets up PCI devices and uses a lot of pre-alloc RAM.
It would be preferable to have all architectures and boards use the same high level API. Excluding x86 here looks more like a (necessary) hack than like a target state.
Fair enough, but on x86 we access PCI long before driver model is up. Generally we don't fully enumerate it in SPL as it is expensive. It is also pointless, since U-Boot proper does it again later.
Regards, Simon
Best regards
Heinrich
Signed-off-by: Simon Glass sjg@chromium.org
common/spl/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index 0062f3f45d9..13d7b1a742f 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) && !IS_ENABLED(CONFIG_X86)) { ret = pci_init(); if (ret) puts(SPL_TPL_PROMPT "Cannot initialize PCI\n");
Am 24. August 2023 01:57:55 MESZ schrieb Simon Glass sjg@chromium.org:
Hi Heinrich,
On Wed, 23 Aug 2023 at 14:19, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 8/23/23 20:47, Simon Glass wrote:
For x86 platforms, PCI is core to their operation and is managed in arch-specific code. Each platform has its own way of doing this. For TPL and some SPL implementations, the full driver model PCI is not used.
A recent change enabled full PCI in TPL/SPL for all boards. This breaks some x86 boards, so adjust it to skip that for x86.
Could you, please, give some more detail?
- Which boards are broken?
For example, chromebook_samus and chromebook_samus_tpl
- Don't these boards have a pci_init() function?
Yes, the same one you are calling.
- In what way does calling pci_init() lead to a failure?
It probes and sets up PCI devices and uses a lot of pre-alloc RAM.
It would be preferable to have all architectures and boards use the same high level API. Excluding x86 here looks more like a (necessary) hack than like a target state.
Fair enough, but on x86 we access PCI long before driver model is up. Generally we don't fully enumerate it in SPL as it is expensive. It is also pointless, since U-Boot proper does it again later.
Regards, Simon
Acked-by: Heinrich Schuchardt xypron.glpk@gmx.de
Best regards
Heinrich
Signed-off-by: Simon Glass sjg@chromium.org
common/spl/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index 0062f3f45d9..13d7b1a742f 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) && !IS_ENABLED(CONFIG_X86)) { ret = pci_init(); if (ret) puts(SPL_TPL_PROMPT "Cannot initialize PCI\n");
Hi Simon,
On Thu, Aug 24, 2023 at 10:11 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 24. August 2023 01:57:55 MESZ schrieb Simon Glass sjg@chromium.org:
Hi Heinrich,
On Wed, 23 Aug 2023 at 14:19, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 8/23/23 20:47, Simon Glass wrote:
For x86 platforms, PCI is core to their operation and is managed in arch-specific code. Each platform has its own way of doing this. For TPL and some SPL implementations, the full driver model PCI is not used.
A recent change enabled full PCI in TPL/SPL for all boards. This breaks some x86 boards, so adjust it to skip that for x86.
Could you, please, give some more detail?
- Which boards are broken?
For example, chromebook_samus and chromebook_samus_tpl
- Don't these boards have a pci_init() function?
Yes, the same one you are calling.
- In what way does calling pci_init() lead to a failure?
It probes and sets up PCI devices and uses a lot of pre-alloc RAM.
It would be preferable to have all architectures and boards use the same high level API. Excluding x86 here looks more like a (necessary) hack than like a target state.
Fair enough, but on x86 we access PCI long before driver model is up. Generally we don't fully enumerate it in SPL as it is expensive. It is also pointless, since U-Boot proper does it again later.
Regards, Simon
Acked-by: Heinrich Schuchardt xypron.glpk@gmx.de
To address Heinrich's concern, how about you turn off CONFIG_SPL_PCI in chromebook_samus and chromebook_samus_tpl?
Regards, Bin
Hi Bin,
On Mon, 4 Sept 2023 at 03:20, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Thu, Aug 24, 2023 at 10:11 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 24. August 2023 01:57:55 MESZ schrieb Simon Glass sjg@chromium.org:
Hi Heinrich,
On Wed, 23 Aug 2023 at 14:19, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 8/23/23 20:47, Simon Glass wrote:
For x86 platforms, PCI is core to their operation and is managed in arch-specific code. Each platform has its own way of doing this. For TPL and some SPL implementations, the full driver model PCI is not used.
A recent change enabled full PCI in TPL/SPL for all boards. This breaks some x86 boards, so adjust it to skip that for x86.
Could you, please, give some more detail?
- Which boards are broken?
For example, chromebook_samus and chromebook_samus_tpl
- Don't these boards have a pci_init() function?
Yes, the same one you are calling.
- In what way does calling pci_init() lead to a failure?
It probes and sets up PCI devices and uses a lot of pre-alloc RAM.
It would be preferable to have all architectures and boards use the same high level API. Excluding x86 here looks more like a (necessary) hack than like a target state.
Fair enough, but on x86 we access PCI long before driver model is up. Generally we don't fully enumerate it in SPL as it is expensive. It is also pointless, since U-Boot proper does it again later.
Regards, Simon
Acked-by: Heinrich Schuchardt xypron.glpk@gmx.de
To address Heinrich's concern, how about you turn off CONFIG_SPL_PCI in chromebook_samus and chromebook_samus_tpl?
I dug into this a bit more...the root cause is that we are turning off CAR so the driver model tables are no-longer available. Accessing them causes a hang.
I will send a v2 which adds a flag to indicate this.
Regards, Simon
One is missing, so add it. Also mention the SoC used in each.
Signed-off-by: Simon Glass sjg@chromium.org ---
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 725a1ae5863..89d3e7ba0e4 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, Aug 24, 2023 at 2:48 AM Simon Glass sjg@chromium.org wrote:
One is missing, so add it. Also mention the SoC used in each.
Signed-off-by: Simon Glass sjg@chromium.org
doc/arch/x86.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
participants (3)
-
Bin Meng -
Heinrich Schuchardt -
Simon Glass