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

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?

> 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).

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

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

Regards,
Simon