Hi Vignesh,
On 2026-08-04T21:55:02, Bryan Brattlof <[email protected]> wrote:
> arm: dts: k3-am62lx: Add binman support
>
> Add the initial configuration to build the boot binaries for the AM62L
> family of devices
>
> Tested-by: Anshul Dalal <[email protected]>
> Reviewed-by: Anshul Dalal <[email protected]>
> Signed-off-by: Vignesh Raghavendra <[email protected]>
> Signed-off-by: Bryan Brattlof <[email protected]>
>
> arch/arm/dts/k3-am62l3-evm-binman.dtsi | 218 +++++++++++++++++++++++++++++++++
> arch/arm/dts/k3-am62l3-evm-u-boot.dtsi | 12 ++
> arch/arm/mach-k3/Kconfig | 21 ++++
> 3 files changed, 251 insertions(+)
> arm: dts: k3-am62lx: Add binman support
>
> Add the initial configuration to build the boot binaries for the AM62L
> family of devices
The body needs more context - how this differs from the existing
k3-binman.dtsi (why AM62L cannot reuse it), and why the new Kconfig
options are being introduced now.
> diff --git a/arch/arm/dts/k3-am62l3-evm-binman.dtsi
> b/arch/arm/dts/k3-am62l3-evm-binman.dtsi
> @@ -0,0 +1,218 @@
> +// SPDX-License-Identifier: GPL-2.0-only or MIT
Per the SPDX spec the operator is upper-case OR, i.e. GPL-2.0-only OR
MIT. Every other K3 binman dtsi in the tree uses plain GPL-2.0 - is
there a reason to dual-license this one?
> diff --git a/arch/arm/dts/k3-am62l3-evm-binman.dtsi
> b/arch/arm/dts/k3-am62l3-evm-binman.dtsi
> @@ -0,0 +1,218 @@
> + fit {
> + description = "U-Boot fitImage for AM62Lx SK";
> +
> + images {
> + uboot {
> + description = "U-Boot for AM62Lx SK";
This board is the AM62L-EVM, not an SK, isn't it? Please update both
description strings (the two k3-am62lx-evm ones look correct already).
> diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
> @@ -129,6 +129,26 @@ config K3_EARLY_CONS_IDX
> +config K3_TIFS_LOAD_ADDR
> + hex "Load address of TI's Foundational Security (TIFS) firmware"
> + default 0x40000
> + help
> + The load address for TI's Foundation Security firmware. This value is
> + used to build the x509 certificate to tell boot ROM where to place the
> + TIFS firmware in memory.
> +
> + Unless you know what you're doing. Do not change this value.
The last line reads as two sentence fragments - how about 'Do not
change this value unless you know what you are doing'.
Second, these options are unconditional so they appear for every K3
SoC, but only AM62L3 consumes them - every other K3 binman dtsi still
hard-codes 0x40000/0x67000. Either gate the prompt on SOC_K3_AM62L3
(with per-SoC defaults) or convert the other dtsi files to use these
too. Introducing a global knob that only one SoC honours is confusing.
> diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
> @@ -129,6 +129,26 @@ config K3_EARLY_CONS_IDX
> +config K3_TIFS_DATA_LOAD_ADDR
> + hex "Load address of TIFS configuration data"
> + default 0x6c000
> + help
> + The load address for TI's Foundation Security configuration data. This
> + value is used to build the x509 certificate to tell ROM where to place
> + the TIFS firmware.
The help text says 'place the TIFS firmware' but this option is the
load address of the TIFS *data*, not the firmware itself.
Regards,
Simon