Re: [PATCH v2 01/10] spl: Decide about Falcon mode boot only once

Hi Simon,

On Tue, Aug 4, 2026 at 5:02 PM Simon Glass <[email protected]> wrote:
>
> Hi Alexey,
>
> On 2026-07-31T17:10:44, Alexey Charkov <[email protected]> wrote:
> > spl: Decide about Falcon mode boot only once
> >
> > Board implementations of spl_start_uboot() are not required to be
> > idempotent - the documented examples sample a GPIO or read a character
> > from the SPL console - so calling it more than once can yield different
> > answers.
> >
> > Add spl_falcon_boot(), which calls spl_start_uboot() at most once and
> > caches the result.
> >
> > Signed-off-by: Alexey Charkov <[email protected]>
> >
> > common/spl/spl.c | 18 ++++++++++++++++++
> >  include/spl.h    | 11 +++++++++++
> >  2 files changed, 29 insertions(+)
>
> > diff --git a/common/spl/spl.c b/common/spl/spl.c
> > @@ -105,6 +105,24 @@ __weak int spl_start_uboot(void)
> > +bool spl_falcon_boot(void)
> > +{
> > +     static int falcon = -1;
> > +
> > +     if (falcon < 0)
> > +             falcon = !spl_start_uboot();
> > +
> > +     return falcon;
> > +}
>
> Every existing loader (spl_mmc.c, spl_nand.c, etc.) still calls
> spl_start_uboot() directly, so the mismatch you describe is still
> possible on any of those paths when combined with spl_invoke_atf() - a
> button released between the loader's call and spl_falcon_boot() in
> spl_atf.c will produce exactly the disagreement this helper is meant
> to prevent. Please convert the existing call sites as part of this
> patch (or at least the ones on the ATF path), so the caching actually
> delivers the guarantee.

Alright, will do. I only personally tested the UFS and Maskrom
boot-from-RAM, but happy to convert other callers as it doesn't seem
to have downsides.

> > diff --git a/common/spl/spl.c b/common/spl/spl.c
> > @@ -105,6 +105,24 @@ __weak int spl_start_uboot(void)
> > +bool spl_falcon_boot(void)
> > +{
> > +     static int falcon = -1;
>
> Minor - since the return type is bool, I'd find it cleaner to use two
> bools (result and 'decided'), or an enum, rather than overloading an
> int with a -1 sentinel. It also keeps the value out of .data, which
> matters a little in SPL.

Sounds good, will adjust in the next version.

Thanks a lot!

Best regards,
Alexey