Hi Quentin, On 2025-02-11 12:43, Quentin Schulz wrote: > Hi Jonas, > > On 2/11/25 12:35 PM, Jonas Karlman wrote: >> Hi Quentin, >> >> On 2025-02-11 11:29, Quentin Schulz wrote: >>> From: Quentin Schulz <quentin.sch...@cherry.de> >>> >>> Essentially the only differences between u-boot-rockchip.bin and >>> u-boot-rockchip-spi.bin are the formatting of idbloader.img which is >>> handled by mkimage (via -T rkspi/rksd) and the offset at which U-Boot >>> proper is flashed, the content of the binaries are identical otherwise. >>> >>> This fixes some issues[1] where binman tries to find the symbols defined >>> in the proper binary to install them in an xPL binary. However, because >>> we use the binary for proper (on Aarch64) generated in simple-bin image >>> node and not simple-bin-spi image node, binman doesn't have access to >>> that symbol anymore. Therefore, let's depend entirely on binaries built >>> by simple-bin in simple-bin-spi so those issues do not arise anymore as >>> nothing is compiled essentially, just assembled. >>> >>> [1] https://lore.kernel.org/u-boot/20250129132529.807031-3-na...@radxa.com/ >>> Signed-off-by: Quentin Schulz <quentin.sch...@cherry.de> >>> --- >>> arch/arm/dts/rockchip-u-boot.dtsi | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi >>> b/arch/arm/dts/rockchip-u-boot.dtsi >>> index >>> c8c928c7e5089db3a2239f2564e6dee1d82aad95..2e8a3bd09e49ed73a8cea3fadfb6d2b592082365 >>> 100644 >>> --- a/arch/arm/dts/rockchip-u-boot.dtsi >>> +++ b/arch/arm/dts/rockchip-u-boot.dtsi >>> @@ -187,18 +187,28 @@ >>> }; >>> #elif defined(CONFIG_TPL) >>> u-boot-tpl { >>> + type = "blob"; >>> + /* sync with >>> /binman/simple-bin/mkimage/u-boot-tpl */ >>> + filename = "tpl/u-boot-tpl.bin"; >>> }; >>> #endif >>> u-boot-spl { >>> + type = "blob"; >>> + /* sync with >>> /binman/simple-bin/mkimage/u-boot-spl */ >>> + filename = "spl/u-boot-spl.bin"; >>> }; >>> }; >>> >>> #if defined(CONFIG_ARM64) || defined(CONFIG_SPL_OPTEE_IMAGE) >>> fit { >>> type = "blob"; >>> + /* sync with /binman/simple-bin/fit */ >>> filename = "u-boot.itb"; >>> #else >>> u-boot-img { >>> + type = "blob"; >>> + /* sync with /binman/simple-bin/u-boot-img */ >>> + filename = "u-boot.img"; >>> #endif >>> /* Sync with u-boot,spl-payload-offset if present */ >>> offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>; >>> >> >> I think changing to using a template may suit binman better, however it >> will be at the expense of having to re-generate the FIT, not sure what >> option is best. >> > > Unnecessarily rebuilding images is not great for build times :) If I'm > not mistaken, binman still compiles sequentially so doubly not great. > > At the same time, this is relying on undefined behavior (according to > binman spec; i.e. using a binary from another image node) but we've been > doing this for a few years now and it seems to be doing ok so far? > >> It would be great if Simon can split out the rockchip-u-boot.dtsi >> improvement commits in a separate series as I suggested in [2]. >> > > That's fine but we need something soon, this is already preventing > people from contributing stuff for their boards. I don't mind either > (haven't checked the template stuff so far though), just no need to wait > too long for the perfect implementation. Also this can be merged as an > intermediary fix and templating done afterwards.
The fit-template patch (and others) was extracted and are included in the "rockchip: binman: Use a template for FIT and other improvements" v4 series, see [3]. I suggest we skip this last patch in this series, and instead apply the change to use a template from that series. If not, a v5 of that series would just end up revering the changes made in this patch any way. The added time for building two (or three with ram boot) times seem to be negligible on my build machine. [3] https://patchwork.ozlabs.org/cover/2066701/ Regards, Jonas > > Cheers, > Quentin
