Replace make-fit-atf.py with binman. Was: migrate u-boot-rockchip.bin to binman and generate an image for SPI

Quentin Schulz quentin.schulz at theobroma-systems.com
Tue Jul 26 11:08:21 CEST 2022


Hi Xavier,

On 7/25/22 19:33, Xavier Drudis Ferran wrote:
> El Mon, Jul 25, 2022 at 07:29:53PM +0200, Xavier Drudis Ferran deia:
>>
>> I copy here the rockchip-u-boot.dtsi file and then 2 patches on top of yours.
>>
> 
> Sorry I copied a dirty version that din't work. The patches were correct, the dtsi wasn't.
>   
>> #else
>> 		collection {
>> 			content = <&/binman/itb>;
> 
> It doesn't like this phandles nor just &itb.
> 
> These patches were the real thing. Sorry for the noise.
> 
> 
>>
>> >From 0f9cf7452a62268ec5978c80f46bf9323a269630 Mon Sep 17 00:00:00 2001
>> From: Xavier Drudis Ferran <xdrudis at tinet.cat>
>> Date: Mon, 25 Jul 2022 17:35:27 +0200
>> Subject: [PATCH] Align the fit images that binman produces to 8 bytes.
>>
>> In commit 570c4636808 ("Makefile: Align fit-dtb.blob and u-boot.itb by
>> 64bits") Michal Simek claims that according to the device tree spec,
>> some things should be aligned to 4 bytes and some to 8, so passes -B 8
>> to mkimage. Do the same when using binman so that we can try to
>> replace make_fit_atf.py .
>>
>> Should this be optional? I haven't found any uses of split-elf that I
>> might be breaking, and Marek said it's from dt spec, so it should be
>> always required. So I'm hard coding it until the need for being
>> optional arises.

Makes sense to me looking at the current Makefile implementation for 
u-boot.itb mkimage flags.

You could also have a fit,align = <8>; property instead of hardcoding 
it. The issue is that this flag seems to be added only for u-boot.itb 
and fit-dtb.blob. I assume there are usecases outside of those two 
binaries where the user does not want the fit header to be aligned (or 
don't need it).

>> ---
>>   tools/binman/btool/mkimage.py | 4 +++-
>>   tools/binman/etype/fit.py     | 1 +
>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/binman/btool/mkimage.py b/tools/binman/btool/mkimage.py
>> index c85bfe053c..d614d72d62 100644
>> --- a/tools/binman/btool/mkimage.py
>> +++ b/tools/binman/btool/mkimage.py
>> @@ -22,7 +22,7 @@ class Bintoolmkimage(bintool.Bintool):
>>   
>>       # pylint: disable=R0913
>>       def run(self, reset_timestamp=False, output_fname=None, external=False,
>> -            pad=None, version=False):
>> +            pad=None, version=False, align=None):
>>           """Run mkimage
>>   
>>           Args:
>> @@ -36,6 +36,8 @@ class Bintoolmkimage(bintool.Bintool):
>>               version: True to get the mkimage version
>>           """
>>           args = []
>> +        if align:
>> +            args += ['-B', f'{align:x}']
>>           if external:
>>               args.append('-E')
>>           if pad:
>> diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
>> index 12306623af..7b99b83fa3 100644
>> --- a/tools/binman/etype/fit.py
>> +++ b/tools/binman/etype/fit.py
>> @@ -420,6 +420,7 @@ class Entry_fit(Entry_section):
>>           ext_offset = self._fit_props.get('fit,external-offset')
>>           if ext_offset is not None:
>>               args = {
>> +                'align': 8,
>>                   'external': True,
>>                   'pad': fdt_util.fdt32_to_cpu(ext_offset.value)
>>                   }
>> -- 
>> 2.20.1
>>
>> >From 9fc65a2eb55f742dd805ed96e3d2028b20078a03 Mon Sep 17 00:00:00 2001
>> From: Xavier Drudis Ferran <xdrudis at tinet.cat>
>> Date: Mon, 25 Jul 2022 18:01:24 +0200
>> Subject: [PATCH] Replace make_fit_atf.py with binman.
>>
>> This boots my Rock Pi 4B, but likely needs more generalisation to
>> cover more boards and configurations.
>> ---
>>   Makefile                           |  3 --
>>   arch/arm/dts/rockchip-u-boot.dtsi  | 70 ++++++++++++++++++++++++++++++
>>   configs/rock-pi-4-rk3399_defconfig |  1 +
>>   3 files changed, 71 insertions(+), 3 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 279aeacee3..ad739ef357 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -997,9 +997,6 @@ endif
>>   
>>   ifeq ($(CONFIG_ARCH_ROCKCHIP)$(CONFIG_SPL),yy)
>>   # Binman image dependencies
>> -ifeq ($(CONFIG_ARM64),y)
>> -INPUTS-y += u-boot.itb
>> -endif
>>   else
>>   INPUTS-y += u-boot.img
>>   endif
>> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
>> index 4c26caa92a..5a613650f5 100644
>> --- a/arch/arm/dts/rockchip-u-boot.dtsi
>> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
>> @@ -13,6 +13,75 @@
>>   
>>   #ifdef CONFIG_SPL
>>   &binman {
>> +	itb {
>> +		filename = "u-boot.itb";
>> +		fit {
>> +			filename = "u-boot.itb";
>> +			description = "U-Boot FIT";
>> +			fit,fdt-list = "of-list";
>> +			fit,external-offset=<0>;
>> +
>> +			images {
>> +			        uboot {
>> +					description = "U-Boot (64-bit)";
>> +					type = "standalone";
>> +					os = "U-Boot";
>> +					arch = "arm64";
>> +					compression = "none";
>> +					load = <CONFIG_SYS_TEXT_BASE>;
>> +					u-boot-nodtb {
>> +					};
>> +				};
>> +#ifdef CONFIG_SPL_ATF
>> +			        @atf_SEQ {
>> +					fit,operation = "split-elf";
>> +					description = "ARM Trusted Firmware";
>> +					type = "firmware";
>> +					arch = "arm64";
>> +					os = "arm-trusted-firmware";
>> +					compression = "none";
>> +					fit,load;
>> +					fit,entry;
>> +					fit,data;
>> +
>> +					atf-bl31 {
>> +					};
>> +			        };
>> +#endif
>> +#ifdef CONFIG_TEE
>> +			        @tee_SEQ {
>> +					fit,operation = "split-elf";
>> +					description = "TEE";
>> +					type = "tee";
>> +					arch = "arm64";
>> +					os = "tee";
>> +					compression = "none";
>> +					fit,load;
>> +					fit,entry;
>> +					fit,data;
>> +
>> +					tee-os {
>> +					};
>> +			        };
>> +#endif
>> +			        @fdt_SEQ {
>> +					description = "NAME.dtb";
>> +					type = "flat_dt";
>> +					compression = "none";
>> +				};
>> +			};
>> +			configurations {
>> +				default = "@config_DEFAULT-SEQ";
>> +
>> +				@config_SEQ {
>> +					description = "NAME.dtb";
>> +					fdt = "fdt_SEQ";
>> +					firmware = "atf_1";
>> +					loadables = "uboot","atf_2","atf_3";

This section will need some more love with some ifdef for ATF_SPL and TEE.

>> +				};
>> +			};
>> +		};
>> +	};
>>   	simple-bin {
>>   		filename = "u-boot-rockchip.bin";
>>   		pad-byte = <0xff>;
>> @@ -62,6 +131,7 @@
>>   #ifdef CONFIG_ARM64
>>   		blob {
>>   			filename = "u-boot.itb";
>> +

This is unfortunately not possible since binman parallelizes the build 
of images in the binman DT node. This means there is no guarantee the 
u-boot.itb file is generated before this section is worked on by binman. 
Or maybe I misunderstood the docs.

But good progress, I guess this phandle thing "just" needs to be fixed 
to have something nice (both for this patch series and mine).

Cheers,
Quentin

>>   #else
>>   		u-boot-img {
>>   #endif
>> diff --git a/configs/rock-pi-4-rk3399_defconfig b/configs/rock-pi-4-rk3399_defconfig
>> index f12cf24cb4..1c07114528 100644
>> --- a/configs/rock-pi-4-rk3399_defconfig
>> +++ b/configs/rock-pi-4-rk3399_defconfig
>> @@ -22,6 +22,7 @@ CONFIG_DEBUG_UART=y
>>   CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
>>   CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x3000000
>>   CONFIG_TPL_SYS_MALLOC_F_LEN=0x4000
>> +# CONFIG_USE_SPL_FIT_GENERATOR is not set
>>   CONFIG_SYS_LOADADDR_ALIGN_DOWN_BITS=16
>>   CONFIG_BOOTSTAGE=y
>>   CONFIG_SPL_BOOTSTAGE=y
>> -- 
>> 2.20.1
>>


More information about the U-Boot mailing list