[Buildroot] [PATCH v9 01/11] boot/ti-k3-r5-loader: allow for full build source customization

[Arnout Vandecappelle]

On 24/06/2023 23:23, Yann E. MORIN wrote:
> Andreas, All,
> 
> On 2023-06-22 11:02 -0500, Andreas Dannenberg via buildroot spake thusly:
>> The TI K3 R5 loader package essentially is a wrapper to build a special
>> version of U-boot SPL used as part of a multi-stage boot flow on TI K3
>> devices, and as such needs full flexibility as to specifying the U-Boot
>> sources used for the build. To accomodate this, add the same options as
>> already available in the regular U-Boot package. For most use cases, the
>> same source settings (repo URL, versions, etc.) should be used for both
>> ti-k3-r5-loader and uboot packages.
> 
> Currently, there is no dependency ("depends on" or "select") between
> uboot and ti-k3-r5-loader; yet, your phrasing seems to imply that it
> does not make sense to build ti-k3-r5-loader without building uboot.

  That's not what I read in that paragraph. I think it means that the same 
source should be used as U-Boot. It's just a separate package because, AFAIU, it 
may use a different .config.

  If that is true, and I'm not missing anything else, then the situation 
actually looks quite similar to barebox-aux. With the difference that for Falcon 
boot you want to have only "u-boot-aux" and not the full u-boot. So, if what I'm 
saying is true, then perhaps it's better to approach it the same way as in barebox?

  At the very minimum, I think there should be a "same version as U-Boot" 
option, which is the default (if U-Boot is enabled). There's a bit the 
unfortunate aspect that this package becomes before U-Boot in the menu, but 
let's assume people typically start from a defconfig.

  Regards,
  Arnout

> 
> Then, in the comment below, you add a note about "Falcon boot (which
> would skip the uboot package completely)".
> 
> So, I'm a bit curious here: what should the user do when uboot is not
> enabled, if we instruct it to use the same version?
> 
>> Signed-off-by: Andreas Dannenberg <dannenberg at ti.com>
>> ---
>>   boot/ti-k3-r5-loader/Config.in          | 65 +++++++++++++++++++++++--
>>   boot/ti-k3-r5-loader/ti-k3-r5-loader.mk | 32 +++++++++++-
>>   2 files changed, 93 insertions(+), 4 deletions(-)
>>
>> diff --git a/boot/ti-k3-r5-loader/Config.in b/boot/ti-k3-r5-loader/Config.in
>> index 10602c8df3..2419babe70 100644
>> --- a/boot/ti-k3-r5-loader/Config.in
>> +++ b/boot/ti-k3-r5-loader/Config.in
>> @@ -1,14 +1,73 @@
>>   config BR2_TARGET_TI_K3_R5_LOADER
>> -	bool "ti-k3-r5-loader"
>> +	bool "TI K3 R5 Loader"
> 
> Our prompts always match the package name in Buildroot, i.e. the
> directory name. In this case, ti-k3-r5-loader is exactly that and we
> want to keep that.
> 
>>   	depends on BR2_aarch64
>>   	help
>> -	  Separate U-Boot build for R5 cores on TI's k3 boards.
>> +	  Separate U-Boot SPL build for R5 core on TI's K3 processors.
>>   	  Usually used to build tiboot3.bin with k3-image-gen.
>>   
>>   if BR2_TARGET_TI_K3_R5_LOADER
>>   
>>   choice
>> -	prompt "Configuration"
>> +	prompt "U-Boot Version"
> 
> It is a bit confusing tho see "U-Boot version" in the "ti-k3-r5-loader"
> package. I know it is really just a uboot being compiled, but still this
> is confusing. I'd keep ti-k3-r5-loader in the prompts. I could be
> convinced for the middle ground "ti-k3-r5-loader U-Boot version" if you
> really want to have U-Boot there.
> 
> Since you want the defaults to be the same as uboot, you can do that:
> 
>      choice
>          bool "U-Boot version"
>          default BR2_TARGET_TI_K3_R5_LOADER_LATEST_VERSION if BR2_TARGET_UBOOT_LATEST_VERSION
>          default BR2_TARGET_TI_K3_R5_LOADER_CUSTOM_VERSION if BR2_TARGET_UBOOT_CUSTOM_VERSION
>          default BR2_TARGET_TI_K3_R5_LOADER_CUSTOM_TARBALL if BR2_TARGET_UBOOT_CUSTOM_TARBALL
>          default BR2_TARGET_TI_K3_R5_LOADER_CUSTOM_GIT if BR2_TARGET_UBOOT_CUSTOM_GIT
>          default BR2_TARGET_TI_K3_R5_LOADER_CUSTOM_HG if BR2_TARGET_UBOOT_CUSTOM_HG
>          default BR2_TARGET_TI_K3_R5_LOADER_CUSTOM_SVN if BR2_TARGET_UBOOT_CUSTOM_SVN
>          default BR2_TARGET_TI_K3_R5_LOADER_LATEST_VERSION  # Fallback if uboot not enabled
> 
>      ...
>      endchoice
> 
>      config BR2_TARGET_TI_K3_R5_LOADER_CUSTOM_VERSION_VALUE
>          string "U-Boot version"
>          default BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE
>          depends on BR2_TARGET_TI_K3_R5_LOADER_CUSTOM_VERSION
> 
>      config BR2_TARGET_TI_K3_R5_LOADER_CUSTOM_TARBALL_LOCATION
>          string "URL of custom U-Boot tarball"
>          default BR2_TARGET_UBOOT_CUSTOM_TARBALL_LOCATION
>          depends on BR2_TARGET_TI_K3_R5_LOADER_CUSTOM_TARBALL
> 
> ... and so on, you get the gist.
> 
> [--SNIP--]
>> +	prompt "U-Boot Configuration"
> 
> Same comment as for the version prompt.
> 
> [--SNIP--]
>> diff --git a/boot/ti-k3-r5-loader/ti-k3-r5-loader.mk b/boot/ti-k3-r5-loader/ti-k3-r5-loader.mk
>> index afa309aa98..341888623e 100644
>> --- a/boot/ti-k3-r5-loader/ti-k3-r5-loader.mk
>> +++ b/boot/ti-k3-r5-loader/ti-k3-r5-loader.mk
>> @@ -2,11 +2,41 @@
>>   #
>>   # ti-k3-r5-loader
>>   #
>> +# The ti-k3-r5 loader package should really be built from the same U-Boot
>> +# sources as the uboot package itself, so for most users so all LOADER_SITE /
>> +# LOADER_SOURCE type definitions should be set the same for both packages.
>> +# However it still makes sense to keep the ti-k3-r5-loader package somewhat
>> +# separated and independent from the uboot package to allow for special use
>> +# cases such as Falcon boot (which would skip the uboot package completely).
> 
> Do you mean that uboot would not be built at all, or that the uboot
> binary would not be loaded at runtime?
> 
> Regards,
> Yann E. MORIN.
>