Skip to contentSkip to conversation

support enter download mode on rockchip platform

PATCH Superseded Andy Yan 3 participants 11 messages Sep 12, 2017 – Sep 21, 2017

Subsystems (3)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Andy Yan

author
#0 lore raw

cover support enter download mode on rockchip platform

Most rockchip platform based boards use a key to instruct the board enter download firmware mode, some boards use a adc to detect the key status, and some use a gpio.

This series try to implement all the download mode releated functions.

Andy Yan (3):
rockchip: make boot_mode releated codes reused across all platforms
rockchip: add support for backing to bootrom download mode
rockchip: check download key before bootup

 arch/arm/include/asm/arch-rockchip/boot_mode.h |  6 +++
 arch/arm/include/asm/arch-rockchip/bootrom.h   |  2 +-
 arch/arm/mach-rockchip/Kconfig                 | 15 +++++++
 arch/arm/mach-rockchip/Makefile                |  5 ++-
 arch/arm/mach-rockchip/boot_mode.c             | 58 ++++++++++++++++++++++++++
 arch/arm/mach-rockchip/bootrom.c               |  2 +-
 arch/arm/mach-rockchip/rk3036-board.c          | 24 -----------
 arch/arm/mach-rockchip/rk322x-board.c          | 24 -----------
 arch/arm/mach-rockchip/rk3288-board.c          | 25 -----------
 arch/arm/mach-rockchip/rk3399-board.c          | 14 +++++++
 arch/arm/mach-rockchip/save_boot_param.S       | 56 +++++++++++++++++++++----
 board/rockchip/evb_rk3399/evb-rk3399.c         | 28 +++++++++++++
 12 files changed, 174 insertions(+), 85 deletions(-)
 create mode 100644 arch/arm/mach-rockchip/boot_mode.c
 create mode 100644 arch/arm/mach-rockchip/rk3399-board.c
Signature · 1 line
2.7.4
author
#1 lore raw

PATCH 1/3 rockchip: make boot_mode releated codes reused across all platforms

setup_boot_mode function use the same logic but different mode register address across all the rockchip platforms, so it's better to make this function reused across all the platforms, and let the mode register address setting from the config file.

---

 arch/arm/include/asm/arch-rockchip/boot_mode.h |  2 ++
 arch/arm/mach-rockchip/Kconfig                 | 14 +++++++++++
 arch/arm/mach-rockchip/Makefile                |  5 +++-
 arch/arm/mach-rockchip/boot_mode.c             | 33 ++++++++++++++++++++++++++
 arch/arm/mach-rockchip/rk3036-board.c          | 24 -------------------
 arch/arm/mach-rockchip/rk322x-board.c          | 24 -------------------
 arch/arm/mach-rockchip/rk3288-board.c          | 25 -------------------
 arch/arm/mach-rockchip/rk3399-board.c          | 14 +++++++++++
 8 files changed, 67 insertions(+), 74 deletions(-)
 create mode 100644 arch/arm/mach-rockchip/boot_mode.c
 create mode 100644 arch/arm/mach-rockchip/rk3399-board.c
arch/arm/include/asm/arch-rockchip/boot_mode.h +2 -0
index bd65f60..163b2e7 100644
@@ -16,4 +16,6 @@
 /* enter usb mass storage mode */
 #define BOOT_UMS		(REBOOT_FLAG + 12)
 
+int setup_boot_mode(void);
+
 #endif
arch/arm/mach-rockchip/Kconfig +14 -0
index d9b25d5..527ca60 100644
@@ -114,6 +114,7 @@ config ROCKCHIP_RK3399
 	select SPL_DRIVERS_MISC_SUPPORT
 	select ENABLE_ARM_SOC_BOOT0_HOOK
 	select DEBUG_UART_BOARD_INIT
+	select BOARD_LATE_INIT
 	help
 	  The Rockchip RK3399 is a ARM-based SoC with a dual-core Cortex-A72
 	  and quad-core Cortex-A53.
@@ -149,6 +150,19 @@ config TPL_ROCKCHIP_BACK_TO_BROM
           SPL will return to the boot rom, which will then load the U-Boot
           binary to keep going on.
 
+config ROCKCHIP_BOOT_MODE_REG
+	hex "Rockchip boot mode flag register address"
+	default 0x200081c8 if ROCKCHIP_RK3036
+	default 0x110005d8 if ROCKCHIP_RK322X
+	default 0xff730094 if ROCKCHIP_RK3288
+	default 0xff738200 if ROCKCHIP_RK3368
+	default 0xff320300 if ROCKCHIP_RK3399
+	default 0x10300580 if ROCKCHIP_RV1108
+	default 0
+	help
+	  The Soc will enter to different boot mode according to different
+	  value of this register.
+
 config ROCKCHIP_SPL_RESERVE_IRAM
 	hex "Size of IRAM reserved in SPL"
 	default 0x4000
arch/arm/mach-rockchip/Makefile +0 -0
index 79e9704..b7624cb 100644
@@ -10,7 +10,6 @@
 # the stack-pointer is valid before switching to the U-Boot stack).
 obj-spl-$(CONFIG_ROCKCHIP_BROM_HELPER) += bootrom.o save_boot_param.o
 obj-tpl-$(CONFIG_ROCKCHIP_BROM_HELPER) += bootrom.o save_boot_param.o
 
 obj-tpl-$(CONFIG_ROCKCHIP_RK3188) += rk3188-board-tpl.o
 obj-tpl-$(CONFIG_ROCKCHIP_RK3368) += rk3368-board-tpl.o

@@ -22,10 +21,14 @@ obj-spl-$(CONFIG_ROCKCHIP_RK3368) += rk3368-board-spl.o spl-boot-order.o
obj-spl-$(CONFIG_ROCKCHIP_RK3399) += rk3399-board-spl.o spl-boot-order.o

ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
+ifneq ($(CONFIG_ROCKCHIP_BOOT_MODE_REG),0)
+obj-y += boot_mode.o
+endif
obj-$(CONFIG_ROCKCHIP_RK3188) += rk3188-board.o
obj-$(CONFIG_ROCKCHIP_RK322X) += rk322x-board.o
obj-$(CONFIG_ROCKCHIP_RK3288) += rk3288-board.o
obj-$(CONFIG_ROCKCHIP_RK3036) += rk3036-board.o
+obj-$(CONFIG_ROCKCHIP_RK3399) += rk3399-board.o
endif

obj-$(CONFIG_$(SPL_TPL_)RAM) += sdram_common.o

arch/arm/mach-rockchip/boot_mode.c new file +33 -0
new file mode 100644
index 0000000..7b3cbc5
@@ -0,0 +1,33 @@
+/*
+ * (C) Copyright 2016 Rockchip Electronics Co., Ltd
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/boot_mode.h>
+
+int setup_boot_mode(void)
+{
+	void *reg = (void *)CONFIG_ROCKCHIP_BOOT_MODE_REG;
+	int boot_mode = readl(reg);
+
+	debug("boot mode %x.\n", boot_mode);
+
+	/* Clear boot mode */
+	writel(BOOT_NORMAL, reg);
+
+	switch (boot_mode) {
+	case BOOT_FASTBOOT:
+		printf("enter fastboot!\n");
+		env_set("preboot", "setenv preboot; fastboot usb0");
+		break;
+	case BOOT_UMS:
+		printf("enter UMS!\n");
+		env_set("preboot", "setenv preboot; ums mmc 0");
+		break;
+	}
+
+	return 0;
+}
arch/arm/mach-rockchip/rk3036-board.c +0 -24
index a3457f3..a5d2571 100644
@@ -19,30 +19,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define GRF_BASE	0x20008000
-
-static void setup_boot_mode(void)
-{
-	struct rk3036_grf *const grf = (void *)GRF_BASE;
-	int boot_mode = readl(&grf->os_reg[4]);
-
-	debug("boot mode %x.\n", boot_mode);
-
-	/* Clear boot mode */
-	writel(BOOT_NORMAL, &grf->os_reg[4]);
-
-	switch (boot_mode) {
-	case BOOT_FASTBOOT:
-		printf("enter fastboot!\n");
-		env_set("preboot", "setenv preboot; fastboot usb0");
-		break;
-	case BOOT_UMS:
-		printf("enter UMS!\n");
-		env_set("preboot", "setenv preboot; ums mmc 0");
-		break;
-	}
-}
-
 __weak int rk_board_late_init(void)
 {
 	return 0;
arch/arm/mach-rockchip/rk322x-board.c +0 -24
index c0ac2e9..3af137e 100644
@@ -15,30 +15,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define GRF_BASE	0x11000000
-
-static void setup_boot_mode(void)
-{
-	struct rk322x_grf *const grf = (void *)GRF_BASE;
-	int boot_mode = readl(&grf->os_reg[4]);
-
-	debug("boot mode %x.\n", boot_mode);
-
-	/* Clear boot mode */
-	writel(BOOT_NORMAL, &grf->os_reg[4]);
-
-	switch (boot_mode) {
-	case BOOT_FASTBOOT:
-		printf("enter fastboot!\n");
-		env_set("preboot", "setenv preboot; fastboot usb0");
-		break;
-	case BOOT_UMS:
-		printf("enter UMS!\n");
-		env_set("preboot", "setenv preboot; ums mmc 0");
-		break;
-	}
-}
-
 __weak int rk_board_late_init(void)
 {
 	return 0;
arch/arm/mach-rockchip/rk3288-board.c +0 -25
index 278bb40..1c53cca 100644
@@ -23,31 +23,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define PMU_BASE	0xff730000
-
-static void setup_boot_mode(void)
-{
-	struct rk3288_pmu *const pmu = (void *)PMU_BASE;
-	int boot_mode = readl(&pmu->sys_reg[0]);
-
-	debug("boot mode %x.\n", boot_mode);
-
-	/* Clear boot mode */
-	writel(BOOT_NORMAL, &pmu->sys_reg[0]);
-
-	switch (boot_mode) {
-	case BOOT_FASTBOOT:
-		printf("enter fastboot!\n");
-		env_set("preboot", "setenv preboot; fastboot usb0");
-		break;
-	case BOOT_UMS:
-		printf("enter UMS!\n");
-		env_set("preboot", "setenv preboot; if mmc dev 0;"
-		       "then ums mmc 0; else ums mmc 1;fi");
-		break;
-	}
-}
-
 __weak int rk_board_late_init(void)
 {
 	return 0;
arch/arm/mach-rockchip/rk3399-board.c new file +14 -0
new file mode 100644
index 0000000..9293843
@@ -0,0 +1,14 @@
+/*
+ * Copyright (c) 2017 Rockchip Electronics Co., Ltd
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/arch/boot_mode.h>
+
+int board_late_init(void)
+{
+	setup_boot_mode();
+	return 0;
+}
Signature · 1 line
2.7.4
#2 lore raw

PATCH 1/3 rockchip: make boot_mode releated codes reused across all platforms

Quoted 10 lines · Andy Yan

setup_boot_mode function use the same logic but different mode register address across all the rockchip platforms, so it's better to make this function reused across all the platforms, and let the mode register address setting from the config file.

---

Quoted 11 lines
 arch/arm/include/asm/arch-rockchip/boot_mode.h |  2 ++
 arch/arm/mach-rockchip/Kconfig                 | 14 +++++++++++
 arch/arm/mach-rockchip/Makefile                |  5 +++-
 arch/arm/mach-rockchip/boot_mode.c             | 33 ++++++++++++++++++++++++++
 arch/arm/mach-rockchip/rk3036-board.c          | 24 -------------------
 arch/arm/mach-rockchip/rk322x-board.c          | 24 -------------------
 arch/arm/mach-rockchip/rk3288-board.c          | 25 -------------------
 arch/arm/mach-rockchip/rk3399-board.c          | 14 +++++++++++
 8 files changed, 67 insertions(+), 74 deletions(-)
 create mode 100644 arch/arm/mach-rockchip/boot_mode.c
 create mode 100644 arch/arm/mach-rockchip/rk3399-board.c
maintainer
#3 lore raw

PATCH 1/3 rockchip: make boot_mode releated codes reused across all platforms

In reply to Andy Yan

Hi Andy,

Am Dienstag, 12. September 2017, 21:57:32 CEST schrieb Andy Yan:

Quoted 47 lines

setup_boot_mode function use the same logic but different mode register address across all the rockchip platforms, so it's better to make this function reused across all the platforms, and let the mode register address setting from the config file.

---

 arch/arm/include/asm/arch-rockchip/boot_mode.h |  2 ++
 arch/arm/mach-rockchip/Kconfig                 | 14 +++++++++++
 arch/arm/mach-rockchip/Makefile                |  5 +++-
 arch/arm/mach-rockchip/boot_mode.c             | 33 ++++++++++++++++++++++++++
 arch/arm/mach-rockchip/rk3036-board.c          | 24 -------------------
 arch/arm/mach-rockchip/rk322x-board.c          | 24 -------------------
 arch/arm/mach-rockchip/rk3288-board.c          | 25 -------------------
 arch/arm/mach-rockchip/rk3399-board.c          | 14 +++++++++++
 8 files changed, 67 insertions(+), 74 deletions(-)
 create mode 100644 arch/arm/mach-rockchip/boot_mode.c
 create mode 100644 arch/arm/mach-rockchip/rk3399-board.c
arch/arm/include/asm/arch-rockchip/boot_mode.h +2 -0
index bd65f60..163b2e7 100644
@@ -16,4 +16,6 @@
 /* enter usb mass storage mode */
 #define BOOT_UMS		(REBOOT_FLAG + 12)
 
+int setup_boot_mode(void);
+
 #endif
arch/arm/mach-rockchip/Kconfig +1 -0
index d9b25d5..527ca60 100644
@@ -114,6 +114,7 @@ config ROCKCHIP_RK3399
 	select SPL_DRIVERS_MISC_SUPPORT
 	select ENABLE_ARM_SOC_BOOT0_HOOK
 	select DEBUG_UART_BOARD_INIT
+	select BOARD_LATE_INIT
 	help
 	  The Rockchip RK3399 is a ARM-based SoC with a dual-core Cortex-A72
 	  and quad-core Cortex-A53.
@@ -149,6 +150,19 @@ config TPL_ROCKCHIP_BACK_TO_BROM
           SPL will return to the boot rom, which will then load the U-Boot

arch/arm/mach-rockchip/Kconfig config TPL_ROCKCHIP_BACK_TO_BROM · line 160

Quoted 10 lines
arch/arm/mach-rockchip/Kconfig +8 -0
           binary to keep going on.
 
+config ROCKCHIP_BOOT_MODE_REG
+	hex "Rockchip boot mode flag register address"
+	default 0x200081c8 if ROCKCHIP_RK3036
+	default 0x110005d8 if ROCKCHIP_RK322X
+	default 0xff730094 if ROCKCHIP_RK3288
+	default 0xff738200 if ROCKCHIP_RK3368
+	default 0xff320300 if ROCKCHIP_RK3399
+	default 0x10300580 if ROCKCHIP_RV1108

could you possibly look up if the rk3188 and rk3066 also support the subsequent boot to maskrom download mode from the later patches and include the register if possible?

Thanks
Heiko

author
#4 lore raw

PATCH 1/3 rockchip: make boot_mode releated codes reused across all platforms

Hi Heiko:

Quoted 52 lines · Heiko Stuebner

Hi Andy,

Am Dienstag, 12. September 2017, 21:57:32 CEST schrieb Andy Yan:

Quoted 49 lines

setup_boot_mode function use the same logic but different mode register address across all the rockchip platforms, so it's better to make this function reused across all the platforms, and let the mode register address setting from the config file.

---

  arch/arm/include/asm/arch-rockchip/boot_mode.h |  2 ++
  arch/arm/mach-rockchip/Kconfig                 | 14 +++++++++++
  arch/arm/mach-rockchip/Makefile                |  5 +++-
  arch/arm/mach-rockchip/boot_mode.c             | 33 ++++++++++++++++++++++++++
  arch/arm/mach-rockchip/rk3036-board.c          | 24 -------------------
  arch/arm/mach-rockchip/rk322x-board.c          | 24 -------------------
  arch/arm/mach-rockchip/rk3288-board.c          | 25 -------------------
  arch/arm/mach-rockchip/rk3399-board.c          | 14 +++++++++++
  8 files changed, 67 insertions(+), 74 deletions(-)

create mode 100644 arch/arm/mach-rockchip/boot_mode.c create mode 100644 arch/arm/mach-rockchip/rk3399-board.c

arch/arm/include/asm/arch-rockchip/boot_mode.h +2 -0
index bd65f60..163b2e7 100644
@@ -16,4 +16,6 @@
  /* enter usb mass storage mode */
  #define BOOT_UMS		(REBOOT_FLAG + 12)
 
+int setup_boot_mode(void);
+
  #endif
arch/arm/mach-rockchip/Kconfig +1 -0
index d9b25d5..527ca60 100644
@@ -114,6 +114,7 @@ config ROCKCHIP_RK3399
  	select SPL_DRIVERS_MISC_SUPPORT
  	select ENABLE_ARM_SOC_BOOT0_HOOK
  	select DEBUG_UART_BOARD_INIT
+	select BOARD_LATE_INIT
  	help
  	  The Rockchip RK3399 is a ARM-based SoC with a dual-core Cortex-A72
  	  and quad-core Cortex-A53.
@@ -149,6 +150,19 @@ config TPL_ROCKCHIP_BACK_TO_BROM
            SPL will return to the boot rom, which will then load the U-Boot
            binary to keep going on.
 
Quoted 11 lines

arch/arm/mach-rockchip/Kconfig config TPL_ROCKCHIP_BACK_TO_BROM · line 160

Quoted 8 lines
arch/arm/mach-rockchip/Kconfig +8 -0
+config ROCKCHIP_BOOT_MODE_REG
+	hex "Rockchip boot mode flag register address"
+	default 0x200081c8 if ROCKCHIP_RK3036
+	default 0x110005d8 if ROCKCHIP_RK322X
+	default 0xff730094 if ROCKCHIP_RK3288
+	default 0xff738200 if ROCKCHIP_RK3368
+	default 0xff320300 if ROCKCHIP_RK3399
+	default 0x10300580 if ROCKCHIP_RV1108

could you possibly look up if the rk3188 and rk3066 also support the subsequent boot to maskrom download mode from the later patches and include the register if possible?

Okay, I will add support for them in next version.

Quoted 8 lines

Thanks
Heiko

author
#5 lore raw

PATCH 2/3 rockchip: add support for backing to bootrom download mode

In reply to Andy Yan

Rockchip bootrom will enter download mode if it returns from spl/tpl with a none-zero value and couldn't find a valid image in the backup partition.
This patch provide a method to instruct the system to back to bootrom download mode by checking the BROM_DOWNLOAD_FLAG register. As the bootrom download function relys on some modules such as interrupts, so we need to back to bootrom as early as possbile before the tpl/tps code override the interrupt settings.

---

 arch/arm/include/asm/arch-rockchip/boot_mode.h |  4 ++
 arch/arm/include/asm/arch-rockchip/bootrom.h   |  2 +-
 arch/arm/mach-rockchip/Kconfig                 |  1 +
 arch/arm/mach-rockchip/bootrom.c               |  2 +-
 arch/arm/mach-rockchip/save_boot_param.S       | 56 +++++++++++++++++++++-----
 5 files changed, 54 insertions(+), 11 deletions(-)
arch/arm/include/asm/arch-rockchip/boot_mode.h +4 -0
index 163b2e7..6b2a610 100644
@@ -15,7 +15,11 @@
 #define BOOT_CHARGING		(REBOOT_FLAG + 11)
 /* enter usb mass storage mode */
 #define BOOT_UMS		(REBOOT_FLAG + 12)
+/* enter bootrom download mode */
+#define BOOT_BROM_DOWNLOAD	0xEF08A53C
 
+#ifndef __ASSEMBLY__
 int setup_boot_mode(void);
+#endif
 
 #endif
arch/arm/include/asm/arch-rockchip/bootrom.h +1 -1
index 92eb878..6ae3e94 100644
@@ -22,6 +22,6 @@ void back_to_bootrom(void);
 /**
  * Assembler component for the above (do not call this directly)
  */
-void _back_to_bootrom_s(void);
+void _back_to_bootrom_s(int mode);
 
 #endif
arch/arm/mach-rockchip/Kconfig +1 -0
index 527ca60..a2b7a7e 100644
@@ -113,6 +113,7 @@ config ROCKCHIP_RK3399
 	select SPL_SERIAL_SUPPORT
 	select SPL_DRIVERS_MISC_SUPPORT
 	select ENABLE_ARM_SOC_BOOT0_HOOK
+	select ROCKCHIP_BROM_HELPER
 	select DEBUG_UART_BOARD_INIT
 	select BOARD_LATE_INIT
 	help
arch/arm/mach-rockchip/bootrom.c +1 -1
index 8380e4e..6f0d583 100644
@@ -12,5 +12,5 @@ void back_to_bootrom(void)
 #if CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT)
 	puts("Returning to boot ROM...\n");
 #endif
-	_back_to_bootrom_s();
+	_back_to_bootrom_s(0);
 }
arch/arm/mach-rockchip/save_boot_param.S +47 -9

101 lines not on this page · raw message

Signature · 1 line
2.7.4
#6 lore raw

PATCH 2/3 rockchip: add support for backing to bootrom download mode

Hi Andy,

Quoted 41 lines · Andy Yan

Rockchip bootrom will enter download mode if it returns from spl/tpl with a none-zero value and couldn't find a valid image in the backup partition.
This patch provide a method to instruct the system to back to bootrom download mode by checking the BROM_DOWNLOAD_FLAG register. As the bootrom download function relys on some modules such as interrupts, so we need to back to bootrom as early as possbile before the tpl/tps code override the interrupt settings.

---

 arch/arm/include/asm/arch-rockchip/boot_mode.h |  4 ++
 arch/arm/include/asm/arch-rockchip/bootrom.h   |  2 +-
 arch/arm/mach-rockchip/Kconfig                 |  1 +
 arch/arm/mach-rockchip/bootrom.c               |  2 +-
 arch/arm/mach-rockchip/save_boot_param.S       | 56 +++++++++++++++++++++-----
 5 files changed, 54 insertions(+), 11 deletions(-)
arch/arm/include/asm/arch-rockchip/boot_mode.h +4 -0
index 163b2e7..6b2a610 100644
@@ -15,7 +15,11 @@
 #define BOOT_CHARGING          (REBOOT_FLAG + 11)
 /* enter usb mass storage mode */
 #define BOOT_UMS               (REBOOT_FLAG + 12)
+/* enter bootrom download mode */
+#define BOOT_BROM_DOWNLOAD     0xEF08A53C
 
+#ifndef __ASSEMBLY__
 int setup_boot_mode(void);
+#endif
 
 #endif
arch/arm/include/asm/arch-rockchip/bootrom.h +0 -0
index 92eb878..6ae3e94 100644

arch/arm/include/asm/arch-rockchip/bootrom.h void back_to_bootrom(void); · line 25

Quoted 5 lines
arch/arm/include/asm/arch-rockchip/bootrom.h +1 -1
@@ -22,6 +22,6 @@ void back_to_bootrom(void);
 /**
  * Assembler component for the above (do not call this directly)
  */
-void _back_to_bootrom_s(void);
+void _back_to_bootrom_s(int mode);

Please document the arg

Quoted 19 lines
arch/arm/include/asm/arch-rockchip/bootrom.h +0 -0
 #endif
arch/arm/mach-rockchip/Kconfig +1 -0
index 527ca60..a2b7a7e 100644
@@ -113,6 +113,7 @@ config ROCKCHIP_RK3399
        select SPL_SERIAL_SUPPORT
        select SPL_DRIVERS_MISC_SUPPORT
        select ENABLE_ARM_SOC_BOOT0_HOOK
+       select ROCKCHIP_BROM_HELPER
        select DEBUG_UART_BOARD_INIT
        select BOARD_LATE_INIT
        help
arch/arm/mach-rockchip/bootrom.c +0 -0
index 8380e4e..6f0d583 100644

arch/arm/mach-rockchip/bootrom.c void back_to_bootrom(void) · line 15

Quoted 5 lines
arch/arm/mach-rockchip/bootrom.c +1 -1
@@ -12,5 +12,5 @@ void back_to_bootrom(void)
 #if CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT)
        puts("Returning to boot ROM...\n");
 #endif
-       _back_to_bootrom_s();
+       _back_to_bootrom_s(0);

What does 0 mean? Should this be an enum / #define ?

Quoted 66 lines
arch/arm/mach-rockchip/bootrom.c +0 -0
 }
arch/arm/mach-rockchip/save_boot_param.S +24 -4
index 50fce20..325e81e 100644
@@ -6,12 +6,25 @@
  */
 
 #include <linux/linkage.h>
+#include <asm/arch/boot_mode.h>
 
 #if defined(CONFIG_ARM64)
 .globl SAVE_SP_ADDR
 SAVE_SP_ADDR:
        .quad 0
 
+ENTRY(check_back_to_brom_dnl_flag)
+       ldr     x8, =CONFIG_ROCKCHIP_BOOT_MODE_REG
+       ldr     x9, [x8]
+       ldr     x0, =BOOT_BROM_DOWNLOAD
+       cmp     x9, x0
+       b.ne    save_boot_params_ret
+       mov     x9, xzr
+       str     x9, [x8]        /* clear flag */
+       mov     x0, #1          /* indicate the bootrom to enter download mode */
+       b       _back_to_bootrom_s
+ENDPROC(check_back_to_brom_dnl_flag)
+
 ENTRY(save_boot_params)
        sub     sp, sp, #0x60
        stp     x29, x30, [sp, #0x50]
@@ -23,14 +36,22 @@ ENTRY(save_boot_params)
        ldr     x8, =SAVE_SP_ADDR
        mov     x9, sp
        str     x9, [x8]
+#if CONFIG_ROCKCHIP_BOOT_MODE_REG
+       b       check_back_to_brom_dnl_flag
+#else
        b       save_boot_params_ret  /* back to my caller */
+#endif
 ENDPROC(save_boot_params)
 
+/*
+ * x0: return value for bootrom, none-zero for bootrom download
+ *     mode and zero for normal boot mode
+ */
 .globl _back_to_bootrom_s
 ENTRY(_back_to_bootrom_s)
-       ldr     x0, =SAVE_SP_ADDR
-       ldr     x0, [x0]
-       mov     sp, x0
+       ldr     x1, =SAVE_SP_ADDR
+       ldr     x1, [x1]
+       mov     sp, x1
        ldp     x29, x30, [sp, #0x50]
        ldp     x27, x28, [sp, #0x40]
        ldp     x25, x26, [sp, #0x30]
@@ -38,7 +59,6 @@ ENTRY(_back_to_bootrom_s)
        ldp     x21, x22, [sp, #0x10]
        ldp     x19, x20, [sp]
        add     sp, sp, #0x60
-       mov     x0, xzr
        ret
 ENDPROC(_back_to_bootrom_s)
 #else

arch/arm/mach-rockchip/save_boot_param.S ENDPROC(_back_to_bootrom_s) · line 69

Quoted 4 lines
arch/arm/mach-rockchip/save_boot_param.S +1 -0
@@ -46,6 +66,18 @@ ENDPROC(_back_to_bootrom_s)
 SAVE_SP_ADDR:
        .word 0
 
+ENTRY(check_back_to_brom_dnl_flag)

Please document what this function does

Quoted 42 lines
arch/arm/mach-rockchip/save_boot_param.S +22 -6
+       ldr     r0, =CONFIG_ROCKCHIP_BOOT_MODE_REG
+       ldr     r1, [r0]
+       ldr     r2, =BOOT_BROM_DOWNLOAD
+       cmp     r1, r2
+       bne     save_boot_params_ret
+       mov     r3, #0
+       str     r3, [r0]        @clear flag
+       mov     r0, #1          @indicate the bootrom to enter download mode
+       b       _back_to_bootrom_s
+ENDPROC(check_back_to_brom_dnl_flag)
+
 /*
  * void save_boot_params
  *
@@ -55,15 +87,21 @@ ENTRY(save_boot_params)
        push    {r1-r12, lr}
        ldr     r0, =SAVE_SP_ADDR
        str     sp, [r0]
-       b       save_boot_params_ret            @ back to my caller
+#if CONFIG_ROCKCHIP_BOOT_MODE_REG
+       b       check_back_to_brom_dnl_flag
+#else
+       b       save_boot_params_ret
+#endif
 ENDPROC(save_boot_params)
 
-
+/*
+ * r0: return value for bootrom, none-zero for bootrom download
+ *     mode and zero for normal boot mode
+ */
 .globl _back_to_bootrom_s
 ENTRY(_back_to_bootrom_s)
-       ldr     r0, =SAVE_SP_ADDR
-       ldr     sp, [r0]
-       mov     r0, #0
+       ldr     r1, =SAVE_SP_ADDR
+       ldr     sp, [r1]
        pop     {r1-r12, pc}
 ENDPROC(_back_to_bootrom_s)
 #endif
--
Quoted 3 lines

2.7.4

Regards,
Simon

author
#7 lore raw

PATCH 3/3 rockchip: check download key before bootup

In reply to Andy Yan

Enter download mode if the download key pressed.

---

 arch/arm/mach-rockchip/boot_mode.c     | 27 ++++++++++++++++++++++++++-
 board/rockchip/evb_rk3399/evb-rk3399.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)
arch/arm/mach-rockchip/boot_mode.c +26 -1
index 7b3cbc5..bff1cbc 100644
@@ -8,11 +8,36 @@
 #include <asm/io.h>
 #include <asm/arch/boot_mode.h>
 
+void set_back_to_bootrom_dnl_flag(void)
+{
+	writel(BOOT_BROM_DOWNLOAD, CONFIG_ROCKCHIP_BOOT_MODE_REG);
+}
+
+/*
+ * some boards use a adc key, but some use gpio
+ */
+__weak int rockchip_dnl_key_pressed(void)
+{
+	return false;
+}
+
+void rockchip_dnl_mode_check(void)
+{
+	if (rockchip_dnl_key_pressed()) {
+		printf("download key pressed, enter download mode...");
+		set_back_to_bootrom_dnl_flag();
+		do_reset(NULL, 0, 0, NULL);
+	}
+}
+
 int setup_boot_mode(void)
 {
 	void* reg = (void *)CONFIG_ROCKCHIP_BOOT_MODE_REG;
-	int boot_mode = readl(reg);
+	int boot_mode;
+
+	rockchip_dnl_mode_check();
 
+	boot_mode = readl(reg);
 	debug("boot mode %x.\n", boot_mode);
 
 	/* Clear boot mode */
board/rockchip/evb_rk3399/evb-rk3399.c +28 -0
index d50c59d..738d942 100644
@@ -4,6 +4,7 @@
  * SPDX-License-Identifier:     GPL-2.0+
  */
 #include <common.h>
+#include <adc.h>
 #include <dm.h>
 #include <ram.h>
 #include <dm/pinctrl.h>
@@ -13,6 +14,33 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#define KEY_DOWN_MIN_VAL	1
+#define KEY_DOWN_MAX_VAL	20
+
+int rockchip_dnl_key_pressed(void)
+{
+	unsigned int ret;
+	unsigned int i;
+	unsigned int val;
+	int cnt = 0;
+
+	for (i = 0; i < 10; i++) {
+		ret = adc_channel_single_shot("saradc", 1, &val);
+		if (ret) {
+			printf("%s adc_channel_single_shot fail!\n", __func__);
+			break;
+		}
+
+		if ((val >= KEY_DOWN_MIN_VAL) && (val <= KEY_DOWN_MAX_VAL))
+			cnt++;
+	}
+
+	if (cnt >= 8)
+		return true;
+	else
+		return false;
+}
+
 int board_init(void)
 {
 	struct udevice *pinctrl, *regulator;
Signature · 1 line
2.7.4
#8 lore raw

PATCH 3/3 rockchip: check download key before bootup

Hi Andy,

Quoted 16 lines · Andy Yan

Enter download mode if the download key pressed.

---

 arch/arm/mach-rockchip/boot_mode.c     | 27 ++++++++++++++++++++++++++-
 board/rockchip/evb_rk3399/evb-rk3399.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)
arch/arm/mach-rockchip/boot_mode.c +0 -0
index 7b3cbc5..bff1cbc 100644
@@ -8,11 +8,36 @@
 #include <asm/io.h>
 #include <asm/arch/boot_mode.h>

arch/arm/mach-rockchip/boot_mode.c line 19

Quoted 10 lines
arch/arm/mach-rockchip/boot_mode.c +9 -0
 
+void set_back_to_bootrom_dnl_flag(void)
+{
+       writel(BOOT_BROM_DOWNLOAD, CONFIG_ROCKCHIP_BOOT_MODE_REG);
+}
+
+/*
+ * some boards use a adc key, but some use gpio
+ */
+__weak int rockchip_dnl_key_pressed(void)

Can you please declare this in a header file and add a full comment as to what this does?

arch/arm/mach-rockchip/boot_mode.c line 27

Quoted 8 lines
arch/arm/mach-rockchip/boot_mode.c +8 -0
+{
+       return false;
+}
+
+void rockchip_dnl_mode_check(void)
+{
+       if (rockchip_dnl_key_pressed()) {
+               printf("download key pressed, enter download mode...");

should that be 'entering'?

Quoted 33 lines
arch/arm/mach-rockchip/boot_mode.c +9 -1
+               set_back_to_bootrom_dnl_flag();
+               do_reset(NULL, 0, 0, NULL);
+       }
+}
+
 int setup_boot_mode(void)
 {
        void* reg = (void *)CONFIG_ROCKCHIP_BOOT_MODE_REG;
-       int boot_mode = readl(reg);
+       int boot_mode;
+
+       rockchip_dnl_mode_check();
 
+       boot_mode = readl(reg);
        debug("boot mode %x.\n", boot_mode);
 
        /* Clear boot mode */
board/rockchip/evb_rk3399/evb-rk3399.c +1 -0
index d50c59d..738d942 100644
@@ -4,6 +4,7 @@
  * SPDX-License-Identifier:     GPL-2.0+
  */
 #include <common.h>
+#include <adc.h>
 #include <dm.h>
 #include <ram.h>
 #include <dm/pinctrl.h>
@@ -13,6 +14,33 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 

board/rockchip/evb_rk3399/evb-rk3399.c line 26

Quoted 10 lines
board/rockchip/evb_rk3399/evb-rk3399.c +10 -0
+#define KEY_DOWN_MIN_VAL       1
+#define KEY_DOWN_MAX_VAL       20
+
+int rockchip_dnl_key_pressed(void)
+{
+       unsigned int ret;
+       unsigned int i;
+       unsigned int val;
+       int cnt = 0;
+

Please document what is going on here - why the loop? What is the check against min/max value?

Quoted 21 lines
board/rockchip/evb_rk3399/evb-rk3399.c +17 -1
+       for (i = 0; i < 10; i++) {
+               ret = adc_channel_single_shot("saradc", 1, &val);
+               if (ret) {
+                       printf("%s adc_channel_single_shot fail!\n", __func__);
+                       break;
+               }
+
+               if ((val >= KEY_DOWN_MIN_VAL) && (val <= KEY_DOWN_MAX_VAL))
+                       cnt++;
+       }
+
+       if (cnt >= 8)
+               return true;
+       else
+               return false;
+}
+
 int board_init(void)
 {
        struct udevice *pinctrl, *regulator;
--
Quoted 3 lines

2.7.4

Regards,
Simon

author
#9 lore raw

PATCH 3/3 rockchip: check download key before bootup

Hi Simon:

Quoted 20 lines · Simon Glass

Hi Andy,

Quoted 17 lines · Andy Yan

Enter download mode if the download key pressed.

---

  arch/arm/mach-rockchip/boot_mode.c     | 27 ++++++++++++++++++++++++++-
  board/rockchip/evb_rk3399/evb-rk3399.c | 28 ++++++++++++++++++++++++++++
  2 files changed, 54 insertions(+), 1 deletion(-)
arch/arm/mach-rockchip/boot_mode.c +0 -0
index 7b3cbc5..bff1cbc 100644
@@ -8,11 +8,36 @@
  #include <asm/io.h>
  #include <asm/arch/boot_mode.h>
 
Quoted 11 lines

arch/arm/mach-rockchip/boot_mode.c line 19

Quoted 9 lines
arch/arm/mach-rockchip/boot_mode.c +9 -0
+void set_back_to_bootrom_dnl_flag(void)
+{
+       writel(BOOT_BROM_DOWNLOAD, CONFIG_ROCKCHIP_BOOT_MODE_REG);
+}
+
+/*
+ * some boards use a adc key, but some use gpio
+ */
+__weak int rockchip_dnl_key_pressed(void)

Can you please declare this in a header file and add a full comment as to what this does?

This function is only used in this file and can be override by
other boards. So I don't
think this is a necessary to declare this in a header file.
Of course, I will add a full comment for it.

Quoted 9 lines

arch/arm/mach-rockchip/boot_mode.c line 27

Quoted 8 lines
arch/arm/mach-rockchip/boot_mode.c +8 -0
+{
+       return false;
+}
+
+void rockchip_dnl_mode_check(void)
+{
+       if (rockchip_dnl_key_pressed()) {
+               printf("download key pressed, enter download mode...");

should that be 'entering'?

Okay, should be 'entering'

Quoted 33 lines
Quoted 33 lines
arch/arm/mach-rockchip/boot_mode.c +9 -1
+               set_back_to_bootrom_dnl_flag();
+               do_reset(NULL, 0, 0, NULL);
+       }
+}
+
  int setup_boot_mode(void)
  {
         void* reg = (void *)CONFIG_ROCKCHIP_BOOT_MODE_REG;
-       int boot_mode = readl(reg);
+       int boot_mode;
+
+       rockchip_dnl_mode_check();
 
+       boot_mode = readl(reg);
         debug("boot mode %x.\n", boot_mode);
 
         /* Clear boot mode */
board/rockchip/evb_rk3399/evb-rk3399.c +1 -0
index d50c59d..738d942 100644
@@ -4,6 +4,7 @@
   * SPDX-License-Identifier:     GPL-2.0+
   */
  #include <common.h>
+#include <adc.h>
  #include <dm.h>
  #include <ram.h>
  #include <dm/pinctrl.h>
@@ -13,6 +14,33 @@
 
  DECLARE_GLOBAL_DATA_PTR;
 
Quoted 12 lines

board/rockchip/evb_rk3399/evb-rk3399.c line 26

Quoted 10 lines
board/rockchip/evb_rk3399/evb-rk3399.c +10 -0
+#define KEY_DOWN_MIN_VAL       1
+#define KEY_DOWN_MAX_VAL       20
+
+int rockchip_dnl_key_pressed(void)
+{
+       unsigned int ret;
+       unsigned int i;
+       unsigned int val;
+       int cnt = 0;
+

Please document what is going on here - why the loop? What is the check against min/max value?

This if for voltage debounce, according to my experience on the
rk3399 evb board, in the 10 adc sample results,
we will met 2 ~ 4 result vals are out of the MIN~MAX range.

Quoted 30 lines
Quoted 22 lines
board/rockchip/evb_rk3399/evb-rk3399.c +17 -1
+       for (i = 0; i < 10; i++) {
+               ret = adc_channel_single_shot("saradc", 1, &val);
+               if (ret) {
+                       printf("%s adc_channel_single_shot fail!\n", __func__);
+                       break;
+               }
+
+               if ((val >= KEY_DOWN_MIN_VAL) && (val <= KEY_DOWN_MAX_VAL))
+                       cnt++;
+       }
+
+       if (cnt >= 8)
+               return true;
+       else
+               return false;
+}
+
  int board_init(void)
  {
         struct udevice *pinctrl, *regulator;
--
Quoted 3 lines

2.7.4

Regards,
Simon

#10 lore raw

PATCH 3/3 rockchip: check download key before bootup

Hi Andy,

Quoted 43 lines · Andy Yan

Hi Simon:

Quoted 24 lines · Simon Glass

Hi Andy,

Quoted 20 lines · Andy Yan

Enter download mode if the download key pressed.

---

  arch/arm/mach-rockchip/boot_mode.c     | 27 ++++++++++++++++++++++++++-
  board/rockchip/evb_rk3399/evb-rk3399.c | 28
++++++++++++++++++++++++++++
  2 files changed, 54 insertions(+), 1 deletion(-)
arch/arm/mach-rockchip/boot_mode.c +0 -0
index 7b3cbc5..bff1cbc 100644
@@ -8,11 +8,36 @@
  #include <asm/io.h>
  #include <asm/arch/boot_mode.h>
 
Quoted 12 lines

arch/arm/mach-rockchip/boot_mode.c line 19

Quoted 9 lines
arch/arm/mach-rockchip/boot_mode.c +9 -0
+void set_back_to_bootrom_dnl_flag(void)
+{
+       writel(BOOT_BROM_DOWNLOAD, CONFIG_ROCKCHIP_BOOT_MODE_REG);
+}
+
+/*
+ * some boards use a adc key, but some use gpio
+ */
+__weak int rockchip_dnl_key_pressed(void)

Can you please declare this in a header file and add a full comment as to what this does?

Quoted 3 lines

This function is only used in this file and can be override by other
boards. So I don't
think this is a necessary to declare this in a header file.

The reason is that all exported functions should be declared somewhere. Otherwise the function can be used / overridden somewhere else without any argument checking.

Quoted 65 lines

Of course, I will add a full comment for it.

Quoted 11 lines

arch/arm/mach-rockchip/boot_mode.c line 27

Quoted 9 lines
arch/arm/mach-rockchip/boot_mode.c +8 -0
+{
+       return false;
+}
+
+void rockchip_dnl_mode_check(void)
+{
+       if (rockchip_dnl_key_pressed()) {
+               printf("download key pressed, enter download mode...");

should that be 'entering'?

Okay, should be 'entering'

Quoted 34 lines
Quoted 34 lines
arch/arm/mach-rockchip/boot_mode.c +9 -1
+               set_back_to_bootrom_dnl_flag();
+               do_reset(NULL, 0, 0, NULL);
+       }
+}
+
  int setup_boot_mode(void)
  {
         void* reg = (void *)CONFIG_ROCKCHIP_BOOT_MODE_REG;
-       int boot_mode = readl(reg);
+       int boot_mode;
+
+       rockchip_dnl_mode_check();
 
+       boot_mode = readl(reg);
         debug("boot mode %x.\n", boot_mode);
 
         /* Clear boot mode */
board/rockchip/evb_rk3399/evb-rk3399.c +1 -0
index d50c59d..738d942 100644
@@ -4,6 +4,7 @@
   * SPDX-License-Identifier:     GPL-2.0+
   */
  #include <common.h>
+#include <adc.h>
  #include <dm.h>
  #include <ram.h>
  #include <dm/pinctrl.h>
@@ -13,6 +14,33 @@
 
  DECLARE_GLOBAL_DATA_PTR;
 
Quoted 13 lines

board/rockchip/evb_rk3399/evb-rk3399.c line 26

Quoted 10 lines
board/rockchip/evb_rk3399/evb-rk3399.c +10 -0
+#define KEY_DOWN_MIN_VAL       1
+#define KEY_DOWN_MAX_VAL       20
+
+int rockchip_dnl_key_pressed(void)
+{
+       unsigned int ret;
+       unsigned int i;
+       unsigned int val;
+       int cnt = 0;
+

Please document what is going on here - why the loop? What is the check against min/max value?

Quoted 3 lines

This if for voltage debounce, according to my experience on the rk3399
evb board, in the 10 adc sample results,
we will met 2 ~ 4 result vals are out of the MIN~MAX range.

OK great, please add a comment here about this.

Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
board/rockchip/evb_rk3399/evb-rk3399.c +14 -0
+       for (i = 0; i < 10; i++) {
+               ret = adc_channel_single_shot("saradc", 1, &val);
+               if (ret) {
+                       printf("%s adc_channel_single_shot fail!\n",
 __func__);
+                       break;
+               }
+
+               if ((val >= KEY_DOWN_MIN_VAL) && (val <=
 KEY_DOWN_MAX_VAL))
+                       cnt++;
+       }
+
+       if (cnt >= 8)
+               return true;
+       else
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
board/rockchip/evb_rk3399/evb-rk3399.c +3 -0
+               return false;
+}
+
  int board_init(void)
  {
         struct udevice *pinctrl, *regulator;

--
2.7.4Regards,

Simon