Hi Jonas, Simon,
On 3/29/25 4:06 PM, Jonas Karlman wrote:
From: Simon Glass <s...@chromium.org>
Declare arch and compression at the top of the file to avoid needing
ifdefs in every usage.
Add a few comments to help with the remaining #ifdefs.
Signed-off-by: Simon Glass <s...@chromium.org>
Signed-off-by: Jonas Karlman <jo...@kwiboo.se>
---
Changes in v4:
- Split from "VBE serial part H: Implement VBE on Rockchip RK3399"
---
arch/arm/dts/rockchip-u-boot.dtsi | 44 +++++++++++++++----------------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/arch/arm/dts/rockchip-u-boot.dtsi
b/arch/arm/dts/rockchip-u-boot.dtsi
index e9ed1d4b5738..2b01dc660562 100644
--- a/arch/arm/dts/rockchip-u-boot.dtsi
+++ b/arch/arm/dts/rockchip-u-boot.dtsi
@@ -5,6 +5,20 @@
#include <config.h>
+#ifdef CONFIG_ARM64
+#define ARCH "arm64"
+#else
+#define ARCH "arm"
+#endif
+
I would refrain from using ARCH here as it's something we already use to
specify the architecture to build (e.g. make ARCH=arm64 CROSS_COMPILE=...).
Maybe FIT_ARCH?
+#if defined(CONFIG_SPL_GZIP)
+#define COMP "gzip"
+#elif defined(CONFIG_SPL_LZMA)
+#define COMP "lzma"
+#else
+#define COMP "none"
+#endif
+
This is specific to the U-Boot proper image node only, maybe we should
have this constant named more appropriately, e.g. FIT_PROPER_COMP or
something like that?
Reading the symbols also, I believe their help text is misleading or
confusing (at least the _GZIP ones, _ZSTD seems okay).
Also, I'm wondering if it actually makes sense to force a specific
U-Boot proper compression algorithm based on a symbol of what
decompression algorithms are supported in U-Boot SPL. Like, why should
gzip be picked if I also have lzma enabled? In any case, nothing related
to this patch.
/ {
binman: binman {
multiple-images;
@@ -51,26 +65,12 @@
description = "U-Boot";
type = "standalone";
os = "u-boot";
-#ifdef CONFIG_ARM64
- arch = "arm64";
-#else
- arch = "arm";
-#endif
-#if defined(CONFIG_SPL_GZIP)
- compression = "gzip";
-#elif defined(CONFIG_SPL_LZMA)
- compression = "lzma";
-#else
- compression = "none";
-#endif
+ arch = ARCH;
+ compression = COMP;
load = <CONFIG_TEXT_BASE>;
entry = <CONFIG_TEXT_BASE>;
u-boot-nodtb {
-#if defined(CONFIG_SPL_GZIP)
- compress = "gzip";
-#elif defined(CONFIG_SPL_LZMA)
- compress = "lzma";
-#endif
+ compress = COMP;
};
#ifdef CONFIG_SPL_FIT_SIGNATURE
hash {
@@ -84,7 +84,7 @@
fit,operation = "split-elf";
description = "ARM Trusted Firmware";
type = "firmware";
- arch = "arm64";
+ arch = ARCH;
os = "arm-trusted-firmware";
compression = "none";
fit,load;
@@ -103,7 +103,7 @@
fit,operation = "split-elf";
description = "TEE";
type = "tee";
- arch = "arm64";
+ arch = ARCH;
os = "tee";
compression = "none";
fit,load;
@@ -119,11 +119,11 @@
};
#endif
};
-#else
+#else /* !CONFIG_ARM64 */
op-tee {
description = "OP-TEE";
type = "tee";
- arch = "arm";
+ arch = ARCH;
os = "tee";
compression = "none";
load = <(CFG_SYS_SDRAM_BASE +
0x8400000)>;
Wondering if we couldn't put some of the Aarch32 and Aarch64 OP-TEE OS
node(s) in common?
The change is fine, finding a more appropriate name for COMP and ARCH
and it's all good.
Cheers,
Quentin
