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

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.

Best regards,
Alexey