Re: [PATCH RFC v3 09/11] ti: dtsi: j721e: Use binman to package sysfw.itb and tiboot3.bin

[Alper Nebi Yasak <alpernebiyasak@gmail.com>]

On 15/06/2022 09:48, Neha Malcom Francis wrote:
> By providing entries in the binman node of the device tree, binman will
> be able to find and package board config binary artifacts generated by
> TIBoardConfig with sysfw.bin and generate the final image sysfw.itb.
> 
> k3-j721e-r5-binman.dtsi has been introduced for R5 specific binman node.
> It can be then be include by files that require it like
> k3-j721e-r5-common-proc-board-u-boot.dtsi.
> 
> 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-r5-binman.dtsi          | 88 +++++++++++++++++++
>  .../k3-j721e-r5-common-proc-board-u-boot.dtsi |  1 +
>  board/ti/j721e/Kconfig                        |  1 +
>  3 files changed, 90 insertions(+)
>  create mode 100644 arch/arm/dts/k3-j721e-r5-binman.dtsi
> 
> diff --git a/arch/arm/dts/k3-j721e-r5-binman.dtsi b/arch/arm/dts/k3-j721e-r5-binman.dtsi
> new file mode 100644
> index 0000000000..e3aeefb08f
> --- /dev/null
> +++ b/arch/arm/dts/k3-j721e-r5-binman.dtsi
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/
> +
> +#include <config.h>
> +
> +/ {
> +	binman: binman {
> +		multiple-images;
> +	};
> +};
> +
> +&binman {
> +	tiboot3 {
> +		filename = "tiboot3.bin";
> +		ti-x509-cert {
> +			content = <&image1>;
> +			core = <16>;
> +			load = <CONFIG_SPL_TEXT_BASE>;
> +		};
> +		image1: u-boot-spl {
> +			no-expanded;
> +		};

Please put empty lines between properties and nodes, and between each
node. Labels could be u_boot_spl, ti_sysfw instead of image1/2.

> +	};
> +	binary {
> +		filename = "sysfw.bin";
> +		ti-x509-cert {
> +			content = <&image2>;
> +			core = <0>;
> +			load = <0x0040000>;
> +		};
> +		image2: ti-sysfw {
> +		};
> +	};
> +	itb {
> +		filename = "sysfw.itb";
> +		fit {
> +			description = "SYSFW and Config Fragments";
> +			#address-cells = <1>;
> +			images {
> +				sysfw.bin {
> +					description = "sysfw";
> +					type = "firmware";
> +					arch = "arm";
> +					compression = "none";
> +					blob {
> +						filename = "sysfw.bin";
> +					};
> +				};

I dislike having the meaningless 'blob' and 'blob-ext' as node names.
Having the extension doesn't make much sense to me for the FIT entry,
but I like it for the blob itself:

    sysfw {
            description = "sysfw";
            type = "firmware";
            arch = "arm";
            compression = "none";

            sysfw.bin {
                    filename = "sysfw.bin";
                    type = "blob";
            };
    };

> +				board-cfg.bin {
> +					description = "board-cfg";
> +					type = "firmware";
> +					arch = "arm";
> +					compression = "none";
> +					blob-ext {
> +						filename = "board-cfg.bin";
> +					};
> +				};
> +				pm-cfg.bin {
> +					description = "pm-cfg";
> +					type = "firmware";
> +					arch = "arm";
> +					compression = "none";
> +					blob-ext {
> +						filename = "pm-cfg.bin";
> +					};
> +				};
> +				rm-cfg.bin {
> +					description = "rm-cfg";
> +					type = "firmware";
> +					arch = "arm";
> +					compression = "none";
> +					blob-ext {
> +						filename = "rm-cfg.bin";
> +					};
> +				};
> +				sec-cfg.bin {
> +					description = "sec-cfg";
> +					type = "firmware";
> +					arch = "arm";
> +					compression = "none";
> +					blob-ext {
> +						filename = "sec-cfg.bin";
> +					};
> +				};
> +			};
> +		};
> +	};
> +};
> diff --git a/arch/arm/dts/k3-j721e-r5-common-proc-board-u-boot.dtsi b/arch/arm/dts/k3-j721e-r5-common-proc-board-u-boot.dtsi
> index 48c6ddf672..75ec722e89 100644
> --- a/arch/arm/dts/k3-j721e-r5-common-proc-board-u-boot.dtsi
> +++ b/arch/arm/dts/k3-j721e-r5-common-proc-board-u-boot.dtsi
> @@ -4,6 +4,7 @@
>   */
>  
>  #include "k3-j721e-common-proc-board-u-boot.dtsi"
> +#include "k3-j721e-r5-binman.dtsi"
>  
>  / {
>  	chosen {
> diff --git a/board/ti/j721e/Kconfig b/board/ti/j721e/Kconfig
> index c28752a658..a3a9d504ae 100644
> --- a/board/ti/j721e/Kconfig
> +++ b/board/ti/j721e/Kconfig
> @@ -24,6 +24,7 @@ config TARGET_J721E_R5_EVM
>  	select RAM
>  	select SPL_RAM
>  	select K3_DDRSS
> +	select BINMAN
>  	imply SYS_K3_SPL_ATF
>  	imply TI_I2C_BOARD_DETECT
>