Re: [meta-ti] [master/scarthgap][PATCH v1 3/3] bsp: u-boot: deploy tispl_falcon.bin to rootfs

[Denys Dmytriyenko <denis@denix.org>]

On Mon, Apr 21, 2025 at 02:31:47PM +0530, Anshul Dalal wrote:
> On Fri Apr 18, 2025 at 12:07 AM IST, Denys Dmytriyenko wrote:
> > On Thu, Apr 17, 2025 at 03:18:03PM +0530, Anshul Dalal wrote:
> >> On Wed Apr 16, 2025 at 8:03 PM IST, Denys Dmytriyenko wrote:
> >> > On Wed, Apr 16, 2025 at 08:29:13AM +0530, Anshul Dalal wrote:
> >> >> On Wed Apr 16, 2025 at 5:41 AM IST, Denys Dmytriyenko wrote:
> >> >> > On Tue, Apr 15, 2025 at 03:06:44PM +0530, Anshul Dalal via lists.yoctoproject.org wrote:
> >> >> >> Falcon boot requires tispl_falcon.bin to be on the same filesystem
> >> >> >> partition as the fitImage.
> >> >> >> 
> >> >> >> This patch creates a new u-boot package called
> >> >> >> 'u-boot-ti-staging-falcon' that is included in IMAGE_INSTALL when
> >> >> >> ti-falcon is enabled which allows us to deploy the required files to
> >> >> >> rootfs.
> >> >> >> 
> >> >> >> Signed-off-by: Anshul Dalal <anshuld@ti.com>
> >> >> >> ---
> >> >> >>  meta-ti-bsp/conf/machine/include/ti-falcon.inc |  1 +
> >> >> >>  meta-ti-bsp/recipes-bsp/u-boot/u-boot-ti.inc   | 11 +++++++++++
> >> >> >>  2 files changed, 12 insertions(+)
> >> >> >> 
> >> >> >> diff --git a/meta-ti-bsp/conf/machine/include/ti-falcon.inc b/meta-ti-bsp/conf/machine/include/ti-falcon.inc
> >> >> >> index b571c04d..9f93c683 100644
> >> >> >> --- a/meta-ti-bsp/conf/machine/include/ti-falcon.inc
> >> >> >> +++ b/meta-ti-bsp/conf/machine/include/ti-falcon.inc
> >> >> >> @@ -1,3 +1,4 @@
> >> >> >> +IMAGE_INSTALL:append = " u-boot-ti-staging-falcon"
> >> >> >
> >> >> > Why += cannot be used here?
> >> >> >
> >> >> >
> >> >> >>  KERNEL_CLASSES:remove = "kernel-fitimage"
> >> >> >>  KERNEL_CLASSES += "kernel-fitimage-legacyhs"
> >> >> >>  UBOOT_DTB_LOADADDRESS = "0x82000000"
> >> >> >> diff --git a/meta-ti-bsp/recipes-bsp/u-boot/u-boot-ti.inc b/meta-ti-bsp/recipes-bsp/u-boot/u-boot-ti.inc
> >> >> >> index 41d4abdd..76803ae3 100644
> >> >> >> --- a/meta-ti-bsp/recipes-bsp/u-boot/u-boot-ti.inc
> >> >> >> +++ b/meta-ti-bsp/recipes-bsp/u-boot/u-boot-ti.inc
> >> >> >> @@ -14,6 +14,8 @@ require u-boot-mergeconfig.inc
> >> >> >>  
> >> >> >>  FILESEXTRAPATHS:prepend := "${THISDIR}/u-boot:"
> >> >> >>  
> >> >> >> +FILES:${PN}-falcon = "boot/tispl_falcon.bin"
> >> >> >> +
> >> >> >>  SUMMARY = "u-boot bootloader for TI devices"
> >> >> >>  
> >> >> >>  LICENSE = "GPL-2.0-or-later"
> >> >> >> @@ -373,4 +375,13 @@ do_deploy:append:am62pxx() {
> >> >> >>  	install -m 0644 ${S}/tools/logos/ti_logo_414x97_32bpp.bmp.gz ${DEPLOYDIR}
> >> >> >>  }
> >> >> >>  
> >> >> >> +do_install:append:ti-falcon() {
> >> >> >> +	install -m 0644 ${B}/tispl_falcon.bin ${D}/boot
> >> >> >> +}
> >> >> >> +
> >> >> >> +PACKAGES:prepend:am62xx-evm = "${PN}-falcon "
> >> >> >> +PACKAGES:prepend:am62axx-evm = "${PN}-falcon "
> >> >> >> +PACKAGES:prepend:am62pxx-evm = "${PN}-falcon "
> >> >> >> +PACKAGES:prepend:am62xx-lp-evm = "${PN}-falcon "
> >> >> >
> >> >> > Shouldn't these be specific to ti-falcon only?
> >> >> >
> >> >> >
> >> >> 
> >> >> These are the only supported platforms at the moment, depending on
> >> >> machine specific overrides allows us to throw an error at the recipe
> >> >> parsing stage instead of having to wait till the do_install task fails
> >> >> to copy the non-existent tispl_falcon.bin in case of unsupported
> >> >> platforms.
> >> >
> >> > What happens when you build for am62xx-evm and ti-falcon is not enabled?
> >> >
> >> 
> >> The ${PN}-falcon package is added to the list of packages but the
> >> do_package task is never run for u-boot since it's not in
> >> IMAGES_INSTALL.
> >
> > All our platforms have u-boot in EXTRA_IMAGEDEPENDS, which means u-boot gets 
> > built and packaged all the time. Moreover, it's common to "bitbake u-boot"
> >
> 
> Recipes in EXTRA_IMAGEDEPENDS don't provide any packages to rootfs[1],
> hence why u-boot has no `package` directory when ti-falcon is not set or
> even before this patch set.
> 
> Therefore having the falcon u-boot package in PACKAGES does not change
> the build time behaviour until it's added to IMAGES_INSTALL (which is
> only done when ti-falcon is set).
> 
> In my testing, there is no difference between the work directory for
> u-boot with ti-falcon unset vs the work directory before this patch
> set[2].
> 
> [1]:
> https://docs.yoctoproject.org/ref-manual/variables.html#term-EXTRA_IMAGEDEPENDS
> 
> [2]:
> U-boot's work directories:
> 
> # before patch set:
> build                     image            recipe-sysroot-native  temp
> deploy-source-date-epoch  license-destdir  source-date-epoch
> deploy-u-boot-ti-staging  pseudo           srcipk-staging
> git                       recipe-sysroot   sysroot-destdir
> 
> # without ti-falcon:
> build                     image            recipe-sysroot-native  temp
> deploy-source-date-epoch  license-destdir  source-date-epoch
> deploy-u-boot-ti-staging  pseudo           srcipk-staging
> git                       recipe-sysroot   sysroot-destdir
> 
> # with ti-falcon:
> build                     package              recipe-sysroot-native
> deploy-ipks               packages-split       source-date-epoch
> deploy-source-date-epoch  pkgdata              spdx
> deploy-u-boot-ti-staging  pkgdata-pdata-input  srcipk-staging
> git                       pkgdata-sysroot      sysroot-destdir
> image                     pseudo               temp
> license-destdir           recipe-sysroot

You should not rely on this behavior expecting that U-boot will be built, but 
not packaged!

In scarthgap, this may be true:

$ MACHINE=am62xx-evm bitbake tisdk-base-image -g
$ grep '\-> "u-boot-ti-staging.do_package' task-depends.dot
$

But in walnascar/master, SPDX 3.x functionality for SW Bill of Materials is 
enabled by default and operates on packages, hence do_package() task will be 
called for U-boot:

$ MACHINE=am62xx-evm bitbake tisdk-base-image -g
$ grep u-boot-ti-staging.do_package task-depends.dot
"u-boot-ti-staging.do_create_spdx" -> "u-boot-ti-staging.do_package"
"u-boot-ti-staging.do_create_spdx" -> "u-boot-ti-staging.do_packagedata"
"u-boot-ti-staging.do_packagedata" -> "u-boot-ti-staging.do_package"
$


And indeed, the work directory contains both package and packages-split 
directories:

$ ls -1 arago-tmp-default/work/am62xx_evm-oe-linux/u-boot-ti-staging/2025.01+git/
build
deploy-source-date-epoch
deploy-u-boot-ti-staging
git
image
license-destdir
package
packages-split
pkgdata
pkgdata-pdata-input
pkgdata-sysroot
pseudo
recipe-sysroot
recipe-sysroot-native
source-date-epoch
sources-unpack
spdx
sysroot-destdir
temp

$ ls -1 arago-tmp-default/work/am62xx_evm-oe-linux/u-boot-ti-staging/2025.01+git/packages-split/
u-boot-ti-staging
u-boot-ti-staging-dbg
u-boot-ti-staging-dev
u-boot-ti-staging-doc
u-boot-ti-staging-env
u-boot-ti-staging-extlinux
u-boot-ti-staging-locale
u-boot-ti-staging-src
u-boot-ti-staging-staticdev


> >> So, the net effect is no change to the u-boot's working directory during
> >> the build process. It's only with ti-falcon that the respective
> >> packages-split directory is created with u-boot-ti-staging-falcon
> >> sub-directory.
> >> 
> >> >
> >> >> As support for more platforms is added, the list can be expanded until
> >> >> all platforms are supported when we can just have a ti-falcon override.