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

Alper Nebi Yasak
2022-07-01T19:10:17Z

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
>