rockchip: back-to-bootrom: replace assembly-implementation with C-code

PATCH Superseded Philipp Tomsich 4 participants 17 messages Sep 18, 2017 – Sep 25, 2017

Subsystems (3)

Arm (32-bit) Arm sub-architectures Arm Rockchip

lore

Share & tools

lore

Philipp Tomsich

author

2017-09-18 18:18 UTC #0 lore raw

v2 cover rockchip: back-to-bootrom: replace assembly-implementation with C-code

Recent discussions confirmed (what the code always assumed): the Rockchip BROM always enters U-Boot with the stack-pointer valid (i.e. the U-Boot startup code is running off the BROM stack).

We can thus replace the back-to-bootrom code (i.e. both the save_boot_params and back_to_bootrom implementations) using C-code based on setjmp/longjmp. The new implementation is already structured to allow an easy drop-in of Andy's changes to enter download-mode when returning to the BROM.

This entails one minor tweak to asm/system.h, which only exported the save_boot_params_ret prototype for ARMv7, but not for AArch64.

For v2, we force bootrom.o to alway be emitted as A32 (not T32), so we can safely call save_boot_params_ret().

It also turned out that I had not caught the RK3188 in my earlier series... and my luck being what it is, the RK3188 needed some extra handholding to adapt to the new regime: instead of passing the context address (for returning to the BROM) from the TPL to the SPL, the SPL now returns to the TPL and the TPL then returns to the BROM.

Changes in v2:
- [added in v2] chain back_to_bootrom calls for SPL, first returning to the TPL (using the same mechanism) and further calling through to the BROM from the TPL by invoking back_to_bootrom again
- adapt the RK3188 spl support file (that I had originally missed)

Philipp Tomsich (5):
arm: make save_boot_params_ret prototype visible for AArch64
rockchip: back-to-bootrom: replace assembly-implementation with C-code
rockchip: back-to-bootrom: rk3188: chain from SPL via TPL to the BROM
rockchip: back-to-bootrom: allow passing a cmd to the bootrom
rockchip: back-to-bootrom: do not compile bootrom.o in thumb mode

 arch/arm/include/asm/arch-rockchip/bootrom.h | 30 +++++++++---
 arch/arm/include/asm/system.h                | 62 ++++++++++++-------------
 arch/arm/mach-rockchip/Makefile              | 10 +++-
 arch/arm/mach-rockchip/bootrom.c             | 54 +++++++++++++++++++++-
 arch/arm/mach-rockchip/rk3036-board-spl.c    |  2 +-
 arch/arm/mach-rockchip/rk3188-board-spl.c    | 14 +-----
 arch/arm/mach-rockchip/rk3188-board-tpl.c    | 19 ++++----
 arch/arm/mach-rockchip/rk322x-board-spl.c    |  2 +-
 arch/arm/mach-rockchip/rk3288-board-spl.c    |  4 +-
 arch/arm/mach-rockchip/rk3368-board-tpl.c    |  2 +-
 arch/arm/mach-rockchip/rk3399-board-spl.c    |  2 +-
 arch/arm/mach-rockchip/save_boot_param.S     | 69 ----------------------------
 12 files changed, 133 insertions(+), 137 deletions(-)
 delete mode 100644 arch/arm/mach-rockchip/save_boot_param.S

Signature · 1 line

2.1.4

Philipp Tomsich

author

2017-09-18 18:18 UTC #1 lore raw

PATCH v2 1/5 arm: make save_boot_params_ret prototype visible for AArch64

The save_boot_params_ret() prototype (for those of us, that have a valid SP on entry and can implement save_boot_params() in C), was previously only defined for !defined(CONFIG_ARM64).

This moves the declaration to a common block to ensure the prototype is available to everyone that might need it.

Signed-off-by Philipp Tomsich <[email protected]>

---

Changes in v2: None

 arch/arm/include/asm/system.h | 62 +++++++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

arch/arm/include/asm/system.h +31 ~~-31~~

index 79bd19a..e2af296 100644
@@ -332,37 +332,6 @@ void psci_arch_init(void);
 
 #ifndef __ASSEMBLY__
 
-/**
- * save_boot_params() - Save boot parameters before starting reset sequence
- *
- * If you provide this function it will be called immediately U-Boot starts,
- * both for SPL and U-Boot proper.
- *
- * All registers are unchanged from U-Boot entry. No registers need be
- * preserved.
- *
- * This is not a normal C function. There is no stack. Return by branching to
- * save_boot_params_ret.
- *
- * void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3);
- */
-
-/**
- * save_boot_params_ret() - Return from save_boot_params()
- *
- * If you provide save_boot_params(), then you should jump back to this
- * function when done. Try to preserve all registers.
- *
- * If your implementation of save_boot_params() is in C then it is acceptable
- * to simply call save_boot_params_ret() at the end of your function. Since
- * there is no link register set up, you cannot just exit the function. U-Boot
- * will return to the (initialised) value of lr, and likely crash/hang.
- *
- * If your implementation of save_boot_params() is in assembler then you
- * should use 'b' or 'bx' to return to save_boot_params_ret.
- */
-void save_boot_params_ret(void);
-
 #ifdef CONFIG_ARMV7_LPAE
 void switch_to_hypervisor_ret(void);
 #endif

@@ -556,6 +525,37 @@ void mmu_page_table_flush(unsigned long start, unsigned long stop);
 
 #ifndef __ASSEMBLY__
 /**
+ * save_boot_params() - Save boot parameters before starting reset sequence
+ *
+ * If you provide this function it will be called immediately U-Boot starts,
+ * both for SPL and U-Boot proper.
+ *
+ * All registers are unchanged from U-Boot entry. No registers need be
+ * preserved.
+ *
+ * This is not a normal C function. There is no stack. Return by branching to
+ * save_boot_params_ret.
+ *
+ * void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3);
+ */
+
+/**
+ * save_boot_params_ret() - Return from save_boot_params()
+ *
+ * If you provide save_boot_params(), then you should jump back to this
+ * function when done. Try to preserve all registers.
+ *
+ * If your implementation of save_boot_params() is in C then it is acceptable
+ * to simply call save_boot_params_ret() at the end of your function. Since
+ * there is no link register set up, you cannot just exit the function. U-Boot
+ * will return to the (initialised) value of lr, and likely crash/hang.
+ *
+ * If your implementation of save_boot_params() is in assembler then you
+ * should use 'b' or 'bx' to return to save_boot_params_ret.
+ */
+void save_boot_params_ret(void);
+
+/**
  * Change the cache settings for a region.
  *
  * \param start		start address of memory region to change

Signature · 1 line

2.1.4

Philipp Tomsich

author

2017-09-18 18:18 UTC #2 lore raw

PATCH v2 2/5 rockchip: back-to-bootrom: replace assembly-implementation with C-code

In reply to Philipp Tomsich

The back-to-bootrom implementation for Rockchip has always relied on the stack-pointer being valid on entry, so there was little reason to have this as an assembly implementation.

This provides a new C-only implementation of save_boot_params and back_to_bootrom (relying on setjmp/longjmp) and removes the older assembly-only implementation.

Signed-off-by Philipp Tomsich <[email protected]>

---

Changes in v2: None

 arch/arm/include/asm/arch-rockchip/bootrom.h | 27 ++++++++---
 arch/arm/mach-rockchip/Makefile              |  4 +-
 arch/arm/mach-rockchip/bootrom.c             | 52 ++++++++++++++++++++-
 arch/arm/mach-rockchip/save_boot_param.S     | 69 ----------------------------
 4 files changed, 73 insertions(+), 79 deletions(-)
 delete mode 100644 arch/arm/mach-rockchip/save_boot_param.S

arch/arm/include/asm/arch-rockchip/bootrom.h +20 -7

index 169cc5e..2f61a33 100644
@@ -1,5 +1,6 @@
 /*
  * (C) Copyright 2017 Heiko Stuebner <[email protected]>
+ * (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH
  *
  * SPDX-License-Identifier:	GPL-2.0
  */

@@ -14,15 +15,27 @@
 extern u32 SAVE_SP_ADDR;
 
 /**
- * Hand control back to the bootrom to load another
- * boot stage.
+ * back_to_bootrom() - return to bootrom (for TPL/SPL), passing a
+ *                     result code
+ *
+ * Transfer control back to the Rockchip BROM, restoring necessary
+ * register context and passing a command/result code to the BROM
+ * to instruct its next actions (e.g. continue boot sequence, enter
+ * download mode, ...).
+ *
+ * This function does not return.
  */
-void back_to_bootrom(void);
+enum rockchip_bootrom_cmd {
+	/*
+	 * These can not start at 0, as 0 has a special meaning
+	 * for setjmp().
+	 */
 
-/**
- * Assembler component for the above (do not call this directly)
- */
-void _back_to_bootrom_s(void);
+	BROM_BOOT_NEXTSTAGE = 1,  /* continue boot-sequence */
+	BROM_BOOT_ENTER_DNL,      /* have BROM enter download-mode */
+};
+
+void back_to_bootrom(void);
 
 /**
  * Boot-device identifiers as used by the BROM

arch/arm/mach-rockchip/Makefile +2 -2

index 79e9704..f8b23ea 100644
@@ -8,8 +8,8 @@
 # this may have entered from ATF with the stack-pointer pointing to
 # inaccessible/protected memory (and the bootrom-helper assumes that
 # 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-spl-$(CONFIG_ROCKCHIP_BROM_HELPER) += bootrom.o
+obj-tpl-$(CONFIG_ROCKCHIP_BROM_HELPER) += bootrom.o
 
 obj-tpl-$(CONFIG_ROCKCHIP_RK3188) += rk3188-board-tpl.o
 obj-tpl-$(CONFIG_ROCKCHIP_RK3368) += rk3368-board-tpl.o

arch/arm/mach-rockchip/bootrom.c +51 -1

62 lines not on this page · raw message

arch/arm/mach-rockchip/save_boot_param.S deleted +0 ~~-69~~

69 lines not on this page · raw message

Signature · 1 line

2.1.4

Philipp Tomsich

author

2017-09-18 18:18 UTC #3 lore raw

PATCH v2 3/5 rockchip: back-to-bootrom: rk3188: chain from SPL via TPL to the BROM

In reply to Philipp Tomsich

The RK3188 implementation previously passed the address of the stack frame created during save_boot_params via pmu->os_reg[2]. This was not strictly necessary, as the save_boot_params() function was called twice (first: for TPL, saving the context for the BROM; next: for SPL, saving the context for the TPL) and a back-to-bootrom from the SPL would thus return to TPL.

To simplify things, we now assume that the state of the TPL is not corrupted during SPL (the binaries are non-overlapping) and that the SPL can safely return to TPL using the back-to-bootrom mechanism. Consequently, the TPL should expect the SPL to return control and then further return to the actual bootrom by performing another back-to-bootrom transition.

Signed-off-by Philipp Tomsich <[email protected]>

---

Changes in v2:
- [added in v2] chain back_to_bootrom calls for SPL, first returning to the TPL (using the same mechanism) and the to the BROM from the TPL

 arch/arm/mach-rockchip/rk3188-board-spl.c | 10 ----------
 arch/arm/mach-rockchip/rk3188-board-tpl.c | 17 ++++++++++-------
 2 files changed, 10 insertions(+), 17 deletions(-)

arch/arm/mach-rockchip/rk3188-board-spl.c +0 ~~-10~~

index d3866bf..05d4ae6 100644
@@ -101,7 +101,6 @@ static int setup_arm_clock(void)
 void board_init_f(ulong dummy)
 {
 	struct udevice *pinctrl, *dev;
-	struct rk3188_pmu *pmu;
 	int ret;
 
 	/* Example code showing how to enable the debug UART on RK3188 */

@@ -145,15 +144,6 @@ void board_init_f(ulong dummy)
 		return;
 	}
 
-	/*
-	 * Recover the bootrom's stackpointer.
-	 * For whatever reason needs to run after rockchip_get_clk.
-	 */
-	pmu = syscon_get_first_range(ROCKCHIP_SYSCON_PMU);
-	if (IS_ERR(pmu))
-		error("pmu syscon returned %ld\n", PTR_ERR(pmu));
-	SAVE_SP_ADDR = readl(&pmu->sys_reg[2]);
-
 	ret = uclass_get_device(UCLASS_PINCTRL, 0, &pinctrl);
 	if (ret) {
 		debug("Pinctrl init failed: %d\n", ret);

arch/arm/mach-rockchip/rk3188-board-tpl.c +10 -7

index b458ef6..c714278 100644
@@ -21,15 +21,16 @@ static int rk3188_num_entries __attribute__ ((section(".data")));
 
 static void jump_to_spl(void)
 {
-	typedef void __noreturn (*image_entry_noargs_t)(void);
+	typedef void (*image_entry_noargs_t)(void);
 
-	struct rk3188_pmu * const pmu = (void *)PMU_BASE;
 	image_entry_noargs_t tpl_entry =
 		(image_entry_noargs_t)(unsigned long)SPL_ENTRY;
 
-	/* Store the SAVE_SP_ADDR in a location shared with SPL. */
-	writel(SAVE_SP_ADDR, &pmu->sys_reg[2]);
 	tpl_entry();
+	/*
+	 * If the SPL stage triggers a 'return to bootrom', it will
+	 * return to here.
+	 */
 }
 
 void board_init_f(ulong dummy)

@@ -77,10 +78,12 @@ void board_init_f(ulong dummy)
 		back_to_bootrom();
 	} else {
 		/*
-		 * TPL part of the loader should now wait for us
-		 * at offset 0xC00 in the sram. Should never return
-		 * from there.
+		 * SPL part of the loader should now wait for us at
+		 * offset 0xC00 in the sram. If the SPL returns to us,
+		 * we should in turn return to the BROM (i.e. chain
+		 * through).
 		 */
 		jump_to_spl();
+		back_to_bootrom();
 	}
 }

Signature · 1 line

2.1.4

Simon Glass

2017-09-25 02:12 UTC #4 lore raw

PATCH v2 3/5 rockchip: back-to-bootrom: rk3188: chain from SPL via TPL to the BROM

Quoted 17 lines · Philipp Tomsich

The RK3188 implementation previously passed the address of the stack frame created during save_boot_params via pmu->os_reg[2]. This was not strictly necessary, as the save_boot_params() function was called twice (first: for TPL, saving the context for the BROM; next: for SPL, saving the context for the TPL) and a back-to-bootrom from the SPL would thus return to TPL.
To simplify things, we now assume that the state of the TPL is not corrupted during SPL (the binaries are non-overlapping) and that the SPL can safely return to TPL using the back-to-bootrom mechanism. Consequently, the TPL should expect the SPL to return control and then further return to the actual bootrom by performing another back-to-bootrom transition.
Signed-off-by Philipp Tomsich <[email protected]>
---

Quoted 8 lines

Changes in v2:
- [added in v2] chain back_to_bootrom calls for SPL, first returning to the TPL (using the same mechanism) and the to the BROM from the TPL
 arch/arm/mach-rockchip/rk3188-board-spl.c | 10 ----------
 arch/arm/mach-rockchip/rk3188-board-tpl.c | 17 ++++++++++-------
 2 files changed, 10 insertions(+), 17 deletions(-)

Reviewed-by Simon Glass <[email protected]>

Philipp Tomsich

author

2017-09-18 18:18 UTC #5 lore raw

PATCH v2 4/5 rockchip: back-to-bootrom: allow passing a cmd to the bootrom

In reply to Philipp Tomsich

The BROM supports forcing it to enter download-mode, if an appropriate result/cmd-word is returned to it. There already is a series to support this in review, so this prepares the (newly C-version) of the back-to-bootrom code to accept a cmd to passed on to the BROM.

All the existing call-sites are adjusted to match the changed function signature.

Signed-off-by Philipp Tomsich <[email protected]>

---

Changes in v2:
- adapt the RK3188 spl support file (that I had originally missed)

 arch/arm/include/asm/arch-rockchip/bootrom.h | 5 ++++-
 arch/arm/mach-rockchip/bootrom.c             | 4 ++--
 arch/arm/mach-rockchip/rk3036-board-spl.c    | 2 +-
 arch/arm/mach-rockchip/rk3188-board-spl.c    | 4 ++--
 arch/arm/mach-rockchip/rk3188-board-tpl.c    | 4 ++--
 arch/arm/mach-rockchip/rk322x-board-spl.c    | 2 +-
 arch/arm/mach-rockchip/rk3288-board-spl.c    | 4 ++--
 arch/arm/mach-rockchip/rk3368-board-tpl.c    | 2 +-
 arch/arm/mach-rockchip/rk3399-board-spl.c    | 2 +-
 9 files changed, 16 insertions(+), 13 deletions(-)

arch/arm/include/asm/arch-rockchip/bootrom.h +4 -1

index 2f61a33..103b799 100644
@@ -24,6 +24,9 @@ extern u32 SAVE_SP_ADDR;
  * download mode, ...).
  *
  * This function does not return.
+ *
+ * @brom_cmd: indicates how the bootrom should continue the boot
+ *            sequence (e.g. load the next stage)
  */
 enum rockchip_bootrom_cmd {
 	/*

@@ -35,7 +38,7 @@ enum rockchip_bootrom_cmd {
 	BROM_BOOT_ENTER_DNL,      /* have BROM enter download-mode */
 };
 
-void back_to_bootrom(void);
+void back_to_bootrom(enum rockchip_bootrom_cmd brom_cmd);
 
 /**
  * Boot-device identifiers as used by the BROM

arch/arm/mach-rockchip/bootrom.c +2 -2

index 7b9b307..e369fdc 100644
@@ -15,12 +15,12 @@
  */
 static jmp_buf brom_ctx __section(".data");
 
-void back_to_bootrom(void)
+void back_to_bootrom(enum rockchip_bootrom_cmd brom_cmd)
 {
 #if CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT)
 	puts("Returning to boot ROM...\n");
 #endif
-	longjmp(brom_ctx, BROM_BOOT_NEXTSTAGE);
+	longjmp(brom_ctx, brom_cmd);
 }
 
 /*

arch/arm/mach-rockchip/rk3036-board-spl.c +1 -1

index 9458201..550e3a1 100644
@@ -40,7 +40,7 @@ void board_init_f(ulong dummy)
 	sdram_init();
 
 	/* return to maskrom */
-	back_to_bootrom();
+	back_to_bootrom(BROM_BOOT_NEXTSTAGE);
 }
 
 /* Place Holders */

arch/arm/mach-rockchip/rk3188-board-spl.c +2 -2

index 05d4ae6..8e3b8ae 100644
@@ -158,7 +158,7 @@ void board_init_f(ulong dummy)
 
 	setup_arm_clock();
 #if CONFIG_IS_ENABLED(ROCKCHIP_BACK_TO_BROM) && !defined(CONFIG_SPL_BOARD_INIT)
-	back_to_bootrom();
+	back_to_bootrom(BROM_BOOT_NEXTSTAGE);
 #endif
 }

@@ -219,7 +219,7 @@ void spl_board_init(void)
 
 	preloader_console_init();
 #if CONFIG_IS_ENABLED(ROCKCHIP_BACK_TO_BROM)
-	back_to_bootrom();
+	back_to_bootrom(BROM_BOOT_NEXTSTAGE);
 #endif
 	return;

arch/arm/mach-rockchip/rk3188-board-tpl.c +2 -2

index c714278..dc5cbf3 100644
@@ -75,7 +75,7 @@ void board_init_f(ulong dummy)
 		 * really early on.
 		 */
 
-		back_to_bootrom();
+		back_to_bootrom(BROM_BOOT_NEXTSTAGE);
 	} else {
 		/*
 		 * SPL part of the loader should now wait for us at

@@ -84,6 +84,6 @@ void board_init_f(ulong dummy)
 		 * through).
 		 */
 		jump_to_spl();
-		back_to_bootrom();
+		back_to_bootrom(BROM_BOOT_NEXTSTAGE);
 	}
 }

arch/arm/mach-rockchip/rk322x-board-spl.c +1 -1

index 4ddb8ba..35f4f97 100644
@@ -76,6 +76,6 @@ void board_init_f(ulong dummy)
 	/* Disable the ddr secure region setting to make it non-secure */
 	rk_clrreg(SGRF_DDR_CON0, 0x4000);
 #if defined(CONFIG_ROCKCHIP_SPL_BACK_TO_BROM) && !defined(CONFIG_SPL_BOARD_INIT)
-	back_to_bootrom();
+	back_to_bootrom(BROM_BOOT_NEXTSTAGE);
 #endif
 }

arch/arm/mach-rockchip/rk3288-board-spl.c +2 -2

index 6b7bf85..6fa4909 100644
@@ -250,7 +250,7 @@ void board_init_f(ulong dummy)
 		return;
 	}
 #if CONFIG_IS_ENABLED(ROCKCHIP_BACK_TO_BROM) && !defined(CONFIG_SPL_BOARD_INIT)
-	back_to_bootrom();
+	back_to_bootrom(BROM_BOOT_NEXTSTAGE);
 #endif
 }

@@ -317,7 +317,7 @@ void spl_board_init(void)
 
 	preloader_console_init();
 #if CONFIG_IS_ENABLED(ROCKCHIP_BACK_TO_BROM)
-	back_to_bootrom();
+	back_to_bootrom(BROM_BOOT_NEXTSTAGE);
 #endif
 	return;
 err:

arch/arm/mach-rockchip/rk3368-board-tpl.c +1 -1

index b3e6ffa..60d5aea 100644
@@ -148,7 +148,7 @@ void board_init_f(ulong dummy)
 
 void board_return_to_bootrom(void)
 {
-	back_to_bootrom();
+	back_to_bootrom(BROM_BOOT_NEXTSTAGE);
 }
 
 u32 spl_boot_device(void)

arch/arm/mach-rockchip/rk3399-board-spl.c +1 -1

index 9c20f56..b96903e 100644
@@ -23,7 +23,7 @@ DECLARE_GLOBAL_DATA_PTR;
 
 void board_return_to_bootrom(void)
 {
-	back_to_bootrom();
+	back_to_bootrom(BROM_BOOT_NEXTSTAGE);
 }
 
 static const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {

Signature · 1 line

2.1.4

Simon Glass

2017-09-25 02:12 UTC #6 lore raw

PATCH v2 4/5 rockchip: back-to-bootrom: allow passing a cmd to the bootrom

Quoted 15 lines · Philipp Tomsich

The BROM supports forcing it to enter download-mode, if an appropriate result/cmd-word is returned to it. There already is a series to support this in review, so this prepares the (newly C-version) of the back-to-bootrom code to accept a cmd to passed on to the BROM.
All the existing call-sites are adjusted to match the changed function signature.
Signed-off-by Philipp Tomsich <[email protected]>
---
Changes in v2:
- adapt the RK3188 spl support file (that I had originally missed)

Quoted 10 lines

 arch/arm/include/asm/arch-rockchip/bootrom.h | 5 ++++-
 arch/arm/mach-rockchip/bootrom.c             | 4 ++--
 arch/arm/mach-rockchip/rk3036-board-spl.c    | 2 +-
 arch/arm/mach-rockchip/rk3188-board-spl.c    | 4 ++--
 arch/arm/mach-rockchip/rk3188-board-tpl.c    | 4 ++--
 arch/arm/mach-rockchip/rk322x-board-spl.c    | 2 +-
 arch/arm/mach-rockchip/rk3288-board-spl.c    | 4 ++--
 arch/arm/mach-rockchip/rk3368-board-tpl.c    | 2 +-
 arch/arm/mach-rockchip/rk3399-board-spl.c    | 2 +-
 9 files changed, 16 insertions(+), 13 deletions(-)

Reviewed-by Simon Glass <[email protected]>

Philipp Tomsich

author

2017-09-18 18:18 UTC #7 lore raw

PATCH v2 5/5 rockchip: back-to-bootrom: do not compile bootrom.o in thumb mode

In reply to Philipp Tomsich

With start.o being compiled to A32 for ARMv7 (and no Thumb mode), we need to ensure that the call to save_boot_params_ret either happens from ARM mode or uses an interwork branch.

To keep things simple, we force bootrom.o to always be A32 code by setting specific CFLAGS for this compilation unit. Note that marking save_boot_params_ret with the 'target(arm)'-function attribute did not generate an interwork branch when calling save_boot_params_ret.

Reported-by Andy Yan <[email protected]>

Signed-off-by Philipp Tomsich <[email protected]>

---

Changes in v2: None

 arch/arm/mach-rockchip/Makefile | 6 ++++++
 1 file changed, 6 insertions(+)

arch/arm/mach-rockchip/Makefile +6 -0

index f8b23ea..405b30c 100644
@@ -4,6 +4,12 @@
 # SPDX-License-Identifier:	GPL-2.0+
 #
 
+# The bootrom-helper needs to be ARM (i.e. not Thumb code) due to the
+# way setjmp/longjmp is implemented.
+ifndef CONFIG_ARM64
+CFLAGS_bootrom.o := -marm
+endif
+
 # We don't want the bootrom-helper present in a full U-Boot build, as
 # this may have entered from ATF with the stack-pointer pointing to
 # inaccessible/protected memory (and the bootrom-helper assumes that

Signature · 1 line

2.1.4

Andy Yan

2017-09-19 02:06 UTC #8 lore raw

v2 cover rockchip: back-to-bootrom: replace assembly-implementation with C-code

In reply to Philipp Tomsich

Hi Philipp:

Quoted 10 lines · Philipp Tomsich

Recent discussions confirmed (what the code always assumed): the Rockchip BROM always enters U-Boot with the stack-pointer valid (i.e. the U-Boot startup code is running off the BROM stack).
We can thus replace the back-to-bootrom code (i.e. both the save_boot_params and back_to_bootrom implementations) using C-code based on setjmp/longjmp. The new implementation is already structured to allow an easy drop-in of Andy's changes to enter download-mode when returning to the BROM.

Quoted 5 lines

This entails one minor tweak to asm/system.h, which only exported the save_boot_params_ret prototype for ARMv7, but not for AArch64.
For v2, we force bootrom.o to alway be emitted as A32 (not T32), so we can safely call save_boot_params_ret().

This still have a problem, because the setjmp implementation for
ARM32 platform has humb code when CONFIG_SYS_THUMB_BUILD is enabled, this is a default setting for most ARMv7 boards.
#if CONFIG_IS_ENABLED(SYS_THUMB_BUILD)
".align 2\n"
"adr r0, jmp_target\n"
"add r0, r0, $1\n" // r0 stored the jump target address and with
bit[0] = 1, this will trigger a thumb switch in longjmp with code "bx r0" #endif

When I force the setjmp code go arm code path, I can back to bootrom successfully, But I got a data abort exception in later. it seems it happens when bootrom finished the uboot code
copy, when jump to sdram, I need a further debug.

Quoted 35 lines

It also turned out that I had not caught the RK3188 in my earlier series... and my luck being what it is, the RK3188 needed some extra handholding to adapt to the new regime: instead of passing the context address (for returning to the BROM) from the TPL to the SPL, the SPL now returns to the TPL and the TPL then returns to the BROM.
Changes in v2:
- [added in v2] chain back_to_bootrom calls for SPL, first returning to the TPL (using the same mechanism) and further calling through to the BROM from the TPL by invoking back_to_bootrom again
- adapt the RK3188 spl support file (that I had originally missed)
Philipp Tomsich (5):
arm: make save_boot_params_ret prototype visible for AArch64
rockchip: back-to-bootrom: replace assembly-implementation with C-code
rockchip: back-to-bootrom: rk3188: chain from SPL via TPL to the BROM
rockchip: back-to-bootrom: allow passing a cmd to the bootrom
rockchip: back-to-bootrom: do not compile bootrom.o in thumb mode
  arch/arm/include/asm/arch-rockchip/bootrom.h | 30 +++++++++---
  arch/arm/include/asm/system.h                | 62 ++++++++++++-------------
  arch/arm/mach-rockchip/Makefile              | 10 +++-
  arch/arm/mach-rockchip/bootrom.c             | 54 +++++++++++++++++++++-
  arch/arm/mach-rockchip/rk3036-board-spl.c    |  2 +-
  arch/arm/mach-rockchip/rk3188-board-spl.c    | 14 +-----
  arch/arm/mach-rockchip/rk3188-board-tpl.c    | 19 ++++----
  arch/arm/mach-rockchip/rk322x-board-spl.c    |  2 +-
  arch/arm/mach-rockchip/rk3288-board-spl.c    |  4 +-
  arch/arm/mach-rockchip/rk3368-board-tpl.c    |  2 +-
  arch/arm/mach-rockchip/rk3399-board-spl.c    |  2 +-
  arch/arm/mach-rockchip/save_boot_param.S     | 69 ----------------------------
  12 files changed, 133 insertions(+), 137 deletions(-)
delete mode 100644 arch/arm/mach-rockchip/save_boot_param.S

Andy Yan

2017-09-19 07:19 UTC #9 lore raw

v2 cover rockchip: back-to-bootrom: replace assembly-implementation with C-code

Hi Philipp:

Quoted 30 lines · Andy Yan

Hi Philipp:
Quoted 10 lines · Philipp Tomsich
Recent discussions confirmed (what the code always assumed): the Rockchip BROM always enters U-Boot with the stack-pointer valid (i.e. the U-Boot startup code is running off the BROM stack).
We can thus replace the back-to-bootrom code (i.e. both the save_boot_params and back_to_bootrom implementations) using C-code based on setjmp/longjmp. The new implementation is already structured to allow an easy drop-in of Andy's changes to enter download-mode when returning to the BROM.
Quoted 5 lines
This entails one minor tweak to asm/system.h, which only exported the save_boot_params_ret prototype for ARMv7, but not for AArch64.
For v2, we force bootrom.o to alway be emitted as A32 (not T32), so we can safely call save_boot_params_ret().
This still have a problem, because the setjmp implementation for
ARM32 platform has humb code when CONFIG_SYS_THUMB_BUILD is enabled, this is a default setting for most ARMv7 boards.
#if CONFIG_IS_ENABLED(SYS_THUMB_BUILD)
".align 2\n"
"adr r0, jmp_target\n"
"add r0, r0, $1\n" // r0 stored the jump target address and with
bit[0] = 1, this will trigger a thumb switch in longjmp with code "bx r0" #endif

Quoted 4 lines

When I force the setjmp code go arm code path, I can back to bootrom successfully, But I got a data abort exception in later. it seems it happens when bootrom finished the uboot code
copy, when jump to sdram, I need a further debug.

I found that r9 also need to be preserved, it seems that it hold the sdram base.

Quoted 40 lines

Quoted 39 lines
It also turned out that I had not caught the RK3188 in my earlier series... and my luck being what it is, the RK3188 needed some extra handholding to adapt to the new regime: instead of passing the context address (for returning to the BROM) from the TPL to the SPL, the SPL now returns to the TPL and the TPL then returns to the BROM.
Changes in v2:
- [added in v2] chain back_to_bootrom calls for SPL, first returning to the TPL (using the same mechanism) and further calling through to the BROM from the TPL by invoking back_to_bootrom again
- adapt the RK3188 spl support file (that I had originally missed)
Philipp Tomsich (5):
arm: make save_boot_params_ret prototype visible for AArch64
rockchip: back-to-bootrom: replace assembly-implementation with
C-code
rockchip: back-to-bootrom: rk3188: chain from SPL via TPL to the BROM
rockchip: back-to-bootrom: allow passing a cmd to the bootrom
rockchip: back-to-bootrom: do not compile bootrom.o in thumb mode
  arch/arm/include/asm/arch-rockchip/bootrom.h | 30 +++++++++---
  arch/arm/include/asm/system.h                | 62 
++++++++++++-------------
  arch/arm/mach-rockchip/Makefile              | 10 +++-
  arch/arm/mach-rockchip/bootrom.c             | 54 
+++++++++++++++++++++-
  arch/arm/mach-rockchip/rk3036-board-spl.c    |  2 +-
  arch/arm/mach-rockchip/rk3188-board-spl.c    | 14 +-----
  arch/arm/mach-rockchip/rk3188-board-tpl.c    | 19 ++++----
  arch/arm/mach-rockchip/rk322x-board-spl.c    |  2 +-
  arch/arm/mach-rockchip/rk3288-board-spl.c    |  4 +-
  arch/arm/mach-rockchip/rk3368-board-tpl.c    |  2 +-
  arch/arm/mach-rockchip/rk3399-board-spl.c    |  2 +-
  arch/arm/mach-rockchip/save_boot_param.S     | 69 
----------------------------
  12 files changed, 133 insertions(+), 137 deletions(-)
delete mode 100644 arch/arm/mach-rockchip/save_boot_param.S

Dr. Philipp Tomsich

author

2017-09-19 09:10 UTC #10 lore raw

v2 cover rockchip: back-to-bootrom: replace assembly-implementation with C-code

Andy,

Quoted 34 lines · Andy Yan

Hi Philipp:
Quoted 28 lines · Andy Yan
Hi Philipp:
Quoted 10 lines · Philipp Tomsich
Recent discussions confirmed (what the code always assumed): the Rockchip BROM always enters U-Boot with the stack-pointer valid (i.e. the U-Boot startup code is running off the BROM stack).
We can thus replace the back-to-bootrom code (i.e. both the save_boot_params and back_to_bootrom implementations) using C-code based on setjmp/longjmp. The new implementation is already structured to allow an easy drop-in of Andy's changes to enter download-mode when returning to the BROM.
Quoted 5 lines
This entails one minor tweak to asm/system.h, which only exported the save_boot_params_ret prototype for ARMv7, but not for AArch64.
For v2, we force bootrom.o to alway be emitted as A32 (not T32), so we can safely call save_boot_params_ret().
This still have a problem, because the setjmp implementation for ARM32 platform has humb code when CONFIG_SYS_THUMB_BUILD is
enabled, this is a default setting for most ARMv7 boards.
#if CONFIG_IS_ENABLED(SYS_THUMB_BUILD)
".align 2\n"
"adr r0, jmp_target\n"
"add r0, r0, $1\n" // r0 stored the jump target address and with bit[0] = 1, this will trigger a thumb switch in longjmp with code "bx r0" #endif

Quoted 4 lines

Quoted 2 lines
When I force the setjmp code go arm code path, I can back to bootrom successfully, But I got a data abort exception in later. it seems it happens when bootrom finished the uboot code copy, when jump to sdram, I need a further debug.
I found that r9 also need to be preserved, it seems that it hold the sdram base.

Thanks for testing and debugging: this is invaluable support, as I only have AArch64 boards to test.

The r9 issue will be easy enough to resolve.
However, it looks like I will need more work on setjmp/longjmp to make this safe both for T32 and A32.
Plus: I need to figure out why this didn’t show in my disassembly (I don’t remember whether it was a rk3188 or rk3288 board I looked at).

Might be tomorrow or Thursday until I can provide an new version.

Regards,
Philipp.

Heiko Stübner

maintainer

2017-09-19 09:12 UTC #11 lore raw

v2 cover rockchip: back-to-bootrom: replace assembly-implementation with C-code

Am Dienstag, 19. September 2017, 11:10:29 CEST schrieb Dr. Philipp Tomsich:

Quoted 46 lines

Andy,
Quoted 34 lines · Andy Yan
Hi Philipp:
Quoted 29 lines · Andy Yan
Hi Philipp:
Quoted 10 lines · Philipp Tomsich
Recent discussions confirmed (what the code always assumed): the Rockchip BROM always enters U-Boot with the stack-pointer valid (i.e. the U-Boot startup code is running off the BROM stack).
We can thus replace the back-to-bootrom code (i.e. both the save_boot_params and back_to_bootrom implementations) using C-code based on setjmp/longjmp. The new implementation is already structured to allow an easy drop-in of Andy's changes to enter download-mode when returning to the BROM.
Quoted 6 lines
This entails one minor tweak to asm/system.h, which only exported the save_boot_params_ret prototype for ARMv7, but not for AArch64.
For v2, we force bootrom.o to alway be emitted as A32 (not T32), so we can safely call save_boot_params_ret().
This still have a problem, because the setjmp implementation for ARM32 platform has humb code when CONFIG_SYS_THUMB_BUILD is>>
enabled, this is a default setting for most ARMv7 boards. #if CONFIG_IS_ENABLED(SYS_THUMB_BUILD)
".align 2\n"
"adr r0, jmp_target\n"
"add r0, r0, $1\n" // r0 stored the jump target address and with bit[0] = 1, this will trigger a thumb switch in longjmp with code "bx r0" #endif
Quoted 7 lines
Quoted 4 lines
When I force the setjmp code go arm code path, I can back to bootrom successfully, But I got a data abort exception in later. it seems it happens when bootrom finished the uboot code copy, when jump to sdram, I need a further debug.
I found that r9 also need to be preserved, it seems that it hold the sdram base.
Thanks for testing and debugging: this is invaluable support, as I only have AArch64 boards to test.

Quoted 7 lines

The r9 issue will be easy enough to resolve.
However, it looks like I will need more work on setjmp/longjmp to make this safe both for T32 and A32. Plus: I need to figure out why this didn’t show in my disassembly (I don’t remember whether it was a rk3188 or rk3288 board I looked at).
Might be tomorrow or Thursday until I can provide an new version.

From this conversation, it looks to me that I should wait for that new version for testing on rk3188, as it will likely show the same issues, right?

Heiko

Dr. Philipp Tomsich

author

2017-09-19 10:16 UTC #12 lore raw

v2 cover rockchip: back-to-bootrom: replace assembly-implementation with C-code

Quoted 56 lines · Heiko Stübner

Am Dienstag, 19. September 2017, 11:10:29 CEST schrieb Dr. Philipp Tomsich:
Quoted 52 lines
Andy,
Quoted 34 lines · Andy Yan
Hi Philipp:
Quoted 29 lines · Andy Yan
Hi Philipp:
Quoted 10 lines · Philipp Tomsich
Recent discussions confirmed (what the code always assumed): the Rockchip BROM always enters U-Boot with the stack-pointer valid (i.e. the U-Boot startup code is running off the BROM stack).
We can thus replace the back-to-bootrom code (i.e. both the save_boot_params and back_to_bootrom implementations) using C-code based on setjmp/longjmp. The new implementation is already structured to allow an easy drop-in of Andy's changes to enter download-mode when returning to the BROM.
Quoted 6 lines
This entails one minor tweak to asm/system.h, which only exported the save_boot_params_ret prototype for ARMv7, but not for AArch64.
For v2, we force bootrom.o to alway be emitted as A32 (not T32), so we can safely call save_boot_params_ret().
This still have a problem, because the setjmp implementation for ARM32 platform has humb code when CONFIG_SYS_THUMB_BUILD is>>
enabled, this is a default setting for most ARMv7 boards. #if CONFIG_IS_ENABLED(SYS_THUMB_BUILD)
".align 2\n"
"adr r0, jmp_target\n"
"add r0, r0, $1\n" // r0 stored the jump target address and with bit[0] = 1, this will trigger a thumb switch in longjmp with code "bx r0" #endif
Quoted 7 lines
Quoted 4 lines
When I force the setjmp code go arm code path, I can back to bootrom successfully, But I got a data abort exception in later. it seems it happens when bootrom finished the uboot code copy, when jump to sdram, I need a further debug.
I found that r9 also need to be preserved, it seems that it hold the sdram base.
Thanks for testing and debugging: this is invaluable support, as I only have AArch64 boards to test.
The r9 issue will be easy enough to resolve.
However, it looks like I will need more work on setjmp/longjmp to make this safe both for T32 and A32. Plus: I need to figure out why this didn’t show in my disassembly (I don’t remember whether it was a rk3188 or rk3288 board I looked at).

Quoted 4 lines

Quoted 1 line
Might be tomorrow or Thursday until I can provide an new version.
From this conversation, it looks to me that I should wait for that new version for testing on rk3188, as it will likely show the same issues, right?

Yes.

Dr. Philipp Tomsich

author

2017-09-21 08:26 UTC #13 lore raw

v2 cover rockchip: back-to-bootrom: replace assembly-implementation with C-code

Heiko,

Quoted 58 lines · Dr. Philipp Tomsich

Quoted 57 lines · Heiko Stübner
Am Dienstag, 19. September 2017, 11:10:29 CEST schrieb Dr. Philipp Tomsich:
Quoted 52 lines
Andy,
Quoted 34 lines · Andy Yan
Hi Philipp:
Quoted 29 lines · Andy Yan
Hi Philipp:
Quoted 10 lines · Philipp Tomsich
Recent discussions confirmed (what the code always assumed): the Rockchip BROM always enters U-Boot with the stack-pointer valid (i.e. the U-Boot startup code is running off the BROM stack).
We can thus replace the back-to-bootrom code (i.e. both the save_boot_params and back_to_bootrom implementations) using C-code based on setjmp/longjmp. The new implementation is already structured to allow an easy drop-in of Andy's changes to enter download-mode when returning to the BROM.
Quoted 6 lines
This entails one minor tweak to asm/system.h, which only exported the save_boot_params_ret prototype for ARMv7, but not for AArch64.
For v2, we force bootrom.o to alway be emitted as A32 (not T32), so we can safely call save_boot_params_ret().
This still have a problem, because the setjmp implementation for ARM32 platform has humb code when CONFIG_SYS_THUMB_BUILD is>>
enabled, this is a default setting for most ARMv7 boards. #if CONFIG_IS_ENABLED(SYS_THUMB_BUILD)
".align 2\n"
"adr r0, jmp_target\n"
"add r0, r0, $1\n" // r0 stored the jump target address and with bit[0] = 1, this will trigger a thumb switch in longjmp with code "bx r0" #endif
Quoted 7 lines
Quoted 4 lines
When I force the setjmp code go arm code path, I can back to bootrom successfully, But I got a data abort exception in later. it seems it happens when bootrom finished the uboot code copy, when jump to sdram, I need a further debug.
I found that r9 also need to be preserved, it seems that it hold the sdram base.
Thanks for testing and debugging: this is invaluable support, as I only have AArch64 boards to test.
The r9 issue will be easy enough to resolve.
However, it looks like I will need more work on setjmp/longjmp to make this safe both for T32 and A32. Plus: I need to figure out why this didn’t show in my disassembly (I don’t remember whether it was a rk3188 or rk3288 board I looked at).

Quoted 6 lines

Quoted 4 lines
Quoted 1 line
Might be tomorrow or Thursday until I can provide an new version.
From this conversation, it looks to me that I should wait for that new version for testing on rk3188, as it will likely show the same issues, right?
Yes.

A new version with a reworked setjmp/longjmp implementation is available at
https://patchwork.ozlabs.org/project/uboot/list/?series=4327

This passed testing for Andy (see the Tested-by: credit) on the boards he works on, but is still pending a test of a RK3188.

Regards,
Philipp.

Heiko Stuebner

maintainer

2017-09-21 08:51 UTC #14 lore raw

v2 cover rockchip: back-to-bootrom: replace assembly-implementation with C-code

Hi Philipp,

Am Donnerstag, 21. September 2017, 10:26:19 CEST schrieb Dr. Philipp Tomsich:

Quoted 63 lines

Quoted 63 lines · Dr. Philipp Tomsich
Quoted 61 lines · Heiko Stübner
Am Dienstag, 19. September 2017, 11:10:29 CEST schrieb Dr. Philipp Tomsich:
Quoted 46 lines
Andy,
Quoted 34 lines · Andy Yan
Hi Philipp:
Quoted 29 lines · Andy Yan
Hi Philipp:
Quoted 10 lines · Philipp Tomsich
Recent discussions confirmed (what the code always assumed): the Rockchip BROM always enters U-Boot with the stack-pointer valid (i.e. the U-Boot startup code is running off the BROM stack).
We can thus replace the back-to-bootrom code (i.e. both the save_boot_params and back_to_bootrom implementations) using C-code based on setjmp/longjmp. The new implementation is already structured to allow an easy drop-in of Andy's changes to enter download-mode when returning to the BROM.
Quoted 6 lines
This entails one minor tweak to asm/system.h, which only exported the save_boot_params_ret prototype for ARMv7, but not for AArch64.
For v2, we force bootrom.o to alway be emitted as A32 (not T32), so we can safely call save_boot_params_ret().
This still have a problem, because the setjmp implementation for ARM32 platform has humb code when CONFIG_SYS_THUMB_BUILD is>>
enabled, this is a default setting for most ARMv7 boards. #if CONFIG_IS_ENABLED(SYS_THUMB_BUILD)
".align 2\n"
"adr r0, jmp_target\n"
"add r0, r0, $1\n" // r0 stored the jump target address and with bit[0] = 1, this will trigger a thumb switch in longjmp with code "bx r0" #endif
Quoted 7 lines
Quoted 4 lines
When I force the setjmp code go arm code path, I can back to bootrom successfully, But I got a data abort exception in later. it seems it happens when bootrom finished the uboot code copy, when jump to sdram, I need a further debug.
I found that r9 also need to be preserved, it seems that it hold the sdram base.
Thanks for testing and debugging: this is invaluable support, as I only have AArch64 boards to test.
Quoted 7 lines
The r9 issue will be easy enough to resolve.
However, it looks like I will need more work on setjmp/longjmp to make this safe both for T32 and A32. Plus: I need to figure out why this didn’t show in my disassembly (I don’t remember whether it was a rk3188 or rk3288 board I looked at).
Might be tomorrow or Thursday until I can provide an new version.
From this conversation, it looks to me that I should wait for that new version for testing on rk3188, as it will likely show the same issues, right?

Quoted 7 lines

Quoted 1 line
Yes.
A new version with a reworked setjmp/longjmp implementation is available at
https://patchwork.ozlabs.org/project/uboot/list/?series=4327
This passed testing for Andy (see the Tested-by: credit) on the boards he works on, but is still pending a test of a RK3188.

Testing that v3 is actually the thing I currently doing, after I found the v3 in my inbox :-) .

Results shortly in the v3 series.

Heiko

Dr. Philipp Tomsich

author

2017-09-19 12:45 UTC #15 lore raw

v2 cover rockchip: back-to-bootrom: replace assembly-implementation with C-code

In reply to Dr. Philipp Tomsich

Andy,

Quoted 41 lines · Dr. Philipp Tomsich

Andy,
Quoted 37 lines · Andy Yan
Hi Philipp:
Quoted 30 lines · Andy Yan
Hi Philipp:
Quoted 10 lines · Philipp Tomsich
Recent discussions confirmed (what the code always assumed): the Rockchip BROM always enters U-Boot with the stack-pointer valid (i.e. the U-Boot startup code is running off the BROM stack).
We can thus replace the back-to-bootrom code (i.e. both the save_boot_params and back_to_bootrom implementations) using C-code based on setjmp/longjmp. The new implementation is already structured to allow an easy drop-in of Andy's changes to enter download-mode when returning to the BROM.
Quoted 5 lines
This entails one minor tweak to asm/system.h, which only exported the save_boot_params_ret prototype for ARMv7, but not for AArch64.
For v2, we force bootrom.o to alway be emitted as A32 (not T32), so we can safely call save_boot_params_ret().
This still have a problem, because the setjmp implementation for ARM32 platform has humb code when CONFIG_SYS_THUMB_BUILD is
enabled, this is a default setting for most ARMv7 boards.
#if CONFIG_IS_ENABLED(SYS_THUMB_BUILD)
".align 2\n"
"adr r0, jmp_target\n"
"add r0, r0, $1\n" // r0 stored the jump target address and with bit[0] = 1, this will trigger a thumb switch in longjmp with code "bx r0" #endif
When I force the setjmp code go arm code path, I can back to bootrom successfully, But I got a data abort exception in later. it seems it happens when bootrom finished the uboot code copy, when jump to sdram, I need a further debug.

Quoted 7 lines

Quoted 1 line
I found that r9 also need to be preserved, it seems that it hold the sdram base.
Thanks for testing and debugging: this is invaluable support, as I only have AArch64 boards to test.
The r9 issue will be easy enough to resolve.
However, it looks like I will need more work on setjmp/longjmp to make this safe both for T32 and A32.
Plus: I need to figure out why this didn’t show in my disassembly (I don’t remember whether it was a rk3188 or rk3288 board I looked at).

I had a quick look and things may be quicker to resolve than I thought.
Before I create a new version, I was wondering what the requirements on the BROM end are:
Without changes to setjmp/longjmp, I can currently preserve "r4-r11, lr, sp” (i.e "r1-r3, ip" will be clobbered).
If the BROM need any of these additional registers preserved (i.e. r1,r2,r3,ip): let me know and I will change setjmp/longjmp to be more conservative.

Andy Yan

2017-09-20 01:51 UTC #16 lore raw

v2 cover rockchip: back-to-bootrom: replace assembly-implementation with C-code

Hi Philipp:

Quoted 63 lines · Dr. Philipp Tomsich

Andy,
Quoted 52 lines
On 19 Sep 2017, at 11:10, Dr. Philipp Tomsich <[email protected] <mailto:[email protected]>> wrote:
Andy,
Quoted 38 lines
On 19 Sep 2017, at 09:19, Andy Yan <[email protected] <mailto:[email protected]>> wrote:
Hi Philipp:
Quoted 31 lines · Andy Yan
Hi Philipp:
Quoted 10 lines · Philipp Tomsich
Recent discussions confirmed (what the code always assumed): the Rockchip BROM always enters U-Boot with the stack-pointer valid (i.e. the U-Boot startup code is running off the BROM stack).
We can thus replace the back-to-bootrom code (i.e. both the save_boot_params and back_to_bootrom implementations) using C-code based on setjmp/longjmp. The new implementation is already structured to allow an easy drop-in of Andy's changes to enter download-mode when returning to the BROM.
Quoted 5 lines
This entails one minor tweak to asm/system.h, which only exported the save_boot_params_ret prototype for ARMv7, but not for AArch64.
For v2, we force bootrom.o to alway be emitted as A32 (not T32), so we can safely call save_boot_params_ret().
This still have a problem, because the setjmp implementation for
ARM32 platform has humb code when CONFIG_SYS_THUMB_BUILD is enabled, this is a default setting for most ARMv7 boards. #if CONFIG_IS_ENABLED(SYS_THUMB_BUILD)
".align 2\n"
"adr r0, jmp_target\n"
"add r0, r0, $1\n" // r0 stored the jump target address and with bit[0] = 1, this will trigger a thumb switch in longjmp with code "bx r0"
#endif
Quoted 7 lines
Quoted 4 lines
When I force the setjmp code go arm code path, I can back to bootrom successfully, But I got a data abort exception in later. it seems it happens when bootrom finished the uboot code copy, when jump to sdram, I need a further debug.
I found that r9 also need to be preserved, it seems that it hold the sdram base.
Quoted 8 lines
Thanks for testing and debugging: this is invaluable support, as I only have AArch64 boards to test.
The r9 issue will be easy enough to resolve.
However, it looks like I will need more work on setjmp/longjmp to make this safe both for T32 and A32.
Plus: I need to figure out why this didn’t show in my disassembly (I don’t remember whether it was a rk3188 or rk3288 board I looked at).

Quoted 9 lines

I had a quick look and things may be quicker to resolve than I thought. Before I create a new version, I was wondering what the requirements on the BROM end are:
Without changes to setjmp/longjmp, I can currently preserve "r4-r11, lr, sp” (i.e "r1-r3, ip" will be clobbered).
If the BROM need any of these additional registers preserved (i.e. r1,r2,r3,ip): let me know and I will change setjmp/longjmp to be more conservative.

The BROM code that call TPL/SPL also write in C like bellow:
fp = (pFunc)(ptr + 4);
ret = (*fp)(); // fp point to TPL/SPL first address
if (ret)

      return;

ptr = (uint8*)SDRAM_ADDR;

So the code doesn't touch the register directly, It's the compile stored SDRAM_ADDR in r9(I saw it on rk3036 platform).