Re: [PATCH v2 03/10] spl: ufs: add Falcon mode load path

Hi Alexey,

On 2026-07-31T17:10:44, Alexey Charkov <[email protected]> wrote:
> spl: ufs: add Falcon mode load path
>
> In Falcon mode, the SPL loads a FIT image containing a Linux kernel
> instead of U-boot proper, and it may need to fall back to loading U-boot
> if Linux is unavailable.
>
> Add support for images containing a Linux kernel, optionally on a
> different UFS LUN and/or offset vs. U-boot proper to enable fallback.
>
> Signed-off-by: Alexey Charkov <[email protected]>
>
> common/spl/Kconfig   | 23 +++++++++++++++++++++
>  common/spl/spl_ufs.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 79 insertions(+), 2 deletions(-)

>     In Falcon mode, the SPL loads a FIT image containing a Linux kernel
>     instead of U-boot proper, and it may need to fall back to loading U-boot
>     if Linux is unavailable.

Please spell it 'U-Boot' (both occurrences). Since this patch actually
accepts TF-A and TEE payloads too, please mention that here - the
current wording suggests only a Linux kernel is expected.

> diff --git a/common/spl/spl_ufs.c b/common/spl/spl_ufs.c
> @@ -20,13 +20,38 @@ static ulong spl_ufs_load_read(struct spl_load_info *load, ulong off, ulong size
> +     if (spl_image->os != IH_OS_LINUX && spl_image->os != IH_OS_TEE &&
> +         spl_image->os != IH_OS_ARM_TRUSTED_FIRMWARE) {
> +             puts("Expected OS image is not found\n");
> +             return -ENOENT;
> +     }

The equivalent check in mmc_load_image_raw_os() only accepts LINUX and
TEE - the ARM_TRUSTED_FIRMWARE addition is new and specific to this
series. Please mention that in the commit message so it is not
mistaken for a copy/paste divergence, and consider whether the
mmc/nand/spi paths should have the same relaxation in a follow-up.

> diff --git a/common/spl/spl_ufs.c b/common/spl/spl_ufs.c
> @@ -20,13 +20,38 @@ static ulong spl_ufs_load_read(struct spl_load_info *load, ulong off, ulong size
> +static int spl_ufs_load_image_raw_os(struct spl_image_info *spl_image,
> +                                  struct spl_boot_device *bootdev,
> +                                  struct spl_load_info *load,
> +                                  struct blk_desc *bd)
> +{
> +     ulong sector = config_opt_enabled(CONFIG_SPL_OS_BOOT,
> +                                       CONFIG_SPL_UFS_RAW_OS_SECTOR,
> +                                       CONFIG_SPL_UFS_RAW_U_BOOT_SECTOR);

bd here is just load->priv - please drop the extra parameter and fetch
log2blksz from load->priv, matching how spl_ufs_load_read() already
does it. Also, config_opt_enabled() is redundant: this helper is only
reachable from the OS_BOOT branch below, so
CONFIG_SPL_UFS_RAW_OS_SECTOR is always defined - just use it directly.

> diff --git a/common/spl/spl_ufs.c b/common/spl/spl_ufs.c
> @@ -36,6 +61,35 @@ static int spl_ufs_load_image(struct spl_image_info *spl_image,
> +             if (os_devnum == devnum) {
> +                     os_bd = bd;
> +                     os_load = load;
> +             } else {

The struct copy of load into os_load is unusual - spl_load_info is
meant to be initialised through spl_load_init(). Since the two
branches only diverge when the device differs, please just pass &load,
bd to the helper in the equal case and drop os_load entirely; only the
!= branch needs a second spl_load_info.

> diff --git a/common/spl/spl_ufs.c b/common/spl/spl_ufs.c
> @@ -36,6 +61,35 @@ static int spl_ufs_load_image(struct spl_image_info *spl_image,
> +             puts("spl_ufs_load_image: Failed to load falcon payload\n");
> +             log_debug("(error=%d)\n", err);
> +             if (CONFIG_IS_ENABLED(OS_BOOT_SECURE))
> +                     return err;
> +     }
> +
> +load_uboot:

The other backends (spl_mmc, spl_nor, ...) print 'Fallback to U-Boot'
before dropping through - please do the same here for consistency,
otherwise the boot log just shows the failure with no indication that
fallback is happening.

Regards,
Simon