Re: [PATCH 1/2] arm: dts: k3-am625: add template for tiboot3.bin

Hi Anshul,

On 2026-07-17T10:42:51, Anshul Dalal <[email protected]> wrote:
> arm: dts: k3-am625: add template for tiboot3.bin
>
> Currently we are duplicating binman nodes responsible for building
> tiboot3.bin binaries across boards that use TI's AM625 SoC.
>
> This patch adds a SoC level k3-am625-binman.dtsi with templates for the
> commonly used tiboot3 variants (i.e hs, hs-fs and gp) which can be
> inserted by the corresponding board's dtsi to promote reuse.

Please use 'i.e.' rather than 'i.e'.

>
> No functional change is intended from this patch.
>
> Signed-off-by: Anshul Dalal <[email protected]>
>
> arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi      |   2 +-
>  arch/arm/dts/k3-am625-binman.dtsi                 | 123 ++++++++++++++++++++++
>  arch/arm/dts/k3-am625-phycore-som-binman.dtsi     | 116 +-------------------
>  arch/arm/dts/k3-am625-r5-beagleplay.dts           |  33 +-----
>  arch/arm/dts/k3-am625-sk-binman.dtsi              | 117 +-------------------
>  arch/arm/dts/k3-am625-verdin-wifi-dev-binman.dtsi | 116 +-------------------
>  6 files changed, 137 insertions(+), 370 deletions(-)

> diff --git a/arch/arm/dts/k3-am625-binman.dtsi b/arch/arm/dts/k3-am625-binman.dtsi
> @@ -0,0 +1,123 @@
> +     tiboot3_am625_hs_fs_template: template-tiboot3-hs-fs {
> +             symlink = "tiboot3.bin";
> +             ti-secure-rom {

This bakes a policy into the template - that hs-fs is the default
tiboot3 - but the hs and gp templates deliberately don't. All three
callers (sk, verdin, phycore) already set symlink = "tiboot3.bin" on
the hs-fs node themselves, and on the r5 beagleplay the symlink lives
on the gp node. Would it be OK to drop this line so all three
templates are symmetric?

> diff --git a/arch/arm/dts/k3-am625-r5-beagleplay.dts b/arch/arm/dts/k3-am625-r5-beagleplay.dts
> @@ -33,38 +33,7 @@
>       tiboot3-am62x-gp-evm.bin {
>               filename = "tiboot3-am62x-gp-evm.bin";
>               symlink = "tiboot3.bin";
> -             ti-secure-rom {
> -                     content = <&u_boot_spl_unsigned>, <&ti_fs_gp>,
> -                             <&combined_tifs_cfg_gp>, <&combined_dm_cfg_gp>;

...
> -                     load = <0x43c00000>;

The original hardcodes load = <0x43c00000> whereas the template uses
load = <CONFIG_SPL_TEXT_BASE>. These resolve to the same value today
(the ARCH_K3 && !ARM64 default is 0x43c00000, and no R5 defconfig here
overrides it), so it is a no-op - but please mention it in the commit
message under 'No functional change is intended' so the next reader
doesn't have to chase Kconfig defaults.

> diff --git a/arch/arm/dts/k3-am625-binman.dtsi b/arch/arm/dts/k3-am625-binman.dtsi
> @@ -0,0 +1,123 @@
> +     tiboot3_am625_gp_template: template-tiboot3-gp {
> +             ti-secure-rom {
> +                     content = <&u_boot_spl_unsigned>, <&ti_fs_gp>,
> +                             <&combined_tifs_cfg_gp>, <&combined_dm_cfg_gp>;

Just to check - the property ordering inside ti-secure-rom here
(content-sbl / load / content-sysfw / load-sysfw / … then sw-rev /
keyfile) differs from the hs and hs-fs templates, which group all
content-* first and load-* last. Both work, but since you are
consolidating anyway it would read better if the three followed the
same layout. What do you think?

In any case:

Reviewed-by: Simon Glass <[email protected]>

Regards,
Simon