Hi Valentin,

On 8/18/26 6:55 PM, Valentin Liu wrote:
Add a configurable Rockchip SPL hotkey feature that checks the
serial console during SPL startup.

Ctrl+B can be used to enter BootROM download (MASKROM) mode and
be widely used. We can add more boot mode support in future.

Add CONFIG_SPL_ROCKCHIP_HOTKEY to enable the feature and wait for
the serial port to be ready to receive input before checking for
hotkeys.

Signed-off-by: Valentin Liu <[email protected]>
---
Changes for v2:
- Simplify the dependencies of SPL_ROCKCHIP_HOTKEY.
- Remove the conditions for the newly added includes.
---
Changes for v3:
- Add a dummy spl_hotkey_init() to avoid undefined reference errors
   when building without CONFIG_SPL_ROCKCHIP_HOTKEY.

  arch/arm/mach-rockchip/Kconfig | 13 ++++++++++
  arch/arm/mach-rockchip/spl.c   | 44 ++++++++++++++++++++++++++++++++++
  2 files changed, 57 insertions(+)

diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index 1a2e7847c9e..f2d2b5520ef 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -743,6 +743,19 @@ config TPL_ROCKCHIP_EARLYRETURN_TO_BROM
  config SPL_MMC
        default y if !SPL_ROCKCHIP_BACK_TO_BROM
+config SPL_ROCKCHIP_HOTKEY
+       bool "SPL hotkey support"

The symbol name and prompt is not clear enough on what it does.

config SPL_ROCKCHIP_ENTER_MASKROM_ON_KEY
    bool "Enter MaskROM on key press during SPL"

maybe?

+       depends on SPL_DM_RESET && SPL_SERIAL
+       help
+         Enable hotkey detection during SPL booting stage.
+
+         When enabled, SPL checks the serial console for a control
+         character and can execute Rockchip-specific hotkey actions,
+         such as entering BootROM download mode (MASKROM) with Ctrl+B.
+
+         The hotkey is checked after the SPL console has been
+         initialized.
+

Simplify to:

"""
When enabled, the SPL will check whether Ctrl+B is pressed and enter MaskROM in that case.
"""

  config ROCKCHIP_SPI_IMAGE
        bool "Build a SPI image for rockchip"
        help
diff --git a/arch/arm/mach-rockchip/spl.c b/arch/arm/mach-rockchip/spl.c
index e989c148079..0bcfb42c306 100644
--- a/arch/arm/mach-rockchip/spl.c
+++ b/arch/arm/mach-rockchip/spl.c
@@ -13,11 +13,14 @@
  #include <log.h>
  #include <mapmem.h>
  #include <ram.h>
+#include <serial.h>
  #include <spl.h>
+#include <asm/arch-rockchip/boot_mode.h>
  #include <asm/arch-rockchip/bootrom.h>
  #include <asm/arch-rockchip/timer.h>
  #include <asm/global_data.h>
  #include <asm/io.h>
+#include <linux/delay.h>
  #include <linux/bitops.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -107,6 +110,44 @@ __weak int arch_cpu_init(void)
        return 0;
  }
+#if IS_ENABLED(CONFIG_SPL_ROCKCHIP_HOTKEY)

Please use CONFIG_IS_ENABLED() instead.

+static void rockchip_reset_from_hotkey(const int code)
+{
+       switch (code) {
+       case 0x02:

Please add a small comment after 0x02: to specify which key combination triggers this code. E.g.:

case 0x02: /* Ctrl+B */

+               printf("SPL Hotkey: Ctrl+B: BootROM download!\n");

Please be consistent with what we have in arch/arm/mach-rockchip/boot_mode.c, that is:

"Ctrl+B pressed, entering download mode..."

I don't like it, as it's typically called MaskROM, but it's something we can fix later on and I prefer being consistent with what we currently have.

+               writel(BOOT_BROM_DOWNLOAD, CONFIG_ROCKCHIP_BOOT_MODE_REG);

We *really* shouldn't be doing this if CONFIG_ROCKCHIP_BOOT_MODE_REG is 0 (the case for most boards).

+               do_reset(NULL, 0, 0, NULL);
+               /*NOTREACHED*/
+       default:
+               if (code <= 0x1a) /* 'z' */
+                       printf("SPL Hotkey: Ctrl+%c\n", code + 'A' - 1);
+               else
+                       printf("SPL Hotkey: Unknown code: 0x%x, ignore\n", code);> 
+       }> +}
+
+static void spl_hotkey_init(void)
+{
+       if (!gd || !(gd->flags & GD_FLG_HAVE_CONSOLE))
+               return;
+       if (gd->flags & GD_FLG_DISABLE_CONSOLE)
+               return;
+
+       /* Wait for the serial port to be ready to receive data. */
+       mdelay(100);
+

Is it not ready by the time we call this function? How did you come up with 100ms?

+       if (serial_tstc())
+               rockchip_reset_from_hotkey(serial_getc());
+       else
+               printf("SPL Hotkey: No key pressed, continue\n");

We don't need to print on the standard path. If you reaaaaaaally want to have something, then use log_debug/debug instead so it isn't printed by default except if you build with debug logging enabled.

+}
+#else
+static void spl_hotkey_init(void)
+{
+}
+#endif
+
  void board_init_f(ulong dummy)
  {
        int ret;
@@ -143,6 +184,9 @@ void board_init_f(ulong dummy)
        }
  #endif
        preloader_console_init();
+
+       if (IS_ENABLED(CONFIG_SPL_ROCKCHIP_HOTKEY))
+               spl_hotkey_init();

Can we merge with the very similar logic we have for an ADC button in arch/arm/mach-rockchip/boot_mode.c instead? I believe it makes more sense to have everything capable of entering MaskROM mode in the same place, with the same logic. I could see an else if() block in rockchip_dnl_mode_check() for example.

Cheers,
Quentin

Reply via email to