Re: [PATCH] rockchip: spl: Cache boot source id for later use
Hi Quentin,
On 2024-03-19 11:19, Quentin Schulz wrote:
> 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 +)
>>board_spl_was_booted_from: brom_bootdevice_id 3 maps to
>> '/spi@ff1d/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@fe33
>>spl_decode_boot_device: could not find udevice for /mmc@fe32
>>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@ff1d/flash@0";
>>stdout-path = "serial2:150n8";
>>u-boot,spl-boot-order = "same-as-spl", "/mmc@fe33",
>> "/mmc@fe32";
>>};
>>
>
> 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?
As you figured out below, this is currently only an issue with SPI flash
because it was only the SPI flash code path that re-used boot source id
after FIT images have been loaded into memory.
>
> """
> 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@fe32'
> 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@fe32";
> stdout-path = "serial0:115200n8";
> u-boot,spl-boot-order = "same-as-spl", "/spi@ff1d/flash@0",
> "/mmc@fe33", "/mmc@fe32";
> };
> """
>
> 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?
>
> mmm
>
> 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@ff1d/flash@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@ff1d/flash@0";
> stdout-path = "serial0:115200n8";
> u-boot,spl-boot-order = "same-as-spl", "/spi@ff1d/flash@0",
> "/mmc@fe33", "/mmc@fe32";
> };
> """
>
> 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
Re: [PATCH] rockchip: spl: Cache boot source id for later use
On 2024-03-19 16:59, Jonas Karlman wrote:
On 2024-03-19 10:44, Dragan Simic wrote:
On 2024-03-15 18:34, Jonas Karlman wrote:
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 we initialize the static variable to a value it will be stored in
the
.data section and takes up binary size. With a static variable like in
this patch this is instead stored in .bss section and gets initialized
to 0 by runtime. 0 is also not a valid boot source id value and the
reset value for the reg.
Is it mandatory that this static variable ends up in the .bss section?
I do not see any point in making the code more complex than it has to
be.
The reg will contain a known boot source id, 1 - 10, set by BROM or
something has gone wrong and the value is not usable any way.
As I already described, making the code more complex would intentionally
introduce a failure point, which would be triggered in case obtaining
valid value from readl() fails the first time. Something like that is
usually known as a canary, which I'm sure you already know about.
Re: [PATCH] rockchip: spl: Cache boot source id for later use
Hi Dragan,
On 2024-03-19 10:44, Dragan Simic wrote:
> 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.
Thanks, will try to incorporate the grammatical fixes in a v2.
>
>> 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 +)
>> board_spl_was_booted_from: brom_bootdevice_id 3 maps to
>> '/spi@ff1d/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@fe33
>> spl_decode_boot_device: could not find udevice for /mmc@fe32
>> 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@ff1d/flash@0";
>> stdout-path = "serial2:150n8";
>> u-boot,spl-boot-order = "same-as-spl", "/mmc@fe33",
>> "/mmc@fe32";
>> };
>>
>> Signed-off-by: Jonas Karlman
>> ---
>> 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 we initialize the static variable to a value it will be stored in the
.data section and takes up binary size. With a static variable like in
this patch this is instead stored in .bss section and gets initialized
to 0 by runtime. 0 is also not a valid boot source id value and the
reset value for the reg.
I do not see any point in making the code more complex than it has to be.
The reg will contain a known boot source id, 1 - 10, set by BROM or
something has gone wrong and the value is not usable any way.
Regards,
Jonas
>
>> if (bootdevice_brom_id < ARRAY_SIZE(boot_devices))
>> bootdevice_ofpath = boot_devices[bootdevice_brom_id];
Re: [PATCH] rockchip: spl: Cache boot source id for later use
Hello Quentin,
On 2024-03-19 11:19, Quentin Schulz wrote:
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 +)
board_spl_was_booted_from: brom_bootdevice_id 3 maps to
'/spi@ff1d/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@fe33
spl_decode_boot_device: could not find udevice for /mmc@fe32
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@ff1d/flash@0";
stdout-path = "serial2:150n8";
u-boot,spl-boot-order = "same-as-spl", "/mmc@fe33",
"/mmc@fe32";
};
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@fe32'
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@fe32";
stdout-path = "serial0:115200n8";
u-boot,spl-boot-order = "same-as-spl", "/spi@ff1d/flash@0",
"/mmc@fe33", "/mmc@fe32";
};
"""
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?
mmm
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@ff1d/flash@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@ff1d/flash@0";
stdout-path = "serial0:115200n8";
u-boot,spl-boot-order = "same-as-spl", "/spi@ff1d/flash@0",
"/mmc@fe33", "/mmc@fe32";
};
"""
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 :) ).
This was a very nice read, thanks for writing it down in detail! :)
Can you please add:
Fixes:
Re: [PATCH] rockchip: spl: Cache boot source id for later use
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 +)
board_spl_was_booted_from: brom_bootdevice_id 3 maps to
'/spi@ff1d/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@fe33
spl_decode_boot_device: could not find udevice for /mmc@fe32
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@ff1d/flash@0";
stdout-path = "serial2:150n8";
u-boot,spl-boot-order = "same-as-spl", "/mmc@fe33", "/mmc@fe32";
};
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@fe32'
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@fe32";
stdout-path = "serial0:115200n8";
u-boot,spl-boot-order = "same-as-spl", "/spi@ff1d/flash@0",
"/mmc@fe33", "/mmc@fe32";
};
"""
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?
mmm
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@ff1d/flash@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@ff1d/flash@0";
stdout-path = "serial0:115200n8";
u-boot,spl-boot-order = "same-as-spl", "/spi@ff1d/flash@0",
"/mmc@fe33", "/mmc@fe32";
};
"""
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
Re: [PATCH] rockchip: spl: Cache boot source id for later use
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 +)
board_spl_was_booted_from: brom_bootdevice_id 3 maps to
'/spi@ff1d/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@fe33
spl_decode_boot_device: could not find udevice for /mmc@fe32
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@ff1d/flash@0";
stdout-path = "serial2:150n8";
u-boot,spl-boot-order = "same-as-spl", "/mmc@fe33",
"/mmc@fe32";
};
Signed-off-by: Jonas Karlman
---
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];
Re: [PATCH] rockchip: spl: Cache boot source id for later use
On 2024/3/16 01: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 +)
board_spl_was_booted_from: brom_bootdevice_id 3 maps to
'/spi@ff1d/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@fe33
spl_decode_boot_device: could not find udevice for /mmc@fe32
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@ff1d/flash@0";
stdout-path = "serial2:150n8";
u-boot,spl-boot-order = "same-as-spl", "/mmc@fe33", "/mmc@fe32";
};
Signed-off-by: Jonas Karlman
Reviewed-by: Kever Yang
Thanks,
- Kever
---
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;
+ }
+
if (bootdevice_brom_id < ARRAY_SIZE(boot_devices))
bootdevice_ofpath = boot_devices[bootdevice_brom_id];
