Versatile Express DT support, take 2

This patch gets rid of the MMIO_P2V and __MMPIO_P2V macros, defining constant virtual base for motherboard and tile peripherals instead.

Additionally, in preparation for the new motherboard memory map, the motherboard peripherals are using base pointers calculated in runtime, instead of compile-time calculated values.

Signed-off-by Pawel Moll <[email protected]>

---
 arch/arm/include/asm/hardware/arm_timer.h         |    5 ++
 arch/arm/mach-vexpress/core.h                     |   12 +++-
 arch/arm/mach-vexpress/ct-ca9x4.c                 |   52 ++++-------------
 arch/arm/mach-vexpress/include/mach/ct-ca9x4.h    |   13 ++---
 arch/arm/mach-vexpress/include/mach/motherboard.h |   53 ++++++++---------
 arch/arm/mach-vexpress/platsmp.c                  |    4 +-
 arch/arm/mach-vexpress/v2m.c                      |   64 ++++++++++++++-------
 7 files changed, 100 insertions(+), 103 deletions(-)

arch/arm/mach-vexpress/ct-ca9x4.c +0 -9

index 2b1e836..47c0733 100644
@@ -30,57 +30,26 @@
 
 #include <plat/clcd.h>
 
-#define V2M_PA_CS7	0x10000000
-
 static struct map_desc ct_ca9x4_io_desc[] __initdata = {
 	{
-		.virtual	= __MMIO_P2V(CT_CA9X4_MPIC),
-		.pfn		= __phys_to_pfn(CT_CA9X4_MPIC),
-		.length		= SZ_16K,
-		.type		= MT_DEVICE,
-	}, {
-		.virtual	= __MMIO_P2V(CT_CA9X4_SP804_TIMER),
-		.pfn		= __phys_to_pfn(CT_CA9X4_SP804_TIMER),

arch/arm/mach-vexpress/v2m.c +0 -0

index 1fafc32..b84fa45 100644

Quoted 19 lines · Pawel Moll

This patch gets rid of the MMIO_P2V and __MMPIO_P2V macros, defining constant virtual base for motherboard and tile peripherals instead.

Additionally, in preparation for the new motherboard memory map, the motherboard peripherals are using base pointers calculated in runtime, instead of compile-time calculated values.

Signed-off-by Pawel Moll <[email protected]>

---
 arch/arm/include/asm/hardware/arm_timer.h         |    5 ++
 arch/arm/mach-vexpress/core.h                     |   12 +++-
 arch/arm/mach-vexpress/ct-ca9x4.c                 |   52 ++++-------------
 arch/arm/mach-vexpress/include/mach/ct-ca9x4.h    |   13 ++---
 arch/arm/mach-vexpress/include/mach/motherboard.h |   53 ++++++++---------
 arch/arm/mach-vexpress/platsmp.c                  |    4 +-
 arch/arm/mach-vexpress/v2m.c                      |   64 ++++++++++++++-------
 7 files changed, 100 insertions(+), 103 deletions(-)

[...]

Quoted 8 lines

arch/arm/mach-vexpress/ct-ca9x4.c +0 -0

index 2b1e836..47c0733 100644
@@ -30,57 +30,26 @@
 
 #include <plat/clcd.h>
 

arch/arm/mach-vexpress/ct-ca9x4.c line 37

Quoted 11 lines

arch/arm/mach-vexpress/ct-ca9x4.c +3 -6

-#define V2M_PA_CS7	0x10000000
-
 static struct map_desc ct_ca9x4_io_desc[] __initdata = {
 	{
-		.virtual	= __MMIO_P2V(CT_CA9X4_MPIC),
-		.pfn		= __phys_to_pfn(CT_CA9X4_MPIC),
-		.length		= SZ_16K,
-		.type		= MT_DEVICE,
+		.virtual        = V2T_PERIPH,
+		.pfn            = __phys_to_pfn(CT_CA9X4_MPIC),
+		.length         = SZ_8K,

Can you explain the size change?

Quoted 4 lines

arch/arm/mach-vexpress/v2m.c +0 -0

index 1fafc32..b84fa45 100644

[...]

arch/arm/mach-vexpress/v2m.c static void __init v2m_populate_ct_desc(void) · line 436

Quoted 7 lines

arch/arm/mach-vexpress/v2m.c +3 -0

@@ -413,6 +431,10 @@ static void __init v2m_populate_ct_desc(void)
 static void __init v2m_map_io(void)
 {
 	iotable_init(v2m_io_desc, ARRAY_SIZE(v2m_io_desc));
+
+	/* Will become an ioremap() when possible */
+	v2m_sysreg_base = V2M_PERIPH_P2V(V2M_SYSREGS);

linux/of_fdt.h has functions for manipulating the flattened device tree blob.

Could that solve our problem? We just need to get the root node compatible property out.

Is there anything else blocking the building of legacy and RS1 memory maps into a single kernel?
(I'm still hazy on how all the remapping stuff works, myself.)

Quoted 31 lines · Dave Martin

Quoted 19 lines · Pawel Moll

This patch gets rid of the MMIO_P2V and __MMPIO_P2V macros, defining constant virtual base for motherboard and tile peripherals instead.

Additionally, in preparation for the new motherboard memory map, the motherboard peripherals are using base pointers calculated in runtime, instead of compile-time calculated values.

Signed-off-by Pawel Moll <[email protected]>

---
 arch/arm/include/asm/hardware/arm_timer.h         |    5 ++
 arch/arm/mach-vexpress/core.h                     |   12 +++-
 arch/arm/mach-vexpress/ct-ca9x4.c                 |   52 ++++-------------
 arch/arm/mach-vexpress/include/mach/ct-ca9x4.h    |   13 ++---
 arch/arm/mach-vexpress/include/mach/motherboard.h |   53 ++++++++---------
 arch/arm/mach-vexpress/platsmp.c                  |    4 +-
 arch/arm/mach-vexpress/v2m.c                      |   64 ++++++++++++++-------
 7 files changed, 100 insertions(+), 103 deletions(-)

[...]

Quoted 8 lines

arch/arm/mach-vexpress/ct-ca9x4.c +0 -0

index 2b1e836..47c0733 100644
@@ -30,57 +30,26 @@
 
 #include <plat/clcd.h>
 

Quoted 13 lines

arch/arm/mach-vexpress/ct-ca9x4.c line 37

Quoted 11 lines

arch/arm/mach-vexpress/ct-ca9x4.c +3 -6

-#define V2M_PA_CS7	0x10000000
-
 static struct map_desc ct_ca9x4_io_desc[] __initdata = {
 	{
-		.virtual	= __MMIO_P2V(CT_CA9X4_MPIC),
-		.pfn		= __phys_to_pfn(CT_CA9X4_MPIC),
-		.length		= SZ_16K,
-		.type		= MT_DEVICE,
+		.virtual        = V2T_PERIPH,
+		.pfn            = __phys_to_pfn(CT_CA9X4_MPIC),
+		.length         = SZ_8K,

Can you explain the size change?

The SCU's "Private memory region" is 8K big, not 16:

http://infocenter.arm.com/help/topic/com.arm.doc.ddi0407g/CACCJFCJ.html
http://infocenter.arm.com/help/topic/com.arm.doc.dui0448e/CHDCBJCB.html

Quoted 19 lines

Quoted 5 lines

arch/arm/mach-vexpress/v2m.c +0 -0

index 1fafc32..b84fa45 100644

[...]

arch/arm/mach-vexpress/v2m.c static void __init v2m_populate_ct_desc(void) · line 436

Quoted 7 lines

arch/arm/mach-vexpress/v2m.c +3 -0

@@ -413,6 +431,10 @@ static void __init v2m_populate_ct_desc(void)
 static void __init v2m_map_io(void)
 {
 	iotable_init(v2m_io_desc, ARRAY_SIZE(v2m_io_desc));
+
+	/* Will become an ioremap() when possible */
+	v2m_sysreg_base = V2M_PERIPH_P2V(V2M_SYSREGS);

linux/of_fdt.h has functions for manipulating the flattened device tree blob.

Quoted 2 lines

Could that solve our problem? We just need to get the root node compatible property out.

What do you exaclty mean? This function is non-DT related. It's not used by DT platform at all...

Quoted 3 lines

Is there anything else blocking the building of legacy and RS1 memory maps into a single kernel?
(I'm still hazy on how all the remapping stuff works, myself.)

This works now - I have a single binary kernel that works on both A9 and A5 coretiles. Of course the ATAG-based support works with A9 only.

Quoted 6 lines

Quoted 3 lines

Is there anything else blocking the building of legacy and RS1 memory maps into a single kernel?
(I'm still hazy on how all the remapping stuff works, myself.)

This works now - I have a single binary kernel that works on both A9 and A5 coretiles. Of course the ATAG-based support works with A9 only.

Cool -- we should get Ryan and Tixy to try it out, if they haven't already. I don't have an A5 handy...

Quoted 4 lines · Dave Martin

Quoted 4 lines

Quoted 3 lines

Is there anything else blocking the building of legacy and RS1 memory maps into a single kernel?
(I'm still hazy on how all the remapping stuff works, myself.)

Quoted 5 lines

Quoted 2 lines

This works now - I have a single binary kernel that works on both A9 and A5 coretiles. Of course the ATAG-based support works with A9 only.

Cool -- we should get Ryan and Tixy to try it out, if they haven't already. I don't have an A5 handy...

Good idea. I'll send them instructions how to boot "DT-enabled" kernel using Boot Monitor tomorrow and ask to try the series.

arch/arm/mach-vexpress/v2m.c +1 -1

index 1fafc32..b84fa45 100644
@@ -39,29 +39,41 @@
 
 static struct map_desc v2m_io_desc[] __initdata = {
 	{
-		.virtual	= __MMIO_P2V(V2M_PA_CS7),
+		.virtual	= V2M_PERIPH,

arch/arm/mach-vexpress/core.h struct amba_device name##_device = { \ · line 25

Quoted 4 lines

arch/arm/mach-vexpress/core.h +4 -0

+/* Tile's peripherals static mappings should start here */
+#define V2T_PERIPH 0xf8200000
+#define V2T_PERIPH_P2V(offset) ((void __iomem *)(V2T_PERIPH | (offset)))
+

Please get rid of these blank lines at the end of files.

arch/arm/mach-vexpress/v2m.c line 52

Quoted 4 lines

arch/arm/mach-vexpress/v2m.c +4 -0

+static void __iomem *v2m_sysreg_base;
+
+
+

More useless blank lines.

arch/arm/mach-vexpress/v2m.c line 56

Quoted 4 lines

static void __init v2m_timer_init(void)
{

arch/arm/mach-vexpress/v2m.c +2 -0

+	void *sysctl_base;
+	void *timer01_base;

Do you not use sparse? __iomem.

arch/arm/mach-vexpress/v2m.c line 64

Quoted 8 lines

arch/arm/mach-vexpress/v2m.c +6 -0

+	unsigned int timer01_irq;
 	u32 scctrl;
 
+		sysctl_base = ioremap(V2M_SYSCTL, SZ_4K);
+		BUG_ON(!sysctl_base);
+		timer01_base = ioremap(V2M_TIMER01, SZ_4K);
+		BUG_ON(!timer01_base);
+		timer01_irq = IRQ_V2M_TIMER0;

What's going on with the indentation here?

arch/arm/mach-vexpress/v2m.c static void __init v2m_populate_ct_desc(void) · line 436

Quoted 7 lines

arch/arm/mach-vexpress/v2m.c +3 -0

@@ -413,6 +431,10 @@ static void __init v2m_populate_ct_desc(void)
 static void __init v2m_map_io(void)
 {
 	iotable_init(v2m_io_desc, ARRAY_SIZE(v2m_io_desc));
+
+	/* Will become an ioremap() when possible */
+	v2m_sysreg_base = V2M_PERIPH_P2V(V2M_SYSREGS);

It won't if it stays here.

Quoted 6 lines · Russell King - ARM Linux

arch/arm/mach-vexpress/core.h struct amba_device name##_device = { \ · line 25

Quoted 4 lines

arch/arm/mach-vexpress/core.h +4 -0

+/* Tile's peripherals static mappings should start here */
+#define V2T_PERIPH 0xf8200000
+#define V2T_PERIPH_P2V(offset) ((void __iomem *)(V2T_PERIPH | (offset)))
+

Please get rid of these blank lines at the end of files.

Patch splitting leftover. Will fix.

Quoted 6 lines

arch/arm/mach-vexpress/v2m.c line 52

Quoted 4 lines

arch/arm/mach-vexpress/v2m.c +4 -0

+static void __iomem *v2m_sysreg_base;
+
+
+

More useless blank lines.

Ok, will change that. I just like the way that the data are visually separated from the code like here:

ceade897 (Russell King 2010-02-11 21:44:53 +0000  68)         .init   = v2m_timer_init,
ceade897 (Russell King 2010-02-11 21:44:53 +0000  69) };
ceade897 (Russell King 2010-02-11 21:44:53 +0000  70) 
ceade897 (Russell King 2010-02-11 21:44:53 +0000  71) 
ceade897 (Russell King 2010-02-11 21:44:53 +0000  72) static DEFINE_SPINLOCK(v2m_cfg_lock);

or here:

ceade897 (Russell King 2010-02-11 21:44:53 +0000 289) &rtc_device,
ceade897 (Russell King 2010-02-11 21:44:53 +0000 290) };
ceade897 (Russell King 2010-02-11 21:44:53 +0000 291)
ceade897 (Russell King 2010-02-11 21:44:53 +0000 292)
ceade897 (Russell King 2010-02-11 21:44:53 +0000 293) static long v2m_osc_round(struct clk *clk, unsigned long rate)

Quoted 7 lines

arch/arm/mach-vexpress/v2m.c line 56

Quoted 5 lines

static void __init v2m_timer_init(void)
{

arch/arm/mach-vexpress/v2m.c +2 -0

+	void *sysctl_base;
+	void *timer01_base;

Do you not use sparse? __iomem.

Ok.

Quoted 10 lines

arch/arm/mach-vexpress/v2m.c line 64

Quoted 8 lines

arch/arm/mach-vexpress/v2m.c +6 -0

+	unsigned int timer01_irq;
 	u32 scctrl;
 
+		sysctl_base = ioremap(V2M_SYSCTL, SZ_4K);
+		BUG_ON(!sysctl_base);
+		timer01_base = ioremap(V2M_TIMER01, SZ_4K);
+		BUG_ON(!timer01_base);
+		timer01_irq = IRQ_V2M_TIMER0;

What's going on with the indentation here?

Patch-splitting artefact again, will fix.

Quoted 9 lines

arch/arm/mach-vexpress/v2m.c static void __init v2m_populate_ct_desc(void) · line 436

Quoted 7 lines

arch/arm/mach-vexpress/v2m.c +3 -0

@@ -413,6 +431,10 @@ static void __init v2m_populate_ct_desc(void)
 static void __init v2m_map_io(void)
 {
 	iotable_init(v2m_io_desc, ARRAY_SIZE(v2m_io_desc));
+
+	/* Will become an ioremap() when possible */
+	v2m_sysreg_base = V2M_PERIPH_P2V(V2M_SYSREGS);

It won't if it stays here.

Excuse my inferior English language capabilities, but what do you suggest here?

This patch removes platform SMP callbacks from ct_desc struct and replaces them with global symbols in preparation for DT-based support code.

arch/arm/mach-vexpress/ct-ca9x4.c +0 -5

-static void ct_ca9x4_init_cpu_map(void)
-{
-	int i, ncores = scu_get_core_count(V2TILE_PERIPH_P2V(A9_MPCORE_SCU));
-
-	if (ncores > nr_cpu_ids) {

Quoted 3 lines · Pawel Moll

This patch removes platform SMP callbacks from ct_desc struct and replaces them with global symbols in preparation for DT-based support code.

Will and myself discussed how to do this, and we came up with the ct_desc solution. Now you're doing something different. It seems to me like there's a disconnect between various different parts of ARM Ltd between people who have different ideas about how problems are to be solved.

So, what's the technical reason for this change?

I can't see how this improves anything.

In fact, this patch reintroduces
a bug which have been previously fixed:

arch/arm/mach-vexpress/ct-ca9x4.c static struct map_desc ct_ca9x4_io_desc[] __initdata = { · line 52

Quoted 10 lines

arch/arm/mach-vexpress/ct-ca9x4.c +10 -0

+static void ct_ca9x4_init_cpu_map(void)
+{
+	int i, ncores;
+	ncores = scu_get_core_count(V2T_PERIPH_P2V(A9_MPCORE_SCU));
+
+	for (i = 0; i < ncores; ++i)
+		set_cpu_possible(i, true);
+
+	set_smp_cross_call(gic_raise_softirq);
+}

vs

Quoted 9 lines

arch/arm/mach-vexpress/ct-ca9x4.c +0 -9

-static void ct_ca9x4_init_cpu_map(void)
-{
-	int i, ncores = scu_get_core_count(V2TILE_PERIPH_P2V(A9_MPCORE_SCU));
-
-	if (ncores > nr_cpu_ids) {
-		pr_warn("SMP: %u cores greater than maximum (%u), clipping\n",
-			ncores, nr_cpu_ids);
-		ncores = nr_cpu_ids;
-	}

arch/arm/mach-vexpress/ct-ca9x4.c static void __init ct_ca9x4_init(void) · line 206

Quoted 6 lines

arch/arm/mach-vexpress/ct-ca9x4.c +0 -6

-
-	for (i = 0; i < ncores; ++i)
-		set_cpu_possible(i, true);
-
-	set_smp_cross_call(gic_raise_softirq);
-}

When you rebase, please pay better attention to the conflicts.

This patch provides hooks for DT-based tile machine implementations and adds Device Tree description for the motherboard.

Signed-off-by Pawel Moll <[email protected]>

---
 Documentation/devicetree/bindings/arm/vexpress    |   92 ++++++++++
 arch/arm/boot/dts/vexpress-v2m-legacy.dtsi        |  190 +++++++++++++++++++++
 arch/arm/mach-vexpress/Kconfig                    |    4 +
 arch/arm/mach-vexpress/core.h                     |    9 +
 arch/arm/mach-vexpress/include/mach/motherboard.h |    8 +
 arch/arm/mach-vexpress/v2m.c                      |  140 +++++++++++++++-
 6 files changed, 442 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/vexpress
 create mode 100644 arch/arm/boot/dts/vexpress-v2m-legacy.dtsi

Documentation/devicetree/bindings/arm/vexpress +0 -0

+ where <model> is the the model, eg:

arch/arm/mach-vexpress/v2m.c +0 -0

arch/arm/mach-vexpress/v2m.c +13 -0

+void __init v2m_dt_init_early(void)
+{
+	struct device_node *node;
+	const __be32 *reg;
+	u32 dt_hbi;
+
+	node = of_find_compatible_node(NULL, NULL, "arm,vexpress-sysreg");
+	BUG_ON(!node);
+	/* The following will become of_iomap() when possible */
+	reg = of_get_property(node, "reg", NULL);
+	BUG_ON(!reg);
+	v2m_sysreg_base = V2M_PERIPH_P2V(be32_to_cpup(reg));
+

Quoted 16 lines · Pawel Moll

This patch provides hooks for DT-based tile machine implementations and adds Device Tree description for the motherboard.

Signed-off-by Pawel Moll <[email protected]>

---
 Documentation/devicetree/bindings/arm/vexpress    |   92 ++++++++++
 arch/arm/boot/dts/vexpress-v2m-legacy.dtsi        |  190 +++++++++++++++++++++
 arch/arm/mach-vexpress/Kconfig                    |    4 +
 arch/arm/mach-vexpress/core.h                     |    9 +
 arch/arm/mach-vexpress/include/mach/motherboard.h |    8 +
 arch/arm/mach-vexpress/v2m.c                      |  140 +++++++++++++++-
 6 files changed, 442 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/vexpress
 create mode 100644 arch/arm/boot/dts/vexpress-v2m-legacy.dtsi

Documentation/devicetree/bindings/arm/vexpress +0 -0

[...]

Documentation/devicetree/bindings/arm/vexpress line 18

Quoted 6 lines

Documentation/devicetree/bindings/arm/vexpress +6 -0

+Required properties in the root node:
+- compatible value:
+  - for motherboard in "legacy" mode:
+	compatible = "arm,vexpress-<model>", "arm,vexpress-legacy", "arm-vexpress";
+  - for motherboard in "RS1" mode:
+	compatible = "arm,vexpress-<model>", "arm-vexpress";

So, we have:

arm,vexpress-* implies arm,vexpress
arm,vexpress-* may imply arm,vexpress-legacy
arm,vexpress-legacy implies arm,vexpress

This means we have no bounded test for RS1-only features:
the needed test is
"compatible(node, "arm,vexpress") && !compatible(node, "arm,vexpress-legacy")

Unfortunately, if there is someday an "rs2" memory map, that will also match the above. Using "inverse compatibility" in this way feels dangerous, because the condition will pass for an arbitrary set of future conditions that we haven't imagined yet.

This means it's, impossible even in principle to panic the kernel cleanly if presented with a device tree for a platform variant which is too new for the kernel to support.

Can we instead have a specific "arm,vexpress-rs1"?

	compatible = "arm,vexpress-<model>", "arm-vexpress-rs1", "arm-vexpress";

Then, we can be exact about compatibility: universal features are compatible with "arm,vexpress"; memory-map-specific features are compatible with either "arm,vexpress-legacy" or "arm,vexpress-rs1".

Quoted 49 lines · Dave Martin

Quoted 16 lines · Pawel Moll

This patch provides hooks for DT-based tile machine implementations and adds Device Tree description for the motherboard.

Signed-off-by Pawel Moll <[email protected]>

---
 Documentation/devicetree/bindings/arm/vexpress    |   92 ++++++++++
 arch/arm/boot/dts/vexpress-v2m-legacy.dtsi        |  190 +++++++++++++++++++++
 arch/arm/mach-vexpress/Kconfig                    |    4 +
 arch/arm/mach-vexpress/core.h                     |    9 +
 arch/arm/mach-vexpress/include/mach/motherboard.h |    8 +
 arch/arm/mach-vexpress/v2m.c                      |  140 +++++++++++++++-
 6 files changed, 442 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/vexpress
 create mode 100644 arch/arm/boot/dts/vexpress-v2m-legacy.dtsi

Documentation/devicetree/bindings/arm/vexpress +0 -0

[...]

Documentation/devicetree/bindings/arm/vexpress line 18

Quoted 6 lines

Documentation/devicetree/bindings/arm/vexpress +6 -0

+Required properties in the root node:
+- compatible value:
+  - for motherboard in "legacy" mode:
+	compatible = "arm,vexpress-<model>", "arm,vexpress-legacy", "arm-vexpress";
+  - for motherboard in "RS1" mode:
+	compatible = "arm,vexpress-<model>", "arm-vexpress";

So, we have:

arm,vexpress-* implies arm,vexpress
arm,vexpress-* may imply arm,vexpress-legacy
arm,vexpress-legacy implies arm,vexpress

This means we have no bounded test for RS1-only features:
the needed test is
"compatible(node, "arm,vexpress") && !compatible(node, "arm,vexpress-legacy")

Unfortunately, if there is someday an "rs2" memory map, that will also match the above. Using "inverse compatibility" in this way feels dangerous, because the condition will pass for an arbitrary set of future conditions that we haven't imagined yet.

This means it's, impossible even in principle to panic the kernel cleanly if presented with a device tree for a platform variant which is too new for the kernel to support.

Can we instead have a specific "arm,vexpress-rs1"?

Quoted 5 lines

	compatible = "arm,vexpress-<model>", "arm-vexpress-rs1", "arm-vexpress";

Then, we can be exact about compatibility: universal features are compatible with "arm,vexpress"; memory-map-specific features are compatible with either "arm,vexpress-legacy" or "arm,vexpress-rs1".

Really, legacy is a bad name.

If you defined the property when the
original vexpress was designed, it never would have had legacy in the name. Generally speaking you never change bindings on old platforms.

So I would have "arm,vexpress" mean legacy and "arm,vexpress-rs1" be the new memory map.

Quoted 1 line

	compatible = "arm,vexpress-<model>", "arm-vexpress-rs1", "arm-vexpress";

Documentation/devicetree/bindings/arm/vexpress line 16

Quoted 1 line

Documentation/devicetree/bindings/arm/vexpress new file +1 -0

+	compatible = "arm,vexpress-<model>", "arm,vexpress-legacy", "arm-vexpress";

Quoted 56 lines · Rob Herring

Quoted 54 lines · Dave Martin

Quoted 16 lines · Pawel Moll

This patch provides hooks for DT-based tile machine implementations and adds Device Tree description for the motherboard.

Signed-off-by Pawel Moll <[email protected]>

---
 Documentation/devicetree/bindings/arm/vexpress    |   92 ++++++++++
 arch/arm/boot/dts/vexpress-v2m-legacy.dtsi        |  190 +++++++++++++++++++++
 arch/arm/mach-vexpress/Kconfig                    |    4 +
 arch/arm/mach-vexpress/core.h                     |    9 +
 arch/arm/mach-vexpress/include/mach/motherboard.h |    8 +
 arch/arm/mach-vexpress/v2m.c                      |  140 +++++++++++++++-
 6 files changed, 442 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/vexpress
 create mode 100644 arch/arm/boot/dts/vexpress-v2m-legacy.dtsi

Documentation/devicetree/bindings/arm/vexpress +0 -0

[...]

Documentation/devicetree/bindings/arm/vexpress line 18

Quoted 6 lines

Documentation/devicetree/bindings/arm/vexpress +6 -0

+Required properties in the root node:
+- compatible value:
+  - for motherboard in "legacy" mode:
+	compatible = "arm,vexpress-<model>", "arm,vexpress-legacy", "arm-vexpress";
+  - for motherboard in "RS1" mode:
+	compatible = "arm,vexpress-<model>", "arm-vexpress";

So, we have:

arm,vexpress-* implies arm,vexpress
arm,vexpress-* may imply arm,vexpress-legacy
arm,vexpress-legacy implies arm,vexpress

This means we have no bounded test for RS1-only features:
the needed test is
"compatible(node, "arm,vexpress") && !compatible(node, "arm,vexpress-legacy")

Unfortunately, if there is someday an "rs2" memory map, that will also match the above. Using "inverse compatibility" in this way feels dangerous, because the condition will pass for an arbitrary set of future conditions that we haven't imagined yet.

This means it's, impossible even in principle to panic the kernel cleanly if presented with a device tree for a platform variant which is too new for the kernel to support.

Can we instead have a specific "arm,vexpress-rs1"?

	compatible = "arm,vexpress-<model>", "arm-vexpress-rs1", "arm-vexpress";

Then, we can be exact about compatibility: universal features are compatible with "arm,vexpress"; memory-map-specific features are compatible with either "arm,vexpress-legacy" or "arm,vexpress-rs1".

Quoted 1 line

Really, legacy is a bad name.

http://en.wikipedia.org/wiki/De_gustibus_non_est_disputandum ;-)

This is de-facto name we use in ARM. See:
http://infocenter.arm.com/help/topic/com.arm.doc.dui0447e/ch04s02s01.html

Quoted 6 lines

If you defined the property when the
original vexpress was designed, it never would have had legacy in the name. Generally speaking you never change bindings on old platforms.

So I would have "arm,vexpress" mean legacy and "arm,vexpress-rs1" be the new memory map.

I'd rather second Dave's idea of having

Quoted 1 line

Quoted 1 line

	compatible = "arm,vexpress-<model>", "arm-vexpress-rs1", "arm-vexpress";

and

Quoted 1 line

Documentation/devicetree/bindings/arm/vexpress line 16

Quoted 1 line

Documentation/devicetree/bindings/arm/vexpress new file +1 -0

+	compatible = "arm,vexpress-<model>", "arm,vexpress-legacy", "arm-vexpress";

It has nothing to do with taste and obviously documentation changes over time. I'm going to start naming everything with legacy because someday it all will be...

It's about how you create compatible strings. They should not be generic, but specific to particular hardware version. If you happen to be compatible with older h/w then you can claim compatibility with that older h/w.

Quoted 9 lines

Quoted 6 lines

If you defined the property when the
original vexpress was designed, it never would have had legacy in the name. Generally speaking you never change bindings on old platforms.

So I would have "arm,vexpress" mean legacy and "arm,vexpress-rs1" be the new memory map.

I'd rather second Dave's idea of having

Quoted 5 lines

Quoted 1 line

Quoted 1 line

	compatible = "arm,vexpress-<model>", "arm-vexpress-rs1", "arm-vexpress";

and

Quoted 1 line

Documentation/devicetree/bindings/arm/vexpress line 16

Quoted 1 line

Documentation/devicetree/bindings/arm/vexpress new file +1 -0

+	compatible = "arm,vexpress-<model>", "arm,vexpress-legacy", "arm-vexpress";

If arm,vexpress-ca9 is the only legacy platform, then just drop arm,vexpress-legacy altogether.

Quoted 6 lines

Quoted 6 lines

I'd rather second Dave's idea of having

Quoted 1 line

Quoted 1 line

	compatible = "arm,vexpress-<model>", "arm-vexpress-rs1", "arm-vexpress";

and

Quoted 4 lines

Quoted 1 line

Quoted 1 line

Documentation/devicetree/bindings/arm/vexpress line 16

Quoted 1 line

Documentation/devicetree/bindings/arm/vexpress new file +1 -0

+	compatible = "arm,vexpress-<model>", "arm,vexpress-legacy", "arm-vexpress";

If arm,vexpress-ca9 is the only legacy platform, then just drop arm,vexpress-legacy altogether.

It's not. There is additional one, which is not publicly available, but is using the motherboard in legacy mode.

Quoted 6 lines · Pawel Moll

Quoted 6 lines

Quoted 6 lines

I'd rather second Dave's idea of having

Quoted 1 line

Quoted 1 line

	compatible = "arm,vexpress-<model>", "arm-vexpress-rs1", "arm-vexpress";

and

Quoted 7 lines

Quoted 4 lines

Quoted 1 line

Quoted 1 line

Review

Quoted 1 line

(quoted diff) +1 -0

+	compatible = "arm,vexpress-<model>", "arm,vexpress-legacy", "arm-vexpress";

If arm,vexpress-ca9 is the only legacy platform, then just drop arm,vexpress-legacy altogether.

It's not. There is additional one, which is not publicly available, but is using the motherboard in legacy mode.

Alternatively, I could add motherboard node property, something like:

/ {
        motherboard {
		arm,v2m-memory-map = "legacy";

and

/ {
        motherboard {
		arm,v2m-memory-map = "rs1";

That way the "legacies" and "rses" will disappear from the main compatible value:

	compatible = "arm,vexpress-<model>", "arm-vexpress";

and everyone will be happy :-) There will be a bit more hassle with getting this property in v2m.c, but not too much. Does it make any sense?

Quoted 8 lines · Rob Herring

It has nothing to do with taste and obviously documentation changes over time. I'm going to start naming everything with legacy because someday it all will be...

It's about how you create compatible strings. They should not be generic, but specific to particular hardware version. If you happen to be compatible with older h/w then you can claim compatibility with that older h/w.

Notice that it's not:

compatible=legacy

not even:

compatible=arm,legacy

It's:

compatible=arm,vexpress-legacy

A specific variant of Versatile Express hardware. It's just that the "legacy" word carries some meaning. Would it looked better if it was called:

compatible=arm,vexpress-nalatenskap

? (thanks, google translate ;-)

Quoted 10 lines

Quoted 10 lines

Quoted 7 lines

If you defined the property when the
original vexpress was designed, it never would have had legacy in the name. Generally speaking you never change bindings on old platforms.

So I would have "arm,vexpress" mean legacy and "arm,vexpress-rs1" be the new memory map.

I'd rather second Dave's idea of having

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

Quoted 1 line

	compatible = "arm,vexpress-<model>", "arm-vexpress-rs1", "arm-vexpress";

and

Quoted 1 line

Documentation/devicetree/bindings/arm/vexpress line 16

Quoted 1 line

Documentation/devicetree/bindings/arm/vexpress new file +1 -0

+	compatible = "arm,vexpress-<model>", "arm,vexpress-legacy", "arm-vexpress";

If arm,vexpress-ca9 is the only legacy platform, then just drop arm,vexpress-legacy altogether.

It's not. There is additional one, which is not publicly available, but is using the motherboard in legacy mode.

Cheers!

Pawe?

Quoted 8 lines · Rob Herring

It has nothing to do with taste and obviously documentation changes over time. I'm going to start naming everything with legacy because someday it all will be...

It's about how you create compatible strings. They should not be generic, but specific to particular hardware version. If you happen to be compatible with older h/w then you can claim compatibility with that older h/w.

Notice that it's not:

compatible=legacy

not even:

compatible=arm,legacy

It's:

compatible=arm,vexpress-legacy

A specific variant of Versatile Express hardware. It's just that the "legacy" word carries some meaning. Would it looked better if it was called:

compatible=arm,vexpress-nalatenskap

? (thanks, google translate ;-)

Quoted 7 lines

Quoted 4 lines

and

Quoted 1 line

Review

Quoted 1 line

(quoted diff) +1 -0

+	compatible = "arm,vexpress-<model>", "arm,vexpress-legacy", "arm-vexpress";

If arm,vexpress-ca9 is the only legacy platform, then just drop arm,vexpress-legacy altogether.

It's not. There is additional one, which is not publicly available, but is using the motherboard in legacy mode.

Come to think of it, is the problem here that we're trying to describe the _motherboard_ using the compatible property on the root node. This is why I talked about universal/generic features -- the set of features common to all platforms sharing a single motherboard configuration.

Arguably that's wrong, and that compatible property belongs on the motherboard node itself, so (excuse the random syntax):

/ {
	compatible = "arm,vexpress-v2p-ca9", "arm,vexpress";

	motherboard {
		compatible = "arm,vexpress-v2m-legacy", "simple-bus";
	};
};

If putting a suitable compatible property on the motherboard node is feasible, it should work for this case.

Quoted 8 lines · Rob Herring

It has nothing to do with taste and obviously documentation changes over time. I'm going to start naming everything with legacy because someday it all will be...

It's about how you create compatible strings. They should not be generic, but specific to particular hardware version. If you happen to be compatible with older h/w then you can claim compatibility with that older h/w.

Notice that it's not:

compatible=legacy

not even:

compatible=arm,legacy

It's:

compatible=arm,vexpress-legacy

A specific variant of Versatile Express hardware. It's just that the "legacy" word carries some meaning. Would it looked better if it was called:

compatible=arm,vexpress-nalatenskap

? (thanks, google translate ;-)

You're totally failing to understand the point.

The point is that when Versatile Express first came out, it had a memory map. At this point in time, there was nothing 'legacy' about it, it was the latest and greatest thing.

Again, this is _not_ how DT is supposed to work. DT is about describing the hardware, not describing the state of something.

So, get rid of that 'legacy' crap in DT naming. It doesn't belong. Or we'll start talling the Cortex-A5 stuff 'legacy' right now as well.

Quoted 2 lines · Russell King - ARM Linux

Again, this is _not_ how DT is supposed to work. DT is about describing the hardware, not describing the state of something.

And that's exactly what I was trying to do. Describe the hardware, using terminology *used by the hardware designers* - fact that you have chosen to ignore, fair enough.

Quoted 1 line

+ where <model> is the the model, eg:

Two "the"s.

What "model" means isn't obvious to the uninitiated -- how about something like "where <model> is the full platform model name" ?

Documentation/devicetree/bindings/arm/vexpress line 27

Quoted 8 lines

Documentation/devicetree/bindings/arm/vexpress new file +8 -0

+  - for Coretile Express A5x2 (V2P-CA5s):
+	compatible = "arm,vexpress-v2p-ca5s", "arm-vexpress";
+  - Coretile Express A9x4 (V2P-CA9):
+	comaptible = "arm,vexpress-v2p-ca9", "arm,vexpress-legacy", "arm-vexpress";
+
+Current Linux implementation requires a "timer" alias pointing
+at a SP804 timer block to be used when tile is not using local
+timer source.

We should specify a list of all the "standard" aliases used by the generic motherboard code here, since these are part of the contract between each board-specific device tree and the motherboard code.

Is "timer" the only one, or are there others?

arch/arm/mach-vexpress/Kconfig config ARCH_VEXPRESS_CA9X4 · line 11

Quoted 8 lines

arch/arm/mach-vexpress/Kconfig +0 -0

index 9311484..4c11e90 100644
@@ -9,4 +9,8 @@ config ARCH_VEXPRESS_CA9X4
 	select ARM_ERRATA_751472
 	select ARM_ERRATA_753970
 

For "non-interactive" config items, can we still please have comments indicating what the item means and how it should be used?

Such comments can be very brief, but having at least something makes the configs easier to understand and maintain, in my view.

Quoted 1 line

arch/arm/mach-vexpress/v2m.c +0 -0

[...]

Quoted 18 lines

arch/arm/mach-vexpress/v2m.c +18 -0

+void __init v2m_dt_init_early(void)
+{
+	struct device_node *node;
+	const __be32 *reg;
+	u32 dt_hbi;
+
+	node = of_find_compatible_node(NULL, NULL, "arm,vexpress-sysreg");
+	BUG_ON(!node);
+	/* The following will become of_iomap() when possible */
+	reg = of_get_property(node, "reg", NULL);
+	BUG_ON(!reg);
+	v2m_sysreg_base = V2M_PERIPH_P2V(be32_to_cpup(reg));
+
+	/* Confirm board type against DT property, if available */
+	if (of_property_read_u32(allnodes, "arm,hbi", &dt_hbi) == 0) {
+		u32 misc = readl(v2m_sysreg_base + V2M_SYS_MISC);
+		u32 id = readl(v2m_sysreg_base + (misc & SYS_MISC_MASTERSITE ?
+				V2M_SYS_PROCID1 : V2M_SYS_PROCID0));

arch/arm/mach-vexpress/v2m.c MACHINE_START(VEXPRESS, "ARM-Versatile Express") · line 587

Quoted 5 lines

arch/arm/mach-vexpress/v2m.c +5 -0

+		u32 hbi = id & SYS_PROCIDx_HBI_MASK;
+
+		if (WARN_ON(dt_hbi != hbi))
+			pr_warning("vexpress: DT HBI (%x) is not matching "
+					"hardware (%x)!\n", dt_hbi, hbi);

Once this code is mature enough, should we panic the kernel here instead?

Quoted 13 lines

Documentation/devicetree/bindings/arm/vexpress line 27

Quoted 8 lines

Documentation/devicetree/bindings/arm/vexpress new file +8 -0

+  - for Coretile Express A5x2 (V2P-CA5s):
+	compatible = "arm,vexpress-v2p-ca5s", "arm-vexpress";
+  - Coretile Express A9x4 (V2P-CA9):
+	comaptible = "arm,vexpress-v2p-ca9", "arm,vexpress-legacy", "arm-vexpress";
+
+Current Linux implementation requires a "timer" alias pointing
+at a SP804 timer block to be used when tile is not using local
+timer source.

We should specify a list of all the "standard" aliases used by the generic motherboard code here, since these are part of the contract between each board-specific device tree and the motherboard code.

Quoted 1 line

Is "timer" the only one, or are there others?

One and only. There were more, but I got rid of them.

Quoted 12 lines

arch/arm/mach-vexpress/Kconfig config ARCH_VEXPRESS_CA9X4 · line 11

Quoted 8 lines

arch/arm/mach-vexpress/Kconfig +0 -0

index 9311484..4c11e90 100644
@@ -9,4 +9,8 @@ config ARCH_VEXPRESS_CA9X4
 	select ARM_ERRATA_751472
 	select ARM_ERRATA_753970
 

For "non-interactive" config items, can we still please have comments indicating what the item means and how it should be used?

Quoted 2 lines

Such comments can be very brief, but having at least something makes the configs easier to understand and maintain, in my view.

Em, what bits do you refer to? The lines you quoted are not changed...

Quoted 23 lines

Quoted 2 lines

arch/arm/mach-vexpress/v2m.c +0 -0

[...]

Quoted 18 lines

arch/arm/mach-vexpress/v2m.c +18 -0

+void __init v2m_dt_init_early(void)
+{
+	struct device_node *node;
+	const __be32 *reg;
+	u32 dt_hbi;
+
+	node = of_find_compatible_node(NULL, NULL, "arm,vexpress-sysreg");
+	BUG_ON(!node);
+	/* The following will become of_iomap() when possible */
+	reg = of_get_property(node, "reg", NULL);
+	BUG_ON(!reg);
+	v2m_sysreg_base = V2M_PERIPH_P2V(be32_to_cpup(reg));
+
+	/* Confirm board type against DT property, if available */
+	if (of_property_read_u32(allnodes, "arm,hbi", &dt_hbi) == 0) {
+		u32 misc = readl(v2m_sysreg_base + V2M_SYS_MISC);
+		u32 id = readl(v2m_sysreg_base + (misc & SYS_MISC_MASTERSITE ?
+				V2M_SYS_PROCID1 : V2M_SYS_PROCID0));

Quoted 8 lines

arch/arm/mach-vexpress/v2m.c MACHINE_START(VEXPRESS, "ARM-Versatile Express") · line 587

Quoted 5 lines

arch/arm/mach-vexpress/v2m.c +5 -0

+		u32 hbi = id & SYS_PROCIDx_HBI_MASK;
+
+		if (WARN_ON(dt_hbi != hbi))
+			pr_warning("vexpress: DT HBI (%x) is not matching "
+					"hardware (%x)!\n", dt_hbi, hbi);

Once this code is mature enough, should we panic the kernel here instead?

I'm not convinced. Actually the first version I wrote was panicking. But then I thought that if it works, it works - why should I prevent this? The use case is simple - FPGA implementations, being mostly-exact-equivalents of the core tiles, will have the logic tile HBI here. And to use them you would have to modify the DTS if this code panicked. With WARN_ON you'll see the message, but if you know what are you doing, that's fine...

Don't know, really. Any strong feelings about it?

Quoted 7 lines

Quoted 5 lines

We should specify a list of all the "standard" aliases used by the generic motherboard code here, since these are part of the contract between each board-specific device tree and the motherboard code.

Is "timer" the only one, or are there others?

One and only. There were more, but I got rid of them.

Are the other alises still used? If not, we should perhaps get rid of them, or keep them on an as-needed basis only?

Since alises are by definition a point of standardisation (otherwise there's no need for an alias) I think they need to be documented alongside bindings wherever we have them.

Quoted 6 lines

Quoted 5 lines

For "non-interactive" config items, can we still please have comments indicating what the item means and how it should be used?

Such comments can be very brief, but having at least something makes the configs easier to understand and maintain, in my view.

Quoted 1 line

Em, what bits do you refer to? The lines you quoted are not changed...

Hmmm, editing snafu there.

I think I was referring to the ARCH_VEXPRESS_DT item, which is defined just after the lines I quoted.

Quoted 10 lines · Dave Martin

Quoted 7 lines

Quoted 5 lines

We should specify a list of all the "standard" aliases used by the generic motherboard code here, since these are part of the contract between each board-specific device tree and the motherboard code.

Is "timer" the only one, or are there others?

One and only. There were more, but I got rid of them.

Are the other alises still used? If not, we should perhaps get rid of them, or keep them on an as-needed basis only?

I've included the serialX and i2cX aliases purely because the other boards do (including Grant's versatile dts).

Quoted 3 lines

Since alises are by definition a point of standardisation (otherwise there's no need for an alias) I think they need to be documented alongside bindings wherever we have them.

You are right, but I think this is out of the vexpress scope. The serialX aliases, for example, are already used by some drivers:

   4   1788  drivers/tty/serial/atmel_serial.c <<atmel_serial_probe>>
             ret = of_alias_get_id(np, "serial");
   5   1301  drivers/tty/serial/imx.c <<serial_imx_probe_dt>>
             ret = of_alias_get_id(np, "serial");

so it sounds like a generic policy-to-be-made.

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

For "non-interactive" config items, can we still please have comments indicating what the item means and how it should be used?

Such comments can be very brief, but having at least something makes the configs easier to understand and maintain, in my view.

Quoted 6 lines

Quoted 1 line

Em, what bits do you refer to? The lines you quoted are not changed...

Hmmm, editing snafu there.

I think I was referring to the ARCH_VEXPRESS_DT item, which is defined just after the lines I quoted.

Ok, will do.

arch/arm/mach-vexpress/v2m.c +7 -0

+#if defined(CONFIG_ARCH_VEXPRESS_DT)
+		int err;
+		const char *path;
+		struct device_node *node;
+
+		node = of_find_compatible_node(NULL, NULL, "arm,sp810");
+		BUG_ON(!node);

arch/arm/mach-vexpress/v2m.c static struct clk_lookup v2m_lookups[] = { · line 418

Quoted 8 lines

arch/arm/mach-vexpress/v2m.c +4 -0

@@ -383,11 +412,18 @@ static struct clk_lookup v2m_lookups[] = {
 	},
 };
 
+static void __init v2m_system_id(void)
+{
+	if (!system_rev)
+		system_rev = readl(v2m_sysreg_base + V2M_SYS_ID);

You do understand that system_rev is for the system _revision_ not for some kind of system ID. For example, it's to identify whether we're on a revision 4, 5 or 6 system.

However, with DT the differences in system revision should be encoded into the DT itself, and the kernel should not be making choices about the hardware off this.

Quoted 7 lines · Russell King - ARM Linux

You do understand that system_rev is for the system _revision_ not for some kind of system ID. For example, it's to identify whether we're on a revision 4, 5 or 6 system.

However, with DT the differences in system revision should be encoded into the DT itself, and the kernel should not be making choices about the hardware off this.

I can't comment on whether this is an abuse of system_rev, since I'm not too familiar with that.

I feel that Whether the value of the V2M_SYS_ID register should be put in the DT is more doubtful though: the DT must describe the hardware which cannot be probed.

Quoted 11 lines · Pawel Moll

arch/arm/mach-vexpress/v2m.c +11 -0

+#if defined(CONFIG_ARCH_VEXPRESS_DT)
+		int err;
+		const char *path;
+		struct device_node *node;
+
+		node = of_find_compatible_node(NULL, NULL, "arm,sp810");
+		BUG_ON(!node);
+		sysctl_base = of_iomap(node, 0);
+		BUG_ON(!sysctl_base);
+
+		err = of_property_read_string(of_aliases, "timer", &path);

arch/arm/mach-vexpress/v2m.c static void __init v2m_timer_init(void) · line 83

Quoted 6 lines

arch/arm/mach-vexpress/v2m.c +6 -0

+		BUG_ON(err);
+		node = of_find_node_by_path(path);
+		BUG_ON(!node);
+		timer01_base = of_iomap(node, 0);
+		BUG_ON(!timer01_base);
+		timer01_irq = irq_of_parse_and_map(node, 0);

Are you sure you have enough BUG_ON()s there?

It's well worth reading
Linus' various messages on the use of BUG(), which can be found:

http://yarchive.net/comp/linux/BUG.html

The lack of the timer and sysctl registers are really a 'report and maybe we could get a message out' kind of scenario rather than a 'oops, kill the machine'.

arch/arm/mach-vexpress/v2m.c static void __init v2m_timer_init(void) · line 91

Quoted 8 lines

arch/arm/mach-vexpress/v2m.c +3 -0

+#endif
+	} else {
 		sysctl_base = ioremap(V2M_SYSCTL, SZ_4K);
 		BUG_ON(!sysctl_base);
 		timer01_base = ioremap(V2M_TIMER01, SZ_4K);
 		BUG_ON(!timer01_base);
 		timer01_irq = IRQ_V2M_TIMER0;
+	}

And in any case, both these paths have a common set of BUG_ON()s:

	BUG_ON(!sysctl_base);
	BUG_ON(!timer01_base);

So do we really need them having this independently?

Quoted 4 lines

(quoted diff) +0 -0

@@ -383,11 +412,18 @@ static struct clk_lookup v2m_lookups[] = {
 	},
 };
 

arch/arm/mach-vexpress/v2m.c static struct clk_lookup v2m_lookups[] = { · line 418

Quoted 4 lines

arch/arm/mach-vexpress/v2m.c +4 -0

+static void __init v2m_system_id(void)
+{
+	if (!system_rev)
+		system_rev = readl(v2m_sysreg_base + V2M_SYS_ID);

You do understand that system_rev is for the system _revision_ not for some kind of system ID. For example, it's to identify whether we're on a revision 4, 5 or 6 system.

However, with DT the differences in system revision should be encoded into the DT itself, and the kernel should not be making choices about the hardware off this.

This patch adds support for RS1 memory map based Versatile Express motherboard. As the RAM location has changed, the ZRE values and PLAT_PHYS_OFFSET defaults are changed to the new address (all future tiles will use RS1 map) and enforces AUTO_ZRELADD and ARM_PATCH_PHYS_VIRT when legacy devices are being used.

Signed-off-by Pawel Moll <[email protected]>

---
 arch/arm/boot/dts/vexpress-v2m-rs1.dtsi           |  190 +++++++++++++++++++++
 arch/arm/mach-vexpress/Kconfig                    |    9 +
 arch/arm/mach-vexpress/Makefile.boot              |    6 +-
 arch/arm/mach-vexpress/include/mach/debug-macro.S |   37 ++++-
 arch/arm/mach-vexpress/include/mach/uncompress.h  |   13 ++-
 arch/arm/mach-vexpress/v2m.c                      |   46 +++++
 6 files changed, 293 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm/boot/dts/vexpress-v2m-rs1.dtsi

arch/arm/mach-vexpress/include/mach/debug-macro.S +22 -4

index fd9e6c7..adc94ce 100644
@@ -10,12 +10,41 @@
  * published by the Free Software Foundation.
  */
 
-#define DEBUG_LL_UART_OFFSET	0x00009000
+#define VEXPRESS_PHYS_BASE_LEGACY	0x10000000
+#define VEXPRESS_UART_OFFSET_LEGACY	0x00009000
+
+#define VEXPRESS_PHYS_BASE_RS1		0x1c000000
+#define VEXPRESS_UART_OFFSET_RS1	0x00090000
+
+#define VEXPRESS_VIRT_BASE		0xf8000000
 
 		.macro	addruart,rp,rv,tmp
-		mov	\rp, #DEBUG_LL_UART_OFFSET
-		orr	\rv, \rp, #0xf8000000	@ virtual base
-		orr	\rp, \rp, #0x10000000	@ physical base
+
+		@ Check the MMU state
+#if defined(CONFIG_MMU)
+		mrc	p15, 0, \tmp, c1, c0	@ SCTRL
+		tst	\tmp, #1		@ MMU enabled?
+		moveq	\tmp, #VEXPRESS_PHYS_BASE_LEGACY
+		movne	\tmp, #VEXPRESS_VIRT_BASE
+#else
+		mov	\tmp, #VEXPRESS_PHYS_BASE_LEGACY
+#endif
+
+		@ PL011 present in "legacy place"?
+		orr	\tmp, \tmp, #VEXPRESS_UART_OFFSET_LEGACY
+		ldr	\tmp, [\tmp, #0xfe0]	@ PeriphID0
+		teq	\tmp, #0x11		@ PL011

arch/arm/mach-vexpress/v2m.c +0 -0

Quoted 1 line

+config ARCH_VEXPRESS_LEGACY

Brief comment explaining what this is?

Quoted 1 line

+config ARCH_VEXPRESS_RS1

Ditto

arch/arm/mach-vexpress/Kconfig config ARCH_VEXPRESS_CA9X4 · line 19

Quoted 10 lines

arch/arm/mach-vexpress/Kconfig +3 -0

+	bool
+
 config ARCH_VEXPRESS_CA9X4
 	bool "Versatile Express Cortex-A9x4 tile"
 	select CPU_V7

@@ -8,6 +16,7 @@ config ARCH_VEXPRESS_CA9X4
 	select ARM_ERRATA_720789
 	select ARM_ERRATA_751472
 	select ARM_ERRATA_753970
+	select ARCH_VEXPRESS_LEGACY

We should add a brief note to the help text explaining the existence of the new combined A9x4/A5x2 DT based support, if were planning to keep both variants. (Probably wise for now, since people may be temporarily stuck old bootloaders etc.)

If there is no help text yet, we should add some, but it should be kept brief.

[...]

Quoted 40 lines

arch/arm/mach-vexpress/include/mach/debug-macro.S +26 -4

index fd9e6c7..adc94ce 100644
@@ -10,12 +10,41 @@
  * published by the Free Software Foundation.
  */
 
-#define DEBUG_LL_UART_OFFSET	0x00009000
+#define VEXPRESS_PHYS_BASE_LEGACY	0x10000000
+#define VEXPRESS_UART_OFFSET_LEGACY	0x00009000
+
+#define VEXPRESS_PHYS_BASE_RS1		0x1c000000
+#define VEXPRESS_UART_OFFSET_RS1	0x00090000
+
+#define VEXPRESS_VIRT_BASE		0xf8000000
 
 		.macro	addruart,rp,rv,tmp
-		mov	\rp, #DEBUG_LL_UART_OFFSET
-		orr	\rv, \rp, #0xf8000000	@ virtual base
-		orr	\rp, \rp, #0x10000000	@ physical base
+
+		@ Check the MMU state
+#if defined(CONFIG_MMU)
+		mrc	p15, 0, \tmp, c1, c0	@ SCTRL
+		tst	\tmp, #1		@ MMU enabled?
+		moveq	\tmp, #VEXPRESS_PHYS_BASE_LEGACY
+		movne	\tmp, #VEXPRESS_VIRT_BASE
+#else
+		mov	\tmp, #VEXPRESS_PHYS_BASE_LEGACY
+#endif
+
+		@ PL011 present in "legacy place"?
+		orr	\tmp, \tmp, #VEXPRESS_UART_OFFSET_LEGACY
+		ldr	\tmp, [\tmp, #0xfe0]	@ PeriphID0
+		teq	\tmp, #0x11		@ PL011
+
+		@ Legacy memory map
+		moveq	\rp, #VEXPRESS_UART_OFFSET_LEGACY
+		orreq	\rv, \rp, #VEXPRESS_VIRT_BASE

arch/arm/mach-vexpress/include/mach/debug-macro.S line 46

Quoted 6 lines

arch/arm/mach-vexpress/include/mach/debug-macro.S +6 -0

+		orreq	\rp, \rp, #VEXPRESS_PHYS_BASE_LEGACY
+
+		@ RS1 memory map
+		movne	\rp, #VEXPRESS_UART_OFFSET_RS1
+		orrne	\rv, \rp, #VEXPRESS_VIRT_BASE
+		orrne	\rp, \rp, #VEXPRESS_PHYS_BASE_RS1

I will assume that this works :)

Without grokking the detail, it looks fairly reasonable.

[...]

Quoted 1 line

arch/arm/mach-vexpress/v2m.c +0 -0

[...]

arch/arm/mach-vexpress/v2m.c static struct of_dev_auxdata v2m_dt_auxdata_lookup[] __initdata = { · line 649

Quoted 9 lines

void __init v2m_dt_init_early(void)

arch/arm/mach-vexpress/v2m.c +4 -0

@@ -601,6 +643,10 @@ static struct of_dev_auxdata v2m_dt_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("arm,vexpress-flash", V2M_NOR0, "physmap-flash",
 			&v2m_flash_data),
 	OF_DEV_AUXDATA("arm,primecell", V2M_MMCI, "mb:mmci", &v2m_mmci_data),
+	/* RS1 memory map */
+	OF_DEV_AUXDATA("arm,vexpress-flash", 0x08000000, "physmap-flash",
+			&v2m_flash_data),
+	OF_DEV_AUXDATA("arm,primecell", 0x1c050000, "mb:mmci", &v2m_mmci_data),

Can we have macros instead of the magic numbers here?

Quoted 4 lines · Dave Martin

Quoted 1 line

+config ARCH_VEXPRESS_LEGACY

Brief comment explaining what this is?

Quoted 1 line

+config ARCH_VEXPRESS_RS1

Ditto

Ok.

Quoted 16 lines

arch/arm/mach-vexpress/Kconfig config ARCH_VEXPRESS_CA9X4 · line 19

Quoted 10 lines

arch/arm/mach-vexpress/Kconfig +3 -0

+	bool
+
 config ARCH_VEXPRESS_CA9X4
 	bool "Versatile Express Cortex-A9x4 tile"
 	select CPU_V7

@@ -8,6 +16,7 @@ config ARCH_VEXPRESS_CA9X4
 	select ARM_ERRATA_720789
 	select ARM_ERRATA_751472
 	select ARM_ERRATA_753970
+	select ARCH_VEXPRESS_LEGACY

We should add a brief note to the help text explaining the existence of the new combined A9x4/A5x2 DT based support, if were planning to keep both variants. (Probably wise for now, since people may be temporarily stuck old bootloaders etc.)

Quoted 2 lines

If there is no help text yet, we should add some, but it should be kept brief.

No probs. All suggestions and "templates" welcomed :-)

Quoted 44 lines

[...]

Quoted 40 lines

arch/arm/mach-vexpress/include/mach/debug-macro.S +26 -4

index fd9e6c7..adc94ce 100644
@@ -10,12 +10,41 @@
  * published by the Free Software Foundation.
  */
 
-#define DEBUG_LL_UART_OFFSET	0x00009000
+#define VEXPRESS_PHYS_BASE_LEGACY	0x10000000
+#define VEXPRESS_UART_OFFSET_LEGACY	0x00009000
+
+#define VEXPRESS_PHYS_BASE_RS1		0x1c000000
+#define VEXPRESS_UART_OFFSET_RS1	0x00090000
+
+#define VEXPRESS_VIRT_BASE		0xf8000000
 
 		.macro	addruart,rp,rv,tmp
-		mov	\rp, #DEBUG_LL_UART_OFFSET
-		orr	\rv, \rp, #0xf8000000	@ virtual base
-		orr	\rp, \rp, #0x10000000	@ physical base
+
+		@ Check the MMU state
+#if defined(CONFIG_MMU)
+		mrc	p15, 0, \tmp, c1, c0	@ SCTRL
+		tst	\tmp, #1		@ MMU enabled?
+		moveq	\tmp, #VEXPRESS_PHYS_BASE_LEGACY
+		movne	\tmp, #VEXPRESS_VIRT_BASE
+#else
+		mov	\tmp, #VEXPRESS_PHYS_BASE_LEGACY
+#endif
+
+		@ PL011 present in "legacy place"?
+		orr	\tmp, \tmp, #VEXPRESS_UART_OFFSET_LEGACY
+		ldr	\tmp, [\tmp, #0xfe0]	@ PeriphID0
+		teq	\tmp, #0x11		@ PL011
+
+		@ Legacy memory map
+		moveq	\rp, #VEXPRESS_UART_OFFSET_LEGACY
+		orreq	\rv, \rp, #VEXPRESS_VIRT_BASE

Quoted 8 lines

arch/arm/mach-vexpress/include/mach/debug-macro.S line 46

Quoted 6 lines

arch/arm/mach-vexpress/include/mach/debug-macro.S +6 -0

+		orreq	\rp, \rp, #VEXPRESS_PHYS_BASE_LEGACY
+
+		@ RS1 memory map
+		movne	\rp, #VEXPRESS_UART_OFFSET_RS1
+		orrne	\rv, \rp, #VEXPRESS_VIRT_BASE
+		orrne	\rp, \rp, #VEXPRESS_PHYS_BASE_RS1

I will assume that this works :)

It does, tested :-) The probing order "legacy-then-RS1" is carefully selected - it doesn't work the other way round.

Quoted 1 line

Without grokking the detail, it looks fairly reasonable.

I was considering something similar to "arch/arm/mach-omap2/include/mach/debug-macro.S", but it made the code even more obscure.

Quoted 7 lines

[...]

Quoted 1 line

arch/arm/mach-vexpress/v2m.c +0 -0

[...]

Quoted 11 lines

arch/arm/mach-vexpress/v2m.c static struct of_dev_auxdata v2m_dt_auxdata_lookup[] __initdata = { · line 649

Quoted 9 lines

void __init v2m_dt_init_early(void)

arch/arm/mach-vexpress/v2m.c +4 -0

@@ -601,6 +643,10 @@ static struct of_dev_auxdata v2m_dt_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("arm,vexpress-flash", V2M_NOR0, "physmap-flash",
 			&v2m_flash_data),
 	OF_DEV_AUXDATA("arm,primecell", V2M_MMCI, "mb:mmci", &v2m_mmci_data),
+	/* RS1 memory map */
+	OF_DEV_AUXDATA("arm,vexpress-flash", 0x08000000, "physmap-flash",
+			&v2m_flash_data),
+	OF_DEV_AUXDATA("arm,primecell", 0x1c050000, "mb:mmci", &v2m_mmci_data),

Can we have macros instead of the magic numbers here?

I don't see any point:
1. These value are used only and only here, so #defining them would just add extra lines of traffic.
2. They correspond to numbers (as in: numbers) in DTS.
3. They will disappear once I'm done with MMCI and sysreg bindings. So probably long before this series is actually merged...

arch/arm/mach-vexpress/Kconfig +8 -0

index 4c11e90..d1b0f35 100644
@@ -1,6 +1,14 @@
 menu "Versatile Express platform type"
 	depends on ARCH_VEXPRESS
 
+config ARCH_VEXPRESS_LEGACY
+	bool
+	select AUTO_ZRELADDR
+	select ARM_PATCH_PHYS_VIRT
+
+config ARCH_VEXPRESS_RS1
+	bool
+
 config ARCH_VEXPRESS_CA9X4
 	bool "Versatile Express Cortex-A9x4 tile"
 	select CPU_V7

arch/arm/mach-vexpress/Kconfig +0 -0

 config ARCH_VEXPRESS_DT
 	bool

arch/arm/mach-vexpress/Makefile.boot +0 -0

index 8630b3d..3278615 100644

arch/arm/mach-vexpress/Kconfig line 11

Quoted 9 lines

arch/arm/mach-vexpress/Kconfig +8 -0

+config ARCH_VEXPRESS_LEGACY
+	bool
+	select AUTO_ZRELADDR
+	select ARM_PATCH_PHYS_VIRT
+
+config ARCH_VEXPRESS_RS1
+	bool
+

This doesn't make sense. Why should CA9x4 be forced to use the P2V stuff and the AUTO_ZRELADDR stuff? Hint: it works fine today without.

arch/arm/mach-vexpress/Makefile.boot line 3

Quoted 7 lines

arch/arm/mach-vexpress/Makefile.boot +3 -3

@@ -1,3 +1,3 @@
-   zreladdr-y	+= 0x60008000
-params_phys-y	:= 0x60000100
-initrd_phys-y	:= 0x60800000
+   zreladdr-y	+= 0x80008000
+params_phys-y	:= 0x80000100
+initrd_phys-y	:= 0x80800000

So is this the reason - you don't want to turn this into a config, so you want to force all the automatic zreladdr stuff on?

You do know that this
will probably cause uboot to load the uImage at 0x80008000 instead of 0x60008000, and therefore we'll lose half the RAM on this board?

Quoted 3 lines · Russell King - ARM Linux

You do know that this
will probably cause uboot to load the uImage at 0x80008000 instead of 0x60008000, and therefore we'll lose half the RAM on this board?

I missed that - I don't use uboot, so I don't build uImages.

As the ARCH_VEXPRESS_LEGACY will be no more (see my other response) I'll revert the order, so the _RS1 will enforce P2V, AUTO_ZRELADDR and change the zrealaddr-y & friends (as in: 0x6* when no _RS1, 0x8* when _RS1=y). Is there any other way of having single binary kernel booting from both 0x60008000 and 0x80008000?

This patch adds Device Trees for ARM Ltd. CoreTile Express A5x2 and CoreTile Express A9x4 used with V2M motherboard and an initial implementation of the DT machine support (this code is separate from the current core tile code).

Signed-off-by Pawel Moll <[email protected]>

---
 arch/arm/boot/dts/vexpress-v2p-ca5s.dts |  132 ++++++++++++++++++++++++++++
 arch/arm/boot/dts/vexpress-v2p-ca9.dts  |  146 +++++++++++++++++++++++++++++++
 arch/arm/mach-vexpress/Kconfig          |   15 +++
 arch/arm/mach-vexpress/Makefile         |    1 +
 arch/arm/mach-vexpress/v2p-ca5s_ca9.c   |  103 ++++++++++++++++++++++
 5 files changed, 397 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/boot/dts/vexpress-v2p-ca5s.dts
 create mode 100644 arch/arm/boot/dts/vexpress-v2p-ca9.dts
 create mode 100644 arch/arm/mach-vexpress/v2p-ca5s_ca9.c

arch/arm/boot/dts/vexpress-v2p-ca5s.dts new file +31 -0

new file mode 100644
index 0000000..d88aa0a
@@ -0,0 +1,132 @@
+/*
+ * ARM Ltd. Versatile Express
+ *
+ * CoreTile Express A5x2
+ * Cortex-A5 MPCore (V2P-CA5s)
+ *
+ * HBI-0225B
+ */
+
+/dts-v1/;
+
+/include/ "skeleton.dtsi"
+
+/ {
+	model = "V2P-CA5s";
+	arm,hbi = <0x225>;
+	compatible = "arm,vexpress-v2p-ca5s", "arm,vexpress";
+	interrupt-parent = <&gic>;
+
+	aliases {
+		serial0 = &mb_serial0;
+		serial1 = &mb_serial1;
+		serial2 = &mb_serial2;
+		serial3 = &mb_serial3;
+		i2c0 = &mb_i2c_dvi;
+		i2c1 = &mb_i2c_pcie;
+		timer = &mb_timer01;
+	};
+
+	memory at 80000000 {
+		device_type = "memory";

arch/arm/boot/dts/vexpress-v2p-ca5s.dts new file +86 -0

+		compatible = "arm,pl354", "arm,primecell";
+		reg = <0x2a190000 0x1000>;
+		interrupts = <0 86 4>,
+			     <0 87 4>;
+	};
+
+	gic: interrupt-controller at 2c001000 {
+		compatible = "arm,cortex-a9-gic";
+		#interrupt-cells = <3>;
+		#address-cells = <0>;
+		interrupt-controller;
+		reg = <0x2c001000 0x1000>,
+		      <0x2c000100 0x100>;
+	};
+
+	L2: cache-controller at 2c0f0000 {
+		compatible = "arm,pl310-cache";
+		reg = <0x2c0f0000 0x1000>;
+		interrupts = <0 84 4>;
+		cache-level = <2>;
+		arm,data-latency = <0>;
+		arm,tag-latency = <0>;
+	};
+
+	pmu {
+		compatible = "arm,cortex-a9-pmu";
+		interrupts = <0 68 4>,
+			     <0 69 4>;
+	};
+
+	motherboard {
+		ranges = <0 0 0x08000000 0x04000000>,
+			 <1 0 0x14000000 0x04000000>,
+			 <2 0 0x18000000 0x04000000>,
+			 <3 0 0x1c000000 0x04000000>,
+			 <4 0 0x0c000000 0x04000000>,
+			 <5 0 0x10000000 0x04000000>;
+
+		interrupt-map-mask = <0 0 63>;
+		interrupt-map = <0 0  0 &gic 0  0 4>,
+				<0 0  1 &gic 0  1 4>,
+				<0 0  2 &gic 0  2 4>,
+				<0 0  3 &gic 0  3 4>,
+				<0 0  4 &gic 0  4 4>,
+				<0 0  5 &gic 0  5 4>,
+				<0 0  6 &gic 0  6 4>,
+				<0 0  7 &gic 0  7 4>,
+				<0 0  8 &gic 0  8 4>,
+				<0 0  9 &gic 0  9 4>,
+				<0 0 10 &gic 0 10 4>,
+				<0 0 11 &gic 0 11 4>,
+				<0 0 12 &gic 0 12 4>,
+				<0 0 13 &gic 0 13 4>,
+				<0 0 14 &gic 0 14 4>,
+				<0 0 15 &gic 0 15 4>,
+				<0 0 16 &gic 0 16 4>,
+				<0 0 17 &gic 0 17 4>,
+				<0 0 18 &gic 0 18 4>,
+				<0 0 19 &gic 0 19 4>,
+				<0 0 20 &gic 0 20 4>,
+				<0 0 21 &gic 0 21 4>,
+				<0 0 22 &gic 0 22 4>,
+				<0 0 23 &gic 0 23 4>,
+				<0 0 24 &gic 0 24 4>,
+				<0 0 25 &gic 0 25 4>,
+				<0 0 26 &gic 0 26 4>,
+				<0 0 27 &gic 0 27 4>,
+				<0 0 28 &gic 0 28 4>,
+				<0 0 29 &gic 0 29 4>,
+				<0 0 30 &gic 0 30 4>,
+				<0 0 31 &gic 0 31 4>,
+				<0 0 32 &gic 0 32 4>,
+				<0 0 33 &gic 0 33 4>,
+				<0 0 34 &gic 0 34 4>,
+				<0 0 35 &gic 0 35 4>,
+				<0 0 36 &gic 0 36 4>,
+				<0 0 37 &gic 0 37 4>,
+				<0 0 38 &gic 0 38 4>,
+				<0 0 39 &gic 0 39 4>,
+				<0 0 40 &gic 0 40 4>,
+				<0 0 41 &gic 0 41 4>,
+				<0 0 42 &gic 0 42 4>;
+	};
+};
+
+/include/ "vexpress-v2m-rs1.dtsi"

arch/arm/boot/dts/vexpress-v2p-ca9.dts new file +146 -0

new file mode 100644
index 0000000..0f26cba
@@ -0,0 +1,146 @@
+/*
+ * ARM Ltd. Versatile Express
+ *
+ * CoreTile Express A9x4
+ * Cortex-A9 MPCore (V2P-CA9)
+ *
+ * HBI-0191B
+ */
+
+/dts-v1/;
+
+/include/ "skeleton.dtsi"
+
+/ {
+	model = "V2P-CA9";
+	arm,hbi = <0x191>;
+	compatible = "arm,vexpress-v2p-ca9", "arm,vexpress-legacy", "arm,vexpress";
+	interrupt-parent = <&gic>;
+
+	aliases {
+		serial0 = &mb_serial0;
+		serial1 = &mb_serial1;
+		serial2 = &mb_serial2;
+		serial3 = &mb_serial3;
+		i2c0 = &mb_i2c_dvi;
+		i2c1 = &mb_i2c_pcie;
+		timer = &mb_timer01;
+	};
+
+	memory at 60000000 {
+		device_type = "memory";
+		reg = <0x60000000 0x40000000>;
+	};
+
+	clcd at 10020000 {
+		compatible = "arm,pl111", "arm,primecell";
+		reg = <0x10020000 0x1000>;
+		interrupts = <0 44 4>;
+	};
+
+	dmc at 100e0000 {
+		compatible = "arm,pl341", "arm,primecell";
+		reg = <0x100e0000 0x1000>;
+	};
+
+	smc at 100e1000 {
+		compatible = "arm,pl354", "arm,primecell";
+		reg = <0x100e1000 0x1000>;
+		interrupts = <0 45 4>,
+			     <0 46 4>;
+	};
+
+	timer at 100e4000 {
+		compatible = "arm,sp804", "arm,primecell";
+		reg = <0x100e4000 0x1000>;
+		interrupts = <0 48 4>,
+			     <0 49 4>;
+	};
+
+	watchdog at 100e5000 {
+		compatible = "arm,sp805", "arm,primecell";
+		reg = <0x100e5000 0x1000>;
+		interrupts = <0 51 4>;
+	};
+
+	gic: interrupt-controller at 1e001000 {
+		compatible = "arm,cortex-a9-gic";
+		#interrupt-cells = <3>;
+		#address-cells = <0>;
+		interrupt-controller;
+		reg = <0x1e001000 0x1000>,
+		      <0x1e000100 0x100>;
+	};
+
+	L2: cache-controller at 1e00a000 {
+		compatible = "arm,pl310-cache";
+		reg = <0x1e00a000 0x1000>;
+		interrupts = <0 43 4>;
+		cache-level = <2>;
+		arm,data-latency = <0>;
+		arm,tag-latency = <0>;
+	};
+
+	pmu {
+		compatible = "arm,cortex-a9-pmu";
+		interrupts = <0 60 4>,
+			     <0 61 4>,
+			     <0 62 4>,
+			     <0 63 4>;
+	};
+
+	motherboard {
+		ranges = <0 0 0x40000000 0x04000000>,
+			 <1 0 0x44000000 0x04000000>,
+			 <2 0 0x48000000 0x04000000>,
+			 <3 0 0x4c000000 0x04000000>,
+			 <7 0 0x10000000 0x00020000>;
+
+		interrupt-map-mask = <0 0 63>;
+		interrupt-map = <0 0  0 &gic 0  0 4>,
+				<0 0  1 &gic 0  1 4>,
+				<0 0  2 &gic 0  2 4>,
+				<0 0  3 &gic 0  3 4>,
+				<0 0  4 &gic 0  4 4>,
+				<0 0  5 &gic 0  5 4>,
+				<0 0  6 &gic 0  6 4>,
+				<0 0  7 &gic 0  7 4>,
+				<0 0  8 &gic 0  8 4>,
+				<0 0  9 &gic 0  9 4>,
+				<0 0 10 &gic 0 10 4>,
+				<0 0 11 &gic 0 11 4>,
+				<0 0 12 &gic 0 12 4>,
+				<0 0 13 &gic 0 13 4>,
+				<0 0 14 &gic 0 14 4>,
+				<0 0 15 &gic 0 15 4>,
+				<0 0 16 &gic 0 16 4>,
+				<0 0 17 &gic 0 17 4>,
+				<0 0 18 &gic 0 18 4>,
+				<0 0 19 &gic 0 19 4>,
+				<0 0 20 &gic 0 20 4>,
+				<0 0 21 &gic 0 21 4>,
+				<0 0 22 &gic 0 22 4>,
+				<0 0 23 &gic 0 23 4>,
+				<0 0 24 &gic 0 24 4>,
+				<0 0 25 &gic 0 25 4>,
+				<0 0 26 &gic 0 26 4>,
+				<0 0 27 &gic 0 27 4>,
+				<0 0 28 &gic 0 28 4>,
+				<0 0 29 &gic 0 29 4>,
+				<0 0 30 &gic 0 30 4>,
+				<0 0 31 &gic 0 31 4>,
+				<0 0 32 &gic 0 32 4>,
+				<0 0 33 &gic 0 33 4>,
+				<0 0 34 &gic 0 34 4>,
+				<0 0 35 &gic 0 35 4>,
+				<0 0 36 &gic 0 36 4>,
+				<0 0 37 &gic 0 37 4>,
+				<0 0 38 &gic 0 38 4>,
+				<0 0 39 &gic 0 39 4>,
+				<0 0 40 &gic 0 40 4>,
+				<0 0 41 &gic 0 41 4>,
+				<0 0 42 &gic 0 42 4>;
+	};
+};
+
+/include/ "vexpress-v2m-legacy.dtsi"

arch/arm/mach-vexpress/Kconfig +0 -0

index d1b0f35..6b79400 100644

arch/arm/mach-vexpress/Kconfig +14 -0

+	bool "DT: CoreTiles Express A5x2 and A9x4"
+	select ARCH_VEXPRESS_RS1
+	select ARCH_VEXPRESS_DT
+	select ARM_ERRATA_720789
+	select ARM_ERRATA_751472
+	select ARM_ERRATA_753970
+	help
+	  This option enabled Device Tree based support for
+	  CoreTile Express A5x2 (V2P-CA5s) and
+	  CoreTile Express A9x4 (V2P-CA9).
+
+	  Note that you must provide kernel with a valid DTB
+	  for the board if you want to use this option.
+
 endmenu

arch/arm/mach-vexpress/Makefile +1 -0

index 90551b9..06e3687 100644
@@ -4,5 +4,6 @@
 
 obj-y					:= v2m.o
 obj-$(CONFIG_ARCH_VEXPRESS_CA9X4)	+= ct-ca9x4.o
+obj-$(CONFIG_ARCH_VEXPRESS_V2P_CA5S_CA9) += v2p-ca5s_ca9.o
 obj-$(CONFIG_SMP)			+= platsmp.o
 obj-$(CONFIG_HOTPLUG_CPU)		+= hotplug.o

arch/arm/mach-vexpress/v2p-ca5s_ca9.c new file +103 -0

new file mode 100644
index 0000000..0f1cd9b
@@ -0,0 +1,103 @@
+/*
+ * Device Tree based support for ARM Versatile Express platform using:
+ * - CoreTile Express A5x2 (V2P-CA5s)
+ * - CoreTile Express A9x4 (V2P-CA9)
+ */
+
+#include <linux/init.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+
+#include <asm/smp_scu.h>
+#include <asm/smp_twd.h>
+#include <asm/hardware/cache-l2x0.h>
+#include <asm/hardware/gic.h>
+#include <asm/mach/arch.h>
+#include <asm/mach/map.h>
+
+#include "core.h"
+
+#define A5_MPCORE_SCU		0x0000
+#define A5_MPCORE_TWD		0x0600
+
+static struct map_desc v2p_ca5s_ca9_io_desc[] __initdata = {
+	{
+		.virtual	= V2T_PERIPH,
+		/* .pfn	set in v2p_ca5s_ca9_map_io() */
+		.length		= SZ_8K,
+		.type		= MT_DEVICE,
+	},
+};
+
+#ifdef CONFIG_SMP
+static void v2p_ca5s_ca9_init_cpu_map(void)
+{
+	int i, ncores = scu_get_core_count(V2T_PERIPH_P2V(A5_MPCORE_SCU));
+
+	for (i = 0; i < ncores; ++i)
+		set_cpu_possible(i, true);
+
+	set_smp_cross_call(gic_raise_softirq);
+}
+
+static void v2p_ca5s_ca9_smp_enable(unsigned int max_cpus)
+{
+	scu_enable(V2T_PERIPH_P2V(A5_MPCORE_SCU));
+}
+#endif
+
+static void __init v2p_ca5s_ca9_map_io(void)
+{
+	u32 mpcore_periph;
+
+	asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (mpcore_periph));
+	v2p_ca5s_ca9_io_desc[0].pfn = __phys_to_pfn(mpcore_periph);
+	iotable_init(v2p_ca5s_ca9_io_desc, ARRAY_SIZE(v2p_ca5s_ca9_io_desc));
+
+	v2m_dt_map_io();
+
+#ifdef CONFIG_SMP
+	vexpress_init_cpu_map = v2p_ca5s_ca9_init_cpu_map;
+	vexpress_smp_enable = v2p_ca5s_ca9_smp_enable;
+#endif
+}
+
+static void __init v2p_ca5s_ca9_init_early(void)
+{
+#ifdef CONFIG_LOCAL_TIMERS
+	twd_base = V2T_PERIPH_P2V(A5_MPCORE_TWD);
+#endif
+	v2m_dt_init_early();
+}
+
+const static struct of_device_id v2p_ca5s_ca9_irq_match[] = {
+	{ .compatible = "arm,cortex-a9-gic", .data = gic_of_init, },
+	{}
+};
+
+static void __init v2p_ca5s_ca9_init_irq(void)
+{
+	of_irq_init(v2p_ca5s_ca9_irq_match);
+}
+
+static void __init v2p_ca5s_ca9_init(void)
+{
+	l2x0_of_init(0x00400000, 0xfe0fffff);
+	of_platform_populate(NULL, of_default_bus_match_table,
+			v2m_dt_get_auxdata(), NULL);
+}
+
+static const char *v2p_ca5s_ca9_dt_match[] __initdata = {
+	"arm,vexpress-v2p-ca5s",
+	"arm,vexpress-v2p-ca9",
+	NULL,
+};
+
+DT_MACHINE_START(VEXPRESS_V2P_CA5S_CA9, "ARM Versatile Express")
+	.map_io		= v2p_ca5s_ca9_map_io,
+	.init_early	= v2p_ca5s_ca9_init_early,
+	.init_irq	= v2p_ca5s_ca9_init_irq,
+	.timer		= &v2m_timer,
+	.init_machine	= v2p_ca5s_ca9_init,
+	.dt_compat	= v2p_ca5s_ca9_dt_match,
+MACHINE_END

Quoted 1 line

+ dmc at 2a150000 {

I missed this earlier, but the preferred generic name is "memory-controller"

Quoted 1 line

+ smc at 2a190000 {

same here.

Quoted 1 line

+config ARCH_VEXPRESS_V2P_CA5S_CA9

You don't expect this to expand to A7 or A15?

This patch adds Device Trees for ARM Ltd. CoreTile Express A5x2 and CoreTile Express A9x4 used with V2M motherboard and an initial implementation of the DT machine support (this code is separate from the current core tile code).

Signed-off-by Pawel Moll <[email protected]>

---
 arch/arm/boot/dts/vexpress-v2p-ca5s.dts |  132 ++++++++++++++++++++++++++++
 arch/arm/boot/dts/vexpress-v2p-ca9.dts  |  146 +++++++++++++++++++++++++++++++
 arch/arm/mach-vexpress/Kconfig          |   15 +++
 arch/arm/mach-vexpress/Makefile         |    1 +
 arch/arm/mach-vexpress/v2p-ca5s_ca9.c   |  103 ++++++++++++++++++++++
 5 files changed, 397 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/boot/dts/vexpress-v2p-ca5s.dts
 create mode 100644 arch/arm/boot/dts/vexpress-v2p-ca9.dts
 create mode 100644 arch/arm/mach-vexpress/v2p-ca5s_ca9.c

arch/arm/boot/dts/vexpress-v2p-ca5s.dts +0 -0

arch/arm/mach-vexpress/Kconfig +1 -0

+
 endmenu

arch/arm/mach-vexpress/Makefile +1 -0

index 90551b9..06e3687 100644
@@ -4,5 +4,6 @@
 
 obj-y					:= v2m.o
 obj-$(CONFIG_ARCH_VEXPRESS_CA9X4)	+= ct-ca9x4.o
+obj-$(CONFIG_ARCH_VEXPRESS_V2P_CA5S_CA9) += v2p-ca5s_ca9.o
 obj-$(CONFIG_SMP)			+= platsmp.o
 obj-$(CONFIG_HOTPLUG_CPU)		+= hotplug.o

arch/arm/mach-vexpress/v2p-ca5s_ca9.c +0 -0

+DT_MACHINE_START(VEXPRESS_V2P_CA5S_CA9, "ARM Versatile Express")

arch/arm/boot/dts/vexpress-v2p-ca5s.dts line 16

Quoted 5 lines

arch/arm/boot/dts/vexpress-v2p-ca5s.dts new file +5 -0

+/include/ "skeleton.dtsi"
+
+/ {
+	model = "V2P-CA5s";
+	arm,hbi = <0x225>;

Are these part numbers formally hex numbers? Is so, fine. Otherwise maybe it would me safer to make properties of this kind strings.

arch/arm/mach-vexpress/Kconfig config ARCH_VEXPRESS_DT · line 26

Quoted 2 lines

arch/arm/mach-vexpress/Kconfig +2 -0

+config ARCH_VEXPRESS_V2P_CA5S_CA9
+	bool "DT: CoreTiles Express A5x2 and A9x4"

This "DT" stuff is a bit cryptic -- also it's an implementation feature rather than part of the core nature of this config option.

Also, pedaitically, the product name is "CoreTile Express".

Finally, this option does not support those core tiles, as such; rather it supports particular whole-platform configurations involving those core tiles.

How about "CoreTile Express A5x2 and A9x4 based platform support"

arch/arm/mach-vexpress/Kconfig config ARCH_VEXPRESS_DT · line 38

Quoted 12 lines

arch/arm/mach-vexpress/Kconfig +12 -0

+	select ARCH_VEXPRESS_RS1
+	select ARCH_VEXPRESS_DT
+	select ARM_ERRATA_720789
+	select ARM_ERRATA_751472
+	select ARM_ERRATA_753970
+	help
+	  This option enabled Device Tree based support for
+	  CoreTile Express A5x2 (V2P-CA5s) and
+	  CoreTile Express A9x4 (V2P-CA9).
+
+	  Note that you must provide kernel with a valid DTB
+	  for the board if you want to use this option.

For the above reasons, the following alternative text might be a bit more user-friendly:

help
This option enables support for systems using any of the the following ARM core tiles on the Versatile Express motherboard:

CoreTile Express A5x2 (V2P-CA5s)
CoreTile Express A9x4 (V2P-CA9)

You must boot using a Flattened Device Tree in order to use these platforms. The traditional (ATAGs) boot method is not usable on these boards with this option.

If you want your kernel to run on one of these platforms and your bootloader supports Flattened Device Tree based booting, say Y.

Do we still support the "old style" V2P-CA9x4 support? If so, we could add a comment explaining that is available as an alternative, for now.

Quoted 13 lines

endmenu

arch/arm/mach-vexpress/Makefile +1 -0

index 90551b9..06e3687 100644
@@ -4,5 +4,6 @@
 
 obj-y					:= v2m.o
 obj-$(CONFIG_ARCH_VEXPRESS_CA9X4)	+= ct-ca9x4.o
+obj-$(CONFIG_ARCH_VEXPRESS_V2P_CA5S_CA9) += v2p-ca5s_ca9.o
 obj-$(CONFIG_SMP)			+= platsmp.o
 obj-$(CONFIG_HOTPLUG_CPU)		+= hotplug.o

arch/arm/mach-vexpress/v2p-ca5s_ca9.c +0 -0

[...]

Quoted 1 line

+DT_MACHINE_START(VEXPRESS_V2P_CA5S_CA9, "ARM Versatile Express")

We could try to name the platform fully here, but these strings would get very long -- so I agree it best to leave that to be specified in the device tree.

Quoted 8 lines · Dave Martin

arch/arm/boot/dts/vexpress-v2p-ca5s.dts line 16

Quoted 5 lines

arch/arm/boot/dts/vexpress-v2p-ca5s.dts new file +5 -0

+/include/ "skeleton.dtsi"
+
+/ {
+	model = "V2P-CA5s";
+	arm,hbi = <0x225>;

Are these part numbers formally hex numbers? Is so, fine. Otherwise maybe it would me safer to make properties of this kind strings.

Hex values. See
http://infocenter.arm.com/help/topic/com.arm.doc.dui0447e/CACBEHGJ.html

Quoted 5 lines

arch/arm/mach-vexpress/Kconfig config ARCH_VEXPRESS_DT · line 26

Quoted 2 lines

arch/arm/mach-vexpress/Kconfig +2 -0

+config ARCH_VEXPRESS_V2P_CA5S_CA9
+	bool "DT: CoreTiles Express A5x2 and A9x4"

This "DT" stuff is a bit cryptic -- also it's an implementation feature rather than part of the core nature of this config option.

Yeah, I know. I just didn't feel very verbose doing the Kconfig changes.

Quoted 2 lines

Also, pedaitically, the product name is "CoreTile Express".

Ha ha - I wanted to express the plurality of A5 _and_ a9 ;-) Never mind, will change it.

Quoted 5 lines

Finally, this option does not support those core tiles, as such; rather it supports particular whole-platform configurations involving those core tiles.

How about "CoreTile Express A5x2 and A9x4 based platform support"

Fine with me - I'll use your wording :-)

Quoted 23 lines

arch/arm/mach-vexpress/Kconfig config ARCH_VEXPRESS_DT · line 38

Quoted 12 lines

arch/arm/mach-vexpress/Kconfig +12 -0

+	select ARCH_VEXPRESS_RS1
+	select ARCH_VEXPRESS_DT
+	select ARM_ERRATA_720789
+	select ARM_ERRATA_751472
+	select ARM_ERRATA_753970
+	help
+	  This option enabled Device Tree based support for
+	  CoreTile Express A5x2 (V2P-CA5s) and
+	  CoreTile Express A9x4 (V2P-CA9).
+
+	  Note that you must provide kernel with a valid DTB
+	  for the board if you want to use this option.

For the above reasons, the following alternative text might be a bit more user-friendly:

help
This option enables support for systems using any of the the following ARM core tiles on the Versatile Express motherboard:

CoreTile Express A5x2 (V2P-CA5s)
CoreTile Express A9x4 (V2P-CA9)

Quoted 6 lines

You must boot using a Flattened Device Tree in order to use these platforms. The traditional (ATAGs) boot method is not usable on these boards with this option.

If you want your kernel to run on one of these platforms and your bootloader supports Flattened Device Tree based booting, say Y.

Same here - will copy this :-)

Quoted 2 lines

Do we still support the "old style" V2P-CA9x4 support? If so, we could add a comment explaining that is available as an alternative, for now.

We do, with the non-DT option (Russell wouldn't be very happy if his serial port multiplexer stopped working ;-)

Quoted 16 lines

Quoted 13 lines

endmenu

arch/arm/mach-vexpress/Makefile +1 -0

index 90551b9..06e3687 100644
@@ -4,5 +4,6 @@
 
 obj-y					:= v2m.o
 obj-$(CONFIG_ARCH_VEXPRESS_CA9X4)	+= ct-ca9x4.o
+obj-$(CONFIG_ARCH_VEXPRESS_V2P_CA5S_CA9) += v2p-ca5s_ca9.o
 obj-$(CONFIG_SMP)			+= platsmp.o
 obj-$(CONFIG_HOTPLUG_CPU)		+= hotplug.o

arch/arm/mach-vexpress/v2p-ca5s_ca9.c +0 -0

[...]

Quoted 5 lines

Quoted 1 line

+DT_MACHINE_START(VEXPRESS_V2P_CA5S_CA9, "ARM Versatile Express")

We could try to name the platform fully here, but these strings would get very long -- so I agree it best to leave that to be specified in the device tree.

Having such machine name here and model in the dts gives such a nice message in the log:

Machine: ARM Versatile Express, model: V2P-CA9

Which is sort-of-correct.

Hello again,

I've manage to clean up the VE DT patches.

Changes since RFC:

* I think I did all the changes suggested by Rob (including using "custom" clock lookups instead of auxdata - do comment, please ;-)

* Merged CA5s and CA9 code into one file.

* Added documentation for the vexpress bindings.

* Reorganized the tree so the "flat aliases search" patch is not longer necessary.

* Moved the alias node from motherboard's to tile's DTS, as this is the place to decide about serial port ordering, timers being used etc.

* Got the SMSC working (my fault, obviously ;-)

As usual - feedback more than welcome (my tests suggest that the "ATAG version" still works, so feel free to test at least that ;-)