[PATCH RFC v2 2/8] arm: dts: k3-j784s4-binman.dtsi: Clean up and templatize boot binaries

Wed Feb 5 16:35:56 CET 2025

On 2/4/25 11:50 PM, Manorit Chawdhry wrote:
> Hi Andrew,
> 
> On 09:43-20250204, Andrew Davis wrote:
>> On 2/3/25 10:43 PM, Manorit Chawdhry wrote:
>>> Hi Andrew,
>>>
>>> On 14:44-20250203, Andrew Davis wrote:
>>>> On 1/6/25 3:34 AM, Manorit Chawdhry wrote:
>>>>> From: Neha Malcom Francis <n-francis at ti.com>
>>>>>
>>>>> Clean up templatized boot binaries for j784s4 soc. This includes
>>>>> modifying the k3-j784s4-binman.dtsi to use SPL_BOARD_DTB,
>>>>> BOARD_DESCRIPTION and UBOOT_BOARD_DESCRIPTION from the files that
>>>>> include it to further reuse code.
>>>>>
>>>>> k3-j784s4-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 at ti.com>
>>>>> [ Do it only for j784s4 ]
>>>>> Signed-off-by: Manorit Chawdhry <m-chawdhry at ti.com>
>>>>> ---
>>>>>     arch/arm/dts/k3-am69-sk-u-boot.dtsi    | 123 +++++++++++++++++++++++++++------
>>>>>     arch/arm/dts/k3-j784s4-binman.dtsi     | 116 +++++++++----------------------
>>>>>     arch/arm/dts/k3-j784s4-evm-u-boot.dtsi |  75 ++++++++++++++++++++
>>>>>     3 files changed, 206 insertions(+), 108 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/dts/k3-am69-sk-u-boot.dtsi b/arch/arm/dts/k3-am69-sk-u-boot.dtsi
>>>>> index 4a82d2fd222669c4b390d4d877bc15329eab8894..adcd89b18ba9df9c72bf2e0fb0600b2bc7d1658c 100644
>>>>> --- a/arch/arm/dts/k3-am69-sk-u-boot.dtsi
>>>>> +++ b/arch/arm/dts/k3-am69-sk-u-boot.dtsi
>>>>> @@ -1,10 +1,109 @@
>>>>>     // SPDX-License-Identifier: GPL-2.0-only
>>>>>     /*
>>>>> - * Copyright (C) 2022-2023 Texas Instruments Incorporated - https://www.ti.com/
>>>>> + * Copyright (C) 2022-2024 Texas Instruments Incorporated - https://www.ti.com/
>>>>>      */
>>>>> +#define SPL_BOARD_DTB "spl/dts/ti/k3-am69-sk.dtb"
>>>>> +#define BOARD_DESCRIPTION "k3-am69-sk"
>>>>> +#define UBOOT_BOARD_DESCRIPTION "U-Boot for AM69 board"
>>>>> +
>>>>>     #include "k3-j784s4-binman.dtsi"
>>>>> +#if defined(CONFIG_CPU_V7R)
>>>>> +
>>>>> +&binman {
>>>>> +	tiboot3-am69-hs {
>>>>> +		insert-template = <&tiboot3_j784s4_hs>;
>>>>> +		filename = "tiboot3-am69-hs-sk.bin";
>>>>
>>>> I think there might be some confusion around the name of this file.
>>>> The format is tiboot3-<SYSFW name>-<SYSFW board config>.bin.
>>>>
>>>> <SYSFW name> is the name of the SYSFW binary that is packaged
>>>> with this file, so should be "j784s4-hs" as this uses:
>>>> "ti-fs-firmware-j784s4-hs-enc.bin".
>>>>
>>>> <SYSFW board config> is the name of the board configs used.
>>>> For both TI EVMs and SK boards there is just one common board config
>>>> we use call "evm". Other board vendors can use custom board configs,
>>>> take Toradex for example, they have a different board config which they
>>>> call "verdin" and they then correctly use that label for their tiboot3
>>>> filename[0]. We should follow our own standard, use "evm" here.
>>>>
>>>> Andrew
>>>>
>>>> [0] https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/k3-am625-verdin-wifi-dev-binman.dtsi
>>>
>>> The following is not some mistake and an intended change honestly.. we
>>> use am69_sk_r5_defconfig as the defconfig name and that giving j784s4 as
>>> the binary name is just counterintuitive, the naming should be based on
>>> the defconfig IMO to avoid confusions.
>>>
>>
>> The binary name does not need to match the marketing name of the board,
>> otherwise we could go name u-boot.img to append a board name too, etc.
>>
>> tiboot3.bin is different as we can have one build producing the same
>> SPL but with different SYSFW blobs appended. To deal with this we
>> append the name of the SYSFW blob to the filename. The SYSFW used here
>> is named "j784s4-hs", so What happens if we get a new SYSFW blob with the
>> name "am69-hs" but you already named this file "tiboot3-am69-hs-sk.bin"
>> even though it includes "j784s4-hs" SYSFW not "am69-hs" SYSFW..
>>
>> This filename is meant to identify the SYSFW name, not the defconfig
>> used. It was already correct, no need to change the standard here.
> 
> The standard has existed since we had a single tiboot3.bin packing multiple
> DTBs and working on AM69 and J784S4 (And similarly for other devices). At
> that time there was no other choice than to use the SYSFW name for
> identifying the binary - as what else could we use anyway?
> 

It goes back way further than that, this started back when the SYSFW blob
was appended to tiboot3.bin by an external tool[0] (I know you know this
but giving context for anyone else trying to follow along here).

> But now, the binaries aren't compatible with each other packing different
> DTBs, and are not the same. The following should've been done during the
> time when the defconfigs' were split itself but at that time templating was
> still being sorted out so we could'nt even make HS-FS as the default that
> time for AM69 ( which is the default device considering AM69 ) - we don't
> even make GP and even that was being built for that - doesn't mean it was
> the right thing to do.
> 

The boot binaries produced by different defconfigs are almost never
compatible with other devices. Sometimes yes they can be booted
across a couple very similar boards but this is rare. So nothing
new here.

> We should keep the "standard" just because it was historical doesn't
> make sense to me. U-boot is an entity and there is no relation
> whatsoever to keep the SYSFW name in the tiboot3 when they aren't even
> compatible anymore with other changes around - it should rather be based
> on what am building "in" U-boot.
> 

We do not name the binaries based on what device they are meant to
boot on. This has never been the case, u-boot.img is always just
named that, it doesn't change based on board. The name we append
to tiboot3.bin is *not* there to let folks know what device it is
compatible with nor what defconfig built it. In a perfect world
we would not append anything, just produce one "tiboot3.bin" no
matter what device/board you build for same as u-boot.img.

The reason for appending any name at all is we have a couple different
versions of SYSFW that need appended, so we name the resulting files
after the name of the SYSFW blob appended to it.

> I understand if you mean to say that the following is somehow being used
> in multiple places and it will have a cascading effect to change it at
> this stage but if you are worried about cascading effect; My suggestion
> would be to "fix" it now instead as the platforms are getting mature and
> then it'd be too late to do it and I don't want seeing
> am69_sk_r5_defconfig building tiboot3-j784s4-hs-fs-evm.bin 10 years down
> the line...
> 

I'm not saying we keep this standard only for historical reasons, I'm
perfectly fine with breaking conventions when it makes sense. Here
it breaks downstream projects that pick up these binaries *and* it
doesn't make sense.

> PS: it's still not too late if you are thinking that given the split for
> AM69 happened just a year back and the other such splits have hardly
> been there for around 8 months so we haven't even gotten the time to
> streamline it.
> 

A cycle or two might not be too late in some cases, but a year is
pushing it. This standard has been around for +5 years at least.
To break it now would mean changing boards like the the Beagle AI64
for instance, since it produces tiboot3-j721e-gp-evm, but it isn't
an "evm", nor an "sk", it is a "beaglebone". And it doesn't use
the J721e, it uses the TDA4VM SoC, etc..

Andrew

[0] https://git.ti.com/cgit/k3-image-gen/k3-image-gen

> Regards,
> Manorit
> 
>>
>> Andrew
>>
>>> Regards,
>>> Manorit