[PATCH 1/6] bootstage: Fix 'stacked' typo

[PATCH 1/6] bootstage: Fix 'stacked' typo

[Simon Glass]

This should be 'stashed'. Fix it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/bootstage.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/bootstage.h b/include/bootstage.h
index f507271375..00c85fb86a 100644
--- a/include/bootstage.h
+++ b/include/bootstage.h
@@ -338,7 +338,7 @@ int bootstage_stash(void *base, int size);
  * @param base	Base address of memory buffer
  * @param size	Size of memory buffer (-1 if unknown)
  * @return 0 if unstashed ok, -ENOENT if bootstage info not found, -ENOSPC if
- *	there is not space for read the stacked data, or other error if
+ *	there is not space for read the stashed data, or other error if
  *	something else went wrong
  */
 int bootstage_unstash(const void *base, int size);
-- 
2.27.0.rc0.183.gde8f92d652-goog

[PATCH 2/6] kconfig: Add support for conditional values

[Simon Glass]

At present if an optional Kconfig value needs to be used it must be
bracketed by #ifdef. For example, with this Kconfig setup:

config WIBBLE
	bool "Support wibbles, the world needs more wibbles"

config WIBBLE_ADDR
	hex "Address of the wibble"
	depends on WIBBLE

then the following code must be used:

 #ifdef CONFIG_WIBBLE
 static void handle_wibble(void)
 {
 	int val = CONFIG_WIBBLE_ADDR;

	...
 }
 #endif

 static void init_machine()
 {
 ...
 #ifdef CONFIG_WIBBLE
	handle_wibble();
 #endif
 }

Add a new IF_ENABLED_INT() to help with this. So now it is possible to
write, without #ifdefs:

 static void handle_wibble(void)
 {
        int val = IF_ENABLED_INT(CONFIG_WIBBLE, CONFIG_WIBBLE_ADDR);

	...
 }

 static void init_machine()
 {
 ...
 if (IS_ENABLED(CONFIG_WIBBLE))
	handle_wibble();
 }

The value will be 0 if CONFIG_WIBBLE is not defined, and
CONFIG_WIBBLE_ADDR if it is. This allows us to reduce the use of #ifdef in
the code, ensuring that the compiler still checks the code even if it is
not ultimately used for a particular build.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/linux/kconfig.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index 3a2da738c4..86cd266540 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -79,6 +79,18 @@
  */
 #define CONFIG_VAL(option)  config_val(option)
 
+/* This use a similar mechanism to config_enabled() above */
+#define config_opt_enabled(cfg, opt_cfg) _config_opt_enabled(cfg, opt_cfg)
+#define _config_opt_enabled(cfg_val, opt_value) \
+	__config_opt_enabled(__ARG_PLACEHOLDER_##cfg_val, opt_value)
+#define __config_opt_enabled(arg1_or_junk, arg2) \
+	___config_opt_enabled(arg1_or_junk arg2, 0)
+#define ___config_opt_enabled(__ignored, val, ...) val
+
+/* Evaluates to 0 if option is not defined, int_option if it is defined */
+#define IF_ENABLED_INT(option, int_option) \
+	config_opt_enabled(option, int_option)
+
 /*
  * CONFIG_IS_ENABLED(FOO) evaluates to
  *  1 if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y' or 'm',
-- 
2.27.0.rc0.183.gde8f92d652-goog

[PATCH 3/6] timer: Allow delays with a 32-bit microsecond timer

[Simon Glass]

The current get_timer_us() uses 64-bit arithmetic. When implementing
microsecond-level timeouts, 32-bits is plenty. Add a new function to
support this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/time.h | 11 +++++++++++
 lib/time.c     |  5 +++++
 2 files changed, 16 insertions(+)

diff --git a/include/time.h b/include/time.h
index e99f9c8012..434e63b075 100644
--- a/include/time.h
+++ b/include/time.h
@@ -17,6 +17,17 @@ unsigned long get_timer(unsigned long base);
 unsigned long timer_get_us(void);
 uint64_t get_timer_us(uint64_t base);
 
+/**
+ * get_timer_us_long() - Get the number of elapsed microseconds
+ *
+ * This uses 32-bit arithmetic on 32-bit machines, which is enough to handle
+ * delays of over an hour.
+ *
+ *@base: Base time to consider
+ *@return elapsed time since @base
+ */
+unsigned long get_timer_us_long(unsigned long base);
+
 /*
  * timer_test_add_offset()
  *
diff --git a/lib/time.c b/lib/time.c
index 65db0f6cda..47f8c84327 100644
--- a/lib/time.c
+++ b/lib/time.c
@@ -152,6 +152,11 @@ uint64_t __weak get_timer_us(uint64_t base)
 	return tick_to_time_us(get_ticks()) - base;
 }
 
+unsigned long __weak get_timer_us_long(unsigned long base)
+{
+	return timer_get_us() - base;
+}
+
 unsigned long __weak notrace timer_get_us(void)
 {
 	return tick_to_time(get_ticks() * 1000);
-- 
2.27.0.rc0.183.gde8f92d652-goog

[PATCH 4/6] coral: Enable the copy framebuffer

[Simon Glass]

Enable this feature on chromebook_coral to speed up the display.

With this change, the time taken to print the environment to the display
without CONFIG_CONSOLE_SCROLL_LINES is reduced from 1830ms to 62ms.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 configs/chromebook_coral_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/chromebook_coral_defconfig b/configs/chromebook_coral_defconfig
index 2039ea6186..0bcdbea8d6 100644
--- a/configs/chromebook_coral_defconfig
+++ b/configs/chromebook_coral_defconfig
@@ -93,6 +93,7 @@ CONFIG_TPM2_CR50_I2C=y
 CONFIG_USB_XHCI_HCD=y
 CONFIG_USB_STORAGE=y
 CONFIG_USB_KEYBOARD=y
+CONFIG_VIDEO_COPY=y
 CONFIG_SPL_FS_CBFS=y
 # CONFIG_SPL_USE_TINY_PRINTF is not set
 CONFIG_TPL_USE_TINY_PRINTF=y
-- 
2.27.0.rc0.183.gde8f92d652-goog

[PATCH 5/6] x86: Avoid #ifdef with CONFIG_HAVE_ACPI_RESUME

[Simon Glass]

At present this enables a few arch-specific members of the global_data
struct which are otherwise not part of the struct. As a result we have to
use #ifdef in various places.

The cost of always having these in the struct is small. Adjust things so
that we can use compile-time code instead of #ifdefs.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/x86/cpu/apollolake/cpu_spl.c    | 13 +++++-----
 arch/x86/cpu/apollolake/fsp_s.c      | 10 ++++----
 arch/x86/cpu/baytrail/acpi.c         |  2 --
 arch/x86/cpu/broadwell/power_state.c |  5 ++--
 arch/x86/cpu/cpu.c                   | 38 ++++++++++++++--------------
 arch/x86/include/asm/global_data.h   |  2 --
 arch/x86/lib/coreboot_table.c        |  6 ++---
 arch/x86/lib/fsp/fsp_common.c        |  2 --
 arch/x86/lib/fsp/fsp_dram.c          | 26 +++++++++++--------
 arch/x86/lib/fsp1/fsp_common.c       | 16 +++++++-----
 arch/x86/lib/fsp2/fsp_dram.c         |  7 +++--
 11 files changed, 63 insertions(+), 64 deletions(-)

diff --git a/arch/x86/cpu/apollolake/cpu_spl.c b/arch/x86/cpu/apollolake/cpu_spl.c
index 707ceb3e64..9f32f2e27e 100644
--- a/arch/x86/cpu/apollolake/cpu_spl.c
+++ b/arch/x86/cpu/apollolake/cpu_spl.c
@@ -247,12 +247,13 @@ static int arch_cpu_init_spl(void)
 	ret = pmc_init(pmc);
 	if (ret < 0)
 		return log_msg_ret("Could not init PMC", ret);
-#ifdef CONFIG_HAVE_ACPI_RESUME
-	ret = pmc_prev_sleep_state(pmc);
-	if (ret < 0)
-		return log_msg_ret("Could not get PMC sleep state", ret);
-	gd->arch.prev_sleep_state = ret;
-#endif
+	if (IS_ENABLED(CONFIG_HAVE_ACPI_RESUME)) {
+		ret = pmc_prev_sleep_state(pmc);
+		if (ret < 0)
+			return log_msg_ret("Could not get PMC sleep state",
+					   ret);
+		gd->arch.prev_sleep_state = ret;
+	}
 
 	return 0;
 }
diff --git a/arch/x86/cpu/apollolake/fsp_s.c b/arch/x86/cpu/apollolake/fsp_s.c
index c5c953f2f8..76cadbe1ca 100644
--- a/arch/x86/cpu/apollolake/fsp_s.c
+++ b/arch/x86/cpu/apollolake/fsp_s.c
@@ -192,16 +192,16 @@ int arch_fsps_preinit(void)
 
 int arch_fsp_init_r(void)
 {
-#ifdef CONFIG_HAVE_ACPI_RESUME
-	bool s3wake = gd->arch.prev_sleep_state == ACPI_S3;
-#else
-	bool s3wake = false;
-#endif
+	bool s3wake;
 	struct udevice *dev, *itss;
 	int ret;
 
 	if (!ll_boot_init())
 		return 0;
+
+	s3wake = IS_ENABLED(CONFIG_HAVE_ACPI_RESUME) &&
+		gd->arch.prev_sleep_state == ACPI_S3;
+
 	/*
 	 * This must be called before any devices are probed. Put any probing
 	 * into arch_fsps_preinit() above.
diff --git a/arch/x86/cpu/baytrail/acpi.c b/arch/x86/cpu/baytrail/acpi.c
index 65f2006a0a..b17bc62a2d 100644
--- a/arch/x86/cpu/baytrail/acpi.c
+++ b/arch/x86/cpu/baytrail/acpi.c
@@ -161,7 +161,6 @@ void acpi_create_gnvs(struct acpi_global_nvs *gnvs)
 		gnvs->iuart_en = 0;
 }
 
-#ifdef CONFIG_HAVE_ACPI_RESUME
 /*
  * The following two routines are called at a very early stage, even before
  * FSP 2nd phase API fsp_init() is called. Registers off ACPI_BASE_ADDRESS
@@ -204,4 +203,3 @@ void chipset_clear_sleep_state(void)
 	pm1_cnt = inl(ACPI_BASE_ADDRESS + PM1_CNT);
 	outl(pm1_cnt & ~(SLP_TYP), ACPI_BASE_ADDRESS + PM1_CNT);
 }
-#endif
diff --git a/arch/x86/cpu/broadwell/power_state.c b/arch/x86/cpu/broadwell/power_state.c
index 99d6f72cf6..62fd2e8d2c 100644
--- a/arch/x86/cpu/broadwell/power_state.c
+++ b/arch/x86/cpu/broadwell/power_state.c
@@ -23,11 +23,10 @@ static int prev_sleep_state(struct chipset_power_state *ps)
 
 	if (ps->pm1_sts & WAK_STS) {
 		switch ((ps->pm1_cnt & SLP_TYP) >> SLP_TYP_SHIFT) {
-#if CONFIG_HAVE_ACPI_RESUME
 		case SLP_TYP_S3:
-			prev_sleep_state = SLEEP_STATE_S3;
+			if (IS_ENABLED(CONFIG_HAVE_ACPI_RESUME))
+				prev_sleep_state = SLEEP_STATE_S3;
 			break;
-#endif
 		case SLP_TYP_S5:
 			prev_sleep_state = SLEEP_STATE_S5;
 			break;
diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
index a814e7d7a6..23a4d633d2 100644
--- a/arch/x86/cpu/cpu.c
+++ b/arch/x86/cpu/cpu.c
@@ -163,10 +163,10 @@ int default_print_cpuinfo(void)
 	       cpu_has_64bit() ? "x86_64" : "x86",
 	       cpu_vendor_name(gd->arch.x86_vendor), gd->arch.x86_device);
 
-#ifdef CONFIG_HAVE_ACPI_RESUME
-	debug("ACPI previous sleep state: %s\n",
-	      acpi_ss_string(gd->arch.prev_sleep_state));
-#endif
+	if (IS_ENABLED(CONFIG_HAVE_ACPI_RESUME)) {
+		debug("ACPI previous sleep state: %s\n",
+		      acpi_ss_string(gd->arch.prev_sleep_state));
+	}
 
 	return 0;
 }
@@ -191,12 +191,12 @@ int last_stage_init(void)
 
 	board_final_cleanup();
 
-#ifdef CONFIG_HAVE_ACPI_RESUME
-	fadt = acpi_find_fadt();
+	if (IS_ENABLED(CONFIG_HAVE_ACPI_RESUME)) {
+		fadt = acpi_find_fadt();
 
-	if (fadt && gd->arch.prev_sleep_state == ACPI_S3)
-		acpi_resume(fadt);
-#endif
+		if (fadt && gd->arch.prev_sleep_state == ACPI_S3)
+			acpi_resume(fadt);
+	}
 
 	write_tables();
 
@@ -277,17 +277,17 @@ int reserve_arch(void)
 	high_table_reserve();
 #endif
 
-#ifdef CONFIG_HAVE_ACPI_RESUME
-	acpi_s3_reserve();
+	if (IS_ENABLED(CONFIG_HAVE_ACPI_RESUME)) {
+		acpi_s3_reserve();
 
-#ifdef CONFIG_HAVE_FSP
-	/*
-	 * Save stack address to CMOS so that at next S3 boot,
-	 * we can use it as the stack address for fsp_contiue()
-	 */
-	fsp_save_s3_stack();
-#endif /* CONFIG_HAVE_FSP */
-#endif /* CONFIG_HAVE_ACPI_RESUME */
+		if (IS_ENABLED(CONFIG_HAVE_FSP)) {
+			/*
+			 * Save stack address to CMOS so that at next S3 boot,
+			 * we can use it as the stack address for fsp_contiue()
+			 */
+			fsp_save_s3_stack();
+		}
+	}
 
 	return 0;
 }
diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h
index 4aee2f3e8c..0e64c8a46d 100644
--- a/arch/x86/include/asm/global_data.h
+++ b/arch/x86/include/asm/global_data.h
@@ -116,10 +116,8 @@ struct arch_global_data {
 	u32 high_table_ptr;
 	u32 high_table_limit;
 #endif
-#ifdef CONFIG_HAVE_ACPI_RESUME
 	int prev_sleep_state;		/* Previous sleep state ACPI_S0/1../5 */
 	ulong backup_mem;		/* Backup memory address for S3 */
-#endif
 #ifdef CONFIG_FSP_VERSION2
 	struct fsp_header *fsp_s_hdr;	/* Pointer to FSP-S header */
 #endif
diff --git a/arch/x86/lib/coreboot_table.c b/arch/x86/lib/coreboot_table.c
index 331c1b7e5a..6cd3244301 100644
--- a/arch/x86/lib/coreboot_table.c
+++ b/arch/x86/lib/coreboot_table.c
@@ -21,11 +21,11 @@ int high_table_reserve(void)
 	gd->arch.high_table_ptr = gd->start_addr_sp;
 
 	/* clear the memory */
-#ifdef CONFIG_HAVE_ACPI_RESUME
-	if (gd->arch.prev_sleep_state != ACPI_S3)
-#endif
+	if (IS_ENABLED(CONFIG_HAVE_ACPI_RESUME) &&
+	    gd->arch.prev_sleep_state != ACPI_S3) {
 		memset((void *)gd->arch.high_table_ptr, 0,
 		       CONFIG_HIGH_TABLE_SIZE);
+	}
 
 	gd->start_addr_sp &= ~0xf;
 
diff --git a/arch/x86/lib/fsp/fsp_common.c b/arch/x86/lib/fsp/fsp_common.c
index cf32b3e512..8e3082d4c8 100644
--- a/arch/x86/lib/fsp/fsp_common.c
+++ b/arch/x86/lib/fsp/fsp_common.c
@@ -60,7 +60,6 @@ void board_final_cleanup(void)
 		debug("OK\n");
 }
 
-#ifdef CONFIG_HAVE_ACPI_RESUME
 int fsp_save_s3_stack(void)
 {
 	struct udevice *dev;
@@ -84,4 +83,3 @@ int fsp_save_s3_stack(void)
 
 	return 0;
 }
-#endif
diff --git a/arch/x86/lib/fsp/fsp_dram.c b/arch/x86/lib/fsp/fsp_dram.c
index ad5a0f79ad..8f8cb5ede4 100644
--- a/arch/x86/lib/fsp/fsp_dram.c
+++ b/arch/x86/lib/fsp/fsp_dram.c
@@ -117,17 +117,21 @@ unsigned int install_e820_map(unsigned int max_entries,
 	entries[num_entries].type = E820_RESERVED;
 	num_entries++;
 
-#ifdef CONFIG_HAVE_ACPI_RESUME
-	/*
-	 * Everything between U-Boot's stack and ram top needs to be
-	 * reserved in order for ACPI S3 resume to work.
-	 */
-	entries[num_entries].addr = gd->start_addr_sp - CONFIG_STACK_SIZE;
-	entries[num_entries].size = gd->ram_top - gd->start_addr_sp +
-		CONFIG_STACK_SIZE;
-	entries[num_entries].type = E820_RESERVED;
-	num_entries++;
-#endif
+	if (IS_ENABLED(CONFIG_HAVE_ACPI_RESUME)) {
+		ulong stack_size;
+
+		stack_size = IF_ENABLED_INT(CONFIG_HAVE_ACPI_RESUME,
+					    CONFIG_STACK_SIZE);
+		/*
+		 * Everything between U-Boot's stack and ram top needs to be
+		 * reserved in order for ACPI S3 resume to work.
+		 */
+		entries[num_entries].addr = gd->start_addr_sp - stack_size;
+		entries[num_entries].size = gd->ram_top - gd->start_addr_sp +
+			stack_size;
+		entries[num_entries].type = E820_RESERVED;
+		num_entries++;
+	}
 
 	return num_entries;
 }
diff --git a/arch/x86/lib/fsp1/fsp_common.c b/arch/x86/lib/fsp1/fsp_common.c
index 43d32b7abe..da351cf097 100644
--- a/arch/x86/lib/fsp1/fsp_common.c
+++ b/arch/x86/lib/fsp1/fsp_common.c
@@ -46,10 +46,12 @@ int arch_fsp_init(void)
 	void *nvs;
 	int stack = CONFIG_FSP_TEMP_RAM_ADDR;
 	int boot_mode = BOOT_FULL_CONFIG;
-#ifdef CONFIG_HAVE_ACPI_RESUME
-	int prev_sleep_state = chipset_prev_sleep_state();
-	gd->arch.prev_sleep_state = prev_sleep_state;
-#endif
+	int prev_sleep_state;
+
+	if (IS_ENABLED(CONFIG_HAVE_ACPI_RESUME)) {
+		prev_sleep_state = chipset_prev_sleep_state();
+		gd->arch.prev_sleep_state = prev_sleep_state;
+	}
 
 	if (!gd->arch.hob_list) {
 		if (IS_ENABLED(CONFIG_ENABLE_MRC_CACHE))
@@ -57,8 +59,8 @@ int arch_fsp_init(void)
 		else
 			nvs = NULL;
 
-#ifdef CONFIG_HAVE_ACPI_RESUME
-		if (prev_sleep_state == ACPI_S3) {
+		if (IS_ENABLED(CONFIG_HAVE_ACPI_RESUME) &&
+		    prev_sleep_state == ACPI_S3) {
 			if (nvs == NULL) {
 				/* If waking from S3 and no cache then */
 				debug("No MRC cache found in S3 resume path\n");
@@ -79,7 +81,7 @@ int arch_fsp_init(void)
 			stack = cmos_read32(CMOS_FSP_STACK_ADDR);
 			boot_mode = BOOT_ON_S3_RESUME;
 		}
-#endif
+
 		/*
 		 * The first time we enter here, call fsp_init().
 		 * Note the execution does not return to this function,
diff --git a/arch/x86/lib/fsp2/fsp_dram.c b/arch/x86/lib/fsp2/fsp_dram.c
index 1c82b81831..c9f6402e6a 100644
--- a/arch/x86/lib/fsp2/fsp_dram.c
+++ b/arch/x86/lib/fsp2/fsp_dram.c
@@ -27,11 +27,10 @@ int dram_init(void)
 		return 0;
 	}
 	if (spl_phase() == PHASE_SPL) {
-#ifdef CONFIG_HAVE_ACPI_RESUME
-		bool s3wake = gd->arch.prev_sleep_state == ACPI_S3;
-#else
 		bool s3wake = false;
-#endif
+
+		s3wake = IS_ENABLED(CONFIG_HAVE_ACPI_RESUME) &&
+			gd->arch.prev_sleep_state == ACPI_S3;
 
 		ret = fsp_memory_init(s3wake,
 			      IS_ENABLED(CONFIG_APL_BOOT_FROM_FAST_SPI_FLASH));
-- 
2.27.0.rc0.183.gde8f92d652-goog

[PATCH 6/6] x86: fsp: Support a warning message when DRAM init is slow

[Simon Glass]

With DDR4, Intel SOCs take quite a long time to init their memory. During
this time, if the user is watching, it looks like SPL has hung. Add a
message in this case.

This works by adding a return code to fspm_update_config() that indicates
whether MRC data was found and a new property to the device tree.

Also add one more debug message while starting.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/x86/cpu/apollolake/fsp_m.c               | 12 ++++++++--
 arch/x86/dts/chromebook_coral.dts             |  1 +
 arch/x86/include/asm/fsp2/fsp_internal.h      |  3 ++-
 arch/x86/lib/fsp2/fsp_meminit.c               | 24 +++++++++++++++----
 .../fsp/fsp2/apollolake/fsp-m.txt             |  3 +++
 5 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/arch/x86/cpu/apollolake/fsp_m.c b/arch/x86/cpu/apollolake/fsp_m.c
index 1301100cd5..65461d85b8 100644
--- a/arch/x86/cpu/apollolake/fsp_m.c
+++ b/arch/x86/cpu/apollolake/fsp_m.c
@@ -16,10 +16,14 @@ int fspm_update_config(struct udevice *dev, struct fspm_upd *upd)
 {
 	struct fsp_m_config *cfg = &upd->config;
 	struct fspm_arch_upd *arch = &upd->arch;
+	int cache_ret = 0;
 	ofnode node;
+	int ret;
 
 	arch->nvs_buffer_ptr = NULL;
-	prepare_mrc_cache(upd);
+	cache_ret = prepare_mrc_cache(upd);
+	if (cache_ret && cache_ret != -ENOENT)
+		return log_msg_ret("mrc", cache_ret);
 	arch->stack_base = (void *)0xfef96000;
 	arch->boot_loader_tolum_size = 0;
 	arch->boot_mode = FSP_BOOT_WITH_FULL_CONFIGURATION;
@@ -28,7 +32,11 @@ int fspm_update_config(struct udevice *dev, struct fspm_upd *upd)
 	if (!ofnode_valid(node))
 		return log_msg_ret("fsp-m settings", -ENOENT);
 
-	return fsp_m_update_config_from_dtb(node, cfg);
+	ret = fsp_m_update_config_from_dtb(node, cfg);
+	if (ret)
+		return log_msg_ret("dtb", cache_ret);
+
+	return cache_ret;
 }
 
 /*
diff --git a/arch/x86/dts/chromebook_coral.dts b/arch/x86/dts/chromebook_coral.dts
index dea35b73a0..aad12f2c4d 100644
--- a/arch/x86/dts/chromebook_coral.dts
+++ b/arch/x86/dts/chromebook_coral.dts
@@ -117,6 +117,7 @@
 			reg = <0x00000000 0 0 0 0>;
 			compatible = "intel,apl-hostbridge";
 			pciex-region-size = <0x10000000>;
+			fspm,training-delay = <21>;
 			/*
 			 * Parameters used by the FSP-S binary blob. This is
 			 * really unfortunate since these parameters mostly
diff --git a/arch/x86/include/asm/fsp2/fsp_internal.h b/arch/x86/include/asm/fsp2/fsp_internal.h
index f751fbf961..b4a4fbbd84 100644
--- a/arch/x86/include/asm/fsp2/fsp_internal.h
+++ b/arch/x86/include/asm/fsp2/fsp_internal.h
@@ -57,7 +57,8 @@ int arch_fsps_preinit(void);
  *
  * @dev: Hostbridge device containing config
  * @upd: Config data to fill in
- * @return 0 if OK, -ve on error
+ * @return 0 if OK, -ENOENT if OK but no MRC-cache data was found, other -ve on
+ *	error
  */
 int fspm_update_config(struct udevice *dev, struct fspm_upd *upd);
 
diff --git a/arch/x86/lib/fsp2/fsp_meminit.c b/arch/x86/lib/fsp2/fsp_meminit.c
index faf9c29aef..ce0b0aff76 100644
--- a/arch/x86/lib/fsp2/fsp_meminit.c
+++ b/arch/x86/lib/fsp2/fsp_meminit.c
@@ -9,6 +9,7 @@
 #include <common.h>
 #include <binman.h>
 #include <bootstage.h>
+#include <dm.h>
 #include <log.h>
 #include <asm/mrccache.h>
 #include <asm/fsp/fsp_infoheader.h>
@@ -63,8 +64,10 @@ int fsp_memory_init(bool s3wake, bool use_spi_flash)
 	struct fsp_header *hdr;
 	struct hob_header *hob;
 	struct udevice *dev;
+	int delay;
 	int ret;
 
+	log_debug("Locating FSP\n");
 	ret = fsp_locate_fsp(FSP_M, &entry, use_spi_flash, &dev, &hdr, NULL);
 	if (ret)
 		return log_msg_ret("locate FSP", ret);
@@ -76,21 +79,32 @@ int fsp_memory_init(bool s3wake, bool use_spi_flash)
 		return log_msg_ret("Bad UPD signature", -EPERM);
 	memcpy(&upd, fsp_upd, sizeof(upd));
 
+	delay = dev_read_u32_default(dev, "fspm,training-delay", 0);
 	ret = fspm_update_config(dev, &upd);
-	if (ret)
-		return log_msg_ret("Could not setup config", ret);
-
-	debug("SDRAM init...");
+	if (ret) {
+		if (ret != -ENOENT)
+			return log_msg_ret("Could not setup config", ret);
+	} else {
+		delay = 0;
+	}
+
+	if (delay)
+		printf("SDRAM training (%d seconds)...", delay);
+	else
+		log_debug("SDRAM init...");
 	bootstage_start(BOOTSTAGE_ID_ACCUM_FSP_M, "fsp-m");
 	func = (fsp_memory_init_func)(hdr->img_base + hdr->fsp_mem_init);
 	ret = func(&upd, &hob);
 	bootstage_accum(BOOTSTAGE_ID_ACCUM_FSP_M);
 	cpu_reinit_fpu();
+	if (delay)
+		printf("done\n");
+	else
+		log_debug("done\n");
 	if (ret)
 		return log_msg_ret("SDRAM init fail\n", ret);
 
 	gd->arch.hob_list = hob;
-	debug("done\n");
 
 	ret = fspm_done(dev);
 	if (ret)
diff --git a/doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-m.txt b/doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-m.txt
index 647a0862d4..67407fa935 100644
--- a/doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-m.txt
+++ b/doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-m.txt
@@ -17,6 +17,9 @@ values of the FSP-M are used.
 [2] https://github.com/IntelFsp/FSP/tree/master/ApolloLakeFspBinPkg/Docs
 
 Optional properties:
+- fspm,training-delay: Time taken to train DDR memory if there is no cached MRC
+    data, in seconds. This is used to show a message if possible. For Intel APL
+    this is typicaly 21 seconds.
 - fspm,serial-debug-port-address: Debug Serial Port Base address
 - fspm,serial-debug-port-type: Debug Serial Port Type
   0: NONE
-- 
2.27.0.rc0.183.gde8f92d652-goog

[PATCH 2/6] kconfig: Add support for conditional values

[Masahiro Yamada]

On Fri, May 22, 2020 at 11:02 AM Simon Glass <sjg@chromium.org> wrote:
>
> At present if an optional Kconfig value needs to be used it must be
> bracketed by #ifdef. For example, with this Kconfig setup:
>
> config WIBBLE
>         bool "Support wibbles, the world needs more wibbles"
>
> config WIBBLE_ADDR
>         hex "Address of the wibble"
>         depends on WIBBLE




I am not sure if this is a good idea.


If you want to always use CONFIG_WIBBLE_ADDR,
you can get rid of 'depends on WIBBLE'.




> then the following code must be used:
>
>  #ifdef CONFIG_WIBBLE
>  static void handle_wibble(void)
>  {
>         int val = CONFIG_WIBBLE_ADDR;
>
>         ...
>  }
>  #endif
>
>  static void init_machine()
>  {
>  ...
>  #ifdef CONFIG_WIBBLE
>         handle_wibble();
>  #endif
>  }
>
> Add a new IF_ENABLED_INT() to help with this. So now it is possible to
> write, without #ifdefs:
>
>  static void handle_wibble(void)
>  {
>         int val = IF_ENABLED_INT(CONFIG_WIBBLE, CONFIG_WIBBLE_ADDR);
>
>         ...
>  }
>
>  static void init_machine()
>  {
>  ...
>  if (IS_ENABLED(CONFIG_WIBBLE))
>         handle_wibble();
>  }
>
> The value will be 0 if CONFIG_WIBBLE is not defined, and
> CONFIG_WIBBLE_ADDR if it is. This allows us to reduce the use of #ifdef in
> the code, ensuring that the compiler still checks the code even if it is
> not ultimately used for a particular build.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  include/linux/kconfig.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
> index 3a2da738c4..86cd266540 100644
> --- a/include/linux/kconfig.h
> +++ b/include/linux/kconfig.h
> @@ -79,6 +79,18 @@
>   */
>  #define CONFIG_VAL(option)  config_val(option)
>
> +/* This use a similar mechanism to config_enabled() above */
> +#define config_opt_enabled(cfg, opt_cfg) _config_opt_enabled(cfg, opt_cfg)
> +#define _config_opt_enabled(cfg_val, opt_value) \
> +       __config_opt_enabled(__ARG_PLACEHOLDER_##cfg_val, opt_value)
> +#define __config_opt_enabled(arg1_or_junk, arg2) \
> +       ___config_opt_enabled(arg1_or_junk arg2, 0)
> +#define ___config_opt_enabled(__ignored, val, ...) val
> +
> +/* Evaluates to 0 if option is not defined, int_option if it is defined */
> +#define IF_ENABLED_INT(option, int_option) \
> +       config_opt_enabled(option, int_option)
> +
>  /*
>   * CONFIG_IS_ENABLED(FOO) evaluates to
>   *  1 if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y' or 'm',
> --
> 2.27.0.rc0.183.gde8f92d652-goog
>


-- 
Best Regards
Masahiro Yamada

[PATCH 2/6] kconfig: Add support for conditional values

[Heinrich Schuchardt]

On 5/22/20 4:17 AM, Masahiro Yamada wrote:
> On Fri, May 22, 2020 at 11:02 AM Simon Glass <sjg@chromium.org> wrote:
>>
>> At present if an optional Kconfig value needs to be used it must be
>> bracketed by #ifdef. For example, with this Kconfig setup:
>>
>> config WIBBLE
>>         bool "Support wibbles, the world needs more wibbles"
>>
>> config WIBBLE_ADDR
>>         hex "Address of the wibble"
>>         depends on WIBBLE
>
>
>
>
> I am not sure if this is a good idea.
>
>
> If you want to always use CONFIG_WIBBLE_ADDR,
> you can get rid of 'depends on WIBBLE'.
>
>

Hello Simon,

what is the effect on the code size if you eliminate the #ifdefs?

Isn't this move growing the size of the U-Boot binary?

Best regards

Heinrich

>
>
>> then the following code must be used:
>>
>>  #ifdef CONFIG_WIBBLE
>>  static void handle_wibble(void)
>>  {
>>         int val = CONFIG_WIBBLE_ADDR;
>>
>>         ...
>>  }
>>  #endif
>>
>>  static void init_machine()
>>  {
>>  ...
>>  #ifdef CONFIG_WIBBLE
>>         handle_wibble();
>>  #endif
>>  }
>>
>> Add a new IF_ENABLED_INT() to help with this. So now it is possible to
>> write, without #ifdefs:
>>
>>  static void handle_wibble(void)
>>  {
>>         int val = IF_ENABLED_INT(CONFIG_WIBBLE, CONFIG_WIBBLE_ADDR);
>>
>>         ...
>>  }
>>
>>  static void init_machine()
>>  {
>>  ...
>>  if (IS_ENABLED(CONFIG_WIBBLE))
>>         handle_wibble();
>>  }
>>
>> The value will be 0 if CONFIG_WIBBLE is not defined, and
>> CONFIG_WIBBLE_ADDR if it is. This allows us to reduce the use of #ifdef in
>> the code, ensuring that the compiler still checks the code even if it is
>> not ultimately used for a particular build.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  include/linux/kconfig.h | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
>> index 3a2da738c4..86cd266540 100644
>> --- a/include/linux/kconfig.h
>> +++ b/include/linux/kconfig.h
>> @@ -79,6 +79,18 @@
>>   */
>>  #define CONFIG_VAL(option)  config_val(option)
>>
>> +/* This use a similar mechanism to config_enabled() above */
>> +#define config_opt_enabled(cfg, opt_cfg) _config_opt_enabled(cfg, opt_cfg)
>> +#define _config_opt_enabled(cfg_val, opt_value) \
>> +       __config_opt_enabled(__ARG_PLACEHOLDER_##cfg_val, opt_value)
>> +#define __config_opt_enabled(arg1_or_junk, arg2) \
>> +       ___config_opt_enabled(arg1_or_junk arg2, 0)
>> +#define ___config_opt_enabled(__ignored, val, ...) val
>> +
>> +/* Evaluates to 0 if option is not defined, int_option if it is defined */
>> +#define IF_ENABLED_INT(option, int_option) \
>> +       config_opt_enabled(option, int_option)
>> +
>>  /*
>>   * CONFIG_IS_ENABLED(FOO) evaluates to
>>   *  1 if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y' or 'm',
>> --
>> 2.27.0.rc0.183.gde8f92d652-goog
>>
>
>

[PATCH 2/6] kconfig: Add support for conditional values

[Simon Glass]

Hi,

On Fri, 22 May 2020 at 00:18, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 5/22/20 4:17 AM, Masahiro Yamada wrote:
> > On Fri, May 22, 2020 at 11:02 AM Simon Glass <sjg@chromium.org> wrote:
> >>
> >> At present if an optional Kconfig value needs to be used it must be
> >> bracketed by #ifdef. For example, with this Kconfig setup:
> >>
> >> config WIBBLE
> >>         bool "Support wibbles, the world needs more wibbles"
> >>
> >> config WIBBLE_ADDR
> >>         hex "Address of the wibble"
> >>         depends on WIBBLE
> >
> >
> >
> >
> > I am not sure if this is a good idea.
> >
> >
> > If you want to always use CONFIG_WIBBLE_ADDR,
> > you can get rid of 'depends on WIBBLE'.

Yes that's right.

But I worry that we end up clutting the Kconfig with unused things.

Another option would be for Kconfig to emit hex CONFIGs always, even
if optional?

> >
> >
>
> Hello Simon,
>
> what is the effect on the code size if you eliminate the #ifdefs?
>
> Isn't this move growing the size of the U-Boot binary?

No it is handled at compile time so the code doesn't make it into the image.

[..]

Regards,
Simon

[PATCH 6/6] x86: fsp: Support a warning message when DRAM init is slow

[Wolfgang Wallner]

Hi Simon,

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

1) The required time is not APL specific, I think it depends on the memory
   configuration. I have tested it on an APL board with just 1 GB RAM,
   and there it only takes ~6 seconds.
   
   How about rewording it as follows:
   "As an example, for Chromebook Coral this is typically 21 seconds."
   
2) Typo: typically

>  - fspm,serial-debug-port-address: Debug Serial Port Base address
>  - fspm,serial-debug-port-type: Debug Serial Port Type
>    0: NONE
> -- 
> 2.27.0.rc0.183.gde8f92d652-goog

Reviewed-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>

Tested-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
Tested on a custom Apollo Lake board