[PATCH] rockchip: spl: Cache boot source id for later use
Quentin Schulz
quentin.schulz at theobroma-systems.com
Tue Mar 19 11:19:01 CET 2024
Hi Jonas,
On 3/15/24 18:34, Jonas Karlman wrote:
> Rockchip BROM write a boot source id at CFG_IRAM_BASE + 0x10, the id
> indicate from what storage media TPL/SPL was loaded from.
>
> SPL use this value 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.
>
> 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 bootdevice_brom_id to cache 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";
> };
>
I'm perplexed. We make use of this spl-boot-device DT property on Puma
(RK3399) and Ringneck (PX30) and I am pretty sure I tested it does what
it's supposed to do. So that is a bit surprising this seems to not work
anymore. Is this related to the BSS/stack memory address location
changes you made recently by any chance? Or did I manage to be very
lucky for a very long time for our boards?
"""
U-Boot SPL 2024.04-rc4-00026-g6ec096a7116-dirty (Mar 19 2024 - 10:50:03
+0100)
board_spl_was_booted_from: brom_bootdevice_id 5 maps to '/mmc at fe320000'
Trying to boot from MMC2
load_simple_fit: Skip load 'atf-5': image size is 0!
NOTICE: BL31: v2.9(release):v2.9.0
NOTICE: BL31: Built : 17:47:58, Jun 21 2023
U-Boot 2024.04-rc4-00026-g6ec096a7116-dirty (Mar 19 2024 - 10:50:03 +0100)
[...]
=> fdt addr ${fdtcontroladdr}
Working FDT set to f1f13d10
=> fdt list /chosen
chosen {
u-boot,spl-boot-device = "/mmc at fe320000";
stdout-path = "serial0:115200n8";
u-boot,spl-boot-order = "same-as-spl", "/spi at ff1d0000/flash at 0",
"/mmc at fe330000", "/mmc at fe320000";
};
"""
for Puma when booting from SD card... I don't see
board_spl_was_booted_from being called a second time after BL31 is loaded?
mmmmmmm
Very interestingly, when booting from SPI-NOR flash:
"""
U-Boot SPL 2024.04-rc4-00026-g6ec096a7116-dirty (Mar 19 2024 - 10:50:03
+0100)
board_spl_was_booted_from: brom_bootdevice_id 3 maps to
'/spi at ff1d0000/flash at 0'
Trying to boot from SPI
load_simple_fit: Skip load 'atf-5': image size is 0!
board_spl_was_booted_from: failed to resolve brom_bootdevice_id 0
NOTICE: BL31: v2.9(release):v2.9.0
NOTICE: BL31: Built : 17:47:58, Jun 21 2023
U-Boot 2024.04-rc4-00026-g6ec096a7116-dirty (Mar 19 2024 - 10:50:03 +0100)
[...]
=> fdt addr ${fdtcontroladdr}
Working FDT set to f1f13d10
=> fdt list /chosen
chosen {
u-boot,spl-boot-device = "/spi at ff1d0000/flash at 0";
stdout-path = "serial0:115200n8";
u-boot,spl-boot-order = "same-as-spl", "/spi at ff1d0000/flash at 0",
"/mmc at fe330000", "/mmc at fe320000";
};
"""
but the DT is properly written...
Ahah! This is because of one of my commits where I added support for
SPI-NOR flashes to spl_perform_fixups. So I think this worked for me
because the SPI-NOR flash is explicitly listed in spl-boot-order for
Puma, so when same-as-spl fails to resolve, the device is still found in
spl-boot-order DT property which means spl_perform_fixup will still be
able to write that spl-boot-device DT property. So basically, the issue
is related to SPI-NOR flash NOT being explicitly listed in
spl-boot-order or/and that the order isn't actually respected because
same-as-spl is basically skipped right now (but it works for Puma
because the next medium in the list is SPI, so skipping same-as-spl for
SPI, would result in checking SPI again :) ).
Can you please add:
Fixes: d57e16c7e712 ("rockchip: find U-boot proper boot device by
inverting the logic that sets it")
to the commit log regardless of the implementation we'll go for?
> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
> ---
> arch/arm/mach-rockchip/spl.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-rockchip/spl.c b/arch/arm/mach-rockchip/spl.c
> index 1586a093fc37..27e996b504e7 100644
> --- a/arch/arm/mach-rockchip/spl.c
> +++ b/arch/arm/mach-rockchip/spl.c
> @@ -32,9 +32,17 @@ __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 bootdevice_brom_id;
> const char *bootdevice_ofpath = NULL;
>
> + if (!bootdevice_brom_id)
> + bootdevice_brom_id = readl(BROM_BOOTSOURCE_ID_ADDR);
> + if (!bootdevice_brom_id) {
> + debug("%s: unknown brom_bootdevice_id %x\n",
> + __func__, bootdevice_brom_id);
> + return NULL;
> + }
I don't think we absolutely need the second if block, as this would be
handled by the else part of the if block that isn't shown in this git
context.
Also, I would suggest to add a new entry to the BROM_BOOTSOURCE_* enum,
e.g. BROM_BOOTSOURCE_INVALID/UNKNOWN = 0 so it's a bit more explicit and
we're also "ready" for the day Rockchip decides to use 0 as a valid BROM
boot source so we know all the places we need to modify the logic.
Moreover, I join Dragan over the use of a "valid" value for deciding to
read from the RAM... but for another reason. If it actually is 0 for
some reason, we would re-read from that address in RAM until we get
something different from 0... which may happen to be written with
something else than 0 when loading that small part of TF-A into SRAM? So
we would then have something completely unexpected as boot source now.
Cheers,
Quentin
More information about the U-Boot
mailing list