Hi Ferass,
On 10/15/25 6:25 PM, Ferass El Hafidi wrote:
Add binman configuration to meson-gx-u-boot.dtsi to automate building
bootable images using amlimage.
Signed-off-by: Ferass El Hafidi <[email protected]>
Reviewed-by: Neil Armstrong <[email protected]>
Quick drive-by comments.
---
arch/arm/dts/meson-gx-u-boot.dtsi | 142 ++++++++++++++++++++++++++++++++++++++
1 file changed, 142 insertions(+)
diff --git a/arch/arm/dts/meson-gx-u-boot.dtsi
b/arch/arm/dts/meson-gx-u-boot.dtsi
index 9e0620f395e..d05f869c06e 100644
--- a/arch/arm/dts/meson-gx-u-boot.dtsi
+++ b/arch/arm/dts/meson-gx-u-boot.dtsi
@@ -2,6 +2,7 @@
/*
* Copyright (c) 2019 BayLibre, SAS.
* Author: Maxime Jourdan <[email protected]>
+ * Copyright (c) 2023 Ferass El Hafidi <[email protected]>
*/
/ {
@@ -15,6 +16,16 @@
soc {
bootph-all;
};
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
I don't believe this is required for binman so please split into its own
commit with justification.
+#if defined(CONFIG_BINMAN)
+ binman: binman {
+ multiple-images;
+ };
+#endif
};
&vpu {
@@ -30,3 +41,134 @@
<0x0 0xc883c000 0x0 0x1000>;
reg-names = "hdmitx", "hhi";
};
+
+#if defined(CONFIG_MESON_GX) && defined(CONFIG_BINMAN)
Isn't MESON_GX implied since we are in meson-gx-u-boot.dtsi?
+/* binman configuration on GXBB and GXL */
+
+#if defined(CONFIG_MESON_GXBB)
+#define BL31_ADDR 0x10100000
+#else /* GXL */
+#define BL31_ADDR 0x05100000
+#endif
+
First question is... can't this be derived from the binary already?
On Rockchip we do
fit,load;
fit,entry;
fit,data;
for OP-TEE OS and TF-A though we do split the binaries into multiple
nodes (see @atf-SEQ and @tee-SEQ). I'm not sure if those properties are
applicable to only tee and atf nodes without a node for each segment, if
it isn't...
I would highly recommend against doing this.
I would instead have a
arch/arm/dts/meson-gxbb-u-boot.dtsi with
&fit {
atf {
entry = <0x10100000>;
load = <0x10100000>;
};
};
and have the default entry and load be 0x05100000 if that really is a
sensible default (otherwise put none in meson-gx-u-boot.dtsi and declare
it in each meson-gxXXX-u-boot.dtsi). Then your boards would include
meson-gxbb-u-boot.dtsi instead of meson-gx-u-boot.dtsi (and the former
DTSI would include the latter DTSI).
The benefit is that it makes it much clearer what would be required to
support a new family/SoC of the Meson GX family. Or what exactly could
differ between families. But also keeps the main -u-boot.dtsi readable
(which is not that easy to do, see our horrendous rockchip-u-boot.dtsi :( )
+/*
+ * On GXBB the base address of the SCP firmware doesn't matter as SPL will
+ * send the firmware to the SCP anyway, and can get the base address from the
+ * FIT. On GXL it matters, as BL31 is supposed to send the firmware, so set the
+ * base address to what GXL BL2 would load the binary to.
+ */
+#define SCP_ADDR 0x13c0000
+
+&binman {
+ u-boot-amlogic {
+ filename = "u-boot-meson-with-spl.bin";
+ pad-byte = <0xff>;
+
+ mkimage {
+ filename = "spl/u-boot-spl-signed.bin";
+#if defined(CONFIG_MESON_GXBB)
+ args = "-n", "gxbb", "-T", "amlimage";
+#elif defined(CONFIG_MESON_GXL)
+ args = "-n", "gxl", "-T", "amlimage";
+#endif
Same here as suggested for the load address of TF-A.
On Rockchip we use CONFIG_SYS_SOC but I don't think you can on Amlogic.
You could set SYS_SOC depending on MESON_GXBB/MESON_GXL/... in
arch/arm/mach-meson/Kconfig and be done with it, but there are some
side-effects (read SYS_SOC doc).
Another way is to add
&binman {
mkimage {
args = "-n", "gxbb", "-T", "amlimage";
};
};
to arch/arm/dts/meson-gxbb-u-boot.dtsi.
+
+ blob {
+ size = <0xb000>; /* The BootROM loads 48K max
(header is 4K) */
+ filename = "spl/u-boot-spl.bin";
+ };
I'm not sure this actually works? It would rely on a binary located at
spl/u-boot-spl.bin, which binman doesn't seem to be generating? Either
we;re missing something to generate it with binman, or something
external to binman generates it?
Typically it is generated with:
u-boot-spl {
};
?
If you're trying to enforce a max size, then we have Kconfig symbols for
those, probably SPL_MAX_SIZE and then I believe binman enforces those
one way or another. If it doesn't, then many boards and architecture
have a problem and we should fix that so you can rely on it.
+ };
+
+ fit: fit {
+ description = "ATF and U-Boot images";
+ #address-cells = <1>;
+ fit,fdt-list = "of-list";
+ fit,external-offset = <CONFIG_FIT_EXTERNAL_OFFSET>;
+ fit,align = <512>;
+ offset = <CONFIG_SPL_PAD_TO>;
+
+ images {
+ u-boot {
+ description = "U-Boot";
+ type = "standalone";
+ os = "u-boot";
+ arch = "arm64";
+ compression = "none";
+ load = <CONFIG_TEXT_BASE>;
+ entry = <CONFIG_TEXT_BASE>;
+
+ u-boot-nodtb {
+ };
Why no hash for U-Boot?
+ };
+
+ atf {
+ description = "ARM Trusted Firmware";
+ type = "firmware";
+ os = "arm-trusted-firmware";
+ arch = "arm64";
+ compression = "none";
+ load = <BL31_ADDR>;
+ entry = <BL31_ADDR>;
+
+ atf-bl31 {
+ filename = "bl31.bin";
I believe this is already the case if you set BL31 environment variable
to bl31.bin. I would simply remove it and let the user decide what's
best for them. For binman this comes from the -a atf-bl31-path=${BL31}
argument.
The benefit is that you can now point to a BL31 outside of U-Boot's
tree, which is super convenient for debugging TF-A, or with build
systems like Yocto or Buildroot.
+ };
+ hash {
+ algo = "sha256";
+ };
This means your boards all need to have CONFIG_SPL_SHA256 if
CONFIG_SPL_FIT_SIGNATURE is provided otherwise the hash cannot be
verified. Please check if that is the case (enforced at the Kconfig
symbol-level would be best).
+ };
+
+ scp {
+ description = "SCP firmware";
+ type = "scp";
+ arch = "arm"; /* The Cortex-M core is
used as SCP */
+ compression = "none";
+ load = <SCP_ADDR>;
+
+ scp {
+ filename = "scp.bin";
+ };
I would recommend to attempt mimicking what's been done for atf-bl31
(then you need to add a new argument to the lists of arguments for
binman in the root Makefile). See above for benefits.
+ hash {
+ /*
+ * The hash is used by the SCP
and passed to it
+ * by U-Boot SPL.
+ */
+ algo = "sha256";
+ };
+ };
+
+ @fdt-SEQ {
+ description = "NAME";
+ type = "flat_dt";
+ compression = "none";
why no hash for FDTs?
+ };
+
+ };
+ configurations {
+ default = "@config-DEFAULT-SEQ";
+ @config-SEQ {
+ description = "NAME.dtb";
+ fdt = "fdt-SEQ";
+ firmware = "atf";
+ loadables = "scp", "u-boot";
+ };
+ };
+ };
+ };
+};
+
Is the below really related to adding binman configuration???
+&vpu {
+ /delete-property/ bootph-all;
+};
+
+&apb {
+ bootph-all;
+};
+
+&sd_emmc_b {
+ bootph-all;
+};
+
+&sd_emmc_c {
+ bootph-all;
+};
+#endif
Cheers,
Quentin
