All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/17] x86: Various fixes for chromebooks
@ 2023-05-04 22:50 Simon Glass
  2023-05-04 22:50 ` [PATCH v3 01/17] dm: Emit the arch_cpu_init_dm() even only before relocation Simon Glass
                   ` (17 more replies)
  0 siblings, 18 replies; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:50 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Bin Meng, Simon Glass, Alper Nebi Yasak, Marek Vasut,
	Mike Frysinger, Neha Malcom Francis, Pavel Herrmann, Peng Fan,
	Philippe Reynes, Stefan Herbrechtsmeier

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(-)

-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v3 01/17] dm: Emit the arch_cpu_init_dm() even only before relocation
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
@ 2023-05-04 22:50 ` Simon Glass
  2023-06-04 12:05   ` Jan Kiszka
  2023-05-04 22:50 ` [PATCH v3 02/17] binman: Support writing symbols for ucode etypes Simon Glass
                   ` (16 subsequent siblings)
  17 siblings, 1 reply; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:50 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass, Marek Vasut, Pavel Herrmann

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,
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 02/17] binman: Support writing symbols for ucode etypes
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
  2023-05-04 22:50 ` [PATCH v3 01/17] dm: Emit the arch_cpu_init_dm() even only before relocation Simon Glass
@ 2023-05-04 22:50 ` Simon Glass
  2023-05-04 22:50 ` [PATCH v3 03/17] sf: Guard against zero erasesize Simon Glass
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:50 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Bin Meng, Simon Glass, Alper Nebi Yasak, Neha Malcom Francis,
	Peng Fan, Philippe Reynes, Stefan Herbrechtsmeier

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
 
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 03/17] sf: Guard against zero erasesize
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
  2023-05-04 22:50 ` [PATCH v3 01/17] dm: Emit the arch_cpu_init_dm() even only before relocation Simon Glass
  2023-05-04 22:50 ` [PATCH v3 02/17] binman: Support writing symbols for ucode etypes Simon Glass
@ 2023-05-04 22:50 ` Simon Glass
  2023-05-04 22:50 ` [PATCH v3 04/17] sf: Rename spi-nor-tiny functions Simon Glass
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:50 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass, Mike Frysinger

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;
 	}
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 04/17] sf: Rename spi-nor-tiny functions
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
                   ` (2 preceding siblings ...)
  2023-05-04 22:50 ` [PATCH v3 03/17] sf: Guard against zero erasesize Simon Glass
@ 2023-05-04 22:50 ` Simon Glass
  2023-05-04 22:50 ` [PATCH v3 05/17] x86: ivybridge: Ensure LPC is available for GPIO base Simon Glass
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:50 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Bin Meng, Simon Glass, Vignesh Raghavendra, Mike Frysinger

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;
 
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 05/17] x86: ivybridge: Ensure LPC is available for GPIO base
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
                   ` (3 preceding siblings ...)
  2023-05-04 22:50 ` [PATCH v3 04/17] sf: Rename spi-nor-tiny functions Simon Glass
@ 2023-05-04 22:50 ` Simon Glass
  2023-05-04 22:50 ` [PATCH v3 06/17] x86: samus: Drop EFI_LOADER Simon Glass
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:50 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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;
 }
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 06/17] x86: samus: Drop EFI_LOADER
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
                   ` (4 preceding siblings ...)
  2023-05-04 22:50 ` [PATCH v3 05/17] x86: ivybridge: Ensure LPC is available for GPIO base Simon Glass
@ 2023-05-04 22:50 ` Simon Glass
  2023-05-04 22:50 ` [PATCH v3 07/17] x86: Support debug UART in 64-bit mode Simon Glass
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:50 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 07/17] x86: Support debug UART in 64-bit mode
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
                   ` (5 preceding siblings ...)
  2023-05-04 22:50 ` [PATCH v3 06/17] x86: samus: Drop EFI_LOADER Simon Glass
@ 2023-05-04 22:50 ` Simon Glass
  2023-05-04 22:50 ` [PATCH v3 08/17] x86: Tidy up availability of string functions Simon Glass
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:50 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 08/17] x86: Tidy up availability of string functions
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
                   ` (6 preceding siblings ...)
  2023-05-04 22:50 ` [PATCH v3 07/17] x86: Support debug UART in 64-bit mode Simon Glass
@ 2023-05-04 22:50 ` Simon Glass
  2023-05-04 22:50 ` [PATCH v3 09/17] x86: mrc: Correct SPL debug message Simon Glass
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:50 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 09/17] x86: mrc: Correct SPL debug message
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
                   ` (7 preceding siblings ...)
  2023-05-04 22:50 ` [PATCH v3 08/17] x86: Tidy up availability of string functions Simon Glass
@ 2023-05-04 22:50 ` Simon Glass
  2023-05-04 22:50 ` [PATCH v3 10/17] x86: spl: Show debugging for BSS Simon Glass
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:50 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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)
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 10/17] x86: spl: Show debugging for BSS
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
                   ` (8 preceding siblings ...)
  2023-05-04 22:50 ` [PATCH v3 09/17] x86: mrc: Correct SPL debug message Simon Glass
@ 2023-05-04 22:50 ` Simon Glass
  2023-05-08  3:34   ` Bin Meng
  2023-05-04 22:50 ` [PATCH v3 11/17] x86: Tidy up address for loading U-Boot from SPL Simon Glass
                   ` (7 subsequent siblings)
  17 siblings, 1 reply; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:50 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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
 
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 11/17] x86: Tidy up address for loading U-Boot from SPL
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
                   ` (9 preceding siblings ...)
  2023-05-04 22:50 ` [PATCH v3 10/17] x86: spl: Show debugging for BSS Simon Glass
@ 2023-05-04 22:50 ` Simon Glass
  2023-05-04 22:50 ` [PATCH v3 12/17] x86: Return mtrr_add_request() to its old purpose Simon Glass
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:50 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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);
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 12/17] x86: Return mtrr_add_request() to its old purpose
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
                   ` (10 preceding siblings ...)
  2023-05-04 22:50 ` [PATCH v3 11/17] x86: Tidy up address for loading U-Boot from SPL Simon Glass
@ 2023-05-04 22:50 ` Simon Glass
  2023-05-08 14:48   ` Bin Meng
  2023-05-04 22:50 ` [PATCH v3 13/17] x86: spl: Avoid using init_cache_f_r() from SPL Simon Glass
                   ` (5 subsequent siblings)
  17 siblings, 1 reply; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:50 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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");
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 13/17] x86: spl: Avoid using init_cache_f_r() from SPL
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
                   ` (11 preceding siblings ...)
  2023-05-04 22:50 ` [PATCH v3 12/17] x86: Return mtrr_add_request() to its old purpose Simon Glass
@ 2023-05-04 22:50 ` Simon Glass
  2023-05-04 22:50 ` [PATCH v3 14/17] spl: Commit MTRRs only in board_init_f_r() Simon Glass
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:50 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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);
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 14/17] spl: Commit MTRRs only in board_init_f_r()
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
                   ` (12 preceding siblings ...)
  2023-05-04 22:50 ` [PATCH v3 13/17] x86: spl: Avoid using init_cache_f_r() from SPL Simon Glass
@ 2023-05-04 22:50 ` Simon Glass
  2023-05-08 14:53   ` Bin Meng
  2023-05-04 22:50 ` [PATCH v3 15/17] x86: Simplify cpu_jump_to_64bit_uboot() Simon Glass
                   ` (3 subsequent siblings)
  17 siblings, 1 reply; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:50 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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)
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 15/17] x86: Simplify cpu_jump_to_64bit_uboot()
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
                   ` (13 preceding siblings ...)
  2023-05-04 22:50 ` [PATCH v3 14/17] spl: Commit MTRRs only in board_init_f_r() Simon Glass
@ 2023-05-04 22:50 ` Simon Glass
  2023-05-04 22:51 ` [PATCH v3 16/17] x86: samus: Don't include audio and SATA in TPL Simon Glass
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:50 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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;
 }
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 16/17] x86: samus: Don't include audio and SATA in TPL
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
                   ` (14 preceding siblings ...)
  2023-05-04 22:50 ` [PATCH v3 15/17] x86: Simplify cpu_jump_to_64bit_uboot() Simon Glass
@ 2023-05-04 22:51 ` Simon Glass
  2023-05-04 22:51 ` [PATCH v3 17/17] x86: samus: Adjust TPL start and pre-reloc memory size Simon Glass
  2023-05-09  8:47 ` [PATCH v3 00/17] x86: Various fixes for chromebooks Bin Meng
  17 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:51 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 17/17] x86: samus: Adjust TPL start and pre-reloc memory size
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
                   ` (15 preceding siblings ...)
  2023-05-04 22:51 ` [PATCH v3 16/17] x86: samus: Don't include audio and SATA in TPL Simon Glass
@ 2023-05-04 22:51 ` Simon Glass
  2023-05-09  8:47 ` [PATCH v3 00/17] x86: Various fixes for chromebooks Bin Meng
  17 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2023-05-04 22:51 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Bin Meng, Simon Glass

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
-- 
2.40.1.521.gf1e218fcd8-goog


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 10/17] x86: spl: Show debugging for BSS
  2023-05-04 22:50 ` [PATCH v3 10/17] x86: spl: Show debugging for BSS Simon Glass
@ 2023-05-08  3:34   ` Bin Meng
  0 siblings, 0 replies; 27+ messages in thread
From: Bin Meng @ 2023-05-08  3:34 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 12/17] x86: Return mtrr_add_request() to its old purpose
  2023-05-04 22:50 ` [PATCH v3 12/17] x86: Return mtrr_add_request() to its old purpose Simon Glass
@ 2023-05-08 14:48   ` Bin Meng
  2023-05-08 14:51     ` Bin Meng
  0 siblings, 1 reply; 27+ messages in thread
From: Bin Meng @ 2023-05-08 14:48 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 12/17] x86: Return mtrr_add_request() to its old purpose
  2023-05-08 14:48   ` Bin Meng
@ 2023-05-08 14:51     ` Bin Meng
  2023-05-09  3:08       ` Simon Glass
  0 siblings, 1 reply; 27+ messages in thread
From: Bin Meng @ 2023-05-08 14:51 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 14/17] spl: Commit MTRRs only in board_init_f_r()
  2023-05-04 22:50 ` [PATCH v3 14/17] spl: Commit MTRRs only in board_init_f_r() Simon Glass
@ 2023-05-08 14:53   ` Bin Meng
  0 siblings, 0 replies; 27+ messages in thread
From: Bin Meng @ 2023-05-08 14:53 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 12/17] x86: Return mtrr_add_request() to its old purpose
  2023-05-08 14:51     ` Bin Meng
@ 2023-05-09  3:08       ` Simon Glass
  2023-05-15  2:56         ` Bin Meng
  0 siblings, 1 reply; 27+ messages in thread
From: Simon Glass @ 2023-05-09  3:08 UTC (permalink / raw)
  To: Bin Meng; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 00/17] x86: Various fixes for chromebooks
  2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
                   ` (16 preceding siblings ...)
  2023-05-04 22:51 ` [PATCH v3 17/17] x86: samus: Adjust TPL start and pre-reloc memory size Simon Glass
@ 2023-05-09  8:47 ` Bin Meng
  17 siblings, 0 replies; 27+ messages in thread
From: Bin Meng @ 2023-05-09  8:47 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Alper Nebi Yasak, Marek Vasut,
	Mike Frysinger, Neha Malcom Francis, Pavel Herrmann, Peng Fan,
	Philippe Reynes, Stefan Herbrechtsmeier

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 12/17] x86: Return mtrr_add_request() to its old purpose
  2023-05-09  3:08       ` Simon Glass
@ 2023-05-15  2:56         ` Bin Meng
  0 siblings, 0 replies; 27+ messages in thread
From: Bin Meng @ 2023-05-15  2:56 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 01/17] dm: Emit the arch_cpu_init_dm() even only before relocation
  2023-05-04 22:50 ` [PATCH v3 01/17] dm: Emit the arch_cpu_init_dm() even only before relocation Simon Glass
@ 2023-06-04 12:05   ` Jan Kiszka
  2023-06-12 21:17     ` Simon Glass
  0 siblings, 1 reply; 27+ messages in thread
From: Jan Kiszka @ 2023-06-04 12:05 UTC (permalink / raw)
  To: Simon Glass, U-Boot Mailing List, Yanhong Wang
  Cc: Bin Meng, Marek Vasut, Pavel Herrmann

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?

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 01/17] dm: Emit the arch_cpu_init_dm() even only before relocation
  2023-06-04 12:05   ` Jan Kiszka
@ 2023-06-12 21:17     ` Simon Glass
  0 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2023-06-12 21:17 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: U-Boot Mailing List, Yanhong Wang, Bin Meng, Marek Vasut, Pavel Herrmann

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2023-06-12 21:18 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-04 22:50 [PATCH v3 00/17] x86: Various fixes for chromebooks Simon Glass
2023-05-04 22:50 ` [PATCH v3 01/17] dm: Emit the arch_cpu_init_dm() even only before relocation Simon Glass
2023-06-04 12:05   ` Jan Kiszka
2023-06-12 21:17     ` Simon Glass
2023-05-04 22:50 ` [PATCH v3 02/17] binman: Support writing symbols for ucode etypes Simon Glass
2023-05-04 22:50 ` [PATCH v3 03/17] sf: Guard against zero erasesize Simon Glass
2023-05-04 22:50 ` [PATCH v3 04/17] sf: Rename spi-nor-tiny functions Simon Glass
2023-05-04 22:50 ` [PATCH v3 05/17] x86: ivybridge: Ensure LPC is available for GPIO base Simon Glass
2023-05-04 22:50 ` [PATCH v3 06/17] x86: samus: Drop EFI_LOADER Simon Glass
2023-05-04 22:50 ` [PATCH v3 07/17] x86: Support debug UART in 64-bit mode Simon Glass
2023-05-04 22:50 ` [PATCH v3 08/17] x86: Tidy up availability of string functions Simon Glass
2023-05-04 22:50 ` [PATCH v3 09/17] x86: mrc: Correct SPL debug message Simon Glass
2023-05-04 22:50 ` [PATCH v3 10/17] x86: spl: Show debugging for BSS Simon Glass
2023-05-08  3:34   ` Bin Meng
2023-05-04 22:50 ` [PATCH v3 11/17] x86: Tidy up address for loading U-Boot from SPL Simon Glass
2023-05-04 22:50 ` [PATCH v3 12/17] x86: Return mtrr_add_request() to its old purpose Simon Glass
2023-05-08 14:48   ` Bin Meng
2023-05-08 14:51     ` Bin Meng
2023-05-09  3:08       ` Simon Glass
2023-05-15  2:56         ` Bin Meng
2023-05-04 22:50 ` [PATCH v3 13/17] x86: spl: Avoid using init_cache_f_r() from SPL Simon Glass
2023-05-04 22:50 ` [PATCH v3 14/17] spl: Commit MTRRs only in board_init_f_r() Simon Glass
2023-05-08 14:53   ` Bin Meng
2023-05-04 22:50 ` [PATCH v3 15/17] x86: Simplify cpu_jump_to_64bit_uboot() Simon Glass
2023-05-04 22:51 ` [PATCH v3 16/17] x86: samus: Don't include audio and SATA in TPL Simon Glass
2023-05-04 22:51 ` [PATCH v3 17/17] x86: samus: Adjust TPL start and pre-reloc memory size Simon Glass
2023-05-09  8:47 ` [PATCH v3 00/17] x86: Various fixes for chromebooks Bin Meng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.