[U-Boot] [PATCH v3 0/6] 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 turned out to require a some tweaking to system.h (making sure that the prototype for save_boot_params_ret is visible for A64)and start.S (so binutils knows that this is a possible function entry and it can correctly insert A32-to-Thumb transitions) and taking the axe to setjmp.h (which created quite a few issues with it not expecting A32/T32/Thumb call-sites and some fragility from GCC being smart about the clobber-list of the inline assembly... which led to r9 not being saved or restored).
Changes in v3: - tracked the root-cause why no interwork branch was emitted and fixed it using a '.type'-directive in start.S to mark save_boot_params_ret as a (possible) function-entry. - converted setjmp/longjmp from inline-assembly to separate .S files to improve predicatability if emitted code
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 - also covers the RK3188 (which I had originally missed)
Philipp Tomsich (6): arm: make save_boot_params_ret prototype visible for AArch64 arm: mark save_boot_params_ret as a function arm: provide a PCS-compliant setjmp implementation 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
arch/arm/cpu/armv7/start.S | 1 + arch/arm/include/asm/arch-rockchip/bootrom.h | 30 ++++++--- arch/arm/include/asm/setjmp.h | 94 ++++------------------------ arch/arm/include/asm/system.h | 62 +++++++++--------- arch/arm/lib/Makefile | 6 ++ arch/arm/lib/setjmp.S | 37 +++++++++++ arch/arm/lib/setjmp_aarch64.S | 42 +++++++++++++ arch/arm/mach-rockchip/Makefile | 4 +- 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 -------------------- 17 files changed, 226 insertions(+), 218 deletions(-) create mode 100644 arch/arm/lib/setjmp.S create mode 100644 arch/arm/lib/setjmp_aarch64.S 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 Tested-by: Andy Yan andy.yan@rock-chips.com ---
Changes in v3: None 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
As no '.type' was set for save_boot_params_ret in start.S, binutils did not track whether it was emitted as A32 or T32. By properly marking save_boot_params_ret as a potential function entry, we can make sure that the compiler will insert the appropriate instructions for branching to save_boot_params_ret both for call-sites emitted as A32 and T32.
Reported-by: Andy Yan andy.yan@rock-chips.com Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com Tested-by: Andy Yan andy.yan@rock-chips.com
---
Changes in v3: - tracked the root-cause why no interwork branch was emitted and fixed it using a '.type'-directive in start.S to mark save_boot_params_ret as a (possible) function-entry.
Changes in v2: None
arch/arm/cpu/armv7/start.S | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 7b84a7a..95a0b52 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -31,6 +31,7 @@
.globl reset .globl save_boot_params_ret + .type save_boot_params_ret,%function #ifdef CONFIG_ARMV7_LPAE .global switch_to_hypervisor_ret #endif
The previous setjmp-implementation (as a static inline function that contained an 'asm volatile' sequence) was extremely fragile: (some versions of) GCC optimised the set of registers. One critical example was the removal of 'r9' from the clobber list, if -ffixed-reg9 was supplied.
To increase robustness and ensure PCS-compliant behaviour, the setjmp and longjmp implementation are now in assembly and closely match what one would expect to find in a libc implementation.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com Tested-by: Andy Yan andy.yan@rock-chips.com
---
Changes in v3: - converted setjmp/longjmp from inline-assembly to separate .S files to improve predicatability if emitted code
Changes in v2: None
arch/arm/include/asm/setjmp.h | 94 ++++++------------------------------------- arch/arm/lib/Makefile | 6 +++ arch/arm/lib/setjmp.S | 37 +++++++++++++++++ arch/arm/lib/setjmp_aarch64.S | 42 +++++++++++++++++++ 4 files changed, 98 insertions(+), 81 deletions(-) create mode 100644 arch/arm/lib/setjmp.S create mode 100644 arch/arm/lib/setjmp_aarch64.S
diff --git a/arch/arm/include/asm/setjmp.h b/arch/arm/include/asm/setjmp.h index c3399a7..517beeb 100644 --- a/arch/arm/include/asm/setjmp.h +++ b/arch/arm/include/asm/setjmp.h @@ -1,6 +1,6 @@ /* - * (C) Copyright 2016 - * Alexander Graf agraf@suse.de + * (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH + * (C) Copyright 2016 Alexander Graf agraf@suse.de * * SPDX-License-Identifier: GPL-2.0+ */ @@ -8,89 +8,21 @@ #ifndef _SETJMP_H_ #define _SETJMP_H_ 1
+/* + * This really should be opaque, but the EFI implementation wrongly + * assumes that a 'struct jmp_buf_data' is defined. + */ struct jmp_buf_data { - ulong target; - ulong regs[5]; - int ret; -}; - -typedef struct jmp_buf_data jmp_buf[1]; - -static inline int setjmp(jmp_buf jmp) -{ - jmp->ret = 0; - -#ifdef CONFIG_ARM64 - asm volatile( - "adr x1, jmp_target\n" - "str x1, %0\n" - "stp x26, x27, %1\n" - "stp x28, x29, %2\n" - "mov x1, sp\n" - "str x1, %3\n" - "jmp_target: " - : "=m" (jmp->target), "=m" (jmp->regs[0]), - "=m" (jmp->regs[2]), "=m" (jmp->regs[4]) - : - : "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7", - "x8", "x9", "x10", "x11", "x12", "x13", "x14", "x15", - "x16", "x17", "x18", "x19", "x20", "x21", "x22", - "x23", "x24", "x25", /* x26, x27, x28, x29, sp */ - "x30", "cc", "memory"); -#else - asm volatile( -#if CONFIG_IS_ENABLED(SYS_THUMB_BUILD) - ".align 2\n" - "adr r0, jmp_target\n" - "add r0, r0, $1\n" +#if defined(__aarch64__) + u64 regs[13]; #else - "adr r0, jmp_target\n" -#endif - "mov r1, %0\n" - "mov r2, sp\n" - "stm r1!, {r0, r2, r4, r5, r6, r7}\n" - ".align 2\n" - "jmp_target: \n" - : - : "l" (&jmp->target) - : "r0", "r1", "r2", "r3", /* "r4", "r5", "r6", "r7", */ - "r8", "r9", "r10", "r11", /* sp, */ "ip", "lr", - "cc", "memory"); -#endif - - return jmp->ret; -} - -static inline __noreturn void longjmp(jmp_buf jmp, int ret) -{ - jmp->ret = ret; - -#ifdef CONFIG_ARM64 - asm volatile( - "ldr x0, %0\n" - "ldr x1, %3\n" - "mov sp, x1\n" - "ldp x26, x27, %1\n" - "ldp x28, x25, %2\n" - "mov x29, x25\n" - "br x0\n" - : - : "m" (jmp->target), "m" (jmp->regs[0]), "m" (jmp->regs[2]), - "m" (jmp->regs[4]) - : "x0", "x1", "x25", "x26", "x27", "x28"); -#else - asm volatile( - "mov r1, %0\n" - "ldm r1!, {r0, r2, r4, r5, r6, r7}\n" - "mov sp, r2\n" - "bx r0\n" - : - : "l" (&jmp->target) - : "r1"); + u32 regs[10]; /* r4-r9, sl, fp, sp, lr */ #endif +};
- while (1) { } -} +typedef struct jmp_buf_data jmp_buf[1];
+int setjmp(jmp_buf jmp); +void longjmp(jmp_buf jmp, int ret);
#endif /* _SETJMP_H_ */ diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 6e1c436..abffa10 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -17,6 +17,12 @@ else obj-y += vectors.o crt0.o endif
+ifdef CONFIG_ARM64 +obj-y += setjmp_aarch64.o +else +obj-y += setjmp.o +endif + ifndef CONFIG_SPL_BUILD ifdef CONFIG_ARM64 obj-y += relocate_64.o diff --git a/arch/arm/lib/setjmp.S b/arch/arm/lib/setjmp.S new file mode 100644 index 0000000..6746e5e --- /dev/null +++ b/arch/arm/lib/setjmp.S @@ -0,0 +1,37 @@ +/* + * (C) 2017 Theobroma Systems Design und Consulting GmbH + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <config.h> +#include <asm/assembler.h> +#include <linux/linkage.h> + +.pushsection .text.setjmp, "ax" +ENTRY(setjmp) + /* + * A subroutine must preserve the contents of the registers + * r4-r8, r10, r11 (v1-v5, v7 and v8) and SP (and r9 in PCS + * variants that designate r9 as v6). + */ + mov ip, sp + stm a1, {v1-v8, ip, lr} + mov a1, #0 + bx lr +ENDPROC(setjmp) +.popsection + +.pushsection .text.longjmp, "ax" +ENTRY(longjmp) + ldm a1, {v1-v8, ip, lr} + mov sp, ip + mov a1, a2 + /* If we were passed a return value of zero, return one instead */ + cmp a1, #0 + bne 1f + mov a1, #1 +1: + bx lr +ENDPROC(longjmp) +.popsection diff --git a/arch/arm/lib/setjmp_aarch64.S b/arch/arm/lib/setjmp_aarch64.S new file mode 100644 index 0000000..b68edb8 --- /dev/null +++ b/arch/arm/lib/setjmp_aarch64.S @@ -0,0 +1,42 @@ +/* + * (C) 2017 Theobroma Systems Design und Consulting GmbH + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <config.h> +#include <asm/macro.h> +#include <linux/linkage.h> + +.pushsection .text.setjmp, "ax" +ENTRY(setjmp) + /* Preserve all callee-saved registers and the SP */ + stp x19, x20, [x0,#0] + stp x21, x22, [x0,#16] + stp x23, x24, [x0,#32] + stp x25, x26, [x0,#48] + stp x27, x28, [x0,#64] + stp x29, x30, [x0,#80] + mov x2, sp + str x2, [x0, #96] + mov x0, #0 + ret +ENDPROC(setjmp) +.popsection + +.pushsection .text.longjmp, "ax" +ENTRY(longjmp) + ldp x19, x20, [x0,#0] + ldp x21, x22, [x0,#16] + ldp x23, x24, [x0,#32] + ldp x25, x26, [x0,#48] + ldp x27, x28, [x0,#64] + ldp x29, x30, [x0,#80] + ldr x2, [x0,#96] + mov sp, x2 + /* Move the return value in place, but return 1 if passed 0. */ + adds x0, xzr, x1 + csinc x0, x0, xzr, ne + ret +ENDPROC(longjmp) +.popsection
On 21.09.17 10:19, Philipp Tomsich wrote:
The previous setjmp-implementation (as a static inline function that contained an 'asm volatile' sequence) was extremely fragile: (some versions of) GCC optimised the set of registers. One critical example was the removal of 'r9' from the clobber list, if -ffixed-reg9 was supplied.
I wouldn't call that fragile, but "works as intended". Gcc only saves the registers it really needs to save - and if r9 is fixed it can safely assume that between setjmp/longjmp it did not change.
Did you encounter other cases where it did something wrong?
To increase robustness and ensure PCS-compliant behaviour, the setjmp and longjmp implementation are now in assembly and closely match what one would expect to find in a libc implementation.
I'm personally quite indifferent on which version we take, but I personally found the inline asm version more readable. At least it was half-way self-documenting and struct offset independent ;).
But again, I really don't have strong feelings. I only wrote the inline asm version because we didn't have any implementation at all. If you opt to maintain yours, be my guest :).
Alex
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 Tested-by: Andy Yan andy.yan@rock-chips.com ---
Changes in v3: None 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 v3: None 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(); } }
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 Tested-by: Andy Yan andy.yan@rock-chips.com
---
Changes in v3: None Changes in v2: - also covers the RK3188 (which 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] = {
Am Donnerstag, 21. September 2017, 10:19:23 CEST schrieb 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.
This turned out to require a some tweaking to system.h (making sure that the prototype for save_boot_params_ret is visible for A64)and start.S (so binutils knows that this is a possible function entry and it can correctly insert A32-to-Thumb transitions) and taking the axe to setjmp.h (which created quite a few issues with it not expecting A32/T32/Thumb call-sites and some fragility from GCC being smart about the clobber-list of the inline assembly... which led to r9 not being saved or restored).
This is missing information on dependant series. Using the u-boot-rockchip repository which is at 782088de7be7 ("rockchip: imply ADC and SARADC_ROCKCHIP on supported SoCs")
patches 1-3 apply, but patch 4 fails to apply as I seem to be missing some dependencies.
And the u-boot mailinglist seems to be configured very strangely, as it seems to rip apart patch-series only sending me some parts.
So far I can at least say, that the u-boot-rockchip repo at the above commit still boots. Could you please point me to mbox versions of needed base patches?
Thanks Heiko
Am Donnerstag, 21. September 2017, 11:09:49 CEST schrieb Heiko Stuebner:
Am Donnerstag, 21. September 2017, 10:19:23 CEST schrieb 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.
This turned out to require a some tweaking to system.h (making sure that the prototype for save_boot_params_ret is visible for A64)and start.S (so binutils knows that this is a possible function entry and it can correctly insert A32-to-Thumb transitions) and taking the axe to setjmp.h (which created quite a few issues with it not expecting A32/T32/Thumb call-sites and some fragility from GCC being smart about the clobber-list of the inline assembly... which led to r9 not being saved or restored).
This is missing information on dependant series. Using the u-boot-rockchip repository which is at 782088de7be7 ("rockchip: imply ADC and SARADC_ROCKCHIP on supported SoCs")
patches 1-3 apply, but patch 4 fails to apply as I seem to be missing some dependencies.
And the u-boot mailinglist seems to be configured very strangely, as it seems to rip apart patch-series only sending me some parts.
So far I can at least say, that the u-boot-rockchip repo at the above commit still boots. Could you please point me to mbox versions of needed base patches?
Also, with patches 1-3 and 5 applied the radxarock board fails to start. I see the SPL banner and a "Returning to boot ROM..." and then nothing.
I do belive it may have something to do with the TPL's + SPL's stack both being at the end of SRAM? Having the SPL go back to TPL and then back to bootrom was my original intention as well, but didn't work at the time.
Heiko
On 21 Sep 2017, at 11:44, Heiko Stuebner heiko@sntech.de wrote:
Am Donnerstag, 21. September 2017, 11:09:49 CEST schrieb Heiko Stuebner:
Am Donnerstag, 21. September 2017, 10:19:23 CEST schrieb 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.
This turned out to require a some tweaking to system.h (making sure that the prototype for save_boot_params_ret is visible for A64)and start.S (so binutils knows that this is a possible function entry and it can correctly insert A32-to-Thumb transitions) and taking the axe to setjmp.h (which created quite a few issues with it not expecting A32/T32/Thumb call-sites and some fragility from GCC being smart about the clobber-list of the inline assembly... which led to r9 not being saved or restored).
This is missing information on dependant series. Using the u-boot-rockchip repository which is at 782088de7be7 ("rockchip: imply ADC and SARADC_ROCKCHIP on supported SoCs")
patches 1-3 apply, but patch 4 fails to apply as I seem to be missing some dependencies.
And the u-boot mailinglist seems to be configured very strangely, as it seems to rip apart patch-series only sending me some parts.
So far I can at least say, that the u-boot-rockchip repo at the above commit still boots. Could you please point me to mbox versions of needed base patches?
Also, with patches 1-3 and 5 applied the radxarock board fails to start. I see the SPL banner and a "Returning to boot ROM..." and then nothing.
I do belive it may have something to do with the TPL's + SPL's stack both being at the end of SRAM? Having the SPL go back to TPL and then back to bootrom was my original intention as well, but didn't work at the time.
I didn’t expect the stacks to overlap… so returning from SPL to TPL can’t work. However, the jump_to_spl() is at least partially to blame (we already have a working C-runtime and there’s no point in reentering through the reset entry-point).
I need to ponder this a bit, but my gut feeling is that the TPL->SPL transition can be done in a less intrusive way and may allow us to retain the TPL stack.
Regards, Philipp.
On 21 Sep 2017, at 12:25, Dr. Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
On 21 Sep 2017, at 11:44, Heiko Stuebner <heiko@sntech.de mailto:heiko@sntech.de> wrote:
Am Donnerstag, 21. September 2017, 11:09:49 CEST schrieb Heiko Stuebner:
Am Donnerstag, 21. September 2017, 10:19:23 CEST schrieb 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.
This turned out to require a some tweaking to system.h (making sure that the prototype for save_boot_params_ret is visible for A64)and start.S (so binutils knows that this is a possible function entry and it can correctly insert A32-to-Thumb transitions) and taking the axe to setjmp.h (which created quite a few issues with it not expecting A32/T32/Thumb call-sites and some fragility from GCC being smart about the clobber-list of the inline assembly... which led to r9 not being saved or restored).
This is missing information on dependant series. Using the u-boot-rockchip repository which is at 782088de7be7 ("rockchip: imply ADC and SARADC_ROCKCHIP on supported SoCs")
patches 1-3 apply, but patch 4 fails to apply as I seem to be missing some dependencies.
And the u-boot mailinglist seems to be configured very strangely, as it seems to rip apart patch-series only sending me some parts.
So far I can at least say, that the u-boot-rockchip repo at the above commit still boots. Could you please point me to mbox versions of needed base patches?
Also, with patches 1-3 and 5 applied the radxarock board fails to start. I see the SPL banner and a "Returning to boot ROM..." and then nothing.
I do belive it may have something to do with the TPL's + SPL's stack both being at the end of SRAM? Having the SPL go back to TPL and then back to bootrom was my original intention as well, but didn't work at the time.
I didn’t expect the stacks to overlap… so returning from SPL to TPL can’t work. However, the jump_to_spl() is at least partially to blame (we already have a working C-runtime and there’s no point in reentering through the reset entry-point).
I need to ponder this a bit, but my gut feeling is that the TPL->SPL transition can be done in a less intrusive way and may allow us to retain the TPL stack.
I had to draw this out on a whiteboard to keep track of what stacks are in use and when they get destroyed (or returned to). Clearly, returning to the TPL and expecting the C-runtime there to be intact will not work. The problem is that when returning to jump_to_spl(), the TPL’s stack will have been overwritten by SPL.
I’ll need to look at the disassembly, but it might be sufficient if I chain into the longjmp directly in jump_to_spl()… or I may need to do something disruptive (e.g. switch stacks) in jump_to_spl.
Regards, Philipp.
Am Donnerstag, 21. September 2017, 12:25:17 CEST schrieb Dr. Philipp Tomsich:
On 21 Sep 2017, at 11:44, Heiko Stuebner heiko@sntech.de wrote:
Am Donnerstag, 21. September 2017, 11:09:49 CEST schrieb Heiko Stuebner:
Am Donnerstag, 21. September 2017, 10:19:23 CEST schrieb 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.
This turned out to require a some tweaking to system.h (making sure that the prototype for save_boot_params_ret is visible for A64)and start.S (so binutils knows that this is a possible function entry and it can correctly insert A32-to-Thumb transitions) and taking the axe to setjmp.h (which created quite a few issues with it not expecting A32/T32/Thumb call-sites and some fragility from GCC being smart about the clobber-list of the inline assembly... which led to r9 not being saved or restored).
This is missing information on dependant series. Using the u-boot-rockchip repository which is at 782088de7be7 ("rockchip: imply ADC and SARADC_ROCKCHIP on supported SoCs")
patches 1-3 apply, but patch 4 fails to apply as I seem to be missing some dependencies.
And the u-boot mailinglist seems to be configured very strangely, as it seems to rip apart patch-series only sending me some parts.
So far I can at least say, that the u-boot-rockchip repo at the above commit still boots. Could you please point me to mbox versions of needed base patches?
Also, with patches 1-3 and 5 applied the radxarock board fails to start. I see the SPL banner and a "Returning to boot ROM..." and then nothing.
I do belive it may have something to do with the TPL's + SPL's stack both being at the end of SRAM? Having the SPL go back to TPL and then back to bootrom was my original intention as well, but didn't work at the time.
I didn’t expect the stacks to overlap… so returning from SPL to TPL can’t work. However, the jump_to_spl() is at least partially to blame (we already have a working C-runtime and there’s no point in reentering through the reset entry-point).
I need to ponder this a bit, but my gut feeling is that the TPL->SPL transition can be done in a less intrusive way and may allow us to retain the TPL stack.
Alternatively, if you can think of an easier solution we could do away with the TPL in its current form. When I did the rk3188 support, this looked like the least-messy way to me, but it really only does the one jump back to the bootrom - so I'm not sure if there isn't simply an easier solution.
And for example the (still wip?) rk3066 series did use spl+tpl in a different way due to bootrom-nand-limitations. rk3066 and rk3188 are quite similar with the rk3066 simply not having the sd-boot capability, so if we want to have nand-boot on rk3188 as well in the future, this may need a different rework again.
Heiko
Hi Philipp, Heiko:
I finally got the upstream u-boot run on a rk3188 board which can be attached by DS5 debugger,
if you have some registers info want to check, please let me know.
On 2017年09月21日 18:44, Heiko Stübner wrote:
Am Donnerstag, 21. September 2017, 12:25:17 CEST schrieb Dr. Philipp Tomsich:
On 21 Sep 2017, at 11:44, Heiko Stuebner heiko@sntech.de wrote:
Am Donnerstag, 21. September 2017, 11:09:49 CEST schrieb Heiko Stuebner:
Am Donnerstag, 21. September 2017, 10:19:23 CEST schrieb 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.
This turned out to require a some tweaking to system.h (making sure that the prototype for save_boot_params_ret is visible for A64)and start.S (so binutils knows that this is a possible function entry and it can correctly insert A32-to-Thumb transitions) and taking the axe to setjmp.h (which created quite a few issues with it not expecting A32/T32/Thumb call-sites and some fragility from GCC being smart about the clobber-list of the inline assembly... which led to r9 not being saved or restored).
This is missing information on dependant series. Using the u-boot-rockchip repository which is at 782088de7be7 ("rockchip: imply ADC and SARADC_ROCKCHIP on supported SoCs")
patches 1-3 apply, but patch 4 fails to apply as I seem to be missing some dependencies.
And the u-boot mailinglist seems to be configured very strangely, as it seems to rip apart patch-series only sending me some parts.
So far I can at least say, that the u-boot-rockchip repo at the above commit still boots. Could you please point me to mbox versions of needed base patches?
Also, with patches 1-3 and 5 applied the radxarock board fails to start. I see the SPL banner and a "Returning to boot ROM..." and then nothing.
I do belive it may have something to do with the TPL's + SPL's stack both being at the end of SRAM? Having the SPL go back to TPL and then back to bootrom was my original intention as well, but didn't work at the time.
I didn’t expect the stacks to overlap… so returning from SPL to TPL can’t work. However, the jump_to_spl() is at least partially to blame (we already have a working C-runtime and there’s no point in reentering through the reset entry-point).
I need to ponder this a bit, but my gut feeling is that the TPL->SPL transition can be done in a less intrusive way and may allow us to retain the TPL stack.
Alternatively, if you can think of an easier solution we could do away with the TPL in its current form. When I did the rk3188 support, this looked like the least-messy way to me, but it really only does the one jump back to the bootrom - so I'm not sure if there isn't simply an easier solution.
And for example the (still wip?) rk3066 series did use spl+tpl in a different way due to bootrom-nand-limitations. rk3066 and rk3188 are quite similar with the rk3066 simply not having the sd-boot capability, so if we want to have nand-boot on rk3188 as well in the future, this may need a different rework again.
Heiko
Andy,
Excellent news. Looks like Heiko and I figured out what breaks the series last week (i.e. the SPL corrupts the TPL’s stack—so my chaining will break things).
I’ll resubmit without the chained returns later and then we can have a final test tomorrow.
Regards, Philipp.
On 25 Sep 2017, at 10:46, Andy Yan andy.yan@rock-chips.com wrote:
Hi Philipp, Heiko:
I finally got the upstream u-boot run on a rk3188 board which can be attached by DS5 debugger,
if you have some registers info want to check, please let me know.
On 2017年09月21日 18:44, Heiko Stübner wrote:
Am Donnerstag, 21. September 2017, 12:25:17 CEST schrieb Dr. Philipp Tomsich:
On 21 Sep 2017, at 11:44, Heiko Stuebner heiko@sntech.de wrote:
Am Donnerstag, 21. September 2017, 11:09:49 CEST schrieb Heiko Stuebner:
Am Donnerstag, 21. September 2017, 10:19:23 CEST schrieb 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.
This turned out to require a some tweaking to system.h (making sure that the prototype for save_boot_params_ret is visible for A64)and start.S (so binutils knows that this is a possible function entry and it can correctly insert A32-to-Thumb transitions) and taking the axe to setjmp.h (which created quite a few issues with it not expecting A32/T32/Thumb call-sites and some fragility from GCC being smart about the clobber-list of the inline assembly... which led to r9 not being saved or restored).
This is missing information on dependant series. Using the u-boot-rockchip repository which is at 782088de7be7 ("rockchip: imply ADC and SARADC_ROCKCHIP on supported SoCs")
patches 1-3 apply, but patch 4 fails to apply as I seem to be missing some dependencies.
And the u-boot mailinglist seems to be configured very strangely, as it seems to rip apart patch-series only sending me some parts.
So far I can at least say, that the u-boot-rockchip repo at the above commit still boots. Could you please point me to mbox versions of needed base patches?
Also, with patches 1-3 and 5 applied the radxarock board fails to start. I see the SPL banner and a "Returning to boot ROM..." and then nothing.
I do belive it may have something to do with the TPL's + SPL's stack both being at the end of SRAM? Having the SPL go back to TPL and then back to bootrom was my original intention as well, but didn't work at the time.
I didn’t expect the stacks to overlap… so returning from SPL to TPL can’t work. However, the jump_to_spl() is at least partially to blame (we already have a working C-runtime and there’s no point in reentering through the reset entry-point).
I need to ponder this a bit, but my gut feeling is that the TPL->SPL transition can be done in a less intrusive way and may allow us to retain the TPL stack.
Alternatively, if you can think of an easier solution we could do away with the TPL in its current form. When I did the rk3188 support, this looked like the least-messy way to me, but it really only does the one jump back to the bootrom - so I'm not sure if there isn't simply an easier solution.
And for example the (still wip?) rk3066 series did use spl+tpl in a different way due to bootrom-nand-limitations. rk3066 and rk3188 are quite similar with the rk3066 simply not having the sd-boot capability, so if we want to have nand-boot on rk3188 as well in the future, this may need a different rework again.
Heiko
On 21 Sep 2017, at 11:09, Heiko Stuebner heiko@sntech.de wrote:
Am Donnerstag, 21. September 2017, 10:19:23 CEST schrieb 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.
This turned out to require a some tweaking to system.h (making sure that the prototype for save_boot_params_ret is visible for A64)and start.S (so binutils knows that this is a possible function entry and it can correctly insert A32-to-Thumb transitions) and taking the axe to setjmp.h (which created quite a few issues with it not expecting A32/T32/Thumb call-sites and some fragility from GCC being smart about the clobber-list of the inline assembly... which led to r9 not being saved or restored).
This is missing information on dependant series. Using the u-boot-rockchip repository which is at 782088de7be7 ("rockchip: imply ADC and SARADC_ROCKCHIP on supported SoCs")
patches 1-3 apply, but patch 4 fails to apply as I seem to be missing some dependencies.
And the u-boot mailinglist seems to be configured very strangely, as it seems to rip apart patch-series only sending me some parts.
So far I can at least say, that the u-boot-rockchip repo at the above commit still boots. Could you please point me to mbox versions of needed base patches?
I seem to be suffering from “too many trees” syndrome. The next reroll will be a clean one again.
Thanks, Philipp.
participants (6)
-
Alexander Graf -
Andy Yan -
Dr. Philipp Tomsich -
Heiko Stuebner -
Heiko Stübner
-
Philipp Tomsich