Re: [PATCH RFC v2 11/11] ti: dtsi: j721e: Use binman to package tispl.bin

[Neha Malcom Francis <n-francis@ti.com>]

Hi Roger,

On 31/05/22 16:32, Roger Quadros wrote:
> 
> 
> On 06/05/2022 07:37, Neha Malcom Francis wrote:
>> Explicit make commands were earlier used to generate tispl.bin image,
>> now it is replaced using binman.
>>
>> Binman picks up and packages entries according to the description of
>> entries given in the binman node in the device tree. The make commands
>> that were earlier responsible for generating tispl.bin has been removed.
>>
>> k3-j721e-a72-binman.dtsi has been introduced for A72 specific binman node.
>> It can be included in files that require it like
>> k3-j721e-common-proc-board-u-boot.dtsi.
>>
>> Note that make commands for secure devices has also been removed as
>> focus is on general purpose devices at present time.
>>
>> Signed-off-by: Tarun Sahu <t-sahu@ti.com>
>> [n-francis@ti.com: prepared patch for upstreaming]
>> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
>> ---
>>   arch/arm/dts/k3-j721e-a72-binman.dtsi         | 86 +++++++++++++++++++
>>   .../k3-j721e-common-proc-board-u-boot.dtsi    |  1 +
>>   arch/arm/mach-k3/config.mk                    | 33 -------
>>   board/ti/j721e/Kconfig                        |  1 +
>>   scripts/Makefile.spl                          |  4 -
>>   5 files changed, 88 insertions(+), 37 deletions(-)
>>   create mode 100644 arch/arm/dts/k3-j721e-a72-binman.dtsi
>>
>> diff --git a/arch/arm/dts/k3-j721e-a72-binman.dtsi b/arch/arm/dts/k3-j721e-a72-binman.dtsi
>> new file mode 100644
>> index 0000000000..beb3424bb9
>> --- /dev/null
>> +++ b/arch/arm/dts/k3-j721e-a72-binman.dtsi
>> @@ -0,0 +1,86 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +// Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/
>> +
>> +#include <config.h>
>> +
>> +#ifdef CONFIG_ARM64
>> +/ {
>> +	binman: binman {
>> +		multiple-images;
>> +	};
>> +};
>> +
>> +&binman {
>> +	tispl {
>> +		filename = "tispl.bin";
>> +		fit {
>> +			description = "FIT IMAGE";
>> +			#address-cells = <1>;
>> +			images {
>> +				atf {
>> +					description = "ARM Trusted Firmware";
>> +					type = "firmware";
>> +					arch = "arm64";
>> +					compression = "none";
>> +					os = "arm-trusted-firmware";
>> +					load = <CONFIG_K3_ATF_LOAD_ADDR>;
>> +					entry = <CONFIG_K3_ATF_LOAD_ADDR>;
>> +					atf-bl31 {
>> +					};
>> +				};
>> +				tee {
>> +					description = "OPTEE";
>> +					type = "tee";
>> +					arch = "arm64";
>> +					compression = "none";
>> +					os = "tee";
>> +					load = <0x9e800000>;
>> +					entry = <0x9e800000>;
>> +					tee-os {
>> +					};
>> +				};
>> +				dm {
>> +					description = "DM binary";
>> +					type = "firmware";
>> +					arch = "arm32";
>> +					compression = "none";
>> +					os = "DM";
>> +					load = <0x89000000>;
>> +					entry = <0x89000000>;
>> +					ti-dm {
>> +					};
>> +				};
>> +				spl {
>> +					description = "SPL (64-bit)";
>> +					type = "standalone";
>> +					os = "U-Boot";
>> +					arch = "arm64";
>> +					compression = "none";
>> +					load = <CONFIG_SPL_TEXT_BASE>;
>> +					entry = <CONFIG_SPL_TEXT_BASE>;
>> +					u-boot-spl-nodtb {
>> +					};
>> +				};
>> +				k3-j721e-common-proc-board.dtb {
>> +					description = "k3-j721e-common-proc-board";
>> +					type = "flat_dt";
>> +					arch = "arm";
>> +					compression = "none";
>> +					blob-ext {
>> +					filename = "spl/dts/k3-j721e-common-proc-board.dtb";
>> +					};
>> +				};
>> +			};
>> +			configurations {
>> +				default = "conf";
>> +				conf {
>> +					description = "k3-j721e-common-proc-board";
>> +					firmware = "atf";
>> +					loadables = "tee", "dm", "spl";
>> +					fdt = "k3-j721e-common-proc-board.dtb";
>> +				};
>> +			};
>> +		};
>> +	};
>> +};
>> +#endif
>> diff --git a/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi b/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi
>> index 677a72d2a2..6490d71f7e 100644
>> --- a/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi
>> +++ b/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi
>> @@ -4,6 +4,7 @@
>>    */
>>   
>>   #include <dt-bindings/net/ti-dp83867.h>
>> +#include "k3-j721e-a72-binman.dtsi"
>>   
>>   / {
>>   	chosen {
>> diff --git a/arch/arm/mach-k3/config.mk b/arch/arm/mach-k3/config.mk
>> index 49f80ae79b..e0d9ea97c9 100644
>> --- a/arch/arm/mach-k3/config.mk
>> +++ b/arch/arm/mach-k3/config.mk
>> @@ -52,38 +52,5 @@ ifeq ($(CONFIG_SOC_K3_J721E),)
>>   export DM := /dev/null
>>   endif
>>   
>> -ifeq ($(CONFIG_TI_SECURE_DEVICE),y)
>> -SPL_ITS := u-boot-spl-k3_HS.its
>> -$(SPL_ITS): export IS_HS=1
>> -INPUTS-y	+= tispl.bin_HS
>> -else
>> -SPL_ITS := u-boot-spl-k3.its
>> -INPUTS-y	+= tispl.bin
>> -endif
>> -
>> -ifeq ($(CONFIG_SPL_OF_LIST),)
>> -LIST_OF_DTB := $(CONFIG_DEFAULT_DEVICE_TREE)
>> -else
>> -LIST_OF_DTB := $(CONFIG_SPL_OF_LIST)
>>   endif
>> -
>> -quiet_cmd_k3_mkits = MKITS   $@
>> -cmd_k3_mkits = \
>> -	$(srctree)/tools/k3_fit_atf.sh \
>> -	$(CONFIG_K3_ATF_LOAD_ADDR) \
>> -	$(patsubst %,$(obj)/dts/%.dtb,$(subst ",,$(LIST_OF_DTB))) > $@
>> -
>> -$(SPL_ITS): FORCE
>> -	$(call cmd,k3_mkits)
>>   endif
>> -
>> -else
>> -
>> -ifeq ($(CONFIG_TI_SECURE_DEVICE),y)
>> -INPUTS-y	+= u-boot.img_HS
>> -else
>> -INPUTS-y	+= u-boot.img
>> -endif
>> -endif
>> -
>> -include $(srctree)/arch/arm/mach-k3/config_secure.mk
> 
> You can't yet remove these as all K3 platforms are not converted to use binman.
> Please include them under #ifndef CONFIG_BINMAN...#endif
> 
> One more question I have is that how are _HS binaries being created with binman?
> 

I will make the changes accordingly that do not affect any of the other 
builds. _HS binaries are not being created with binman in this series.

But there is a patch series that does create HS images (tispl.bin and 
u-boot.img) using binman for AM64x: 
https://patchwork.ozlabs.org/project/uboot/patch/20220509072936.12899-4-rogerq@kernel.org/

>> diff --git a/board/ti/j721e/Kconfig b/board/ti/j721e/Kconfig
>> index a3a9d504ae..3cf05f0d3b 100644
>> --- a/board/ti/j721e/Kconfig
>> +++ b/board/ti/j721e/Kconfig
>> @@ -14,6 +14,7 @@ config TARGET_J721E_A72_EVM
>>   	select BOARD_LATE_INIT
>>   	imply TI_I2C_BOARD_DETECT
>>   	select SYS_DISABLE_DCACHE_OPS
>> +	select BINMAN
>>   
>>   config TARGET_J721E_R5_EVM
>>   	bool "TI K3 based J721E EVM running on R5"
>> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
>> index 6ad82cecfb..5f8a8487c5 100644
>> --- a/scripts/Makefile.spl
>> +++ b/scripts/Makefile.spl
>> @@ -577,7 +577,3 @@ $(obj)/$(SPL_BIN).multidtb.fit.gz: $(obj)/$(SPL_BIN).multidtb.fit
>>   $(obj)/$(SPL_BIN).multidtb.fit.lzo: $(obj)/$(SPL_BIN).multidtb.fit
>>   	@lzop -f9 $< > $@
>>   
>> -ifdef CONFIG_ARCH_K3
>> -tispl.bin: $(obj)/u-boot-spl-nodtb.bin $(SHRUNK_ARCH_DTB) $(SPL_ITS) FORCE
>> -	$(call if_changed,mkfitimage)
>> -endif
> 
> Please don't remove this but include it under #ifndef CONFIG_BINMAN...#endif
> 
> cheers,
> -roger

-- 
Thanking You
Neha Malcom Francis