Hi Alexey,
On 2026-09-18T06:35:41, Alexey Charkov <[email protected]> wrote:
> spl: ufs: allow loading U-Boot from the boot logical unit
>
> Naming a fixed SCSI device number cannot express "use the currently
> activated boot LU". Offer the boot W-LU as an alternative source, so
> that flipping bBootLunEn between boot LU A and boot LU B moves U-Boot
> along with it and the SPL needs no reconfiguration.
>
> Signed-off-by: Alexey Charkov <[email protected]>
>
> common/spl/Kconfig | 30 +++++++++++++++++++++++++++++-
> common/spl/spl_ufs.c | 18 ++++++++++++++----
> 2 files changed, 43 insertions(+), 5 deletions(-)
> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> @@ -1635,9 +1635,37 @@ config SPL_UFS_SUPPORT
> +config SPL_UFS_RAW_U_BOOT_USE_BOOT_WLUN
> + bool "The UFS boot well known logical unit"
> + help
> + Load U-Boot through the boot W-LU, which the device maps onto
> whichever
> + logical unit bBootLunEn currently selects (boot LU A or boot LU B).
> This
> + follows an A/B switch made by the device itself, so the SPL does not
> have
> + to know which of the two is active.
Just to check, SPL_UFS_RAW_U_BOOT_SECTOR still applies here, so with
the Rockchip default of 0x800 U-Boot sits 8MB into the boot LU. Is
that what you intend? If so, please can you mention in the help that
the sector offset is relative to the start of the boot LU, since that
option's help only talks about 'the block device'.
BTW several of these help lines run past 80 columns, so please rewrap them.
> diff --git a/common/spl/spl_ufs.c b/common/spl/spl_ufs.c
> @@ -24,16 +25,25 @@ static int spl_ufs_load_image(struct spl_image_info
> *spl_image,
> - int devnum = CONFIG_SPL_UFS_RAW_U_BOOT_DEVNUM;
> + int devnum = config_opt_enabled(CONFIG_SPL_UFS_RAW_U_BOOT_USE_DEVNUM,
> + CONFIG_SPL_UFS_RAW_U_BOOT_DEVNUM, -1);
This mixes two styles: the full CONFIG_SPL_ name here and
CONFIG_IS_ENABLED() below. It also leaves a -1 which is never used,
since the W-LU path ignores devnum. I suspect it is cleaner to drop
the variable and use IF_ENABLED_INT() in the else branch, which gives
a build error if the value is ever used without the option:
if (IS_ENABLED(CONFIG_SPL_UFS_RAW_U_BOOT_USE_BOOT_WLUN)) {
...
} else {
int devnum = IF_ENABLED_INT(CONFIG_SPL_UFS_RAW_U_BOOT_USE_DEVNUM,
CONFIG_SPL_UFS_RAW_U_BOOT_DEVNUM);
bd = blk_get_devnum_by_uclass_id(UCLASS_SCSI, devnum);
if (!bd)
return -ENODEV;
}
What do you think?
> diff --git a/common/spl/spl_ufs.c b/common/spl/spl_ufs.c
> @@ -24,16 +25,25 @@ static int spl_ufs_load_image(struct spl_image_info
> *spl_image,
> + /* A UFS controller only ever has a single target */
> + if (scsi_get_blk_by_lun(0, UFS_UPIU_BOOT_WLUN, &bd)) {
> + puts("spl_ufs_load_image: UFS boot LU not found\n");
> + return -ENODEV;
> + }
I built rock-4d-rk3576 with the new option enabled and the SPL builds
and links, with only this path left in.
The message says 'boot LU' but what is missing is the boot W-LU. Since
the likely cause is bBootLunEn being 0, please can you say 'boot W-LU'
here so it matches the Kconfig help?
Regards,
Simon
