diff mbox series

[RFC] rockchip: Reset to bootrom download mode on hang

Message ID 20240202001221.531207-1-jonas@kwiboo.se
State RFC
Delegated to: Kever Yang
Headers show
Series [RFC] rockchip: Reset to bootrom download mode on hang | expand

Commit Message

Jonas Karlman Feb. 2, 2024, 12:12 a.m. UTC
Add support to reset to bootrom download mode on hang in U-Boot SPL and
proper. ROCKCHIP_HANG_TO_BROM can be used to enable this feature.

Example when SPL cannot load FIT:

  U-Boot SPL 2024.04-rc1 (Feb 01 2024 - 23:01:12 +0000)
  Trying to boot from MMC1
  mmc_load_image_raw_sector: mmc block read error
  Trying to boot from MMC2
  Card did not respond to voltage select! : -110
  spl: mmc init failed with error: -95
  Trying to boot from MMC1
  mmc_load_image_raw_sector: mmc block read error
  SPL: failed to boot from all boot devices
  ### ERROR ### Please RESET the board ###
  entering download mode ...
  resetting ...

Procedure to start bootrom download mode:
- U-Boot SPL or proper write 0xEF08A53C to BOOT_MODE_REG and then reset
- Bootrom loads and run boot code (TPL) from e.g. SPI > eMMC > SD-card
- TPL check for 0xEF08A53C in BOOT_MODE_REG very early, i.e. Rockchip
  TPL blobs check for this value directly at start
- TPL return to bootrom with a return value != 0
- Bootrom enter download mode

This also fixes an issue where the BOOT_MODE_REG is reset to 0 when
board is reset on RK35xx after TF-A has been loaded. To fix this the
SOC_CON1 reg value is reset prior to issuing a global reset.

The RK356x TF-A blobs will clear SOC_CON1 as part of a PSCI reset,
however the RK3588 TF-A blobs does not seem to clear SOC_CON1.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
- Is reset to bootrom download mode on hang a feature of intereset to
  anyone else?
- How can we best handle the issue with BOOT_MODE_REG being reset?
- Should we instead enable SYSRESET_PSCI and relay on TF-A to properly
  reset without loosing value in BOOT_MODE_REG? Currently works on
  RK356x bl31 blobs but not on the RK3588 bl31 blobs.
---
 arch/arm/mach-rockchip/Kconfig     |  9 +++++++
 arch/arm/mach-rockchip/Makefile    |  9 ++++---
 arch/arm/mach-rockchip/boot_mode.c | 38 ++++++++++++++++++++++++++----
 configs/generic-rk3568_defconfig   |  1 +
 configs/generic-rk3588_defconfig   |  1 +
 5 files changed, 48 insertions(+), 10 deletions(-)

Comments

Dragan Simic Feb. 2, 2024, 4:26 a.m. UTC | #1
Hello Jonas,

On 2024-02-02 01:12, Jonas Karlman wrote:
> Add support to reset to bootrom download mode on hang in U-Boot SPL and
> proper. ROCKCHIP_HANG_TO_BROM can be used to enable this feature.
> 
> Example when SPL cannot load FIT:
> 
>   U-Boot SPL 2024.04-rc1 (Feb 01 2024 - 23:01:12 +0000)
>   Trying to boot from MMC1
>   mmc_load_image_raw_sector: mmc block read error
>   Trying to boot from MMC2
>   Card did not respond to voltage select! : -110
>   spl: mmc init failed with error: -95
>   Trying to boot from MMC1
>   mmc_load_image_raw_sector: mmc block read error
>   SPL: failed to boot from all boot devices
>   ### ERROR ### Please RESET the board ###
>   entering download mode ...
>   resetting ...
> 
> Procedure to start bootrom download mode:
> - U-Boot SPL or proper write 0xEF08A53C to BOOT_MODE_REG and then reset
> - Bootrom loads and run boot code (TPL) from e.g. SPI > eMMC > SD-card
> - TPL check for 0xEF08A53C in BOOT_MODE_REG very early, i.e. Rockchip
>   TPL blobs check for this value directly at start
> - TPL return to bootrom with a return value != 0
> - Bootrom enter download mode
> 
> This also fixes an issue where the BOOT_MODE_REG is reset to 0 when
> board is reset on RK35xx after TF-A has been loaded. To fix this the
> SOC_CON1 reg value is reset prior to issuing a global reset.
> 
> The RK356x TF-A blobs will clear SOC_CON1 as part of a PSCI reset,
> however the RK3588 TF-A blobs does not seem to clear SOC_CON1.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> - Is reset to bootrom download mode on hang a feature of intereset to
>   anyone else?

I find this usable, for example during development.  Thus, I'd support
this feature to be polished further and eventually merged.

> - How can we best handle the issue with BOOT_MODE_REG being reset?
> - Should we instead enable SYSRESET_PSCI and relay on TF-A to properly
>   reset without loosing value in BOOT_MODE_REG? Currently works on
>   RK356x bl31 blobs but not on the RK3588 bl31 blobs.
> ---
>  arch/arm/mach-rockchip/Kconfig     |  9 +++++++
>  arch/arm/mach-rockchip/Makefile    |  9 ++++---
>  arch/arm/mach-rockchip/boot_mode.c | 38 ++++++++++++++++++++++++++----
>  configs/generic-rk3568_defconfig   |  1 +
>  configs/generic-rk3588_defconfig   |  1 +
>  5 files changed, 48 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/mach-rockchip/Kconfig 
> b/arch/arm/mach-rockchip/Kconfig
> index 6ff0aa6911e2..66556d30af27 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -508,6 +508,15 @@ config TPL_ROCKCHIP_EARLYRETURN_TO_BROM
>  	  This enables support code in the BOOT0 hook for the TPL stage
>  	  to allow multiple entries.
> 
> +config ROCKCHIP_HANG_TO_BROM
> +	bool "Reset to bootrom download mode on hang"
> +	select SHOW_BOOT_PROGRESS
> +
> +config SPL_ROCKCHIP_HANG_TO_BROM
> +	bool "Reset to bootrom download mode on hang in SPL"
> +	default y if ROCKCHIP_HANG_TO_BROM
> +	select SPL_SHOW_BOOT_PROGRESS
> +
>  config SPL_MMC
>  	default y if !SPL_ROCKCHIP_BACK_TO_BROM
> 
> diff --git a/arch/arm/mach-rockchip/Makefile 
> b/arch/arm/mach-rockchip/Makefile
> index 1dc92066bbf3..ab4446bb6b51 100644
> --- a/arch/arm/mach-rockchip/Makefile
> +++ b/arch/arm/mach-rockchip/Makefile
> @@ -16,17 +16,16 @@ obj-tpl-$(CONFIG_ROCKCHIP_PX30) += px30-board-tpl.o
>  obj-spl-$(CONFIG_ROCKCHIP_RK3036) += rk3036-board-spl.o
> 
>  ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
> +obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o
> +obj-$(CONFIG_MISC_INIT_R) += misc.o
> +endif
> 
> +ifeq ($(CONFIG_TPL_BUILD),)
>  # Always include boot_mode.o, as we bypass it (i.e. turn it off)
>  # inside of boot_mode.c when CONFIG_ROCKCHIP_BOOT_MODE_REG is 0.  This 
> way,
>  # we can have the preprocessor correctly recognise both 0x0 and 0
>  # meaning "turn it off".
>  obj-y += boot_mode.o
> -obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o
> -obj-$(CONFIG_MISC_INIT_R) += misc.o
> -endif
> -
> -ifeq ($(CONFIG_TPL_BUILD),)
>  obj-$(CONFIG_DISPLAY_CPUINFO) += cpu-info.o
>  endif
> 
> diff --git a/arch/arm/mach-rockchip/boot_mode.c
> b/arch/arm/mach-rockchip/boot_mode.c
> index eb8f65ae4e9d..de9ec1414926 100644
> --- a/arch/arm/mach-rockchip/boot_mode.c
> +++ b/arch/arm/mach-rockchip/boot_mode.c
> @@ -5,6 +5,7 @@
> 
>  #include <common.h>
>  #include <adc.h>
> +#include <bootstage.h>
>  #include <command.h>
>  #include <env.h>
>  #include <log.h>
> @@ -23,11 +24,31 @@ int setup_boot_mode(void)
> 
>  #else
> 
> -void set_back_to_bootrom_dnl_flag(void)
> +static void set_back_to_bootrom_dnl_flag(void)
>  {
> +	/*
> +	 * These SOC_CON1 regs needs to be cleared before a reset or the
> +	 * BOOT_MODE_REG do not retain its value and it is not possible
> +	 * to reset to bootrom download mode once TF-A has been started.
> +	 *
> +	 * TF-A blobs for RK3568 already clear SOC_CON1 for PSCI reset.
> +	 * However, the TF-A blobs for RK3588 does not clear SOC_CON1.
> +	 */
> +	if (IS_ENABLED(CONFIG_ROCKCHIP_RK3568))
> +		writel(0x40000, 0xFDC20104);
> +	if (IS_ENABLED(CONFIG_ROCKCHIP_RK3588))
> +		writel(0xFFFF0000, 0xFD58A004);
> +
>  	writel(BOOT_BROM_DOWNLOAD, CONFIG_ROCKCHIP_BOOT_MODE_REG);
>  }
> 
> +static void rockchip_reset_to_dnl_mode(void)
> +{
> +	puts("entering download mode ...\n");
> +	set_back_to_bootrom_dnl_flag();
> +	do_reset(NULL, 0, 0, NULL);
> +}
> +
>  /*
>   * detect download key status by adc, most rockchip
>   * based boards use adc sample the download key status,
> @@ -71,12 +92,19 @@ __weak int rockchip_dnl_key_pressed(void)
>  		return false;
>  }
> 
> -void rockchip_dnl_mode_check(void)
> +#if CONFIG_IS_ENABLED(ROCKCHIP_HANG_TO_BROM)
> +void show_boot_progress(int val)
> +{
> +	if (val == -BOOTSTAGE_ID_NEED_RESET)
> +		rockchip_reset_to_dnl_mode();
> +}
> +#endif
> +
> +static void rockchip_dnl_mode_check(void)
>  {
>  	if (rockchip_dnl_key_pressed()) {
> -		printf("download key pressed, entering download mode...");
> -		set_back_to_bootrom_dnl_flag();
> -		do_reset(NULL, 0, 0, NULL);
> +		puts("download key pressed, ");
> +		rockchip_reset_to_dnl_mode();
>  	}
>  }
> 
> diff --git a/configs/generic-rk3568_defconfig 
> b/configs/generic-rk3568_defconfig
> index b90ecb643dc7..9edd4c33000e 100644
> --- a/configs/generic-rk3568_defconfig
> +++ b/configs/generic-rk3568_defconfig
> @@ -11,6 +11,7 @@ CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xc00000
>  CONFIG_DEFAULT_DEVICE_TREE="rk3568-generic"
>  CONFIG_ROCKCHIP_RK3568=y
>  CONFIG_SPL_ROCKCHIP_COMMON_BOARD=y
> +CONFIG_ROCKCHIP_HANG_TO_BROM=y
>  CONFIG_SPL_SERIAL=y
>  CONFIG_SPL_STACK_R_ADDR=0x600000
>  CONFIG_SPL_STACK=0x400000
> diff --git a/configs/generic-rk3588_defconfig 
> b/configs/generic-rk3588_defconfig
> index a9329eb1c625..5872bd4802c5 100644
> --- a/configs/generic-rk3588_defconfig
> +++ b/configs/generic-rk3588_defconfig
> @@ -11,6 +11,7 @@ CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xc00000
>  CONFIG_DEFAULT_DEVICE_TREE="rk3588-generic"
>  CONFIG_ROCKCHIP_RK3588=y
>  CONFIG_SPL_ROCKCHIP_COMMON_BOARD=y
> +CONFIG_ROCKCHIP_HANG_TO_BROM=y
>  CONFIG_SPL_SERIAL=y
>  CONFIG_SPL_STACK_R_ADDR=0x600000
>  CONFIG_TARGET_EVB_RK3588=y
Quentin Schulz Feb. 5, 2024, 10:49 a.m. UTC | #2
Hi Jonas,

On 2/2/24 01:12, Jonas Karlman wrote:
> Add support to reset to bootrom download mode on hang in U-Boot SPL and
> proper. ROCKCHIP_HANG_TO_BROM can be used to enable this feature.
> 
> Example when SPL cannot load FIT:
> 
>    U-Boot SPL 2024.04-rc1 (Feb 01 2024 - 23:01:12 +0000)
>    Trying to boot from MMC1
>    mmc_load_image_raw_sector: mmc block read error
>    Trying to boot from MMC2
>    Card did not respond to voltage select! : -110
>    spl: mmc init failed with error: -95
>    Trying to boot from MMC1
>    mmc_load_image_raw_sector: mmc block read error
>    SPL: failed to boot from all boot devices
>    ### ERROR ### Please RESET the board ###
>    entering download mode ...
>    resetting ...
> 
> Procedure to start bootrom download mode:
> - U-Boot SPL or proper write 0xEF08A53C to BOOT_MODE_REG and then reset
> - Bootrom loads and run boot code (TPL) from e.g. SPI > eMMC > SD-card
> - TPL check for 0xEF08A53C in BOOT_MODE_REG very early, i.e. Rockchip
>    TPL blobs check for this value directly at start
> - TPL return to bootrom with a return value != 0
> - Bootrom enter download mode
> 
> This also fixes an issue where the BOOT_MODE_REG is reset to 0 when
> board is reset on RK35xx after TF-A has been loaded. To fix this the
> SOC_CON1 reg value is reset prior to issuing a global reset.
> 
> The RK356x TF-A blobs will clear SOC_CON1 as part of a PSCI reset,
> however the RK3588 TF-A blobs does not seem to clear SOC_CON1.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>

I'm wondering if there isn't a simpler way to do this?

board_boot_order() could parse u-boot,spl-boot-order for e.g. "bootrom", 
the same way it does for "same-as-spl" for example.

If it is set, then add BOOT_DEVICE_BOOTROM to spl_boot_list.

Then this would call spl_return_to_bootrom() from 
common/spl/spl_bootrom.c which in turns would call 
board_return_to_bootrom() and back_to_bootrom().

What do you think?

Cheers,
Quentin
Jonas Karlman Feb. 5, 2024, 12:34 p.m. UTC | #3
Hi Quentin,

On 2024-02-05 11:49, Quentin Schulz wrote:
> Hi Jonas,
> 
> On 2/2/24 01:12, Jonas Karlman wrote:
>> Add support to reset to bootrom download mode on hang in U-Boot SPL and
>> proper. ROCKCHIP_HANG_TO_BROM can be used to enable this feature.
>>
>> Example when SPL cannot load FIT:
>>
>>    U-Boot SPL 2024.04-rc1 (Feb 01 2024 - 23:01:12 +0000)
>>    Trying to boot from MMC1
>>    mmc_load_image_raw_sector: mmc block read error
>>    Trying to boot from MMC2
>>    Card did not respond to voltage select! : -110
>>    spl: mmc init failed with error: -95
>>    Trying to boot from MMC1
>>    mmc_load_image_raw_sector: mmc block read error
>>    SPL: failed to boot from all boot devices
>>    ### ERROR ### Please RESET the board ###
>>    entering download mode ...
>>    resetting ...
>>
>> Procedure to start bootrom download mode:
>> - U-Boot SPL or proper write 0xEF08A53C to BOOT_MODE_REG and then reset
>> - Bootrom loads and run boot code (TPL) from e.g. SPI > eMMC > SD-card
>> - TPL check for 0xEF08A53C in BOOT_MODE_REG very early, i.e. Rockchip
>>    TPL blobs check for this value directly at start
>> - TPL return to bootrom with a return value != 0
>> - Bootrom enter download mode
>>
>> This also fixes an issue where the BOOT_MODE_REG is reset to 0 when
>> board is reset on RK35xx after TF-A has been loaded. To fix this the
>> SOC_CON1 reg value is reset prior to issuing a global reset.
>>
>> The RK356x TF-A blobs will clear SOC_CON1 as part of a PSCI reset,
>> however the RK3588 TF-A blobs does not seem to clear SOC_CON1.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> 
> I'm wondering if there isn't a simpler way to do this?
> 
> board_boot_order() could parse u-boot,spl-boot-order for e.g. "bootrom", 
> the same way it does for "same-as-spl" for example.
> 
> If it is set, then add BOOT_DEVICE_BOOTROM to spl_boot_list.
> 
> Then this would call spl_return_to_bootrom() from 
> common/spl/spl_bootrom.c which in turns would call 
> board_return_to_bootrom() and back_to_bootrom().
> 
> What do you think?

Something like that is what I did in my initial implementation :-)

I appended a BOOT_DEVICE_BOOTROM at the end of board_boot_order() and
implemented necessary parts in spl_return_to_bootrom(), and that worked
great for SPL.

However, to catch a hang() in U-Boot proper, e.g. a failed initcall, the
only existing integration callback for hang() is to override
show_boot_progress().

Because the use of show_boot_progress() worked for both SPL and U-Boot
proper (probably also TPL) it seemed better to only have one integration
part for all phases.

I am still not sure exactly when we would want to fall back into bootrom
download mode. Always at a hang() or at controlled parts e.g. when no
FIT can be loaded.

I am open for any suggestions.

Regards,
Jonas

> 
> Cheers,
> Quentin
Eugen Hristev Feb. 5, 2024, 3:49 p.m. UTC | #4
Hello Jonas,

On 2/5/24 14:34, Jonas Karlman wrote:
> Hi Quentin,
> 
> On 2024-02-05 11:49, Quentin Schulz wrote:
>> Hi Jonas,
>>
>> On 2/2/24 01:12, Jonas Karlman wrote:
>>> Add support to reset to bootrom download mode on hang in U-Boot SPL and
>>> proper. ROCKCHIP_HANG_TO_BROM can be used to enable this feature.
>>>
>>> Example when SPL cannot load FIT:
>>>
>>>    U-Boot SPL 2024.04-rc1 (Feb 01 2024 - 23:01:12 +0000)
>>>    Trying to boot from MMC1
>>>    mmc_load_image_raw_sector: mmc block read error
>>>    Trying to boot from MMC2
>>>    Card did not respond to voltage select! : -110
>>>    spl: mmc init failed with error: -95
>>>    Trying to boot from MMC1
>>>    mmc_load_image_raw_sector: mmc block read error
>>>    SPL: failed to boot from all boot devices
>>>    ### ERROR ### Please RESET the board ###
>>>    entering download mode ...
>>>    resetting ...
>>>
>>> Procedure to start bootrom download mode:
>>> - U-Boot SPL or proper write 0xEF08A53C to BOOT_MODE_REG and then reset
>>> - Bootrom loads and run boot code (TPL) from e.g. SPI > eMMC > SD-card
>>> - TPL check for 0xEF08A53C in BOOT_MODE_REG very early, i.e. Rockchip
>>>    TPL blobs check for this value directly at start
>>> - TPL return to bootrom with a return value != 0
>>> - Bootrom enter download mode
>>>
>>> This also fixes an issue where the BOOT_MODE_REG is reset to 0 when
>>> board is reset on RK35xx after TF-A has been loaded. To fix this the
>>> SOC_CON1 reg value is reset prior to issuing a global reset.
>>>
>>> The RK356x TF-A blobs will clear SOC_CON1 as part of a PSCI reset,
>>> however the RK3588 TF-A blobs does not seem to clear SOC_CON1.

Recently I tested BL31 with TF-A, it works, and you can send a patch to fix bugs in
there if you find them !
Its open source, I gathered the patches and tested them here:

https://gitlab.collabora.com/hardware-enablement/rockchip-3588/trusted-firmware-a

>>>
>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>
>> I'm wondering if there isn't a simpler way to do this?
>>
>> board_boot_order() could parse u-boot,spl-boot-order for e.g. "bootrom", 
>> the same way it does for "same-as-spl" for example.
>>
>> If it is set, then add BOOT_DEVICE_BOOTROM to spl_boot_list.
>>
>> Then this would call spl_return_to_bootrom() from 
>> common/spl/spl_bootrom.c which in turns would call 
>> board_return_to_bootrom() and back_to_bootrom().
>>
>> What do you think?
> 
> Something like that is what I did in my initial implementation :-)
> 
> I appended a BOOT_DEVICE_BOOTROM at the end of board_boot_order() and
> implemented necessary parts in spl_return_to_bootrom(), and that worked
> great for SPL.
> 
> However, to catch a hang() in U-Boot proper, e.g. a failed initcall, the
> only existing integration callback for hang() is to override
> show_boot_progress().
> 
> Because the use of show_boot_progress() worked for both SPL and U-Boot
> proper (probably also TPL) it seemed better to only have one integration
> part for all phases.
> 
> I am still not sure exactly when we would want to fall back into bootrom
> download mode. Always at a hang() or at controlled parts e.g. when no
> FIT can be loaded.
> 
> I am open for any suggestions.

I implemented DFU in SPL at some point last year (due to DWC3 being unaligned with
Linux the patches weren't accepted), and defaulting to DFU is also an option when
nothing boots.

(
https://patchwork.ozlabs.org/project/uboot/cover/20230801072811.10354-1-eugen.hristev@collabora.com/#3185956
)

Eugen
> 
> Regards,
> Jonas
> 
>>
>> Cheers,
>> Quentin
>
Kever Yang Feb. 6, 2024, 2:25 a.m. UTC | #5
Hi Jonas,

On 2024/2/5 20:34, Jonas Karlman wrote:
> Hi Quentin,
>
> On 2024-02-05 11:49, Quentin Schulz wrote:
>> Hi Jonas,
>>
>> On 2/2/24 01:12, Jonas Karlman wrote:
>>> Add support to reset to bootrom download mode on hang in U-Boot SPL and
>>> proper. ROCKCHIP_HANG_TO_BROM can be used to enable this feature.
>>>
>>> Example when SPL cannot load FIT:
>>>
>>>     U-Boot SPL 2024.04-rc1 (Feb 01 2024 - 23:01:12 +0000)
>>>     Trying to boot from MMC1
>>>     mmc_load_image_raw_sector: mmc block read error
>>>     Trying to boot from MMC2
>>>     Card did not respond to voltage select! : -110
>>>     spl: mmc init failed with error: -95
>>>     Trying to boot from MMC1
>>>     mmc_load_image_raw_sector: mmc block read error
>>>     SPL: failed to boot from all boot devices
>>>     ### ERROR ### Please RESET the board ###
>>>     entering download mode ...
>>>     resetting ...
>>>
>>> Procedure to start bootrom download mode:
>>> - U-Boot SPL or proper write 0xEF08A53C to BOOT_MODE_REG and then reset
>>> - Bootrom loads and run boot code (TPL) from e.g. SPI > eMMC > SD-card
>>> - TPL check for 0xEF08A53C in BOOT_MODE_REG very early, i.e. Rockchip
>>>     TPL blobs check for this value directly at start
>>> - TPL return to bootrom with a return value != 0
>>> - Bootrom enter download mode
>>>
>>> This also fixes an issue where the BOOT_MODE_REG is reset to 0 when
>>> board is reset on RK35xx after TF-A has been loaded. To fix this the
>>> SOC_CON1 reg value is reset prior to issuing a global reset.
>>>
>>> The RK356x TF-A blobs will clear SOC_CON1 as part of a PSCI reset,
>>> however the RK3588 TF-A blobs does not seem to clear SOC_CON1.
>>>
>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> I'm wondering if there isn't a simpler way to do this?
>>
>> board_boot_order() could parse u-boot,spl-boot-order for e.g. "bootrom",
>> the same way it does for "same-as-spl" for example.
>>
>> If it is set, then add BOOT_DEVICE_BOOTROM to spl_boot_list.
>>
>> Then this would call spl_return_to_bootrom() from
>> common/spl/spl_bootrom.c which in turns would call
>> board_return_to_bootrom() and back_to_bootrom().
>>
>> What do you think?
> Something like that is what I did in my initial implementation :-)
>
> I appended a BOOT_DEVICE_BOOTROM at the end of board_boot_order() and
> implemented necessary parts in spl_return_to_bootrom(), and that worked
> great for SPL.
>
> However, to catch a hang() in U-Boot proper, e.g. a failed initcall, the
> only existing integration callback for hang() is to override
> show_boot_progress().
>
> Because the use of show_boot_progress() worked for both SPL and U-Boot
> proper (probably also TPL) it seemed better to only have one integration
> part for all phases.
>
> I am still not sure exactly when we would want to fall back into bootrom
> download mode. Always at a hang() or at controlled parts e.g. when no
> FIT can be loaded.
There are two case need to consider when hang() happen:
- For release version: a reboot and get into reset into bootrom download 
mode if boot fail more than 3 times;
- For debug version: a dump stack with hang() will be better(which is 
used in rockchip vendor u-boot);
Reboot to bootrom download when hang() happen is handy for U-Boot error 
happen, no need to press the reset
and recovery/download key in this case.
The final logic would be: fall back to usb boot(bootrom) whenever we 
fail to boot in to next stage,
so that we have chance to update an available firmware.

Thanks,
- Kever
>
> I am open for any suggestions.
>
> Regards,
> Jonas
>
>> Cheers,
>> Quentin
Dragan Simic Feb. 6, 2024, 3:41 a.m. UTC | #6
On 2024-02-06 03:25, Kever Yang wrote:
> On 2024/2/5 20:34, Jonas Karlman wrote:
>> On 2024-02-05 11:49, Quentin Schulz wrote:
>>> On 2/2/24 01:12, Jonas Karlman wrote:
>>>> Add support to reset to bootrom download mode on hang in U-Boot SPL 
>>>> and
>>>> proper. ROCKCHIP_HANG_TO_BROM can be used to enable this feature.
>>>> 
>>>> Example when SPL cannot load FIT:
>>>> 
>>>>     U-Boot SPL 2024.04-rc1 (Feb 01 2024 - 23:01:12 +0000)
>>>>     Trying to boot from MMC1
>>>>     mmc_load_image_raw_sector: mmc block read error
>>>>     Trying to boot from MMC2
>>>>     Card did not respond to voltage select! : -110
>>>>     spl: mmc init failed with error: -95
>>>>     Trying to boot from MMC1
>>>>     mmc_load_image_raw_sector: mmc block read error
>>>>     SPL: failed to boot from all boot devices
>>>>     ### ERROR ### Please RESET the board ###
>>>>     entering download mode ...
>>>>     resetting ...
>>>> 
>>>> Procedure to start bootrom download mode:
>>>> - U-Boot SPL or proper write 0xEF08A53C to BOOT_MODE_REG and then 
>>>> reset
>>>> - Bootrom loads and run boot code (TPL) from e.g. SPI > eMMC > 
>>>> SD-card
>>>> - TPL check for 0xEF08A53C in BOOT_MODE_REG very early, i.e. 
>>>> Rockchip
>>>>     TPL blobs check for this value directly at start
>>>> - TPL return to bootrom with a return value != 0
>>>> - Bootrom enter download mode
>>>> 
>>>> This also fixes an issue where the BOOT_MODE_REG is reset to 0 when
>>>> board is reset on RK35xx after TF-A has been loaded. To fix this the
>>>> SOC_CON1 reg value is reset prior to issuing a global reset.
>>>> 
>>>> The RK356x TF-A blobs will clear SOC_CON1 as part of a PSCI reset,
>>>> however the RK3588 TF-A blobs does not seem to clear SOC_CON1.
>>>> 
>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>> I'm wondering if there isn't a simpler way to do this?
>>> 
>>> board_boot_order() could parse u-boot,spl-boot-order for e.g. 
>>> "bootrom",
>>> the same way it does for "same-as-spl" for example.
>>> 
>>> If it is set, then add BOOT_DEVICE_BOOTROM to spl_boot_list.
>>> 
>>> Then this would call spl_return_to_bootrom() from
>>> common/spl/spl_bootrom.c which in turns would call
>>> board_return_to_bootrom() and back_to_bootrom().
>>> 
>>> What do you think?
>> Something like that is what I did in my initial implementation :-)
>> 
>> I appended a BOOT_DEVICE_BOOTROM at the end of board_boot_order() and
>> implemented necessary parts in spl_return_to_bootrom(), and that 
>> worked
>> great for SPL.
>> 
>> However, to catch a hang() in U-Boot proper, e.g. a failed initcall, 
>> the
>> only existing integration callback for hang() is to override
>> show_boot_progress().
>> 
>> Because the use of show_boot_progress() worked for both SPL and U-Boot
>> proper (probably also TPL) it seemed better to only have one 
>> integration
>> part for all phases.
>> 
>> I am still not sure exactly when we would want to fall back into 
>> bootrom
>> download mode. Always at a hang() or at controlled parts e.g. when no
>> FIT can be loaded.
> There are two case need to consider when hang() happen:
> - For release version: a reboot and get into reset into bootrom
> download mode if boot fail more than 3 times;
> - For debug version: a dump stack with hang() will be better(which is
> used in rockchip vendor u-boot);
> Reboot to bootrom download when hang() happen is handy for U-Boot
> error happen, no need to press the reset
> and recovery/download key in this case.
> The final logic would be: fall back to usb boot(bootrom) whenever we
> fail to boot in to next stage,
> so that we have chance to update an available firmware.

Just as a note, falling back to BROM may actually introduce some
security issues in some setups, by allowing rather low-level access
to the system that may not be desired.

Perhaps not enabling the fallback to BROM by default and leaving it
to the board configurations would be the desired way to move forward.
diff mbox series

Patch

diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index 6ff0aa6911e2..66556d30af27 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -508,6 +508,15 @@  config TPL_ROCKCHIP_EARLYRETURN_TO_BROM
 	  This enables support code in the BOOT0 hook for the TPL stage
 	  to allow multiple entries.
 
+config ROCKCHIP_HANG_TO_BROM
+	bool "Reset to bootrom download mode on hang"
+	select SHOW_BOOT_PROGRESS
+
+config SPL_ROCKCHIP_HANG_TO_BROM
+	bool "Reset to bootrom download mode on hang in SPL"
+	default y if ROCKCHIP_HANG_TO_BROM
+	select SPL_SHOW_BOOT_PROGRESS
+
 config SPL_MMC
 	default y if !SPL_ROCKCHIP_BACK_TO_BROM
 
diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
index 1dc92066bbf3..ab4446bb6b51 100644
--- a/arch/arm/mach-rockchip/Makefile
+++ b/arch/arm/mach-rockchip/Makefile
@@ -16,17 +16,16 @@  obj-tpl-$(CONFIG_ROCKCHIP_PX30) += px30-board-tpl.o
 obj-spl-$(CONFIG_ROCKCHIP_RK3036) += rk3036-board-spl.o
 
 ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
+obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o
+obj-$(CONFIG_MISC_INIT_R) += misc.o
+endif
 
+ifeq ($(CONFIG_TPL_BUILD),)
 # Always include boot_mode.o, as we bypass it (i.e. turn it off)
 # inside of boot_mode.c when CONFIG_ROCKCHIP_BOOT_MODE_REG is 0.  This way,
 # we can have the preprocessor correctly recognise both 0x0 and 0
 # meaning "turn it off".
 obj-y += boot_mode.o
-obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o
-obj-$(CONFIG_MISC_INIT_R) += misc.o
-endif
-
-ifeq ($(CONFIG_TPL_BUILD),)
 obj-$(CONFIG_DISPLAY_CPUINFO) += cpu-info.o
 endif
 
diff --git a/arch/arm/mach-rockchip/boot_mode.c b/arch/arm/mach-rockchip/boot_mode.c
index eb8f65ae4e9d..de9ec1414926 100644
--- a/arch/arm/mach-rockchip/boot_mode.c
+++ b/arch/arm/mach-rockchip/boot_mode.c
@@ -5,6 +5,7 @@ 
 
 #include <common.h>
 #include <adc.h>
+#include <bootstage.h>
 #include <command.h>
 #include <env.h>
 #include <log.h>
@@ -23,11 +24,31 @@  int setup_boot_mode(void)
 
 #else
 
-void set_back_to_bootrom_dnl_flag(void)
+static void set_back_to_bootrom_dnl_flag(void)
 {
+	/*
+	 * These SOC_CON1 regs needs to be cleared before a reset or the
+	 * BOOT_MODE_REG do not retain its value and it is not possible
+	 * to reset to bootrom download mode once TF-A has been started.
+	 *
+	 * TF-A blobs for RK3568 already clear SOC_CON1 for PSCI reset.
+	 * However, the TF-A blobs for RK3588 does not clear SOC_CON1.
+	 */
+	if (IS_ENABLED(CONFIG_ROCKCHIP_RK3568))
+		writel(0x40000, 0xFDC20104);
+	if (IS_ENABLED(CONFIG_ROCKCHIP_RK3588))
+		writel(0xFFFF0000, 0xFD58A004);
+
 	writel(BOOT_BROM_DOWNLOAD, CONFIG_ROCKCHIP_BOOT_MODE_REG);
 }
 
+static void rockchip_reset_to_dnl_mode(void)
+{
+	puts("entering download mode ...\n");
+	set_back_to_bootrom_dnl_flag();
+	do_reset(NULL, 0, 0, NULL);
+}
+
 /*
  * detect download key status by adc, most rockchip
  * based boards use adc sample the download key status,
@@ -71,12 +92,19 @@  __weak int rockchip_dnl_key_pressed(void)
 		return false;
 }
 
-void rockchip_dnl_mode_check(void)
+#if CONFIG_IS_ENABLED(ROCKCHIP_HANG_TO_BROM)
+void show_boot_progress(int val)
+{
+	if (val == -BOOTSTAGE_ID_NEED_RESET)
+		rockchip_reset_to_dnl_mode();
+}
+#endif
+
+static void rockchip_dnl_mode_check(void)
 {
 	if (rockchip_dnl_key_pressed()) {
-		printf("download key pressed, entering download mode...");
-		set_back_to_bootrom_dnl_flag();
-		do_reset(NULL, 0, 0, NULL);
+		puts("download key pressed, ");
+		rockchip_reset_to_dnl_mode();
 	}
 }
 
diff --git a/configs/generic-rk3568_defconfig b/configs/generic-rk3568_defconfig
index b90ecb643dc7..9edd4c33000e 100644
--- a/configs/generic-rk3568_defconfig
+++ b/configs/generic-rk3568_defconfig
@@ -11,6 +11,7 @@  CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xc00000
 CONFIG_DEFAULT_DEVICE_TREE="rk3568-generic"
 CONFIG_ROCKCHIP_RK3568=y
 CONFIG_SPL_ROCKCHIP_COMMON_BOARD=y
+CONFIG_ROCKCHIP_HANG_TO_BROM=y
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL_STACK_R_ADDR=0x600000
 CONFIG_SPL_STACK=0x400000
diff --git a/configs/generic-rk3588_defconfig b/configs/generic-rk3588_defconfig
index a9329eb1c625..5872bd4802c5 100644
--- a/configs/generic-rk3588_defconfig
+++ b/configs/generic-rk3588_defconfig
@@ -11,6 +11,7 @@  CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xc00000
 CONFIG_DEFAULT_DEVICE_TREE="rk3588-generic"
 CONFIG_ROCKCHIP_RK3588=y
 CONFIG_SPL_ROCKCHIP_COMMON_BOARD=y
+CONFIG_ROCKCHIP_HANG_TO_BROM=y
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL_STACK_R_ADDR=0x600000
 CONFIG_TARGET_EVB_RK3588=y