[PATCH 0/5] sunxi: remove lowlevel_init

[PATCH 0/5] sunxi: remove lowlevel_init

[Andre Przywara]

For a long time the Allwinner platform was using the lowlevel_init()
routine in U-Boot's boot process, even though there did not seem to be
a real need for that. The README file suggests it should only be used
for "bare minimum" tasks to reach board_init_f(). The routines called
in arch/arm/mach-sunxi/board.c:s_init() did nothing that really
warranted the early and special execution. Since we always run with an
SPL, the board_init_f() routine in there seems to be the right place
for early peripherals initialisation.
Having those routines in s_init() actually forced other parts of U-Boot
to follow suit: for instance we added a lowlevel_init() function to the
armv8 side, just for sunxi. The same game seems to repeat for the
ARM9 support we need for the F1C100s chip, and possibly for the RISC-V
part as well.

To avoid this and to cleanup some code on the way, remove the need for
lowlevel_init for sunxi completely, by:
- moving the early peripheral init (clocks, timer, pinmux) to
  board_init_f(). This also avoids executing those bits a second time when
  we boot up U-Boot proper. (patch 1/5)
- move the "SRAM setup" bits for just a few old SoCs into a separate file,
  and include this only if needed. (patch 3/5)
- move the ACTLR.SMPEN bit setting from board.c into start.S, because we
  actually need to set this bit early. Other platforms should be able to
  scrap their own copies and piggy back on this change. (patch 2/5)
This allows us to actually remove the lowlevel_init.S file from armv8 in
patch 4/5, as sunxi was the only user.
For good measure, we also clean up some bits in sunxi_common.h. (patch 5/5)

I compile tested this for all sunxi boards, and boot tested (into Linux) on
Pine64, PineH64, BananaPi M1, OrangePi Zero. But there is a high chance of
something breaking, so please give it a good shake on whatever Allwinner
hardware you have access to.

Thanks,
Andre

Andre Przywara (5):
  sunxi: move non-essential code out of s_init()
  sunxi: move Cortex SMPEN setting into start.S
  sunxi: move early "SRAM setup" into separate file
  armv8: remove no longer needed lowlevel_init.S
  sunxi-common.h: remove pointless #ifdefs

 arch/arm/Kconfig                   |  3 --
 arch/arm/cpu/armv7/Kconfig         |  7 +++
 arch/arm/cpu/armv7/start.S         | 11 +++++
 arch/arm/cpu/armv7/sunxi/Makefile  |  3 ++
 arch/arm/cpu/armv7/sunxi/sram.c    | 45 +++++++++++++++++
 arch/arm/cpu/armv8/Makefile        |  1 -
 arch/arm/cpu/armv8/lowlevel_init.S | 43 ----------------
 arch/arm/mach-sunxi/Kconfig        |  2 -
 arch/arm/mach-sunxi/board.c        | 78 ++++++++----------------------
 include/configs/sunxi-common.h     |  7 ---
 10 files changed, 86 insertions(+), 114 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/sunxi/sram.c
 delete mode 100644 arch/arm/cpu/armv8/lowlevel_init.S

-- 
2.17.6

[PATCH 1/5] sunxi: move non-essential code out of s_init()

[Andre Przywara]

So far all Allwinner based boards were doing some not-so-lowlevel-setup
in lowlevel's s_init() routine.
This includes the initial clock, timer and pinmux setup, among other
things. This is clearly out of the "absolute bare minimum to get started"
scope that lowlevel_init.S suggests for this function.

Since we have an SPL, which is called right after s_init(), move those
calls to our board_init_f() function. As we overwrite this only for
the SPL, this has the added benefit of not doing this setup *again*
shortly afterwards, when running U-Boot proper.

This makes gpio_init() to be called from the SPL only, so pull this code
into a CONFIG_SPL_BUILD protected part to avoid build warnings.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/mach-sunxi/board.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
index 3ef179742c5..390c9f8850d 100644
--- a/arch/arm/mach-sunxi/board.c
+++ b/arch/arm/mach-sunxi/board.c
@@ -75,6 +75,7 @@ ulong board_get_usable_ram_top(ulong total_size)
 }
 #endif
 
+#ifdef CONFIG_SPL_BUILD
 static int gpio_init(void)
 {
 	__maybe_unused uint val;
@@ -172,7 +173,6 @@ static int gpio_init(void)
 	return 0;
 }
 
-#if defined(CONFIG_SPL_BOARD_LOAD_IMAGE) && defined(CONFIG_SPL_BUILD)
 static int spl_board_load_image(struct spl_image_info *spl_image,
 				struct spl_boot_device *bootdev)
 {
@@ -227,18 +227,6 @@ void s_init(void)
 		"mcr p15, 0, r0, c1, c0, 1\n"
 		::: "r0");
 #endif
-#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I_H3
-	/* Enable non-secure access to some peripherals */
-	tzpc_init();
-#endif
-
-	clock_init();
-	timer_init();
-	gpio_init();
-#if !CONFIG_IS_ENABLED(DM_I2C)
-	i2c_init_board();
-#endif
-	eth_init_board();
 }
 
 #define SUNXI_INVALID_BOOT_SOURCE	-1
@@ -335,6 +323,19 @@ u32 spl_boot_device(void)
 
 void board_init_f(ulong dummy)
 {
+#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I_H3
+	/* Enable non-secure access to some peripherals */
+	tzpc_init();
+#endif
+
+	clock_init();
+	timer_init();
+	gpio_init();
+#if !CONFIG_IS_ENABLED(DM_I2C)
+	i2c_init_board();
+#endif
+	eth_init_board();
+
 	spl_init();
 	preloader_console_init();
 
-- 
2.17.6

[PATCH 2/5] sunxi: move Cortex SMPEN setting into start.S

[Andre Przywara]

According to their TRMs, Cortex ARMv7 CPUs with SMP support require the
ACTLR.SMPEN bit to be set as early as possible, before any cache or TLB
maintenance operations are done. As we do those things still in start.S,
we need to move the SMPEN bit setting there, too.

This introduces a new ARMv7 wide symbol and code to set bit 6 in ACTLR
very early in start.S, and moves sunxi boards over to use that instead
of the custom code we had in our board.c file (where it was called
technically too late).

In practice we got away with this so far, because at this point all the
other cores were still in reset, so any broadcasting would have been
ignored anyway. But it is architecturally cleaner to do it early, and
we move a core specific piece of code out of board.c.

This also gets rid of the ARM_CORTEX_CPU_IS_UP kludge I introduced a few
years back, and moves the respective logic into the new Kconfig entry.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/Kconfig            |  3 ---
 arch/arm/cpu/armv7/Kconfig  |  7 +++++++
 arch/arm/cpu/armv7/start.S  | 11 +++++++++++
 arch/arm/mach-sunxi/Kconfig |  2 --
 arch/arm/mach-sunxi/board.c |  9 ---------
 5 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 6b11c3a50d9..7893d74fab2 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -452,9 +452,6 @@ config ENABLE_ARM_SOC_BOOT0_HOOK
 	  values, then choose this option, and create a file included as
 	  <asm/arch/boot0.h> which contains the required assembler code.
 
-config ARM_CORTEX_CPU_IS_UP
-	bool
-
 config USE_ARCH_MEMCPY
 	bool "Use an assembly optimized implementation of memcpy"
 	default y if !ARM64
diff --git a/arch/arm/cpu/armv7/Kconfig b/arch/arm/cpu/armv7/Kconfig
index 60bb0a9e1ec..85e5229a060 100644
--- a/arch/arm/cpu/armv7/Kconfig
+++ b/arch/arm/cpu/armv7/Kconfig
@@ -76,4 +76,11 @@ config ARMV7_LPAE
 	Say Y here to use the long descriptor page table format. This is
 	required if U-Boot runs in HYP mode.
 
+config ARMV7_SET_CORTEX_SMPEN
+	bool "Enable ACTLR.SMP bit"
+	default n if MACH_SUN4I || MACH_SUN5I
+	default y if ARCH_SUNXI && !ARM64
+	help
+	  Enable the ARM Cortex ACTLR.SMP enable bit on startup.
+
 endif
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
index 698e15b8e18..af87a5432ae 100644
--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -173,6 +173,17 @@ ENDPROC(switch_to_hypervisor)
  *
  *************************************************************************/
 ENTRY(cpu_init_cp15)
+
+#if CONFIG_IS_ENABLED(ARMV7_SET_CORTEX_SMPEN)
+	/*
+	 * The Arm Cortex-A7 TRM says this bit must be enabled before
+	 * "any cache or TLB maintenance operations are performed".
+	 */
+	mrc	p15, 0, r0, c1, c0, 1	@ read auxilary control register
+	orr	r0, r0, #1 << 6		@ set SMP bit to enable coherency
+	mcr	p15, 0, r0, c1, c0, 1	@ write auxilary control register
+#endif
+
 	/*
 	 * Invalidate L1 I/D
 	 */
diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index 2c18cf02d1a..4e49ad4f0c0 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -186,7 +186,6 @@ choice
 config MACH_SUN4I
 	bool "sun4i (Allwinner A10)"
 	select CPU_V7A
-	select ARM_CORTEX_CPU_IS_UP
 	select PHY_SUN4I_USB
 	select DRAM_SUN4I
 	select SUNXI_GEN_SUN4I
@@ -197,7 +196,6 @@ config MACH_SUN4I
 config MACH_SUN5I
 	bool "sun5i (Allwinner A13)"
 	select CPU_V7A
-	select ARM_CORTEX_CPU_IS_UP
 	select DRAM_SUN4I
 	select PHY_SUN4I_USB
 	select SUNXI_GEN_SUN4I
diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
index 390c9f8850d..8667ddf58e3 100644
--- a/arch/arm/mach-sunxi/board.c
+++ b/arch/arm/mach-sunxi/board.c
@@ -218,15 +218,6 @@ void s_init(void)
 	/* A83T BSP never modifies SUNXI_SRAMC_BASE + 0x44 */
 	/* No H3 BSP, boot0 seems to not modify SUNXI_SRAMC_BASE + 0x44 */
 #endif
-
-#if !defined(CONFIG_ARM_CORTEX_CPU_IS_UP) && !defined(CONFIG_ARM64)
-	/* Enable SMP mode for CPU0, by setting bit 6 of Auxiliary Ctl reg */
-	asm volatile(
-		"mrc p15, 0, r0, c1, c0, 1\n"
-		"orr r0, r0, #1 << 6\n"
-		"mcr p15, 0, r0, c1, c0, 1\n"
-		::: "r0");
-#endif
 }
 
 #define SUNXI_INVALID_BOOT_SOURCE	-1
-- 
2.17.6

[PATCH 3/5] sunxi: move early "SRAM setup" into separate file

[Andre Przywara]

Currently we do some magic "SRAM setup" MMIO writes in s_init(), copied
from the original BSP U-Boot. The comment speaks of this being required
before DRAM access gets enabled, but there is no indication that this
would actually be required that early.

Move this out of s_init(), into board_init_f(). Since this actually only
affects a very few older SoCs, the actual code goes into the cpu/armv7
directory, to move it out of the way for all other SoCs.

This also uses the opportunity to convert some #ifdefs over to the fancy
IS_ENABLED() macros used in actual C code.

We keep the s_init() stub around for now, since armv8's lowlevel_init
still relies on it.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/cpu/armv7/sunxi/Makefile |  3 +++
 arch/arm/cpu/armv7/sunxi/sram.c   | 45 +++++++++++++++++++++++++++++++
 arch/arm/mach-sunxi/board.c       | 38 +++++---------------------
 3 files changed, 54 insertions(+), 32 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/sunxi/sram.c

diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
index 1d40d6a18dc..ad11be78632 100644
--- a/arch/arm/cpu/armv7/sunxi/Makefile
+++ b/arch/arm/cpu/armv7/sunxi/Makefile
@@ -10,6 +10,9 @@ obj-y	+= timer.o
 obj-$(CONFIG_MACH_SUN6I)	+= tzpc.o
 obj-$(CONFIG_MACH_SUN8I_H3)	+= tzpc.o
 
+obj-$(CONFIG_MACH_SUN6I)	+= sram.o
+obj-$(CONFIG_MACH_SUN8I)	+= sram.o
+
 ifndef CONFIG_SPL_BUILD
 obj-$(CONFIG_ARMV7_PSCI)	+= psci.o
 endif
diff --git a/arch/arm/cpu/armv7/sunxi/sram.c b/arch/arm/cpu/armv7/sunxi/sram.c
new file mode 100644
index 00000000000..19395cce17c
--- /dev/null
+++ b/arch/arm/cpu/armv7/sunxi/sram.c
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2012 Henrik Nordstrom <henrik@henriknordstrom.net>
+ *
+ * (C) Copyright 2007-2011
+ * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
+ * Tom Cubie <tangliang@allwinnertech.com>
+ *
+ * SRAM init for older sunxi SoCs.
+ */
+
+#include <common.h>
+#include <init.h>
+#include <asm/io.h>
+
+void sunxi_sram_init(void)
+{
+	/*
+	 * Undocumented magic taken from boot0, without this DRAM
+	 * access gets messed up (seems cache related).
+	 * The boot0 sources describe this as: "config ema for cache sram"
+	 * Newer SoCs (A83T, H3 and anything beyond) don't need this anymore.
+	 */
+	if (IS_ENABLED(CONFIG_MACH_SUN6I))
+		setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
+
+	if (IS_ENABLED(CONFIG_MACH_SUN8I)) {
+		uint version;
+
+		/* Unlock sram version info reg, read it, relock */
+		setbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15));
+		version = readl(SUNXI_SRAMC_BASE + 0x24) >> 16;
+		clrbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15));
+
+		if (IS_ENABLED(CONFIG_MACH_SUN8I_A23)) {
+			if (version == 0x1650)
+				setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
+			else /* 0x1661 ? */
+				setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0);
+		} else if (IS_ENABLED(CONFIG_MACH_SUN8I_A33)) {
+			if (version != 0x1667)
+				setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0);
+		}
+	}
+}
diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
index 8667ddf58e3..42ec02d96e3 100644
--- a/arch/arm/mach-sunxi/board.c
+++ b/arch/arm/mach-sunxi/board.c
@@ -186,38 +186,6 @@ SPL_LOAD_IMAGE_METHOD("FEL", 0, BOOT_DEVICE_BOARD, spl_board_load_image);
 
 void s_init(void)
 {
-	/*
-	 * Undocumented magic taken from boot0, without this DRAM
-	 * access gets messed up (seems cache related).
-	 * The boot0 sources describe this as: "config ema for cache sram"
-	 */
-#if defined CONFIG_MACH_SUN6I
-	setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
-#elif defined CONFIG_MACH_SUN8I
-	__maybe_unused uint version;
-
-	/* Unlock sram version info reg, read it, relock */
-	setbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15));
-	version = readl(SUNXI_SRAMC_BASE + 0x24) >> 16;
-	clrbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15));
-
-	/*
-	 * Ideally this would be a switch case, but we do not know exactly
-	 * which versions there are and which version needs which settings,
-	 * so reproduce the per SoC code from the BSP.
-	 */
-#if defined CONFIG_MACH_SUN8I_A23
-	if (version == 0x1650)
-		setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
-	else /* 0x1661 ? */
-		setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0);
-#elif defined CONFIG_MACH_SUN8I_A33
-	if (version != 0x1667)
-		setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0);
-#endif
-	/* A83T BSP never modifies SUNXI_SRAMC_BASE + 0x44 */
-	/* No H3 BSP, boot0 seems to not modify SUNXI_SRAMC_BASE + 0x44 */
-#endif
 }
 
 #define SUNXI_INVALID_BOOT_SOURCE	-1
@@ -312,8 +280,14 @@ u32 spl_boot_device(void)
 	return sunxi_get_boot_device();
 }
 
+__weak void sunxi_sram_init(void)
+{
+}
+
 void board_init_f(ulong dummy)
 {
+	sunxi_sram_init();
+
 #if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I_H3
 	/* Enable non-secure access to some peripherals */
 	tzpc_init();
-- 
2.17.6

[PATCH 4/5] armv8: remove no longer needed lowlevel_init.S

[Andre Przywara]

When we added Allwinner SoC support to ARMv8, we needed to pull in an
implementation of lowlevel_init, as sunxi required it at this time.

The last few patches got rid of this bogus requirement, and as sunxi was
still the only user, we can now remove lowlevel_init from ARMv8
altogether.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/cpu/armv8/Makefile        |  1 -
 arch/arm/cpu/armv8/lowlevel_init.S | 43 ------------------------------
 arch/arm/mach-sunxi/board.c        |  4 ---
 3 files changed, 48 deletions(-)
 delete mode 100644 arch/arm/cpu/armv8/lowlevel_init.S

diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile
index d85ddde430a..85fe0475c86 100644
--- a/arch/arm/cpu/armv8/Makefile
+++ b/arch/arm/cpu/armv8/Makefile
@@ -42,6 +42,5 @@ obj-$(CONFIG_FSL_LAYERSCAPE) += fsl-layerscape/
 obj-$(CONFIG_S32V234) += s32v234/
 obj-$(CONFIG_TARGET_HIKEY) += hisilicon/
 obj-$(CONFIG_ARMV8_PSCI) += psci.o
-obj-$(CONFIG_ARCH_SUNXI) += lowlevel_init.o
 obj-$(CONFIG_TARGET_BCMNS3) += bcmns3/
 obj-$(CONFIG_XEN) += xen/
diff --git a/arch/arm/cpu/armv8/lowlevel_init.S b/arch/arm/cpu/armv8/lowlevel_init.S
deleted file mode 100644
index f4f0cdce9b3..00000000000
--- a/arch/arm/cpu/armv8/lowlevel_init.S
+++ /dev/null
@@ -1,43 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- * A lowlevel_init function that sets up the stack to call a C function to
- * perform further init.
- */
-
-#include <asm-offsets.h>
-#include <config.h>
-#include <linux/linkage.h>
-
-ENTRY(lowlevel_init)
-	/*
-	 * Setup a temporary stack. Global data is not available yet.
-	 */
-#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK)
-	ldr	w0, =CONFIG_SPL_STACK
-#else
-	ldr	w0, =CONFIG_SYS_INIT_SP_ADDR
-#endif
-	bic	sp, x0, #0xf	/* 16-byte alignment for ABI compliance */
-
-	/*
-	 * Save the old LR(passed in x29) and the current LR to stack
-	 */
-	stp	x29, x30, [sp, #-16]!
-
-	/*
-	 * Call the very early init function. This should do only the
-	 * absolute bare minimum to get started. It should not:
-	 *
-	 * - set up DRAM
-	 * - use global_data
-	 * - clear BSS
-	 * - try to start a console
-	 *
-	 * For boards with SPL this should be empty since SPL can do all of
-	 * this init in the SPL board_init_f() function which is called
-	 * immediately after this.
-	 */
-	bl	s_init
-	ldp	x29, x30, [sp]
-	ret
-ENDPROC(lowlevel_init)
diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
index 42ec02d96e3..a60dc6b299c 100644
--- a/arch/arm/mach-sunxi/board.c
+++ b/arch/arm/mach-sunxi/board.c
@@ -184,10 +184,6 @@ static int spl_board_load_image(struct spl_image_info *spl_image,
 SPL_LOAD_IMAGE_METHOD("FEL", 0, BOOT_DEVICE_BOARD, spl_board_load_image);
 #endif
 
-void s_init(void)
-{
-}
-
 #define SUNXI_INVALID_BOOT_SOURCE	-1
 
 static int sunxi_get_boot_source(void)
-- 
2.17.6

[PATCH 5/5] sunxi-common.h: remove pointless #ifdefs

[Andre Przywara]

Remove some pointless #ifdefs from this file, as there are quite too
many of them already.

Some definitions don't really hurt to have in any case, so remove the
pointless CONFIG_MMC guard around CONFIG_MMC_SUNXI_SLOT.

The BOARD_SIZE_LIMIT applies regardless of ARM64 or not (now), so remove
that guard as well. The maximum number of MMC devices does not depend on
CONFIG_ENV_IS_IN_MMC, so move that out to simplify the file.

Last but not least CONFIG_SPL_BOARD_LOAD_IMAGE serves no real purpose
anymore: it's unconditionally defined for all sunxi boards, and protects
nothing applicable outside of sunxi code anymore. Just remove it.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 include/configs/sunxi-common.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index 7260eb72a40..01c3cfced20 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -93,13 +93,9 @@
 #endif
 
 /* mmc config */
-#ifdef CONFIG_MMC
 #define CONFIG_MMC_SUNXI_SLOT		0
-#endif
 
 #if defined(CONFIG_ENV_IS_IN_MMC)
-
-#ifdef CONFIG_ARM64
 /*
  * This is actually (CONFIG_ENV_OFFSET -
  * (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512)), but the value will be used
@@ -109,7 +105,6 @@
 #endif
 
 #define CONFIG_SYS_MMC_MAX_DEVICE	4
-#endif
 
 /*
  * Miscellaneous configurable options
@@ -124,8 +119,6 @@
 
 #define CONFIG_SYS_MONITOR_LEN		(768 << 10)	/* 768 KiB */
 
-#define CONFIG_SPL_BOARD_LOAD_IMAGE
-
 /*
  * We cannot use expressions here, because expressions won't be evaluated in
  * autoconf.mk.
-- 
2.17.6

Re: [PATCH 1/5] sunxi: move non-essential code out of s_init()

[Samuel Holland]

On 1/24/22 7:15 PM, Andre Przywara wrote:
> So far all Allwinner based boards were doing some not-so-lowlevel-setup
> in lowlevel's s_init() routine.
> This includes the initial clock, timer and pinmux setup, among other
> things. This is clearly out of the "absolute bare minimum to get started"
> scope that lowlevel_init.S suggests for this function.
> 
> Since we have an SPL, which is called right after s_init(), move those
> calls to our board_init_f() function. As we overwrite this only for
> the SPL, this has the added benefit of not doing this setup *again*
> shortly afterwards, when running U-Boot proper.
> 
> This makes gpio_init() to be called from the SPL only, so pull this code
> into a CONFIG_SPL_BUILD protected part to avoid build warnings.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Reviewed-by: Samuel Holland <samuel@sholland.org>

> ---
>  arch/arm/mach-sunxi/board.c | 27 ++++++++++++++-------------
>  1 file changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> index 3ef179742c5..390c9f8850d 100644
> --- a/arch/arm/mach-sunxi/board.c
> +++ b/arch/arm/mach-sunxi/board.c
> @@ -75,6 +75,7 @@ ulong board_get_usable_ram_top(ulong total_size)
>  }
>  #endif
>  
> +#ifdef CONFIG_SPL_BUILD
>  static int gpio_init(void)
>  {
>  	__maybe_unused uint val;
> @@ -172,7 +173,6 @@ static int gpio_init(void)
>  	return 0;
>  }
>  
> -#if defined(CONFIG_SPL_BOARD_LOAD_IMAGE) && defined(CONFIG_SPL_BUILD)

As far as I can tell, this is the _only_ place that checks
CONFIG_SPL_BOARD_LOAD_IMAGE, so that definition could be removed from everywhere
now.

>  static int spl_board_load_image(struct spl_image_info *spl_image,
>  				struct spl_boot_device *bootdev)
>  {
> @@ -227,18 +227,6 @@ void s_init(void)
>  		"mcr p15, 0, r0, c1, c0, 1\n"
>  		::: "r0");
>  #endif
> -#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I_H3
> -	/* Enable non-secure access to some peripherals */
> -	tzpc_init();
> -#endif
> -
> -	clock_init();
> -	timer_init();
> -	gpio_init();
> -#if !CONFIG_IS_ENABLED(DM_I2C)
> -	i2c_init_board();
> -#endif
> -	eth_init_board();
>  }
>  
>  #define SUNXI_INVALID_BOOT_SOURCE	-1
> @@ -335,6 +323,19 @@ u32 spl_boot_device(void)
>  
>  void board_init_f(ulong dummy)
>  {
> +#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I_H3
> +	/* Enable non-secure access to some peripherals */
> +	tzpc_init();
> +#endif
> +
> +	clock_init();
> +	timer_init();
> +	gpio_init();
> +#if !CONFIG_IS_ENABLED(DM_I2C)
> +	i2c_init_board();

You could move this down a few lines, near the call to i2c_init, inside the
existing #if.

> +#endif
> +	eth_init_board();
> +
>  	spl_init();
>  	preloader_console_init();
>  
> 

Re: [PATCH 2/5] sunxi: move Cortex SMPEN setting into start.S

[Samuel Holland]

On 1/24/22 7:15 PM, Andre Przywara wrote:
> According to their TRMs, Cortex ARMv7 CPUs with SMP support require the
> ACTLR.SMPEN bit to be set as early as possible, before any cache or TLB
> maintenance operations are done. As we do those things still in start.S,
> we need to move the SMPEN bit setting there, too.
> 
> This introduces a new ARMv7 wide symbol and code to set bit 6 in ACTLR
> very early in start.S, and moves sunxi boards over to use that instead
> of the custom code we had in our board.c file (where it was called
> technically too late).
> 
> In practice we got away with this so far, because at this point all the
> other cores were still in reset, so any broadcasting would have been
> ignored anyway. But it is architecturally cleaner to do it early, and
> we move a core specific piece of code out of board.c.
> 
> This also gets rid of the ARM_CORTEX_CPU_IS_UP kludge I introduced a few
> years back, and moves the respective logic into the new Kconfig entry.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm/Kconfig            |  3 ---
>  arch/arm/cpu/armv7/Kconfig  |  7 +++++++
>  arch/arm/cpu/armv7/start.S  | 11 +++++++++++
>  arch/arm/mach-sunxi/Kconfig |  2 --
>  arch/arm/mach-sunxi/board.c |  9 ---------
>  5 files changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 6b11c3a50d9..7893d74fab2 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -452,9 +452,6 @@ config ENABLE_ARM_SOC_BOOT0_HOOK
>  	  values, then choose this option, and create a file included as
>  	  <asm/arch/boot0.h> which contains the required assembler code.
>  
> -config ARM_CORTEX_CPU_IS_UP
> -	bool
> -
>  config USE_ARCH_MEMCPY
>  	bool "Use an assembly optimized implementation of memcpy"
>  	default y if !ARM64
> diff --git a/arch/arm/cpu/armv7/Kconfig b/arch/arm/cpu/armv7/Kconfig
> index 60bb0a9e1ec..85e5229a060 100644
> --- a/arch/arm/cpu/armv7/Kconfig
> +++ b/arch/arm/cpu/armv7/Kconfig
> @@ -76,4 +76,11 @@ config ARMV7_LPAE
>  	Say Y here to use the long descriptor page table format. This is
>  	required if U-Boot runs in HYP mode.
>  
> +config ARMV7_SET_CORTEX_SMPEN
> +	bool "Enable ACTLR.SMP bit"
> +	default n if MACH_SUN4I || MACH_SUN5I
> +	default y if ARCH_SUNXI && !ARM64

If this is required by the CPU, why is it shown to the user? Shouldn't it be
selected by MACH_SUN6/7/8/9I? That is what I see platforms doing with
ARMV8_SET_SMPEN.

> +	help
> +	  Enable the ARM Cortex ACTLR.SMP enable bit on startup.
> +
>  endif
> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> index 698e15b8e18..af87a5432ae 100644
> --- a/arch/arm/cpu/armv7/start.S
> +++ b/arch/arm/cpu/armv7/start.S
> @@ -173,6 +173,17 @@ ENDPROC(switch_to_hypervisor)
>   *
>   *************************************************************************/
>  ENTRY(cpu_init_cp15)
> +
> +#if CONFIG_IS_ENABLED(ARMV7_SET_CORTEX_SMPEN)
> +	/*
> +	 * The Arm Cortex-A7 TRM says this bit must be enabled before
> +	 * "any cache or TLB maintenance operations are performed".
> +	 */
> +	mrc	p15, 0, r0, c1, c0, 1	@ read auxilary control register
> +	orr	r0, r0, #1 << 6		@ set SMP bit to enable coherency
> +	mcr	p15, 0, r0, c1, c0, 1	@ write auxilary control register
> +#endif
> +
>  	/*
>  	 * Invalidate L1 I/D
>  	 */
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index 2c18cf02d1a..4e49ad4f0c0 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -186,7 +186,6 @@ choice
>  config MACH_SUN4I
>  	bool "sun4i (Allwinner A10)"
>  	select CPU_V7A
> -	select ARM_CORTEX_CPU_IS_UP
>  	select PHY_SUN4I_USB
>  	select DRAM_SUN4I
>  	select SUNXI_GEN_SUN4I
> @@ -197,7 +196,6 @@ config MACH_SUN4I
>  config MACH_SUN5I
>  	bool "sun5i (Allwinner A13)"
>  	select CPU_V7A
> -	select ARM_CORTEX_CPU_IS_UP
>  	select DRAM_SUN4I
>  	select PHY_SUN4I_USB
>  	select SUNXI_GEN_SUN4I
> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> index 390c9f8850d..8667ddf58e3 100644
> --- a/arch/arm/mach-sunxi/board.c
> +++ b/arch/arm/mach-sunxi/board.c
> @@ -218,15 +218,6 @@ void s_init(void)
>  	/* A83T BSP never modifies SUNXI_SRAMC_BASE + 0x44 */
>  	/* No H3 BSP, boot0 seems to not modify SUNXI_SRAMC_BASE + 0x44 */
>  #endif
> -
> -#if !defined(CONFIG_ARM_CORTEX_CPU_IS_UP) && !defined(CONFIG_ARM64)
> -	/* Enable SMP mode for CPU0, by setting bit 6 of Auxiliary Ctl reg */
> -	asm volatile(
> -		"mrc p15, 0, r0, c1, c0, 1\n"
> -		"orr r0, r0, #1 << 6\n"
> -		"mcr p15, 0, r0, c1, c0, 1\n"
> -		::: "r0");
> -#endif
>  }
>  
>  #define SUNXI_INVALID_BOOT_SOURCE	-1
> 

Re: [PATCH 3/5] sunxi: move early "SRAM setup" into separate file

[Samuel Holland]

On 1/24/22 7:15 PM, Andre Przywara wrote:
> Currently we do some magic "SRAM setup" MMIO writes in s_init(), copied
> from the original BSP U-Boot. The comment speaks of this being required
> before DRAM access gets enabled, but there is no indication that this
> would actually be required that early.
> 
> Move this out of s_init(), into board_init_f(). Since this actually only
> affects a very few older SoCs, the actual code goes into the cpu/armv7
> directory, to move it out of the way for all other SoCs.
> 
> This also uses the opportunity to convert some #ifdefs over to the fancy
> IS_ENABLED() macros used in actual C code.
> 
> We keep the s_init() stub around for now, since armv8's lowlevel_init
> still relies on it.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm/cpu/armv7/sunxi/Makefile |  3 +++
>  arch/arm/cpu/armv7/sunxi/sram.c   | 45 +++++++++++++++++++++++++++++++
>  arch/arm/mach-sunxi/board.c       | 38 +++++---------------------
>  3 files changed, 54 insertions(+), 32 deletions(-)
>  create mode 100644 arch/arm/cpu/armv7/sunxi/sram.c
> 
> diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
> index 1d40d6a18dc..ad11be78632 100644
> --- a/arch/arm/cpu/armv7/sunxi/Makefile
> +++ b/arch/arm/cpu/armv7/sunxi/Makefile
> @@ -10,6 +10,9 @@ obj-y	+= timer.o
>  obj-$(CONFIG_MACH_SUN6I)	+= tzpc.o
>  obj-$(CONFIG_MACH_SUN8I_H3)	+= tzpc.o
>  
> +obj-$(CONFIG_MACH_SUN6I)	+= sram.o
> +obj-$(CONFIG_MACH_SUN8I)	+= sram.o
> +
>  ifndef CONFIG_SPL_BUILD
>  obj-$(CONFIG_ARMV7_PSCI)	+= psci.o
>  endif
> diff --git a/arch/arm/cpu/armv7/sunxi/sram.c b/arch/arm/cpu/armv7/sunxi/sram.c
> new file mode 100644
> index 00000000000..19395cce17c
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/sunxi/sram.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2012 Henrik Nordstrom <henrik@henriknordstrom.net>
> + *
> + * (C) Copyright 2007-2011
> + * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
> + * Tom Cubie <tangliang@allwinnertech.com>
> + *
> + * SRAM init for older sunxi SoCs.
> + */
> +
> +#include <common.h>
> +#include <init.h>
> +#include <asm/io.h>
> +
> +void sunxi_sram_init(void)
> +{
> +	/*
> +	 * Undocumented magic taken from boot0, without this DRAM
> +	 * access gets messed up (seems cache related).
> +	 * The boot0 sources describe this as: "config ema for cache sram"
> +	 * Newer SoCs (A83T, H3 and anything beyond) don't need this anymore.
> +	 */
> +	if (IS_ENABLED(CONFIG_MACH_SUN6I))
> +		setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
> +
> +	if (IS_ENABLED(CONFIG_MACH_SUN8I)) {
> +		uint version;
> +
> +		/* Unlock sram version info reg, read it, relock */
> +		setbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15));
> +		version = readl(SUNXI_SRAMC_BASE + 0x24) >> 16;
> +		clrbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15));

This is an open-coded version of sunxi_get_sram_id().

> +
> +		if (IS_ENABLED(CONFIG_MACH_SUN8I_A23)) {
> +			if (version == 0x1650)
> +				setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
> +			else /* 0x1661 ? */
> +				setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0);
> +		} else if (IS_ENABLED(CONFIG_MACH_SUN8I_A33)) {
> +			if (version != 0x1667)
> +				setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0);
> +		}
> +	}
> +}
> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> index 8667ddf58e3..42ec02d96e3 100644
> --- a/arch/arm/mach-sunxi/board.c
> +++ b/arch/arm/mach-sunxi/board.c
> @@ -186,38 +186,6 @@ SPL_LOAD_IMAGE_METHOD("FEL", 0, BOOT_DEVICE_BOARD, spl_board_load_image);
>  
>  void s_init(void)
>  {
> -	/*
> -	 * Undocumented magic taken from boot0, without this DRAM
> -	 * access gets messed up (seems cache related).
> -	 * The boot0 sources describe this as: "config ema for cache sram"
> -	 */
> -#if defined CONFIG_MACH_SUN6I
> -	setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
> -#elif defined CONFIG_MACH_SUN8I
> -	__maybe_unused uint version;
> -
> -	/* Unlock sram version info reg, read it, relock */
> -	setbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15));
> -	version = readl(SUNXI_SRAMC_BASE + 0x24) >> 16;
> -	clrbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15));
> -
> -	/*
> -	 * Ideally this would be a switch case, but we do not know exactly
> -	 * which versions there are and which version needs which settings,
> -	 * so reproduce the per SoC code from the BSP.
> -	 */
> -#if defined CONFIG_MACH_SUN8I_A23
> -	if (version == 0x1650)
> -		setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
> -	else /* 0x1661 ? */
> -		setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0);
> -#elif defined CONFIG_MACH_SUN8I_A33
> -	if (version != 0x1667)
> -		setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0);
> -#endif
> -	/* A83T BSP never modifies SUNXI_SRAMC_BASE + 0x44 */
> -	/* No H3 BSP, boot0 seems to not modify SUNXI_SRAMC_BASE + 0x44 */
> -#endif
>  }
>  
>  #define SUNXI_INVALID_BOOT_SOURCE	-1
> @@ -312,8 +280,14 @@ u32 spl_boot_device(void)
>  	return sunxi_get_boot_device();
>  }
>  
> +__weak void sunxi_sram_init(void)
> +{
> +}

In configurations other than MACH_SUN6/8I, the strong definition of
sunxi_sram_init() is also an empty function, so this is no more efficient than
unconditionally compiling sram.c.

> +
>  void board_init_f(ulong dummy)
>  {
> +	sunxi_sram_init();
> +
>  #if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I_H3
>  	/* Enable non-secure access to some peripherals */
>  	tzpc_init();
> 

Re: [PATCH 4/5] armv8: remove no longer needed lowlevel_init.S

[Samuel Holland]

On 1/24/22 7:15 PM, Andre Przywara wrote:
> When we added Allwinner SoC support to ARMv8, we needed to pull in an
> implementation of lowlevel_init, as sunxi required it at this time.
> 
> The last few patches got rid of this bogus requirement, and as sunxi was
> still the only user, we can now remove lowlevel_init from ARMv8
> altogether.

Well, we are removing lowlevel_init.S anyway. There is still a weak definition
of lowlevel_init in arch/arm/cpu/armv8/start.S, which we are now picking up.

> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Reviewed-by: Samuel Holland <samuel@sholland.org>

> ---
>  arch/arm/cpu/armv8/Makefile        |  1 -
>  arch/arm/cpu/armv8/lowlevel_init.S | 43 ------------------------------
>  arch/arm/mach-sunxi/board.c        |  4 ---
>  3 files changed, 48 deletions(-)
>  delete mode 100644 arch/arm/cpu/armv8/lowlevel_init.S
> 
> diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile
> index d85ddde430a..85fe0475c86 100644
> --- a/arch/arm/cpu/armv8/Makefile
> +++ b/arch/arm/cpu/armv8/Makefile
> @@ -42,6 +42,5 @@ obj-$(CONFIG_FSL_LAYERSCAPE) += fsl-layerscape/
>  obj-$(CONFIG_S32V234) += s32v234/
>  obj-$(CONFIG_TARGET_HIKEY) += hisilicon/
>  obj-$(CONFIG_ARMV8_PSCI) += psci.o
> -obj-$(CONFIG_ARCH_SUNXI) += lowlevel_init.o
>  obj-$(CONFIG_TARGET_BCMNS3) += bcmns3/
>  obj-$(CONFIG_XEN) += xen/
> diff --git a/arch/arm/cpu/armv8/lowlevel_init.S b/arch/arm/cpu/armv8/lowlevel_init.S
> deleted file mode 100644
> index f4f0cdce9b3..00000000000
> --- a/arch/arm/cpu/armv8/lowlevel_init.S
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0+ */
> -/*
> - * A lowlevel_init function that sets up the stack to call a C function to
> - * perform further init.
> - */
> -
> -#include <asm-offsets.h>
> -#include <config.h>
> -#include <linux/linkage.h>
> -
> -ENTRY(lowlevel_init)
> -	/*
> -	 * Setup a temporary stack. Global data is not available yet.
> -	 */
> -#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK)
> -	ldr	w0, =CONFIG_SPL_STACK
> -#else
> -	ldr	w0, =CONFIG_SYS_INIT_SP_ADDR
> -#endif
> -	bic	sp, x0, #0xf	/* 16-byte alignment for ABI compliance */
> -
> -	/*
> -	 * Save the old LR(passed in x29) and the current LR to stack
> -	 */
> -	stp	x29, x30, [sp, #-16]!
> -
> -	/*
> -	 * Call the very early init function. This should do only the
> -	 * absolute bare minimum to get started. It should not:
> -	 *
> -	 * - set up DRAM
> -	 * - use global_data
> -	 * - clear BSS
> -	 * - try to start a console
> -	 *
> -	 * For boards with SPL this should be empty since SPL can do all of
> -	 * this init in the SPL board_init_f() function which is called
> -	 * immediately after this.
> -	 */
> -	bl	s_init
> -	ldp	x29, x30, [sp]
> -	ret
> -ENDPROC(lowlevel_init)
> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> index 42ec02d96e3..a60dc6b299c 100644
> --- a/arch/arm/mach-sunxi/board.c
> +++ b/arch/arm/mach-sunxi/board.c
> @@ -184,10 +184,6 @@ static int spl_board_load_image(struct spl_image_info *spl_image,
>  SPL_LOAD_IMAGE_METHOD("FEL", 0, BOOT_DEVICE_BOARD, spl_board_load_image);
>  #endif
>  
> -void s_init(void)
> -{
> -}
> -
>  #define SUNXI_INVALID_BOOT_SOURCE	-1
>  
>  static int sunxi_get_boot_source(void)
> 

Re: [PATCH 5/5] sunxi-common.h: remove pointless #ifdefs

[Samuel Holland]

On 1/24/22 7:15 PM, Andre Przywara wrote:
> Remove some pointless #ifdefs from this file, as there are quite too
> many of them already.
> 
> Some definitions don't really hurt to have in any case, so remove the
> pointless CONFIG_MMC guard around CONFIG_MMC_SUNXI_SLOT.
> 
> The BOARD_SIZE_LIMIT applies regardless of ARM64 or not (now), so remove
> that guard as well. The maximum number of MMC devices does not depend on
> CONFIG_ENV_IS_IN_MMC, so move that out to simplify the file.
> 
> Last but not least CONFIG_SPL_BOARD_LOAD_IMAGE serves no real purpose
> anymore: it's unconditionally defined for all sunxi boards, and protects
> nothing applicable outside of sunxi code anymore. Just remove it.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Reviewed-by: Samuel Holland <samuel@sholland.org>

Re: [PATCH 2/5] sunxi: move Cortex SMPEN setting into start.S

[Andre Przywara]

On Mon, 24 Jan 2022 20:47:30 -0600
Samuel Holland <samuel@sholland.org> wrote:

Hi Samuel,

many thanks for having a look!

> On 1/24/22 7:15 PM, Andre Przywara wrote:
> > According to their TRMs, Cortex ARMv7 CPUs with SMP support require the
> > ACTLR.SMPEN bit to be set as early as possible, before any cache or TLB
> > maintenance operations are done. As we do those things still in start.S,
> > we need to move the SMPEN bit setting there, too.
> > 
> > This introduces a new ARMv7 wide symbol and code to set bit 6 in ACTLR
> > very early in start.S, and moves sunxi boards over to use that instead
> > of the custom code we had in our board.c file (where it was called
> > technically too late).
> > 
> > In practice we got away with this so far, because at this point all the
> > other cores were still in reset, so any broadcasting would have been
> > ignored anyway. But it is architecturally cleaner to do it early, and
> > we move a core specific piece of code out of board.c.
> > 
> > This also gets rid of the ARM_CORTEX_CPU_IS_UP kludge I introduced a few
> > years back, and moves the respective logic into the new Kconfig entry.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  arch/arm/Kconfig            |  3 ---
> >  arch/arm/cpu/armv7/Kconfig  |  7 +++++++
> >  arch/arm/cpu/armv7/start.S  | 11 +++++++++++
> >  arch/arm/mach-sunxi/Kconfig |  2 --
> >  arch/arm/mach-sunxi/board.c |  9 ---------
> >  5 files changed, 18 insertions(+), 14 deletions(-)
> > 
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index 6b11c3a50d9..7893d74fab2 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -452,9 +452,6 @@ config ENABLE_ARM_SOC_BOOT0_HOOK
> >  	  values, then choose this option, and create a file included as
> >  	  <asm/arch/boot0.h> which contains the required assembler code.
> >  
> > -config ARM_CORTEX_CPU_IS_UP
> > -	bool
> > -
> >  config USE_ARCH_MEMCPY
> >  	bool "Use an assembly optimized implementation of memcpy"
> >  	default y if !ARM64
> > diff --git a/arch/arm/cpu/armv7/Kconfig b/arch/arm/cpu/armv7/Kconfig
> > index 60bb0a9e1ec..85e5229a060 100644
> > --- a/arch/arm/cpu/armv7/Kconfig
> > +++ b/arch/arm/cpu/armv7/Kconfig
> > @@ -76,4 +76,11 @@ config ARMV7_LPAE
> >  	Say Y here to use the long descriptor page table format. This is
> >  	required if U-Boot runs in HYP mode.
> >  
> > +config ARMV7_SET_CORTEX_SMPEN
> > +	bool "Enable ACTLR.SMP bit"
> > +	default n if MACH_SUN4I || MACH_SUN5I
> > +	default y if ARCH_SUNXI && !ARM64  
> 
> If this is required by the CPU, why is it shown to the user? Shouldn't it be
> selected by MACH_SUN6/7/8/9I? That is what I see platforms doing with
> ARMV8_SET_SMPEN.

You are right, it shouldn't be user visible, instead selected by platform
Kconfigs. Technically we could even work that out at runtime (read MIDR &
MPIDR), but I think there are situations were we want to avoid this, so we
would probably need an opt-in from the platform anyways.
This patch was just trying to mimic the existing logic as close as
possible, since we know that works for years. Changing this is scary
enough. And the negative logic is shorter than enumerating all ARMv7 SMP
cores, the list of which could even grow.

But indeed there is more cleanup potential here. I see some platforms
setting this bit in their code (similar to what sunxi did), but I didn't
dare to touch those. Maybe I add a follow up patch to move those platforms
over, and possibly merge this with ARMV8_SET_SMPEN on the way.

Cheers,
Andre

> > +	help
> > +	  Enable the ARM Cortex ACTLR.SMP enable bit on startup.
> > +
> >  endif
> > diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> > index 698e15b8e18..af87a5432ae 100644
> > --- a/arch/arm/cpu/armv7/start.S
> > +++ b/arch/arm/cpu/armv7/start.S
> > @@ -173,6 +173,17 @@ ENDPROC(switch_to_hypervisor)
> >   *
> >   *************************************************************************/
> >  ENTRY(cpu_init_cp15)
> > +
> > +#if CONFIG_IS_ENABLED(ARMV7_SET_CORTEX_SMPEN)
> > +	/*
> > +	 * The Arm Cortex-A7 TRM says this bit must be enabled before
> > +	 * "any cache or TLB maintenance operations are performed".
> > +	 */
> > +	mrc	p15, 0, r0, c1, c0, 1	@ read auxilary control register
> > +	orr	r0, r0, #1 << 6		@ set SMP bit to enable coherency
> > +	mcr	p15, 0, r0, c1, c0, 1	@ write auxilary control register
> > +#endif
> > +
> >  	/*
> >  	 * Invalidate L1 I/D
> >  	 */
> > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> > index 2c18cf02d1a..4e49ad4f0c0 100644
> > --- a/arch/arm/mach-sunxi/Kconfig
> > +++ b/arch/arm/mach-sunxi/Kconfig
> > @@ -186,7 +186,6 @@ choice
> >  config MACH_SUN4I
> >  	bool "sun4i (Allwinner A10)"
> >  	select CPU_V7A
> > -	select ARM_CORTEX_CPU_IS_UP
> >  	select PHY_SUN4I_USB
> >  	select DRAM_SUN4I
> >  	select SUNXI_GEN_SUN4I
> > @@ -197,7 +196,6 @@ config MACH_SUN4I
> >  config MACH_SUN5I
> >  	bool "sun5i (Allwinner A13)"
> >  	select CPU_V7A
> > -	select ARM_CORTEX_CPU_IS_UP
> >  	select DRAM_SUN4I
> >  	select PHY_SUN4I_USB
> >  	select SUNXI_GEN_SUN4I
> > diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> > index 390c9f8850d..8667ddf58e3 100644
> > --- a/arch/arm/mach-sunxi/board.c
> > +++ b/arch/arm/mach-sunxi/board.c
> > @@ -218,15 +218,6 @@ void s_init(void)
> >  	/* A83T BSP never modifies SUNXI_SRAMC_BASE + 0x44 */
> >  	/* No H3 BSP, boot0 seems to not modify SUNXI_SRAMC_BASE + 0x44 */
> >  #endif
> > -
> > -#if !defined(CONFIG_ARM_CORTEX_CPU_IS_UP) && !defined(CONFIG_ARM64)
> > -	/* Enable SMP mode for CPU0, by setting bit 6 of Auxiliary Ctl reg */
> > -	asm volatile(
> > -		"mrc p15, 0, r0, c1, c0, 1\n"
> > -		"orr r0, r0, #1 << 6\n"
> > -		"mcr p15, 0, r0, c1, c0, 1\n"
> > -		::: "r0");
> > -#endif
> >  }
> >  
> >  #define SUNXI_INVALID_BOOT_SOURCE	-1
> >   
> 

Re: [PATCH 0/5] sunxi: remove lowlevel_init

[Jesse Taube]

On 1/24/22 20:15, Andre Przywara wrote:
> For a long time the Allwinner platform was using the lowlevel_init()
> routine in U-Boot's boot process, even though there did not seem to be
> a real need for that. The README file suggests it should only be used
> for "bare minimum" tasks to reach board_init_f(). The routines called
> in arch/arm/mach-sunxi/board.c:s_init() did nothing that really
> warranted the early and special execution. Since we always run with an
> SPL, the board_init_f() routine in there seems to be the right place
> for early peripherals initialisation.
> Having those routines in s_init() actually forced other parts of U-Boot
> to follow suit: for instance we added a lowlevel_init() function to the
> armv8 side, just for sunxi. The same game seems to repeat for the
> ARM9 support we need for the F1C100s chip, and possibly for the RISC-V
> part as well.
> 
> To avoid this and to cleanup some code on the way, remove the need for
> lowlevel_init for sunxi completely, by:
> - moving the early peripheral init (clocks, timer, pinmux) to
>    board_init_f(). This also avoids executing those bits a second time when
>    we boot up U-Boot proper. (patch 1/5)
> - move the "SRAM setup" bits for just a few old SoCs into a separate file,
>    and include this only if needed. (patch 3/5)
> - move the ACTLR.SMPEN bit setting from board.c into start.S, because we
>    actually need to set this bit early. Other platforms should be able to
>    scrap their own copies and piggy back on this change. (patch 2/5)
> This allows us to actually remove the lowlevel_init.S file from armv8 in
> patch 4/5, as sunxi was the only user.
> For good measure, we also clean up some bits in sunxi_common.h. (patch 5/5)
> 
> I compile tested this for all sunxi boards, and boot tested (into Linux) on
> Pine64, PineH64, BananaPi M1, OrangePi Zero. But there is a high chance of
> something breaking, so please give it a good shake on whatever Allwinner
> hardware you have access to.
> 
> Thanks,
> Andre
> 
> Andre Przywara (5):
>    sunxi: move non-essential code out of s_init()
>    sunxi: move Cortex SMPEN setting into start.S
>    sunxi: move early "SRAM setup" into separate file
>    armv8: remove no longer needed lowlevel_init.S
>    sunxi-common.h: remove pointless #ifdefs
> 
>   arch/arm/Kconfig                   |  3 --
>   arch/arm/cpu/armv7/Kconfig         |  7 +++
>   arch/arm/cpu/armv7/start.S         | 11 +++++
>   arch/arm/cpu/armv7/sunxi/Makefile  |  3 ++
>   arch/arm/cpu/armv7/sunxi/sram.c    | 45 +++++++++++++++++
>   arch/arm/cpu/armv8/Makefile        |  1 -
>   arch/arm/cpu/armv8/lowlevel_init.S | 43 ----------------
>   arch/arm/mach-sunxi/Kconfig        |  2 -
>   arch/arm/mach-sunxi/board.c        | 78 ++++++++----------------------
>   include/configs/sunxi-common.h     |  7 ---
>   10 files changed, 86 insertions(+), 114 deletions(-)
>   create mode 100644 arch/arm/cpu/armv7/sunxi/sram.c
>   delete mode 100644 arch/arm/cpu/armv8/lowlevel_init.S
> 

Tested-By: Jesse Taube <Mr.Bossman075@gmail.com>

Tested on Lamobo_R1_defconfig.

Thanks,
	Jesse Taube

Re: [PATCH 0/5] sunxi: remove lowlevel_init

[Jesse Taube]

On 1/25/22 18:47, Jesse Taube wrote:
> 
> 
> On 1/24/22 20:15, Andre Przywara wrote:
>> For a long time the Allwinner platform was using the lowlevel_init()
>> routine in U-Boot's boot process, even though there did not seem to be
>> a real need for that. The README file suggests it should only be used
>> for "bare minimum" tasks to reach board_init_f(). The routines called
>> in arch/arm/mach-sunxi/board.c:s_init() did nothing that really
>> warranted the early and special execution. Since we always run with an
>> SPL, the board_init_f() routine in there seems to be the right place
>> for early peripherals initialisation.
>> Having those routines in s_init() actually forced other parts of U-Boot
>> to follow suit: for instance we added a lowlevel_init() function to the
>> armv8 side, just for sunxi. The same game seems to repeat for the
>> ARM9 support we need for the F1C100s chip, and possibly for the RISC-V
>> part as well.
>>
>> To avoid this and to cleanup some code on the way, remove the need for
>> lowlevel_init for sunxi completely, by:
>> - moving the early peripheral init (clocks, timer, pinmux) to
>>     board_init_f(). This also avoids executing those bits a second time when
>>     we boot up U-Boot proper. (patch 1/5)
>> - move the "SRAM setup" bits for just a few old SoCs into a separate file,
>>     and include this only if needed. (patch 3/5)
>> - move the ACTLR.SMPEN bit setting from board.c into start.S, because we
>>     actually need to set this bit early. Other platforms should be able to
>>     scrap their own copies and piggy back on this change. (patch 2/5)
>> This allows us to actually remove the lowlevel_init.S file from armv8 in
>> patch 4/5, as sunxi was the only user.
>> For good measure, we also clean up some bits in sunxi_common.h. (patch 5/5)
>>
>> I compile tested this for all sunxi boards, and boot tested (into Linux) on
>> Pine64, PineH64, BananaPi M1, OrangePi Zero. But there is a high chance of
>> something breaking, so please give it a good shake on whatever Allwinner
>> hardware you have access to.
>>
>> Thanks,
>> Andre
>>
>> Andre Przywara (5):
>>     sunxi: move non-essential code out of s_init()
>>     sunxi: move Cortex SMPEN setting into start.S
>>     sunxi: move early "SRAM setup" into separate file
>>     armv8: remove no longer needed lowlevel_init.S
>>     sunxi-common.h: remove pointless #ifdefs
>>
>>    arch/arm/Kconfig                   |  3 --
>>    arch/arm/cpu/armv7/Kconfig         |  7 +++
>>    arch/arm/cpu/armv7/start.S         | 11 +++++
>>    arch/arm/cpu/armv7/sunxi/Makefile  |  3 ++
>>    arch/arm/cpu/armv7/sunxi/sram.c    | 45 +++++++++++++++++
>>    arch/arm/cpu/armv8/Makefile        |  1 -
>>    arch/arm/cpu/armv8/lowlevel_init.S | 43 ----------------
>>    arch/arm/mach-sunxi/Kconfig        |  2 -
>>    arch/arm/mach-sunxi/board.c        | 78 ++++++++----------------------
>>    include/configs/sunxi-common.h     |  7 ---
>>    10 files changed, 86 insertions(+), 114 deletions(-)
>>    create mode 100644 arch/arm/cpu/armv7/sunxi/sram.c
>>    delete mode 100644 arch/arm/cpu/armv8/lowlevel_init.S
>>
> 
> Tested-By: Jesse Taube <Mr.Bossman075@gmail.com>
> 
> Tested on Lamobo_R1_defconfig.

Tested on Lamobo_R1_defconfig and f1c100s.

> 
> Thanks,
> 	Jesse Taube

Re: [PATCH 1/5] sunxi: move non-essential code out of s_init()

[Simon Glass]

On Mon, 24 Jan 2022 at 18:15, Andre Przywara <andre.przywara@arm.com> wrote:
>
> So far all Allwinner based boards were doing some not-so-lowlevel-setup
> in lowlevel's s_init() routine.
> This includes the initial clock, timer and pinmux setup, among other
> things. This is clearly out of the "absolute bare minimum to get started"
> scope that lowlevel_init.S suggests for this function.
>
> Since we have an SPL, which is called right after s_init(), move those
> calls to our board_init_f() function. As we overwrite this only for
> the SPL, this has the added benefit of not doing this setup *again*
> shortly afterwards, when running U-Boot proper.
>
> This makes gpio_init() to be called from the SPL only, so pull this code
> into a CONFIG_SPL_BUILD protected part to avoid build warnings.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm/mach-sunxi/board.c | 27 ++++++++++++++-------------
>  1 file changed, 14 insertions(+), 13 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Tested on: orangepi_pc2, orangepi_pc, A20-OLinuXino_MICRO
Tested-by: Simon Glass <sjg@chromium.org>


>
> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> index 3ef179742c5..390c9f8850d 100644
> --- a/arch/arm/mach-sunxi/board.c
> +++ b/arch/arm/mach-sunxi/board.c
> @@ -75,6 +75,7 @@ ulong board_get_usable_ram_top(ulong total_size)
>  }
>  #endif
>
> +#ifdef CONFIG_SPL_BUILD
>  static int gpio_init(void)
>  {
>         __maybe_unused uint val;
> @@ -172,7 +173,6 @@ static int gpio_init(void)
>         return 0;
>  }
>
> -#if defined(CONFIG_SPL_BOARD_LOAD_IMAGE) && defined(CONFIG_SPL_BUILD)
>  static int spl_board_load_image(struct spl_image_info *spl_image,
>                                 struct spl_boot_device *bootdev)
>  {
> @@ -227,18 +227,6 @@ void s_init(void)
>                 "mcr p15, 0, r0, c1, c0, 1\n"
>                 ::: "r0");
>  #endif
> -#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I_H3
> -       /* Enable non-secure access to some peripherals */
> -       tzpc_init();
> -#endif
> -
> -       clock_init();
> -       timer_init();
> -       gpio_init();
> -#if !CONFIG_IS_ENABLED(DM_I2C)
> -       i2c_init_board();
> -#endif
> -       eth_init_board();
>  }
>
>  #define SUNXI_INVALID_BOOT_SOURCE      -1
> @@ -335,6 +323,19 @@ u32 spl_boot_device(void)
>
>  void board_init_f(ulong dummy)
>  {
> +#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I_H3
> +       /* Enable non-secure access to some peripherals */
> +       tzpc_init();
> +#endif
> +
> +       clock_init();
> +       timer_init();
> +       gpio_init();
> +#if !CONFIG_IS_ENABLED(DM_I2C)
> +       i2c_init_board();
> +#endif
> +       eth_init_board();
> +
>         spl_init();
>         preloader_console_init();
>
> --
> 2.17.6
>

Re: [PATCH 3/5] sunxi: move early "SRAM setup" into separate file

[Simon Glass]

On Mon, 24 Jan 2022 at 18:15, Andre Przywara <andre.przywara@arm.com> wrote:
>
> Currently we do some magic "SRAM setup" MMIO writes in s_init(), copied
> from the original BSP U-Boot. The comment speaks of this being required
> before DRAM access gets enabled, but there is no indication that this
> would actually be required that early.
>
> Move this out of s_init(), into board_init_f(). Since this actually only
> affects a very few older SoCs, the actual code goes into the cpu/armv7
> directory, to move it out of the way for all other SoCs.
>
> This also uses the opportunity to convert some #ifdefs over to the fancy
> IS_ENABLED() macros used in actual C code.
>
> We keep the s_init() stub around for now, since armv8's lowlevel_init
> still relies on it.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm/cpu/armv7/sunxi/Makefile |  3 +++
>  arch/arm/cpu/armv7/sunxi/sram.c   | 45 +++++++++++++++++++++++++++++++
>  arch/arm/mach-sunxi/board.c       | 38 +++++---------------------
>  3 files changed, 54 insertions(+), 32 deletions(-)
>  create mode 100644 arch/arm/cpu/armv7/sunxi/sram.c

Reviewed-by: Simon Glass <sjg@chromium.org>
Tested on: orangepi_pc2, orangepi_pc, A20-OLinuXino_MICRO
Tested-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 4/5] armv8: remove no longer needed lowlevel_init.S

[Simon Glass]

On Mon, 24 Jan 2022 at 18:15, Andre Przywara <andre.przywara@arm.com> wrote:
>
> When we added Allwinner SoC support to ARMv8, we needed to pull in an
> implementation of lowlevel_init, as sunxi required it at this time.
>
> The last few patches got rid of this bogus requirement, and as sunxi was
> still the only user, we can now remove lowlevel_init from ARMv8
> altogether.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm/cpu/armv8/Makefile        |  1 -
>  arch/arm/cpu/armv8/lowlevel_init.S | 43 ------------------------------
>  arch/arm/mach-sunxi/board.c        |  4 ---
>  3 files changed, 48 deletions(-)
>  delete mode 100644 arch/arm/cpu/armv8/lowlevel_init.S

Reviewed-by: Simon Glass <sjg@chromium.org>
Tested on: orangepi_pc2, orangepi_pc, A20-OLinuXino_MICRO
Tested-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 5/5] sunxi-common.h: remove pointless #ifdefs

[Simon Glass]

On Mon, 24 Jan 2022 at 18:15, Andre Przywara <andre.przywara@arm.com> wrote:
>
> Remove some pointless #ifdefs from this file, as there are quite too
> many of them already.
>
> Some definitions don't really hurt to have in any case, so remove the
> pointless CONFIG_MMC guard around CONFIG_MMC_SUNXI_SLOT.
>
> The BOARD_SIZE_LIMIT applies regardless of ARM64 or not (now), so remove
> that guard as well. The maximum number of MMC devices does not depend on
> CONFIG_ENV_IS_IN_MMC, so move that out to simplify the file.
>
> Last but not least CONFIG_SPL_BOARD_LOAD_IMAGE serves no real purpose
> anymore: it's unconditionally defined for all sunxi boards, and protects
> nothing applicable outside of sunxi code anymore. Just remove it.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  include/configs/sunxi-common.h | 7 -------
>  1 file changed, 7 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>
Tested on: orangepi_pc2, orangepi_pc, A20-OLinuXino_MICRO
Tested-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 3/5] sunxi: move early "SRAM setup" into separate file

[Andre Przywara]

On Mon, 24 Jan 2022 20:56:12 -0600
Samuel Holland <samuel@sholland.org> wrote:

Hi Samuel,

> On 1/24/22 7:15 PM, Andre Przywara wrote:
> > Currently we do some magic "SRAM setup" MMIO writes in s_init(), copied
> > from the original BSP U-Boot. The comment speaks of this being required
> > before DRAM access gets enabled, but there is no indication that this
> > would actually be required that early.
> > 
> > Move this out of s_init(), into board_init_f(). Since this actually only
> > affects a very few older SoCs, the actual code goes into the cpu/armv7
> > directory, to move it out of the way for all other SoCs.
> > 
> > This also uses the opportunity to convert some #ifdefs over to the fancy
> > IS_ENABLED() macros used in actual C code.
> > 
> > We keep the s_init() stub around for now, since armv8's lowlevel_init
> > still relies on it.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  arch/arm/cpu/armv7/sunxi/Makefile |  3 +++
> >  arch/arm/cpu/armv7/sunxi/sram.c   | 45 +++++++++++++++++++++++++++++++
> >  arch/arm/mach-sunxi/board.c       | 38 +++++---------------------
> >  3 files changed, 54 insertions(+), 32 deletions(-)
> >  create mode 100644 arch/arm/cpu/armv7/sunxi/sram.c
> > 
> > diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
> > index 1d40d6a18dc..ad11be78632 100644
> > --- a/arch/arm/cpu/armv7/sunxi/Makefile
> > +++ b/arch/arm/cpu/armv7/sunxi/Makefile
> > @@ -10,6 +10,9 @@ obj-y	+= timer.o
> >  obj-$(CONFIG_MACH_SUN6I)	+= tzpc.o
> >  obj-$(CONFIG_MACH_SUN8I_H3)	+= tzpc.o
> >  
> > +obj-$(CONFIG_MACH_SUN6I)	+= sram.o
> > +obj-$(CONFIG_MACH_SUN8I)	+= sram.o
> > +
> >  ifndef CONFIG_SPL_BUILD
> >  obj-$(CONFIG_ARMV7_PSCI)	+= psci.o
> >  endif
> > diff --git a/arch/arm/cpu/armv7/sunxi/sram.c b/arch/arm/cpu/armv7/sunxi/sram.c
> > new file mode 100644
> > index 00000000000..19395cce17c
> > --- /dev/null
> > +++ b/arch/arm/cpu/armv7/sunxi/sram.c
> > @@ -0,0 +1,45 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * (C) Copyright 2012 Henrik Nordstrom <henrik@henriknordstrom.net>
> > + *
> > + * (C) Copyright 2007-2011
> > + * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
> > + * Tom Cubie <tangliang@allwinnertech.com>
> > + *
> > + * SRAM init for older sunxi SoCs.
> > + */
> > +
> > +#include <common.h>
> > +#include <init.h>
> > +#include <asm/io.h>
> > +
> > +void sunxi_sram_init(void)
> > +{
> > +	/*
> > +	 * Undocumented magic taken from boot0, without this DRAM
> > +	 * access gets messed up (seems cache related).
> > +	 * The boot0 sources describe this as: "config ema for cache sram"
> > +	 * Newer SoCs (A83T, H3 and anything beyond) don't need this anymore.
> > +	 */
> > +	if (IS_ENABLED(CONFIG_MACH_SUN6I))
> > +		setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
> > +
> > +	if (IS_ENABLED(CONFIG_MACH_SUN8I)) {
> > +		uint version;
> > +
> > +		/* Unlock sram version info reg, read it, relock */
> > +		setbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15));
> > +		version = readl(SUNXI_SRAMC_BASE + 0x24) >> 16;
> > +		clrbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15));  
> 
> This is an open-coded version of sunxi_get_sram_id().

Indeed, thanks. Needs a prototype, though, I just picked one header file
in all of this mess ;-)

> > +
> > +		if (IS_ENABLED(CONFIG_MACH_SUN8I_A23)) {
> > +			if (version == 0x1650)
> > +				setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
> > +			else /* 0x1661 ? */
> > +				setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0);
> > +		} else if (IS_ENABLED(CONFIG_MACH_SUN8I_A33)) {
> > +			if (version != 0x1667)
> > +				setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0);
> > +		}
> > +	}
> > +}
> > diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> > index 8667ddf58e3..42ec02d96e3 100644
> > --- a/arch/arm/mach-sunxi/board.c
> > +++ b/arch/arm/mach-sunxi/board.c
> > @@ -186,38 +186,6 @@ SPL_LOAD_IMAGE_METHOD("FEL", 0, BOOT_DEVICE_BOARD, spl_board_load_image);
> >  
> >  void s_init(void)
> >  {
> > -	/*
> > -	 * Undocumented magic taken from boot0, without this DRAM
> > -	 * access gets messed up (seems cache related).
> > -	 * The boot0 sources describe this as: "config ema for cache sram"
> > -	 */
> > -#if defined CONFIG_MACH_SUN6I
> > -	setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
> > -#elif defined CONFIG_MACH_SUN8I
> > -	__maybe_unused uint version;
> > -
> > -	/* Unlock sram version info reg, read it, relock */
> > -	setbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15));
> > -	version = readl(SUNXI_SRAMC_BASE + 0x24) >> 16;
> > -	clrbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15));
> > -
> > -	/*
> > -	 * Ideally this would be a switch case, but we do not know exactly
> > -	 * which versions there are and which version needs which settings,
> > -	 * so reproduce the per SoC code from the BSP.
> > -	 */
> > -#if defined CONFIG_MACH_SUN8I_A23
> > -	if (version == 0x1650)
> > -		setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
> > -	else /* 0x1661 ? */
> > -		setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0);
> > -#elif defined CONFIG_MACH_SUN8I_A33
> > -	if (version != 0x1667)
> > -		setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0);
> > -#endif
> > -	/* A83T BSP never modifies SUNXI_SRAMC_BASE + 0x44 */
> > -	/* No H3 BSP, boot0 seems to not modify SUNXI_SRAMC_BASE + 0x44 */
> > -#endif
> >  }
> >  
> >  #define SUNXI_INVALID_BOOT_SOURCE	-1
> > @@ -312,8 +280,14 @@ u32 spl_boot_device(void)
> >  	return sunxi_get_boot_device();
> >  }
> >  
> > +__weak void sunxi_sram_init(void)
> > +{
> > +}  
> 
> In configurations other than MACH_SUN6/8I, the strong definition of
> sunxi_sram_init() is also an empty function, so this is no more efficient than
> unconditionally compiling sram.c.

I know, but I put this here to be able to keep this SRAM cruft in
cpu/armv7/sunxi, and avoid compiling this file for everyone else (ARM9,
ARMv8, RISC-V).
Do you have a better idea? I actually tried several ways before, and
this seemed to be the cleanest. Yes, we do a call to a ret, for most
boards, but we probably have bigger problems.
And I consider this just the beginning of a sunxi cleanup journey, so
we can revisit this later.

Cheers,
Andre

> 
> > +
> >  void board_init_f(ulong dummy)
> >  {
> > +	sunxi_sram_init();
> > +
> >  #if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I_H3
> >  	/* Enable non-secure access to some peripherals */
> >  	tzpc_init();
> >   
> 

Re: [PATCH 3/5] sunxi: move early "SRAM setup" into separate file

[Samuel Holland]

On 1/27/22 6:59 PM, Andre Przywara wrote:
> On Mon, 24 Jan 2022 20:56:12 -0600
> Samuel Holland <samuel@sholland.org> wrote:
> 
> Hi Samuel,
> 
>> On 1/24/22 7:15 PM, Andre Przywara wrote:
>>> Currently we do some magic "SRAM setup" MMIO writes in s_init(), copied
>>> from the original BSP U-Boot. The comment speaks of this being required
>>> before DRAM access gets enabled, but there is no indication that this
>>> would actually be required that early.
>>>
>>> Move this out of s_init(), into board_init_f(). Since this actually only
>>> affects a very few older SoCs, the actual code goes into the cpu/armv7
>>> directory, to move it out of the way for all other SoCs.
>>>
>>> This also uses the opportunity to convert some #ifdefs over to the fancy
>>> IS_ENABLED() macros used in actual C code.
>>>
>>> We keep the s_init() stub around for now, since armv8's lowlevel_init
>>> still relies on it.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> ---
>>>  arch/arm/cpu/armv7/sunxi/Makefile |  3 +++
>>>  arch/arm/cpu/armv7/sunxi/sram.c   | 45 +++++++++++++++++++++++++++++++
>>>  arch/arm/mach-sunxi/board.c       | 38 +++++---------------------
>>>  3 files changed, 54 insertions(+), 32 deletions(-)
>>>  create mode 100644 arch/arm/cpu/armv7/sunxi/sram.c
>>>
>>> diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
>>> index 1d40d6a18dc..ad11be78632 100644
>>> --- a/arch/arm/cpu/armv7/sunxi/Makefile
>>> +++ b/arch/arm/cpu/armv7/sunxi/Makefile
>>> @@ -10,6 +10,9 @@ obj-y	+= timer.o
>>>  obj-$(CONFIG_MACH_SUN6I)	+= tzpc.o
>>>  obj-$(CONFIG_MACH_SUN8I_H3)	+= tzpc.o
>>>  
>>> +obj-$(CONFIG_MACH_SUN6I)	+= sram.o
>>> +obj-$(CONFIG_MACH_SUN8I)	+= sram.o
>>> +
>>>  ifndef CONFIG_SPL_BUILD
>>>  obj-$(CONFIG_ARMV7_PSCI)	+= psci.o
>>>  endif
>>> diff --git a/arch/arm/cpu/armv7/sunxi/sram.c b/arch/arm/cpu/armv7/sunxi/sram.c
>>> new file mode 100644
>>> index 00000000000..19395cce17c
>>> --- /dev/null
>>> +++ b/arch/arm/cpu/armv7/sunxi/sram.c
>>> @@ -0,0 +1,45 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * (C) Copyright 2012 Henrik Nordstrom <henrik@henriknordstrom.net>
>>> + *
>>> + * (C) Copyright 2007-2011
>>> + * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
>>> + * Tom Cubie <tangliang@allwinnertech.com>
>>> + *
>>> + * SRAM init for older sunxi SoCs.
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <init.h>
>>> +#include <asm/io.h>
>>> +
>>> +void sunxi_sram_init(void)
>>> +{
>>> +	/*
>>> +	 * Undocumented magic taken from boot0, without this DRAM
>>> +	 * access gets messed up (seems cache related).
>>> +	 * The boot0 sources describe this as: "config ema for cache sram"
>>> +	 * Newer SoCs (A83T, H3 and anything beyond) don't need this anymore.
>>> +	 */
>>> +	if (IS_ENABLED(CONFIG_MACH_SUN6I))
>>> +		setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
>>> +
>>> +	if (IS_ENABLED(CONFIG_MACH_SUN8I)) {
>>> +		uint version;
>>> +
>>> +		/* Unlock sram version info reg, read it, relock */
>>> +		setbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15));
>>> +		version = readl(SUNXI_SRAMC_BASE + 0x24) >> 16;
>>> +		clrbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15));  
>>
>> This is an open-coded version of sunxi_get_sram_id().
> 
> Indeed, thanks. Needs a prototype, though, I just picked one header file
> in all of this mess ;-)
> 
>>> +
>>> +		if (IS_ENABLED(CONFIG_MACH_SUN8I_A23)) {
>>> +			if (version == 0x1650)
>>> +				setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
>>> +			else /* 0x1661 ? */
>>> +				setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0);
>>> +		} else if (IS_ENABLED(CONFIG_MACH_SUN8I_A33)) {
>>> +			if (version != 0x1667)
>>> +				setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0);
>>> +		}
>>> +	}
>>> +}
>>> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
>>> index 8667ddf58e3..42ec02d96e3 100644
>>> --- a/arch/arm/mach-sunxi/board.c
>>> +++ b/arch/arm/mach-sunxi/board.c
>>> @@ -186,38 +186,6 @@ SPL_LOAD_IMAGE_METHOD("FEL", 0, BOOT_DEVICE_BOARD, spl_board_load_image);
>>>  
>>>  void s_init(void)
>>>  {
>>> -	/*
>>> -	 * Undocumented magic taken from boot0, without this DRAM
>>> -	 * access gets messed up (seems cache related).
>>> -	 * The boot0 sources describe this as: "config ema for cache sram"
>>> -	 */
>>> -#if defined CONFIG_MACH_SUN6I
>>> -	setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
>>> -#elif defined CONFIG_MACH_SUN8I
>>> -	__maybe_unused uint version;
>>> -
>>> -	/* Unlock sram version info reg, read it, relock */
>>> -	setbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15));
>>> -	version = readl(SUNXI_SRAMC_BASE + 0x24) >> 16;
>>> -	clrbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15));
>>> -
>>> -	/*
>>> -	 * Ideally this would be a switch case, but we do not know exactly
>>> -	 * which versions there are and which version needs which settings,
>>> -	 * so reproduce the per SoC code from the BSP.
>>> -	 */
>>> -#if defined CONFIG_MACH_SUN8I_A23
>>> -	if (version == 0x1650)
>>> -		setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
>>> -	else /* 0x1661 ? */
>>> -		setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0);
>>> -#elif defined CONFIG_MACH_SUN8I_A33
>>> -	if (version != 0x1667)
>>> -		setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0);
>>> -#endif
>>> -	/* A83T BSP never modifies SUNXI_SRAMC_BASE + 0x44 */
>>> -	/* No H3 BSP, boot0 seems to not modify SUNXI_SRAMC_BASE + 0x44 */
>>> -#endif
>>>  }
>>>  
>>>  #define SUNXI_INVALID_BOOT_SOURCE	-1
>>> @@ -312,8 +280,14 @@ u32 spl_boot_device(void)
>>>  	return sunxi_get_boot_device();
>>>  }
>>>  
>>> +__weak void sunxi_sram_init(void)
>>> +{
>>> +}  
>>
>> In configurations other than MACH_SUN6/8I, the strong definition of
>> sunxi_sram_init() is also an empty function, so this is no more efficient than
>> unconditionally compiling sram.c.
> 
> I know, but I put this here to be able to keep this SRAM cruft in
> cpu/armv7/sunxi, and avoid compiling this file for everyone else (ARM9,
> ARMv8, RISC-V).

Ah, right, that makes sense. I missed the directory change here.

> Do you have a better idea? I actually tried several ways before, and
> this seemed to be the cleanest. Yes, we do a call to a ret, for most
> boards, but we probably have bigger problems.

No, I am fine with your method.

Regards,
Samuel

> And I consider this just the beginning of a sunxi cleanup journey, so
> we can revisit this later.
> 
> Cheers,
> Andre
> 
>>
>>> +
>>>  void board_init_f(ulong dummy)
>>>  {
>>> +	sunxi_sram_init();
>>> +
>>>  #if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I_H3
>>>  	/* Enable non-secure access to some peripherals */
>>>  	tzpc_init();
>>>   
>>
>