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

[Ryan Eatmon <reatmon@ti.com>]

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.


> 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"
>>

-- 
Ryan Eatmon                reatmon@ti.com
-----------------------------------------
Texas Instruments, Inc.  -  LCPD  -  MGTS