[PATCH v2 2/3] rockchip: spl: Cache boot source id for later use

[Dragan Simic]

On 2024-03-22 21:50, Jonas Karlman wrote:
> Rockchip BROM writes a boot source id at CFG_IRAM_BASE + 0x10, this id
> indicates from what storage media TPL/SPL was loaded from.
> 
> SPL uses this id to determine what device "same-as-spl" represent when
> determining from where FIT should be loaded. This works as long as the
> boot_devices array contain a matching id <-> node path entry.
> 
> However, SPL typically load a small part of TF-A into SRAM and on 
> RK3399
> this overwrites the CFG_IRAM_BASE + 0x10 addr used for boot source id.
> 
> For affected devices the u-boot,spl-boot-device would not be set when
> booting from SPI flash and the flash at 0 node was not explicitly listed
> in the u-boot,spl-boot-order prop.
> 
> Here boot source id is 3 before FIT images is loaded, and 0 after:
> 
>   U-Boot SPL 2024.04-rc4 (Mar 15 2024 - 17:26:19 +0000)
>   board_spl_was_booted_from: brom_bootdevice_id 3 maps to
> '/spi at ff1d0000/flash at 0'
>   Trying to boot from SPI
>   ## Checking hash(es) for config config-1 ... OK
>   ## Checking hash(es) for Image atf-1 ... sha256+ OK
>   ## Checking hash(es) for Image u-boot ... sha256+ OK
>   ## Checking hash(es) for Image fdt-1 ... sha256+ OK
>   ## Checking hash(es) for Image atf-2 ... sha256+ OK
>   ## Checking hash(es) for Image atf-3 ... sha256+ OK
>   board_spl_was_booted_from: failed to resolve brom_bootdevice_id 0
>   spl_decode_boot_device: could not find udevice for /mmc at fe330000
>   spl_decode_boot_device: could not find udevice for /mmc at fe320000
>   spl_perform_fixups: could not map boot_device to ofpath: -19
> 
> Use a static brom_bootsource_id_cache to save the boot source id after
> an initial read from SRAM to fix this, this allow spl_perform_fixups()
> to resolve correct boot source path for "same-as-spl" after SPL have
> loaded TF-A related FIT images into memory.
> 
> With this the spl-boot-device prop can correctly be resolved to the
> SPI flash node in the control FDT:
> 
>   => fdt addr ${fdtcontroladdr}
>   Working FDT set to f1ee6710
>   => fdt list /chosen
>   chosen {
>       u-boot,spl-boot-device = "/spi at ff1d0000/flash at 0";
>       stdout-path = "serial2:1500000n8";
>       u-boot,spl-boot-order = "same-as-spl", "/mmc at fe330000", 
> "/mmc at fe320000";
>   };
> 
> Fixes: d57e16c7e712 ("rockchip: find U-boot proper boot device by
> inverting the logic that sets it")
> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
> Reviewed-by: Kever Yang <kever.yang at rock-chips.com>

Looking good to me!  Thanks for taking the time to implement this
version, as a more robust approach.

Reviewed-by: Dragan Simic <dsimic at manjaro.org>

> ---
> v2:
> - Only cache boot source id that can be resolved to a node path
> - Make use of the new BROM_BOOTSOURCE_UNKNOWN enum value
> - Update commit message
> - Add fixes tag
> - Collect r-b tag
> ---
>  arch/arm/mach-rockchip/spl.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-rockchip/spl.c 
> b/arch/arm/mach-rockchip/spl.c
> index 1586a093fc37..3ce7e792b5a2 100644
> --- a/arch/arm/mach-rockchip/spl.c
> +++ b/arch/arm/mach-rockchip/spl.c
> @@ -32,18 +32,26 @@ __weak const char * const
> boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
> 
>  const char *board_spl_was_booted_from(void)
>  {
> -	u32  bootdevice_brom_id = readl(BROM_BOOTSOURCE_ID_ADDR);
> +	static u32 brom_bootsource_id_cache = BROM_BOOTSOURCE_UNKNOWN;
> +	u32 bootdevice_brom_id;
>  	const char *bootdevice_ofpath = NULL;
> 
> +	if (brom_bootsource_id_cache != BROM_BOOTSOURCE_UNKNOWN)
> +		bootdevice_brom_id = brom_bootsource_id_cache;
> +	else
> +		bootdevice_brom_id = readl(BROM_BOOTSOURCE_ID_ADDR);
> +
>  	if (bootdevice_brom_id < ARRAY_SIZE(boot_devices))
>  		bootdevice_ofpath = boot_devices[bootdevice_brom_id];
> 
> -	if (bootdevice_ofpath)
> +	if (bootdevice_ofpath) {
> +		brom_bootsource_id_cache = bootdevice_brom_id;
>  		debug("%s: brom_bootdevice_id %x maps to '%s'\n",
>  		      __func__, bootdevice_brom_id, bootdevice_ofpath);
> -	else
> +	} else {
>  		debug("%s: failed to resolve brom_bootdevice_id %x\n",
>  		      __func__, bootdevice_brom_id);
> +	}
> 
>  	return bootdevice_ofpath;
>  }