Re: [PATCH v2 09/10] rockchip: binman: Add support for Falcon mode FIT images with TF-A+Linux

Hi Alexey,

On Tue, 4 Aug 2026 at 08:38, Alexey Charkov <[email protected]> wrote:
>
> Hi Simon,
>
> On Tue, Aug 4, 2026 at 5:03 PM Simon Glass <[email protected]> wrote:
> >
> > Hi Alexey,
> >
> > On 2026-07-31T17:10:44, Alexey Charkov <[email protected]> wrote:
> > > rockchip: binman: Add support for Falcon mode FIT images with TF-A+Linux
> > >
> > > Modern Rockchip SoCs such as RK3576 require TF-A to be running to provide
> > > firmware services to the OS. To enable booting Linux in Falcon mode on
> > > such SoCs, add a binman template for generation of FIT images containing
> > > appropriately split TF-A and TEE binaries (as is currently done by binman
> > > for U-boot proper images), externally provided Linux kernel and optionally
> > > an initrd.
> > >
> > > Any FDTs provided to the generator will be pre-patched with a /chosen
> > > node containing an initrd load address and size (if an initrd is provided)
> > > and a bootargs property containing the kernel command line (if provided),
> > > so that the kernel can be booted directly without external preprocessing.
> > >
> > > A config fragment rockchip-falcon.config can be used to enable Falcon
> > > mode boot and the building of TF-A+Falcon with binman, e.g.:
> > >
> > >   make nanopi-m5-rk3576_defconfig \
> > >         rockchip-falcon.config
> > >
> > > [...]
> > >
> > > arch/arm/dts/rockchip-u-boot.dtsi     | 204 ++++++++++++++++++++++++++++------
> > >  arch/arm/mach-rockchip/Kconfig        |  49 ++++++++
> > >  board/rockchip/rockchip-falcon.config |   3 +
> > >  3 files changed, 225 insertions(+), 31 deletions(-)
> >
> > > diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
> > > @@ -260,6 +372,36 @@
> > > +             payload {
> > > +                     type = "section";
> > > +                     align = <CONFIG_SYS_CACHELINE_SIZE>;
> > > +                     /*
> > > +                      * Must be named 'u-boot-any', unlike the non-Falcon
> > > +                      * image which can call this 'fit': SPL declares
> > > +                      * binman_sym(u_boot_any, image_pos) unconditionally, and
> > > +                      * binman resolves that symbol against an entry of that
> > > +                      * exact name or one called u-boot[-elf|-img|-nodtb].
> > > +                      * Those only exist here by way of a U-Boot image inside
> > > +                      * the FIT, which a Falcon FIT does not carry, so without
> > > +                      * this name the symbol cannot be resolved and binman
> > > +                      * fails. The type property is what selects the etype.
> > > +                      */
> > > +                     u-boot-any {
> > > +                             type = "fit";
> > > +                             insert-template = <&fit_falcon_template>;
> > > +                     };
> >
> > A node called u-boot-any whose type is fit - needing a nine-line
> > comment to explain why - is a strong hint that we are papering over
> > something. Patch 8 already adds binman_sym_declare_optional() for the
> > payload symbol on the RAM_DEVICE path; the cleaner fix is to make
> > u_boot_any similarly optional (or key off SPL_OS_BOOT/RAM_DEVICE) in
> > common/spl/spl.c so a Falcon-only image does not need this pretence.
> > Then this entry can simply be named fit like its sibling in
> > simple-bin-usb472 and the comment goes away. What do you think?
>
> Hmm, that woud be nice indeed. Let me give it a try.
>
> > > diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
> > > @@ -161,6 +182,97 @@
> > > +                     ramdisk {
> > > +                             description = "ramdisk";
> > > +                             type = "ramdisk";
> > > +                             arch = FIT_ARCH;
> > > +                             compression = "none";
> > > +                             load = <FALCON_INITRD_LOAD>;
> > > +
> > > +                             linux-initrd {
> > > +                                     optional;
> > > +                             };
> >
> > Just to check - when LINUX_INITRD is not provided, does the outer
> > ramdisk image node still land in the FIT as a zero-byte data blob, and
> > does SPL then try to load a zero-byte ramdisk loadable to
> > FALCON_INITRD_LOAD? _process_firmware_prop() skips missing entries
> > when populating firmware, but I couldn't convince myself the same
> > happens for the enclosing image node itself. If it does not, please
> > add a testcase (or drop the ramdisk node entirely when the inner entry
> > is absent).
>
> The node still lands in the FIT with a size of zero, and the loader
> skips it based on its zero size. See also a related fix here:
>
> https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/
>
> > > diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> > > @@ -758,6 +758,55 @@ config ROCKCHIP_MASKROM_IMAGE
> > > +config ROCKCHIP_FALCON_KERNEL_LOAD
> > > +     hex "Falcon mode kernel load offset"
> > > +     depends on ROCKCHIP_FALCON_IMAGE
> > > +     default 0x10000000
> >
> > These three offsets are per-SoC in practice (BL31 / OP-TEE carve-outs
> > differ between RK3568, RK3576, RK3588…) yet the defaults are the same
> > for every Rockchip target. Please either narrow the default to the SoC
> > families you have validated, or add a note in the help text listing
> > which SoC the 0x10000000 / 0x1c000000 / 0x1e000000 layout matches, so
> > users of other Rockchip parts do not silently overlap BL31 and wonder
> > why the kernel never starts.
>
> Indeed, I haven't checked which segments BL31/TEE want to load to on
> SoCs other than RK3576; I only checked the addresses they hardcode
> elsewhere in U-Boot. Let me revisit this part, thanks for calling it
> out.
>
> > > diff --git a/board/rockchip/rockchip-falcon.config b/board/rockchip/rockchip-falcon.config
> > > @@ -0,0 +1,3 @@
> > > +CONFIG_ROCKCHIP_FALCON_IMAGE=y
> > > +CONFIG_SPL_OS_BOOT=y
> > > +CONFIG_SYS_SPI_KERNEL_OFFS=0x0
> >
> > CONFIG_SYS_SPI_KERNEL_OFFS depends on SPL_SPI_FLASH_SUPPORT &&
> > SPL_OS_BOOT and is only read by common/spl/spl_spi.c - none of the
> > boot paths added by this series use it. Please drop this line, or
> > explain in the commit message why it needs to be forced to 0 here.
>
> The nanopi-m5 defconfig enables SPL_SPI_FLASH_SUPPORT, so without
> giving this option a value I end up with an endless loop of prompts
> for it when building non-interactively (even though it's never used in
> my setup). Maybe defaulting it to the U-Boot offset inside the
> relevant Kconfig would be cleaner - let me check if that works.

OK see how you go, but if you do need it, just mention it in the commit message.

Regards,
Simon