[PATCH 00/14] fdt: Improvements for fdt and tracing

[PATCH 00/14] fdt: Improvements for fdt and tracing

[Simon Glass]

This series aims to make it easier to measure boot performance,
particularly aimed at rockpro64. It includes the following:

- enabling bootstage on rockpro64 (TPL, SPL, U-Boot proper)
- minor fixes and tidy-ups for tracing
- minor FDT code tidy-ups

It also reverts a patch which slows down the boot by 30ms. This revert
was requested some time ago but with this series we have the data showing
the impact.

Changes in v1:
- Add detail of impact

Simon Glass (14):
  trace: Use notrace for short
  arm: Support trace on armv8
  tpm: Add a proper Kconfig option for crc8 in SPL
  fdt: Avoid exporting fdtdec_prepare_fdt()
  fdt: Drop ifdefs in fdtdec_prepare_fdt()
  fdt: Pass the device tree to fdtdec_prepare_fdt()
  fdt: Check for overlapping data and FDT
  trace: Move trace pointer to data section
  mkimage: Add a few more messages for FIT failures
  trace: Adjust flags in proftool
  trace: Update trace-format generator for newer version
  trace: Don't require TIMER_EARLY
  rockchip: Enable bootstage on rockpro64
  Revert "fdtdec: drop needlessly convoluted CONFIG_PHANDLE_CHECK_SEQ"

 arch/arm/cpu/armv7/s5p-common/timer.c   |  2 +-
 arch/arm/cpu/armv8/generic_timer.c      |  6 +-
 arch/arm/mach-exynos/include/mach/cpu.h |  6 +-
 arch/arm/mach-rockchip/tpl.c            | 18 ++++-
 arch/x86/include/asm/global_data.h      |  2 +-
 arch/x86/include/asm/msr.h              |  2 +-
 arch/x86/include/asm/u-boot-x86.h       |  2 +-
 common/spl/spl.c                        |  2 +-
 common/spl/spl_fit.c                    |  1 +
 configs/am65x_evm_a53_defconfig         |  1 +
 configs/evb-ast2600_defconfig           |  1 +
 configs/rockpro64-rk3399_defconfig      |  8 +++
 configs/sama7g5ek_mmc1_defconfig        |  1 +
 configs/sama7g5ek_mmc_defconfig         |  1 +
 doc/develop/trace.rst                   |  2 +-
 include/fdtdec.h                        |  9 ---
 lib/Kconfig                             | 26 ++++++-
 lib/Makefile                            |  3 +-
 lib/efi_loader/efi_freestanding.c       |  4 +-
 lib/fdtdec.c                            | 92 ++++++++++++++++---------
 lib/trace.c                             | 29 ++++----
 tools/fit_image.c                       |  4 +-
 tools/image-host.c                      |  6 +-
 tools/proftool.c                        | 43 +++++++-----
 24 files changed, 175 insertions(+), 96 deletions(-)

-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 01/14] trace: Use notrace for short

[Simon Glass]

The attribute syntax is quite verbose. Use the macro provided for this
purpose.

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

 arch/arm/cpu/armv7/s5p-common/timer.c   |  2 +-
 arch/arm/mach-exynos/include/mach/cpu.h |  6 +++---
 arch/x86/include/asm/global_data.h      |  2 +-
 arch/x86/include/asm/msr.h              |  2 +-
 arch/x86/include/asm/u-boot-x86.h       |  2 +-
 common/spl/spl_fit.c                    |  1 +
 doc/develop/trace.rst                   |  2 +-
 lib/efi_loader/efi_freestanding.c       |  4 ++--
 lib/trace.c                             | 26 +++++++++++--------------
 9 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/arch/arm/cpu/armv7/s5p-common/timer.c b/arch/arm/cpu/armv7/s5p-common/timer.c
index 8533d04878c..65e3ec39ae3 100644
--- a/arch/arm/cpu/armv7/s5p-common/timer.c
+++ b/arch/arm/cpu/armv7/s5p-common/timer.c
@@ -86,7 +86,7 @@ unsigned long get_timer(unsigned long base)
 	return time_ms - base;
 }
 
-unsigned long __attribute__((no_instrument_function)) timer_get_us(void)
+unsigned long notrace timer_get_us(void)
 {
 	static unsigned long base_time_us;
 
diff --git a/arch/arm/mach-exynos/include/mach/cpu.h b/arch/arm/mach-exynos/include/mach/cpu.h
index fb5fdaf3ba8..dab148e3320 100644
--- a/arch/arm/mach-exynos/include/mach/cpu.h
+++ b/arch/arm/mach-exynos/include/mach/cpu.h
@@ -248,7 +248,7 @@ static inline char *s5p_get_cpu_name(void)
 }
 
 #define IS_SAMSUNG_TYPE(type, id)			\
-static inline int __attribute__((no_instrument_function)) cpu_is_##type(void) \
+static inline int notrace cpu_is_##type(void)		\
 {							\
 	return (s5p_cpu_id >> 12) == id;		\
 }
@@ -257,7 +257,7 @@ IS_SAMSUNG_TYPE(exynos4, 0x4)
 IS_SAMSUNG_TYPE(exynos5, 0x5)
 
 #define IS_EXYNOS_TYPE(type, id)			\
-static inline int __attribute__((no_instrument_function)) \
+static inline int notrace				\
 	proid_is_##type(void)				\
 {							\
 	return s5p_cpu_id == id;			\
@@ -272,7 +272,7 @@ IS_EXYNOS_TYPE(exynos5422, 0x5422)
 #define proid_is_exynos542x() (proid_is_exynos5420() || proid_is_exynos5422())
 
 #define SAMSUNG_BASE(device, base)				\
-static inline unsigned long __attribute__((no_instrument_function)) \
+static inline unsigned long notrace				\
 	samsung_get_base_##device(void) \
 {								\
 	if (cpu_is_exynos4()) {				\
diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h
index 23693f85a78..22d103df4ee 100644
--- a/arch/x86/include/asm/global_data.h
+++ b/arch/x86/include/asm/global_data.h
@@ -137,7 +137,7 @@ struct arch_global_data {
 
 #define DECLARE_GLOBAL_DATA_PTR   extern struct global_data *global_data_ptr
 # else
-static inline __attribute__((no_instrument_function)) gd_t *get_fs_gd_ptr(void)
+static inline notrace gd_t *get_fs_gd_ptr(void)
 {
 	gd_t *gd_ptr;
 
diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index 3e613de6cef..27764fc56cb 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -71,7 +71,7 @@ static inline unsigned long long native_read_tscp(unsigned int *aux)
 #define EAX_EDX_RET(val, low, high)	"=A" (val)
 #endif
 
-static inline __attribute__((no_instrument_function))
+static inline notrace
 	unsigned long long native_read_msr(unsigned int msr)
 {
 	DECLARE_ARGS(val, low, high);
diff --git a/arch/x86/include/asm/u-boot-x86.h b/arch/x86/include/asm/u-boot-x86.h
index 4cf41e93541..8f38c2d1c60 100644
--- a/arch/x86/include/asm/u-boot-x86.h
+++ b/arch/x86/include/asm/u-boot-x86.h
@@ -108,7 +108,7 @@ void	board_init_f_r(void) __attribute__ ((noreturn));
 int arch_misc_init(void);
 
 /* Read the time stamp counter */
-static inline __attribute__((no_instrument_function)) uint64_t rdtsc(void)
+static inline notrace uint64_t rdtsc(void)
 {
 	uint32_t high, low;
 	__asm__ __volatile__("rdtsc" : "=a" (low), "=d" (high));
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 08da7fed88e..0e026bb3d8e 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -599,6 +599,7 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node,
 			debug("Ignoring compatible = %s property\n",
 			      compatible);
 	}
+	return 0;
 
 	ret = fpga_load(devnum, (void *)fpga_image->load_addr,
 			fpga_image->size, BIT_FULL, flags);
diff --git a/doc/develop/trace.rst b/doc/develop/trace.rst
index b22e068ef9e..5c7802da51a 100644
--- a/doc/develop/trace.rst
+++ b/doc/develop/trace.rst
@@ -185,7 +185,7 @@ this produces sensible results for your board. Suitable sources for
 this timer include high resolution timers, PWMs or profile timers if
 available. Most modern SOCs have a suitable timer for this. Make sure
 that you mark this timer (and anything it calls) with
-__attribute__((no_instrument_function)) so that the trace library can
+notrace so that the trace library can
 use it without causing an infinite loop.
 
 
diff --git a/lib/efi_loader/efi_freestanding.c b/lib/efi_loader/efi_freestanding.c
index c85df026f0d..4b65fc64dd0 100644
--- a/lib/efi_loader/efi_freestanding.c
+++ b/lib/efi_loader/efi_freestanding.c
@@ -100,7 +100,7 @@ void *memset(void *s, int c, size_t n)
  * func_ptr:	Pointer to function being entered
  * caller:	Pointer to function which called this function
  */
-void __attribute__((no_instrument_function))
+void notrace
 __cyg_profile_func_enter(void *func_ptr, void *caller)
 {
 }
@@ -116,7 +116,7 @@ __cyg_profile_func_enter(void *func_ptr, void *caller)
  * func_ptr:	Pointer to function being entered
  * caller:	Pointer to function which called this function
  */
-void __attribute__((no_instrument_function))
+void notrace
 __cyg_profile_func_exit(void *func_ptr, void *caller)
 {
 }
diff --git a/lib/trace.c b/lib/trace.c
index 54f0bf2f578..880d90ebd5c 100644
--- a/lib/trace.c
+++ b/lib/trace.c
@@ -68,7 +68,7 @@ static volatile gd_t *trace_gd;
 /**
  * trace_save_gd() - save the value of the gd register
  */
-static void __attribute__((no_instrument_function)) trace_save_gd(void)
+static void notrace trace_save_gd(void)
 {
 	trace_gd = gd;
 }
@@ -81,7 +81,7 @@ static void __attribute__((no_instrument_function)) trace_save_gd(void)
  * have to set the gd register to the U-Boot value when entering a trace
  * point and set it back to the application value when exiting the trace point.
  */
-static void __attribute__((no_instrument_function)) trace_swap_gd(void)
+static void notrace trace_swap_gd(void)
 {
 	volatile gd_t *temp_gd = trace_gd;
 
@@ -91,18 +91,17 @@ static void __attribute__((no_instrument_function)) trace_swap_gd(void)
 
 #else
 
-static void __attribute__((no_instrument_function)) trace_save_gd(void)
+static void notrace trace_save_gd(void)
 {
 }
 
-static void __attribute__((no_instrument_function)) trace_swap_gd(void)
+static void notrace trace_swap_gd(void)
 {
 }
 
 #endif
 
-static void __attribute__((no_instrument_function)) add_ftrace(void *func_ptr,
-				void *caller, ulong flags)
+static void notrace add_ftrace(void *func_ptr, void *caller, ulong flags)
 {
 	if (hdr->depth > hdr->depth_limit) {
 		hdr->ftrace_too_deep_count++;
@@ -118,7 +117,7 @@ static void __attribute__((no_instrument_function)) add_ftrace(void *func_ptr,
 	hdr->ftrace_count++;
 }
 
-static void __attribute__((no_instrument_function)) add_textbase(void)
+static void notrace add_textbase(void)
 {
 	if (hdr->ftrace_count < hdr->ftrace_size) {
 		struct trace_call *rec = &hdr->ftrace[hdr->ftrace_count];
@@ -139,8 +138,7 @@ static void __attribute__((no_instrument_function)) add_textbase(void)
  * @func_ptr:	pointer to function being entered
  * @caller:	pointer to function which called this function
  */
-void __attribute__((no_instrument_function)) __cyg_profile_func_enter(
-		void *func_ptr, void *caller)
+void notrace __cyg_profile_func_enter(void *func_ptr, void *caller)
 {
 	if (trace_enabled) {
 		int func;
@@ -167,8 +165,7 @@ void __attribute__((no_instrument_function)) __cyg_profile_func_enter(
  * @func_ptr:	pointer to function being entered
  * @caller:	pointer to function which called this function
  */
-void __attribute__((no_instrument_function)) __cyg_profile_func_exit(
-		void *func_ptr, void *caller)
+void notrace __cyg_profile_func_exit(void *func_ptr, void *caller)
 {
 	if (trace_enabled) {
 		trace_swap_gd();
@@ -327,7 +324,7 @@ void trace_print_stats(void)
 	puts(" calls not traced due to depth\n");
 }
 
-void __attribute__((no_instrument_function)) trace_set_enabled(int enabled)
+void notrace trace_set_enabled(int enabled)
 {
 	trace_enabled = enabled != 0;
 }
@@ -339,8 +336,7 @@ void __attribute__((no_instrument_function)) trace_set_enabled(int enabled)
  * @buff_size:	Size of trace buffer
  * Return:	0 if ok
  */
-int __attribute__((no_instrument_function)) trace_init(void *buff,
-		size_t buff_size)
+int notrace trace_init(void *buff, size_t buff_size)
 {
 	ulong func_count = gd->mon_len / FUNC_SITE_SIZE;
 	size_t needed;
@@ -404,7 +400,7 @@ int __attribute__((no_instrument_function)) trace_init(void *buff,
  *
  * Return:	0 if ok, -ENOSPC if not enough memory is available
  */
-int __attribute__((no_instrument_function)) trace_early_init(void)
+int notrace trace_early_init(void)
 {
 	ulong func_count = gd->mon_len / FUNC_SITE_SIZE;
 	size_t buff_size = CONFIG_TRACE_EARLY_SIZE;
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 02/14] arm: Support trace on armv8

[Simon Glass]

Use the notrace attribute so that timer functions can be used when
tracing. This is required to avoid infinite loops when recording a trace.

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

 arch/arm/cpu/armv8/generic_timer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/cpu/armv8/generic_timer.c b/arch/arm/cpu/armv8/generic_timer.c
index f27a74b9d09..8f83372cbca 100644
--- a/arch/arm/cpu/armv8/generic_timer.c
+++ b/arch/arm/cpu/armv8/generic_timer.c
@@ -17,7 +17,7 @@ DECLARE_GLOBAL_DATA_PTR;
 /*
  * Generic timer implementation of get_tbclk()
  */
-unsigned long get_tbclk(void)
+unsigned long notrace get_tbclk(void)
 {
 	unsigned long cntfrq;
 	asm volatile("mrs %0, cntfrq_el0" : "=r" (cntfrq));
@@ -78,7 +78,7 @@ unsigned long timer_read_counter(void)
 /*
  * timer_read_counter() using the Arm Generic Timer (aka arch timer).
  */
-unsigned long timer_read_counter(void)
+unsigned long notrace timer_read_counter(void)
 {
 	unsigned long cntpct;
 
@@ -89,7 +89,7 @@ unsigned long timer_read_counter(void)
 }
 #endif
 
-uint64_t get_ticks(void)
+uint64_t notrace get_ticks(void)
 {
 	unsigned long ticks = timer_read_counter();
 
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 03/14] tpm: Add a proper Kconfig option for crc8 in SPL

[Simon Glass]

The current approach is a bit of a hack and only works for the tpm
subsystem. Add a Kconfig so that crc8 can be enabled in SPL for other
purposes.

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

 lib/Kconfig  | 17 +++++++++++++++++
 lib/Makefile |  3 ++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/lib/Kconfig b/lib/Kconfig
index def36f275ce..b51455a5f77 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -422,6 +422,7 @@ config TPM
 config SPL_TPM
 	bool "Trusted Platform Module (TPM) Support in SPL"
 	depends on SPL_DM
+	imply SPL_CRC8
 	help
 	  This enables support for TPMs which can be used to provide security
 	  features for your board. The TPM can be connected via LPC or I2C
@@ -617,6 +618,22 @@ config SPL_MD5
 	  security applications, but it can be useful for providing a quick
 	  checksum of a block of data.
 
+config CRC8
+	def_bool y
+	help
+	  Enables CRC8 support in U-Boot. This is normally required. CRC8 is
+	  a simple and fast checksumming algorithm which does a bytewise
+	  checksum with feedback to produce an 8-bit result. The code is small
+	  and it does not require a lookup table (unlike CRC32).
+
+config SPL_CRC8
+	bool "Support CRC8 in SPL"
+	help
+	  Enables CRC8 support in SPL. This is not normally required. CRC8 is
+	  a simple and fast checksumming algorithm which does a bytewise
+	  checksum with feedback to produce an 8-bit result. The code is small
+	  and it does not require a lookup table (unlike CRC32).
+
 config CRC32
 	def_bool y
 	help
diff --git a/lib/Makefile b/lib/Makefile
index d77b33e7f48..a282e40258c 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -57,12 +57,13 @@ endif
 
 obj-$(CONFIG_$(SPL_TPL_)TPM) += tpm-common.o
 ifeq ($(CONFIG_$(SPL_TPL_)TPM),y)
-obj-y += crc8.o
 obj-$(CONFIG_TPM) += tpm_api.o
 obj-$(CONFIG_TPM_V1) += tpm-v1.o
 obj-$(CONFIG_TPM_V2) += tpm-v2.o
 endif
 
+obj-$(CONFIG_$(SPL_TPL_)CRC8) += crc8.o
+
 obj-y += crypto/
 
 obj-$(CONFIG_$(SPL_TPL_)GENERATE_ACPI_TABLE) += acpi/
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 04/14] fdt: Avoid exporting fdtdec_prepare_fdt()

[Simon Glass]

This function is not used outside this file. Make it static.

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

 include/fdtdec.h |  9 ---------
 lib/fdtdec.c     | 26 +++++++++++++-------------
 2 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/include/fdtdec.h b/include/fdtdec.h
index 12355afd7fa..aa61a0fca1a 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -554,15 +554,6 @@ uint64_t fdtdec_get_uint64(const void *blob, int node, const char *prop_name,
  */
 int fdtdec_get_is_enabled(const void *blob, int node);
 
-/**
- * Make sure we have a valid fdt available to control U-Boot.
- *
- * If not, a message is printed to the console if the console is ready.
- *
- * Return: 0 if all ok, -1 if not
- */
-int fdtdec_prepare_fdt(void);
-
 /**
  * Checks that we have a valid fdt available to control U-Boot.
 
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 64c5b3da15e..6388bb8b897 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -586,24 +586,12 @@ int fdtdec_get_chosen_node(const void *blob, const char *name)
 	return fdt_path_offset(blob, prop);
 }
 
-int fdtdec_check_fdt(void)
-{
-	/*
-	 * We must have an FDT, but we cannot panic() yet since the console
-	 * is not ready. So for now, just assert(). Boards which need an early
-	 * FDT (prior to console ready) will need to make their own
-	 * arrangements and do their own checks.
-	 */
-	assert(!fdtdec_prepare_fdt());
-	return 0;
-}
-
 /*
  * This function is a little odd in that it accesses global data. At some
  * point if the architecture board.c files merge this will make more sense.
  * Even now, it is common code.
  */
-int fdtdec_prepare_fdt(void)
+static int fdtdec_prepare_fdt(void)
 {
 	if (!gd->fdt_blob || ((uintptr_t)gd->fdt_blob & 3) ||
 	    fdt_check_header(gd->fdt_blob)) {
@@ -625,6 +613,18 @@ int fdtdec_prepare_fdt(void)
 	return 0;
 }
 
+int fdtdec_check_fdt(void)
+{
+	/*
+	 * We must have an FDT, but we cannot panic() yet since the console
+	 * is not ready. So for now, just assert(). Boards which need an early
+	 * FDT (prior to console ready) will need to make their own
+	 * arrangements and do their own checks.
+	 */
+	assert(!fdtdec_prepare_fdt());
+	return 0;
+}
+
 int fdtdec_lookup_phandle(const void *blob, int node, const char *prop_name)
 {
 	const u32 *phandle;
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 05/14] fdt: Drop ifdefs in fdtdec_prepare_fdt()

[Simon Glass]

This function is a bit messy with several #ifdefs. Convert them to use C
for the conditions.

Rewrite the function comment since most of it is stale.

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

 lib/fdtdec.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 6388bb8b897..891b274aa3c 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -13,6 +13,7 @@
 #include <log.h>
 #include <malloc.h>
 #include <net.h>
+#include <spl.h>
 #include <env.h>
 #include <errno.h>
 #include <fdtdec.h>
@@ -586,30 +587,31 @@ int fdtdec_get_chosen_node(const void *blob, const char *name)
 	return fdt_path_offset(blob, prop);
 }
 
-/*
- * This function is a little odd in that it accesses global data. At some
- * point if the architecture board.c files merge this will make more sense.
- * Even now, it is common code.
+/**
+ * fdtdec_prepare_fdt() - Check we have a valid fdt available to control U-Boot
+ *
+ * If not, a message is printed to the console if the console is ready.
+ *
+ * Return: 0 if all ok, -ENOENT if not
  */
 static int fdtdec_prepare_fdt(void)
 {
 	if (!gd->fdt_blob || ((uintptr_t)gd->fdt_blob & 3) ||
 	    fdt_check_header(gd->fdt_blob)) {
-#ifdef CONFIG_SPL_BUILD
-		puts("Missing DTB\n");
-#else
-		printf("No valid device tree binary found at %p\n",
-		       gd->fdt_blob);
-# ifdef DEBUG
-		if (gd->fdt_blob) {
-			printf("fdt_blob=%p\n", gd->fdt_blob);
+		if (spl_phase() <= PHASE_SPL) {
+			puts("Missing DTB\n");
+		} else {
+			printf("No valid device tree binary found at %p\n",
+			       gd->fdt_blob);
+			if (_DEBUG && gd->fdt_blob) {
+				printf("fdt_blob=%p\n", gd->fdt_blob);
 			print_buffer((ulong)gd->fdt_blob, gd->fdt_blob, 4,
 				     32, 0);
+			}
 		}
-# endif
-#endif
-		return -1;
+		return -ENOENT;
 	}
+
 	return 0;
 }
 
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 06/14] fdt: Pass the device tree to fdtdec_prepare_fdt()

[Simon Glass]

This function uses gd->fdt_blob a lot and cannot be used to check any
other device tree. Use a parameter instead.

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

 lib/fdtdec.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 891b274aa3c..03c9ceab773 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -590,23 +590,23 @@ int fdtdec_get_chosen_node(const void *blob, const char *name)
 /**
  * fdtdec_prepare_fdt() - Check we have a valid fdt available to control U-Boot
  *
+ * @blob: Blob to check
+ *
  * If not, a message is printed to the console if the console is ready.
  *
  * Return: 0 if all ok, -ENOENT if not
  */
-static int fdtdec_prepare_fdt(void)
+static int fdtdec_prepare_fdt(const void *blob)
 {
-	if (!gd->fdt_blob || ((uintptr_t)gd->fdt_blob & 3) ||
-	    fdt_check_header(gd->fdt_blob)) {
+	if (!blob || ((uintptr_t)blob & 3) || fdt_check_header(blob)) {
 		if (spl_phase() <= PHASE_SPL) {
 			puts("Missing DTB\n");
 		} else {
 			printf("No valid device tree binary found at %p\n",
-			       gd->fdt_blob);
-			if (_DEBUG && gd->fdt_blob) {
-				printf("fdt_blob=%p\n", gd->fdt_blob);
-			print_buffer((ulong)gd->fdt_blob, gd->fdt_blob, 4,
-				     32, 0);
+			       blob);
+			if (_DEBUG && blob) {
+				printf("fdt_blob=%p\n", blob);
+				print_buffer((ulong)blob, blob, 4, 32, 0);
 			}
 		}
 		return -ENOENT;
@@ -623,7 +623,7 @@ int fdtdec_check_fdt(void)
 	 * FDT (prior to console ready) will need to make their own
 	 * arrangements and do their own checks.
 	 */
-	assert(!fdtdec_prepare_fdt());
+	assert(!fdtdec_prepare_fdt(gd->fdt_blob));
 	return 0;
 }
 
@@ -1668,7 +1668,7 @@ int fdtdec_setup(void)
 	if (CONFIG_IS_ENABLED(MULTI_DTB_FIT))
 		setup_multi_dtb_fit();
 
-	ret = fdtdec_prepare_fdt();
+	ret = fdtdec_prepare_fdt(gd->fdt_blob);
 	if (!ret)
 		ret = fdtdec_board_setup(gd->fdt_blob);
 	oftree_reset();
@@ -1700,7 +1700,7 @@ int fdtdec_resetup(int *rescan)
 
 		*rescan = 1;
 		gd->fdt_blob = fdt_blob;
-		return fdtdec_prepare_fdt();
+		return fdtdec_prepare_fdt(fdt_blob);
 	}
 
 	/*
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 07/14] fdt: Check for overlapping data and FDT

[Simon Glass]

If the FDT overlaps with the data region of the image, or with the stack,
it can become corrupted before relocation. Add a check for this, behind a
debug flag, as it can be very confusing and time-consuming to debug.

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

 lib/fdtdec.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 03c9ceab773..8d5c68860ec 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -1231,6 +1231,29 @@ static void *fdt_find_separate(void)
 #else
 	/* FDT is at end of image */
 	fdt_blob = (ulong *)&_end;
+
+	if (_DEBUG && !fdtdec_prepare_fdt(fdt_blob)) {
+		int stack_ptr;
+		const void *top = fdt_blob + fdt_totalsize(fdt_blob);
+
+		/*
+		 * Perform a sanity check on the memory layout. If this fails,
+		 * it indicates that the device tree is positioned above the
+		 * global data pointer or the stack pointer. This should not
+		 * happen.
+		 *
+		 * If this fails, check that SYS_INIT_SP_ADDR has enough space
+		 * below it for SYS_MALLOC_F_LEN and global_data, as well as the
+		 * stack, without overwriting the device tree or U-Boot itself.
+		 * Since the device tree is sitting at _end (the start of the
+		 * BSS region), we need the top of the device tree to be below
+		 * any memory allocated by board_init_f_alloc_reserve().
+		 */
+		if (top > (void *)gd || top > (void *)&stack_ptr) {
+			printf("FDT %p gd %p\n", fdt_blob, gd);
+			panic("FDT overlap");
+		}
+	}
 #endif
 
 	return fdt_blob;
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 08/14] trace: Move trace pointer to data section

[Simon Glass]

This can be written before relocation. Move it to the data section, since
accessing BSS before relocation is not permitted.

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

 lib/trace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/trace.c b/lib/trace.c
index 880d90ebd5c..b9dc6d2e4b5 100644
--- a/lib/trace.c
+++ b/lib/trace.c
@@ -40,7 +40,8 @@ struct trace_hdr {
 	int max_depth;
 };
 
-static struct trace_hdr *hdr;	/* Pointer to start of trace buffer */
+/* Pointer to start of trace buffer */
+static struct trace_hdr *hdr __section(".data");
 
 static inline uintptr_t __attribute__((no_instrument_function))
 		func_ptr_to_num(void *func_ptr)
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 09/14] mkimage: Add a few more messages for FIT failures

[Simon Glass]

Add messages to make it clearer which part of the FIT creation is failing.
This can happen when an invalid 'algo' property is provided in the .its
file.

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

 tools/fit_image.c  | 4 +++-
 tools/image-host.c | 6 +++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/fit_image.c b/tools/fit_image.c
index 923a9755b70..8a18b1b0ba9 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -36,8 +36,10 @@ static int fit_add_file_data(struct image_tool_params *params, size_t size_inc,
 
 	tfd = mmap_fdt(params->cmdname, tmpfile, size_inc, &ptr, &sbuf, true,
 		       false);
-	if (tfd < 0)
+	if (tfd < 0) {
+		fprintf(stderr, "Cannot map FDT file '%s'\n", tmpfile);
 		return -EIO;
+	}
 
 	if (params->keydest) {
 		struct stat dest_sbuf;
diff --git a/tools/image-host.c b/tools/image-host.c
index 4e0512be634..4a24dee8153 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -1292,8 +1292,12 @@ int fit_add_verification_data(const char *keydir, const char *keyfile,
 		ret = fit_image_add_verification_data(keydir, keyfile, keydest,
 				fit, noffset, comment, require_keys, engine_id,
 				cmdname, algo_name);
-		if (ret)
+		if (ret) {
+			printf("Can't add verification data for node '%s' (%s)\n",
+			       fdt_get_name(fit, noffset, NULL),
+			       fdt_strerror(ret));
 			return ret;
+		}
 	}
 
 	/* If there are no keys, we can't sign configurations */
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 10/14] trace: Adjust flags in proftool

[Simon Glass]

The flags in this tool don't match the comments or help. Also the variable
names are quite confusing. Update them for consistency.

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

 tools/proftool.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/proftool.c b/tools/proftool.c
index ea7d07a277e..0ba84d30c19 100644
--- a/tools/proftool.c
+++ b/tools/proftool.c
@@ -81,14 +81,15 @@ static void outf(int level, const char *fmt, ...)
 static void usage(void)
 {
 	fprintf(stderr,
-		"Usage: proftool -cds -v3 <cmd> <profdata>\n"
+		"Usage: proftool [-cmtv] <cmd> <profdata>\n"
 		"\n"
 		"Commands\n"
 		"   dump-ftrace\t\tDump out textual data in ftrace format\n"
 		"\n"
 		"Options:\n"
+		"   -c <cfg>\tSpecific config file\n"
 		"   -m <map>\tSpecify Systen.map file\n"
-		"   -t <trace>\tSpecific trace data file (from U-Boot)\n"
+		"   -t <fname>\tSpecify trace data file (from U-Boot 'trace calls')\n"
 		"   -v <0-4>\tSpecify verbosity\n");
 	exit(EXIT_FAILURE);
 }
@@ -562,23 +563,23 @@ static int prof_tool(int argc, char *const argv[],
 int main(int argc, char *argv[])
 {
 	const char *map_fname = "System.map";
-	const char *prof_fname = NULL;
-	const char *trace_config_fname = NULL;
+	const char *trace_fname = NULL;
+	const char *config_fname = NULL;
 	int opt;
 
 	verbose = 2;
-	while ((opt = getopt(argc, argv, "m:p:t:v:")) != -1) {
+	while ((opt = getopt(argc, argv, "c:m:t:v:")) != -1) {
 		switch (opt) {
-		case 'm':
-			map_fname = optarg;
+		case 'c':
+			config_fname = optarg;
 			break;
 
-		case 'p':
-			prof_fname = optarg;
+		case 'm':
+			map_fname = optarg;
 			break;
 
 		case 't':
-			trace_config_fname = optarg;
+			trace_fname = optarg;
 			break;
 
 		case 'v':
@@ -594,6 +595,5 @@ int main(int argc, char *argv[])
 		usage();
 
 	debug("Debug enabled\n");
-	return prof_tool(argc, argv, prof_fname, map_fname,
-			 trace_config_fname);
+	return prof_tool(argc, argv, trace_fname, map_fname, config_fname);
 }
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 11/14] trace: Update trace-format generator for newer version

[Simon Glass]

This now includes flags and the layout has changed slightly in recent
versions of Linux. Update the generator accordingly.

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

 tools/proftool.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/tools/proftool.c b/tools/proftool.c
index 0ba84d30c19..b66ea556486 100644
--- a/tools/proftool.c
+++ b/tools/proftool.c
@@ -163,7 +163,7 @@ static int read_data(FILE *fin, void *buff, int size)
 	if (!err)
 		return 1;
 	if (err != size) {
-		error("Cannot read profile file at pos %ld\n", ftell(fin));
+		error("Cannot read profile file at pos %lx\n", ftell(fin));
 		return -1;
 	}
 	return 0;
@@ -496,10 +496,17 @@ static int make_ftrace(void)
 	int missing_count = 0, skip_count = 0;
 	int i;
 
-	printf("# tracer: ftrace\n"
-		"#\n"
-		"#           TASK-PID   CPU#    TIMESTAMP  FUNCTION\n"
-		"#              | |      |          |         |\n");
+	printf("# tracer: function\n"
+	      "#\n"
+	      "# entries-in-buffer/entries-written: 140080/250280   #P:4\n"
+	      "#\n"
+	      "#                              _-----=> irqs-off\n"
+	      "#                             / _----=> need-resched\n"
+	      "#                            | / _---=> hardirq/softirq\n"
+	      "#                            || / _--=> preempt-depth\n"
+	      "#                            ||| /     delay\n"
+	      "#           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION\n"
+	      "#              | |       |   ||||       |         |\n");
 	for (i = 0, call = call_list; i < call_count; i++, call++) {
 		struct func_info *func = find_func_by_offset(call->func);
 		ulong time = call->flags & FUNCF_TIMESTAMP_MASK;
@@ -521,7 +528,7 @@ static int make_ftrace(void)
 			continue;
 		}
 
-		printf("%16s-%-5d [01] %lu.%06lu: ", "uboot", 1,
+		printf("%16s-%-5d [000] ....  %lu.%06lu: ", "uboot", 1,
 		       time / 1000000, time % 1000000);
 
 		out_func(call->func, 0, " <- ");
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 12/14] trace: Don't require TIMER_EARLY

[Simon Glass]

Some platforms cannot honour this and don't need trace before relocation.
Use 'imply' instead, so boards can disable this.

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

 lib/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/Kconfig b/lib/Kconfig
index b51455a5f77..1643f7d878a 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -316,7 +316,7 @@ config BITREVERSE
 config TRACE
 	bool "Support for tracing of function calls and timing"
 	imply CMD_TRACE
-	select TIMER_EARLY
+	imply TIMER_EARLY
 	help
 	  Enables function tracing within U-Boot. This allows recording of call
 	  traces including timing information. The command can write data to
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 13/14] rockchip: Enable bootstage on rockpro64

[Simon Glass]

This board is useful for benchmarking overall U-Boot performance. Enable
the bootstage feature so we get a report.

Since this returns to the boot rom before finishing executing
board_init_r() in SPL, add a few bootstage calls so that we can collect
timing from TPL.

For the stash region, use a portion of SRAM, 64KB below the stack top.
This allows the TPL image to be up to nearly 120KB (it is typically about
64KB). SPL normally runs from SDRAM at 0, so can use the same stash
region.

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

 arch/arm/mach-rockchip/tpl.c       | 18 +++++++++++++++---
 common/spl/spl.c                   |  2 +-
 configs/rockpro64-rk3399_defconfig |  8 ++++++++
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-rockchip/tpl.c b/arch/arm/mach-rockchip/tpl.c
index ed46a9ad286..2b7f852a28f 100644
--- a/arch/arm/mach-rockchip/tpl.c
+++ b/arch/arm/mach-rockchip/tpl.c
@@ -4,6 +4,7 @@
  */
 
 #include <common.h>
+#include <bootstage.h>
 #include <debug_uart.h>
 #include <dm.h>
 #include <hang.h>
@@ -70,15 +71,17 @@ void board_init_f(ulong dummy)
 				U_BOOT_TIME ")\n");
 #endif
 #endif
+	/* Init secure timer */
+	rockchip_stimer_init();
+
+	puts("u_boot_first_phase(): ");
+	printhex8(u_boot_first_phase());
 	ret = spl_early_init();
 	if (ret) {
 		debug("spl_early_init() failed: %d\n", ret);
 		hang();
 	}
 
-	/* Init secure timer */
-	rockchip_stimer_init();
-
 	/* Init ARM arch timer */
 	if (IS_ENABLED(CONFIG_SYS_ARCH_TIMER))
 		timer_init();
@@ -93,6 +96,15 @@ void board_init_f(ulong dummy)
 int board_return_to_bootrom(struct spl_image_info *spl_image,
 			    struct spl_boot_device *bootdev)
 {
+#ifdef CONFIG_BOOTSTAGE_STASH
+	int ret;
+
+	bootstage_mark_name(BOOTSTAGE_ID_END_TPL, "end tpl");
+	ret = bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
+			      CONFIG_BOOTSTAGE_STASH_SIZE);
+	if (ret)
+		debug("Failed to stash bootstage: err=%d\n", ret);
+#endif
 	back_to_bootrom(BROM_BOOT_NEXTSTAGE);
 
 	return 0;
diff --git a/common/spl/spl.c b/common/spl/spl.c
index 1d2e8fda728..71cded774b8 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -910,7 +910,7 @@ void preloader_console_init(void)
 	gd->have_console = 1;
 
 #if CONFIG_IS_ENABLED(BANNER_PRINT)
-	puts("\nU-Boot " SPL_TPL_NAME " " PLAIN_VERSION " (" U_BOOT_DATE " - "
+	puts("\nxU-Boot " SPL_TPL_NAME " " PLAIN_VERSION " (" U_BOOT_DATE " - "
 	     U_BOOT_TIME " " U_BOOT_TZ ")\n");
 #endif
 #ifdef CONFIG_SPL_DISPLAY_PRINT
diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
index 5b8d678f6bb..2f1ae156bd4 100644
--- a/configs/rockpro64-rk3399_defconfig
+++ b/configs/rockpro64-rk3399_defconfig
@@ -9,6 +9,7 @@ CONFIG_ENV_OFFSET=0x3F8000
 CONFIG_DEFAULT_DEVICE_TREE="rk3399-rockpro64"
 CONFIG_ROCKCHIP_RK3399=y
 CONFIG_TARGET_ROCKPRO64_RK3399=y
+CONFIG_BOOTSTAGE_STASH_ADDR=0xff8e0000
 CONFIG_DEBUG_UART_BASE=0xFF1A0000
 CONFIG_DEBUG_UART_CLOCK=24000000
 CONFIG_SPL_SPI_FLASH_SUPPORT=y
@@ -17,6 +18,12 @@ CONFIG_SYS_LOAD_ADDR=0x800800
 CONFIG_DEBUG_UART=y
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
 CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x300000
+CONFIG_BOOTSTAGE=y
+CONFIG_SPL_BOOTSTAGE=y
+CONFIG_TPL_BOOTSTAGE=y
+CONFIG_BOOTSTAGE_REPORT=y
+CONFIG_SPL_BOOTSTAGE_RECORD_COUNT=10
+CONFIG_BOOTSTAGE_STASH=y
 CONFIG_USE_PREBOOT=y
 CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-rockpro64.dtb"
 CONFIG_DISPLAY_BOARDINFO_LATE=y
@@ -40,6 +47,7 @@ CONFIG_CMD_PCI=y
 CONFIG_CMD_USB=y
 # CONFIG_CMD_SETEXPR is not set
 CONFIG_CMD_TIME=y
+CONFIG_CMD_BOOTSTAGE=y
 CONFIG_SPL_OF_CONTROL=y
 CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents"
 CONFIG_ENV_IS_IN_SPI_FLASH=y
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH 14/14] Revert "fdtdec: drop needlessly convoluted CONFIG_PHANDLE_CHECK_SEQ"

[Simon Glass]

The fdt_path_offset() function is slow since it must scan the tree.
This substantial overhead now applies to all boards.

The original code may not be ideal but it is fit for purpose and is only
needed on a few boards.

Reverting this reduces time to set up driver model by about 30ms.

Before revert:

Accumulated time:
                47,170  dm_r
                53,237  dm_spl
               572,986  dm_f

Accumulated time:
                44,598  dm_r
                50,347  dm_spl
               549,133  dm_f

This reverts commit 26f981f295d00351b6f0c69b5317b254b2361cc0.

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

Changes in v1:
- Add detail of impact

 configs/am65x_evm_a53_defconfig  | 1 +
 configs/evb-ast2600_defconfig    | 1 +
 configs/sama7g5ek_mmc1_defconfig | 1 +
 configs/sama7g5ek_mmc_defconfig  | 1 +
 lib/Kconfig                      | 7 +++++++
 lib/fdtdec.c                     | 7 +++++--
 6 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/configs/am65x_evm_a53_defconfig b/configs/am65x_evm_a53_defconfig
index 84ca386eafd..e4b936b6ebb 100644
--- a/configs/am65x_evm_a53_defconfig
+++ b/configs/am65x_evm_a53_defconfig
@@ -178,3 +178,4 @@ CONFIG_USB_GADGET_VENDOR_NUM=0x0451
 CONFIG_USB_GADGET_PRODUCT_NUM=0x6162
 CONFIG_USB_GADGET_DOWNLOAD=y
 CONFIG_OF_LIBFDT_OVERLAY=y
+CONFIG_PHANDLE_CHECK_SEQ=y
diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
index 11f3d57a8f4..ae84b4962a2 100644
--- a/configs/evb-ast2600_defconfig
+++ b/configs/evb-ast2600_defconfig
@@ -119,3 +119,4 @@ CONFIG_WDT=y
 CONFIG_SHA384=y
 CONFIG_HEXDUMP=y
 # CONFIG_EFI_LOADER is not set
+CONFIG_PHANDLE_CHECK_SEQ=y
diff --git a/configs/sama7g5ek_mmc1_defconfig b/configs/sama7g5ek_mmc1_defconfig
index f004e448039..ecb4dfc7854 100644
--- a/configs/sama7g5ek_mmc1_defconfig
+++ b/configs/sama7g5ek_mmc1_defconfig
@@ -79,3 +79,4 @@ CONFIG_TIMER=y
 CONFIG_MCHP_PIT64B_TIMER=y
 CONFIG_OF_LIBFDT_OVERLAY=y
 # CONFIG_EFI_LOADER_HII is not set
+CONFIG_PHANDLE_CHECK_SEQ=y
diff --git a/configs/sama7g5ek_mmc_defconfig b/configs/sama7g5ek_mmc_defconfig
index 5b42fc63f30..1d5bccd15b3 100644
--- a/configs/sama7g5ek_mmc_defconfig
+++ b/configs/sama7g5ek_mmc_defconfig
@@ -79,3 +79,4 @@ CONFIG_TIMER=y
 CONFIG_MCHP_PIT64B_TIMER=y
 CONFIG_OF_LIBFDT_OVERLAY=y
 # CONFIG_EFI_LOADER_HII is not set
+CONFIG_PHANDLE_CHECK_SEQ=y
diff --git a/lib/Kconfig b/lib/Kconfig
index 1643f7d878a..4e6c1a53da7 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -1044,6 +1044,13 @@ config LMB_RESERVED_REGIONS
 	  Define the number of supported reserved regions in the library logical
 	  memory blocks.
 
+config PHANDLE_CHECK_SEQ
+	bool "Enable phandle check while getting sequence number"
+	help
+	  When there are multiple device tree nodes with same name,
+          enable this config option to distinguish them using
+	  phandles in fdtdec_get_alias_seq() function.
+
 endmenu
 
 menu "FWU Multi Bank Updates"
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 8d5c68860ec..0827e16859f 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -519,8 +519,11 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int offset,
 		 * Adding an extra check to distinguish DT nodes with
 		 * same name
 		 */
-		if (offset != fdt_path_offset(blob, prop))
-			continue;
+		if (IS_ENABLED(CONFIG_PHANDLE_CHECK_SEQ)) {
+			if (fdt_get_phandle(blob, offset) !=
+			    fdt_get_phandle(blob, fdt_path_offset(blob, prop)))
+				continue;
+		}
 
 		val = trailing_strtol(name);
 		if (val != -1) {
-- 
2.39.0.314.g84b9a713c41-goog

Re: [PATCH 03/14] tpm: Add a proper Kconfig option for crc8 in SPL

[Pali Rohár]

On Wednesday 21 December 2022 16:08:17 Simon Glass wrote:
> The current approach is a bit of a hack and only works for the tpm
> subsystem. Add a Kconfig so that crc8 can be enabled in SPL for other
> purposes.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  lib/Kconfig  | 17 +++++++++++++++++
>  lib/Makefile |  3 ++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/Kconfig b/lib/Kconfig
> index def36f275ce..b51455a5f77 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -422,6 +422,7 @@ config TPM
>  config SPL_TPM
>  	bool "Trusted Platform Module (TPM) Support in SPL"
>  	depends on SPL_DM
> +	imply SPL_CRC8
>  	help
>  	  This enables support for TPMs which can be used to provide security
>  	  features for your board. The TPM can be connected via LPC or I2C
> @@ -617,6 +618,22 @@ config SPL_MD5
>  	  security applications, but it can be useful for providing a quick
>  	  checksum of a block of data.
>  
> +config CRC8
> +	def_bool y
> +	help
> +	  Enables CRC8 support in U-Boot. This is normally required. CRC8 is
> +	  a simple and fast checksumming algorithm which does a bytewise
> +	  checksum with feedback to produce an 8-bit result. The code is small
> +	  and it does not require a lookup table (unlike CRC32).
> +
> +config SPL_CRC8
> +	bool "Support CRC8 in SPL"

Should be there some symbol dependency on SPL?

> +	help
> +	  Enables CRC8 support in SPL. This is not normally required. CRC8 is
> +	  a simple and fast checksumming algorithm which does a bytewise
> +	  checksum with feedback to produce an 8-bit result. The code is small
> +	  and it does not require a lookup table (unlike CRC32).
> +
>  config CRC32
>  	def_bool y
>  	help
> diff --git a/lib/Makefile b/lib/Makefile
> index d77b33e7f48..a282e40258c 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -57,12 +57,13 @@ endif
>  
>  obj-$(CONFIG_$(SPL_TPL_)TPM) += tpm-common.o
>  ifeq ($(CONFIG_$(SPL_TPL_)TPM),y)
> -obj-y += crc8.o
>  obj-$(CONFIG_TPM) += tpm_api.o
>  obj-$(CONFIG_TPM_V1) += tpm-v1.o
>  obj-$(CONFIG_TPM_V2) += tpm-v2.o
>  endif
>  
> +obj-$(CONFIG_$(SPL_TPL_)CRC8) += crc8.o
> +
>  obj-y += crypto/
>  
>  obj-$(CONFIG_$(SPL_TPL_)GENERATE_ACPI_TABLE) += acpi/
> -- 
> 2.39.0.314.g84b9a713c41-goog
> 

Re: [PATCH 13/14] rockchip: Enable bootstage on rockpro64

[Pali Rohár]

On Wednesday 21 December 2022 16:08:27 Simon Glass wrote:
> This board is useful for benchmarking overall U-Boot performance. Enable
> the bootstage feature so we get a report.
> 
> Since this returns to the boot rom before finishing executing
> board_init_r() in SPL, add a few bootstage calls so that we can collect
> timing from TPL.
> 
> For the stash region, use a portion of SRAM, 64KB below the stack top.
> This allows the TPL image to be up to nearly 120KB (it is typically about
> 64KB). SPL normally runs from SDRAM at 0, so can use the same stash
> region.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  arch/arm/mach-rockchip/tpl.c       | 18 +++++++++++++++---
>  common/spl/spl.c                   |  2 +-
>  configs/rockpro64-rk3399_defconfig |  8 ++++++++
>  3 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-rockchip/tpl.c b/arch/arm/mach-rockchip/tpl.c
> index ed46a9ad286..2b7f852a28f 100644
> --- a/arch/arm/mach-rockchip/tpl.c
> +++ b/arch/arm/mach-rockchip/tpl.c
> @@ -4,6 +4,7 @@
>   */
>  
>  #include <common.h>
> +#include <bootstage.h>
>  #include <debug_uart.h>
>  #include <dm.h>
>  #include <hang.h>
> @@ -70,15 +71,17 @@ void board_init_f(ulong dummy)
>  				U_BOOT_TIME ")\n");
>  #endif
>  #endif
> +	/* Init secure timer */
> +	rockchip_stimer_init();
> +
> +	puts("u_boot_first_phase(): ");
> +	printhex8(u_boot_first_phase());
>  	ret = spl_early_init();
>  	if (ret) {
>  		debug("spl_early_init() failed: %d\n", ret);
>  		hang();
>  	}
>  
> -	/* Init secure timer */
> -	rockchip_stimer_init();
> -
>  	/* Init ARM arch timer */
>  	if (IS_ENABLED(CONFIG_SYS_ARCH_TIMER))
>  		timer_init();
> @@ -93,6 +96,15 @@ void board_init_f(ulong dummy)
>  int board_return_to_bootrom(struct spl_image_info *spl_image,
>  			    struct spl_boot_device *bootdev)
>  {
> +#ifdef CONFIG_BOOTSTAGE_STASH
> +	int ret;
> +
> +	bootstage_mark_name(BOOTSTAGE_ID_END_TPL, "end tpl");
> +	ret = bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
> +			      CONFIG_BOOTSTAGE_STASH_SIZE);
> +	if (ret)
> +		debug("Failed to stash bootstage: err=%d\n", ret);
> +#endif
>  	back_to_bootrom(BROM_BOOT_NEXTSTAGE);
>  
>  	return 0;
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index 1d2e8fda728..71cded774b8 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -910,7 +910,7 @@ void preloader_console_init(void)
>  	gd->have_console = 1;
>  
>  #if CONFIG_IS_ENABLED(BANNER_PRINT)
> -	puts("\nU-Boot " SPL_TPL_NAME " " PLAIN_VERSION " (" U_BOOT_DATE " - "
> +	puts("\nxU-Boot " SPL_TPL_NAME " " PLAIN_VERSION " (" U_BOOT_DATE " - "

Why 'x'?

>  	     U_BOOT_TIME " " U_BOOT_TZ ")\n");
>  #endif
>  #ifdef CONFIG_SPL_DISPLAY_PRINT
> diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
> index 5b8d678f6bb..2f1ae156bd4 100644
> --- a/configs/rockpro64-rk3399_defconfig
> +++ b/configs/rockpro64-rk3399_defconfig
> @@ -9,6 +9,7 @@ CONFIG_ENV_OFFSET=0x3F8000
>  CONFIG_DEFAULT_DEVICE_TREE="rk3399-rockpro64"
>  CONFIG_ROCKCHIP_RK3399=y
>  CONFIG_TARGET_ROCKPRO64_RK3399=y
> +CONFIG_BOOTSTAGE_STASH_ADDR=0xff8e0000
>  CONFIG_DEBUG_UART_BASE=0xFF1A0000
>  CONFIG_DEBUG_UART_CLOCK=24000000
>  CONFIG_SPL_SPI_FLASH_SUPPORT=y
> @@ -17,6 +18,12 @@ CONFIG_SYS_LOAD_ADDR=0x800800
>  CONFIG_DEBUG_UART=y
>  CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
>  CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x300000
> +CONFIG_BOOTSTAGE=y
> +CONFIG_SPL_BOOTSTAGE=y
> +CONFIG_TPL_BOOTSTAGE=y
> +CONFIG_BOOTSTAGE_REPORT=y
> +CONFIG_SPL_BOOTSTAGE_RECORD_COUNT=10
> +CONFIG_BOOTSTAGE_STASH=y
>  CONFIG_USE_PREBOOT=y
>  CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-rockpro64.dtb"
>  CONFIG_DISPLAY_BOARDINFO_LATE=y
> @@ -40,6 +47,7 @@ CONFIG_CMD_PCI=y
>  CONFIG_CMD_USB=y
>  # CONFIG_CMD_SETEXPR is not set
>  CONFIG_CMD_TIME=y
> +CONFIG_CMD_BOOTSTAGE=y
>  CONFIG_SPL_OF_CONTROL=y
>  CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents"
>  CONFIG_ENV_IS_IN_SPI_FLASH=y
> -- 
> 2.39.0.314.g84b9a713c41-goog
> 

Re: [PATCH 13/14] rockchip: Enable bootstage on rockpro64

[Simon Glass]

Hi Pali,

On Wed, 21 Dec 2022 at 16:12, Pali Rohár <pali@kernel.org> wrote:
>
> On Wednesday 21 December 2022 16:08:27 Simon Glass wrote:
> > This board is useful for benchmarking overall U-Boot performance. Enable
> > the bootstage feature so we get a report.
> >
> > Since this returns to the boot rom before finishing executing
> > board_init_r() in SPL, add a few bootstage calls so that we can collect
> > timing from TPL.
> >
> > For the stash region, use a portion of SRAM, 64KB below the stack top.
> > This allows the TPL image to be up to nearly 120KB (it is typically about
> > 64KB). SPL normally runs from SDRAM at 0, so can use the same stash
> > region.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  arch/arm/mach-rockchip/tpl.c       | 18 +++++++++++++++---
> >  common/spl/spl.c                   |  2 +-
> >  configs/rockpro64-rk3399_defconfig |  8 ++++++++
> >  3 files changed, 24 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/mach-rockchip/tpl.c b/arch/arm/mach-rockchip/tpl.c
> > index ed46a9ad286..2b7f852a28f 100644
> > --- a/arch/arm/mach-rockchip/tpl.c
> > +++ b/arch/arm/mach-rockchip/tpl.c
> > @@ -4,6 +4,7 @@
> >   */
> >
> >  #include <common.h>
> > +#include <bootstage.h>
> >  #include <debug_uart.h>
> >  #include <dm.h>
> >  #include <hang.h>
> > @@ -70,15 +71,17 @@ void board_init_f(ulong dummy)
> >                               U_BOOT_TIME ")\n");
> >  #endif
> >  #endif
> > +     /* Init secure timer */
> > +     rockchip_stimer_init();
> > +
> > +     puts("u_boot_first_phase(): ");
> > +     printhex8(u_boot_first_phase());
> >       ret = spl_early_init();
> >       if (ret) {
> >               debug("spl_early_init() failed: %d\n", ret);
> >               hang();
> >       }
> >
> > -     /* Init secure timer */
> > -     rockchip_stimer_init();
> > -
> >       /* Init ARM arch timer */
> >       if (IS_ENABLED(CONFIG_SYS_ARCH_TIMER))
> >               timer_init();
> > @@ -93,6 +96,15 @@ void board_init_f(ulong dummy)
> >  int board_return_to_bootrom(struct spl_image_info *spl_image,
> >                           struct spl_boot_device *bootdev)
> >  {
> > +#ifdef CONFIG_BOOTSTAGE_STASH
> > +     int ret;
> > +
> > +     bootstage_mark_name(BOOTSTAGE_ID_END_TPL, "end tpl");
> > +     ret = bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
> > +                           CONFIG_BOOTSTAGE_STASH_SIZE);
> > +     if (ret)
> > +             debug("Failed to stash bootstage: err=%d\n", ret);
> > +#endif
> >       back_to_bootrom(BROM_BOOT_NEXTSTAGE);
> >
> >       return 0;
> > diff --git a/common/spl/spl.c b/common/spl/spl.c
> > index 1d2e8fda728..71cded774b8 100644
> > --- a/common/spl/spl.c
> > +++ b/common/spl/spl.c
> > @@ -910,7 +910,7 @@ void preloader_console_init(void)
> >       gd->have_console = 1;
> >
> >  #if CONFIG_IS_ENABLED(BANNER_PRINT)
> > -     puts("\nU-Boot " SPL_TPL_NAME " " PLAIN_VERSION " (" U_BOOT_DATE " - "
> > +     puts("\nxU-Boot " SPL_TPL_NAME " " PLAIN_VERSION " (" U_BOOT_DATE " - "
>
> Why 'x'?
>

Oops, and there is another debugging bit added also.

Regards,
Simon

Re: [PATCH 14/14] Revert "fdtdec: drop needlessly convoluted CONFIG_PHANDLE_CHECK_SEQ"

[Simon Glass]

The fdt_path_offset() function is slow since it must scan the tree.
This substantial overhead now applies to all boards.

The original code may not be ideal but it is fit for purpose and is only
needed on a few boards.

Reverting this reduces time to set up driver model by about 30ms.

Before revert:

Accumulated time:
                47,170  dm_r
                53,237  dm_spl
               572,986  dm_f

Accumulated time:
                44,598  dm_r
                50,347  dm_spl
               549,133  dm_f

This reverts commit 26f981f295d00351b6f0c69b5317b254b2361cc0.

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

Changes in v1:
- Add detail of impact

 configs/am65x_evm_a53_defconfig  | 1 +
 configs/evb-ast2600_defconfig    | 1 +
 configs/sama7g5ek_mmc1_defconfig | 1 +
 configs/sama7g5ek_mmc_defconfig  | 1 +
 lib/Kconfig                      | 7 +++++++
 lib/fdtdec.c                     | 7 +++++--
 6 files changed, 16 insertions(+), 2 deletions(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH 11/14] trace: Update trace-format generator for newer version

[Simon Glass]

This now includes flags and the layout has changed slightly in recent
versions of Linux. Update the generator accordingly.

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

 tools/proftool.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH 10/14] trace: Adjust flags in proftool

[Simon Glass]

The flags in this tool don't match the comments or help. Also the variable
names are quite confusing. Update them for consistency.

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

 tools/proftool.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH 09/14] mkimage: Add a few more messages for FIT failures

[Simon Glass]

Add messages to make it clearer which part of the FIT creation is failing.
This can happen when an invalid 'algo' property is provided in the .its
file.

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

 tools/fit_image.c  | 4 +++-
 tools/image-host.c | 6 +++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH 08/14] trace: Move trace pointer to data section

[Simon Glass]

This can be written before relocation. Move it to the data section, since
accessing BSS before relocation is not permitted.

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

 lib/trace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH 02/14] arm: Support trace on armv8

[Simon Glass]

Use the notrace attribute so that timer functions can be used when
tracing. This is required to avoid infinite loops when recording a trace.

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

 arch/arm/cpu/armv8/generic_timer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH 01/14] trace: Use notrace for short

[Simon Glass]

The attribute syntax is quite verbose. Use the macro provided for this
purpose.

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

 arch/arm/cpu/armv7/s5p-common/timer.c   |  2 +-
 arch/arm/mach-exynos/include/mach/cpu.h |  6 +++---
 arch/x86/include/asm/global_data.h      |  2 +-
 arch/x86/include/asm/msr.h              |  2 +-
 arch/x86/include/asm/u-boot-x86.h       |  2 +-
 common/spl/spl_fit.c                    |  1 +
 doc/develop/trace.rst                   |  2 +-
 lib/efi_loader/efi_freestanding.c       |  4 ++--
 lib/trace.c                             | 26 +++++++++++--------------
 9 files changed, 22 insertions(+), 25 deletions(-)

Applied to u-boot-dm/next, thanks!

Re: [PATCH 01/14] trace: Use notrace for short

[Stefan Herbrechtsmeier]

Hi Simon,

Am 22.12.2022 um 00:08 schrieb Simon Glass:
> The attribute syntax is quite verbose. Use the macro provided for this
> purpose.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   arch/arm/cpu/armv7/s5p-common/timer.c   |  2 +-
>   arch/arm/mach-exynos/include/mach/cpu.h |  6 +++---
>   arch/x86/include/asm/global_data.h      |  2 +-
>   arch/x86/include/asm/msr.h              |  2 +-
>   arch/x86/include/asm/u-boot-x86.h       |  2 +-
>   common/spl/spl_fit.c                    |  1 +
>   doc/develop/trace.rst                   |  2 +-
>   lib/efi_loader/efi_freestanding.c       |  4 ++--
>   lib/trace.c                             | 26 +++++++++++--------------
>   9 files changed, 22 insertions(+), 25 deletions(-)
>

[snip]

> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 08da7fed88e..0e026bb3d8e 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -599,6 +599,7 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node,
>   			debug("Ignoring compatible = %s property\n",
>   			      compatible);
>   	}
> +	return 0;
>   
>   	ret = fpga_load(devnum, (void *)fpga_image->load_addr,
>   			fpga_image->size, BIT_FULL, flags);


This return makes the spl_fit_upload_fpga function useless. I assume 
this change was by accident.

Regards
   Stefan

Re: [PATCH 01/14] trace: Use notrace for short

[Simon Glass]

Hi Stefan,

On Tue, 24 Jan 2023 at 01:12, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> Hi Simon,
>
> Am 22.12.2022 um 00:08 schrieb Simon Glass:
> > The attribute syntax is quite verbose. Use the macro provided for this
> > purpose.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >   arch/arm/cpu/armv7/s5p-common/timer.c   |  2 +-
> >   arch/arm/mach-exynos/include/mach/cpu.h |  6 +++---
> >   arch/x86/include/asm/global_data.h      |  2 +-
> >   arch/x86/include/asm/msr.h              |  2 +-
> >   arch/x86/include/asm/u-boot-x86.h       |  2 +-
> >   common/spl/spl_fit.c                    |  1 +
> >   doc/develop/trace.rst                   |  2 +-
> >   lib/efi_loader/efi_freestanding.c       |  4 ++--
> >   lib/trace.c                             | 26 +++++++++++--------------
> >   9 files changed, 22 insertions(+), 25 deletions(-)
> >
>
> [snip]
>
> > diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> > index 08da7fed88e..0e026bb3d8e 100644
> > --- a/common/spl/spl_fit.c
> > +++ b/common/spl/spl_fit.c
> > @@ -599,6 +599,7 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node,
> >                       debug("Ignoring compatible = %s property\n",
> >                             compatible);
> >       }
> > +     return 0;
> >
> >       ret = fpga_load(devnum, (void *)fpga_image->load_addr,
> >                       fpga_image->size, BIT_FULL, flags);
>
>
> This return makes the spl_fit_upload_fpga function useless. I assume
> this change was by accident.

Yes that was a mistake. I'll send a patch.

Do you think it would be possible to create a sandbox driver for the
FPGA uclass? Then we could add some tests for this code.

Regards,
Simon

Re: [PATCH 01/14] trace: Use notrace for short

[Stefan Herbrechtsmeier]

Hi Simon,

Am 24.01.2023 um 11:55 schrieb Simon Glass:
> Hi Stefan,
>
> On Tue, 24 Jan 2023 at 01:12, Stefan Herbrechtsmeier
> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>> Hi Simon,
>>
>> Am 22.12.2022 um 00:08 schrieb Simon Glass:
>>> The attribute syntax is quite verbose. Use the macro provided for this
>>> purpose.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>>    arch/arm/cpu/armv7/s5p-common/timer.c   |  2 +-
>>>    arch/arm/mach-exynos/include/mach/cpu.h |  6 +++---
>>>    arch/x86/include/asm/global_data.h      |  2 +-
>>>    arch/x86/include/asm/msr.h              |  2 +-
>>>    arch/x86/include/asm/u-boot-x86.h       |  2 +-
>>>    common/spl/spl_fit.c                    |  1 +
>>>    doc/develop/trace.rst                   |  2 +-
>>>    lib/efi_loader/efi_freestanding.c       |  4 ++--
>>>    lib/trace.c                             | 26 +++++++++++--------------
>>>    9 files changed, 22 insertions(+), 25 deletions(-)
>>>
>> [snip]
>>
>>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>>> index 08da7fed88e..0e026bb3d8e 100644
>>> --- a/common/spl/spl_fit.c
>>> +++ b/common/spl/spl_fit.c
>>> @@ -599,6 +599,7 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node,
>>>                        debug("Ignoring compatible = %s property\n",
>>>                              compatible);
>>>        }
>>> +     return 0;
>>>
>>>        ret = fpga_load(devnum, (void *)fpga_image->load_addr,
>>>                        fpga_image->size, BIT_FULL, flags);
>>
>> This return makes the spl_fit_upload_fpga function useless. I assume
>> this change was by accident.
> Yes that was a mistake. I'll send a patch.

Thanks.

> Do you think it would be possible to create a sandbox driver for the
> FPGA uclass? Then we could add some tests for this code.

A sandbox driver exists but the uclass is only an empty dummy.

The fpga subsystem use its own structs to describe the hardware and the 
fpga_load function for example is a wrapper for xilinx_load, altera_load 
and lattice_load.

Regards
   Stefan

Re: [PATCH 01/14] trace: Use notrace for short

[Simon Glass]

Hi Stefan,

On Tue, 24 Jan 2023 at 05:14, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> Hi Simon,
>
> Am 24.01.2023 um 11:55 schrieb Simon Glass:
> > Hi Stefan,
> >
> > On Tue, 24 Jan 2023 at 01:12, Stefan Herbrechtsmeier
> > <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
> >> Hi Simon,
> >>
> >> Am 22.12.2022 um 00:08 schrieb Simon Glass:
> >>> The attribute syntax is quite verbose. Use the macro provided for this
> >>> purpose.
> >>>
> >>> Signed-off-by: Simon Glass <sjg@chromium.org>
> >>> ---
> >>>
> >>>    arch/arm/cpu/armv7/s5p-common/timer.c   |  2 +-
> >>>    arch/arm/mach-exynos/include/mach/cpu.h |  6 +++---
> >>>    arch/x86/include/asm/global_data.h      |  2 +-
> >>>    arch/x86/include/asm/msr.h              |  2 +-
> >>>    arch/x86/include/asm/u-boot-x86.h       |  2 +-
> >>>    common/spl/spl_fit.c                    |  1 +
> >>>    doc/develop/trace.rst                   |  2 +-
> >>>    lib/efi_loader/efi_freestanding.c       |  4 ++--
> >>>    lib/trace.c                             | 26 +++++++++++--------------
> >>>    9 files changed, 22 insertions(+), 25 deletions(-)
> >>>
> >> [snip]
> >>
> >>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> >>> index 08da7fed88e..0e026bb3d8e 100644
> >>> --- a/common/spl/spl_fit.c
> >>> +++ b/common/spl/spl_fit.c
> >>> @@ -599,6 +599,7 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node,
> >>>                        debug("Ignoring compatible = %s property\n",
> >>>                              compatible);
> >>>        }
> >>> +     return 0;
> >>>
> >>>        ret = fpga_load(devnum, (void *)fpga_image->load_addr,
> >>>                        fpga_image->size, BIT_FULL, flags);
> >>
> >> This return makes the spl_fit_upload_fpga function useless. I assume
> >> this change was by accident.
> > Yes that was a mistake. I'll send a patch.
>
> Thanks.
>
> > Do you think it would be possible to create a sandbox driver for the
> > FPGA uclass? Then we could add some tests for this code.
>
> A sandbox driver exists but the uclass is only an empty dummy.
>
> The fpga subsystem use its own structs to describe the hardware and the
> fpga_load function for example is a wrapper for xilinx_load, altera_load
> and lattice_load.

Yes, I think fpga_load should become a uclass operation. See how this
is done for other uclasses.

Regards,
Simon

Re: [PATCH 01/14] trace: Use notrace for short

[Stefan Herbrechtsmeier]

Hi Simon,

Am 24.01.2023 um 23:44 schrieb Simon Glass:
> Hi Stefan,
>
> On Tue, 24 Jan 2023 at 05:14, Stefan Herbrechtsmeier
> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>> Hi Simon,
>>
>> Am 24.01.2023 um 11:55 schrieb Simon Glass:
>>> Hi Stefan,
>>>
>>> On Tue, 24 Jan 2023 at 01:12, Stefan Herbrechtsmeier
>>> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>>>> Hi Simon,
>>>>
>>>> Am 22.12.2022 um 00:08 schrieb Simon Glass:
>>>>> The attribute syntax is quite verbose. Use the macro provided for this
>>>>> purpose.
>>>>>
>>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>>> ---
>>>>>
>>>>>     arch/arm/cpu/armv7/s5p-common/timer.c   |  2 +-
>>>>>     arch/arm/mach-exynos/include/mach/cpu.h |  6 +++---
>>>>>     arch/x86/include/asm/global_data.h      |  2 +-
>>>>>     arch/x86/include/asm/msr.h              |  2 +-
>>>>>     arch/x86/include/asm/u-boot-x86.h       |  2 +-
>>>>>     common/spl/spl_fit.c                    |  1 +
>>>>>     doc/develop/trace.rst                   |  2 +-
>>>>>     lib/efi_loader/efi_freestanding.c       |  4 ++--
>>>>>     lib/trace.c                             | 26 +++++++++++--------------
>>>>>     9 files changed, 22 insertions(+), 25 deletions(-)
>>>>>
>>>> [snip]
>>>>
>>>>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>>>>> index 08da7fed88e..0e026bb3d8e 100644
>>>>> --- a/common/spl/spl_fit.c
>>>>> +++ b/common/spl/spl_fit.c
>>>>> @@ -599,6 +599,7 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node,
>>>>>                         debug("Ignoring compatible = %s property\n",
>>>>>                               compatible);
>>>>>         }
>>>>> +     return 0;
>>>>>
>>>>>         ret = fpga_load(devnum, (void *)fpga_image->load_addr,
>>>>>                         fpga_image->size, BIT_FULL, flags);
>>>> This return makes the spl_fit_upload_fpga function useless. I assume
>>>> this change was by accident.
>>> Yes that was a mistake. I'll send a patch.
>> Thanks.
>>
>>> Do you think it would be possible to create a sandbox driver for the
>>> FPGA uclass? Then we could add some tests for this code.
>> A sandbox driver exists but the uclass is only an empty dummy.
>>
>> The fpga subsystem use its own structs to describe the hardware and the
>> fpga_load function for example is a wrapper for xilinx_load, altera_load
>> and lattice_load.
> Yes, I think fpga_load should become a uclass operation. See how this
> is done for other uclasses.

The problem is that the fpga subsystem doesn't use uclass at the moment. 
The fpga is init and added by the cpu code (see 
arch/arm/mach-zynq/cpu.c). The subsystem functions use the devnum 
(index) to select the added structure.

Is it possible to add a ulcass device from cpu code as a first step to 
convert the functions into uclass functions.

Regards
   Stefan

Re: [PATCH 01/14] trace: Use notrace for short

[Simon Glass]

Hi Stefan,

On Wed, 25 Jan 2023 at 00:53, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> Hi Simon,
>
> Am 24.01.2023 um 23:44 schrieb Simon Glass:
> > Hi Stefan,
> >
> > On Tue, 24 Jan 2023 at 05:14, Stefan Herbrechtsmeier
> > <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
> >> Hi Simon,
> >>
> >> Am 24.01.2023 um 11:55 schrieb Simon Glass:
> >>> Hi Stefan,
> >>>
> >>> On Tue, 24 Jan 2023 at 01:12, Stefan Herbrechtsmeier
> >>> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
> >>>> Hi Simon,
> >>>>
> >>>> Am 22.12.2022 um 00:08 schrieb Simon Glass:
> >>>>> The attribute syntax is quite verbose. Use the macro provided for this
> >>>>> purpose.
> >>>>>
> >>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
> >>>>> ---
> >>>>>
> >>>>>     arch/arm/cpu/armv7/s5p-common/timer.c   |  2 +-
> >>>>>     arch/arm/mach-exynos/include/mach/cpu.h |  6 +++---
> >>>>>     arch/x86/include/asm/global_data.h      |  2 +-
> >>>>>     arch/x86/include/asm/msr.h              |  2 +-
> >>>>>     arch/x86/include/asm/u-boot-x86.h       |  2 +-
> >>>>>     common/spl/spl_fit.c                    |  1 +
> >>>>>     doc/develop/trace.rst                   |  2 +-
> >>>>>     lib/efi_loader/efi_freestanding.c       |  4 ++--
> >>>>>     lib/trace.c                             | 26 +++++++++++--------------
> >>>>>     9 files changed, 22 insertions(+), 25 deletions(-)
> >>>>>
> >>>> [snip]
> >>>>
> >>>>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> >>>>> index 08da7fed88e..0e026bb3d8e 100644
> >>>>> --- a/common/spl/spl_fit.c
> >>>>> +++ b/common/spl/spl_fit.c
> >>>>> @@ -599,6 +599,7 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node,
> >>>>>                         debug("Ignoring compatible = %s property\n",
> >>>>>                               compatible);
> >>>>>         }
> >>>>> +     return 0;
> >>>>>
> >>>>>         ret = fpga_load(devnum, (void *)fpga_image->load_addr,
> >>>>>                         fpga_image->size, BIT_FULL, flags);
> >>>> This return makes the spl_fit_upload_fpga function useless. I assume
> >>>> this change was by accident.
> >>> Yes that was a mistake. I'll send a patch.
> >> Thanks.
> >>
> >>> Do you think it would be possible to create a sandbox driver for the
> >>> FPGA uclass? Then we could add some tests for this code.
> >> A sandbox driver exists but the uclass is only an empty dummy.
> >>
> >> The fpga subsystem use its own structs to describe the hardware and the
> >> fpga_load function for example is a wrapper for xilinx_load, altera_load
> >> and lattice_load.
> > Yes, I think fpga_load should become a uclass operation. See how this
> > is done for other uclasses.
>
> The problem is that the fpga subsystem doesn't use uclass at the moment.
> The fpga is init and added by the cpu code (see
> arch/arm/mach-zynq/cpu.c). The subsystem functions use the devnum
> (index) to select the added structure.
>
> Is it possible to add a ulcass device from cpu code as a first step to
> convert the functions into uclass functions.

OK that sounds good, thanks. It should be specified in the DT.

Regards,
Simon