Re: [PATCH v2 08/10] rockchip: spl: default to Falcon mode boot in SPL if enabled

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: spl: default to Falcon mode boot in SPL if enabled
> >
> > Common code calls into a platform specific function to determine whethe=
r
> > to attempt OS boot in Falcon mode.
> >
> > There is currently no platform logic to drive that decision on Rockchip=
,
> > so attempt Falcon mode boot whenever it is enabled in the configuration=
.
> >
> > This can be overridden by board code if needed with appropriate logic, =
such
> > as checking for a button state to skip Falcon mode boot when pressed.
> >
> > Signed-off-by: Alexey Charkov <[email protected]>
> >
> > arch/arm/mach-rockchip/spl.c | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
>
> > diff --git a/arch/arm/mach-rockchip/spl.c b/arch/arm/mach-rockchip/spl.=
c
> > @@ -153,6 +153,35 @@ void spl_board_prepare_for_boot(void)
> > +/*
> > + * This overrides the weak default in common/spl/spl.c, so it must not=
 itself
> > + * be weak: two weak definitions of the same symbol would leave the ch=
oice
> > + * between them up to archive link order.
> > + */
> > +int spl_start_uboot(void)
> > +{
> > +     return board_spl_start_uboot();
> > +}
> > +#endif
>
> You've already seen the vyasa-rk3288 problem here.

Yes, it's an odd one out, will fix up in the next iteration.

> > diff --git a/arch/arm/mach-rockchip/spl.c b/arch/arm/mach-rockchip/spl.=
c
> > @@ -153,6 +153,35 @@ void spl_board_prepare_for_boot(void)
> > +#if CONFIG_IS_ENABLED(OS_BOOT)
> > +/**
> > + * board_spl_start_uboot() - decides whether to boot the OS or U-Boot =
proper
> > + *
> > + * If Falcon mode (direct Linux boot via TF-A from SPL) is selected vi=
a config,
> > + * try booting to Linux first, with automatic fallback to U-Boot if it=
 fails to
> > + * load. Boards that want to implement conditional U-Boot/Linux select=
ion logic
> > + * override this, for instance to look at a button.
>
> Introducing a Rockchip-only board_spl_start_uboot() alongside the
> existing standard spl_start_uboot() hook is awkward - a board author
> has to know that on Rockchip the hook to override is the new one, not
> the documented one. Would it not be cleaner to leave spl_start_uboot()
> weak in the common code, drop this wrapper, and just have the
> mach-rockchip default live behind the existing weak symbol? The
> link-order concern applies equally to every other arch that provides a
> non-weak default (arm-k3, layerscape, zynqmp=E2=80=A6), and none of them =
add a
> second indirection.

It would be cleaner, but it foregoes the board-level override by
construction, which is a useful thing to support. Maybe I'll add a
Kconfig symbol which defaults y if CONFIG_ARM64 && CONFIG_SPL_OS_BOOT
&& MACH_ROCKCHIP and just let the boards disable it via defconfig when
they define a smarter version.

Best regards,
Alexey