[U-Boot] [PATCH v2 0/5] 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
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 philipp.tomsich@theobroma-systems.com ---
Changes in v2: None
arch/arm/include/asm/system.h | 62 +++++++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 31 deletions(-)
diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h index 79bd19a..e2af296 100644 --- a/arch/arm/include/asm/system.h +++ b/arch/arm/include/asm/system.h @@ -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
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 philipp.tomsich@theobroma-systems.com ---
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
diff --git a/arch/arm/include/asm/arch-rockchip/bootrom.h b/arch/arm/include/asm/arch-rockchip/bootrom.h index 169cc5e..2f61a33 100644 --- a/arch/arm/include/asm/arch-rockchip/bootrom.h +++ b/arch/arm/include/asm/arch-rockchip/bootrom.h @@ -1,5 +1,6 @@ /* * (C) Copyright 2017 Heiko Stuebner heiko@sntech.de + * (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 diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile index 79e9704..f8b23ea 100644 --- a/arch/arm/mach-rockchip/Makefile +++ b/arch/arm/mach-rockchip/Makefile @@ -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 diff --git a/arch/arm/mach-rockchip/bootrom.c b/arch/arm/mach-rockchip/bootrom.c index 8380e4e..7b9b307 100644 --- a/arch/arm/mach-rockchip/bootrom.c +++ b/arch/arm/mach-rockchip/bootrom.c @@ -6,11 +6,61 @@
#include <common.h> #include <asm/arch/bootrom.h> +#include <asm/setjmp.h> +#include <asm/system.h> + +/* + * Force the jmp_buf to the data-section, as .bss will not be valid + * when save_boot_params is invoked. + */ +static jmp_buf brom_ctx __section(".data");
void back_to_bootrom(void) { #if CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT) puts("Returning to boot ROM...\n"); #endif - _back_to_bootrom_s(); + longjmp(brom_ctx, BROM_BOOT_NEXTSTAGE); +} + +/* + * All Rockchip BROM implementations enter with a valid stack-pointer, + * so this can safely be implemented in C (providing a single + * implementation both for ARMv7 and AArch64). + */ +int save_boot_params(void) +{ + int ret = setjmp(brom_ctx); + + switch (ret) { + case 0: + /* + * This is the initial pass through this function + * (i.e. saving the context), setjmp just setup up the + * brom_ctx: transfer back into the startup-code at + * 'save_boot_params_ret' and let the compiler know + * that this will not return. + */ + save_boot_params_ret(); + while (true) + /* does not return */; + break; + + case BROM_BOOT_NEXTSTAGE: + /* + * To instruct the BROM to boot the next stage, we + * need to return 0 to it: i.e. we need to rewrite + * the return code once more. + */ + ret = 0; + break; + + default: +#if CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT) + puts("FATAL: unexpected command to back_to_bootrom()\n"); +#endif + hang(); + }; + + return ret; } diff --git a/arch/arm/mach-rockchip/save_boot_param.S b/arch/arm/mach-rockchip/save_boot_param.S deleted file mode 100644 index 50fce20..0000000 --- a/arch/arm/mach-rockchip/save_boot_param.S +++ /dev/null @@ -1,69 +0,0 @@ -/* - * (C) Copyright 2016 Rockchip Electronics Co., Ltd - * (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH - * - * SPDX-License-Identifier: GPL-2.0+ - */ - -#include <linux/linkage.h> - -#if defined(CONFIG_ARM64) -.globl SAVE_SP_ADDR -SAVE_SP_ADDR: - .quad 0 - -ENTRY(save_boot_params) - sub sp, sp, #0x60 - stp x29, x30, [sp, #0x50] - stp x27, x28, [sp, #0x40] - stp x25, x26, [sp, #0x30] - stp x23, x24, [sp, #0x20] - stp x21, x22, [sp, #0x10] - stp x19, x20, [sp, #0] - ldr x8, =SAVE_SP_ADDR - mov x9, sp - str x9, [x8] - b save_boot_params_ret /* back to my caller */ -ENDPROC(save_boot_params) - -.globl _back_to_bootrom_s -ENTRY(_back_to_bootrom_s) - ldr x0, =SAVE_SP_ADDR - ldr x0, [x0] - mov sp, x0 - ldp x29, x30, [sp, #0x50] - ldp x27, x28, [sp, #0x40] - ldp x25, x26, [sp, #0x30] - ldp x23, x24, [sp, #0x20] - ldp x21, x22, [sp, #0x10] - ldp x19, x20, [sp] - add sp, sp, #0x60 - mov x0, xzr - ret -ENDPROC(_back_to_bootrom_s) -#else -.globl SAVE_SP_ADDR -SAVE_SP_ADDR: - .word 0 - -/* - * void save_boot_params - * - * Save sp, lr, r1~r12 - */ -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 -ENDPROC(save_boot_params) - - -.globl _back_to_bootrom_s -ENTRY(_back_to_bootrom_s) - ldr r0, =SAVE_SP_ADDR - ldr sp, [r0] - mov r0, #0 - pop {r1-r12, pc} -ENDPROC(_back_to_bootrom_s) -#endif
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 philipp.tomsich@theobroma-systems.com ---
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(-)
diff --git a/arch/arm/mach-rockchip/rk3188-board-spl.c b/arch/arm/mach-rockchip/rk3188-board-spl.c index d3866bf..05d4ae6 100644 --- a/arch/arm/mach-rockchip/rk3188-board-spl.c +++ b/arch/arm/mach-rockchip/rk3188-board-spl.c @@ -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); diff --git a/arch/arm/mach-rockchip/rk3188-board-tpl.c b/arch/arm/mach-rockchip/rk3188-board-tpl.c index b458ef6..c714278 100644 --- a/arch/arm/mach-rockchip/rk3188-board-tpl.c +++ b/arch/arm/mach-rockchip/rk3188-board-tpl.c @@ -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(); } }
On 18 September 2017 at 12:18, Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
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 philipp.tomsich@theobroma-systems.com
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 sjg@chromium.org
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 philipp.tomsich@theobroma-systems.com
---
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(-)
diff --git a/arch/arm/include/asm/arch-rockchip/bootrom.h b/arch/arm/include/asm/arch-rockchip/bootrom.h index 2f61a33..103b799 100644 --- a/arch/arm/include/asm/arch-rockchip/bootrom.h +++ b/arch/arm/include/asm/arch-rockchip/bootrom.h @@ -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 diff --git a/arch/arm/mach-rockchip/bootrom.c b/arch/arm/mach-rockchip/bootrom.c index 7b9b307..e369fdc 100644 --- a/arch/arm/mach-rockchip/bootrom.c +++ b/arch/arm/mach-rockchip/bootrom.c @@ -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); }
/* diff --git a/arch/arm/mach-rockchip/rk3036-board-spl.c b/arch/arm/mach-rockchip/rk3036-board-spl.c index 9458201..550e3a1 100644 --- a/arch/arm/mach-rockchip/rk3036-board-spl.c +++ b/arch/arm/mach-rockchip/rk3036-board-spl.c @@ -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 */ diff --git a/arch/arm/mach-rockchip/rk3188-board-spl.c b/arch/arm/mach-rockchip/rk3188-board-spl.c index 05d4ae6..8e3b8ae 100644 --- a/arch/arm/mach-rockchip/rk3188-board-spl.c +++ b/arch/arm/mach-rockchip/rk3188-board-spl.c @@ -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;
diff --git a/arch/arm/mach-rockchip/rk3188-board-tpl.c b/arch/arm/mach-rockchip/rk3188-board-tpl.c index c714278..dc5cbf3 100644 --- a/arch/arm/mach-rockchip/rk3188-board-tpl.c +++ b/arch/arm/mach-rockchip/rk3188-board-tpl.c @@ -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); } } diff --git a/arch/arm/mach-rockchip/rk322x-board-spl.c b/arch/arm/mach-rockchip/rk322x-board-spl.c index 4ddb8ba..35f4f97 100644 --- a/arch/arm/mach-rockchip/rk322x-board-spl.c +++ b/arch/arm/mach-rockchip/rk322x-board-spl.c @@ -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 } diff --git a/arch/arm/mach-rockchip/rk3288-board-spl.c b/arch/arm/mach-rockchip/rk3288-board-spl.c index 6b7bf85..6fa4909 100644 --- a/arch/arm/mach-rockchip/rk3288-board-spl.c +++ b/arch/arm/mach-rockchip/rk3288-board-spl.c @@ -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: diff --git a/arch/arm/mach-rockchip/rk3368-board-tpl.c b/arch/arm/mach-rockchip/rk3368-board-tpl.c index b3e6ffa..60d5aea 100644 --- a/arch/arm/mach-rockchip/rk3368-board-tpl.c +++ b/arch/arm/mach-rockchip/rk3368-board-tpl.c @@ -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) diff --git a/arch/arm/mach-rockchip/rk3399-board-spl.c b/arch/arm/mach-rockchip/rk3399-board-spl.c index 9c20f56..b96903e 100644 --- a/arch/arm/mach-rockchip/rk3399-board-spl.c +++ b/arch/arm/mach-rockchip/rk3399-board-spl.c @@ -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] = {
On 18 September 2017 at 12:18, Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
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 philipp.tomsich@theobroma-systems.com
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(-)
Reviewed-by: Simon Glass sjg@chromium.org
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 andy.yan@rock-chips.com Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
---
Changes in v2: None
arch/arm/mach-rockchip/Makefile | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile index f8b23ea..405b30c 100644 --- a/arch/arm/mach-rockchip/Makefile +++ b/arch/arm/mach-rockchip/Makefile @@ -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
Hi Philipp:
On 2017年09月19日 02:18, Philipp Tomsich wrote:
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().
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.
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
Hi Philipp:
On 2017年09月19日 10:06, Andy Yan wrote:
Hi Philipp:
On 2017年09月19日 02:18, Philipp Tomsich wrote:
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().
This still have a problem, because the setjmp implementation forARM32 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.
I found that r9 also need to be preserved, it seems that it hold the sdram base.
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,
On 19 Sep 2017, at 09:19, Andy Yan andy.yan@rock-chips.com wrote:
Hi Philipp:
On 2017年09月19日 10:06, Andy Yan wrote:
Hi Philipp:
On 2017年09月19日 02:18, Philipp Tomsich wrote:
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().
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.
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.
Am Dienstag, 19. September 2017, 11:10:29 CEST schrieb Dr. Philipp Tomsich:
Andy,
On 19 Sep 2017, at 09:19, Andy Yan andy.yan@rock-chips.com wrote:
Hi Philipp:
On 2017年09月19日 10:06, Andy Yan wrote:
Hi Philipp:
On 2017年09月19日 02:18, Philipp Tomsich wrote:
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().
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.
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.
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
On 19 Sep 2017, at 11:12, Heiko Stübner heiko@sntech.de wrote:
Am Dienstag, 19. September 2017, 11:10:29 CEST schrieb Dr. Philipp Tomsich:
Andy,
On 19 Sep 2017, at 09:19, Andy Yan andy.yan@rock-chips.com wrote:
Hi Philipp:
On 2017年09月19日 10:06, Andy Yan wrote:
Hi Philipp:
On 2017年09月19日 02:18, Philipp Tomsich wrote:
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().
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.
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.
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.
Heiko,
On 19 Sep 2017, at 12:16, Dr. Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
On 19 Sep 2017, at 11:12, Heiko Stübner heiko@sntech.de wrote:
Am Dienstag, 19. September 2017, 11:10:29 CEST schrieb Dr. Philipp Tomsich:
Andy,
On 19 Sep 2017, at 09:19, Andy Yan andy.yan@rock-chips.com wrote:
Hi Philipp:
On 2017年09月19日 10:06, Andy Yan wrote:
Hi Philipp:
On 2017年09月19日 02:18, Philipp Tomsich wrote:
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().
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.
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.
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.
Hi Philipp,
Am Donnerstag, 21. September 2017, 10:26:19 CEST schrieb Dr. Philipp Tomsich:
On 19 Sep 2017, at 12:16, Dr. Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
On 19 Sep 2017, at 11:12, Heiko Stübner heiko@sntech.de wrote:
Am Dienstag, 19. September 2017, 11:10:29 CEST schrieb Dr. Philipp Tomsich:
Andy,
On 19 Sep 2017, at 09:19, Andy Yan andy.yan@rock-chips.com wrote:
Hi Philipp:
On 2017年09月19日 10:06, Andy Yan wrote:
Hi Philipp:
On 2017年09月19日 02:18, Philipp Tomsich wrote: > 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(). > 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.
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.
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.
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
Andy,
On 19 Sep 2017, at 11:10, Dr. Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
Andy,
On 19 Sep 2017, at 09:19, Andy Yan andy.yan@rock-chips.com wrote:
Hi Philipp:
On 2017年09月19日 10:06, Andy Yan wrote:
Hi Philipp:
On 2017年09月19日 02:18, Philipp Tomsich wrote:
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().
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.
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.
Hi Philipp:
On 2017年09月19日 20:45, Dr. Philipp Tomsich wrote:
Andy,
On 19 Sep 2017, at 11:10, Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com mailto:philipp.tomsich@theobroma-systems.com> wrote:
Andy,
On 19 Sep 2017, at 09:19, Andy Yan <andy.yan@rock-chips.com mailto:andy.yan@rock-chips.com> wrote:
Hi Philipp:
On 2017年09月19日 10:06, Andy Yan wrote:
Hi Philipp:
On 2017年09月19日 02:18, Philipp Tomsich wrote:
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().
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.
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.
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).
participants (6)
-
Andy Yan -
Dr. Philipp Tomsich -
Heiko Stuebner -
Heiko Stübner
-
Philipp Tomsich
-
Simon Glass