Re: [meta-ti][scarthgap][PATCH] conf: machine: am69: Update filenames for initial bootloader tiboot3

[Aniket Limaye <a-limaye@ti.com>]

On 03/02/25 21:10, Ryan Eatmon wrote:
> 
> 
> On 2/3/2025 9:35 AM, Ryan Eatmon via lists.yoctoproject.org wrote:
>>
>>
>> On 2/3/2025 9:25 AM, Aniket Limaye wrote:
>>>
>>>
>>> On 03/02/25 20:16, Ryan Eatmon wrote:
>>>>
>>>>
>>>> On 2/3/2025 7:18 AM, Aniket Limaye wrote:
>>>>> Update the filenames for tiboot3.bin variants built by the u-boot 
>>>>> recipe
>>>>> and also the files expected by the wic image.
>>>>>
>>>>> So far am69-sk and j784s4-evm have shared the same binman include file
>>>>> and hence the same filenames for u-boot images. However, with the
>>>>> configs and the devicetree being different, the same built binary does
>>>>> not support both boards. Hence the filenames of images built for 
>>>>> am69-sk
>>>>> should reflect support for the same. The corresponding
>>>>>
>>>>> This change is currently only applicable to ti-u-boot-2025.01, and 
>>>>> hence
>>>>> it is restricted to the override bsp-ti-6_12.
>>>>
>>>> Actually... I would prefer to align all of the defaults with the 
>>>> current default BSP.  Which right now is 6.12.
>>>>
>>>> So can you flip this patch around?  Make the default be the new name 
>>>> and add multiple settings for the old name on the other BSPs?  I 
>>>> think it would be more confusing to try and unravel the twisty maze 
>>>> of settings otherwise if we don't have a standard.
>>>>
>>>
>>> Yeah I can do that... I was wondering the same but thought I would 
>>> keep the patch smaller by adding an override for only the BSP that 
>>> requires it, instead of override for every other BSP. But yeah since 
>>> overrides will not be required for upstream eventually, makes sense 
>>> to do it the other way around from the get go. Will send a v2
>>>
>>> Another question I had was about the SYSFW_* variables... These are 
>>> ONLY being used by SPL_BINARY variable defined in k3r5.inc for the 
>>> u-boot recipe. Should I rename those variables to UBOOT_*, since it 
>>> is quite confusing currently.
>>> Eg: SYSFW_SOC = am69 but will use tifs binary with j784s4 and produce 
>>> tiboot3-am69-*.bin
>>>
>>> Infact why not define SPL_BINARY directly in <board>-k3r5.conf 
>>> instead of having these additional unnecessary variables to configure 
>>> the SPL_BINARY from k3r5.inc
>>
>> Generally speaking, I'm not opposed to cleaning up variable names to 
>> be more clear, but since this would be a change that would impact all 
>> platforms, I would prefer this patch leave the SYSFW stuff and then 
>> submit a second patch that cleans up all of the platforms at once.
>>
>> The only issue I can see with naming them UBOOT_* is that it might be 
>> confusing that these variable are used in the uboot recipe itself. 
>> Maybe TI_UBOOT_* would better to show that they are TI specific 
>> variables.
> 
> I also meant to say...
> 
> Using the multiple variables makes it really easy to build a consistent 
> naming system for SPL_BINARY.  By setting it directly in each machine 
> config you run the risk of the naming drifting across platforms.

Fair enough...
And yes was going to keep it a separate patch for the cleanup.

Thanks,
Aniket

> 
>>
>>> Maybe I'll address this in v2 as well...
>>>
>>>>
>>>>> Signed-off-by: Aniket Limaye <a-limaye@ti.com>
>>>>> ---
>>>>> This patch needs to be merged in sync with the corresponding patch for
>>>>> ti-u-boot-2025.01.
>>>>>
>>>>> The u-boot patch has also been posted to upstream u-boot and when 
>>>>> merged
>>>>> there, the next/upstream BSP packages will also need this change.
>>>>> https://lore.kernel.org/u-boot/20250106-b4-upstream-j742s2-v2-2-42d6129ce333@ti.com/
>>>>> ---
>>>>>   meta-ti-bsp/conf/machine/am69-sk-k3r5.conf | 2 ++
>>>>>   meta-ti-bsp/conf/machine/include/am69.inc  | 7 ++++---
>>>>>   2 files changed, 6 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/meta-ti-bsp/conf/machine/am69-sk-k3r5.conf 
>>>>> b/meta-ti-bsp/conf/machine/am69-sk-k3r5.conf
>>>>> index 9d2b4cd6..376d8153 100644
>>>>> --- a/meta-ti-bsp/conf/machine/am69-sk-k3r5.conf
>>>>> +++ b/meta-ti-bsp/conf/machine/am69-sk-k3r5.conf
>>>>> @@ -5,7 +5,9 @@
>>>>>   require conf/machine/include/k3r5.inc
>>>>>   SYSFW_SOC = "j784s4"
>>>>> +SYSFW_SOC:bsp-ti-6_12 = "am69"
>>>>>   SYSFW_CONFIG = "evm"
>>>>> +SYSFW_CONFIG:bsp-ti-6_12 = "sk"
>>>>>   SYSFW_SUFFIX = "hs-fs"
>>>>>   UBOOT_MACHINE = "am69_sk_r5_defconfig"
>>>>> diff --git a/meta-ti-bsp/conf/machine/include/am69.inc 
>>>>> b/meta-ti-bsp/conf/machine/include/am69.inc
>>>>> index da8a5a8b..e82d58e2 100644
>>>>> --- a/meta-ti-bsp/conf/machine/include/am69.inc
>>>>> +++ b/meta-ti-bsp/conf/machine/include/am69.inc
>>>>> @@ -9,10 +9,11 @@ require conf/machine/include/mesa-pvr.inc
>>>>>   PREFERRED_PROVIDER_virtual/gpudriver ?= 
>>>>> "${BSP_ROGUE_DRIVER_PROVIDER}"
>>>>>   # Default tiboot3.bin on AM69 is for SR1.0 HS-FS
>>>>> -IMAGE_BOOT_FILES += "tiboot3-j784s4-hs-fs-evm.bin"
>>>>> +# Add tiboot3.bin for all SOC types, to the wic image.
>>>>> +BSP_BOOTLOADER_FILES = "tiboot3-j784s4-hs-fs-evm.bin 
>>>>> tiboot3-j784s4-hs-evm.bin"
>>>>> +BSP_BOOTLOADER_FILES:bsp-ti-6_12 = "tiboot3-am69-hs-fs-sk.bin 
>>>>> tiboot3-am69-hs-sk.bin"
>>>>> -# Since default tiboot3.bin on AM69 is for HS-FS, add a version 
>>>>> for SR1.0 HS-SE
>>>>> -IMAGE_BOOT_FILES += "tiboot3-j784s4-hs-evm.bin"
>>>>> +IMAGE_BOOT_FILES += "${BSP_BOOTLOADER_FILES}"
>>>>>   TFA_BOARD = "j784s4"
>>>>
>>
>>
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#18245): 
>> https://lists.yoctoproject.org/g/meta-ti/message/18245
>> Mute This Topic: https://lists.yoctoproject.org/mt/110970137/6551054
>> Group Owner: meta-ti+owner@lists.yoctoproject.org
>> Unsubscribe: https://lists.yoctoproject.org/g/meta-ti/unsub 
>> [reatmon@ti.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>
>