[PATCH 0/3] sunxi: FEL boot fixes

[PATCH 0/3] sunxi: FEL boot fixes

[Andre Przywara]

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(-)

-- 
2.25.1

[PATCH 1/3] sunxi: armv8: fel: load only 32-bit values

[Andre Przywara]

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
-- 
2.25.1

[PATCH 2/3] sunxi: h616: lower SPL stack address to avoid BROM data

[Andre Przywara]

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
-- 
2.25.1

[PATCH 3/3] sunxi: fel: drop redundant "control register" save/restore

[Andre Przywara]

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");
-- 
2.25.1

Re: [PATCH 1/3] sunxi: armv8: fel: load only 32-bit values

[Samuel Holland]

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>

Re: [PATCH 3/3] sunxi: fel: drop redundant "control register" save/restore

[Samuel Holland]

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>

Re: [PATCH 2/3] sunxi: h616: lower SPL stack address to avoid BROM data

[Samuel Holland]

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>