Hello Jonas,
Please see a few comments below.
On 2024-03-15 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.
s/write/writes/
s/indicate/indicates/
There are also a few more similar small grammar issues in the rest
of the patch descroption below.
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.
s/use/uses/
etc.
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@ff1d0000/flash@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@fe330000
spl_decode_boot_device: could not find udevice for /mmc@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@ff1d0000/flash@0";
stdout-path = "serial2:1500000n8";
u-boot,spl-boot-order = "same-as-spl", "/mmc@fe330000",
"/mmc@fe320000";
};
Signed-off-by: Jonas Karlman <jo...@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;
+ }
+
Maybe it would be better to execute readl(BROM_BOOTSOURCE_ID_ADDR)
only once, i.e. to have something like this instead:
+ static u32 bootdevice_brom_id = -1;
+ if (bootdevice_brom_id == -1) {
+ bootdevice_brom_id = readl(BROM_BOOTSOURCE_ID_ADDR);
+ if (!bootdevice_brom_id)
+ debug("%s: unknown brom_bootdevice_id %x\n",
+ __func__, bootdevice_brom_id);
+ }
+
+ if (!bootdevice_brom_id) /* fail on subsequent tries */
+ return NULL;
+
The logic behind such an approach would be to try only once and fail
on subsequent (re)tries. That way, it would also serve as some kind of
a runtime canary test, because the first try should succeed, which may
prove useful in the field.
if (bootdevice_brom_id < ARRAY_SIZE(boot_devices))
bootdevice_ofpath = boot_devices[bootdevice_brom_id];
