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

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 whether
> 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, su=
ch
> 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 i=
tself
> + * be weak: two weak definitions of the same symbol would leave the choi=
ce
> + * 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.

> 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 pr=
oper
> + *
> + * If Falcon mode (direct Linux boot via TF-A from SPL) is selected via =
config,
> + * try booting to Linux first, with automatic fallback to U-Boot if it f=
ails to
> + * load. Boards that want to implement conditional U-Boot/Linux selectio=
n 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 ad=
d a
second indirection.

Regards,
Simon