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=E2=80=AFPM 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+Linu=
x
> >
> > Modern Rockchip SoCs such as RK3576 require TF-A to be running to provi=
de
> > firmware services to the OS. To enable booting Linux in Falcon mode on
> > such SoCs, add a binman template for generation of FIT images containin=
g
> > appropriately split TF-A and TEE binaries (as is currently done by binm=
an
> > for U-boot proper images), externally provided Linux kernel and optiona=
lly
> > 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 provid=
ed)
> > and a bootargs property containing the kernel command line (if provided=
),
> > so that the kernel can be booted directly without external preprocessin=
g.
> >
> > 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 =3D "section";
> > +                     align =3D <CONFIG_SYS_CACHELINE_SIZE>;
> > +                     /*
> > +                      * Must be named 'u-boot-any', unlike the non-Fal=
con
> > +                      * image which can call this 'fit': SPL declares
> > +                      * binman_sym(u_boot_any, image_pos) unconditiona=
lly, and
> > +                      * binman resolves that symbol against an entry o=
f that
> > +                      * exact name or one called u-boot[-elf|-img|-nod=
tb].
> > +                      * 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 bi=
nman
> > +                      * fails. The type property is what selects the e=
type.
> > +                      */
> > +                     u-boot-any {
> > +                             type =3D "fit";
> > +                             insert-template =3D <&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 =3D "ramdisk";
> > +                             type =3D "ramdisk";
> > +                             arch =3D FIT_ARCH;
> > +                             compression =3D "none";
> > +                             load =3D <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/20260730-b4-spl-fit-clear-=
[email protected]/

> > diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kc=
onfig
> > @@ -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=E2=80=A6) 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/roc=
kchip-falcon.config
> > @@ -0,0 +1,3 @@
> > +CONFIG_ROCKCHIP_FALCON_IMAGE=3Dy
> > +CONFIG_SPL_OS_BOOT=3Dy
> > +CONFIG_SYS_SPI_KERNEL_OFFS=3D0x0
>
> 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