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 <valentinliu@icloud.com>
---
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