Hi Simon,
On 2025-02-09 15:27, Simon Glass wrote:
> Hi Jonas,
>
> On Wed, 5 Feb 2025 at 09:06, Jonas Karlman <jo...@kwiboo.se> wrote:
>>
>> Hi Simon,
>>
>> On 2025-02-05 02:55, Simon Glass wrote:
>>> Add support for this new phase, which runs after TPL. It determines the
>>> state of the machine, then selects which SPL image to use. SDRAM init is
>>> then done in SPL, so that it is updatable.
>>
>> I am still unsure how this makes it more updatable, what happen when
>> DRAM init fails and board freezes? Are we relying on e.g. a watchdog to
>> properly reset a frozen board and try next image?
>
> Yes, that's right. Also, when you do an upgrade you are not changing
> the only copy, thus avoiding bricking devices with a bad or partial
> update.
Yes, it is unfortunate that the older Rockchip SoCs BootROM does not do
hash/checksum validation, thankfully the newer SoCs does, so for newer
SoCs it is more likely that bad code brick your device instead of a bad
or partial update.
Also, Rockchips "Flash Open Source Solution Developer Guide" even
recommend to write multiple idblock headers to flash storage, this may
be something that we should consider. Typically, Rockchip BootROM will
check 5 positions in MMC for a valid idblock.
Is there some code that configures a watchdog or similar that can help
unfreeze due to a bad code update using this?
Regards,
Jonas
>
>>
>>>
>>> Signed-off-by: Simon Glass <s...@chromium.org>
>>> ---
>>>
>>> arch/arm/include/asm/spl.h | 1 +
>>> arch/arm/mach-rockchip/Kconfig | 25 +++++-
>>> arch/arm/mach-rockchip/Makefile | 6 +-
>>> arch/arm/mach-rockchip/rk3399/Kconfig | 9 ++
>>> arch/arm/mach-rockchip/spl-boot-order.c | 3 +-
>>> arch/arm/mach-rockchip/spl.c | 3 +
>>> arch/arm/mach-rockchip/tpl.c | 2 +-
>>> arch/arm/mach-rockchip/u-boot-vpl-v8.lds | 107 +++++++++++++++++++++++
>>> arch/arm/mach-rockchip/vpl.c | 53 +++++++++++
>>> common/spl/Kconfig | 1 +
>>> 10 files changed, 203 insertions(+), 7 deletions(-)
>>> create mode 100644 arch/arm/mach-rockchip/u-boot-vpl-v8.lds
>>> create mode 100644 arch/arm/mach-rockchip/vpl.c
>>>
>>
>> [snip]
>>
>>> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
>>> index 269c219a6f8..7e6e9411e79 100644
>>> --- a/arch/arm/mach-rockchip/Kconfig
>>> +++ b/arch/arm/mach-rockchip/Kconfig
>>
>> [snip]
>>
>>> @@ -455,7 +459,7 @@ config SPL_ROCKCHIP_BACK_TO_BROM
>>>
>>> config TPL_ROCKCHIP_BACK_TO_BROM
>>> bool "TPL returns to bootrom"
>>> - default y
>>> + default y if !VPL
>>> select ROCKCHIP_BROM_HELPER if !ROCKCHIP_RK3066
>>> select TPL_BOOTROM_SUPPORT
>>> depends on TPL
>>> @@ -496,6 +500,16 @@ config ROCKCHIP_EXTERNAL_TPL
>>> Enable this option and build with ROCKCHIP_TPL=/path/to/ddr.bin to
>>> include the external TPL in the image built by binman.
>>>
>>> +config VPL_ROCKCHIP_COMMON_BOARD
>>> + bool "Rockchip VPL common board file"
>>> + depends on VPL
>>> + default y
>>> + help
>>> + Rockchip SoCs have similar boot process, prefer to use TPL to start
>>> + VPL, then ATF for DRAM init and back to bootrom, and SPL as Trust
>>> + ATF/U-Boot loader. VPL common board is a basic VPL board init which
>>> + can be shared for most SoCs to avoid copy-paste for different SoCs.
>>
>> This help text is not correct, with VPL=y back to bootrom is not used
>> and TF-A is never used for DRAM init, only TPL/SPL or Rockchip TPL.
>
> OK I copied it from above. I'll rewrite it.
>
>>
>>> +
>>> config ROCKCHIP_BOOT_MODE_REG
>>> hex "Rockchip boot mode flag register address"
>>> help
>>
>> [snip]
>>
>>> diff --git a/arch/arm/mach-rockchip/spl-boot-order.c
>>> b/arch/arm/mach-rockchip/spl-boot-order.c
>>> index 3dce9b30898..d580d4fc84c 100644
>>> --- a/arch/arm/mach-rockchip/spl-boot-order.c
>>> +++ b/arch/arm/mach-rockchip/spl-boot-order.c
>>> @@ -99,7 +99,8 @@ __weak const char *board_spl_was_booted_from(void)
>>> void board_boot_order(u32 *spl_boot_list)
>>> {
>>> /* In case of no fdt (or only plat), use spl_boot_device() */
>>> - if (!CONFIG_IS_ENABLED(OF_CONTROL) || CONFIG_IS_ENABLED(OF_PLATDATA))
>>> {
>>> + if (IS_ENABLED(CONFIG_VPL) || !CONFIG_IS_ENABLED(OF_CONTROL) ||
>>> + CONFIG_IS_ENABLED(OF_PLATDATA)) {
>>> spl_boot_list[0] = spl_boot_device();
>>> return;
>>> }
>>
>> Another option could be to just skip spl-boot-order.o for VPL, the
>> default implementation should work as-is, not sure what is best.
>
> OK, I'll do that, thanks.
>
> Regards,
> Simon
