Re: [PATCH 3/4] arm: dts: k3-*-binman.dtsi: Clean up and templatize boot binaries

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

Hi Manorit

On 25/03/24 14:50, Manorit Chawdhry wrote:
> Hi Neha,
> 
> On 18:40-20240322, Neha Malcom Francis wrote:
>> Clean up templatized boot binaries for all K3 boards. This includes
>> modifying the k3-binman.dtsi to use SPL_BOARD_DTB, BOARD_DESCRIPTION and
>> UBOOT_BOARD_DESCRIPTION from the files that include it to further reuse
>> code.
>>
>> All k3-<soc>-binman.dtsi will contain only templates. Only required boot
>> binaries can be built from the templates in the boards' respective
>> -u-boot.dtsi file (or k3-<board>-binman.dtsi if it exists). This allows
>> clear distinction between the SoC common stuff vs. what is additionally
>> needed to boot up a specific board.
>>
>> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
>> ---
>>   arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi  | 161 +---------
>>   arch/arm/dts/k3-am625-phycore-som-binman.dtsi | 291 +----------------
>>   arch/arm/dts/k3-am625-r5-beagleplay.dts       |  39 ---
>>   arch/arm/dts/k3-am625-sk-binman.dtsi          | 148 +--------
>>   arch/arm/dts/k3-am625-sk-u-boot.dtsi          |  42 +++
>>   .../dts/k3-am625-verdin-wifi-dev-binman.dtsi  | 296 +-----------------
>>   arch/arm/dts/k3-am62a-sk-binman.dtsi          | 146 +--------
>>   arch/arm/dts/k3-am62a7-sk-u-boot.dtsi         |  42 +++
>>   arch/arm/dts/k3-am642-evm-u-boot.dtsi         |  42 +++
>>   arch/arm/dts/k3-am642-sk-u-boot.dtsi          |  42 +++
>>   arch/arm/dts/k3-am64x-binman.dtsi             | 239 +-------------
>>   arch/arm/dts/k3-am654-base-board-u-boot.dtsi  |  49 +++
>>   arch/arm/dts/k3-am65x-binman.dtsi             | 144 +--------
>>   .../arm/dts/k3-am68-sk-base-board-u-boot.dtsi |  26 ++
>>   arch/arm/dts/k3-am69-sk-u-boot.dtsi           |  31 +-
>>   arch/arm/dts/k3-binman.dtsi                   |  96 ++++++
>>   arch/arm/dts/k3-j7200-binman.dtsi             | 145 +--------
>>   .../k3-j7200-common-proc-board-u-boot.dtsi    |  40 +++
>>   .../dts/k3-j721e-beagleboneai64-u-boot.dtsi   | 154 +--------
>>   arch/arm/dts/k3-j721e-binman.dtsi             | 262 +++-------------
>>   .../k3-j721e-common-proc-board-u-boot.dtsi    |  84 +++++
>>   arch/arm/dts/k3-j721e-r5-beagleboneai64.dts   |  91 +-----
>>   arch/arm/dts/k3-j721e-sk-u-boot.dtsi          |  84 +++++
>>   arch/arm/dts/k3-j721s2-binman.dtsi            | 231 +-------------
>>   .../k3-j721s2-common-proc-board-u-boot.dtsi   |  42 +++
>>   arch/arm/dts/k3-j784s4-binman.dtsi            | 154 +--------
>>   arch/arm/dts/k3-j784s4-evm-u-boot.dtsi        |  42 +++
>>   27 files changed, 858 insertions(+), 2305 deletions(-)
>>
> 
> [ snip ]
> 
>> diff --git a/arch/arm/dts/k3-am625-phycore-som-binman.dtsi b/arch/arm/dts/k3-am625-phycore-som-binman.dtsi
>> index ed50bfeb031..14fc8468c56 100644
>> --- a/arch/arm/dts/k3-am625-phycore-som-binman.dtsi
>> +++ b/arch/arm/dts/k3-am625-phycore-som-binman.dtsi
>> @@ -6,309 +6,48 @@
>>    * Author: Wadim Egorov <w.egorov@phytec.de>
>>    */
>>   
>> -#include "k3-binman.dtsi"
>> +#define SPL_BOARD_DTB "spl/dts/k3-am625-phyboard-lyra-rdk.dtb"
>> +#define BOARD_DESCRIPTION "k3-am625-phyboard-lyra-rdk"
>> +#define UBOOT_BOARD_DESCRIPTION "U-Boot for phyCORE-AM62x"
>> +
>> +#include "k3-am625-sk-binman.dtsi"
>>   
>>   #ifdef CONFIG_TARGET_PHYCORE_AM62X_R5
>>   &binman {
>>   	tiboot3-am62x-hs-phycore-som.bin {
>>   		filename = "tiboot3-am62x-hs-phycore-som.bin";
>> -		ti-secure-rom {
>> -			content = <&u_boot_spl>, <&ti_fs_enc>, <&combined_tifs_cfg>,
>> -				<&combined_dm_cfg>, <&sysfw_inner_cert>;
>> -			combined;
>> -			dm-data;
>> -			sysfw-inner-cert;
>> -			keyfile = "custMpk.pem";
>> -			sw-rev = <1>;
>> -			content-sbl = <&u_boot_spl>;
>> -			content-sysfw = <&ti_fs_enc>;
>> -			content-sysfw-data = <&combined_tifs_cfg>;
>> -			content-sysfw-inner-cert = <&sysfw_inner_cert>;
>> -			content-dm-data = <&combined_dm_cfg>;
> 
> I was looking between SoCs that we have and all this data seems common
> to me on first glance, like there is only some delta in tiboot3
> templates as well and maybe we can minimise those as well if am not
> missing anything.
> 
> in k3-binman.dtsi:
> 
> tiboot3_combined_gp: template-x {
> 	section {
> 		ti-secure-rom {
> 			content = <&u_boot_spl_unsigned>, <&ti_fs_gp>,
> 				<&combined_tifs_cfg_gp>, <&combined_dm_cfg_gp>;
> 			combined;
> 			dm-data;
> 			content-sbl = <&u_boot_spl_unsigned>;
> 			content-sysfw = <&ti_fs_gp>;
> 			content-sysfw-data = <&combined_tifs_cfg_gp>;
> 			content-dm-data = <&combined_dm_cfg_gp>;
> 			sw-rev = <1>;
> 			keyfile = "ti-degenerate-key.pem";
> 		};
> 		u_boot_spl_unsigned: u-boot-spl {
> 			no-expanded;
> 		};
> 		ti_fs_gp: ti-fs-gp.bin {
> 			type = "blob-ext";
> 			optional;
> 		};
> 		combined_tifs_cfg_gp: combined-tifs-cfg-gp.bin {
> 			filename = "combined-tifs-cfg.bin";
> 			type = "blob-ext";
> 		};
> 		combined_dm_cfg_gp: combined-dm-cfg-gp.bin {
> 			filename = "combined-dm-cfg.bin";
> 			type = "blob-ext";
> 		};
> 	};
> };
> 
> in k3-j721s2-binman.dtsi:
> 
> tiboot3_j721s2_gp_evm {
> 	insert-template = <&tiboot3_combined_gp>;
> 	filename = "tiboot3-j721s2-gp-evm.bin";
> 	section {
> 		ti-secure-rom {
> 			load = <0x41c00000>;
> 			load-sysfw = <0x40000>;
> 			load-sysfw-data = <0x67000>;
> 			load-dm-data = <0x41c80000>;
> 		}
> 		ti_fs_gp {
> 			filename = "ti-sysfw/ti-fs-firmware-j721s2-gp.bin";
> 		}
> 	}
> };
> 
> in k3-j784s4-binman.dtsi:
> 
> tiboot3_j784s4_gp_evm {
> 	insert-template = <&tiboot3_combined_gp>;
> 	filename = "tiboot3-j784s4-gp-evm.bin";
> 	section {
> 		ti-secure-rom {
> 			load = <0x41c00000>;
> 			load-sysfw = <0x40000>;
> 			load-sysfw-data = <0x66800>;
> 			load-dm-data = <0x41c80000>;
> 		}
> 		ti_fs_gp {
> 			filename = "ti-sysfw/ti-fs-firmware-j784s4-gp.bin";
> 		}
> 	}
> 
> }
> 
> Similar re-use can be done for other GP devices as well based on
> this template and similar can be extended for other variants HS-FS,
> HS-SE and other SR variants. This might end up a bit complicated as well
> as we do have a lot of combinations of combined boot and other boot
> variants that we have so let me know if you don't find improvement with
> this but I feel this might help in more readability and understanding
> for the initial bootloader template designs as well.
> 
> Regards,
> Manorit

Yes I think we can pull these changes into k3-binman.dtsi as well, will reduce 
code duplication a great deal. Thanks!

> 
>> -			load = <0x43c00000>;
>> -			load-sysfw = <0x40000>;
>> -			load-sysfw-data = <0x67000>;
>> -			load-dm-data = <0x43c3a800>;
> 
> 
>> -		};
>> -		u_boot_spl: u-boot-spl {
>> -			no-expanded;
>> -		};
>> -		ti_fs_enc: ti-fs-enc.bin {
>> -			filename = "ti-sysfw/ti-fs-firmware-am62x-hs-enc.bin";
>> -			type = "blob-ext";
>> -			optional;
>> -		};
>> -		combined_tifs_cfg: combined-tifs-cfg.bin {
>> -			filename = "combined-tifs-cfg.bin";
>> -			type = "blob-ext";
>> -		};
>> -		sysfw_inner_cert: sysfw-inner-cert {
>> -			filename = "ti-sysfw/ti-fs-firmware-am62x-hs-cert.bin";
>> -			type = "blob-ext";
>> -			optional;
>> -		};
>> -		combined_dm_cfg: combined-dm-cfg.bin {
>> -			filename = "combined-dm-cfg.bin";
>> -			type = "blob-ext";
>> -		};
>> +		insert-template = <&tiboot3_am62x_hs>;
>>   	};
>> -};
>>   
>> -&binman {
>>   	tiboot3-am62x-hs-fs-phycore-som.bin {
>>   		filename = "tiboot3-am62x-hs-fs-phycore-som.bin";
>> -		symlink = "tiboot3.bin";
>> -		ti-secure-rom {
>> -			content = <&u_boot_spl_fs>, <&ti_fs_enc_fs>, <&combined_tifs_cfg_fs>,
>> -				<&combined_dm_cfg_fs>, <&sysfw_inner_cert_fs>;
>> -			combined;
>> -			dm-data;
>> -			sysfw-inner-cert;
>> -			keyfile = "custMpk.pem";
>> -			sw-rev = <1>;
>> -			content-sbl = <&u_boot_spl_fs>;
>> -			content-sysfw = <&ti_fs_enc_fs>;
>> -			content-sysfw-data = <&combined_tifs_cfg_fs>;
>> -			content-sysfw-inner-cert = <&sysfw_inner_cert_fs>;
>> -			content-dm-data = <&combined_dm_cfg_fs>;
>> -			load = <0x43c00000>;
>> -			load-sysfw = <0x40000>;
>> -			load-sysfw-data = <0x67000>;
>> -			load-dm-data = <0x43c3a800>;
>> -		};
>> -		u_boot_spl_fs: u-boot-spl {
>> -			no-expanded;
>> -		};
>> -		ti_fs_enc_fs: ti-fs-enc.bin {
>> -			filename = "ti-sysfw/ti-fs-firmware-am62x-hs-fs-enc.bin";
>> -			type = "blob-ext";
>> -			optional;
>> -		};
>> -		combined_tifs_cfg_fs: combined-tifs-cfg.bin {
>> -			filename = "combined-tifs-cfg.bin";
>> -			type = "blob-ext";
>> -		};
>> -		sysfw_inner_cert_fs: sysfw-inner-cert {
>> -			filename = "ti-sysfw/ti-fs-firmware-am62x-hs-fs-cert.bin";
>> -			type = "blob-ext";
>> -			optional;
>> -		};
>> -		combined_dm_cfg_fs: combined-dm-cfg.bin {
>> -			filename = "combined-dm-cfg.bin";
>> -			type = "blob-ext";
>> -		};
>> +		insert-template = <&tiboot3_am62x_hs_fs>;
>>   	};
>> -};
>>   
>> -&binman {
>>   	tiboot3-am62x-gp-phycore-som.bin {
>>   		filename = "tiboot3-am62x-gp-phycore-som.bin";
>> -		ti-secure-rom {
>> -			content = <&u_boot_spl_unsigned>, <&ti_fs_gp>,
>> -				<&combined_tifs_cfg_gp>, <&combined_dm_cfg_gp>;
>> -			combined;
>> -			dm-data;
>> -			content-sbl = <&u_boot_spl_unsigned>;
>> -			load = <0x43c00000>;
>> -			content-sysfw = <&ti_fs_gp>;
>> -			load-sysfw = <0x40000>;
>> -			content-sysfw-data = <&combined_tifs_cfg_gp>;
>> -			load-sysfw-data = <0x67000>;
>> -			content-dm-data = <&combined_dm_cfg_gp>;
>> -			load-dm-data = <0x43c3a800>;
>> -			sw-rev = <1>;
>> -			keyfile = "ti-degenerate-key.pem";
>> -		};
>> -		u_boot_spl_unsigned: u-boot-spl {
>> -			no-expanded;
>> -		};
>> -		ti_fs_gp: ti-fs-gp.bin {
>> -			filename = "ti-sysfw/ti-fs-firmware-am62x-gp.bin";
>> -			type = "blob-ext";
>> -			optional;
>> -		};
>> -		combined_tifs_cfg_gp: combined-tifs-cfg-gp.bin {
>> -			filename = "combined-tifs-cfg.bin";
>> -			type = "blob-ext";
>> -		};
>> -		combined_dm_cfg_gp: combined-dm-cfg-gp.bin {
>> -			filename = "combined-dm-cfg.bin";
>> -			type = "blob-ext";
>> -		};
>> +		insert-template = <&tiboot3_am62x_gp>;
>>   	};
>>   };
>>   #endif /* CONFIG_TARGET_PHYCORE_AM62X_R5 */

-- 
Thanking You
Neha Malcom Francis