Support Marvell bootloaders remapping registers at different locations

ENTRY(armada_xp_secondary_startup)

arch/arm/mach-mvebu/headsmp.S +6 -0

+	/* Get coherency fabric base physical address */
+	adr	r0, 1f
+	ldmia	r0, {r1, r2}
+	sub	r0, r0, r1
+	add	r2, r2, r0
+	ldr	r0, [r2]

You may simplify the above like this:

	adr	r0, 1f
	ldr	r1, [r0]
	ldr	r0, [r0, r1]
	...,

1: .long coherency_phys_base - .

Now that the coherency_init() function is called a bit earlier, we can actually read the physical address of the coherency unit registers from the Device Tree, and communicate that to the headsmp.S code, which avoids hardcoding a physical address.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 arch/arm/mach-mvebu/coherency.c |  4 ++++
 arch/arm/mach-mvebu/headsmp.S   | 19 +++++++++++--------
 2 files changed, 15 insertions(+), 8 deletions(-)

arch/arm/mach-mvebu/coherency.c +1 -0

index d74794a..3d8f40f 100644
@@ -27,6 +27,7 @@
 #include <asm/smp_plat.h>
 #include "armada-370-xp.h"
 
+unsigned long __cpuinitdata coherency_phys_base;
 static void __iomem *coherency_base;
 static void __iomem *coherency_cpu_base;
 

arch/arm/mach-mvebu/coherency.c +0 -0

 		coherency_base = of_iomap(np, 0);
 		coherency_cpu_base = of_iomap(np, 1);
 		set_cpu_coherent(cpu_logical_map(smp_processor_id()), 0);

arch/arm/mach-mvebu/headsmp.S +0 -6

index a06e0ed..f500e9c 100644
@@ -21,12 +21,6 @@
 #include <linux/linkage.h>
 #include <linux/init.h>
 
-/*
- * At this stage the secondary CPUs don't have acces yet to the MMU, so
- * we have to provide physical addresses
- */
-#define ARMADA_XP_CFB_BASE	     0xD0020200
-
 	__CPUINIT
 
 /*

arch/arm/mach-mvebu/coherency.c int __init coherency_init(void) · line 131

Quoted 4 lines

arch/arm/mach-mvebu/coherency.c +3 -0

+		struct resource res;
 		pr_info("Initializing Coherency fabric\n");
+		of_address_to_resource(np, 0, &res);
+		coherency_phys_base = res.start;

So you have the primary CPU writing this address, before it is coherent...

arch/arm/mach-mvebu/headsmp.S line 37

Quoted 7 lines

ENTRY(armada_xp_secondary_startup)

arch/arm/mach-mvebu/headsmp.S +6 -0

+	/* Get coherency fabric base physical address */
+	adr	r0, 1f
+	ldmia	r0, {r1, r2}
+	sub	r0, r0, r1
+	add	r2, r2, r0
+	ldr	r0, [r2]

... and the secondaries reading it, before they are coherent. How do you ensure that the write is indeed visible?

Furthermore, it's worth noting that of_find_matching_node takes the devtree_lock spinlock, so you need to be careful calling that if you're not coherent.

Now that the coherency_init() function is called a bit earlier, we can actually read the physical address of the coherency unit registers from the Device Tree, and communicate that to the headsmp.S code, which avoids hardcoding a physical address.

Signed-off-by Thomas Petazzoni <[email protected]>

---
Changes from v1 to v2:

* Simplified the code in headsmp.S to compute the physical address of the coherency unit base address. Suggested by Nicolas Pitre.

* Added a sync_cache_w() call to ensure the physical address of the coherency unit stored in the coherency_phys_base is visible by the other CPUs. The other CPUs are reading this before they join the coherency fabric, so we need to make some manual cache coherency here. Noticed by Will Deacon.

 arch/arm/mach-mvebu/coherency.c | 12 ++++++++++++
 arch/arm/mach-mvebu/headsmp.S   | 16 ++++++++--------
 2 files changed, 20 insertions(+), 8 deletions(-)

Now that the coherency_init() function is called a bit earlier, we can actually read the physical address of the coherency unit registers from the Device Tree, and communicate that to the headsmp.S code, which avoids hardcoding a physical address.

Signed-off-by Thomas Petazzoni <[email protected]>

---
Changes from v1 to v2:

* Simplified the code in headsmp.S to compute the physical address of the coherency unit base address. Suggested by Nicolas Pitre.

* Added a sync_cache_w() call to ensure the physical address of the coherency unit stored in the coherency_phys_base is visible by the other CPUs. The other CPUs are reading this before they join the coherency fabric, so we need to make some manual cache coherency here. Noticed by Will Deacon.

 arch/arm/mach-mvebu/coherency.c | 12 ++++++++++++
 arch/arm/mach-mvebu/headsmp.S   | 16 ++++++++--------
 2 files changed, 20 insertions(+), 8 deletions(-)

arch/arm/mach-mvebu/coherency.c +12 -0

index d74794a..32fcf69 100644
@@ -25,8 +25,10 @@
 #include <linux/dma-mapping.h>
 #include <linux/platform_device.h>
 #include <asm/smp_plat.h>
+#include <asm/cacheflush.h>
 #include "armada-370-xp.h"
 
+unsigned long __cpuinitdata coherency_phys_base;
 static void __iomem *coherency_base;
 static void __iomem *coherency_cpu_base;
 

@@ -124,7 +126,17 @@ int __init coherency_init(void)
 
 	np = of_find_matching_node(NULL, of_coherency_table);
 	if (np) {
+		struct resource res;
 		pr_info("Initializing Coherency fabric\n");
+		of_address_to_resource(np, 0, &res);
+		coherency_phys_base = res.start;
+		/*
+		 * Ensure secondary CPUs will see the updated value,
+		 * which they read before they join the coherency
+		 * fabric, and therefore before they are coherent with
+		 * the boot CPU cache.
+		 */
+		sync_cache_w(&coherency_phys_base);
 		coherency_base = of_iomap(np, 0);
 		coherency_cpu_base = of_iomap(np, 1);
 		set_cpu_coherent(cpu_logical_map(smp_processor_id()), 0);

arch/arm/mach-mvebu/headsmp.S +8 -8

index a06e0ed..7147300 100644
@@ -21,12 +21,6 @@
 #include <linux/linkage.h>
 #include <linux/init.h>
 
-/*
- * At this stage the secondary CPUs don't have acces yet to the MMU, so
- * we have to provide physical addresses
- */
-#define ARMADA_XP_CFB_BASE	     0xD0020200
-
 	__CPUINIT
 
 /*

@@ -35,15 +29,21 @@
  * startup
  */
 ENTRY(armada_xp_secondary_startup)
+	/* Get coherency fabric base physical address */
+	adr	r0, 1f
+	ldr	r1, [r0]
+	ldr	r0, [r0, r1]
 
 	/* Read CPU id */
 	mrc     p15, 0, r1, c0, c0, 5
 	and     r1, r1, #0xF
 
 	/* Add CPU to coherency fabric */
-	ldr     r0, =ARMADA_XP_CFB_BASE
-
 	bl	ll_set_cpu_coherent
 	b	secondary_startup
 
 ENDPROC(armada_xp_secondary_startup)
+
+	.align 2
+1:
+	.long	coherency_phys_base - .

Signature · 1 line

1.8.1.2

Now that the coherency_init() function is called a bit earlier, we can actually read the physical address of the coherency unit registers from the Device Tree, and communicate that to the headsmp.S code, which avoids hardcoding a physical address.

Signed-off-by Thomas Petazzoni <[email protected]>

---
Changes from v1 to v2:

* Simplified the code in headsmp.S to compute the physical address of the coherency unit base address. Suggested by Nicolas Pitre.

* Added a sync_cache_w() call to ensure the physical address of the coherency unit stored in the coherency_phys_base is visible by the other CPUs. The other CPUs are reading this before they join the coherency fabric, so we need to make some manual cache coherency here. Noticed by Will Deacon.

 arch/arm/mach-mvebu/coherency.c | 12 ++++++++++++
 arch/arm/mach-mvebu/headsmp.S   | 16 ++++++++--------
 2 files changed, 20 insertions(+), 8 deletions(-)

In order to support both old and new bootloaders, we add a new Kconfig option for the earlyprintk UART selection. The existing option allows to work with old bootloaders (that keep the internal registers mapped at 0xd0000000), while the newly introduced option allows to work with new bootloaders (that remap the internal registers at 0xf1000000).

Signed-off-by Thomas Petazzoni <[email protected]>

---
 arch/arm/Kconfig.debug         | 30 ++++++++++++++++++++++++++++--
 arch/arm/include/debug/mvebu.S |  5 +++++
 2 files changed, 33 insertions(+), 2 deletions(-)

arch/arm/Kconfig.debug +0 -0

index 1d41908..e6a6ab1 100644

arch/arm/Kconfig.debug +3 -1

-		bool "Kernel low-level debugging messages via MVEBU UART"
+		bool "Kernel low-level debugging messages via MVEBU UART (old bootloaders)"
 		depends on ARCH_MVEBU
 		help
 		  Say Y here if you want kernel low-level debugging support
 		  on MVEBU based platforms.
 
+		  This option should be used with the old bootloaders
+		  that left the internal registers mapped at

arch/arm/Kconfig.debug +14 -1

+		depends on ARCH_MVEBU
+		help
+		  Say Y here if you want kernel low-level debugging support
+		  on MVEBU based platforms.
+
+		  This option should be used with the new bootloaders
+		  that remap the internal registers at 0xf1000000.
+
+		  If the wrong DEBUG_MVEBU_UART* option is selected,
+		  when u-boot hands over to the kernel, the system
+		  silently crashes, with no serial output at all.
+
 	config DEBUG_NOMADIK_UART
 		bool "Kernel low-level debugging messages via NOMADIK UART"
 		depends on ARCH_NOMADIK

@@ -632,7 +657,8 @@ config DEBUG_LL_INCLUDE
 				 DEBUG_IMX51_UART || \
 				 DEBUG_IMX53_UART ||\
 				 DEBUG_IMX6Q_UART
-	default "debug/mvebu.S" if DEBUG_MVEBU_UART
+	default "debug/mvebu.S" if DEBUG_MVEBU_UART || \
+				   DEBUG_MVEBU_UART_ALTERNATE
 	default "debug/mxs.S" if DEBUG_IMX23_UART || DEBUG_IMX28_UART
 	default "debug/nomadik.S" if DEBUG_NOMADIK_UART
 	default "debug/omap2plus.S" if DEBUG_OMAP2PLUS_UART

arch/arm/include/debug/mvebu.S +0 -0

index df191af..6517311 100644

arch/arm/include/debug/mvebu.S +1 -0

+
 #define ARMADA_370_XP_REGS_VIRT_BASE	0xfec00000
 
 	.macro	addruart, rp, rv, tmp

Signature · 1 line

1.8.1.2

Quoted 1 line

config DEBUG_MVEBU_UART

Perhaps we should change this to DEBUG_MVEBU_UART_LEGACY?

arch/arm/Kconfig.debug choice · line 323

Quoted 1 line

arch/arm/Kconfig.debug +1 -0

+	config DEBUG_MVEBU_UART_ALTERNATE

And this to DEBUG_MVEBU_UART?

Dear Jason Cooper,

Quoted 7 lines · Jason Cooper

Quoted 1 line

config DEBUG_MVEBU_UART

Perhaps we should change this to DEBUG_MVEBU_UART_LEGACY?

arch/arm/Kconfig.debug choice · line 323

Quoted 1 line

arch/arm/Kconfig.debug +1 -0

+	config DEBUG_MVEBU_UART_ALTERNATE

And this to DEBUG_MVEBU_UART?

I'm fine with doing that, but it means that people who will keep their current .config that has CONFIG_DEBUG_MVEBU_UART=y will suddenly see their earlyprintk use 0xf1 instead of 0xd0. So in terms of kconfig backward compatibility, it's not the nicest thing, but if you believe that's ok, I'll change the patch as you proposed.

On Marvell platforms, the base address of the "internal registers" (i.e registers of all peripherals) can be configured at runtime. At reset, it is set to 0xD0000000, but it can freely be changed to some other location.

On all previous Marvell SoC families, Linux assumes the bootloader has remapped the internal registers at 0xF1000000, and Marvell is now moving to do the same on Armada 370/XP, since it allows to use as much RAM as possible in the 0 -> 4G area. However, due to early issues, until now, Marvell bootloaders were leaving the internal registers address set to 0xD0000000, and recently, Marvell has started shipping bootloaders that remapped those registers to 0xF1000000.

In order to support those two cases, we want to make sure the Device Tree is the only location where the physical base address of the internal registers is stored. This patch set removes the places where the physical base address of the registers was hardcoded and provides two Kconfig options for the earlyprintk UART address selection (one for 0xD0000000 and one for 0xF1000000).

It no longer uses the CP15 trick, nor it tries to do runtime remapping to 0xF1 if it wasn't done by the bootloader. But it allows to easily boot a kernel with both old and new bootloaders by doing a simple change, in a single location in the Device Tree.

This patch series is based on jcooper/mvebu/fixes-non-critical and jcooper/mvebu/cleanup, which contains 3 patches that are necessary to make this patch series work properly.

This patch series is based on jcooper/mvebu/fixes-non-critical and jcooper/mvebu/cleanup, which contains 3 patches that are necessary to make this patch series work properly.

Quoted 3 lines · Thomas Petazzoni

This patch series is based on jcooper/mvebu/fixes-non-critical and jcooper/mvebu/cleanup, which contains 3 patches that are necessary to make this patch series work properly.

Thomas,

I've tentatively pulled your series into mvebu/regmap with appropriate dependencies.

I was going to put this in for-next, but I got a build error after merging it. Could you do me a favor and try merging this with mvebu/seb_clk? Just shoot me the merge resolution that builds mvebu_defconfig correctly, and I'll incorporate it into the merge. No need to rebase the series.

If you want to reproduce what I saw, just merge mvebu/regmap with for-next (up shortly), and build mvebu_defconfig.

Quoted 9 lines

I've tentatively pulled your series into mvebu/regmap with appropriate dependencies.

I was going to put this in for-next, but I got a build error after merging it. Could you do me a favor and try merging this with mvebu/seb_clk? Just shoot me the merge resolution that builds mvebu_defconfig correctly, and I'll incorporate it into the merge. No need to rebase the series.

Quoted 2 lines

If you want to reproduce what I saw, just merge mvebu/regmap with for-next (up shortly), and build mvebu_defconfig.

This has now been resolved, whole series, with v2 of the one patch is in mvebu/regmap.