[PATCH 0/3] sunxi: FEL boot fixes
While investigating a FEL boot failure on the OrangePi Zero 2 board (with an H616 SoC), I stared at our FEL code and found some issues, which this series fixes.
Unfortunately those didn't fix the H616 problem, but they are worth having anyway.
For the records on the FEL failure: with certain H616 DRAM parameters, newer GCCs (starting with GCC 11, GCC 10 is fine) generate an SPL binary that doesn't properly return to the BootROM after the SPL code has finished. This does not occur on other H616 boards using different DRAM parameters. Staring at the disassemblies from the different compiler versions for a while didn't show anything obvious, the best theory so far is that it's due to a subtle timing issue in the DRAM initialisation code. Inserting a "udelay(0);" at the beginning of mctl_phy_read_calibration() seems to avoid the problem, but is obviously not a proper fix. I will keep looking.
However the issues addressed in this series should be fixed, regardless.
Cheers, Andre
Andre Przywara (3): sunxi: armv8: fel: load only 32-bit values sunxi: h616: lower SPL stack address to avoid BROM data sunxi: fel: drop redundant "control register" save/restore
arch/arm/cpu/armv7/sunxi/fel_utils.S | 4 ---- arch/arm/cpu/armv8/fel_utils.S | 8 ++++---- arch/arm/mach-sunxi/board.c | 1 - include/configs/sunxi-common.h | 2 +- 4 files changed, 5 insertions(+), 10 deletions(-)
Both the values and the MMIO addresses that we need during the 64-bit FEL restore are smaller than 2^32, so we don't need to do any 64-bit loads.
Change the loads to only load 32 bits worth of data, that saves us some bytes for storing the values.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- arch/arm/cpu/armv8/fel_utils.S | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/cpu/armv8/fel_utils.S b/arch/arm/cpu/armv8/fel_utils.S index 5266515f14..2fe38a1a04 100644 --- a/arch/arm/cpu/armv8/fel_utils.S +++ b/arch/arm/cpu/armv8/fel_utils.S @@ -39,15 +39,15 @@ ENTRY(return_to_fel) adr x1, fel_stash_addr // to find the fel_stash address in AA32 str w2, [x1]
- ldr x0, =0xfa50392f // CPU hotplug magic + ldr w0, =0xfa50392f // CPU hotplug magic #ifdef CONFIG_MACH_SUN50I_H616 - ldr x2, =(SUNXI_R_CPUCFG_BASE + 0x1c0) + ldr w2, =(SUNXI_R_CPUCFG_BASE + 0x1c0) str w0, [x2], #0x4 #elif CONFIG_MACH_SUN50I_H6 - ldr x2, =(SUNXI_RTC_BASE + 0x1b8) // BOOT_CPU_HP_FLAG_REG + ldr w2, =(SUNXI_RTC_BASE + 0x1b8) // BOOT_CPU_HP_FLAG_REG str w0, [x2], #0x4 #else - ldr x2, =(SUNXI_CPUCFG_BASE + 0x1a4) // offset for CPU hotplug base + ldr w2, =(SUNXI_CPUCFG_BASE + 0x1a4) // offset for CPU hotplug base str w0, [x2, #0x8] #endif adr x0, back_in_32
On 7/13/22 10:27, Andre Przywara wrote:
Both the values and the MMIO addresses that we need during the 64-bit FEL restore are smaller than 2^32, so we don't need to do any 64-bit loads.
Change the loads to only load 32 bits worth of data, that saves us some bytes for storing the values.
Signed-off-by: Andre Przywara andre.przywara@arm.com
arch/arm/cpu/armv8/fel_utils.S | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Samuel Holland samuel@sholland.org Tested-by: Samuel Holland samuel@sholland.org
When using the USB OTG FEL mode on the Allwinner H616, the BootROM stores some data at the end of SRAM C. This is also the location where we place the initial SPL stack, so it will overwrite this data. We still need the BROM code after running the SPL, so should leave that area alone. Interestingly this does not seem to have an adverse effect, I guess on the "way out" (when we return to FEL after the SPL has run), this data is not needed by the BROM, for just the trailing end of the USB operation. However this is still wrong, and we should not clobber BROM data.
Lower the SPL stack address to be situated right below the swap buffers we use in sunxi-fel: that should be out of the way of everyone else.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- include/configs/sunxi-common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 0f0ef4f64b..d36b59daf7 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -105,7 +105,7 @@ #endif /* !CONFIG_ARM64 */ #elif CONFIG_SUNXI_SRAM_ADDRESS == 0x20000 #ifdef CONFIG_MACH_SUN50I_H616 -#define LOW_LEVEL_SRAM_STACK 0x58000 +#define LOW_LEVEL_SRAM_STACK 0x52a00 /* below FEL buffers */ #else /* end of SRAM A2 on H6 for now */ #define LOW_LEVEL_SRAM_STACK 0x00118000
On 7/13/22 10:27, Andre Przywara wrote:
When using the USB OTG FEL mode on the Allwinner H616, the BootROM stores some data at the end of SRAM C. This is also the location where we place the initial SPL stack, so it will overwrite this data. We still need the BROM code after running the SPL, so should leave that area alone. Interestingly this does not seem to have an adverse effect, I guess on the "way out" (when we return to FEL after the SPL has run), this data is not needed by the BROM, for just the trailing end of the USB operation. However this is still wrong, and we should not clobber BROM data.
Lower the SPL stack address to be situated right below the swap buffers we use in sunxi-fel: that should be out of the way of everyone else.
If we are going to limit SPL size based on these buffers, we should ensure they are as close to the end of SRAM as possible.
Signed-off-by: Andre Przywara andre.przywara@arm.com
include/configs/sunxi-common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Samuel Holland samuel@sholland.org Tested-by: Samuel Holland samuel@sholland.org
For some reasons shrouded in mystery, the code saving the FEL state was saving the SCTLR register twice, with the second copy trying to justify itself by using its ancient "control register" alias name.
Drop the redundant second copy, both from the fel_stash data structure, and also the code saving and restoring it.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- arch/arm/cpu/armv7/sunxi/fel_utils.S | 4 ---- arch/arm/mach-sunxi/board.c | 1 - 2 files changed, 5 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/fel_utils.S b/arch/arm/cpu/armv7/sunxi/fel_utils.S index b2310751d9..78bb1657fc 100644 --- a/arch/arm/cpu/armv7/sunxi/fel_utils.S +++ b/arch/arm/cpu/armv7/sunxi/fel_utils.S @@ -20,8 +20,6 @@ ENTRY(save_boot_params) str lr, [r0, #12] mrc p15, 0, lr, c12, c0, 0 @ Read VBAR str lr, [r0, #16] - mrc p15, 0, lr, c1, c0, 0 @ Read CP15 Control Register - str lr, [r0, #20] b save_boot_params_ret ENDPROC(save_boot_params)
@@ -29,8 +27,6 @@ ENTRY(return_to_fel) mov sp, r0 mov lr, r1 ldr r0, =fel_stash - ldr r1, [r0, #20] - mcr p15, 0, r1, c1, c0, 0 @ Write CP15 Control Register ldr r1, [r0, #16] mcr p15, 0, r1, c12, c0, 0 @ Write VBAR ldr r1, [r0, #12] diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c index 8f7c894286..385eac7a61 100644 --- a/arch/arm/mach-sunxi/board.c +++ b/arch/arm/mach-sunxi/board.c @@ -35,7 +35,6 @@ struct fel_stash { uint32_t cpsr; uint32_t sctlr; uint32_t vbar; - uint32_t cr; };
struct fel_stash fel_stash __section(".data");
On 7/13/22 10:27, Andre Przywara wrote:
For some reasons shrouded in mystery, the code saving the FEL state was saving the SCTLR register twice, with the second copy trying to justify itself by using its ancient "control register" alias name.
Drop the redundant second copy, both from the fel_stash data structure, and also the code saving and restoring it.
Signed-off-by: Andre Przywara andre.przywara@arm.com
arch/arm/cpu/armv7/sunxi/fel_utils.S | 4 ---- arch/arm/mach-sunxi/board.c | 1 - 2 files changed, 5 deletions(-)
Reviewed-by: Samuel Holland samuel@sholland.org Tested-by: Samuel Holland samuel@sholland.org
participants (2)
-
Andre Przywara -
Samuel Holland