* [PATCH v2 0/7] arm64: zynqmp: Convert platforms to use binman
@ 2024-11-01 9:17 Michal Simek
2024-11-01 9:17 ` [PATCH v2 1/7] binman: Add option for pointing to separate description Michal Simek
` (8 more replies)
0 siblings, 9 replies; 28+ messages in thread
From: Michal Simek @ 2024-11-01 9:17 UTC (permalink / raw)
To: u-boot, git, Simon Glass, neal.frager
Cc: AKASHI Takahiro, Adam Ford, Bryan Brattlof, Caleb Connolly,
Charlie Johnston, Christian Marangi, Csókás Bence,
Fabio Estevam, Heinrich Schuchardt, Ilias Apalodimas,
Jerome Forissier, Jonas Karlman, Kever Yang, Marek Vasut,
Marek Vasut, Neil Armstrong, Oliver Gaskell, Patrick Rudolph,
Peter Robinson, Prasad Kummari, Quentin Schulz, Rasmus Villemoes,
Rayagonda Kokatanur, Sean Anderson, Sughosh Ganu, Sumit Garg,
Tejas Bhumkar, Tom Rini, Tony Dinh, Venkatesh Yadav Abbarapu
Hi,
I have put togethere couple of patches to convert platforms to use binman.
The first patch has been sent separately. The third (SOM description) has
been also sent out for ilustration as RFC.
The last one is just cherry-pick the patch which has been reverted because
our platform wasn't converted to binman yet.
v1 of external description
https://lore.kernel.org/r/fbed0251437b61a2f7a85596d7403b5b9c8237c1.1728306322.git.michal.simek@amd.com
RFC for SOM:
https://lore.kernel.org/r/de1b8dbabd5ab7f20d7aac217ec4f5074d39f1da.1728462767.git.michal.simek@amd.com
Thanks,
Michal
Changes in v2:
- Change subject, align commit message
- binman - rename variable
- Use relative path
- Use separate variable in makefile to also handle empty string and default
to u-boot.dtb
- new patch
- new patch
- Align fit,fdt-list-val to have shorter lines
- Add reference to defconfig
- Rename zynqmp-som-binman.dts to zynqmp-binman-som.dts
- Use conf instead of config
- Change image name from image.bin to qspi.bin
- Remove RFC
- Change default addresses for BL31/BL32
- new patch
- new patch
- Rebase on the top of my series which do conversion to binman
- Taken from git with all tags
Marek Vasut (1):
Makefile: Drop SPL_FIT_GENERATOR support
Michal Simek (6):
binman: Add option for pointing to separate description
common: binman: Calling initr_binman() when BINMAN_FDT
arm64: zynqmp: Describe empty binman node
arm64: zynqmp: Add binman description for SOM
arm64: zynqmp: Generate u-boot.itb and QSPI image via binman
arm64: zynqmp: Remove mkimage fit script
Makefile | 29 +--
arch/arm/Kconfig | 1 +
arch/arm/dts/Makefile | 3 +
arch/arm/dts/zynqmp-binman-mini.dts | 10 +
arch/arm/dts/zynqmp-binman-som.dts | 225 ++++++++++++++++
arch/arm/dts/zynqmp-binman.dts | 111 ++++++++
arch/arm/dts/zynqmp-u-boot.dtsi | 11 +
arch/arm/mach-zynqmp/Kconfig | 14 +
arch/arm/mach-zynqmp/mkimage_fit_atf.sh | 240 ------------------
boot/Kconfig | 15 --
common/board_r.c | 7 +-
configs/xilinx_zynqmp_kria_defconfig | 3 +
configs/xilinx_zynqmp_mini_defconfig | 2 +
configs/xilinx_zynqmp_mini_emmc0_defconfig | 3 +
configs/xilinx_zynqmp_mini_emmc1_defconfig | 3 +
configs/xilinx_zynqmp_mini_nand_defconfig | 2 +
.../xilinx_zynqmp_mini_nand_single_defconfig | 2 +
configs/xilinx_zynqmp_mini_qspi_defconfig | 3 +
configs/xilinx_zynqmp_virt_defconfig | 3 +
doc/usage/fit/howto.rst | 4 -
lib/Kconfig | 9 +
21 files changed, 418 insertions(+), 282 deletions(-)
create mode 100644 arch/arm/dts/zynqmp-binman-mini.dts
create mode 100644 arch/arm/dts/zynqmp-binman-som.dts
create mode 100644 arch/arm/dts/zynqmp-binman.dts
create mode 100644 arch/arm/dts/zynqmp-u-boot.dtsi
delete mode 100755 arch/arm/mach-zynqmp/mkimage_fit_atf.sh
--
2.43.0
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 1/7] binman: Add option for pointing to separate description
2024-11-01 9:17 [PATCH v2 0/7] arm64: zynqmp: Convert platforms to use binman Michal Simek
@ 2024-11-01 9:17 ` Michal Simek
2024-12-06 19:17 ` Simon Glass
2024-11-01 9:17 ` [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT Michal Simek
` (7 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Michal Simek @ 2024-11-01 9:17 UTC (permalink / raw)
To: u-boot, git, Simon Glass, neal.frager
Cc: AKASHI Takahiro, Bryan Brattlof, Heinrich Schuchardt,
Ilias Apalodimas, Marek Vasut, Patrick Rudolph, Peter Robinson,
Sughosh Ganu, Sumit Garg, Tom Rini
Adding binman node with target images description can be unwanted feature
but as of today there is no way to disable it.
Also on size constrained systems it is not useful to add binman description
to DTB.
Introduce BINMAN_DTB Kconfig symbol which allows separate DTB for target
from DTB for binman itself.
Signed-off-by: Michal Simek <michal.simek@amd.com>
---
Changes in v2:
- Change subject, align commit message
- binman - rename variable
- Use relative path
- Use separate variable in makefile to also handle empty string and default
to u-boot.dtb
Makefile | 11 ++++++++++-
lib/Kconfig | 9 +++++++++
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 7275a02f24ca..03337450f636 100644
--- a/Makefile
+++ b/Makefile
@@ -1392,12 +1392,21 @@ endif
default_dt := $(if $(DEVICE_TREE),$(DEVICE_TREE),$(CONFIG_DEFAULT_DEVICE_TREE))
endif
+binman_dtb := $(shell echo $(CONFIG_BINMAN_DTB))
+ifeq ($(strip $(binman_dtb)),)
+ifeq ($(CONFIG_OF_EMBED),y)
+binman_dtb = ./dts/dt.dtb
+else
+binman_dtb = ./u-boot.dtb
+endif
+endif
+
quiet_cmd_binman = BINMAN $@
cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
$(foreach f,$(BINMAN_TOOLPATHS),--toolpath $(f)) \
--toolpath $(objtree)/tools \
$(if $(BINMAN_VERBOSE),-v$(BINMAN_VERBOSE)) \
- build -u -d u-boot.dtb -O . -m \
+ build -u -d $(binman_dtb) -O . -m \
--allow-missing $(if $(BINMAN_ALLOW_MISSING),--ignore-missing) \
-I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \
$(foreach f,$(of_list_dirs),-I $(f)) -a of-list=$(of_list) \
diff --git a/lib/Kconfig b/lib/Kconfig
index 56ffdfa18396..0b089814d149 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -45,6 +45,15 @@ config BINMAN_FDT
locate entries in the firmware image. See binman.h for the available
functionality.
+config BINMAN_DTB
+ string "binman DTB description"
+ depends on BINMAN
+ help
+ This enables option to point to different DTB file with binman node which
+ is outside of DTB used by the firmware. Use this option if information
+ about generated images shouldn't be the part of target binary. Or on system
+ with limited storage.
+
config CC_OPTIMIZE_LIBS_FOR_SPEED
bool "Optimize libraries for speed"
help
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT
2024-11-01 9:17 [PATCH v2 0/7] arm64: zynqmp: Convert platforms to use binman Michal Simek
2024-11-01 9:17 ` [PATCH v2 1/7] binman: Add option for pointing to separate description Michal Simek
@ 2024-11-01 9:17 ` Michal Simek
2024-12-06 19:20 ` Simon Glass
2024-11-01 9:17 ` [PATCH v2 3/7] arm64: zynqmp: Describe empty binman node Michal Simek
` (6 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Michal Simek @ 2024-11-01 9:17 UTC (permalink / raw)
To: u-boot, git, Simon Glass, neal.frager
Cc: Christian Marangi, Ilias Apalodimas, Jerome Forissier,
Rasmus Villemoes, Sughosh Ganu, Tom Rini
Calling empty function when BINMAN_FDT is adding +64B for nothing which is
not helping on size sensitive configurations as Xilinx mini configurations.
Signed-off-by: Michal Simek <michal.simek@amd.com>
---
Changes in v2:
- new patch
From my perspective there is no reason to call empty function. It is just
increase footprint for nothing and we are not far from that limit now.
---
common/board_r.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c
index 62228a723e12..ff9bce88dc93 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -287,13 +287,10 @@ static int initr_announce(void)
return 0;
}
-static int initr_binman(void)
+static int __maybe_unused initr_binman(void)
{
int ret;
- if (!CONFIG_IS_ENABLED(BINMAN_FDT))
- return 0;
-
ret = binman_init();
if (ret)
printf("binman_init failed:%d\n", ret);
@@ -635,7 +632,9 @@ static init_fnc_t init_sequence_r[] = {
#ifdef CONFIG_EFI_LOADER
efi_memory_init,
#endif
+#ifdef CONFIG_BINMAN_FDT
initr_binman,
+#endif
#ifdef CONFIG_FSP_VERSION2
arch_fsp_init_r,
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 3/7] arm64: zynqmp: Describe empty binman node
2024-11-01 9:17 [PATCH v2 0/7] arm64: zynqmp: Convert platforms to use binman Michal Simek
2024-11-01 9:17 ` [PATCH v2 1/7] binman: Add option for pointing to separate description Michal Simek
2024-11-01 9:17 ` [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT Michal Simek
@ 2024-11-01 9:17 ` Michal Simek
2024-12-06 19:19 ` Simon Glass
2024-11-01 9:17 ` [PATCH v2 4/7] arm64: zynqmp: Add binman description for SOM Michal Simek
` (5 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Michal Simek @ 2024-11-01 9:17 UTC (permalink / raw)
To: u-boot, git, Simon Glass, neal.frager
Cc: Adam Ford, Fabio Estevam, Heinrich Schuchardt, Ilias Apalodimas,
Jerome Forissier, Jonas Karlman, Kever Yang, Marek Vasut,
Neil Armstrong, Oliver Gaskell, Prasad Kummari, Sean Anderson,
Sumit Garg, Tom Rini, Tony Dinh, Venkatesh Yadav Abbarapu
For enabling binman by default there is a need to have at least empty node
present that's why create -u-boot.dtsi with empty node to cover all ZynqMP
platforms.
Signed-off-by: Michal Simek <michal.simek@amd.com>
---
Changes in v2:
- new patch
arch/arm/dts/Makefile | 1 +
arch/arm/dts/zynqmp-binman-mini.dts | 10 ++++++++++
arch/arm/dts/zynqmp-u-boot.dtsi | 11 +++++++++++
configs/xilinx_zynqmp_mini_defconfig | 2 ++
configs/xilinx_zynqmp_mini_emmc0_defconfig | 3 +++
configs/xilinx_zynqmp_mini_emmc1_defconfig | 3 +++
configs/xilinx_zynqmp_mini_nand_defconfig | 2 ++
configs/xilinx_zynqmp_mini_nand_single_defconfig | 2 ++
configs/xilinx_zynqmp_mini_qspi_defconfig | 3 +++
9 files changed, 37 insertions(+)
create mode 100644 arch/arm/dts/zynqmp-binman-mini.dts
create mode 100644 arch/arm/dts/zynqmp-u-boot.dtsi
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index aeccfa93fc53..253d883d6156 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -274,6 +274,7 @@ dtb-$(CONFIG_ARCH_ZYNQMP) += \
zynqmp-mini-qspi-x1-stacked.dtb \
zynqmp-mini-qspi-x2-single.dtb \
zynqmp-mini-qspi-x2-stacked.dtb \
+ zynqmp-binman-mini.dtb \
zynqmp-sc-revB.dtb \
zynqmp-sc-revC.dtb \
zynqmp-sm-k24-revA.dtb \
diff --git a/arch/arm/dts/zynqmp-binman-mini.dts b/arch/arm/dts/zynqmp-binman-mini.dts
new file mode 100644
index 000000000000..8f3d18ef394b
--- /dev/null
+++ b/arch/arm/dts/zynqmp-binman-mini.dts
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * (C) Copyright 2024, Advanced Micro Devices, Inc.
+ *
+ * Michal Simek <michal.simek@amd.com>
+ */
+
+/dts-v1/;
+
+#include "zynqmp-u-boot.dtsi"
diff --git a/arch/arm/dts/zynqmp-u-boot.dtsi b/arch/arm/dts/zynqmp-u-boot.dtsi
new file mode 100644
index 000000000000..9a7527ed5a10
--- /dev/null
+++ b/arch/arm/dts/zynqmp-u-boot.dtsi
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * (C) Copyright 2024, Advanced Micro Devices, Inc.
+ *
+ * Michal Simek <michal.simek@amd.com>
+ */
+
+/ {
+ binman: binman {
+ };
+};
diff --git a/configs/xilinx_zynqmp_mini_defconfig b/configs/xilinx_zynqmp_mini_defconfig
index 7aab69c9e46b..f0af27bbd4e6 100644
--- a/configs/xilinx_zynqmp_mini_defconfig
+++ b/configs/xilinx_zynqmp_mini_defconfig
@@ -59,6 +59,8 @@ CONFIG_NO_NET=y
# CONFIG_DM_MAILBOX is not set
# CONFIG_MMC is not set
CONFIG_ARM_DCC=y
+# CONFIG_BINMAN_FDT is not set
+CONFIG_BINMAN_DTB="./arch/arm/dts/zynqmp-binman-mini.dtb"
CONFIG_PANIC_HANG=y
# CONFIG_GZIP is not set
# CONFIG_LMB is not set
diff --git a/configs/xilinx_zynqmp_mini_emmc0_defconfig b/configs/xilinx_zynqmp_mini_emmc0_defconfig
index c56b1e830d67..9b3ab06f7c00 100644
--- a/configs/xilinx_zynqmp_mini_emmc0_defconfig
+++ b/configs/xilinx_zynqmp_mini_emmc0_defconfig
@@ -28,6 +28,7 @@ CONFIG_BOARD_EARLY_INIT_R=y
# CONFIG_BOARD_LATE_INIT is not set
CONFIG_CLOCKS=y
CONFIG_SPL_MAX_SIZE=0x40000
+# CONFIG_SPL_BINMAN_SYMBOLS is not set
# CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
CONFIG_SPL_SYS_MALLOC=y
CONFIG_SPL_HAS_CUSTOM_MALLOC_START=y
@@ -73,6 +74,8 @@ CONFIG_SUPPORT_EMMC_BOOT=y
CONFIG_MMC_SDHCI=y
CONFIG_MMC_SDHCI_ZYNQ=y
CONFIG_ARM_DCC=y
+# CONFIG_BINMAN_FDT is not set
+CONFIG_BINMAN_DTB="./arch/arm/dts/zynqmp-binman-mini.dtb"
CONFIG_PANIC_HANG=y
# CONFIG_GZIP is not set
# CONFIG_LMB is not set
diff --git a/configs/xilinx_zynqmp_mini_emmc1_defconfig b/configs/xilinx_zynqmp_mini_emmc1_defconfig
index a8dbf0056da3..3b088c5002a7 100644
--- a/configs/xilinx_zynqmp_mini_emmc1_defconfig
+++ b/configs/xilinx_zynqmp_mini_emmc1_defconfig
@@ -28,6 +28,7 @@ CONFIG_BOARD_EARLY_INIT_R=y
# CONFIG_BOARD_LATE_INIT is not set
CONFIG_CLOCKS=y
CONFIG_SPL_MAX_SIZE=0x40000
+# CONFIG_SPL_BINMAN_SYMBOLS is not set
# CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
CONFIG_SPL_SYS_MALLOC=y
CONFIG_SPL_HAS_CUSTOM_MALLOC_START=y
@@ -73,6 +74,8 @@ CONFIG_SUPPORT_EMMC_BOOT=y
CONFIG_MMC_SDHCI=y
CONFIG_MMC_SDHCI_ZYNQ=y
CONFIG_ARM_DCC=y
+# CONFIG_BINMAN_FDT is not set
+CONFIG_BINMAN_DTB="./arch/arm/dts/zynqmp-binman-mini.dtb"
CONFIG_PANIC_HANG=y
# CONFIG_GZIP is not set
# CONFIG_LMB is not set
diff --git a/configs/xilinx_zynqmp_mini_nand_defconfig b/configs/xilinx_zynqmp_mini_nand_defconfig
index ba8f02c5b11d..83bd3c9a5345 100644
--- a/configs/xilinx_zynqmp_mini_nand_defconfig
+++ b/configs/xilinx_zynqmp_mini_nand_defconfig
@@ -59,6 +59,8 @@ CONFIG_NAND_ARASAN=y
CONFIG_SYS_NAND_ONFI_DETECTION=y
CONFIG_SYS_NAND_MAX_CHIPS=2
CONFIG_ARM_DCC=y
+# CONFIG_BINMAN_FDT is not set
+CONFIG_BINMAN_DTB="./arch/arm/dts/zynqmp-binman-mini.dtb"
CONFIG_PANIC_HANG=y
# CONFIG_GZIP is not set
# CONFIG_LMB is not set
diff --git a/configs/xilinx_zynqmp_mini_nand_single_defconfig b/configs/xilinx_zynqmp_mini_nand_single_defconfig
index a8a0055f2e5b..fea0c6e08415 100644
--- a/configs/xilinx_zynqmp_mini_nand_single_defconfig
+++ b/configs/xilinx_zynqmp_mini_nand_single_defconfig
@@ -58,6 +58,8 @@ CONFIG_MTD_RAW_NAND=y
CONFIG_NAND_ARASAN=y
CONFIG_SYS_NAND_ONFI_DETECTION=y
CONFIG_ARM_DCC=y
+# CONFIG_BINMAN_FDT is not set
+CONFIG_BINMAN_DTB="./arch/arm/dts/zynqmp-binman-mini.dtb"
CONFIG_PANIC_HANG=y
# CONFIG_GZIP is not set
# CONFIG_LMB is not set
diff --git a/configs/xilinx_zynqmp_mini_qspi_defconfig b/configs/xilinx_zynqmp_mini_qspi_defconfig
index c08b10c6944f..39e50ff8c4f8 100644
--- a/configs/xilinx_zynqmp_mini_qspi_defconfig
+++ b/configs/xilinx_zynqmp_mini_qspi_defconfig
@@ -31,6 +31,7 @@ CONFIG_LOGLEVEL=0
# CONFIG_BOARD_LATE_INIT is not set
CONFIG_CLOCKS=y
CONFIG_SPL_MAX_SIZE=0x40000
+# CONFIG_SPL_BINMAN_SYMBOLS is not set
# CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
CONFIG_SPL_SYS_MALLOC=y
CONFIG_SPL_HAS_CUSTOM_MALLOC_START=y
@@ -91,6 +92,8 @@ CONFIG_SPI_FLASH_WINBOND=y
CONFIG_ARM_DCC=y
CONFIG_SPI=y
CONFIG_ZYNQMP_GQSPI=y
+# CONFIG_BINMAN_FDT is not set
+CONFIG_BINMAN_DTB="./arch/arm/dts/zynqmp-binman-mini.dtb"
CONFIG_PANIC_HANG=y
# CONFIG_GZIP is not set
# CONFIG_LMB is not set
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 4/7] arm64: zynqmp: Add binman description for SOM
2024-11-01 9:17 [PATCH v2 0/7] arm64: zynqmp: Convert platforms to use binman Michal Simek
` (2 preceding siblings ...)
2024-11-01 9:17 ` [PATCH v2 3/7] arm64: zynqmp: Describe empty binman node Michal Simek
@ 2024-11-01 9:17 ` Michal Simek
2024-12-06 19:19 ` Simon Glass
2024-11-01 9:17 ` [PATCH v2 5/7] arm64: zynqmp: Generate u-boot.itb and QSPI image via binman Michal Simek
` (4 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Michal Simek @ 2024-11-01 9:17 UTC (permalink / raw)
To: u-boot, git, Simon Glass, neal.frager
Cc: Adam Ford, Caleb Connolly, Charlie Johnston,
Csókás Bence, Fabio Estevam, Jonas Karlman, Kever Yang,
Marek Vasut, Neil Armstrong, Oliver Gaskell, Patrick Rudolph,
Peter Robinson, Prasad Kummari, Rayagonda Kokatanur, Sumit Garg,
Tejas Bhumkar, Tom Rini, Tony Dinh, Venkatesh Yadav Abbarapu
There is necessary to do some steps to compose boot images. These steps
were in scripts in layers for a while. That's why introduce description via
binman to simplify wiring and remove all scripting around.
This should make sure that everybody is up2date with the latest versions.
The first step is to create fit image with DTBs with descriptions in
configuration node which is written as regular expression to match all SOM
versions.
Description is there for k24 and k26 in spite of low level psu_init
configuration is different. The reason is that it goes to u-boot.itb image
which is the same for k24 and k26.
u-boot.itb is another image which is generated. It is normally generated
via arch/arm/mach-zynqmp/mkimage_fit_atf.sh but this script is supposed to
be deprecated.
FIT image by purpose is using 64bit addresses to have default option to
move images to high DDR (above 4GB). TF-A and TEE are optional components
but in the most cases TF-A is present all the time and TEE(OP-TEE) is used
by some configurations too.
3rd generated image is boot.bin with updated user field which contains
version number. This image can be used with updated Image Selector
which supports A/B update mechanisms with rollback protection.
4th image is image.bin which binary file which contains boot.bin and
u-boot.itb together and can be programmed via origin Image Selector.
This image can be also used for creating one capsule which contains both
boot images (in SPL boot flow).
Signed-off-by: Michal Simek <michal.simek@amd.com>
---
Changes in v2:
- Align fit,fdt-list-val to have shorter lines
- Add reference to defconfig
- Rename zynqmp-som-binman.dts to zynqmp-binman-som.dts
- Use conf instead of config
- Change image name from image.bin to qspi.bin
- Remove RFC
- Change default addresses for BL31/BL32
arch/arm/Kconfig | 1 +
arch/arm/dts/Makefile | 1 +
arch/arm/dts/zynqmp-binman-som.dts | 225 +++++++++++++++++++++++++++
arch/arm/mach-zynqmp/Kconfig | 14 ++
configs/xilinx_zynqmp_kria_defconfig | 3 +
5 files changed, 244 insertions(+)
create mode 100644 arch/arm/dts/zynqmp-binman-som.dts
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 787f983ffd46..bbf04f301188 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1324,6 +1324,7 @@ config ARCH_ZYNQMP_R5
config ARCH_ZYNQMP
bool "Xilinx ZynqMP based platform"
select ARM64
+ select BINMAN
select CLK
select DM
select DEBUG_UART_BOARD_INIT if SPL && DEBUG_UART
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 253d883d6156..5d28a17b8b8c 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -370,6 +370,7 @@ dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-sm-k24-revA-sck-kv-g-revB.dtb
dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-smk-k24-revA-sck-kv-g-revB.dtb
dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-sm-k24-revA-sck-kr-g-revB.dtb
dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-smk-k24-revA-sck-kr-g-revB.dtb
+dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-binman-som.dtb
dtb-$(CONFIG_ARCH_VERSAL) += \
versal-mini.dtb \
diff --git a/arch/arm/dts/zynqmp-binman-som.dts b/arch/arm/dts/zynqmp-binman-som.dts
new file mode 100644
index 000000000000..3d9d8476c98e
--- /dev/null
+++ b/arch/arm/dts/zynqmp-binman-som.dts
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dts file for Xilinx ZynqMP SOMs (k24/k26)
+ *
+ * (C) Copyright 2024, Advanced Micro Devices, Inc.
+ *
+ * Michal Simek <michal.simek@amd.com>
+ */
+
+#include <config.h>
+
+/dts-v1/;
+/ {
+ binman: binman {
+ multiple-images;
+ fit-dtb.blob {
+ filename = "fit-dtb.blob";
+ pad-byte = <0>;
+ fit {
+ fit,align = <0x8>;
+ fit,external-offset = <0x0>;
+ description = "DTBs for SOMs+CCs";
+ fit,fdt-list-val = "zynqmp-smk-k26-revA", "zynqmp-smk-k26-revA-sck-kr-g-revA",
+ "zynqmp-smk-k26-revA-sck-kr-g-revB", "zynqmp-smk-k26-revA-sck-kv-g-revA",
+ "zynqmp-smk-k26-revA-sck-kv-g-revB", "zynqmp-sm-k26-revA-sck-kv-g-revA",
+ "zynqmp-sm-k26-revA-sck-kv-g-revB", "zynqmp-sm-k26-revA-sck-kr-g-revB",
+ "zynqmp-smk-k24-revA-sck-kd-g-revA", "zynqmp-smk-k24-revA-sck-kv-g-revB",
+ "zynqmp-smk-k24-revA-sck-kr-g-revB", "zynqmp-sm-k24-revA-sck-kd-g-revA",
+ "zynqmp-sm-k24-revA-sck-kv-g-revB", "zynqmp-sm-k24-revA-sck-kr-g-revB";
+
+ images {
+ @fdt-SEQ {
+ description = "NAME";
+ type = "flat_dt";
+ arch = "arm64";
+ compression = "none";
+ hash-1 {
+ algo = "md5";
+ };
+ };
+ };
+ configurations {
+ default = "conf-1";
+ conf-1 {
+ description = "SOM itself";
+ fdt = "fdt-1";
+ };
+ conf-2 {
+ description = "zynqmp-smk-k26-.*-sck-kr-g-revA";
+ fdt = "fdt-2";
+ };
+ conf-3 {
+ description = "zynqmp-smk-k26-.*-sck-kr-g-.*";
+ fdt = "fdt-3";
+ };
+ conf-4 {
+ description = "zynqmp-smk-k26-.*-sck-kv-g-rev[AZ]";
+ fdt = "fdt-4";
+ };
+ conf-5 {
+ description = "zynqmp-smk-k26-.*-sck-kv-g-.*";
+ fdt = "fdt-5";
+ };
+ conf-6 {
+ description = "zynqmp-sm-k26-.*-sck-kv-g-rev[AZ]";
+ fdt = "fdt-6";
+ };
+ conf-7 {
+ description = "zynqmp-sm-k26-.*-sck-kv-g-.*";
+ fdt = "fdt-7";
+ };
+ conf-8 {
+ description = "zynqmp-sm-k26-.*-sck-kr-g-.*";
+ fdt = "fdt-8";
+ };
+ conf-9 {
+ description = "zynqmp-smk-k24-.*-sck-kd-g-.*";
+ fdt = "fdt-9";
+ };
+ conf-10 {
+ description = "zynqmp-smk-k24-.*-sck-kv-g-.*";
+ fdt = "fdt-10";
+ };
+ conf-11 {
+ description = "zynqmp-smk-k24-.*-sck-kr-g-.*";
+ fdt = "fdt-11";
+ };
+ conf-12 {
+ description = "zynqmp-sm-k24-.*-sck-kd-g-.*";
+ fdt = "fdt-12";
+ };
+ conf-13 {
+ description = "zynqmp-sm-k24-.*-sck-kv-g-.*";
+ fdt = "fdt-13";
+ };
+ conf-14 {
+ description = "zynqmp-sm-k24-.*-sck-kr-g-.*";
+ fdt = "fdt-14";
+ };
+ };
+ };
+ };
+
+ /* u-boot.itb generation in a static way */
+ itb {
+ filename = "u-boot.itb";
+ pad-byte = <0>;
+
+ fit {
+ description = "Configuration for Xilinx ZynqMP SoC";
+ fit,align = <0x8>;
+ fit,external-offset = <0x0>;
+ images {
+ uboot {
+ description = "U-Boot (64-bit)";
+ type = "firmware";
+ os = "u-boot";
+ arch = "arm64";
+ compression = "none";
+ load = /bits/ 64 <CONFIG_TEXT_BASE>;
+ entry = /bits/ 64 <CONFIG_TEXT_BASE>;
+ hash {
+ algo = "md5";
+ };
+ u-boot-nodtb {
+ };
+ };
+ atf {
+ description = "Trusted Firmware-A";
+ type = "firmware";
+ os = "arm-trusted-firmware";
+ arch = "arm64";
+ compression = "none";
+ load = /bits/ 64 <CONFIG_BL31_LOAD_ADDR>;
+ entry = /bits/ 64 <CONFIG_BL31_LOAD_ADDR>;
+ hash {
+ algo = "md5";
+ };
+ atf-bl31 {
+ optional;
+ };
+ };
+ tee {
+ description = "OP-TEE";
+ type = "tee";
+ arch = "arm64";
+ compression = "none";
+ os = "tee";
+ load = /bits/ 64 <CONFIG_BL31_LOAD_ADDR>;
+ entry = /bits/ 64 <CONFIG_BL31_LOAD_ADDR>;
+ tee-os {
+ optional;
+ };
+ };
+ fdt {
+ description = "Multi DTB fit image";
+ type = "flat_dt";
+ arch = "arm64";
+ compression = "none";
+ load = <0x0 0x100000>;
+ hash {
+ algo = "md5";
+ };
+ fdt-blob {
+ filename = "fit-dtb.blob";
+ type = "blob-ext";
+ };
+ };
+ };
+ configurations {
+ default = "conf-1";
+ conf-1 {
+ description = "Multi DTB with TF-A/TEE";
+ firmware = "atf";
+ loadables = "tee", "uboot", "fdt";
+ };
+ };
+ };
+ };
+
+ /* boot.bin generated with version string inside */
+ bootimage {
+ filename = "boot.bin";
+ pad-byte = <0>;
+
+ blob-ext@1 {
+ offset = <0x0>;
+ filename = "spl/boot.bin";
+ };
+ /* Optional version string at offset 0x70 */
+ blob-ext@2 {
+ offset = <0x70>;
+ filename = "version.bin";
+ overlap;
+ optional;
+ };
+ /* Optional version string at offset 0x94 */
+ blob-ext@3 {
+ offset = <0x94>;
+ filename = "version.bin";
+ overlap;
+ optional;
+ };
+ };
+
+#ifdef CONFIG_SYS_SPI_U_BOOT_OFFS
+ /* Full QSPI image for recovery app */
+ image {
+ filename = "qspi.bin";
+ pad-byte = <0>;
+
+ blob-ext@1 {
+ offset = <0x0>;
+ filename = "boot.bin";
+ };
+ blob-ext@2 {
+ offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>;
+ filename = "u-boot.itb";
+ };
+ fdtmap {
+ };
+ };
+#endif
+ };
+};
diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
index aea13622b68a..92d61e84319d 100644
--- a/arch/arm/mach-zynqmp/Kconfig
+++ b/arch/arm/mach-zynqmp/Kconfig
@@ -132,6 +132,20 @@ config SPL_ZYNQMP_RESTORE_JTAG
even if no eFuses were burnt. This option restores the interface if
possible.
+config BL31_LOAD_ADDR
+ hex "Load address of BL31 image (mostly TF-A)"
+ default 0xfffea000
+ help
+ The load address for the BL31 image. This value is used to build the
+ FIT image header that places BL31 in memory where it will run.
+
+config BL32_LOAD_ADDR
+ hex "Load address of BL32 image (mostly secure OS)"
+ default 0
+ help
+ The load address for the BL32 image. This value is used to build the
+ FIT image header that places BL32 in memory where it will run.
+
config ZYNQ_SDHCI_MAX_FREQ
default 200000000
diff --git a/configs/xilinx_zynqmp_kria_defconfig b/configs/xilinx_zynqmp_kria_defconfig
index 5757a0c9be9b..6d66a9423797 100644
--- a/configs/xilinx_zynqmp_kria_defconfig
+++ b/configs/xilinx_zynqmp_kria_defconfig
@@ -47,6 +47,7 @@ CONFIG_SYS_PBSIZE=2073
CONFIG_BOARD_EARLY_INIT_R=y
CONFIG_CLOCKS=y
CONFIG_SPL_MAX_SIZE=0x40000
+# CONFIG_SPL_BINMAN_SYMBOLS is not set
# CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
CONFIG_SPL_FS_LOAD_KERNEL_NAME=""
CONFIG_SPL_FS_LOAD_ARGS_NAME=""
@@ -224,6 +225,8 @@ CONFIG_VIDEO_ZYNQMP_DPSUB=y
CONFIG_VIRTIO_MMIO=y
CONFIG_VIRTIO_NET=y
CONFIG_VIRTIO_BLK=y
+# CONFIG_BINMAN_FDT is not set
+CONFIG_BINMAN_DTB="./arch/arm/dts/zynqmp-binman-som.dtb"
CONFIG_PANIC_HANG=y
CONFIG_TPM=y
CONFIG_SPL_GZIP=y
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 5/7] arm64: zynqmp: Generate u-boot.itb and QSPI image via binman
2024-11-01 9:17 [PATCH v2 0/7] arm64: zynqmp: Convert platforms to use binman Michal Simek
` (3 preceding siblings ...)
2024-11-01 9:17 ` [PATCH v2 4/7] arm64: zynqmp: Add binman description for SOM Michal Simek
@ 2024-11-01 9:17 ` Michal Simek
2024-12-06 19:20 ` Simon Glass
2024-11-01 9:17 ` [PATCH v2 6/7] arm64: zynqmp: Remove mkimage fit script Michal Simek
` (3 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Michal Simek @ 2024-11-01 9:17 UTC (permalink / raw)
To: u-boot, git, Simon Glass, neal.frager
Cc: Adam Ford, Fabio Estevam, Jonas Karlman, Kever Yang, Marek Vasut,
Neil Armstrong, Oliver Gaskell, Prasad Kummari, Sumit Garg,
Tejas Bhumkar, Tom Rini, Tony Dinh, Venkatesh Yadav Abbarapu
u-boot.itb has been generated via mkimage_fit_atf.sh but it is on the way
out that's why convert it's description to binman.
Compare to script binman description is not able to configure BL31 and BL32
load/entry addresses which should be done separately.
Signed-off-by: Michal Simek <michal.simek@amd.com>
---
Changes in v2:
- new patch
arch/arm/dts/Makefile | 1 +
arch/arm/dts/zynqmp-binman.dts | 111 +++++++++++++++++++++++++++
configs/xilinx_zynqmp_virt_defconfig | 3 +
3 files changed, 115 insertions(+)
create mode 100644 arch/arm/dts/zynqmp-binman.dts
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 5d28a17b8b8c..25945efa5ae3 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -320,6 +320,7 @@ dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-p-a2197-00-revA-x-prc-02-revA.dtb
dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-p-a2197-00-revA-x-prc-03-revA.dtb
dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-p-a2197-00-revA-x-prc-04-revA.dtb
dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-p-a2197-00-revA-x-prc-05-revA.dtb
+dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-binman.dtb
zynqmp-sc-vek280-revA-dtbs := zynqmp-sc-revB.dtb zynqmp-sc-vek280-revA.dtbo
zynqmp-sc-vek280-revB-dtbs := zynqmp-sc-revC.dtb zynqmp-sc-vek280-revB.dtbo
diff --git a/arch/arm/dts/zynqmp-binman.dts b/arch/arm/dts/zynqmp-binman.dts
new file mode 100644
index 000000000000..df0fdf465107
--- /dev/null
+++ b/arch/arm/dts/zynqmp-binman.dts
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dts file for Xilinx ZynqMP platforms
+ *
+ * (C) Copyright 2024, Advanced Micro Devices, Inc.
+ *
+ * Michal Simek <michal.simek@amd.com>
+ */
+
+#include <config.h>
+
+/dts-v1/;
+/ {
+ binman: binman {
+ multiple-images;
+
+ /* u-boot.itb generation in a static way */
+ itb {
+ filename = "u-boot.itb";
+ pad-byte = <0>;
+
+ fit {
+ description = "Configuration for Xilinx ZynqMP SoC";
+ fit,align = <0x8>;
+ fit,external-offset = <0x0>;
+ fit,fdt-list = "of-list";
+ images {
+ uboot {
+ description = "U-Boot (64-bit)";
+ type = "firmware";
+ os = "u-boot";
+ arch = "arm64";
+ compression = "none";
+ load = /bits/ 64 <CONFIG_TEXT_BASE>;
+ entry = /bits/ 64 <CONFIG_TEXT_BASE>;
+ hash {
+ algo = "md5";
+ };
+ u-boot-nodtb {
+ };
+ };
+ atf {
+ description = "Trusted Firmware-A";
+ type = "firmware";
+ os = "arm-trusted-firmware";
+ arch = "arm64";
+ compression = "none";
+ load = /bits/ 64 <CONFIG_BL31_LOAD_ADDR>;
+ entry = /bits/ 64 <CONFIG_BL31_LOAD_ADDR>;
+ hash {
+ algo = "md5";
+ };
+ atf-bl31 {
+ optional;
+ };
+ };
+ tee {
+ description = "OP-TEE";
+ type = "tee";
+ arch = "arm64";
+ compression = "none";
+ os = "tee";
+ load = /bits/ 64 <CONFIG_BL31_LOAD_ADDR>;
+ entry = /bits/ 64 <CONFIG_BL31_LOAD_ADDR>;
+ tee-os {
+ optional;
+ };
+ };
+ @fdt-SEQ {
+ description = "NAME";
+ type = "flat_dt";
+ arch = "arm64";
+ compression = "none";
+ load = <0x0 0x100000>;
+ hash-1 {
+ algo = "md5";
+ };
+ };
+ };
+ configurations {
+ default = "@conf-DEFAULT-SEQ";
+ @conf-SEQ {
+ description = "NAME";
+ firmware = "atf";
+ loadables = "tee", "uboot";
+ fdt = "fdt-SEQ";
+ };
+ };
+ };
+ };
+
+#ifdef CONFIG_SYS_SPI_U_BOOT_OFFS
+ /* QSPI image for testing QSPI boot mode */
+ image {
+ filename = "qspi.bin";
+ pad-byte = <0>;
+
+ blob-ext@1 {
+ offset = <0x0>;
+ filename = "spl/boot.bin";
+ };
+ blob-ext@2 {
+ offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>;
+ filename = "u-boot.itb";
+ };
+ fdtmap {
+ };
+ };
+#endif
+ };
+};
diff --git a/configs/xilinx_zynqmp_virt_defconfig b/configs/xilinx_zynqmp_virt_defconfig
index 310efdf2338a..09f487acf0dc 100644
--- a/configs/xilinx_zynqmp_virt_defconfig
+++ b/configs/xilinx_zynqmp_virt_defconfig
@@ -41,6 +41,7 @@ CONFIG_SYS_PBSIZE=2073
CONFIG_BOARD_EARLY_INIT_R=y
CONFIG_CLOCKS=y
CONFIG_SPL_MAX_SIZE=0x40000
+# CONFIG_SPL_BINMAN_SYMBOLS is not set
# CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
CONFIG_SPL_SYS_MALLOC=y
CONFIG_SPL_HAS_CUSTOM_MALLOC_START=y
@@ -241,6 +242,8 @@ CONFIG_BMP_32BPP=y
CONFIG_VIRTIO_MMIO=y
CONFIG_VIRTIO_NET=y
CONFIG_VIRTIO_BLK=y
+# CONFIG_BINMAN_FDT is not set
+CONFIG_BINMAN_DTB="./arch/arm/dts/zynqmp-binman.dtb"
CONFIG_PANIC_HANG=y
CONFIG_TPM=y
CONFIG_SPL_GZIP=y
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 6/7] arm64: zynqmp: Remove mkimage fit script
2024-11-01 9:17 [PATCH v2 0/7] arm64: zynqmp: Convert platforms to use binman Michal Simek
` (4 preceding siblings ...)
2024-11-01 9:17 ` [PATCH v2 5/7] arm64: zynqmp: Generate u-boot.itb and QSPI image via binman Michal Simek
@ 2024-11-01 9:17 ` Michal Simek
2024-12-06 19:20 ` Simon Glass
2024-11-01 9:18 ` [PATCH v2 7/7] Makefile: Drop SPL_FIT_GENERATOR support Michal Simek
` (2 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Michal Simek @ 2024-11-01 9:17 UTC (permalink / raw)
To: u-boot, git, Simon Glass, neal.frager
Cc: Heinrich Schuchardt, Ilias Apalodimas, Marek Vasut,
Peter Robinson, Quentin Schulz, Tom Rini
Platform has been switched to binman that's why there is no need for this
script and also Kconfig symbols.
Signed-off-by: Michal Simek <michal.simek@amd.com>
---
Changes in v2:
- new patch
arch/arm/mach-zynqmp/mkimage_fit_atf.sh | 240 ------------------------
boot/Kconfig | 1 -
2 files changed, 241 deletions(-)
delete mode 100755 arch/arm/mach-zynqmp/mkimage_fit_atf.sh
diff --git a/arch/arm/mach-zynqmp/mkimage_fit_atf.sh b/arch/arm/mach-zynqmp/mkimage_fit_atf.sh
deleted file mode 100755
index cdecb1c1d351..000000000000
--- a/arch/arm/mach-zynqmp/mkimage_fit_atf.sh
+++ /dev/null
@@ -1,240 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0+
-#
-# script to generate FIT image source for Xilinx ZynqMP boards with
-# ARM Trusted Firmware and multiple device trees (given on the command line)
-#
-# usage: $0 <dt_name> [<dt_name> [<dt_name] ...]
-
-BL33="u-boot-nodtb.bin"
-[ -z "$BL31" ] && BL31="bl31.bin"
-BL31_ELF="${BL31%.*}.elf"
-[ -f ${BL31_ELF} ] && ATF_LOAD_ADDR=`${CROSS_COMPILE}readelf -l "${BL31_ELF}" | \
-awk '/Entry point/ { print $3 }'`
-
-[ -z "$ATF_LOAD_ADDR" ] && ATF_LOAD_ADDR="0xfffea000"
-ATF_LOAD_ADDR_LOW=`printf 0x%x $((ATF_LOAD_ADDR & 0xffffffff))`
-ATF_LOAD_ADDR_HIGH=`printf 0x%x $((ATF_LOAD_ADDR >> 32))`
-
-[ -z "$BL32" ] && BL32="tee.bin"
-BL32_ELF="${BL32%.*}.elf"
-[ -f ${BL32_ELF} ] && TEE_LOAD_ADDR=`${CROSS_COMPILE}readelf -l "${BL32_ELF}" | \
-awk '/Entry point/ { print $3 }'`
-
-[ -z "$TEE_LOAD_ADDR" ] && TEE_LOAD_ADDR="0x60000000"
-TEE_LOAD_ADDR_LOW=`printf 0x%x $((TEE_LOAD_ADDR & 0xffffffff))`
-TEE_LOAD_ADDR_HIGH=`printf 0x%x $((TEE_LOAD_ADDR >> 32))`
-
-if [ -z "$BL33_LOAD_ADDR" ];then
- BL33_LOAD_ADDR=`awk '/CONFIG_TEXT_BASE/ { print $3 }' include/generated/autoconf.h`
-fi
-BL33_LOAD_ADDR_LOW=`printf 0x%x $((BL33_LOAD_ADDR & 0xffffffff))`
-BL33_LOAD_ADDR_HIGH=`printf 0x%x $((BL33_LOAD_ADDR >> 32))`
-
-DTB_LOAD_ADDR=`awk '/CONFIG_XILINX_OF_BOARD_DTB_ADDR/ { print $3 }' include/generated/autoconf.h`
-if [ ! -z "$DTB_LOAD_ADDR" ]; then
- DTB_LOAD_ADDR_LOW=`printf 0x%x $((DTB_LOAD_ADDR & 0xffffffff))`
- DTB_LOAD_ADDR_HIGH=`printf 0x%x $((DTB_LOAD_ADDR >> 32))`
- DTB_LOAD="load = <$DTB_LOAD_ADDR_HIGH $DTB_LOAD_ADDR_LOW>;"
-else
- DTB_LOAD=""
-fi
-
-if [ -z "$*" ]; then
- DT=arch/arm/dts/${DEVICE_TREE}.dtb
-else
- DT=$*
-fi
-
-if [ ! -f $BL31 ]; then
- echo "WARNING: BL31 file $BL31 NOT found, U-Boot will run in EL3" >&2
- BL31=/dev/null
-fi
-
-cat << __HEADER_EOF
-// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
-
-/dts-v1/;
-
-/ {
- description = "Configuration for Xilinx ZynqMP SoC";
-
- images {
- uboot {
- description = "U-Boot (64-bit)";
- data = /incbin/("$BL33");
- type = "firmware";
- os = "u-boot";
- arch = "arm64";
- compression = "none";
- load = <$BL33_LOAD_ADDR_HIGH $BL33_LOAD_ADDR_LOW>;
- entry = <$BL33_LOAD_ADDR_HIGH $BL33_LOAD_ADDR_LOW>;
- hash {
- algo = "md5";
- };
- };
-__HEADER_EOF
-
-if [ -f $BL31 ]; then
-cat << __ATF
- atf {
- description = "Trusted Firmware-A";
- data = /incbin/("$BL31");
- type = "firmware";
- os = "arm-trusted-firmware";
- arch = "arm64";
- compression = "none";
- load = <$ATF_LOAD_ADDR_HIGH $ATF_LOAD_ADDR_LOW>;
- entry = <$ATF_LOAD_ADDR_HIGH $ATF_LOAD_ADDR_LOW>;
- hash {
- algo = "md5";
- };
- };
-__ATF
-fi
-
-if [ -f $BL32 ]; then
-cat << __TEE
- tee {
- description = "TEE firmware";
- data = /incbin/("$BL32");
- type = "firmware";
- os = "tee";
- arch = "arm64";
- compression = "none";
- load = <$TEE_LOAD_ADDR_HIGH $TEE_LOAD_ADDR_LOW>;
- entry = <$TEE_LOAD_ADDR_HIGH $TEE_LOAD_ADDR_LOW>;
- hash {
- algo = "md5";
- };
- };
-__TEE
-fi
-
-MULTI_DTB=`awk '/CONFIG_MULTI_DTB_FIT / { print $3 }' include/generated/autoconf.h`
-
-if [ 1"$MULTI_DTB" -eq 11 ]; then
- cat << __FDT_IMAGE_EOF
- fdt_1 {
- description = "Multi DTB fit image";
- data = /incbin/("fit-dtb.blob");
- type = "flat_dt";
- arch = "arm64";
- compression = "none";
- $DTB_LOAD
- hash {
- algo = "md5";
- };
- };
- };
- configurations {
- default = "config_1";
-__FDT_IMAGE_EOF
-
-if [ ! -f $BL31 ]; then
-cat << __CONF_SECTION1_EOF
- config_1 {
- description = "Multi DTB without TF-A";
- firmware = "uboot";
- loadables = "fdt_1";
- };
-__CONF_SECTION1_EOF
-else
-if [ -f $BL32 ]; then
-cat << __CONF_SECTION1_EOF
- config_1 {
- description = "Multi DTB with TF-A and TEE";
- firmware = "atf";
- loadables = "uboot", "tee", "fdt_1";
- };
-__CONF_SECTION1_EOF
-else
-cat << __CONF_SECTION1_EOF
- config_1 {
- description = "Multi DTB with TF-A";
- firmware = "atf";
- loadables = "uboot", "fdt_1";
- };
-__CONF_SECTION1_EOF
-fi
-fi
-
-cat << __ITS_EOF
- };
-};
-__ITS_EOF
-
-else
-
-DEFAULT=1
-cnt=1
-for dtname in $DT
-do
- cat << __FDT_IMAGE_EOF
- fdt_$cnt {
- description = "$(basename $dtname .dtb)";
- data = /incbin/("$dtname");
- type = "flat_dt";
- arch = "arm64";
- compression = "none";
- $DTB_LOAD
- hash {
- algo = "md5";
- };
- };
-__FDT_IMAGE_EOF
-
-[ "x$(basename $dtname .dtb)" = "x${DEVICE_TREE}" ] && DEFAULT=$cnt
-
-cnt=$((cnt+1))
-done
-
-cat << __CONF_HEADER_EOF
- };
- configurations {
- default = "config_$DEFAULT";
-
-__CONF_HEADER_EOF
-
-cnt=1
-for dtname in $DT
-do
-if [ ! -f $BL31 ]; then
-cat << __CONF_SECTION1_EOF
- config_$cnt {
- description = "$(basename $dtname .dtb)";
- firmware = "uboot";
- fdt = "fdt_$cnt";
- };
-__CONF_SECTION1_EOF
-else
-if [ -f $BL32 ]; then
-cat << __CONF_SECTION1_EOF
- config_$cnt {
- description = "$(basename $dtname .dtb)";
- firmware = "atf";
- loadables = "uboot", "tee";
- fdt = "fdt_$cnt";
- };
-__CONF_SECTION1_EOF
-else
-cat << __CONF_SECTION1_EOF
- config_$cnt {
- description = "$(basename $dtname .dtb)";
- firmware = "atf";
- loadables = "uboot";
- fdt = "fdt_$cnt";
- };
-__CONF_SECTION1_EOF
-fi
-fi
-
-cnt=$((cnt+1))
-done
-
-cat << __ITS_EOF
- };
-};
-__ITS_EOF
-
-fi
diff --git a/boot/Kconfig b/boot/Kconfig
index 7dd30a030e39..e1a79976da1c 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -291,7 +291,6 @@ config USE_SPL_FIT_GENERATOR
config SPL_FIT_GENERATOR
string ".its file generator script for U-Boot FIT image"
depends on USE_SPL_FIT_GENERATOR
- default "arch/arm/mach-zynqmp/mkimage_fit_atf.sh" if SPL_LOAD_FIT && ARCH_ZYNQMP
help
Specifies a (platform specific) script file to generate the FIT
source file used to build the U-Boot FIT image file. This gets
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 7/7] Makefile: Drop SPL_FIT_GENERATOR support
2024-11-01 9:17 [PATCH v2 0/7] arm64: zynqmp: Convert platforms to use binman Michal Simek
` (5 preceding siblings ...)
2024-11-01 9:17 ` [PATCH v2 6/7] arm64: zynqmp: Remove mkimage fit script Michal Simek
@ 2024-11-01 9:18 ` Michal Simek
2024-12-06 19:16 ` Simon Glass
2024-11-11 11:46 ` [PATCH v2 0/7] arm64: zynqmp: Convert platforms to use binman Michal Simek
2024-11-27 7:59 ` Michal Simek
8 siblings, 1 reply; 28+ messages in thread
From: Michal Simek @ 2024-11-01 9:18 UTC (permalink / raw)
To: u-boot, git, Simon Glass, neal.frager
Cc: Marek Vasut, Peter Robinson, Bryan Brattlof, Heinrich Schuchardt,
Ilias Apalodimas, Quentin Schulz, Sean Anderson, Sumit Garg,
Tom Rini
From: Marek Vasut <marex@denx.de>
The SPL_FIT_GENERATOR is long superseded by binman, drop SPL_FIT_GENERATOR
support as there are no more users.
Signed-off-by: Marek Vasut <marex@denx.de>
Reviewed-by: Peter Robinson <pbrobinson@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Michal Simek <michal.simek@amd.com>
---
Changes in v2:
- Rebase on the top of my series which do conversion to binman
- Taken from git with all tags
Makefile | 18 ------------------
boot/Kconfig | 14 --------------
doc/usage/fit/howto.rst | 4 ----
3 files changed, 36 deletions(-)
diff --git a/Makefile b/Makefile
index 03337450f636..6a86e3df2d53 100644
--- a/Makefile
+++ b/Makefile
@@ -1148,13 +1148,6 @@ ifeq ($(CONFIG_OF_EMBED)$(CONFIG_EFI_APP),y)
@echo >&2 "CONFIG_OF_SEPARATE for boards in mainline."
@echo >&2 "See doc/develop/devicetree/control.rst for more info."
@echo >&2 "===================================================="
-endif
-ifneq ($(CONFIG_SPL_FIT_GENERATOR),)
- @echo >&2 "===================== WARNING ======================"
- @echo >&2 "This board uses CONFIG_SPL_FIT_GENERATOR. Please migrate"
- @echo >&2 "to binman instead, to avoid the proliferation of"
- @echo >&2 "arch-specific scripts with no tests."
- @echo >&2 "===================================================="
endif
$(call deprecated,CONFIG_WDT,DM watchdog,v2019.10,\
$(CONFIG_WATCHDOG)$(CONFIG_HW_WATCHDOG))
@@ -1435,17 +1428,6 @@ OBJCOPYFLAGS_u-boot.ldr.srec := -I binary -O srec
u-boot.ldr.hex u-boot.ldr.srec: u-boot.ldr FORCE
$(call if_changed,objcopy)
-# Boards with more complex image requirements can provide an .its source file
-# or a generator script
-# NOTE: Please do not use this. We are migrating away from Makefile rules to use
-# binman instead.
-ifneq ($(CONFIG_USE_SPL_FIT_GENERATOR),)
-U_BOOT_ITS := u-boot.its
-$(U_BOOT_ITS): $(U_BOOT_ITS_DEPS) FORCE
- $(srctree)/$(CONFIG_SPL_FIT_GENERATOR) \
- $(patsubst %,$(dt_dir)/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) > $@
-endif
-
ifdef CONFIG_SPL_LOAD_FIT
MKIMAGEFLAGS_u-boot.img = -f auto -A $(ARCH) -T firmware -C none -O u-boot \
-a $(CONFIG_TEXT_BASE) -e $(CONFIG_SYS_UBOOT_START) \
diff --git a/boot/Kconfig b/boot/Kconfig
index e1a79976da1c..af54a0660e3b 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -283,20 +283,6 @@ config SPL_FIT_IMAGE_POST_PROCESS
injected into the FIT creation (i.e. the blobs would have been pre-
processed before being added to the FIT image).
-config USE_SPL_FIT_GENERATOR
- bool "Use a script to generate the .its script"
- depends on SPL_FIT
- default y if SPL_FIT && ARCH_ZYNQMP
-
-config SPL_FIT_GENERATOR
- string ".its file generator script for U-Boot FIT image"
- depends on USE_SPL_FIT_GENERATOR
- help
- Specifies a (platform specific) script file to generate the FIT
- source file used to build the U-Boot FIT image file. This gets
- passed a list of supported device tree file stub names to
- include in the generated image.
-
if VPL
config VPL_FIT
diff --git a/doc/usage/fit/howto.rst b/doc/usage/fit/howto.rst
index 280eff724f6e..675c9aa5bb0f 100644
--- a/doc/usage/fit/howto.rst
+++ b/doc/usage/fit/howto.rst
@@ -57,10 +57,6 @@ own subnode under the /images node, which should then be referenced from one or
multiple /configurations subnodes. The required images must be enumerated in
the "loadables" property as a list of strings.
-CONFIG_SPL_FIT_GENERATOR can point to a script which generates this image source
-file during the build process. It gets passed a list of device tree files (taken
-from the CONFIG_OF_LIST symbol).
-
The SPL also records to a DT all additional images (called loadables) which are
loaded. The information about loadables locations is passed via the DT node with
fit-images name.
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/7] arm64: zynqmp: Convert platforms to use binman
2024-11-01 9:17 [PATCH v2 0/7] arm64: zynqmp: Convert platforms to use binman Michal Simek
` (6 preceding siblings ...)
2024-11-01 9:18 ` [PATCH v2 7/7] Makefile: Drop SPL_FIT_GENERATOR support Michal Simek
@ 2024-11-11 11:46 ` Michal Simek
2024-11-27 7:59 ` Michal Simek
8 siblings, 0 replies; 28+ messages in thread
From: Michal Simek @ 2024-11-11 11:46 UTC (permalink / raw)
To: u-boot, git, Simon Glass, neal.frager
Cc: AKASHI Takahiro, Adam Ford, Bryan Brattlof, Caleb Connolly,
Charlie Johnston, Christian Marangi, Csókás Bence,
Fabio Estevam, Heinrich Schuchardt, Ilias Apalodimas,
Jerome Forissier, Jonas Karlman, Kever Yang, Marek Vasut,
Marek Vasut, Neil Armstrong, Oliver Gaskell, Patrick Rudolph,
Peter Robinson, Prasad Kummari, Quentin Schulz, Rasmus Villemoes,
Rayagonda Kokatanur, Sean Anderson, Sughosh Ganu, Sumit Garg,
Tejas Bhumkar, Tom Rini, Tony Dinh, Venkatesh Yadav Abbarapu
Hi Simon,
On 11/1/24 10:17, Michal Simek wrote:
> Hi,
>
> I have put togethere couple of patches to convert platforms to use binman.
> The first patch has been sent separately. The third (SOM description) has
> been also sent out for ilustration as RFC.
> The last one is just cherry-pick the patch which has been reverted because
> our platform wasn't converted to binman yet.
>
> v1 of external description
> https://lore.kernel.org/r/fbed0251437b61a2f7a85596d7403b5b9c8237c1.1728306322.git.michal.simek@amd.com
>
> RFC for SOM:
> https://lore.kernel.org/r/de1b8dbabd5ab7f20d7aac217ec4f5074d39f1da.1728462767.git.michal.simek@amd.com
>
> Thanks,
> Michal
>
> Changes in v2:
> - Change subject, align commit message
> - binman - rename variable
> - Use relative path
> - Use separate variable in makefile to also handle empty string and default
> to u-boot.dtb
> - new patch
> - new patch
> - Align fit,fdt-list-val to have shorter lines
> - Add reference to defconfig
> - Rename zynqmp-som-binman.dts to zynqmp-binman-som.dts
> - Use conf instead of config
> - Change image name from image.bin to qspi.bin
> - Remove RFC
> - Change default addresses for BL31/BL32
> - new patch
> - new patch
> - Rebase on the top of my series which do conversion to binman
> - Taken from git with all tags
>
> Marek Vasut (1):
> Makefile: Drop SPL_FIT_GENERATOR support
>
> Michal Simek (6):
> binman: Add option for pointing to separate description
> common: binman: Calling initr_binman() when BINMAN_FDT
> arm64: zynqmp: Describe empty binman node
> arm64: zynqmp: Add binman description for SOM
> arm64: zynqmp: Generate u-boot.itb and QSPI image via binman
> arm64: zynqmp: Remove mkimage fit script
>
> Makefile | 29 +--
> arch/arm/Kconfig | 1 +
> arch/arm/dts/Makefile | 3 +
> arch/arm/dts/zynqmp-binman-mini.dts | 10 +
> arch/arm/dts/zynqmp-binman-som.dts | 225 ++++++++++++++++
> arch/arm/dts/zynqmp-binman.dts | 111 ++++++++
> arch/arm/dts/zynqmp-u-boot.dtsi | 11 +
> arch/arm/mach-zynqmp/Kconfig | 14 +
> arch/arm/mach-zynqmp/mkimage_fit_atf.sh | 240 ------------------
> boot/Kconfig | 15 --
> common/board_r.c | 7 +-
> configs/xilinx_zynqmp_kria_defconfig | 3 +
> configs/xilinx_zynqmp_mini_defconfig | 2 +
> configs/xilinx_zynqmp_mini_emmc0_defconfig | 3 +
> configs/xilinx_zynqmp_mini_emmc1_defconfig | 3 +
> configs/xilinx_zynqmp_mini_nand_defconfig | 2 +
> .../xilinx_zynqmp_mini_nand_single_defconfig | 2 +
> configs/xilinx_zynqmp_mini_qspi_defconfig | 3 +
> configs/xilinx_zynqmp_virt_defconfig | 3 +
> doc/usage/fit/howto.rst | 4 -
> lib/Kconfig | 9 +
> 21 files changed, 418 insertions(+), 282 deletions(-)
> create mode 100644 arch/arm/dts/zynqmp-binman-mini.dts
> create mode 100644 arch/arm/dts/zynqmp-binman-som.dts
> create mode 100644 arch/arm/dts/zynqmp-binman.dts
> create mode 100644 arch/arm/dts/zynqmp-u-boot.dtsi
> delete mode 100755 arch/arm/mach-zynqmp/mkimage_fit_atf.sh
>
Any issue with this series?
Thanks,
Michal
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/7] arm64: zynqmp: Convert platforms to use binman
2024-11-01 9:17 [PATCH v2 0/7] arm64: zynqmp: Convert platforms to use binman Michal Simek
` (7 preceding siblings ...)
2024-11-11 11:46 ` [PATCH v2 0/7] arm64: zynqmp: Convert platforms to use binman Michal Simek
@ 2024-11-27 7:59 ` Michal Simek
8 siblings, 0 replies; 28+ messages in thread
From: Michal Simek @ 2024-11-27 7:59 UTC (permalink / raw)
To: u-boot, git, Simon Glass, neal.frager
Cc: AKASHI Takahiro, Adam Ford, Bryan Brattlof, Caleb Connolly,
Charlie Johnston, Christian Marangi, Csókás Bence,
Fabio Estevam, Heinrich Schuchardt, Ilias Apalodimas,
Jerome Forissier, Jonas Karlman, Kever Yang, Marek Vasut,
Marek Vasut, Neil Armstrong, Oliver Gaskell, Patrick Rudolph,
Peter Robinson, Prasad Kummari, Quentin Schulz, Rasmus Villemoes,
Rayagonda Kokatanur, Sean Anderson, Sughosh Ganu, Sumit Garg,
Tejas Bhumkar, Tom Rini, Tony Dinh, Venkatesh Yadav Abbarapu
On 11/1/24 10:17, Michal Simek wrote:
> Hi,
>
> I have put togethere couple of patches to convert platforms to use binman.
> The first patch has been sent separately. The third (SOM description) has
> been also sent out for ilustration as RFC.
> The last one is just cherry-pick the patch which has been reverted because
> our platform wasn't converted to binman yet.
>
> v1 of external description
> https://lore.kernel.org/r/fbed0251437b61a2f7a85596d7403b5b9c8237c1.1728306322.git.michal.simek@amd.com
>
> RFC for SOM:
> https://lore.kernel.org/r/de1b8dbabd5ab7f20d7aac217ec4f5074d39f1da.1728462767.git.michal.simek@amd.com
>
> Thanks,
> Michal
>
> Changes in v2:
> - Change subject, align commit message
> - binman - rename variable
> - Use relative path
> - Use separate variable in makefile to also handle empty string and default
> to u-boot.dtb
> - new patch
> - new patch
> - Align fit,fdt-list-val to have shorter lines
> - Add reference to defconfig
> - Rename zynqmp-som-binman.dts to zynqmp-binman-som.dts
> - Use conf instead of config
> - Change image name from image.bin to qspi.bin
> - Remove RFC
> - Change default addresses for BL31/BL32
> - new patch
> - new patch
> - Rebase on the top of my series which do conversion to binman
> - Taken from git with all tags
>
> Marek Vasut (1):
> Makefile: Drop SPL_FIT_GENERATOR support
>
> Michal Simek (6):
> binman: Add option for pointing to separate description
> common: binman: Calling initr_binman() when BINMAN_FDT
> arm64: zynqmp: Describe empty binman node
> arm64: zynqmp: Add binman description for SOM
> arm64: zynqmp: Generate u-boot.itb and QSPI image via binman
> arm64: zynqmp: Remove mkimage fit script
>
> Makefile | 29 +--
> arch/arm/Kconfig | 1 +
> arch/arm/dts/Makefile | 3 +
> arch/arm/dts/zynqmp-binman-mini.dts | 10 +
> arch/arm/dts/zynqmp-binman-som.dts | 225 ++++++++++++++++
> arch/arm/dts/zynqmp-binman.dts | 111 ++++++++
> arch/arm/dts/zynqmp-u-boot.dtsi | 11 +
> arch/arm/mach-zynqmp/Kconfig | 14 +
> arch/arm/mach-zynqmp/mkimage_fit_atf.sh | 240 ------------------
> boot/Kconfig | 15 --
> common/board_r.c | 7 +-
> configs/xilinx_zynqmp_kria_defconfig | 3 +
> configs/xilinx_zynqmp_mini_defconfig | 2 +
> configs/xilinx_zynqmp_mini_emmc0_defconfig | 3 +
> configs/xilinx_zynqmp_mini_emmc1_defconfig | 3 +
> configs/xilinx_zynqmp_mini_nand_defconfig | 2 +
> .../xilinx_zynqmp_mini_nand_single_defconfig | 2 +
> configs/xilinx_zynqmp_mini_qspi_defconfig | 3 +
> configs/xilinx_zynqmp_virt_defconfig | 3 +
> doc/usage/fit/howto.rst | 4 -
> lib/Kconfig | 9 +
> 21 files changed, 418 insertions(+), 282 deletions(-)
> create mode 100644 arch/arm/dts/zynqmp-binman-mini.dts
> create mode 100644 arch/arm/dts/zynqmp-binman-som.dts
> create mode 100644 arch/arm/dts/zynqmp-binman.dts
> create mode 100644 arch/arm/dts/zynqmp-u-boot.dtsi
> delete mode 100755 arch/arm/mach-zynqmp/mkimage_fit_atf.sh
>
Applied and queue to next.
Thanks,
Michal
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 7/7] Makefile: Drop SPL_FIT_GENERATOR support
2024-11-01 9:18 ` [PATCH v2 7/7] Makefile: Drop SPL_FIT_GENERATOR support Michal Simek
@ 2024-12-06 19:16 ` Simon Glass
0 siblings, 0 replies; 28+ messages in thread
From: Simon Glass @ 2024-12-06 19:16 UTC (permalink / raw)
To: Michal Simek
Cc: u-boot, git, neal.frager, Marek Vasut, Peter Robinson,
Bryan Brattlof, Heinrich Schuchardt, Ilias Apalodimas,
Quentin Schulz, Sean Anderson, Sumit Garg, Tom Rini
On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
>
> From: Marek Vasut <marex@denx.de>
>
> The SPL_FIT_GENERATOR is long superseded by binman, drop SPL_FIT_GENERATOR
> support as there are no more users.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Reviewed-by: Peter Robinson <pbrobinson@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> ---
>
> Changes in v2:
> - Rebase on the top of my series which do conversion to binman
> - Taken from git with all tags
>
> Makefile | 18 ------------------
> boot/Kconfig | 14 --------------
> doc/usage/fit/howto.rst | 4 ----
> 3 files changed, 36 deletions(-)
>
Reviewed-by: Simon Glass <sjg@chromium.org>
Thanks!
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/7] binman: Add option for pointing to separate description
2024-11-01 9:17 ` [PATCH v2 1/7] binman: Add option for pointing to separate description Michal Simek
@ 2024-12-06 19:17 ` Simon Glass
0 siblings, 0 replies; 28+ messages in thread
From: Simon Glass @ 2024-12-06 19:17 UTC (permalink / raw)
To: Michal Simek
Cc: u-boot, git, neal.frager, AKASHI Takahiro, Bryan Brattlof,
Heinrich Schuchardt, Ilias Apalodimas, Marek Vasut,
Patrick Rudolph, Peter Robinson, Sughosh Ganu, Sumit Garg,
Tom Rini
Hi Michal,
On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
>
> Adding binman node with target images description can be unwanted feature
> but as of today there is no way to disable it.
> Also on size constrained systems it is not useful to add binman description
> to DTB.
> Introduce BINMAN_DTB Kconfig symbol which allows separate DTB for target
> from DTB for binman itself.
>
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> ---
>
> Changes in v2:
> - Change subject, align commit message
> - binman - rename variable
> - Use relative path
> - Use separate variable in makefile to also handle empty string and default
> to u-boot.dtb
>
> Makefile | 11 ++++++++++-
> lib/Kconfig | 9 +++++++++
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
Reviewed-by: Simon Glass <sjg@chromium.org>
This seems OK for now, and unblocks the migration.
Longer term, I would rather have a way to tell binman to remove the
description, as mentioned.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 3/7] arm64: zynqmp: Describe empty binman node
2024-11-01 9:17 ` [PATCH v2 3/7] arm64: zynqmp: Describe empty binman node Michal Simek
@ 2024-12-06 19:19 ` Simon Glass
0 siblings, 0 replies; 28+ messages in thread
From: Simon Glass @ 2024-12-06 19:19 UTC (permalink / raw)
To: Michal Simek
Cc: u-boot, git, neal.frager, Adam Ford, Fabio Estevam,
Heinrich Schuchardt, Ilias Apalodimas, Jerome Forissier,
Jonas Karlman, Kever Yang, Marek Vasut, Neil Armstrong,
Oliver Gaskell, Prasad Kummari, Sean Anderson, Sumit Garg,
Tom Rini, Tony Dinh, Venkatesh Yadav Abbarapu
On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
>
> For enabling binman by default there is a need to have at least empty node
> present that's why create -u-boot.dtsi with empty node to cover all ZynqMP
> platforms.
>
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> ---
>
> Changes in v2:
> - new patch
>
> arch/arm/dts/Makefile | 1 +
> arch/arm/dts/zynqmp-binman-mini.dts | 10 ++++++++++
> arch/arm/dts/zynqmp-u-boot.dtsi | 11 +++++++++++
> configs/xilinx_zynqmp_mini_defconfig | 2 ++
> configs/xilinx_zynqmp_mini_emmc0_defconfig | 3 +++
> configs/xilinx_zynqmp_mini_emmc1_defconfig | 3 +++
> configs/xilinx_zynqmp_mini_nand_defconfig | 2 ++
> configs/xilinx_zynqmp_mini_nand_single_defconfig | 2 ++
> configs/xilinx_zynqmp_mini_qspi_defconfig | 3 +++
> 9 files changed, 37 insertions(+)
> create mode 100644 arch/arm/dts/zynqmp-binman-mini.dts
> create mode 100644 arch/arm/dts/zynqmp-u-boot.dtsi
>
Reviewed-by: Simon Glass <sjg@chromium.org>
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index aeccfa93fc53..253d883d6156 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -274,6 +274,7 @@ dtb-$(CONFIG_ARCH_ZYNQMP) += \
> zynqmp-mini-qspi-x1-stacked.dtb \
> zynqmp-mini-qspi-x2-single.dtb \
> zynqmp-mini-qspi-x2-stacked.dtb \
> + zynqmp-binman-mini.dtb \
> zynqmp-sc-revB.dtb \
> zynqmp-sc-revC.dtb \
> zynqmp-sm-k24-revA.dtb \
> diff --git a/arch/arm/dts/zynqmp-binman-mini.dts b/arch/arm/dts/zynqmp-binman-mini.dts
> new file mode 100644
> index 000000000000..8f3d18ef394b
> --- /dev/null
> +++ b/arch/arm/dts/zynqmp-binman-mini.dts
> @@ -0,0 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * (C) Copyright 2024, Advanced Micro Devices, Inc.
> + *
> + * Michal Simek <michal.simek@amd.com>
> + */
> +
> +/dts-v1/;
> +
> +#include "zynqmp-u-boot.dtsi"
> diff --git a/arch/arm/dts/zynqmp-u-boot.dtsi b/arch/arm/dts/zynqmp-u-boot.dtsi
> new file mode 100644
> index 000000000000..9a7527ed5a10
> --- /dev/null
> +++ b/arch/arm/dts/zynqmp-u-boot.dtsi
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * (C) Copyright 2024, Advanced Micro Devices, Inc.
> + *
> + * Michal Simek <michal.simek@amd.com>
> + */
> +
> +/ {
> + binman: binman {
> + };
> +};
> diff --git a/configs/xilinx_zynqmp_mini_defconfig b/configs/xilinx_zynqmp_mini_defconfig
> index 7aab69c9e46b..f0af27bbd4e6 100644
> --- a/configs/xilinx_zynqmp_mini_defconfig
> +++ b/configs/xilinx_zynqmp_mini_defconfig
> @@ -59,6 +59,8 @@ CONFIG_NO_NET=y
> # CONFIG_DM_MAILBOX is not set
> # CONFIG_MMC is not set
> CONFIG_ARM_DCC=y
> +# CONFIG_BINMAN_FDT is not set
> +CONFIG_BINMAN_DTB="./arch/arm/dts/zynqmp-binman-mini.dtb"
> CONFIG_PANIC_HANG=y
> # CONFIG_GZIP is not set
> # CONFIG_LMB is not set
> diff --git a/configs/xilinx_zynqmp_mini_emmc0_defconfig b/configs/xilinx_zynqmp_mini_emmc0_defconfig
> index c56b1e830d67..9b3ab06f7c00 100644
> --- a/configs/xilinx_zynqmp_mini_emmc0_defconfig
> +++ b/configs/xilinx_zynqmp_mini_emmc0_defconfig
> @@ -28,6 +28,7 @@ CONFIG_BOARD_EARLY_INIT_R=y
> # CONFIG_BOARD_LATE_INIT is not set
> CONFIG_CLOCKS=y
> CONFIG_SPL_MAX_SIZE=0x40000
> +# CONFIG_SPL_BINMAN_SYMBOLS is not set
> # CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
> CONFIG_SPL_SYS_MALLOC=y
> CONFIG_SPL_HAS_CUSTOM_MALLOC_START=y
> @@ -73,6 +74,8 @@ CONFIG_SUPPORT_EMMC_BOOT=y
> CONFIG_MMC_SDHCI=y
> CONFIG_MMC_SDHCI_ZYNQ=y
> CONFIG_ARM_DCC=y
> +# CONFIG_BINMAN_FDT is not set
> +CONFIG_BINMAN_DTB="./arch/arm/dts/zynqmp-binman-mini.dtb"
> CONFIG_PANIC_HANG=y
> # CONFIG_GZIP is not set
> # CONFIG_LMB is not set
> diff --git a/configs/xilinx_zynqmp_mini_emmc1_defconfig b/configs/xilinx_zynqmp_mini_emmc1_defconfig
> index a8dbf0056da3..3b088c5002a7 100644
> --- a/configs/xilinx_zynqmp_mini_emmc1_defconfig
> +++ b/configs/xilinx_zynqmp_mini_emmc1_defconfig
> @@ -28,6 +28,7 @@ CONFIG_BOARD_EARLY_INIT_R=y
> # CONFIG_BOARD_LATE_INIT is not set
> CONFIG_CLOCKS=y
> CONFIG_SPL_MAX_SIZE=0x40000
> +# CONFIG_SPL_BINMAN_SYMBOLS is not set
> # CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
> CONFIG_SPL_SYS_MALLOC=y
> CONFIG_SPL_HAS_CUSTOM_MALLOC_START=y
> @@ -73,6 +74,8 @@ CONFIG_SUPPORT_EMMC_BOOT=y
> CONFIG_MMC_SDHCI=y
> CONFIG_MMC_SDHCI_ZYNQ=y
> CONFIG_ARM_DCC=y
> +# CONFIG_BINMAN_FDT is not set
> +CONFIG_BINMAN_DTB="./arch/arm/dts/zynqmp-binman-mini.dtb"
> CONFIG_PANIC_HANG=y
> # CONFIG_GZIP is not set
> # CONFIG_LMB is not set
> diff --git a/configs/xilinx_zynqmp_mini_nand_defconfig b/configs/xilinx_zynqmp_mini_nand_defconfig
> index ba8f02c5b11d..83bd3c9a5345 100644
> --- a/configs/xilinx_zynqmp_mini_nand_defconfig
> +++ b/configs/xilinx_zynqmp_mini_nand_defconfig
> @@ -59,6 +59,8 @@ CONFIG_NAND_ARASAN=y
> CONFIG_SYS_NAND_ONFI_DETECTION=y
> CONFIG_SYS_NAND_MAX_CHIPS=2
> CONFIG_ARM_DCC=y
> +# CONFIG_BINMAN_FDT is not set
> +CONFIG_BINMAN_DTB="./arch/arm/dts/zynqmp-binman-mini.dtb"
> CONFIG_PANIC_HANG=y
> # CONFIG_GZIP is not set
> # CONFIG_LMB is not set
> diff --git a/configs/xilinx_zynqmp_mini_nand_single_defconfig b/configs/xilinx_zynqmp_mini_nand_single_defconfig
> index a8a0055f2e5b..fea0c6e08415 100644
> --- a/configs/xilinx_zynqmp_mini_nand_single_defconfig
> +++ b/configs/xilinx_zynqmp_mini_nand_single_defconfig
> @@ -58,6 +58,8 @@ CONFIG_MTD_RAW_NAND=y
> CONFIG_NAND_ARASAN=y
> CONFIG_SYS_NAND_ONFI_DETECTION=y
> CONFIG_ARM_DCC=y
> +# CONFIG_BINMAN_FDT is not set
> +CONFIG_BINMAN_DTB="./arch/arm/dts/zynqmp-binman-mini.dtb"
> CONFIG_PANIC_HANG=y
> # CONFIG_GZIP is not set
> # CONFIG_LMB is not set
> diff --git a/configs/xilinx_zynqmp_mini_qspi_defconfig b/configs/xilinx_zynqmp_mini_qspi_defconfig
> index c08b10c6944f..39e50ff8c4f8 100644
> --- a/configs/xilinx_zynqmp_mini_qspi_defconfig
> +++ b/configs/xilinx_zynqmp_mini_qspi_defconfig
> @@ -31,6 +31,7 @@ CONFIG_LOGLEVEL=0
> # CONFIG_BOARD_LATE_INIT is not set
> CONFIG_CLOCKS=y
> CONFIG_SPL_MAX_SIZE=0x40000
> +# CONFIG_SPL_BINMAN_SYMBOLS is not set
> # CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
> CONFIG_SPL_SYS_MALLOC=y
> CONFIG_SPL_HAS_CUSTOM_MALLOC_START=y
> @@ -91,6 +92,8 @@ CONFIG_SPI_FLASH_WINBOND=y
> CONFIG_ARM_DCC=y
> CONFIG_SPI=y
> CONFIG_ZYNQMP_GQSPI=y
> +# CONFIG_BINMAN_FDT is not set
> +CONFIG_BINMAN_DTB="./arch/arm/dts/zynqmp-binman-mini.dtb"
> CONFIG_PANIC_HANG=y
> # CONFIG_GZIP is not set
> # CONFIG_LMB is not set
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/7] arm64: zynqmp: Add binman description for SOM
2024-11-01 9:17 ` [PATCH v2 4/7] arm64: zynqmp: Add binman description for SOM Michal Simek
@ 2024-12-06 19:19 ` Simon Glass
0 siblings, 0 replies; 28+ messages in thread
From: Simon Glass @ 2024-12-06 19:19 UTC (permalink / raw)
To: Michal Simek
Cc: u-boot, git, neal.frager, Adam Ford, Caleb Connolly,
Charlie Johnston, Csókás Bence, Fabio Estevam,
Jonas Karlman, Kever Yang, Marek Vasut, Neil Armstrong,
Oliver Gaskell, Patrick Rudolph, Peter Robinson, Prasad Kummari,
Rayagonda Kokatanur, Sumit Garg, Tejas Bhumkar, Tom Rini,
Tony Dinh, Venkatesh Yadav Abbarapu
On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
>
> There is necessary to do some steps to compose boot images. These steps
> were in scripts in layers for a while. That's why introduce description via
> binman to simplify wiring and remove all scripting around.
> This should make sure that everybody is up2date with the latest versions.
>
> The first step is to create fit image with DTBs with descriptions in
> configuration node which is written as regular expression to match all SOM
> versions.
> Description is there for k24 and k26 in spite of low level psu_init
> configuration is different. The reason is that it goes to u-boot.itb image
> which is the same for k24 and k26.
> u-boot.itb is another image which is generated. It is normally generated
> via arch/arm/mach-zynqmp/mkimage_fit_atf.sh but this script is supposed to
> be deprecated.
> FIT image by purpose is using 64bit addresses to have default option to
> move images to high DDR (above 4GB). TF-A and TEE are optional components
> but in the most cases TF-A is present all the time and TEE(OP-TEE) is used
> by some configurations too.
>
> 3rd generated image is boot.bin with updated user field which contains
> version number. This image can be used with updated Image Selector
> which supports A/B update mechanisms with rollback protection.
>
> 4th image is image.bin which binary file which contains boot.bin and
> u-boot.itb together and can be programmed via origin Image Selector.
> This image can be also used for creating one capsule which contains both
> boot images (in SPL boot flow).
>
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> ---
>
> Changes in v2:
> - Align fit,fdt-list-val to have shorter lines
> - Add reference to defconfig
> - Rename zynqmp-som-binman.dts to zynqmp-binman-som.dts
> - Use conf instead of config
> - Change image name from image.bin to qspi.bin
> - Remove RFC
> - Change default addresses for BL31/BL32
>
> arch/arm/Kconfig | 1 +
> arch/arm/dts/Makefile | 1 +
> arch/arm/dts/zynqmp-binman-som.dts | 225 +++++++++++++++++++++++++++
> arch/arm/mach-zynqmp/Kconfig | 14 ++
> configs/xilinx_zynqmp_kria_defconfig | 3 +
> 5 files changed, 244 insertions(+)
> create mode 100644 arch/arm/dts/zynqmp-binman-som.dts
>
Reviewed-by: Simon Glass <sjg@chromium.org>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 787f983ffd46..bbf04f301188 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1324,6 +1324,7 @@ config ARCH_ZYNQMP_R5
> config ARCH_ZYNQMP
> bool "Xilinx ZynqMP based platform"
> select ARM64
> + select BINMAN
> select CLK
> select DM
> select DEBUG_UART_BOARD_INIT if SPL && DEBUG_UART
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 253d883d6156..5d28a17b8b8c 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -370,6 +370,7 @@ dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-sm-k24-revA-sck-kv-g-revB.dtb
> dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-smk-k24-revA-sck-kv-g-revB.dtb
> dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-sm-k24-revA-sck-kr-g-revB.dtb
> dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-smk-k24-revA-sck-kr-g-revB.dtb
> +dtb-$(CONFIG_ARCH_ZYNQMP) += zynqmp-binman-som.dtb
>
> dtb-$(CONFIG_ARCH_VERSAL) += \
> versal-mini.dtb \
> diff --git a/arch/arm/dts/zynqmp-binman-som.dts b/arch/arm/dts/zynqmp-binman-som.dts
> new file mode 100644
> index 000000000000..3d9d8476c98e
> --- /dev/null
> +++ b/arch/arm/dts/zynqmp-binman-som.dts
> @@ -0,0 +1,225 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * dts file for Xilinx ZynqMP SOMs (k24/k26)
> + *
> + * (C) Copyright 2024, Advanced Micro Devices, Inc.
> + *
> + * Michal Simek <michal.simek@amd.com>
> + */
> +
> +#include <config.h>
> +
> +/dts-v1/;
> +/ {
> + binman: binman {
> + multiple-images;
> + fit-dtb.blob {
> + filename = "fit-dtb.blob";
> + pad-byte = <0>;
> + fit {
> + fit,align = <0x8>;
> + fit,external-offset = <0x0>;
> + description = "DTBs for SOMs+CCs";
> + fit,fdt-list-val = "zynqmp-smk-k26-revA", "zynqmp-smk-k26-revA-sck-kr-g-revA",
> + "zynqmp-smk-k26-revA-sck-kr-g-revB", "zynqmp-smk-k26-revA-sck-kv-g-revA",
> + "zynqmp-smk-k26-revA-sck-kv-g-revB", "zynqmp-sm-k26-revA-sck-kv-g-revA",
> + "zynqmp-sm-k26-revA-sck-kv-g-revB", "zynqmp-sm-k26-revA-sck-kr-g-revB",
> + "zynqmp-smk-k24-revA-sck-kd-g-revA", "zynqmp-smk-k24-revA-sck-kv-g-revB",
> + "zynqmp-smk-k24-revA-sck-kr-g-revB", "zynqmp-sm-k24-revA-sck-kd-g-revA",
> + "zynqmp-sm-k24-revA-sck-kv-g-revB", "zynqmp-sm-k24-revA-sck-kr-g-revB";
> +
> + images {
> + @fdt-SEQ {
> + description = "NAME";
> + type = "flat_dt";
> + arch = "arm64";
> + compression = "none";
> + hash-1 {
> + algo = "md5";
> + };
> + };
> + };
> + configurations {
> + default = "conf-1";
> + conf-1 {
> + description = "SOM itself";
> + fdt = "fdt-1";
> + };
> + conf-2 {
> + description = "zynqmp-smk-k26-.*-sck-kr-g-revA";
> + fdt = "fdt-2";
> + };
> + conf-3 {
> + description = "zynqmp-smk-k26-.*-sck-kr-g-.*";
> + fdt = "fdt-3";
> + };
> + conf-4 {
> + description = "zynqmp-smk-k26-.*-sck-kv-g-rev[AZ]";
> + fdt = "fdt-4";
> + };
> + conf-5 {
> + description = "zynqmp-smk-k26-.*-sck-kv-g-.*";
> + fdt = "fdt-5";
> + };
> + conf-6 {
> + description = "zynqmp-sm-k26-.*-sck-kv-g-rev[AZ]";
> + fdt = "fdt-6";
> + };
> + conf-7 {
> + description = "zynqmp-sm-k26-.*-sck-kv-g-.*";
> + fdt = "fdt-7";
> + };
> + conf-8 {
> + description = "zynqmp-sm-k26-.*-sck-kr-g-.*";
> + fdt = "fdt-8";
> + };
> + conf-9 {
> + description = "zynqmp-smk-k24-.*-sck-kd-g-.*";
> + fdt = "fdt-9";
> + };
> + conf-10 {
> + description = "zynqmp-smk-k24-.*-sck-kv-g-.*";
> + fdt = "fdt-10";
> + };
> + conf-11 {
> + description = "zynqmp-smk-k24-.*-sck-kr-g-.*";
> + fdt = "fdt-11";
> + };
> + conf-12 {
> + description = "zynqmp-sm-k24-.*-sck-kd-g-.*";
> + fdt = "fdt-12";
> + };
> + conf-13 {
> + description = "zynqmp-sm-k24-.*-sck-kv-g-.*";
> + fdt = "fdt-13";
> + };
> + conf-14 {
> + description = "zynqmp-sm-k24-.*-sck-kr-g-.*";
> + fdt = "fdt-14";
> + };
> + };
> + };
> + };
> +
> + /* u-boot.itb generation in a static way */
> + itb {
> + filename = "u-boot.itb";
> + pad-byte = <0>;
> +
> + fit {
> + description = "Configuration for Xilinx ZynqMP SoC";
> + fit,align = <0x8>;
> + fit,external-offset = <0x0>;
> + images {
> + uboot {
> + description = "U-Boot (64-bit)";
> + type = "firmware";
> + os = "u-boot";
> + arch = "arm64";
> + compression = "none";
> + load = /bits/ 64 <CONFIG_TEXT_BASE>;
> + entry = /bits/ 64 <CONFIG_TEXT_BASE>;
> + hash {
> + algo = "md5";
> + };
> + u-boot-nodtb {
> + };
> + };
> + atf {
> + description = "Trusted Firmware-A";
> + type = "firmware";
> + os = "arm-trusted-firmware";
> + arch = "arm64";
> + compression = "none";
> + load = /bits/ 64 <CONFIG_BL31_LOAD_ADDR>;
> + entry = /bits/ 64 <CONFIG_BL31_LOAD_ADDR>;
> + hash {
> + algo = "md5";
> + };
> + atf-bl31 {
> + optional;
> + };
> + };
> + tee {
> + description = "OP-TEE";
> + type = "tee";
> + arch = "arm64";
> + compression = "none";
> + os = "tee";
> + load = /bits/ 64 <CONFIG_BL31_LOAD_ADDR>;
> + entry = /bits/ 64 <CONFIG_BL31_LOAD_ADDR>;
> + tee-os {
> + optional;
> + };
> + };
> + fdt {
> + description = "Multi DTB fit image";
> + type = "flat_dt";
> + arch = "arm64";
> + compression = "none";
> + load = <0x0 0x100000>;
> + hash {
> + algo = "md5";
> + };
> + fdt-blob {
> + filename = "fit-dtb.blob";
> + type = "blob-ext";
> + };
> + };
> + };
> + configurations {
> + default = "conf-1";
> + conf-1 {
> + description = "Multi DTB with TF-A/TEE";
> + firmware = "atf";
> + loadables = "tee", "uboot", "fdt";
> + };
> + };
> + };
> + };
> +
> + /* boot.bin generated with version string inside */
> + bootimage {
> + filename = "boot.bin";
> + pad-byte = <0>;
> +
> + blob-ext@1 {
> + offset = <0x0>;
> + filename = "spl/boot.bin";
> + };
> + /* Optional version string at offset 0x70 */
> + blob-ext@2 {
> + offset = <0x70>;
> + filename = "version.bin";
> + overlap;
> + optional;
> + };
> + /* Optional version string at offset 0x94 */
> + blob-ext@3 {
> + offset = <0x94>;
> + filename = "version.bin";
> + overlap;
> + optional;
> + };
> + };
> +
> +#ifdef CONFIG_SYS_SPI_U_BOOT_OFFS
> + /* Full QSPI image for recovery app */
> + image {
> + filename = "qspi.bin";
> + pad-byte = <0>;
> +
> + blob-ext@1 {
> + offset = <0x0>;
> + filename = "boot.bin";
> + };
> + blob-ext@2 {
> + offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>;
> + filename = "u-boot.itb";
> + };
> + fdtmap {
> + };
> + };
> +#endif
> + };
> +};
> diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
> index aea13622b68a..92d61e84319d 100644
> --- a/arch/arm/mach-zynqmp/Kconfig
> +++ b/arch/arm/mach-zynqmp/Kconfig
> @@ -132,6 +132,20 @@ config SPL_ZYNQMP_RESTORE_JTAG
> even if no eFuses were burnt. This option restores the interface if
> possible.
>
> +config BL31_LOAD_ADDR
> + hex "Load address of BL31 image (mostly TF-A)"
> + default 0xfffea000
> + help
> + The load address for the BL31 image. This value is used to build the
> + FIT image header that places BL31 in memory where it will run.
> +
> +config BL32_LOAD_ADDR
> + hex "Load address of BL32 image (mostly secure OS)"
> + default 0
> + help
> + The load address for the BL32 image. This value is used to build the
> + FIT image header that places BL32 in memory where it will run.
> +
> config ZYNQ_SDHCI_MAX_FREQ
> default 200000000
>
> diff --git a/configs/xilinx_zynqmp_kria_defconfig b/configs/xilinx_zynqmp_kria_defconfig
> index 5757a0c9be9b..6d66a9423797 100644
> --- a/configs/xilinx_zynqmp_kria_defconfig
> +++ b/configs/xilinx_zynqmp_kria_defconfig
> @@ -47,6 +47,7 @@ CONFIG_SYS_PBSIZE=2073
> CONFIG_BOARD_EARLY_INIT_R=y
> CONFIG_CLOCKS=y
> CONFIG_SPL_MAX_SIZE=0x40000
> +# CONFIG_SPL_BINMAN_SYMBOLS is not set
> # CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
> CONFIG_SPL_FS_LOAD_KERNEL_NAME=""
> CONFIG_SPL_FS_LOAD_ARGS_NAME=""
> @@ -224,6 +225,8 @@ CONFIG_VIDEO_ZYNQMP_DPSUB=y
> CONFIG_VIRTIO_MMIO=y
> CONFIG_VIRTIO_NET=y
> CONFIG_VIRTIO_BLK=y
> +# CONFIG_BINMAN_FDT is not set
> +CONFIG_BINMAN_DTB="./arch/arm/dts/zynqmp-binman-som.dtb"
> CONFIG_PANIC_HANG=y
> CONFIG_TPM=y
> CONFIG_SPL_GZIP=y
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/7] arm64: zynqmp: Remove mkimage fit script
2024-11-01 9:17 ` [PATCH v2 6/7] arm64: zynqmp: Remove mkimage fit script Michal Simek
@ 2024-12-06 19:20 ` Simon Glass
0 siblings, 0 replies; 28+ messages in thread
From: Simon Glass @ 2024-12-06 19:20 UTC (permalink / raw)
To: Michal Simek
Cc: u-boot, git, neal.frager, Heinrich Schuchardt, Ilias Apalodimas,
Marek Vasut, Peter Robinson, Quentin Schulz, Tom Rini
On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
>
> Platform has been switched to binman that's why there is no need for this
> script and also Kconfig symbols.
>
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> ---
>
> Changes in v2:
> - new patch
>
> arch/arm/mach-zynqmp/mkimage_fit_atf.sh | 240 ------------------------
> boot/Kconfig | 1 -
> 2 files changed, 241 deletions(-)
> delete mode 100755 arch/arm/mach-zynqmp/mkimage_fit_atf.sh
>
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 5/7] arm64: zynqmp: Generate u-boot.itb and QSPI image via binman
2024-11-01 9:17 ` [PATCH v2 5/7] arm64: zynqmp: Generate u-boot.itb and QSPI image via binman Michal Simek
@ 2024-12-06 19:20 ` Simon Glass
0 siblings, 0 replies; 28+ messages in thread
From: Simon Glass @ 2024-12-06 19:20 UTC (permalink / raw)
To: Michal Simek
Cc: u-boot, git, neal.frager, Adam Ford, Fabio Estevam, Jonas Karlman,
Kever Yang, Marek Vasut, Neil Armstrong, Oliver Gaskell,
Prasad Kummari, Sumit Garg, Tejas Bhumkar, Tom Rini, Tony Dinh,
Venkatesh Yadav Abbarapu
On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
>
> u-boot.itb has been generated via mkimage_fit_atf.sh but it is on the way
> out that's why convert it's description to binman.
> Compare to script binman description is not able to configure BL31 and BL32
> load/entry addresses which should be done separately.
>
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> ---
>
> Changes in v2:
> - new patch
>
> arch/arm/dts/Makefile | 1 +
> arch/arm/dts/zynqmp-binman.dts | 111 +++++++++++++++++++++++++++
> configs/xilinx_zynqmp_virt_defconfig | 3 +
> 3 files changed, 115 insertions(+)
> create mode 100644 arch/arm/dts/zynqmp-binman.dts
>
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT
2024-11-01 9:17 ` [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT Michal Simek
@ 2024-12-06 19:20 ` Simon Glass
2024-12-09 15:26 ` Michal Simek
0 siblings, 1 reply; 28+ messages in thread
From: Simon Glass @ 2024-12-06 19:20 UTC (permalink / raw)
To: Michal Simek
Cc: u-boot, git, neal.frager, Christian Marangi, Ilias Apalodimas,
Jerome Forissier, Rasmus Villemoes, Sughosh Ganu, Tom Rini
On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
>
> Calling empty function when BINMAN_FDT is adding +64B for nothing which is
> not helping on size sensitive configurations as Xilinx mini configurations.
>
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> ---
>
> Changes in v2:
> - new patch
>
> From my perspective there is no reason to call empty function. It is just
> increase footprint for nothing and we are not far from that limit now.
>
> ---
> common/board_r.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
This is a bit odd, though. Do you have LTO enabled?
>
> diff --git a/common/board_r.c b/common/board_r.c
> index 62228a723e12..ff9bce88dc93 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -287,13 +287,10 @@ static int initr_announce(void)
> return 0;
> }
>
> -static int initr_binman(void)
> +static int __maybe_unused initr_binman(void)
> {
> int ret;
>
> - if (!CONFIG_IS_ENABLED(BINMAN_FDT))
> - return 0;
> -
> ret = binman_init();
> if (ret)
> printf("binman_init failed:%d\n", ret);
> @@ -635,7 +632,9 @@ static init_fnc_t init_sequence_r[] = {
> #ifdef CONFIG_EFI_LOADER
> efi_memory_init,
> #endif
> +#ifdef CONFIG_BINMAN_FDT
> initr_binman,
> +#endif
> #ifdef CONFIG_FSP_VERSION2
> arch_fsp_init_r,
> #endif
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT
2024-12-06 19:20 ` Simon Glass
@ 2024-12-09 15:26 ` Michal Simek
2024-12-09 15:32 ` Tom Rini
0 siblings, 1 reply; 28+ messages in thread
From: Michal Simek @ 2024-12-09 15:26 UTC (permalink / raw)
To: Simon Glass
Cc: u-boot, git, neal.frager, Christian Marangi, Ilias Apalodimas,
Jerome Forissier, Rasmus Villemoes, Sughosh Ganu, Tom Rini
On 12/6/24 20:20, Simon Glass wrote:
> On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
>>
>> Calling empty function when BINMAN_FDT is adding +64B for nothing which is
>> not helping on size sensitive configurations as Xilinx mini configurations.
>>
>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>> ---
>>
>> Changes in v2:
>> - new patch
>>
>> From my perspective there is no reason to call empty function. It is just
>> increase footprint for nothing and we are not far from that limit now.
>>
>> ---
>> common/board_r.c | 7 +++----
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> This is a bit odd, though. Do you have LTO enabled?
>
yes LTO is enabled. And there are other candidates like this.
Is LTO able to fix function arrays which is calling empty function?
(without this patch)
00000000fffc0eb4 <initr_of_live>:
fffc0eb4: 52800000 mov w0, #0x0 // #0
fffc0eb8: d65f03c0 ret
00000000fffc0ebc <initr_dm_devices>:
fffc0ebc: 52800000 mov w0, #0x0 // #0
fffc0ec0: d65f03c0 ret
00000000fffc0ec4 <initr_bootstage>:
fffc0ec4: 52800000 mov w0, #0x0 // #0
fffc0ec8: d65f03c0 ret
00000000fffc0ecc <power_init_board>:
fffc0ecc: 52800000 mov w0, #0x0 // #0
fffc0ed0: d65f03c0 ret
00000000fffc0ed4 <initr_announce>:
fffc0ed4: 52800000 mov w0, #0x0 // #0
fffc0ed8: d65f03c0 ret
00000000fffc0edc <initr_binman>:
fffc0edc: 52800000 mov w0, #0x0 // #0
fffc0ee0: d65f03c0 ret
00000000fffc0ee4 <initr_status_led>:
fffc0ee4: 52800000 mov w0, #0x0 // #0
fffc0ee8: d65f03c0 ret
00000000fffc0eec <initr_boot_led_blink>:
fffc0eec: 52800000 mov w0, #0x0 // #0
fffc0ef0: d65f03c0 ret
00000000fffc0ef4 <initr_boot_led_on>:
fffc0ef4: 52800000 mov w0, #0x0 // #0
fffc0ef8: d65f03c0 ret
00000000fffc0efc <initr_lmb>:
fffc0efc: 52800000 mov w0, #0x0 // #0
fffc0f00: d65f03c0 ret
thanks,
Michal
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT
2024-12-09 15:26 ` Michal Simek
@ 2024-12-09 15:32 ` Tom Rini
2024-12-09 15:47 ` Simon Glass
0 siblings, 1 reply; 28+ messages in thread
From: Tom Rini @ 2024-12-09 15:32 UTC (permalink / raw)
To: Michal Simek
Cc: Simon Glass, u-boot, git, neal.frager, Christian Marangi,
Ilias Apalodimas, Jerome Forissier, Rasmus Villemoes,
Sughosh Ganu
[-- Attachment #1: Type: text/plain, Size: 2887 bytes --]
On Mon, Dec 09, 2024 at 04:26:15PM +0100, Michal Simek wrote:
>
>
> On 12/6/24 20:20, Simon Glass wrote:
> > On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
> > >
> > > Calling empty function when BINMAN_FDT is adding +64B for nothing which is
> > > not helping on size sensitive configurations as Xilinx mini configurations.
> > >
> > > Signed-off-by: Michal Simek <michal.simek@amd.com>
> > > ---
> > >
> > > Changes in v2:
> > > - new patch
> > >
> > > From my perspective there is no reason to call empty function. It is just
> > > increase footprint for nothing and we are not far from that limit now.
> > >
> > > ---
> > > common/board_r.c | 7 +++----
> > > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > This is a bit odd, though. Do you have LTO enabled?
> >
>
> yes LTO is enabled. And there are other candidates like this.
> Is LTO able to fix function arrays which is calling empty function?
>
> (without this patch)
>
> 00000000fffc0eb4 <initr_of_live>:
> fffc0eb4: 52800000 mov w0, #0x0 // #0
> fffc0eb8: d65f03c0 ret
>
> 00000000fffc0ebc <initr_dm_devices>:
> fffc0ebc: 52800000 mov w0, #0x0 // #0
> fffc0ec0: d65f03c0 ret
>
> 00000000fffc0ec4 <initr_bootstage>:
> fffc0ec4: 52800000 mov w0, #0x0 // #0
> fffc0ec8: d65f03c0 ret
>
> 00000000fffc0ecc <power_init_board>:
> fffc0ecc: 52800000 mov w0, #0x0 // #0
> fffc0ed0: d65f03c0 ret
>
> 00000000fffc0ed4 <initr_announce>:
> fffc0ed4: 52800000 mov w0, #0x0 // #0
> fffc0ed8: d65f03c0 ret
>
> 00000000fffc0edc <initr_binman>:
> fffc0edc: 52800000 mov w0, #0x0 // #0
> fffc0ee0: d65f03c0 ret
>
> 00000000fffc0ee4 <initr_status_led>:
> fffc0ee4: 52800000 mov w0, #0x0 // #0
> fffc0ee8: d65f03c0 ret
>
> 00000000fffc0eec <initr_boot_led_blink>:
> fffc0eec: 52800000 mov w0, #0x0 // #0
> fffc0ef0: d65f03c0 ret
>
> 00000000fffc0ef4 <initr_boot_led_on>:
> fffc0ef4: 52800000 mov w0, #0x0 // #0
> fffc0ef8: d65f03c0 ret
>
> 00000000fffc0efc <initr_lmb>:
> fffc0efc: 52800000 mov w0, #0x0 // #0
> fffc0f00: d65f03c0 ret
No, but maybe Simon would prefer if we marked all of the could-be-empty
functions as __maybe_unused and did:
CONFIG_IS_ENABLED(BINMAN_FDT, initr_binman),
etc in the list instead?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT
2024-12-09 15:32 ` Tom Rini
@ 2024-12-09 15:47 ` Simon Glass
2024-12-09 18:34 ` Michal Simek
0 siblings, 1 reply; 28+ messages in thread
From: Simon Glass @ 2024-12-09 15:47 UTC (permalink / raw)
To: Tom Rini
Cc: Michal Simek, u-boot, git, neal.frager, Christian Marangi,
Ilias Apalodimas, Jerome Forissier, Rasmus Villemoes,
Sughosh Ganu
Hi,
On Mon, 9 Dec 2024 at 08:32, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Dec 09, 2024 at 04:26:15PM +0100, Michal Simek wrote:
> >
> >
> > On 12/6/24 20:20, Simon Glass wrote:
> > > On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
> > > >
> > > > Calling empty function when BINMAN_FDT is adding +64B for nothing which is
> > > > not helping on size sensitive configurations as Xilinx mini configurations.
> > > >
> > > > Signed-off-by: Michal Simek <michal.simek@amd.com>
> > > > ---
> > > >
> > > > Changes in v2:
> > > > - new patch
> > > >
> > > > From my perspective there is no reason to call empty function. It is just
> > > > increase footprint for nothing and we are not far from that limit now.
> > > >
> > > > ---
> > > > common/board_r.c | 7 +++----
> > > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > >
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > >
> > > This is a bit odd, though. Do you have LTO enabled?
> > >
> >
> > yes LTO is enabled. And there are other candidates like this.
> > Is LTO able to fix function arrays which is calling empty function?
> >
> > (without this patch)
> >
> > 00000000fffc0eb4 <initr_of_live>:
> > fffc0eb4: 52800000 mov w0, #0x0 // #0
> > fffc0eb8: d65f03c0 ret
> >
> > 00000000fffc0ebc <initr_dm_devices>:
> > fffc0ebc: 52800000 mov w0, #0x0 // #0
> > fffc0ec0: d65f03c0 ret
> >
> > 00000000fffc0ec4 <initr_bootstage>:
> > fffc0ec4: 52800000 mov w0, #0x0 // #0
> > fffc0ec8: d65f03c0 ret
> >
> > 00000000fffc0ecc <power_init_board>:
> > fffc0ecc: 52800000 mov w0, #0x0 // #0
> > fffc0ed0: d65f03c0 ret
> >
> > 00000000fffc0ed4 <initr_announce>:
> > fffc0ed4: 52800000 mov w0, #0x0 // #0
> > fffc0ed8: d65f03c0 ret
> >
> > 00000000fffc0edc <initr_binman>:
> > fffc0edc: 52800000 mov w0, #0x0 // #0
> > fffc0ee0: d65f03c0 ret
> >
> > 00000000fffc0ee4 <initr_status_led>:
> > fffc0ee4: 52800000 mov w0, #0x0 // #0
> > fffc0ee8: d65f03c0 ret
> >
> > 00000000fffc0eec <initr_boot_led_blink>:
> > fffc0eec: 52800000 mov w0, #0x0 // #0
> > fffc0ef0: d65f03c0 ret
> >
> > 00000000fffc0ef4 <initr_boot_led_on>:
> > fffc0ef4: 52800000 mov w0, #0x0 // #0
> > fffc0ef8: d65f03c0 ret
> >
> > 00000000fffc0efc <initr_lmb>:
> > fffc0efc: 52800000 mov w0, #0x0 // #0
> > fffc0f00: d65f03c0 ret
>
> No, but maybe Simon would prefer if we marked all of the could-be-empty
> functions as __maybe_unused and did:
> CONFIG_IS_ENABLED(BINMAN_FDT, initr_binman),
> etc in the list instead?
Yes that looks better.
Michal, see also [1] in case you can work out why it 'stopped
working'. I could have sworn inlining the function was a win when it
was applied, but no amount of toolchain juggling could make it be a
win when I came back to it later.
Regards,
SImon
[1] e7f59dea880 Revert "initcall: Move to inline function"
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT
2024-12-09 15:47 ` Simon Glass
@ 2024-12-09 18:34 ` Michal Simek
2024-12-09 19:23 ` Tom Rini
2024-12-09 19:27 ` Simon Glass
0 siblings, 2 replies; 28+ messages in thread
From: Michal Simek @ 2024-12-09 18:34 UTC (permalink / raw)
To: Simon Glass, Tom Rini
Cc: u-boot, git, neal.frager, Christian Marangi, Ilias Apalodimas,
Jerome Forissier, Rasmus Villemoes, Sughosh Ganu
On 12/9/24 16:47, Simon Glass wrote:
> Hi,
>
> On Mon, 9 Dec 2024 at 08:32, Tom Rini <trini@konsulko.com> wrote:
>>
>> On Mon, Dec 09, 2024 at 04:26:15PM +0100, Michal Simek wrote:
>>>
>>>
>>> On 12/6/24 20:20, Simon Glass wrote:
>>>> On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
>>>>>
>>>>> Calling empty function when BINMAN_FDT is adding +64B for nothing which is
>>>>> not helping on size sensitive configurations as Xilinx mini configurations.
>>>>>
>>>>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>>>>> ---
>>>>>
>>>>> Changes in v2:
>>>>> - new patch
>>>>>
>>>>> From my perspective there is no reason to call empty function. It is just
>>>>> increase footprint for nothing and we are not far from that limit now.
>>>>>
>>>>> ---
>>>>> common/board_r.c | 7 +++----
>>>>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>>>
>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>>
>>>> This is a bit odd, though. Do you have LTO enabled?
>>>>
>>>
>>> yes LTO is enabled. And there are other candidates like this.
>>> Is LTO able to fix function arrays which is calling empty function?
>>>
>>> (without this patch)
>>>
>>> 00000000fffc0eb4 <initr_of_live>:
>>> fffc0eb4: 52800000 mov w0, #0x0 // #0
>>> fffc0eb8: d65f03c0 ret
>>>
>>> 00000000fffc0ebc <initr_dm_devices>:
>>> fffc0ebc: 52800000 mov w0, #0x0 // #0
>>> fffc0ec0: d65f03c0 ret
>>>
>>> 00000000fffc0ec4 <initr_bootstage>:
>>> fffc0ec4: 52800000 mov w0, #0x0 // #0
>>> fffc0ec8: d65f03c0 ret
>>>
>>> 00000000fffc0ecc <power_init_board>:
>>> fffc0ecc: 52800000 mov w0, #0x0 // #0
>>> fffc0ed0: d65f03c0 ret
>>>
>>> 00000000fffc0ed4 <initr_announce>:
>>> fffc0ed4: 52800000 mov w0, #0x0 // #0
>>> fffc0ed8: d65f03c0 ret
>>>
>>> 00000000fffc0edc <initr_binman>:
>>> fffc0edc: 52800000 mov w0, #0x0 // #0
>>> fffc0ee0: d65f03c0 ret
>>>
>>> 00000000fffc0ee4 <initr_status_led>:
>>> fffc0ee4: 52800000 mov w0, #0x0 // #0
>>> fffc0ee8: d65f03c0 ret
>>>
>>> 00000000fffc0eec <initr_boot_led_blink>:
>>> fffc0eec: 52800000 mov w0, #0x0 // #0
>>> fffc0ef0: d65f03c0 ret
>>>
>>> 00000000fffc0ef4 <initr_boot_led_on>:
>>> fffc0ef4: 52800000 mov w0, #0x0 // #0
>>> fffc0ef8: d65f03c0 ret
>>>
>>> 00000000fffc0efc <initr_lmb>:
>>> fffc0efc: 52800000 mov w0, #0x0 // #0
>>> fffc0f00: d65f03c0 ret
>>
>> No, but maybe Simon would prefer if we marked all of the could-be-empty
>> functions as __maybe_unused and did:
>> CONFIG_IS_ENABLED(BINMAN_FDT, initr_binman),
>> etc in the list instead?
>
> Yes that looks better.
But we are talking about using macro inside array at best with using #ifdefs.
Or maybe I am not seeing what you are saying.
>
> Michal, see also [1] in case you can work out why it 'stopped
> working'. I could have sworn inlining the function was a win when it
> was applied, but no amount of toolchain juggling could make it be a
> win when I came back to it later.
Are you saying that it worked in past?
Thanks,
Michal
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT
2024-12-09 18:34 ` Michal Simek
@ 2024-12-09 19:23 ` Tom Rini
2024-12-09 19:27 ` Simon Glass
2024-12-09 19:27 ` Simon Glass
1 sibling, 1 reply; 28+ messages in thread
From: Tom Rini @ 2024-12-09 19:23 UTC (permalink / raw)
To: Michal Simek
Cc: Simon Glass, u-boot, git, neal.frager, Christian Marangi,
Ilias Apalodimas, Jerome Forissier, Rasmus Villemoes,
Sughosh Ganu
[-- Attachment #1: Type: text/plain, Size: 4547 bytes --]
On Mon, Dec 09, 2024 at 07:34:34PM +0100, Michal Simek wrote:
>
>
> On 12/9/24 16:47, Simon Glass wrote:
> > Hi,
> >
> > On Mon, 9 Dec 2024 at 08:32, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Mon, Dec 09, 2024 at 04:26:15PM +0100, Michal Simek wrote:
> > > >
> > > >
> > > > On 12/6/24 20:20, Simon Glass wrote:
> > > > > On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
> > > > > >
> > > > > > Calling empty function when BINMAN_FDT is adding +64B for nothing which is
> > > > > > not helping on size sensitive configurations as Xilinx mini configurations.
> > > > > >
> > > > > > Signed-off-by: Michal Simek <michal.simek@amd.com>
> > > > > > ---
> > > > > >
> > > > > > Changes in v2:
> > > > > > - new patch
> > > > > >
> > > > > > From my perspective there is no reason to call empty function. It is just
> > > > > > increase footprint for nothing and we are not far from that limit now.
> > > > > >
> > > > > > ---
> > > > > > common/board_r.c | 7 +++----
> > > > > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > > > >
> > > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > > >
> > > > > This is a bit odd, though. Do you have LTO enabled?
> > > > >
> > > >
> > > > yes LTO is enabled. And there are other candidates like this.
> > > > Is LTO able to fix function arrays which is calling empty function?
> > > >
> > > > (without this patch)
> > > >
> > > > 00000000fffc0eb4 <initr_of_live>:
> > > > fffc0eb4: 52800000 mov w0, #0x0 // #0
> > > > fffc0eb8: d65f03c0 ret
> > > >
> > > > 00000000fffc0ebc <initr_dm_devices>:
> > > > fffc0ebc: 52800000 mov w0, #0x0 // #0
> > > > fffc0ec0: d65f03c0 ret
> > > >
> > > > 00000000fffc0ec4 <initr_bootstage>:
> > > > fffc0ec4: 52800000 mov w0, #0x0 // #0
> > > > fffc0ec8: d65f03c0 ret
> > > >
> > > > 00000000fffc0ecc <power_init_board>:
> > > > fffc0ecc: 52800000 mov w0, #0x0 // #0
> > > > fffc0ed0: d65f03c0 ret
> > > >
> > > > 00000000fffc0ed4 <initr_announce>:
> > > > fffc0ed4: 52800000 mov w0, #0x0 // #0
> > > > fffc0ed8: d65f03c0 ret
> > > >
> > > > 00000000fffc0edc <initr_binman>:
> > > > fffc0edc: 52800000 mov w0, #0x0 // #0
> > > > fffc0ee0: d65f03c0 ret
> > > >
> > > > 00000000fffc0ee4 <initr_status_led>:
> > > > fffc0ee4: 52800000 mov w0, #0x0 // #0
> > > > fffc0ee8: d65f03c0 ret
> > > >
> > > > 00000000fffc0eec <initr_boot_led_blink>:
> > > > fffc0eec: 52800000 mov w0, #0x0 // #0
> > > > fffc0ef0: d65f03c0 ret
> > > >
> > > > 00000000fffc0ef4 <initr_boot_led_on>:
> > > > fffc0ef4: 52800000 mov w0, #0x0 // #0
> > > > fffc0ef8: d65f03c0 ret
> > > >
> > > > 00000000fffc0efc <initr_lmb>:
> > > > fffc0efc: 52800000 mov w0, #0x0 // #0
> > > > fffc0f00: d65f03c0 ret
> > >
> > > No, but maybe Simon would prefer if we marked all of the could-be-empty
> > > functions as __maybe_unused and did:
> > > CONFIG_IS_ENABLED(BINMAN_FDT, initr_binman),
> > > etc in the list instead?
> >
> > Yes that looks better.
>
> But we are talking about using macro inside array at best with using #ifdefs.
> Or maybe I am not seeing what you are saying.
No, nevermind, sorry. I was hoping that:
diff --git a/common/board_r.c b/common/board_r.c
index ff9bce88dc93..c18997446dfa 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -610,9 +610,7 @@ static init_fnc_t init_sequence_r[] = {
noncached_init,
#endif
initr_of_live,
-#ifdef CONFIG_DM
- initr_dm,
-#endif
+ CONFIG_IS_ENABLED(CONFIG_DM, initr_dm),
#ifdef CONFIG_ADDR_MAP
init_addr_map,
#endif
@@ -632,9 +630,7 @@ static init_fnc_t init_sequence_r[] = {
#ifdef CONFIG_EFI_LOADER
efi_memory_init,
#endif
-#ifdef CONFIG_BINMAN_FDT
- initr_binman,
-#endif
+ CONFIG_IS_ENABLED(CONFIG_BINMAN_FDT, initr_binman),
#ifdef CONFIG_FSP_VERSION2
arch_fsp_init_r,
#endif
would work, but the macro doesn't evaluate how I'd hope it did and that
just blows up.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT
2024-12-09 18:34 ` Michal Simek
2024-12-09 19:23 ` Tom Rini
@ 2024-12-09 19:27 ` Simon Glass
2024-12-10 12:40 ` Michal Simek
1 sibling, 1 reply; 28+ messages in thread
From: Simon Glass @ 2024-12-09 19:27 UTC (permalink / raw)
To: Michal Simek
Cc: Tom Rini, u-boot, git, neal.frager, Christian Marangi,
Ilias Apalodimas, Jerome Forissier, Rasmus Villemoes,
Sughosh Ganu
Hi Michal,
On Mon, 9 Dec 2024 at 11:34, Michal Simek <michal.simek@amd.com> wrote:
>
>
>
> On 12/9/24 16:47, Simon Glass wrote:
> > Hi,
> >
> > On Mon, 9 Dec 2024 at 08:32, Tom Rini <trini@konsulko.com> wrote:
> >>
> >> On Mon, Dec 09, 2024 at 04:26:15PM +0100, Michal Simek wrote:
> >>>
> >>>
> >>> On 12/6/24 20:20, Simon Glass wrote:
> >>>> On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
> >>>>>
> >>>>> Calling empty function when BINMAN_FDT is adding +64B for nothing which is
> >>>>> not helping on size sensitive configurations as Xilinx mini configurations.
> >>>>>
> >>>>> Signed-off-by: Michal Simek <michal.simek@amd.com>
> >>>>> ---
> >>>>>
> >>>>> Changes in v2:
> >>>>> - new patch
> >>>>>
> >>>>> From my perspective there is no reason to call empty function. It is just
> >>>>> increase footprint for nothing and we are not far from that limit now.
> >>>>>
> >>>>> ---
> >>>>> common/board_r.c | 7 +++----
> >>>>> 1 file changed, 3 insertions(+), 4 deletions(-)
> >>>>
> >>>> Reviewed-by: Simon Glass <sjg@chromium.org>
> >>>>
> >>>> This is a bit odd, though. Do you have LTO enabled?
> >>>>
> >>>
> >>> yes LTO is enabled. And there are other candidates like this.
> >>> Is LTO able to fix function arrays which is calling empty function?
> >>>
> >>> (without this patch)
> >>>
> >>> 00000000fffc0eb4 <initr_of_live>:
> >>> fffc0eb4: 52800000 mov w0, #0x0 // #0
> >>> fffc0eb8: d65f03c0 ret
> >>>
> >>> 00000000fffc0ebc <initr_dm_devices>:
> >>> fffc0ebc: 52800000 mov w0, #0x0 // #0
> >>> fffc0ec0: d65f03c0 ret
> >>>
> >>> 00000000fffc0ec4 <initr_bootstage>:
> >>> fffc0ec4: 52800000 mov w0, #0x0 // #0
> >>> fffc0ec8: d65f03c0 ret
> >>>
> >>> 00000000fffc0ecc <power_init_board>:
> >>> fffc0ecc: 52800000 mov w0, #0x0 // #0
> >>> fffc0ed0: d65f03c0 ret
> >>>
> >>> 00000000fffc0ed4 <initr_announce>:
> >>> fffc0ed4: 52800000 mov w0, #0x0 // #0
> >>> fffc0ed8: d65f03c0 ret
> >>>
> >>> 00000000fffc0edc <initr_binman>:
> >>> fffc0edc: 52800000 mov w0, #0x0 // #0
> >>> fffc0ee0: d65f03c0 ret
> >>>
> >>> 00000000fffc0ee4 <initr_status_led>:
> >>> fffc0ee4: 52800000 mov w0, #0x0 // #0
> >>> fffc0ee8: d65f03c0 ret
> >>>
> >>> 00000000fffc0eec <initr_boot_led_blink>:
> >>> fffc0eec: 52800000 mov w0, #0x0 // #0
> >>> fffc0ef0: d65f03c0 ret
> >>>
> >>> 00000000fffc0ef4 <initr_boot_led_on>:
> >>> fffc0ef4: 52800000 mov w0, #0x0 // #0
> >>> fffc0ef8: d65f03c0 ret
> >>>
> >>> 00000000fffc0efc <initr_lmb>:
> >>> fffc0efc: 52800000 mov w0, #0x0 // #0
> >>> fffc0f00: d65f03c0 ret
> >>
> >> No, but maybe Simon would prefer if we marked all of the could-be-empty
> >> functions as __maybe_unused and did:
> >> CONFIG_IS_ENABLED(BINMAN_FDT, initr_binman),
> >> etc in the list instead?
> >
> > Yes that looks better.
>
> But we are talking about using macro inside array at best with using #ifdefs.
> Or maybe I am not seeing what you are saying.
>
> >
> > Michal, see also [1] in case you can work out why it 'stopped
> > working'. I could have sworn inlining the function was a win when it
> > was applied, but no amount of toolchain juggling could make it be a
> > win when I came back to it later.
>
> Are you saying that it worked in past?
I wasn't able to verify that post facto, but I believe I do remember
checking it at the time. If you read the original commit message:
47870afab92 initcall: Move to inline function
The board_r init function was complaining that we are looping through
an array, calling all our tiny init stubs sequentially via indirect
function calls (which can't be speculated, so they are slow).
The solution to that is pretty easy though. All we need to do is inline
the function that loops through the functions and the compiler will
automatically convert almost all indirect calls into direct inlined code.
With this patch, the overall code size drops (by 40 bytes on riscv64)
and boot time should become measurably faster for every target.
Signed-off-by: Alexander Graf <agraf@suse.de>
Despite this hopeful sentiment, I seriously doubt any improvement in boot time.
Regards,
Simon
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT
2024-12-09 19:23 ` Tom Rini
@ 2024-12-09 19:27 ` Simon Glass
2024-12-09 20:08 ` Tom Rini
0 siblings, 1 reply; 28+ messages in thread
From: Simon Glass @ 2024-12-09 19:27 UTC (permalink / raw)
To: Tom Rini
Cc: Michal Simek, u-boot, git, neal.frager, Christian Marangi,
Ilias Apalodimas, Jerome Forissier, Rasmus Villemoes,
Sughosh Ganu
Hi Tom,
On Mon, 9 Dec 2024 at 12:23, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Dec 09, 2024 at 07:34:34PM +0100, Michal Simek wrote:
> >
> >
> > On 12/9/24 16:47, Simon Glass wrote:
> > > Hi,
> > >
> > > On Mon, 9 Dec 2024 at 08:32, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Mon, Dec 09, 2024 at 04:26:15PM +0100, Michal Simek wrote:
> > > > >
> > > > >
> > > > > On 12/6/24 20:20, Simon Glass wrote:
> > > > > > On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
> > > > > > >
> > > > > > > Calling empty function when BINMAN_FDT is adding +64B for nothing which is
> > > > > > > not helping on size sensitive configurations as Xilinx mini configurations.
> > > > > > >
> > > > > > > Signed-off-by: Michal Simek <michal.simek@amd.com>
> > > > > > > ---
> > > > > > >
> > > > > > > Changes in v2:
> > > > > > > - new patch
> > > > > > >
> > > > > > > From my perspective there is no reason to call empty function. It is just
> > > > > > > increase footprint for nothing and we are not far from that limit now.
> > > > > > >
> > > > > > > ---
> > > > > > > common/board_r.c | 7 +++----
> > > > > > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > > > >
> > > > > > This is a bit odd, though. Do you have LTO enabled?
> > > > > >
> > > > >
> > > > > yes LTO is enabled. And there are other candidates like this.
> > > > > Is LTO able to fix function arrays which is calling empty function?
> > > > >
> > > > > (without this patch)
> > > > >
> > > > > 00000000fffc0eb4 <initr_of_live>:
> > > > > fffc0eb4: 52800000 mov w0, #0x0 // #0
> > > > > fffc0eb8: d65f03c0 ret
> > > > >
> > > > > 00000000fffc0ebc <initr_dm_devices>:
> > > > > fffc0ebc: 52800000 mov w0, #0x0 // #0
> > > > > fffc0ec0: d65f03c0 ret
> > > > >
> > > > > 00000000fffc0ec4 <initr_bootstage>:
> > > > > fffc0ec4: 52800000 mov w0, #0x0 // #0
> > > > > fffc0ec8: d65f03c0 ret
> > > > >
> > > > > 00000000fffc0ecc <power_init_board>:
> > > > > fffc0ecc: 52800000 mov w0, #0x0 // #0
> > > > > fffc0ed0: d65f03c0 ret
> > > > >
> > > > > 00000000fffc0ed4 <initr_announce>:
> > > > > fffc0ed4: 52800000 mov w0, #0x0 // #0
> > > > > fffc0ed8: d65f03c0 ret
> > > > >
> > > > > 00000000fffc0edc <initr_binman>:
> > > > > fffc0edc: 52800000 mov w0, #0x0 // #0
> > > > > fffc0ee0: d65f03c0 ret
> > > > >
> > > > > 00000000fffc0ee4 <initr_status_led>:
> > > > > fffc0ee4: 52800000 mov w0, #0x0 // #0
> > > > > fffc0ee8: d65f03c0 ret
> > > > >
> > > > > 00000000fffc0eec <initr_boot_led_blink>:
> > > > > fffc0eec: 52800000 mov w0, #0x0 // #0
> > > > > fffc0ef0: d65f03c0 ret
> > > > >
> > > > > 00000000fffc0ef4 <initr_boot_led_on>:
> > > > > fffc0ef4: 52800000 mov w0, #0x0 // #0
> > > > > fffc0ef8: d65f03c0 ret
> > > > >
> > > > > 00000000fffc0efc <initr_lmb>:
> > > > > fffc0efc: 52800000 mov w0, #0x0 // #0
> > > > > fffc0f00: d65f03c0 ret
> > > >
> > > > No, but maybe Simon would prefer if we marked all of the could-be-empty
> > > > functions as __maybe_unused and did:
> > > > CONFIG_IS_ENABLED(BINMAN_FDT, initr_binman),
> > > > etc in the list instead?
> > >
> > > Yes that looks better.
> >
> > But we are talking about using macro inside array at best with using #ifdefs.
> > Or maybe I am not seeing what you are saying.
>
> No, nevermind, sorry. I was hoping that:
> diff --git a/common/board_r.c b/common/board_r.c
> index ff9bce88dc93..c18997446dfa 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -610,9 +610,7 @@ static init_fnc_t init_sequence_r[] = {
> noncached_init,
> #endif
> initr_of_live,
> -#ifdef CONFIG_DM
> - initr_dm,
> -#endif
> + CONFIG_IS_ENABLED(CONFIG_DM, initr_dm),
> #ifdef CONFIG_ADDR_MAP
> init_addr_map,
> #endif
> @@ -632,9 +630,7 @@ static init_fnc_t init_sequence_r[] = {
> #ifdef CONFIG_EFI_LOADER
> efi_memory_init,
> #endif
> -#ifdef CONFIG_BINMAN_FDT
> - initr_binman,
> -#endif
> + CONFIG_IS_ENABLED(CONFIG_BINMAN_FDT, initr_binman),
> #ifdef CONFIG_FSP_VERSION2
> arch_fsp_init_r,
> #endif
>
> would work, but the macro doesn't evaluate how I'd hope it did and that
> just blows up.
You should drop the CONFIG_ inside the brackets. Also, you need
something like this, since the comma must not be present unless the
option is enabled:
CONFIG_IS_ENABLED(BINMAN_FDT, (initr_binman,))
If we want to do this more generally, we could:
- convert more things to use an event (then the cost is just 4-8 bytes per item)
- add a helper to initcall.h to make it easier, e.g.
INITCALL(BINMAN_FDT, initr_binman)
Regards,
SImon
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT
2024-12-09 19:27 ` Simon Glass
@ 2024-12-09 20:08 ` Tom Rini
2024-12-10 13:43 ` Michal Simek
0 siblings, 1 reply; 28+ messages in thread
From: Tom Rini @ 2024-12-09 20:08 UTC (permalink / raw)
To: Simon Glass
Cc: Michal Simek, u-boot, git, neal.frager, Christian Marangi,
Ilias Apalodimas, Jerome Forissier, Rasmus Villemoes,
Sughosh Ganu
[-- Attachment #1: Type: text/plain, Size: 5955 bytes --]
On Mon, Dec 09, 2024 at 12:27:31PM -0700, Simon Glass wrote:
> Hi Tom,
>
> On Mon, 9 Dec 2024 at 12:23, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Dec 09, 2024 at 07:34:34PM +0100, Michal Simek wrote:
> > >
> > >
> > > On 12/9/24 16:47, Simon Glass wrote:
> > > > Hi,
> > > >
> > > > On Mon, 9 Dec 2024 at 08:32, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Mon, Dec 09, 2024 at 04:26:15PM +0100, Michal Simek wrote:
> > > > > >
> > > > > >
> > > > > > On 12/6/24 20:20, Simon Glass wrote:
> > > > > > > On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
> > > > > > > >
> > > > > > > > Calling empty function when BINMAN_FDT is adding +64B for nothing which is
> > > > > > > > not helping on size sensitive configurations as Xilinx mini configurations.
> > > > > > > >
> > > > > > > > Signed-off-by: Michal Simek <michal.simek@amd.com>
> > > > > > > > ---
> > > > > > > >
> > > > > > > > Changes in v2:
> > > > > > > > - new patch
> > > > > > > >
> > > > > > > > From my perspective there is no reason to call empty function. It is just
> > > > > > > > increase footprint for nothing and we are not far from that limit now.
> > > > > > > >
> > > > > > > > ---
> > > > > > > > common/board_r.c | 7 +++----
> > > > > > > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > > > > > >
> > > > > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > > > > >
> > > > > > > This is a bit odd, though. Do you have LTO enabled?
> > > > > > >
> > > > > >
> > > > > > yes LTO is enabled. And there are other candidates like this.
> > > > > > Is LTO able to fix function arrays which is calling empty function?
> > > > > >
> > > > > > (without this patch)
> > > > > >
> > > > > > 00000000fffc0eb4 <initr_of_live>:
> > > > > > fffc0eb4: 52800000 mov w0, #0x0 // #0
> > > > > > fffc0eb8: d65f03c0 ret
> > > > > >
> > > > > > 00000000fffc0ebc <initr_dm_devices>:
> > > > > > fffc0ebc: 52800000 mov w0, #0x0 // #0
> > > > > > fffc0ec0: d65f03c0 ret
> > > > > >
> > > > > > 00000000fffc0ec4 <initr_bootstage>:
> > > > > > fffc0ec4: 52800000 mov w0, #0x0 // #0
> > > > > > fffc0ec8: d65f03c0 ret
> > > > > >
> > > > > > 00000000fffc0ecc <power_init_board>:
> > > > > > fffc0ecc: 52800000 mov w0, #0x0 // #0
> > > > > > fffc0ed0: d65f03c0 ret
> > > > > >
> > > > > > 00000000fffc0ed4 <initr_announce>:
> > > > > > fffc0ed4: 52800000 mov w0, #0x0 // #0
> > > > > > fffc0ed8: d65f03c0 ret
> > > > > >
> > > > > > 00000000fffc0edc <initr_binman>:
> > > > > > fffc0edc: 52800000 mov w0, #0x0 // #0
> > > > > > fffc0ee0: d65f03c0 ret
> > > > > >
> > > > > > 00000000fffc0ee4 <initr_status_led>:
> > > > > > fffc0ee4: 52800000 mov w0, #0x0 // #0
> > > > > > fffc0ee8: d65f03c0 ret
> > > > > >
> > > > > > 00000000fffc0eec <initr_boot_led_blink>:
> > > > > > fffc0eec: 52800000 mov w0, #0x0 // #0
> > > > > > fffc0ef0: d65f03c0 ret
> > > > > >
> > > > > > 00000000fffc0ef4 <initr_boot_led_on>:
> > > > > > fffc0ef4: 52800000 mov w0, #0x0 // #0
> > > > > > fffc0ef8: d65f03c0 ret
> > > > > >
> > > > > > 00000000fffc0efc <initr_lmb>:
> > > > > > fffc0efc: 52800000 mov w0, #0x0 // #0
> > > > > > fffc0f00: d65f03c0 ret
> > > > >
> > > > > No, but maybe Simon would prefer if we marked all of the could-be-empty
> > > > > functions as __maybe_unused and did:
> > > > > CONFIG_IS_ENABLED(BINMAN_FDT, initr_binman),
> > > > > etc in the list instead?
> > > >
> > > > Yes that looks better.
> > >
> > > But we are talking about using macro inside array at best with using #ifdefs.
> > > Or maybe I am not seeing what you are saying.
> >
> > No, nevermind, sorry. I was hoping that:
> > diff --git a/common/board_r.c b/common/board_r.c
> > index ff9bce88dc93..c18997446dfa 100644
> > --- a/common/board_r.c
> > +++ b/common/board_r.c
> > @@ -610,9 +610,7 @@ static init_fnc_t init_sequence_r[] = {
> > noncached_init,
> > #endif
> > initr_of_live,
> > -#ifdef CONFIG_DM
> > - initr_dm,
> > -#endif
> > + CONFIG_IS_ENABLED(CONFIG_DM, initr_dm),
> > #ifdef CONFIG_ADDR_MAP
> > init_addr_map,
> > #endif
> > @@ -632,9 +630,7 @@ static init_fnc_t init_sequence_r[] = {
> > #ifdef CONFIG_EFI_LOADER
> > efi_memory_init,
> > #endif
> > -#ifdef CONFIG_BINMAN_FDT
> > - initr_binman,
> > -#endif
> > + CONFIG_IS_ENABLED(CONFIG_BINMAN_FDT, initr_binman),
> > #ifdef CONFIG_FSP_VERSION2
> > arch_fsp_init_r,
> > #endif
> >
> > would work, but the macro doesn't evaluate how I'd hope it did and that
> > just blows up.
>
> You should drop the CONFIG_ inside the brackets. Also, you need
> something like this, since the comma must not be present unless the
> option is enabled:
>
> CONFIG_IS_ENABLED(BINMAN_FDT, (initr_binman,))
>
> If we want to do this more generally, we could:
> - convert more things to use an event (then the cost is just 4-8 bytes per item)
> - add a helper to initcall.h to make it easier, e.g.
> INITCALL(BINMAN_FDT, initr_binman)
We should probably just do a follow-up cleanup to, as you note
CONFIG_IS_ENABLED(FOO, (initr_foo,))
I don't think a more complex macro in there is worth while, but saving a
few bytes overall here for free would be good. We can figure out later
the cost/benefit on moving stuff here to events.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT
2024-12-09 19:27 ` Simon Glass
@ 2024-12-10 12:40 ` Michal Simek
2024-12-10 16:17 ` Simon Glass
0 siblings, 1 reply; 28+ messages in thread
From: Michal Simek @ 2024-12-10 12:40 UTC (permalink / raw)
To: Simon Glass, Jerome Forissier
Cc: Tom Rini, u-boot, git, neal.frager, Christian Marangi,
Ilias Apalodimas, Jerome Forissier, Rasmus Villemoes,
Sughosh Ganu
Hi Simon,
On 12/9/24 20:27, Simon Glass wrote:
> Hi Michal,
>
> On Mon, 9 Dec 2024 at 11:34, Michal Simek <michal.simek@amd.com> wrote:
>>
>>
>>
>> On 12/9/24 16:47, Simon Glass wrote:
>>> Hi,
>>>
>>> On Mon, 9 Dec 2024 at 08:32, Tom Rini <trini@konsulko.com> wrote:
>>>>
>>>> On Mon, Dec 09, 2024 at 04:26:15PM +0100, Michal Simek wrote:
>>>>>
>>>>>
>>>>> On 12/6/24 20:20, Simon Glass wrote:
>>>>>> On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
>>>>>>>
>>>>>>> Calling empty function when BINMAN_FDT is adding +64B for nothing which is
>>>>>>> not helping on size sensitive configurations as Xilinx mini configurations.
>>>>>>>
>>>>>>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>>>>>>> ---
>>>>>>>
>>>>>>> Changes in v2:
>>>>>>> - new patch
>>>>>>>
>>>>>>> From my perspective there is no reason to call empty function. It is just
>>>>>>> increase footprint for nothing and we are not far from that limit now.
>>>>>>>
>>>>>>> ---
>>>>>>> common/board_r.c | 7 +++----
>>>>>>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>>>>
>>>>>> This is a bit odd, though. Do you have LTO enabled?
>>>>>>
>>>>>
>>>>> yes LTO is enabled. And there are other candidates like this.
>>>>> Is LTO able to fix function arrays which is calling empty function?
>>>>>
>>>>> (without this patch)
>>>>>
>>>>> 00000000fffc0eb4 <initr_of_live>:
>>>>> fffc0eb4: 52800000 mov w0, #0x0 // #0
>>>>> fffc0eb8: d65f03c0 ret
>>>>>
>>>>> 00000000fffc0ebc <initr_dm_devices>:
>>>>> fffc0ebc: 52800000 mov w0, #0x0 // #0
>>>>> fffc0ec0: d65f03c0 ret
>>>>>
>>>>> 00000000fffc0ec4 <initr_bootstage>:
>>>>> fffc0ec4: 52800000 mov w0, #0x0 // #0
>>>>> fffc0ec8: d65f03c0 ret
>>>>>
>>>>> 00000000fffc0ecc <power_init_board>:
>>>>> fffc0ecc: 52800000 mov w0, #0x0 // #0
>>>>> fffc0ed0: d65f03c0 ret
>>>>>
>>>>> 00000000fffc0ed4 <initr_announce>:
>>>>> fffc0ed4: 52800000 mov w0, #0x0 // #0
>>>>> fffc0ed8: d65f03c0 ret
>>>>>
>>>>> 00000000fffc0edc <initr_binman>:
>>>>> fffc0edc: 52800000 mov w0, #0x0 // #0
>>>>> fffc0ee0: d65f03c0 ret
>>>>>
>>>>> 00000000fffc0ee4 <initr_status_led>:
>>>>> fffc0ee4: 52800000 mov w0, #0x0 // #0
>>>>> fffc0ee8: d65f03c0 ret
>>>>>
>>>>> 00000000fffc0eec <initr_boot_led_blink>:
>>>>> fffc0eec: 52800000 mov w0, #0x0 // #0
>>>>> fffc0ef0: d65f03c0 ret
>>>>>
>>>>> 00000000fffc0ef4 <initr_boot_led_on>:
>>>>> fffc0ef4: 52800000 mov w0, #0x0 // #0
>>>>> fffc0ef8: d65f03c0 ret
>>>>>
>>>>> 00000000fffc0efc <initr_lmb>:
>>>>> fffc0efc: 52800000 mov w0, #0x0 // #0
>>>>> fffc0f00: d65f03c0 ret
>>>>
>>>> No, but maybe Simon would prefer if we marked all of the could-be-empty
>>>> functions as __maybe_unused and did:
>>>> CONFIG_IS_ENABLED(BINMAN_FDT, initr_binman),
>>>> etc in the list instead?
>>>
>>> Yes that looks better.
>>
>> But we are talking about using macro inside array at best with using #ifdefs.
>> Or maybe I am not seeing what you are saying.
>>
>>>
>>> Michal, see also [1] in case you can work out why it 'stopped
>>> working'. I could have sworn inlining the function was a win when it
>>> was applied, but no amount of toolchain juggling could make it be a
>>> win when I came back to it later.
>>
>> Are you saying that it worked in past?
>
> I wasn't able to verify that post facto, but I believe I do remember
> checking it at the time. If you read the original commit message:
>
> 47870afab92 initcall: Move to inline function
>
> The board_r init function was complaining that we are looping through
> an array, calling all our tiny init stubs sequentially via indirect
> function calls (which can't be speculated, so they are slow).
>
> The solution to that is pretty easy though. All we need to do is inline
> the function that loops through the functions and the compiler will
> automatically convert almost all indirect calls into direct inlined code.
>
> With this patch, the overall code size drops (by 40 bytes on riscv64)
> and boot time should become measurably faster for every target.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
>
> Despite this hopeful sentiment, I seriously doubt any improvement in boot time.
I am not able to replicate this observation on arm64 or riscv64.
Loop unrolling is not happening even if you pass -funroll-all-loops flag.
Maybe different toolchains should be used to see this behavior.
Thanks,
Michal
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT
2024-12-09 20:08 ` Tom Rini
@ 2024-12-10 13:43 ` Michal Simek
0 siblings, 0 replies; 28+ messages in thread
From: Michal Simek @ 2024-12-10 13:43 UTC (permalink / raw)
To: Tom Rini, Simon Glass
Cc: u-boot, git, neal.frager, Christian Marangi, Ilias Apalodimas,
Jerome Forissier, Rasmus Villemoes, Sughosh Ganu
Hi,
On 12/9/24 21:08, Tom Rini wrote:
> On Mon, Dec 09, 2024 at 12:27:31PM -0700, Simon Glass wrote:
>> Hi Tom,
>>
>> On Mon, 9 Dec 2024 at 12:23, Tom Rini <trini@konsulko.com> wrote:
>>>
>>> On Mon, Dec 09, 2024 at 07:34:34PM +0100, Michal Simek wrote:
>>>>
>>>>
>>>> On 12/9/24 16:47, Simon Glass wrote:
>>>>> Hi,
>>>>>
>>>>> On Mon, 9 Dec 2024 at 08:32, Tom Rini <trini@konsulko.com> wrote:
>>>>>>
>>>>>> On Mon, Dec 09, 2024 at 04:26:15PM +0100, Michal Simek wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 12/6/24 20:20, Simon Glass wrote:
>>>>>>>> On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
>>>>>>>>>
>>>>>>>>> Calling empty function when BINMAN_FDT is adding +64B for nothing which is
>>>>>>>>> not helping on size sensitive configurations as Xilinx mini configurations.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> Changes in v2:
>>>>>>>>> - new patch
>>>>>>>>>
>>>>>>>>> From my perspective there is no reason to call empty function. It is just
>>>>>>>>> increase footprint for nothing and we are not far from that limit now.
>>>>>>>>>
>>>>>>>>> ---
>>>>>>>>> common/board_r.c | 7 +++----
>>>>>>>>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>>>>>>>
>>>>>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>>>>>>
>>>>>>>> This is a bit odd, though. Do you have LTO enabled?
>>>>>>>>
>>>>>>>
>>>>>>> yes LTO is enabled. And there are other candidates like this.
>>>>>>> Is LTO able to fix function arrays which is calling empty function?
>>>>>>>
>>>>>>> (without this patch)
>>>>>>>
>>>>>>> 00000000fffc0eb4 <initr_of_live>:
>>>>>>> fffc0eb4: 52800000 mov w0, #0x0 // #0
>>>>>>> fffc0eb8: d65f03c0 ret
>>>>>>>
>>>>>>> 00000000fffc0ebc <initr_dm_devices>:
>>>>>>> fffc0ebc: 52800000 mov w0, #0x0 // #0
>>>>>>> fffc0ec0: d65f03c0 ret
>>>>>>>
>>>>>>> 00000000fffc0ec4 <initr_bootstage>:
>>>>>>> fffc0ec4: 52800000 mov w0, #0x0 // #0
>>>>>>> fffc0ec8: d65f03c0 ret
>>>>>>>
>>>>>>> 00000000fffc0ecc <power_init_board>:
>>>>>>> fffc0ecc: 52800000 mov w0, #0x0 // #0
>>>>>>> fffc0ed0: d65f03c0 ret
>>>>>>>
>>>>>>> 00000000fffc0ed4 <initr_announce>:
>>>>>>> fffc0ed4: 52800000 mov w0, #0x0 // #0
>>>>>>> fffc0ed8: d65f03c0 ret
>>>>>>>
>>>>>>> 00000000fffc0edc <initr_binman>:
>>>>>>> fffc0edc: 52800000 mov w0, #0x0 // #0
>>>>>>> fffc0ee0: d65f03c0 ret
>>>>>>>
>>>>>>> 00000000fffc0ee4 <initr_status_led>:
>>>>>>> fffc0ee4: 52800000 mov w0, #0x0 // #0
>>>>>>> fffc0ee8: d65f03c0 ret
>>>>>>>
>>>>>>> 00000000fffc0eec <initr_boot_led_blink>:
>>>>>>> fffc0eec: 52800000 mov w0, #0x0 // #0
>>>>>>> fffc0ef0: d65f03c0 ret
>>>>>>>
>>>>>>> 00000000fffc0ef4 <initr_boot_led_on>:
>>>>>>> fffc0ef4: 52800000 mov w0, #0x0 // #0
>>>>>>> fffc0ef8: d65f03c0 ret
>>>>>>>
>>>>>>> 00000000fffc0efc <initr_lmb>:
>>>>>>> fffc0efc: 52800000 mov w0, #0x0 // #0
>>>>>>> fffc0f00: d65f03c0 ret
>>>>>>
>>>>>> No, but maybe Simon would prefer if we marked all of the could-be-empty
>>>>>> functions as __maybe_unused and did:
>>>>>> CONFIG_IS_ENABLED(BINMAN_FDT, initr_binman),
>>>>>> etc in the list instead?
>>>>>
>>>>> Yes that looks better.
>>>>
>>>> But we are talking about using macro inside array at best with using #ifdefs.
>>>> Or maybe I am not seeing what you are saying.
>>>
>>> No, nevermind, sorry. I was hoping that:
>>> diff --git a/common/board_r.c b/common/board_r.c
>>> index ff9bce88dc93..c18997446dfa 100644
>>> --- a/common/board_r.c
>>> +++ b/common/board_r.c
>>> @@ -610,9 +610,7 @@ static init_fnc_t init_sequence_r[] = {
>>> noncached_init,
>>> #endif
>>> initr_of_live,
>>> -#ifdef CONFIG_DM
>>> - initr_dm,
>>> -#endif
>>> + CONFIG_IS_ENABLED(CONFIG_DM, initr_dm),
>>> #ifdef CONFIG_ADDR_MAP
>>> init_addr_map,
>>> #endif
>>> @@ -632,9 +630,7 @@ static init_fnc_t init_sequence_r[] = {
>>> #ifdef CONFIG_EFI_LOADER
>>> efi_memory_init,
>>> #endif
>>> -#ifdef CONFIG_BINMAN_FDT
>>> - initr_binman,
>>> -#endif
>>> + CONFIG_IS_ENABLED(CONFIG_BINMAN_FDT, initr_binman),
>>> #ifdef CONFIG_FSP_VERSION2
>>> arch_fsp_init_r,
>>> #endif
>>>
>>> would work, but the macro doesn't evaluate how I'd hope it did and that
>>> just blows up.
>>
>> You should drop the CONFIG_ inside the brackets. Also, you need
>> something like this, since the comma must not be present unless the
>> option is enabled:
>>
>> CONFIG_IS_ENABLED(BINMAN_FDT, (initr_binman,))
>>
>> If we want to do this more generally, we could:
>> - convert more things to use an event (then the cost is just 4-8 bytes per item)
>> - add a helper to initcall.h to make it easier, e.g.
>> INITCALL(BINMAN_FDT, initr_binman)
>
> We should probably just do a follow-up cleanup to, as you note
> CONFIG_IS_ENABLED(FOO, (initr_foo,))
>
> I don't think a more complex macro in there is worth while, but saving a
> few bytes overall here for free would be good. We can figure out later
> the cost/benefit on moving stuff here to events.
I have sent RFC with this here.
https://lore.kernel.org/r/9786c6124c959ca230dd2c23e8da794680f09867.1733837980.git.michal.simek@amd.com
There are likely some other steps should be happen on the top of this because
there are some entries which depends on more symbols. Also some macros are not
converted yet to CONFIG_ (CFG_) and there are functions which don't setup any
dependency but should be there.
But likely this should be done with steps.
Thanks,
Michal
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT
2024-12-10 12:40 ` Michal Simek
@ 2024-12-10 16:17 ` Simon Glass
0 siblings, 0 replies; 28+ messages in thread
From: Simon Glass @ 2024-12-10 16:17 UTC (permalink / raw)
To: Michal Simek
Cc: Jerome Forissier, Tom Rini, u-boot, git, neal.frager,
Christian Marangi, Ilias Apalodimas, Rasmus Villemoes,
Sughosh Ganu
Hi Michal,
On Tue, 10 Dec 2024 at 05:41, Michal Simek <michal.simek@amd.com> wrote:
>
> Hi Simon,
>
> On 12/9/24 20:27, Simon Glass wrote:
> > Hi Michal,
> >
> > On Mon, 9 Dec 2024 at 11:34, Michal Simek <michal.simek@amd.com> wrote:
> >>
> >>
> >>
> >> On 12/9/24 16:47, Simon Glass wrote:
> >>> Hi,
> >>>
> >>> On Mon, 9 Dec 2024 at 08:32, Tom Rini <trini@konsulko.com> wrote:
> >>>>
> >>>> On Mon, Dec 09, 2024 at 04:26:15PM +0100, Michal Simek wrote:
> >>>>>
> >>>>>
> >>>>> On 12/6/24 20:20, Simon Glass wrote:
> >>>>>> On Fri, 1 Nov 2024 at 03:18, Michal Simek <michal.simek@amd.com> wrote:
> >>>>>>>
> >>>>>>> Calling empty function when BINMAN_FDT is adding +64B for nothing which is
> >>>>>>> not helping on size sensitive configurations as Xilinx mini configurations.
> >>>>>>>
> >>>>>>> Signed-off-by: Michal Simek <michal.simek@amd.com>
> >>>>>>> ---
> >>>>>>>
> >>>>>>> Changes in v2:
> >>>>>>> - new patch
> >>>>>>>
> >>>>>>> From my perspective there is no reason to call empty function. It is just
> >>>>>>> increase footprint for nothing and we are not far from that limit now.
> >>>>>>>
> >>>>>>> ---
> >>>>>>> common/board_r.c | 7 +++----
> >>>>>>> 1 file changed, 3 insertions(+), 4 deletions(-)
> >>>>>>
> >>>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
> >>>>>>
> >>>>>> This is a bit odd, though. Do you have LTO enabled?
> >>>>>>
> >>>>>
> >>>>> yes LTO is enabled. And there are other candidates like this.
> >>>>> Is LTO able to fix function arrays which is calling empty function?
> >>>>>
> >>>>> (without this patch)
> >>>>>
> >>>>> 00000000fffc0eb4 <initr_of_live>:
> >>>>> fffc0eb4: 52800000 mov w0, #0x0 // #0
> >>>>> fffc0eb8: d65f03c0 ret
> >>>>>
> >>>>> 00000000fffc0ebc <initr_dm_devices>:
> >>>>> fffc0ebc: 52800000 mov w0, #0x0 // #0
> >>>>> fffc0ec0: d65f03c0 ret
> >>>>>
> >>>>> 00000000fffc0ec4 <initr_bootstage>:
> >>>>> fffc0ec4: 52800000 mov w0, #0x0 // #0
> >>>>> fffc0ec8: d65f03c0 ret
> >>>>>
> >>>>> 00000000fffc0ecc <power_init_board>:
> >>>>> fffc0ecc: 52800000 mov w0, #0x0 // #0
> >>>>> fffc0ed0: d65f03c0 ret
> >>>>>
> >>>>> 00000000fffc0ed4 <initr_announce>:
> >>>>> fffc0ed4: 52800000 mov w0, #0x0 // #0
> >>>>> fffc0ed8: d65f03c0 ret
> >>>>>
> >>>>> 00000000fffc0edc <initr_binman>:
> >>>>> fffc0edc: 52800000 mov w0, #0x0 // #0
> >>>>> fffc0ee0: d65f03c0 ret
> >>>>>
> >>>>> 00000000fffc0ee4 <initr_status_led>:
> >>>>> fffc0ee4: 52800000 mov w0, #0x0 // #0
> >>>>> fffc0ee8: d65f03c0 ret
> >>>>>
> >>>>> 00000000fffc0eec <initr_boot_led_blink>:
> >>>>> fffc0eec: 52800000 mov w0, #0x0 // #0
> >>>>> fffc0ef0: d65f03c0 ret
> >>>>>
> >>>>> 00000000fffc0ef4 <initr_boot_led_on>:
> >>>>> fffc0ef4: 52800000 mov w0, #0x0 // #0
> >>>>> fffc0ef8: d65f03c0 ret
> >>>>>
> >>>>> 00000000fffc0efc <initr_lmb>:
> >>>>> fffc0efc: 52800000 mov w0, #0x0 // #0
> >>>>> fffc0f00: d65f03c0 ret
> >>>>
> >>>> No, but maybe Simon would prefer if we marked all of the could-be-empty
> >>>> functions as __maybe_unused and did:
> >>>> CONFIG_IS_ENABLED(BINMAN_FDT, initr_binman),
> >>>> etc in the list instead?
> >>>
> >>> Yes that looks better.
> >>
> >> But we are talking about using macro inside array at best with using #ifdefs.
> >> Or maybe I am not seeing what you are saying.
> >>
> >>>
> >>> Michal, see also [1] in case you can work out why it 'stopped
> >>> working'. I could have sworn inlining the function was a win when it
> >>> was applied, but no amount of toolchain juggling could make it be a
> >>> win when I came back to it later.
> >>
> >> Are you saying that it worked in past?
> >
> > I wasn't able to verify that post facto, but I believe I do remember
> > checking it at the time. If you read the original commit message:
> >
> > 47870afab92 initcall: Move to inline function
> >
> > The board_r init function was complaining that we are looping through
> > an array, calling all our tiny init stubs sequentially via indirect
> > function calls (which can't be speculated, so they are slow).
> >
> > The solution to that is pretty easy though. All we need to do is inline
> > the function that loops through the functions and the compiler will
> > automatically convert almost all indirect calls into direct inlined code.
> >
> > With this patch, the overall code size drops (by 40 bytes on riscv64)
> > and boot time should become measurably faster for every target.
> >
> > Signed-off-by: Alexander Graf <agraf@suse.de>
> >
> > Despite this hopeful sentiment, I seriously doubt any improvement in boot time.
>
> I am not able to replicate this observation on arm64 or riscv64.
>
> Loop unrolling is not happening even if you pass -funroll-all-loops flag.
>
> Maybe different toolchains should be used to see this behavior.
Yes, maybe. Or perhaps it changed for some reason.
Regards,
Simon
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2024-12-10 16:17 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-01 9:17 [PATCH v2 0/7] arm64: zynqmp: Convert platforms to use binman Michal Simek
2024-11-01 9:17 ` [PATCH v2 1/7] binman: Add option for pointing to separate description Michal Simek
2024-12-06 19:17 ` Simon Glass
2024-11-01 9:17 ` [PATCH v2 2/7] common: binman: Calling initr_binman() when BINMAN_FDT Michal Simek
2024-12-06 19:20 ` Simon Glass
2024-12-09 15:26 ` Michal Simek
2024-12-09 15:32 ` Tom Rini
2024-12-09 15:47 ` Simon Glass
2024-12-09 18:34 ` Michal Simek
2024-12-09 19:23 ` Tom Rini
2024-12-09 19:27 ` Simon Glass
2024-12-09 20:08 ` Tom Rini
2024-12-10 13:43 ` Michal Simek
2024-12-09 19:27 ` Simon Glass
2024-12-10 12:40 ` Michal Simek
2024-12-10 16:17 ` Simon Glass
2024-11-01 9:17 ` [PATCH v2 3/7] arm64: zynqmp: Describe empty binman node Michal Simek
2024-12-06 19:19 ` Simon Glass
2024-11-01 9:17 ` [PATCH v2 4/7] arm64: zynqmp: Add binman description for SOM Michal Simek
2024-12-06 19:19 ` Simon Glass
2024-11-01 9:17 ` [PATCH v2 5/7] arm64: zynqmp: Generate u-boot.itb and QSPI image via binman Michal Simek
2024-12-06 19:20 ` Simon Glass
2024-11-01 9:17 ` [PATCH v2 6/7] arm64: zynqmp: Remove mkimage fit script Michal Simek
2024-12-06 19:20 ` Simon Glass
2024-11-01 9:18 ` [PATCH v2 7/7] Makefile: Drop SPL_FIT_GENERATOR support Michal Simek
2024-12-06 19:16 ` Simon Glass
2024-11-11 11:46 ` [PATCH v2 0/7] arm64: zynqmp: Convert platforms to use binman Michal Simek
2024-11-27 7:59 ` Michal Simek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox