All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/8] linux/kconfig.h: simplify logic for choosing CONFIG_{SPL_, TPL_, }*
@ 2020-07-03 16:37 Simon Glass
  2020-07-03 16:37 ` [PATCH v2 2/8] linux/kconfig.h: remove unused helper macros Simon Glass
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: Simon Glass @ 2020-07-03 16:37 UTC (permalink / raw)
  To: u-boot

From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

Instead of using the arg1_or_junk trick to pick between two choices,
with a bit of duplication between the branches (and most of the
CONFIG_TPL_BUILD case being redundant, as _IS_TPL is known to be
defined to 1 in that case), simply define a prefix that we inject
between CONFIG_ and the given config symbol.

This only requires one level of indirection (to get the
_CONFIG_PREFIX macro expanded before the token concatenation takes
place), and makes it easy to, say, introduce a CONFIG_HOSTTOOL_
prefix. [I would expect most HOSTTOOL_ symbols to just be def_bool y,
but it would allow us to clean up some of the ifdef HOSTCC mess in the
sources shared between U-Boot and host tools.]

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 include/linux/kconfig.h | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index 3a2da738c4..56b8e5d787 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -47,30 +47,19 @@
  * U-Boot add-on: Helper macros to reference to different macros
  * (CONFIG_ or CONFIG_SPL_ prefixed), depending on the build context.
  */
-#ifdef CONFIG_SPL_BUILD
-#define _IS_SPL 1
-#endif
-
-#ifdef CONFIG_TPL_BUILD
-#define _IS_TPL 1
-#endif
 
 #if defined(CONFIG_TPL_BUILD)
-#define config_val(cfg) _config_val(_IS_TPL, cfg)
-#define _config_val(x, cfg) __config_val(x, cfg)
-#define __config_val(x, cfg) ___config_val(__ARG_PLACEHOLDER_##x, cfg)
-#define ___config_val(arg1_or_junk, cfg)  \
-	____config_val(arg1_or_junk CONFIG_TPL_##cfg, CONFIG_##cfg)
-#define ____config_val(__ignored, val, ...) val
+#define _CONFIG_PREFIX TPL_
+#elif defined(CONFIG_SPL_BUILD)
+#define _CONFIG_PREFIX SPL_
 #else
-#define config_val(cfg) _config_val(_IS_SPL, cfg)
-#define _config_val(x, cfg) __config_val(x, cfg)
-#define __config_val(x, cfg) ___config_val(__ARG_PLACEHOLDER_##x, cfg)
-#define ___config_val(arg1_or_junk, cfg)  \
-	____config_val(arg1_or_junk CONFIG_SPL_##cfg, CONFIG_##cfg)
-#define ____config_val(__ignored, val, ...) val
+#define _CONFIG_PREFIX
 #endif
 
+#define   config_val(cfg)       _config_val(_CONFIG_PREFIX, cfg)
+#define  _config_val(pfx, cfg) __config_val(pfx, cfg)
+#define __config_val(pfx, cfg) CONFIG_ ## pfx ## cfg
+
 /*
  * CONFIG_VAL(FOO) evaluates to the value of
  *  CONFIG_FOO if CONFIG_SPL_BUILD is undefined,
-- 
2.27.0.212.ge8ba1cc988-goog

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

* [PATCH v2 2/8] linux/kconfig.h: remove unused helper macros
  2020-07-03 16:37 [PATCH v2 1/8] linux/kconfig.h: simplify logic for choosing CONFIG_{SPL_, TPL_, }* Simon Glass
@ 2020-07-03 16:37 ` Simon Glass
  2020-07-07  7:18   ` Bin Meng
  2020-07-03 16:37 ` [PATCH v2 3/8] linux/kconfig.h: create two- and three-argument versions of CONFIG_IS_ENABLED Simon Glass
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2020-07-03 16:37 UTC (permalink / raw)
  To: u-boot

From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

U-Boot does not have loadable modules, and nothing currently uses any
of the (CONFIG_)?IS_(BUILTIN|MODULE) macros - only
the (CONFIG_)?IS_ENABLED variants are ever used.

While I understand the desire to keep this somewhat synchronized with
linux, we've already departed by the introduction of the
CONFIG_IS_ENABLED extra logic, and deleting these makes the next patch
much simpler, since I won't have to duplicate a lot of logic for no
real gain (as there are no users).

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 include/linux/kconfig.h      | 40 +++++-------------------------------
 scripts/config_whitelist.txt |  2 --
 2 files changed, 5 insertions(+), 37 deletions(-)

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index 56b8e5d787..caea505a0e 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -23,25 +23,12 @@
 #define ___config_enabled(__ignored, val, ...) val
 
 /*
- * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
+ * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y',
  * 0 otherwise.
  *
  */
 #define IS_ENABLED(option) \
-	(config_enabled(option) || config_enabled(option##_MODULE))
-
-/*
- * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
- * otherwise. For boolean options, this is equivalent to
- * IS_ENABLED(CONFIG_FOO).
- */
-#define IS_BUILTIN(option) config_enabled(option)
-
-/*
- * IS_MODULE(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'm', 0
- * otherwise.
- */
-#define IS_MODULE(option) config_enabled(option##_MODULE)
+	(config_enabled(option))
 
 /*
  * U-Boot add-on: Helper macros to reference to different macros
@@ -70,29 +57,12 @@
 
 /*
  * CONFIG_IS_ENABLED(FOO) evaluates to
- *  1 if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y' or 'm',
- *  1 if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y' or 'm',
- *  1 if CONFIG_TPL_BUILD is defined and CONFIG_TPL_FOO is set to 'y' or 'm',
- *  0 otherwise.
- */
-#define CONFIG_IS_ENABLED(option) \
-	(config_enabled(CONFIG_VAL(option)) ||		\
-	 config_enabled(CONFIG_VAL(option##_MODULE)))
-
-/*
- * CONFIG_IS_BUILTIN(FOO) evaluates to
  *  1 if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y',
  *  1 if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y',
+ *  1 if CONFIG_TPL_BUILD is defined and CONFIG_TPL_FOO is set to 'y',
  *  0 otherwise.
  */
-#define CONFIG_IS_BUILTIN(option) config_enabled(CONFIG_VAL(option))
-
-/*
- * CONFIG_IS_MODULE(FOO) evaluates to
- *  1 if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'm',
- *  1 if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'm',
- *  0 otherwise.
- */
-#define CONFIG_IS_MODULE(option) config_enabled(CONFIG_VAL(option##_MODULE))
+#define CONFIG_IS_ENABLED(option) \
+	(config_enabled(CONFIG_VAL(option)))
 
 #endif /* __LINUX_KCONFIG_H */
diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
index 2210f46e44..352db2b57a 100644
--- a/scripts/config_whitelist.txt
+++ b/scripts/config_whitelist.txt
@@ -875,9 +875,7 @@ CONFIG_IRAM_SIZE
 CONFIG_IRAM_STACK
 CONFIG_IRAM_TOP
 CONFIG_IRDA_BASE
-CONFIG_IS_BUILTIN
 CONFIG_IS_ENABLED
-CONFIG_IS_MODULE
 CONFIG_JFFS2_CMDLINE
 CONFIG_JFFS2_DEV
 CONFIG_JFFS2_LZO
-- 
2.27.0.212.ge8ba1cc988-goog

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

* [PATCH v2 3/8] linux/kconfig.h: create two- and three-argument versions of CONFIG_IS_ENABLED
  2020-07-03 16:37 [PATCH v2 1/8] linux/kconfig.h: simplify logic for choosing CONFIG_{SPL_, TPL_, }* Simon Glass
  2020-07-03 16:37 ` [PATCH v2 2/8] linux/kconfig.h: remove unused helper macros Simon Glass
@ 2020-07-03 16:37 ` Simon Glass
  2020-07-07  7:19   ` Bin Meng
  2020-07-03 16:37 ` [PATCH v2 4/8] bootstage: Fix 'stacked' typo Simon Glass
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2020-07-03 16:37 UTC (permalink / raw)
  To: u-boot

From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

This adds a bunch of preprocessor magic to extend the capabilities of
CONFIG_IS_ENABLED. The existing semantics of

  CONFIG_IS_ENABLED(FOO)

expanding to a 1 or 0 (depending on build context and the defined-ness
or not of the appropriate CONFIG_FOO/CONFIG_SPL_FOO/CONFIG_TPL_FOO)
are of course preserved. With this, one is also allowed a two-argument
form

  CONFIG_IS_ENABLED(FOO, (something))

which expands to something precisely when CONFIG_IS_ENABLED(FOO) would
expand to 1, and expands to nothing otherwise. It is, in other words,
completely equivalent to the three lines

  #if CONFIG_IS_ENABLED(FOO)
  something
  #endif

The second argument must be parenthesized in order to allow any
tokens, including a trailing comma, to appear - one use case for this
is precisely to make it a bit more ergonomic to build an array and
only include certain items depending on .config. That should increase
both readability and not least "git grep"ability.

A third variant is also introduced,

  CONFIG_IS_ENABLED(FOO, (xxx), (yyy))

which corresponds to

  #if CONFIG_IS_ENABLED(FOO)
  xxx
  #else
  yyy
  #endif

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 include/linux/kconfig.h | 48 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index caea505a0e..d109ed3119 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -56,13 +56,55 @@
 #define CONFIG_VAL(option)  config_val(option)
 
 /*
- * CONFIG_IS_ENABLED(FOO) evaluates to
+ * Count number of arguments to a variadic macro. Currently only need
+ * it for 1, 2 or 3 arguments.
+ */
+#define __arg6(a1, a2, a3, a4, a5, a6, ...) a6
+#define __count_args(...) __arg6(dummy, ##__VA_ARGS__, 4, 3, 2, 1, 0)
+
+#define __concat(a, b)   ___concat(a, b)
+#define ___concat(a, b)  a ## b
+
+#define __unwrap(...) __VA_ARGS__
+#define __unwrap1(case1, case0) __unwrap case1
+#define __unwrap0(case1, case0) __unwrap case0
+
+#define __CONFIG_IS_ENABLED_1(option)        __CONFIG_IS_ENABLED_3(option, (1), (0))
+#define __CONFIG_IS_ENABLED_2(option, case1) __CONFIG_IS_ENABLED_3(option, case1, ())
+#define __CONFIG_IS_ENABLED_3(option, case1, case0) \
+	__concat(__unwrap, config_enabled(CONFIG_VAL(option))) (case1, case0)
+
+/*
+ * CONFIG_IS_ENABLED(FOO) expands to
  *  1 if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y',
  *  1 if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y',
  *  1 if CONFIG_TPL_BUILD is defined and CONFIG_TPL_FOO is set to 'y',
  *  0 otherwise.
+ *
+ * CONFIG_IS_ENABLED(FOO, (abc)) expands to
+ *  abc if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y',
+ *  abc if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y',
+ *  abc if CONFIG_TPL_BUILD is defined and CONFIG_TPL_FOO is set to 'y',
+ *  nothing otherwise.
+ *
+ * CONFIG_IS_ENABLED(FOO, (abc), (def)) expands to
+ *  abc if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y',
+ *  abc if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y',
+ *  abc if CONFIG_TPL_BUILD is defined and CONFIG_TPL_FOO is set to 'y',
+ *  def otherwise.
+ *
+ * The optional second and third arguments must be parenthesized; that
+ * allows one to include a trailing comma, e.g. for use in
+ *
+ * CONFIG_IS_ENABLED(ACME, ({.compatible = "acme,frobnozzle"},))
+ *
+ * which adds an entry to the array being defined if CONFIG_ACME (or
+ * CONFIG_SPL_ACME/CONFIG_TPL_ACME, depending on build context) is
+ * set, and nothing otherwise.
  */
-#define CONFIG_IS_ENABLED(option) \
-	(config_enabled(CONFIG_VAL(option)))
+
+#define CONFIG_IS_ENABLED(option, ...)					\
+	__concat(__CONFIG_IS_ENABLED_, __count_args(option, ##__VA_ARGS__)) (option, ##__VA_ARGS__)
+
 
 #endif /* __LINUX_KCONFIG_H */
-- 
2.27.0.212.ge8ba1cc988-goog

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

* [PATCH v2 4/8] bootstage: Fix 'stacked' typo
  2020-07-03 16:37 [PATCH v2 1/8] linux/kconfig.h: simplify logic for choosing CONFIG_{SPL_, TPL_, }* Simon Glass
  2020-07-03 16:37 ` [PATCH v2 2/8] linux/kconfig.h: remove unused helper macros Simon Glass
  2020-07-03 16:37 ` [PATCH v2 3/8] linux/kconfig.h: create two- and three-argument versions of CONFIG_IS_ENABLED Simon Glass
@ 2020-07-03 16:37 ` Simon Glass
  2020-07-07  7:07   ` Bin Meng
  2020-07-03 16:37 ` [PATCH v2 5/8] timer: Allow delays with a 32-bit microsecond timer Simon Glass
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2020-07-03 16:37 UTC (permalink / raw)
  To: u-boot

This should be 'stashed'. Fix it.

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

(no changes since v1)

 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.212.ge8ba1cc988-goog

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

* [PATCH v2 5/8] timer: Allow delays with a 32-bit microsecond timer
  2020-07-03 16:37 [PATCH v2 1/8] linux/kconfig.h: simplify logic for choosing CONFIG_{SPL_, TPL_, }* Simon Glass
                   ` (2 preceding siblings ...)
  2020-07-03 16:37 ` [PATCH v2 4/8] bootstage: Fix 'stacked' typo Simon Glass
@ 2020-07-03 16:37 ` Simon Glass
  2020-07-07  7:09   ` Bin Meng
  2020-07-03 16:37 ` [PATCH v2 6/8] coral: Enable the copy framebuffer Simon Glass
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2020-07-03 16:37 UTC (permalink / raw)
  To: u-boot

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

(no changes since v1)

 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.212.ge8ba1cc988-goog

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

* [PATCH v2 6/8] coral: Enable the copy framebuffer
  2020-07-03 16:37 [PATCH v2 1/8] linux/kconfig.h: simplify logic for choosing CONFIG_{SPL_, TPL_, }* Simon Glass
                   ` (3 preceding siblings ...)
  2020-07-03 16:37 ` [PATCH v2 5/8] timer: Allow delays with a 32-bit microsecond timer Simon Glass
@ 2020-07-03 16:37 ` Simon Glass
  2020-07-07  7:09   ` Bin Meng
  2020-07-03 16:37 ` [PATCH v2 7/8] x86: Avoid #ifdef with CONFIG_HAVE_ACPI_RESUME Simon Glass
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2020-07-03 16:37 UTC (permalink / raw)
  To: u-boot

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

(no changes since v1)

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

diff --git a/configs/chromebook_coral_defconfig b/configs/chromebook_coral_defconfig
index d7cab2334b..a2a29cf842 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.212.ge8ba1cc988-goog

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

* [PATCH v2 7/8] x86: Avoid #ifdef with CONFIG_HAVE_ACPI_RESUME
  2020-07-03 16:37 [PATCH v2 1/8] linux/kconfig.h: simplify logic for choosing CONFIG_{SPL_, TPL_, }* Simon Glass
                   ` (4 preceding siblings ...)
  2020-07-03 16:37 ` [PATCH v2 6/8] coral: Enable the copy framebuffer Simon Glass
@ 2020-07-03 16:37 ` Simon Glass
  2020-07-07  7:13   ` Bin Meng
  2020-07-03 16:37 ` [PATCH v2 8/8] x86: fsp: Support a warning message when DRAM init is slow Simon Glass
  2020-07-07  7:17 ` [PATCH v2 1/8] linux/kconfig.h: simplify logic for choosing CONFIG_{SPL_, TPL_, }* Bin Meng
  7 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2020-07-03 16:37 UTC (permalink / raw)
  To: u-boot

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

Changes in v2:
- Use new three-argument CONFIG_IS_ENABLED() instead

 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 767ddfe680..13e6b20f08 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..01d498c21e 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 = CONFIG_IS_ENABLED(HAVE_ACPI_RESUME,
+					       (CONFIG_STACK_SIZE), (0));
+		/*
+		 * 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.212.ge8ba1cc988-goog

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

* [PATCH v2 8/8] x86: fsp: Support a warning message when DRAM init is slow
  2020-07-03 16:37 [PATCH v2 1/8] linux/kconfig.h: simplify logic for choosing CONFIG_{SPL_, TPL_, }* Simon Glass
                   ` (5 preceding siblings ...)
  2020-07-03 16:37 ` [PATCH v2 7/8] x86: Avoid #ifdef with CONFIG_HAVE_ACPI_RESUME Simon Glass
@ 2020-07-03 16:37 ` Simon Glass
  2020-07-07  7:17 ` [PATCH v2 1/8] linux/kconfig.h: simplify logic for choosing CONFIG_{SPL_, TPL_, }* Bin Meng
  7 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2020-07-03 16:37 UTC (permalink / raw)
  To: u-boot

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>
Reviewed-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
Tested-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
---

Changes in v2:
- Use the three-argument CONFIG_IS_ENABLED() instead of IF_ENABLED_INT()
- Update binding to mention timing for coral and a 1GB APL board
- Drop patch 'kconfig: Add support for conditional values'

 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             |  4 ++++
 5 files changed, 36 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 965d9f387d..a17a9c2800 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..5311938f43 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,10 @@ 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 Chromebook
+    Coral this is typically 21 seconds. For an APL board with 1GB of RAM, it may
+    be only 6 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.212.ge8ba1cc988-goog

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

* [PATCH v2 4/8] bootstage: Fix 'stacked' typo
  2020-07-03 16:37 ` [PATCH v2 4/8] bootstage: Fix 'stacked' typo Simon Glass
@ 2020-07-07  7:07   ` Bin Meng
  2020-07-07  7:19     ` Bin Meng
  0 siblings, 1 reply; 20+ messages in thread
From: Bin Meng @ 2020-07-07  7:07 UTC (permalink / raw)
  To: u-boot

On Sat, Jul 4, 2020 at 12:38 AM Simon Glass <sjg@chromium.org> wrote:
>
> This should be 'stashed'. Fix it.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>  include/bootstage.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

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

* [PATCH v2 5/8] timer: Allow delays with a 32-bit microsecond timer
  2020-07-03 16:37 ` [PATCH v2 5/8] timer: Allow delays with a 32-bit microsecond timer Simon Glass
@ 2020-07-07  7:09   ` Bin Meng
  2020-07-07  7:20     ` Bin Meng
  2020-07-10  0:28     ` Simon Glass
  0 siblings, 2 replies; 20+ messages in thread
From: Bin Meng @ 2020-07-07  7:09 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Sat, Jul 4, 2020 at 12:38 AM Simon Glass <sjg@chromium.org> wrote:
>
> 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>
> ---
>
> (no changes since v1)
>
>  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);

The function name does not clear indicates this is 32-bit value
because unsigned long is still 64-bit when building for 64-bit U-Boot.

> +
>  /*
>   * 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);
> --

Regards,
Bin

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

* [PATCH v2 6/8] coral: Enable the copy framebuffer
  2020-07-03 16:37 ` [PATCH v2 6/8] coral: Enable the copy framebuffer Simon Glass
@ 2020-07-07  7:09   ` Bin Meng
  0 siblings, 0 replies; 20+ messages in thread
From: Bin Meng @ 2020-07-07  7:09 UTC (permalink / raw)
  To: u-boot

On Sat, Jul 4, 2020 at 12:38 AM Simon Glass <sjg@chromium.org> wrote:
>
> 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>
> ---
>
> (no changes since v1)
>
>  configs/chromebook_coral_defconfig | 1 +
>  1 file changed, 1 insertion(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

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

* [PATCH v2 7/8] x86: Avoid #ifdef with CONFIG_HAVE_ACPI_RESUME
  2020-07-03 16:37 ` [PATCH v2 7/8] x86: Avoid #ifdef with CONFIG_HAVE_ACPI_RESUME Simon Glass
@ 2020-07-07  7:13   ` Bin Meng
  0 siblings, 0 replies; 20+ messages in thread
From: Bin Meng @ 2020-07-07  7:13 UTC (permalink / raw)
  To: u-boot

On Sat, Jul 4, 2020 at 12:37 AM Simon Glass <sjg@chromium.org> wrote:
>
> 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>
> ---
>
> Changes in v2:
> - Use new three-argument CONFIG_IS_ENABLED() instead
>
>  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(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

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

* [PATCH v2 1/8] linux/kconfig.h: simplify logic for choosing CONFIG_{SPL_, TPL_, }*
  2020-07-03 16:37 [PATCH v2 1/8] linux/kconfig.h: simplify logic for choosing CONFIG_{SPL_, TPL_, }* Simon Glass
                   ` (6 preceding siblings ...)
  2020-07-03 16:37 ` [PATCH v2 8/8] x86: fsp: Support a warning message when DRAM init is slow Simon Glass
@ 2020-07-07  7:17 ` Bin Meng
  7 siblings, 0 replies; 20+ messages in thread
From: Bin Meng @ 2020-07-07  7:17 UTC (permalink / raw)
  To: u-boot

On Sat, Jul 4, 2020 at 12:37 AM Simon Glass <sjg@chromium.org> wrote:
>
> From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>
> Instead of using the arg1_or_junk trick to pick between two choices,
> with a bit of duplication between the branches (and most of the
> CONFIG_TPL_BUILD case being redundant, as _IS_TPL is known to be
> defined to 1 in that case), simply define a prefix that we inject
> between CONFIG_ and the given config symbol.
>
> This only requires one level of indirection (to get the
> _CONFIG_PREFIX macro expanded before the token concatenation takes
> place), and makes it easy to, say, introduce a CONFIG_HOSTTOOL_
> prefix. [I would expect most HOSTTOOL_ symbols to just be def_bool y,
> but it would allow us to clean up some of the ifdef HOSTCC mess in the
> sources shared between U-Boot and host tools.]
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>  include/linux/kconfig.h | 27 ++++++++-------------------
>  1 file changed, 8 insertions(+), 19 deletions(-)
>

applied to u-boot-x86, thanks!

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

* [PATCH v2 2/8] linux/kconfig.h: remove unused helper macros
  2020-07-03 16:37 ` [PATCH v2 2/8] linux/kconfig.h: remove unused helper macros Simon Glass
@ 2020-07-07  7:18   ` Bin Meng
  0 siblings, 0 replies; 20+ messages in thread
From: Bin Meng @ 2020-07-07  7:18 UTC (permalink / raw)
  To: u-boot

On Sat, Jul 4, 2020 at 12:37 AM Simon Glass <sjg@chromium.org> wrote:
>
> From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>
> U-Boot does not have loadable modules, and nothing currently uses any
> of the (CONFIG_)?IS_(BUILTIN|MODULE) macros - only
> the (CONFIG_)?IS_ENABLED variants are ever used.
>
> While I understand the desire to keep this somewhat synchronized with
> linux, we've already departed by the introduction of the
> CONFIG_IS_ENABLED extra logic, and deleting these makes the next patch
> much simpler, since I won't have to duplicate a lot of logic for no
> real gain (as there are no users).
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>  include/linux/kconfig.h      | 40 +++++-------------------------------
>  scripts/config_whitelist.txt |  2 --
>  2 files changed, 5 insertions(+), 37 deletions(-)
>

applied to u-boot-x86, thanks!

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

* [PATCH v2 3/8] linux/kconfig.h: create two- and three-argument versions of CONFIG_IS_ENABLED
  2020-07-03 16:37 ` [PATCH v2 3/8] linux/kconfig.h: create two- and three-argument versions of CONFIG_IS_ENABLED Simon Glass
@ 2020-07-07  7:19   ` Bin Meng
  0 siblings, 0 replies; 20+ messages in thread
From: Bin Meng @ 2020-07-07  7:19 UTC (permalink / raw)
  To: u-boot

On Sat, Jul 4, 2020 at 12:38 AM Simon Glass <sjg@chromium.org> wrote:
>
> From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>
> This adds a bunch of preprocessor magic to extend the capabilities of
> CONFIG_IS_ENABLED. The existing semantics of
>
>   CONFIG_IS_ENABLED(FOO)
>
> expanding to a 1 or 0 (depending on build context and the defined-ness
> or not of the appropriate CONFIG_FOO/CONFIG_SPL_FOO/CONFIG_TPL_FOO)
> are of course preserved. With this, one is also allowed a two-argument
> form
>
>   CONFIG_IS_ENABLED(FOO, (something))
>
> which expands to something precisely when CONFIG_IS_ENABLED(FOO) would
> expand to 1, and expands to nothing otherwise. It is, in other words,
> completely equivalent to the three lines
>
>   #if CONFIG_IS_ENABLED(FOO)
>   something
>   #endif
>
> The second argument must be parenthesized in order to allow any
> tokens, including a trailing comma, to appear - one use case for this
> is precisely to make it a bit more ergonomic to build an array and
> only include certain items depending on .config. That should increase
> both readability and not least "git grep"ability.
>
> A third variant is also introduced,
>
>   CONFIG_IS_ENABLED(FOO, (xxx), (yyy))
>
> which corresponds to
>
>   #if CONFIG_IS_ENABLED(FOO)
>   xxx
>   #else
>   yyy
>   #endif
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>  include/linux/kconfig.h | 48 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 45 insertions(+), 3 deletions(-)
>

applied to u-boot-x86, thanks!

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

* [PATCH v2 4/8] bootstage: Fix 'stacked' typo
  2020-07-07  7:07   ` Bin Meng
@ 2020-07-07  7:19     ` Bin Meng
  0 siblings, 0 replies; 20+ messages in thread
From: Bin Meng @ 2020-07-07  7:19 UTC (permalink / raw)
  To: u-boot

On Tue, Jul 7, 2020 at 3:07 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Sat, Jul 4, 2020 at 12:38 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > This should be 'stashed'. Fix it.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > (no changes since v1)
> >
> >  include/bootstage.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

applied to u-boot-x86, thanks!

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

* [PATCH v2 5/8] timer: Allow delays with a 32-bit microsecond timer
  2020-07-07  7:09   ` Bin Meng
@ 2020-07-07  7:20     ` Bin Meng
  2020-07-10  0:28     ` Simon Glass
  1 sibling, 0 replies; 20+ messages in thread
From: Bin Meng @ 2020-07-07  7:20 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Tue, Jul 7, 2020 at 3:09 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Sat, Jul 4, 2020 at 12:38 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > 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>
> > ---
> >
> > (no changes since v1)
> >
> >  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);
>
> The function name does not clear indicates this is 32-bit value
> because unsigned long is still 64-bit when building for 64-bit U-Boot.
>
> > +
> >  /*
> >   * 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);
> > --

I've applied the first 4 patches in this series to u-boot-x86.

Regards,
Bin

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

* [PATCH v2 5/8] timer: Allow delays with a 32-bit microsecond timer
  2020-07-07  7:09   ` Bin Meng
  2020-07-07  7:20     ` Bin Meng
@ 2020-07-10  0:28     ` Simon Glass
  2020-07-10  0:44       ` Bin Meng
  1 sibling, 1 reply; 20+ messages in thread
From: Simon Glass @ 2020-07-10  0:28 UTC (permalink / raw)
  To: u-boot

Hi Bin,

On Tue, 7 Jul 2020 at 01:09, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Sat, Jul 4, 2020 at 12:38 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > 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>
> > ---
> >
> > (no changes since v1)
> >
> >  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);
>
> The function name does not clear indicates this is 32-bit value
> because unsigned long is still 64-bit when building for 64-bit U-Boot.

Yes that's right. My purpose is to use the natural long time.

Shall I update the commit message?

Regards,
Simon

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

* [PATCH v2 5/8] timer: Allow delays with a 32-bit microsecond timer
  2020-07-10  0:28     ` Simon Glass
@ 2020-07-10  0:44       ` Bin Meng
  2020-07-10  0:56         ` Simon Glass
  0 siblings, 1 reply; 20+ messages in thread
From: Bin Meng @ 2020-07-10  0:44 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Fri, Jul 10, 2020 at 8:28 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Tue, 7 Jul 2020 at 01:09, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Sat, Jul 4, 2020 at 12:38 AM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > 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>
> > > ---
> > >
> > > (no changes since v1)
> > >
> > >  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);
> >
> > The function name does not clear indicates this is 32-bit value
> > because unsigned long is still 64-bit when building for 64-bit U-Boot.
>
> Yes that's right. My purpose is to use the natural long time.
>
> Shall I update the commit message?

I think we need to explicitly declare the return value to u32 or
uint32_t to make a 32-bit variant function.

Regards,
Bin

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

* [PATCH v2 5/8] timer: Allow delays with a 32-bit microsecond timer
  2020-07-10  0:44       ` Bin Meng
@ 2020-07-10  0:56         ` Simon Glass
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2020-07-10  0:56 UTC (permalink / raw)
  To: u-boot

Hi Bin,

On Thu, 9 Jul 2020 at 18:44, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Fri, Jul 10, 2020 at 8:28 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Tue, 7 Jul 2020 at 01:09, Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Sat, Jul 4, 2020 at 12:38 AM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > 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>
> > > > ---
> > > >
> > > > (no changes since v1)
> > > >
> > > >  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);
> > >
> > > The function name does not clear indicates this is 32-bit value
> > > because unsigned long is still 64-bit when building for 64-bit U-Boot.
> >
> > Yes that's right. My purpose is to use the natural long time.
> >
> > Shall I update the commit message?
>
> I think we need to explicitly declare the return value to u32 or
> uint32_t to make a 32-bit variant function.

But that is not my objective here. I just want something that is
efficient on both 32- and 64-bit systems.

Regards,
Simon

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

end of thread, other threads:[~2020-07-10  0:56 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-03 16:37 [PATCH v2 1/8] linux/kconfig.h: simplify logic for choosing CONFIG_{SPL_, TPL_, }* Simon Glass
2020-07-03 16:37 ` [PATCH v2 2/8] linux/kconfig.h: remove unused helper macros Simon Glass
2020-07-07  7:18   ` Bin Meng
2020-07-03 16:37 ` [PATCH v2 3/8] linux/kconfig.h: create two- and three-argument versions of CONFIG_IS_ENABLED Simon Glass
2020-07-07  7:19   ` Bin Meng
2020-07-03 16:37 ` [PATCH v2 4/8] bootstage: Fix 'stacked' typo Simon Glass
2020-07-07  7:07   ` Bin Meng
2020-07-07  7:19     ` Bin Meng
2020-07-03 16:37 ` [PATCH v2 5/8] timer: Allow delays with a 32-bit microsecond timer Simon Glass
2020-07-07  7:09   ` Bin Meng
2020-07-07  7:20     ` Bin Meng
2020-07-10  0:28     ` Simon Glass
2020-07-10  0:44       ` Bin Meng
2020-07-10  0:56         ` Simon Glass
2020-07-03 16:37 ` [PATCH v2 6/8] coral: Enable the copy framebuffer Simon Glass
2020-07-07  7:09   ` Bin Meng
2020-07-03 16:37 ` [PATCH v2 7/8] x86: Avoid #ifdef with CONFIG_HAVE_ACPI_RESUME Simon Glass
2020-07-07  7:13   ` Bin Meng
2020-07-03 16:37 ` [PATCH v2 8/8] x86: fsp: Support a warning message when DRAM init is slow Simon Glass
2020-07-07  7:17 ` [PATCH v2 1/8] linux/kconfig.h: simplify logic for choosing CONFIG_{SPL_, TPL_, }* 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.