[PATCH 0/5] Versatile Express DT support, take 2

[PATCH 0/5] Versatile Express DT support, take 2

[Pawel Moll]

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

Cheers!

Pawel



Pawel Moll (5):
  ARM: vexpress: Get rid of MMIO_P2V
  ARM: vexpress: Remove platform SMP functions from ct_desc
  ARM: vexpress: Add DT support in v2m
  ARM: vexpress: Initial RS1 memory map support
  ARM: vexpress: DT-based support for CoreTiles Express A5x2 and A9x4

 Documentation/devicetree/bindings/arm/vexpress    |   92 ++++++++
 arch/arm/boot/dts/vexpress-v2m-legacy.dtsi        |  190 ++++++++++++++++
 arch/arm/boot/dts/vexpress-v2m-rs1.dtsi           |  190 ++++++++++++++++
 arch/arm/boot/dts/vexpress-v2p-ca5s.dts           |  132 +++++++++++
 arch/arm/boot/dts/vexpress-v2p-ca9.dts            |  146 ++++++++++++
 arch/arm/include/asm/hardware/arm_timer.h         |    5 +
 arch/arm/mach-vexpress/Kconfig                    |   28 +++
 arch/arm/mach-vexpress/Makefile                   |    1 +
 arch/arm/mach-vexpress/Makefile.boot              |    6 +-
 arch/arm/mach-vexpress/core.h                     |   27 ++-
 arch/arm/mach-vexpress/ct-ca9x4.c                 |   96 +++------
 arch/arm/mach-vexpress/include/mach/ct-ca9x4.h    |   13 +-
 arch/arm/mach-vexpress/include/mach/debug-macro.S |   37 +++-
 arch/arm/mach-vexpress/include/mach/motherboard.h |   65 +++---
 arch/arm/mach-vexpress/include/mach/uncompress.h  |   13 +-
 arch/arm/mach-vexpress/platsmp.c                  |   11 +-
 arch/arm/mach-vexpress/v2m.c                      |  250 +++++++++++++++++++--
 arch/arm/mach-vexpress/v2p-ca5s_ca9.c             |  103 +++++++++
 18 files changed, 1263 insertions(+), 142 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/vexpress
 create mode 100644 arch/arm/boot/dts/vexpress-v2m-legacy.dtsi
 create mode 100644 arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
 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

[PATCH 1/5] ARM: vexpress: Get rid of MMIO_P2V

[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 <pawel.moll@arm.com>
---
 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(-)

diff --git a/arch/arm/include/asm/hardware/arm_timer.h b/arch/arm/include/asm/hardware/arm_timer.h
index c0f4e7b..d6030ff 100644
--- a/arch/arm/include/asm/hardware/arm_timer.h
+++ b/arch/arm/include/asm/hardware/arm_timer.h
@@ -9,7 +9,12 @@
  *
  * Integrator AP has 16-bit timers, Integrator CP, Versatile and Realview
  * can have 16-bit or 32-bit selectable via a bit in the control register.
+ *
+ * Every SP804 contains two identical timers.
  */
+#define TIMER_1_BASE	0x00
+#define TIMER_2_BASE	0x20
+
 #define TIMER_LOAD	0x00			/* ACVR rw */
 #define TIMER_VALUE	0x04			/* ACVR ro */
 #define TIMER_CTRL	0x08			/* ACVR rw */
diff --git a/arch/arm/mach-vexpress/core.h b/arch/arm/mach-vexpress/core.h
index f439715..27a622b 100644
--- a/arch/arm/mach-vexpress/core.h
+++ b/arch/arm/mach-vexpress/core.h
@@ -1,6 +1,3 @@
-#define __MMIO_P2V(x)	(((x) & 0xfffff) | (((x) & 0x0f000000) >> 4) | 0xf8000000)
-#define MMIO_P2V(x)	((void __iomem *)__MMIO_P2V(x))
-
 #define AMBA_DEVICE(name,busid,base,plat)	\
 struct amba_device name##_device = {		\
 	.dev		= {			\
@@ -17,3 +14,12 @@ struct amba_device name##_device = {		\
 	.irq		= IRQ_##base,		\
 	/* .dma		= DMA_##base,*/		\
 }
+
+/* 2MB large area for motherboard's peripherals static mapping */
+#define V2M_PERIPH 0xf8000000
+#define V2M_PERIPH_P2V(offset) ((void __iomem *)(V2M_PERIPH | (offset)))
+
+/* Tile's peripherals static mappings should start here */
+#define V2T_PERIPH 0xf8200000
+#define V2T_PERIPH_P2V(offset) ((void __iomem *)(V2T_PERIPH | (offset)))
+
diff --git a/arch/arm/mach-vexpress/ct-ca9x4.c b/arch/arm/mach-vexpress/ct-ca9x4.c
index 2b1e836..47c0733 100644
--- a/arch/arm/mach-vexpress/ct-ca9x4.c
+++ b/arch/arm/mach-vexpress/ct-ca9x4.c
@@ -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),
-		.length		= SZ_4K,
-		.type		= MT_DEVICE,
-	}, {
-		.virtual	= __MMIO_P2V(CT_CA9X4_L2CC),
-		.pfn		= __phys_to_pfn(CT_CA9X4_L2CC),
-		.length		= SZ_4K,
-		.type		= MT_DEVICE,
+		.virtual        = V2T_PERIPH,
+		.pfn            = __phys_to_pfn(CT_CA9X4_MPIC),
+		.length         = SZ_8K,
+		.type           = MT_DEVICE,
 	},
 };
 
 static void __init ct_ca9x4_map_io(void)
 {
-#ifdef CONFIG_LOCAL_TIMERS
-	twd_base = MMIO_P2V(A9_MPCORE_TWD);
-#endif
 	iotable_init(ct_ca9x4_io_desc, ARRAY_SIZE(ct_ca9x4_io_desc));
 }
 
 static void __init ct_ca9x4_init_irq(void)
 {
-	gic_init(0, 29, MMIO_P2V(A9_MPCORE_GIC_DIST),
-		 MMIO_P2V(A9_MPCORE_GIC_CPU));
-}
-
-#if 0
-static void __init ct_ca9x4_timer_init(void)
-{
-	writel(0, MMIO_P2V(CT_CA9X4_TIMER0) + TIMER_CTRL);
-	writel(0, MMIO_P2V(CT_CA9X4_TIMER1) + TIMER_CTRL);
-
-	sp804_clocksource_init(MMIO_P2V(CT_CA9X4_TIMER1), "ct-timer1");
-	sp804_clockevents_init(MMIO_P2V(CT_CA9X4_TIMER0), IRQ_CT_CA9X4_TIMER0,
-		"ct-timer0");
+	gic_init(0, 29, V2T_PERIPH_P2V(A9_MPCORE_GIC_DIST),
+		 V2T_PERIPH_P2V(A9_MPCORE_GIC_CPU));
 }
 
-static struct sys_timer ct_ca9x4_timer = {
-	.init	= ct_ca9x4_timer_init,
-};
-#endif
-
 static void ct_ca9x4_clcd_enable(struct clcd_fb *fb)
 {
 	v2m_cfg_write(SYS_CFG_MUXFPGA | SYS_CFG_SITE_DB1, 0);
@@ -193,6 +162,9 @@ static struct platform_device pmu_device = {
 
 static void __init ct_ca9x4_init_early(void)
 {
+#ifdef CONFIG_LOCAL_TIMERS
+	twd_base = V2T_PERIPH_P2V(A9_MPCORE_TWD);
+#endif
 	clkdev_add_table(lookups, ARRAY_SIZE(lookups));
 }
 
@@ -201,7 +173,7 @@ static void __init ct_ca9x4_init(void)
 	int i;
 
 #ifdef CONFIG_CACHE_L2X0
-	void __iomem *l2x0_base = MMIO_P2V(CT_CA9X4_L2CC);
+	void __iomem *l2x0_base = ioremap(CT_CA9X4_L2CC, SZ_4K);
 
 	/* set RAM latencies to 1 cycle for this core tile. */
 	writel(0, l2x0_base + L2X0_TAG_LATENCY_CTRL);
@@ -219,7 +191,7 @@ static void __init ct_ca9x4_init(void)
 #ifdef CONFIG_SMP
 static void ct_ca9x4_init_cpu_map(void)
 {
-	int i, ncores = scu_get_core_count(MMIO_P2V(A9_MPCORE_SCU));
+	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",
@@ -235,7 +207,7 @@ static void ct_ca9x4_init_cpu_map(void)
 
 static void ct_ca9x4_smp_enable(unsigned int max_cpus)
 {
-	scu_enable(MMIO_P2V(A9_MPCORE_SCU));
+	scu_enable(V2TILE_PERIPH_P2V(A9_MPCORE_SCU));
 }
 #endif
 
diff --git a/arch/arm/mach-vexpress/include/mach/ct-ca9x4.h b/arch/arm/mach-vexpress/include/mach/ct-ca9x4.h
index a34d3d4..8f962fb 100644
--- a/arch/arm/mach-vexpress/include/mach/ct-ca9x4.h
+++ b/arch/arm/mach-vexpress/include/mach/ct-ca9x4.h
@@ -22,14 +22,11 @@
 #define CT_CA9X4_SYSWDT		(0x1e007000)
 #define CT_CA9X4_L2CC		(0x1e00a000)
 
-#define CT_CA9X4_TIMER0		(CT_CA9X4_SP804_TIMER + 0x000)
-#define CT_CA9X4_TIMER1		(CT_CA9X4_SP804_TIMER + 0x020)
-
-#define A9_MPCORE_SCU		(CT_CA9X4_MPIC + 0x0000)
-#define A9_MPCORE_GIC_CPU	(CT_CA9X4_MPIC + 0x0100)
-#define A9_MPCORE_GIT		(CT_CA9X4_MPIC + 0x0200)
-#define A9_MPCORE_TWD		(CT_CA9X4_MPIC + 0x0600)
-#define A9_MPCORE_GIC_DIST	(CT_CA9X4_MPIC + 0x1000)
+#define A9_MPCORE_SCU		0x0000
+#define A9_MPCORE_GIC_CPU	0x0100
+#define A9_MPCORE_GIT		0x0200
+#define A9_MPCORE_TWD		0x0600
+#define A9_MPCORE_GIC_DIST	0x1000
 
 /*
  * Interrupts.  Those in {} are for AMBA devices
diff --git a/arch/arm/mach-vexpress/include/mach/motherboard.h b/arch/arm/mach-vexpress/include/mach/motherboard.h
index 0a3a375..da9ac29 100644
--- a/arch/arm/mach-vexpress/include/mach/motherboard.h
+++ b/arch/arm/mach-vexpress/include/mach/motherboard.h
@@ -39,34 +39,30 @@
 #define V2M_CF			(V2M_PA_CS7 + 0x0001a000)
 #define V2M_CLCD		(V2M_PA_CS7 + 0x0001f000)
 
-#define V2M_SYS_ID		(V2M_SYSREGS + 0x000)
-#define V2M_SYS_SW		(V2M_SYSREGS + 0x004)
-#define V2M_SYS_LED		(V2M_SYSREGS + 0x008)
-#define V2M_SYS_100HZ		(V2M_SYSREGS + 0x024)
-#define V2M_SYS_FLAGS		(V2M_SYSREGS + 0x030)
-#define V2M_SYS_FLAGSSET	(V2M_SYSREGS + 0x030)
-#define V2M_SYS_FLAGSCLR	(V2M_SYSREGS + 0x034)
-#define V2M_SYS_NVFLAGS		(V2M_SYSREGS + 0x038)
-#define V2M_SYS_NVFLAGSSET	(V2M_SYSREGS + 0x038)
-#define V2M_SYS_NVFLAGSCLR	(V2M_SYSREGS + 0x03c)
-#define V2M_SYS_MCI		(V2M_SYSREGS + 0x048)
-#define V2M_SYS_FLASH		(V2M_SYSREGS + 0x03c)
-#define V2M_SYS_CFGSW		(V2M_SYSREGS + 0x058)
-#define V2M_SYS_24MHZ		(V2M_SYSREGS + 0x05c)
-#define V2M_SYS_MISC		(V2M_SYSREGS + 0x060)
-#define V2M_SYS_DMA		(V2M_SYSREGS + 0x064)
-#define V2M_SYS_PROCID0		(V2M_SYSREGS + 0x084)
-#define V2M_SYS_PROCID1		(V2M_SYSREGS + 0x088)
-#define V2M_SYS_CFGDATA		(V2M_SYSREGS + 0x0a0)
-#define V2M_SYS_CFGCTRL		(V2M_SYSREGS + 0x0a4)
-#define V2M_SYS_CFGSTAT		(V2M_SYSREGS + 0x0a8)
-
-#define V2M_TIMER0		(V2M_TIMER01 + 0x000)
-#define V2M_TIMER1		(V2M_TIMER01 + 0x020)
-
-#define V2M_TIMER2		(V2M_TIMER23 + 0x000)
-#define V2M_TIMER3		(V2M_TIMER23 + 0x020)
-
+/*
+ * Offsets from SYSREGS base
+ */
+#define V2M_SYS_ID		0x000
+#define V2M_SYS_SW		0x004
+#define V2M_SYS_LED		0x008
+#define V2M_SYS_100HZ		0x024
+#define V2M_SYS_FLAGS		0x030
+#define V2M_SYS_FLAGSSET	0x030
+#define V2M_SYS_FLAGSCLR	0x034
+#define V2M_SYS_NVFLAGS		0x038
+#define V2M_SYS_NVFLAGSSET	0x038
+#define V2M_SYS_NVFLAGSCLR	0x03c
+#define V2M_SYS_MCI		0x048
+#define V2M_SYS_FLASH		0x03c
+#define V2M_SYS_CFGSW		0x058
+#define V2M_SYS_24MHZ		0x05c
+#define V2M_SYS_MISC		0x060
+#define V2M_SYS_DMA		0x064
+#define V2M_SYS_PROCID0		0x084
+#define V2M_SYS_PROCID1		0x088
+#define V2M_SYS_CFGDATA		0x0a0
+#define V2M_SYS_CFGCTRL		0x0a4
+#define V2M_SYS_CFGSTAT		0x0a8
 
 /*
  * Interrupts.  Those in {} are for AMBA devices
@@ -117,6 +113,7 @@
 
 int v2m_cfg_write(u32 devfn, u32 data);
 int v2m_cfg_read(u32 devfn, u32 *data);
+void v2m_flags_set(u32 data);
 
 /*
  * Core tile IDs
diff --git a/arch/arm/mach-vexpress/platsmp.c b/arch/arm/mach-vexpress/platsmp.c
index 2b5f7ac..e8be99d 100644
--- a/arch/arm/mach-vexpress/platsmp.c
+++ b/arch/arm/mach-vexpress/platsmp.c
@@ -45,7 +45,5 @@ void __init platform_smp_prepare_cpus(unsigned int max_cpus)
 	 * until it receives a soft interrupt, and then the
 	 * secondary CPU branches to this address.
 	 */
-	writel(~0, MMIO_P2V(V2M_SYS_FLAGSCLR));
-	writel(BSYM(virt_to_phys(versatile_secondary_startup)),
-		MMIO_P2V(V2M_SYS_FLAGSSET));
+	v2m_flags_set(BSYM(virt_to_phys(versatile_secondary_startup)));
 }
diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
index 1fafc32..b84fa45 100644
--- a/arch/arm/mach-vexpress/v2m.c
+++ b/arch/arm/mach-vexpress/v2m.c
@@ -39,29 +39,41 @@
 
 static struct map_desc v2m_io_desc[] __initdata = {
 	{
-		.virtual	= __MMIO_P2V(V2M_PA_CS7),
+		.virtual	= V2M_PERIPH,
 		.pfn		= __phys_to_pfn(V2M_PA_CS7),
 		.length		= SZ_128K,
 		.type		= MT_DEVICE,
 	},
 };
 
+static void __iomem *v2m_sysreg_base;
+
+
+
 static void __init v2m_timer_init(void)
 {
+	void *sysctl_base;
+	void *timer01_base;
+	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;
 	/* Select 1MHz TIMCLK as the reference clock for SP804 timers */
-	scctrl = readl(MMIO_P2V(V2M_SYSCTL + SCCTRL));
+	scctrl = readl(sysctl_base + SCCTRL);
 	scctrl |= SCCTRL_TIMEREN0SEL_TIMCLK;
 	scctrl |= SCCTRL_TIMEREN1SEL_TIMCLK;
-	writel(scctrl, MMIO_P2V(V2M_SYSCTL + SCCTRL));
+	writel(scctrl, sysctl_base + SCCTRL);
 
-	writel(0, MMIO_P2V(V2M_TIMER0) + TIMER_CTRL);
-	writel(0, MMIO_P2V(V2M_TIMER1) + TIMER_CTRL);
+	writel(0, timer01_base + TIMER_1_BASE + TIMER_CTRL);
+	writel(0, timer01_base + TIMER_2_BASE + TIMER_CTRL);
 
-	sp804_clocksource_init(MMIO_P2V(V2M_TIMER1), "v2m-timer1");
-	sp804_clockevents_init(MMIO_P2V(V2M_TIMER0), IRQ_V2M_TIMER0,
-		"v2m-timer0");
+	sp804_clocksource_init(timer01_base + TIMER_2_BASE, "v2m-timer1");
+	sp804_clockevents_init(timer01_base + TIMER_1_BASE, timer01_irq,
+			"v2m-timer0");
 }
 
 static struct sys_timer v2m_timer = {
@@ -81,14 +93,14 @@ int v2m_cfg_write(u32 devfn, u32 data)
 	devfn |= SYS_CFG_START | SYS_CFG_WRITE;
 
 	spin_lock(&v2m_cfg_lock);
-	val = readl(MMIO_P2V(V2M_SYS_CFGSTAT));
-	writel(val & ~SYS_CFG_COMPLETE, MMIO_P2V(V2M_SYS_CFGSTAT));
+	val = readl(v2m_sysreg_base + V2M_SYS_CFGSTAT);
+	writel(val & ~SYS_CFG_COMPLETE, v2m_sysreg_base + V2M_SYS_CFGSTAT);
 
-	writel(data, MMIO_P2V(V2M_SYS_CFGDATA));
-	writel(devfn, MMIO_P2V(V2M_SYS_CFGCTRL));
+	writel(data, v2m_sysreg_base +  V2M_SYS_CFGDATA);
+	writel(devfn, v2m_sysreg_base + V2M_SYS_CFGCTRL);
 
 	do {
-		val = readl(MMIO_P2V(V2M_SYS_CFGSTAT));
+		val = readl(v2m_sysreg_base + V2M_SYS_CFGSTAT);
 	} while (val == 0);
 	spin_unlock(&v2m_cfg_lock);
 
@@ -102,22 +114,27 @@ int v2m_cfg_read(u32 devfn, u32 *data)
 	devfn |= SYS_CFG_START;
 
 	spin_lock(&v2m_cfg_lock);
-	writel(0, MMIO_P2V(V2M_SYS_CFGSTAT));
-	writel(devfn, MMIO_P2V(V2M_SYS_CFGCTRL));
+	writel(0, v2m_sysreg_base + V2M_SYS_CFGSTAT);
+	writel(devfn, v2m_sysreg_base + V2M_SYS_CFGCTRL);
 
 	mb();
 
 	do {
 		cpu_relax();
-		val = readl(MMIO_P2V(V2M_SYS_CFGSTAT));
+		val = readl(v2m_sysreg_base + V2M_SYS_CFGSTAT);
 	} while (val == 0);
 
-	*data = readl(MMIO_P2V(V2M_SYS_CFGDATA));
+	*data = readl(v2m_sysreg_base + V2M_SYS_CFGDATA);
 	spin_unlock(&v2m_cfg_lock);
 
 	return !!(val & SYS_CFG_ERR);
 }
 
+void __init v2m_flags_set(u32 data)
+{
+	writel(~0, v2m_sysreg_base + V2M_SYS_FLAGSCLR);
+	writel(data, v2m_sysreg_base + V2M_SYS_FLAGSSET);
+}
 
 static struct resource v2m_pcie_i2c_resource = {
 	.start	= V2M_SERIAL_BUS_PCI,
@@ -203,7 +220,7 @@ static struct platform_device v2m_usb_device = {
 
 static void v2m_flash_set_vpp(struct platform_device *pdev, int on)
 {
-	writel(on != 0, MMIO_P2V(V2M_SYS_FLASH));
+	writel(on != 0, v2m_sysreg_base + V2M_SYS_FLASH);
 }
 
 static struct physmap_flash_data v2m_flash_data = {
@@ -257,7 +274,7 @@ static struct platform_device v2m_cf_device = {
 
 static unsigned int v2m_mmci_status(struct device *dev)
 {
-	return readl(MMIO_P2V(V2M_SYS_MCI)) & (1 << 0);
+	return readl(v2m_sysreg_base + V2M_SYS_MCI) & (1 << 0);
 }
 
 static struct mmci_platform_data v2m_mmci_data = {
@@ -370,7 +387,7 @@ static void __init v2m_init_early(void)
 {
 	ct_desc->init_early();
 	clkdev_add_table(v2m_lookups, ARRAY_SIZE(v2m_lookups));
-	versatile_sched_clock_init(MMIO_P2V(V2M_SYS_24MHZ), 24000000);
+	versatile_sched_clock_init(v2m_sysreg_base + V2M_SYS_24MHZ, 24000000);
 }
 
 static void v2m_power_off(void)
@@ -399,7 +416,8 @@ static void __init v2m_populate_ct_desc(void)
 	u32 current_tile_id;
 
 	ct_desc = NULL;
-	current_tile_id = readl(MMIO_P2V(V2M_SYS_PROCID0)) & V2M_CT_ID_MASK;
+	current_tile_id = readl(v2m_sysreg_base + V2M_SYS_PROCID0)
+				& V2M_CT_ID_MASK;
 
 	for (i = 0; i < ARRAY_SIZE(ct_descs) && !ct_desc; ++i)
 		if (ct_descs[i]->id == current_tile_id)
@@ -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);
+
 	v2m_populate_ct_desc();
 	ct_desc->map_io();
 }
-- 
1.6.3.3

Re: [PATCH 1/5] ARM: vexpress: Get rid of MMIO_P2V

[Dave Martin]

On Fri, Nov 11, 2011 at 06:27:02PM +0000, Pawel Moll wrote:
> 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 <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> ---
>  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(-)

[...]

> diff --git a/arch/arm/mach-vexpress/ct-ca9x4.c b/arch/arm/mach-vexpress/ct-ca9x4.c
> index 2b1e836..47c0733 100644
> --- a/arch/arm/mach-vexpress/ct-ca9x4.c
> +++ b/arch/arm/mach-vexpress/ct-ca9x4.c
> @@ -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),
> -		.length		= SZ_4K,
> -		.type		= MT_DEVICE,
> -	}, {
> -		.virtual	= __MMIO_P2V(CT_CA9X4_L2CC),
> -		.pfn		= __phys_to_pfn(CT_CA9X4_L2CC),
> -		.length		= SZ_4K,
> -		.type		= MT_DEVICE,
> +		.virtual        = V2T_PERIPH,
> +		.pfn            = __phys_to_pfn(CT_CA9X4_MPIC),
> +		.length         = SZ_8K,

Can you explain the size change?

[...]

> diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
> index 1fafc32..b84fa45 100644
> --- a/arch/arm/mach-vexpress/v2m.c
> +++ b/arch/arm/mach-vexpress/v2m.c

[...]

> @@ -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.)


Cheers
---Dave

Re: [PATCH 1/5] ARM: vexpress: Get rid of MMIO_P2V

[Pawel Moll]

On Wed, 2011-11-16 at 15:35 +0000, Dave Martin wrote:
> On Fri, Nov 11, 2011 at 06:27:02PM +0000, Pawel Moll wrote:
> > 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 <pawel.moll@arm.com>
> > ---
> >  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(-)
> 
> [...]
> 
> > diff --git a/arch/arm/mach-vexpress/ct-ca9x4.c b/arch/arm/mach-vexpress/ct-ca9x4.c
> > index 2b1e836..47c0733 100644
> > --- a/arch/arm/mach-vexpress/ct-ca9x4.c
> > +++ b/arch/arm/mach-vexpress/ct-ca9x4.c
> > @@ -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        = 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

> > diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
> > index 1fafc32..b84fa45 100644
> > --- a/arch/arm/mach-vexpress/v2m.c
> > +++ b/arch/arm/mach-vexpress/v2m.c
> 
> [...]
> 
> > @@ -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.

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

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

Paweł



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/5] ARM: vexpress: Get rid of MMIO_P2V

[Dave Martin]

On Wed, Nov 16, 2011 at 04:16:45PM +0000, Pawel Moll wrote:
> On Wed, 2011-11-16 at 15:35 +0000, Dave Martin wrote:
> > On Fri, Nov 11, 2011 at 06:27:02PM +0000, Pawel Moll wrote:
> > > 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 <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> > > ---
> > >  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(-)
> > 
> > [...]
> > 
> > > diff --git a/arch/arm/mach-vexpress/ct-ca9x4.c b/arch/arm/mach-vexpress/ct-ca9x4.c
> > > index 2b1e836..47c0733 100644
> > > --- a/arch/arm/mach-vexpress/ct-ca9x4.c
> > > +++ b/arch/arm/mach-vexpress/ct-ca9x4.c
> > > @@ -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        = 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

OK, so that's just tidyup of an oversized mapping.  Fair enough.

> 
> > > diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
> > > index 1fafc32..b84fa45 100644
> > > --- a/arch/arm/mach-vexpress/v2m.c
> > > +++ b/arch/arm/mach-vexpress/v2m.c
> > 
> > [...]
> > 
> > > @@ -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.
> 
> What do you exaclty mean? This function is non-DT related. It's not used
> by DT platform at all...

I failed to notice that-- if this function isn't used at all for the DT case,
then I guess this isn't important.

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

Cheers
---Dave

Re: [PATCH 1/5] ARM: vexpress: Get rid of MMIO_P2V

[Pawel Moll]

On Wed, 2011-11-16 at 17:28 +0000, Dave Martin wrote:
> > > 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...

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

Cheers!

Paweł



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/5] ARM: vexpress: Get rid of MMIO_P2V

[Ryan Harkin]

On Wed, 2011-11-16 at 17:30 +0000, Pawel Moll wrote:
> On Wed, 2011-11-16 at 17:28 +0000, Dave Martin wrote:
> > > > 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...
> 
> Good idea. I'll send them instructions how to boot "DT-enabled" kernel
> using Boot Monitor tomorrow and ask to try the series.

I have both boards here waiting your command.

Re: [PATCH 1/5] ARM: vexpress: Get rid of MMIO_P2V

[Russell King - ARM Linux]

On Fri, Nov 11, 2011 at 06:27:02PM +0000, Pawel Moll wrote:
> @@ -17,3 +14,12 @@ struct amba_device name##_device = {		\
>  	.irq		= IRQ_##base,		\
>  	/* .dma		= DMA_##base,*/		\
>  }
> +
> +/* 2MB large area for motherboard's peripherals static mapping */
> +#define V2M_PERIPH 0xf8000000
> +#define V2M_PERIPH_P2V(offset) ((void __iomem *)(V2M_PERIPH | (offset)))
> +
> +/* 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.

> diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
> index 1fafc32..b84fa45 100644
> --- a/arch/arm/mach-vexpress/v2m.c
> +++ b/arch/arm/mach-vexpress/v2m.c
> @@ -39,29 +39,41 @@
>  
>  static struct map_desc v2m_io_desc[] __initdata = {
>  	{
> -		.virtual	= __MMIO_P2V(V2M_PA_CS7),
> +		.virtual	= V2M_PERIPH,
>  		.pfn		= __phys_to_pfn(V2M_PA_CS7),
>  		.length		= SZ_128K,
>  		.type		= MT_DEVICE,
>  	},
>  };
>  
> +static void __iomem *v2m_sysreg_base;
> +
> +
> +

More useless blank lines.

>  static void __init v2m_timer_init(void)
>  {
> +	void *sysctl_base;
> +	void *timer01_base;

Do you not use sparse?  __iomem.

> +	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?

> @@ -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.

Re: [PATCH 1/5] ARM: vexpress: Get rid of MMIO_P2V

[Pawel Moll]

On Thu, 2011-11-17 at 15:43 +0000, Russell King - ARM Linux wrote:
> > +/* 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.

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

> >  static void __init v2m_timer_init(void)
> >  {
> > +	void *sysctl_base;
> > +	void *timer01_base;
> 
> Do you not use sparse?  __iomem.

Ok.

> > +	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.

> > @@ -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?

All comments appreciated as always, thanks!

Pawel

Re: [PATCH 1/5] ARM: vexpress: Get rid of MMIO_P2V

[Russell King - ARM Linux]

On Fri, Nov 18, 2011 at 12:20:48PM +0000, Pawel Moll wrote:
> On Thu, 2011-11-17 at 15:43 +0000, Russell King - ARM Linux wrote:
> > > +/* 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.
> 
> > > +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)

Note the difference in the number.  Two is acceptable to split sections
of code.  Three is getting excessive.

> 
> > >  static void __init v2m_timer_init(void)
> > >  {
> > > +	void *sysctl_base;
> > > +	void *timer01_base;
> > 
> > Do you not use sparse?  __iomem.
> 
> Ok.
> 
> > > +	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.
> 
> > > @@ -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?

What I'm saying is your comment is wrong.  ioremap() will never be
possible in the map_io function.

[PATCH 2/5] ARM: vexpress: Remove platform SMP functions from ct_desc

[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.

This is a temporary measure till "SoC descriptors" code
gets into the main line.

Signed-off-by: Pawel Moll <pawel.moll@arm.com>
---
 arch/arm/mach-vexpress/core.h                     |    6 +++
 arch/arm/mach-vexpress/ct-ca9x4.c                 |   50 +++++++++-----------
 arch/arm/mach-vexpress/include/mach/motherboard.h |    4 --
 arch/arm/mach-vexpress/platsmp.c                  |    7 ++-
 4 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/arch/arm/mach-vexpress/core.h b/arch/arm/mach-vexpress/core.h
index 27a622b..2139b37 100644
--- a/arch/arm/mach-vexpress/core.h
+++ b/arch/arm/mach-vexpress/core.h
@@ -23,3 +23,9 @@ struct amba_device name##_device = {		\
 #define V2T_PERIPH 0xf8200000
 #define V2T_PERIPH_P2V(offset) ((void __iomem *)(V2T_PERIPH | (offset)))
 
+/* Will disappear once platform SMP calls are "abstracted" */
+#if defined(CONFIG_SMP)
+extern void (*vexpress_init_cpu_map)(void);
+extern void (*vexpress_smp_enable)(unsigned int);
+#endif
+
diff --git a/arch/arm/mach-vexpress/ct-ca9x4.c b/arch/arm/mach-vexpress/ct-ca9x4.c
index 47c0733..d6dc80a 100644
--- a/arch/arm/mach-vexpress/ct-ca9x4.c
+++ b/arch/arm/mach-vexpress/ct-ca9x4.c
@@ -39,9 +39,32 @@ static struct map_desc ct_ca9x4_io_desc[] __initdata = {
 	},
 };
 
+#ifdef CONFIG_SMP
+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);
+}
+
+static void ct_ca9x4_smp_enable(unsigned int max_cpus)
+{
+	scu_enable(V2T_PERIPH_P2V(A9_MPCORE_SCU));
+}
+#endif
+
 static void __init ct_ca9x4_map_io(void)
 {
 	iotable_init(ct_ca9x4_io_desc, ARRAY_SIZE(ct_ca9x4_io_desc));
+#ifdef CONFIG_SMP
+	/* Will be gone once the SoC descriptors are in */
+	vexpress_init_cpu_map = ct_ca9x4_init_cpu_map;
+	vexpress_smp_enable = ct_ca9x4_smp_enable;
+#endif
 }
 
 static void __init ct_ca9x4_init_irq(void)
@@ -188,29 +211,6 @@ static void __init ct_ca9x4_init(void)
 	platform_device_register(&pmu_device);
 }
 
-#ifdef CONFIG_SMP
-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;
-	}
-
-	for (i = 0; i < ncores; ++i)
-		set_cpu_possible(i, true);
-
-	set_smp_cross_call(gic_raise_softirq);
-}
-
-static void ct_ca9x4_smp_enable(unsigned int max_cpus)
-{
-	scu_enable(V2TILE_PERIPH_P2V(A9_MPCORE_SCU));
-}
-#endif
-
 struct ct_desc ct_ca9x4_desc __initdata = {
 	.id		= V2M_CT_ID_CA9,
 	.name		= "CA9x4",
@@ -218,8 +218,4 @@ struct ct_desc ct_ca9x4_desc __initdata = {
 	.init_early	= ct_ca9x4_init_early,
 	.init_irq	= ct_ca9x4_init_irq,
 	.init_tile	= ct_ca9x4_init,
-#ifdef CONFIG_SMP
-	.init_cpu_map	= ct_ca9x4_init_cpu_map,
-	.smp_enable	= ct_ca9x4_smp_enable,
-#endif
 };
diff --git a/arch/arm/mach-vexpress/include/mach/motherboard.h b/arch/arm/mach-vexpress/include/mach/motherboard.h
index da9ac29..848353b 100644
--- a/arch/arm/mach-vexpress/include/mach/motherboard.h
+++ b/arch/arm/mach-vexpress/include/mach/motherboard.h
@@ -129,10 +129,6 @@ struct ct_desc {
 	void			(*init_early)(void);
 	void			(*init_irq)(void);
 	void			(*init_tile)(void);
-#ifdef CONFIG_SMP
-	void			(*init_cpu_map)(void);
-	void			(*smp_enable)(unsigned int);
-#endif
 };
 
 extern struct ct_desc *ct_desc;
diff --git a/arch/arm/mach-vexpress/platsmp.c b/arch/arm/mach-vexpress/platsmp.c
index e8be99d..b41e4e8 100644
--- a/arch/arm/mach-vexpress/platsmp.c
+++ b/arch/arm/mach-vexpress/platsmp.c
@@ -20,6 +20,9 @@
 
 #include "core.h"
 
+void (*vexpress_init_cpu_map)(void);
+void (*vexpress_smp_enable)(unsigned int);
+
 extern void versatile_secondary_startup(void);
 
 /*
@@ -28,7 +31,7 @@ extern void versatile_secondary_startup(void);
  */
 void __init smp_init_cpus(void)
 {
-	ct_desc->init_cpu_map();
+	vexpress_init_cpu_map();
 }
 
 void __init platform_smp_prepare_cpus(unsigned int max_cpus)
@@ -37,7 +40,7 @@ void __init platform_smp_prepare_cpus(unsigned int max_cpus)
 	 * Initialise the present map, which describes the set of CPUs
 	 * actually populated at the present time.
 	 */
-	ct_desc->smp_enable(max_cpus);
+	vexpress_smp_enable(max_cpus);
 
 	/*
 	 * Write the address of secondary startup into the
-- 
1.6.3.3

Re: [PATCH 2/5] ARM: vexpress: Remove platform SMP functions from ct_desc

[Russell King - ARM Linux]

On Fri, Nov 11, 2011 at 06:27:03PM +0000, Pawel Moll wrote:
> 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:

> +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
> -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;
> -	}
> -
> -	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.

Re: [PATCH 2/5] ARM: vexpress: Remove platform SMP functions from ct_desc

[Pawel Moll]

On Thu, 2011-11-17 at 15:31 +0000, Russell King - ARM Linux wrote:
> On Fri, Nov 11, 2011 at 06:27:03PM +0000, Pawel Moll wrote:
> > 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.

There are gaps (about 50-100m) between the buildings here indeed. And
you have to cross a road... Thankfully we have some network cables
underneath it. 

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

I was remember when Will was adding the tile-detection code. That seemed
the best solution at the time (and spared us getting new mach type
number for every new tile), with no DT at the horizon. Situation changed
since and the tiles are just separate DT-only machine descriptions, as
your original implementation. There will be no more users of the
ct_desc.

> I can't see how this improves anything. 

It's not a improvement, just a workaround because that:

 void __init smp_init_cpus(void)
 {
-       ct_desc->init_cpu_map();
 }

implies existence of the ct_desc. But fair enough, I'll simply create
fake ones for the DT cases so this code won't have to be changed. As
soon as the platform SMP calls are abstracted, which as I understand is
one of steps on the mythical "single binary kernel" way, the problem
will disappear.

>  In fact, this patch reintroduces
> a bug which have been previously fixed:
> 
> > +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
> > -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;
> > -	}
> > -
> > -	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.

Thanks for spotting that, fixed.

All comments appreciated as always, thanks!

Pawel

[PATCH 3/5] ARM: vexpress: Add DT support in v2m

[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 <pawel.moll@arm.com>
---
 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

diff --git a/Documentation/devicetree/bindings/arm/vexpress b/Documentation/devicetree/bindings/arm/vexpress
new file mode 100644
index 0000000..7bf4602
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/vexpress
@@ -0,0 +1,92 @@
+ARM Versatile Express boards family
+-----------------------------------
+
+ARM's Versatile Express platform consists of a motherboard
+and one or more daughterboards (tiles). The motherboard provides
+set of peripherals. Processor and RAM "live" on the tiles.
+Both parts of the system should be described in two separate
+Device Tree source files, with the tile's description including
+motherboard's file. As the motherboard can be work in one of two
+possible configuration ("legacy" and "RS1"), care must be taken
+to include the correct one.
+
+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";
+  where <model> is the the model, eg:
+  - 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.
+
+Optional properties in the root node:
+- tile model name (use the same names as in the tile's Technical
+  Reference Manuals, eg. "V2P-CA5s")
+	model = "<model>";
+- tile's HBI number (unique ARM's board model ID, visible on the
+  PCB's silkscreen) in hexadecimal transcription:
+	arm,hbi = <0xhbi>
+  eg:
+  - for Coretile Express A5x2 (V2P-CA5s) HBI-0191:
+	arm,hbi = <0x191>;
+  - Coretile Express A9x4 (V2P-CA9) HBI-0225:
+	arm,hbi = <0x225>;
+
+Motherboard .dtsi files provide set of phandles to peripherals that
+can be used in the tile's aliases node:
+- UARTs:
+	mb_serial0, mb_serial1, mb_serial2 and mb_serial3
+- I2C controllers:
+	mb_i2c_dvi and mb_i2c_pcie
+- SP804 timers:
+	mb_timer01 and mb_timer23
+
+The motherboard description file provides single "motherboard" node
+using 2 address cells corresponding to the Static Memory Bus used
+between the motherboard and the tile. First cell defines Chip Select
+(CS) line number, the second cell address offset within the CS.
+All interrupts lines between the motherboard and the tile are active
+high and are described using single cell.
+
+The tile description must define "ranges", "interrupt-map-mask" and
+"interrupt-map" properties to translate the motherboard's address
+and interrupt space into one used by the tile's processor.
+
+Abbreviated example:
+
+/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;
+		timer = &mb_timer01;
+	};
+
+	gic: interrupt-controller@2c001000 {
+		compatible = "arm,cortex-a9-gic";
+	};
+
+	motherboard {
+		/* CS0 is visible at 0x08000000 */
+		ranges = <0 0 0x08000000 0x04000000>;
+		interrupt-map-mask = <0 0 63>;
+		/* Active high IRQ 0 is connected to GIC's SPI0 */
+		interrupt-map = <0 0  0 &gic 0  0 4>;
+	};
+}
+
+/include/ "vexpress-v2m-rs1.dtsi"
diff --git a/arch/arm/boot/dts/vexpress-v2m-legacy.dtsi b/arch/arm/boot/dts/vexpress-v2m-legacy.dtsi
new file mode 100644
index 0000000..50905a6
--- /dev/null
+++ b/arch/arm/boot/dts/vexpress-v2m-legacy.dtsi
@@ -0,0 +1,190 @@
+/*
+ * ARM Ltd. Versatile Express
+ *
+ * Motherboard Express uATX
+ * V2M-P1
+ *
+ * HBI-0190D
+ *
+ * Legacy memory map
+ *
+ * WARNING! The hardware described in this file is independent from the
+ * RS1 variant (vexpress-v2m-rs1.dtsi), but there is a strong
+ * correspondence between the two configurations.
+ *
+ * TAKE CARE WHEN MAINTAINING THIS FILE TO PROPAGATE ANY RELEVANT
+ * CHANGES TO vexpress-v2m-rs1.dtsi!
+ */
+
+/ {
+	motherboard {
+		compatible = "simple-bus";
+		#address-cells = <2>; /* SMB chipselect number and offset */
+		#size-cells = <1>;
+		#interrupt-cells = <1>;
+
+		flash@0,00000000 {
+			compatible = "arm,vexpress-flash", "cfi-flash";
+			reg = <0 0x00000000 0x04000000>,
+			      <1 0x00000000 0x04000000>;
+			bank-width = <4>;
+		};
+
+		psram@2,00000000 {
+			compatible = "mtd-ram";
+			reg = <2 0x00000000 0x02000000>;
+			bank-width = <4>;
+		};
+
+		ethernet@3,02000000 {
+			compatible = "smsc,lan9118", "smsc,lan9115";
+			reg = <3 0x02000000 0x10000>;
+			interrupts = <15>;
+			phy-mode = "mii";
+			reg-io-width = <4>;
+			smsc,irq-active-high;
+			smsc,irq-push-pull;
+		};
+
+		usb@3,03000000 {
+			compatible = "nxp,usb-isp1761";
+			reg = <3 0x03000000 0x20000>;
+			interrupts = <16>;
+			port1-otg;
+		};
+
+		iofpga@7,00000000 {
+			compatible = "arm,amba-bus", "simple-bus";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0 7 0 0x20000>;
+
+			sysreg@00000 {
+				compatible = "arm,vexpress-sysreg";
+				reg = <0x00000 0x1000>;
+			};
+
+			sysctl@01000 {
+				compatible = "arm,sp810", "arm,primecell";
+				reg = <0x01000 0x1000>;
+			};
+
+			/* PCI-E I2C bus */
+			mb_i2c_pcie: i2c@02000 {
+				compatible = "arm,versatile-i2c";
+				reg = <0x02000 0x1000>;
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				pcie-switch@60 {
+					compatible = "idt,89hpes32h8";
+					reg = <0x60>;
+				};
+			};
+
+			aaci@04000 {
+				compatible = "arm,pl041", "arm,primecell";
+				reg = <0x04000 0x1000>;
+				interrupts = <11>;
+			};
+
+			mmci@05000 {
+				compatible = "arm,pl180", "arm,primecell";
+				reg = <0x05000 0x1000>;
+				interrupts = <9 10>;
+			};
+
+			kmi@06000 {
+				compatible = "arm,pl050", "arm,primecell";
+				reg = <0x06000 0x1000>;
+				interrupts = <12>;
+			};
+
+			kmi@07000 {
+				compatible = "arm,pl050", "arm,primecell";
+				reg = <0x07000 0x1000>;
+				interrupts = <13>;
+			};
+
+			mb_serial0: uart@09000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x09000 0x1000>;
+				interrupts = <5>;
+			};
+
+			mb_serial1: uart@0a000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x0a000 0x1000>;
+				interrupts = <6>;
+			};
+
+			mb_serial2: uart@0b000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x0b000 0x1000>;
+				interrupts = <7>;
+			};
+
+			mb_serial3: uart@0c000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x0c000 0x1000>;
+				interrupts = <8>;
+			};
+
+			wdt@0f000 {
+				compatible = "arm,sp805", "arm,primecell";
+				reg = <0x0f000 0x1000>;
+				interrupts = <0>;
+			};
+
+			mb_timer01: timer@11000 {
+				compatible = "arm,sp804", "arm,primecell";
+				reg = <0x11000 0x1000>;
+				interrupts = <2>;
+			};
+
+			mb_timer23: timer@12000 {
+				compatible = "arm,sp804", "arm,primecell";
+				reg = <0x12000 0x1000>;
+			};
+
+			/* DVI I2C bus */
+			mb_i2c_dvi: i2c@16000 {
+				compatible = "arm,versatile-i2c";
+				reg = <0x16000 0x1000>;
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				dvi-transmitter@39 {
+					compatible = "sil,sii9022-tpi", "sil,sii9022";
+					reg = <0x39>;
+				};
+
+				dvi-transmitter@60 {
+					compatible = "sil,sii9022-cpi", "sil,sii9022";
+					reg = <0x60>;
+				};
+			};
+
+			rtc@17000 {
+				compatible = "arm,pl031", "arm,primecell";
+				reg = <0x17000 0x1000>;
+				interrupts = <4>;
+			};
+
+			compact-flash@1a000 {
+				compatible = "ata-generic";
+				reg = <0x1a000 0x100
+				       0x1a100 0xf00>;
+				reg-shift = <2>;
+			};
+
+			clcd@1f000 {
+				compatible = "arm,pl111", "arm,primecell";
+				reg = <0x1f000 0x1000>;
+				interrupts = <14>;
+			};
+		};
+	};
+};
diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
index 9311484..4c11e90 100644
--- a/arch/arm/mach-vexpress/Kconfig
+++ b/arch/arm/mach-vexpress/Kconfig
@@ -9,4 +9,8 @@ config ARCH_VEXPRESS_CA9X4
 	select ARM_ERRATA_751472
 	select ARM_ERRATA_753970
 
+config ARCH_VEXPRESS_DT
+	bool
+	select OF
+
 endmenu
diff --git a/arch/arm/mach-vexpress/core.h b/arch/arm/mach-vexpress/core.h
index 2139b37..e5eaba6 100644
--- a/arch/arm/mach-vexpress/core.h
+++ b/arch/arm/mach-vexpress/core.h
@@ -29,3 +29,12 @@ extern void (*vexpress_init_cpu_map)(void);
 extern void (*vexpress_smp_enable)(unsigned int);
 #endif
 
+#if defined(CONFIG_ARCH_VEXPRESS_DT)
+
+extern struct sys_timer v2m_timer;
+
+void __init v2m_dt_map_io(void);
+void __init v2m_dt_init_early(void);
+struct of_dev_auxdata * __init v2m_dt_get_auxdata(void);
+
+#endif
diff --git a/arch/arm/mach-vexpress/include/mach/motherboard.h b/arch/arm/mach-vexpress/include/mach/motherboard.h
index 848353b..37b56ed 100644
--- a/arch/arm/mach-vexpress/include/mach/motherboard.h
+++ b/arch/arm/mach-vexpress/include/mach/motherboard.h
@@ -116,6 +116,14 @@ int v2m_cfg_read(u32 devfn, u32 *data);
 void v2m_flags_set(u32 data);
 
 /*
+ * Miscellaneous
+ */
+#define SYS_MISC_MASTERSITE     (1 << 14)
+#define SYS_PROCIDx_HBI_MASK	0xfff
+
+
+
+/*
  * Core tile IDs
  */
 #define V2M_CT_ID_CA9		0x0c000191
diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
index b84fa45..9ad772d 100644
--- a/arch/arm/mach-vexpress/v2m.c
+++ b/arch/arm/mach-vexpress/v2m.c
@@ -6,6 +6,10 @@
 #include <linux/amba/mmci.h>
 #include <linux/io.h>
 #include <linux/init.h>
+#include <linux/of_address.h>
+#include <linux/of_fdt.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/ata_platform.h>
 #include <linux/smsc911x.h>
@@ -17,6 +21,7 @@
 
 #include <asm/mach-types.h>
 #include <asm/sizes.h>
+#include <asm/system.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
 #include <asm/mach/time.h>
@@ -31,6 +36,7 @@
 
 #include "core.h"
 
+/* Legacy memory map values for non-DT code */
 #define V2M_PA_CS0	0x40000000
 #define V2M_PA_CS1	0x44000000
 #define V2M_PA_CS2	0x48000000
@@ -57,11 +63,33 @@ static void __init v2m_timer_init(void)
 	unsigned int timer01_irq;
 	u32 scctrl;
 
+	if (of_have_populated_dt()) {
+#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);
+		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);
+#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;
+	}
+
 	/* Select 1MHz TIMCLK as the reference clock for SP804 timers */
 	scctrl = readl(sysctl_base + SCCTRL);
 	scctrl |= SCCTRL_TIMEREN0SEL_TIMCLK;
@@ -76,7 +104,8 @@ static void __init v2m_timer_init(void)
 			"v2m-timer0");
 }
 
-static struct sys_timer v2m_timer = {
+/* Used by DT-powered core tiles */
+struct sys_timer v2m_timer = {
 	.init	= v2m_timer_init,
 };
 
@@ -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);
+}
+
 static void __init v2m_init_early(void)
 {
 	ct_desc->init_early();
 	clkdev_add_table(v2m_lookups, ARRAY_SIZE(v2m_lookups));
 	versatile_sched_clock_init(v2m_sysreg_base + V2M_SYS_24MHZ, 24000000);
+	v2m_system_id();
 }
 
 static void v2m_power_off(void)
@@ -472,3 +508,105 @@ MACHINE_START(VEXPRESS, "ARM-Versatile Express")
 	.timer		= &v2m_timer,
 	.init_machine	= v2m_init,
 MACHINE_END
+
+
+
+#if defined(CONFIG_ARCH_VEXPRESS_DT)
+
+void __init v2m_dt_map_io(void)
+{
+		iotable_init(v2m_rs1_io_desc, ARRAY_SIZE(v2m_rs1_io_desc));
+}
+
+static struct clk_lookup v2m_dt_lookups[] = {
+	{	/* AMBA bus clock */
+		.con_id		= "apb_pclk",
+		.clk		= &dummy_apb_pclk,
+	}, {	/* SP804 timers */
+		.dev_id		= "sp804",
+		.con_id		= "v2m-timer0",
+		.clk		= &v2m_sp804_clk,
+	}, {	/* SP804 timers */
+		.dev_id		= "sp804",
+		.con_id		= "v2m-timer1",
+		.clk		= &v2m_sp804_clk,
+	},
+	/* Legacy memory map */
+	{	/* PL180 MMCI */
+		.dev_id		= "mb:mmci", /* 10005000.mmci */
+		.clk		= &osc2_clk,
+	}, {	/* PL050 KMI0 */
+		.dev_id		= "10006000.kmi",
+		.clk		= &osc2_clk,
+	}, {	/* PL050 KMI1 */
+		.dev_id		= "10007000.kmi",
+		.clk		= &osc2_clk,
+	}, {	/* PL011 UART0 */
+		.dev_id		= "10009000.uart",
+		.clk		= &osc2_clk,
+	}, {	/* PL011 UART1 */
+		.dev_id		= "1000a000.uart",
+		.clk		= &osc2_clk,
+	}, {	/* PL011 UART2 */
+		.dev_id		= "1000b000.uart",
+		.clk		= &osc2_clk,
+	}, {	/* PL011 UART3 */
+		.dev_id		= "1000c000.uart",
+		.clk		= &osc2_clk,
+	}, {	/* SP805 WDT */
+		.dev_id		= "1000f000.wdt",
+		.clk		= &v2m_ref_clk,
+	}, {	/* PL111 CLCD */
+		.dev_id		= "1001f000.clcd",
+		.clk		= &osc1_clk,
+	},
+};
+
+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));
+		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);
+	}
+
+	clkdev_add_table(v2m_dt_lookups, ARRAY_SIZE(v2m_dt_lookups));
+	versatile_sched_clock_init(v2m_sysreg_base + V2M_SYS_24MHZ, 24000000);
+
+	pm_power_off = v2m_power_off;
+	arm_pm_restart = v2m_restart;
+
+	v2m_system_id();
+}
+
+static struct of_dev_auxdata v2m_dt_auxdata_lookup[] __initdata = {
+	/* Legacy memory map */
+	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),
+	{}
+};
+
+struct of_dev_auxdata * __init v2m_dt_get_auxdata(void)
+{
+	return v2m_dt_auxdata_lookup;
+}
+
+#endif
-- 
1.6.3.3

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Dave Martin]

On Fri, Nov 11, 2011 at 06:27:04PM +0000, Pawel Moll wrote:
> This patch provides hooks for DT-based tile machine implementations
> and adds Device Tree description for the motherboard.
> 
> Signed-off-by: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> ---
>  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
> 
> diff --git a/Documentation/devicetree/bindings/arm/vexpress b/Documentation/devicetree/bindings/arm/vexpress

[...]

> +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".


> +  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" ?

> +  - 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?

[...]

> diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
> index 9311484..4c11e90 100644
> --- a/arch/arm/mach-vexpress/Kconfig
> +++ b/arch/arm/mach-vexpress/Kconfig
> @@ -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.

[...]

> diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c

[...]

> +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));
> +		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?

Cheers
---Dave

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Rob Herring]

On 11/16/2011 09:44 AM, Dave Martin wrote:
> On Fri, Nov 11, 2011 at 06:27:04PM +0000, Pawel Moll wrote:
>> This patch provides hooks for DT-based tile machine implementations
>> and adds Device Tree description for the motherboard.
>>
>> Signed-off-by: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
>> ---
>>  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
>>
>> diff --git a/Documentation/devicetree/bindings/arm/vexpress b/Documentation/devicetree/bindings/arm/vexpress
> 
> [...]
> 
>> +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".
> 

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.

Rob

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Pawel Moll]

On Wed, 2011-11-16 at 16:26 +0000, Rob Herring wrote:
> On 11/16/2011 09:44 AM, Dave Martin wrote:
> > On Fri, Nov 11, 2011 at 06:27:04PM +0000, Pawel Moll wrote:
> >> This patch provides hooks for DT-based tile machine implementations
> >> and adds Device Tree description for the motherboard.
> >>
> >> Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> >> ---
> >>  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
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/vexpress b/Documentation/devicetree/bindings/arm/vexpress
> > 
> > [...]
> > 
> >> +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".
>
> 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

> 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

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

and

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

Cheers!

Paweł



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Rob Herring]

On 11/16/2011 10:37 AM, Pawel Moll wrote:
> On Wed, 2011-11-16 at 16:26 +0000, Rob Herring wrote:
>> On 11/16/2011 09:44 AM, Dave Martin wrote:
>>> On Fri, Nov 11, 2011 at 06:27:04PM +0000, Pawel Moll wrote:
>>>> This patch provides hooks for DT-based tile machine implementations
>>>> and adds Device Tree description for the motherboard.
>>>>
>>>> Signed-off-by: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
>>>> ---
>>>>  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
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/vexpress b/Documentation/devicetree/bindings/arm/vexpress
>>>
>>> [...]
>>>
>>>> +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".
>>
>> 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
> 

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.

>> 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
> 
>>> 	compatible = "arm,vexpress-<model>", "arm-vexpress-rs1", "arm-vexpress";
> 
> and
> 
>>> +	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.

Rob

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Pawel Moll]

On Wed, 2011-11-16 at 16:59 +0000, Rob Herring wrote:
> 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 ;-)

> >> 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
> > 
> >>> 	compatible = "arm,vexpress-<model>", "arm-vexpress-rs1", "arm-vexpress";
> > 
> > and
> > 
> >>> +	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ł



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Pawel Moll]

On Wed, 2011-11-16 at 17:07 +0000, Pawel Moll wrote:
> > > I'd rather second Dave's idea of having
> > > 
> > >>> 	compatible = "arm,vexpress-<model>", "arm-vexpress-rs1", "arm-vexpress";
> > > 
> > > and
> > > 
> > >>> +	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?

Cheers!

Paweł



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Dave Martin]

On Wed, Nov 16, 2011 at 05:37:34PM +0000, Pawel Moll wrote:
> On Wed, 2011-11-16 at 17:07 +0000, Pawel Moll wrote:
> > > > I'd rather second Dave's idea of having
> > > > 
> > > >>> 	compatible = "arm,vexpress-<model>", "arm-vexpress-rs1", "arm-vexpress";
> > > > 
> > > > and
> > > > 
> > > >>> +	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?

Yes, I think this will work.

Because this value is not used for driver binding etc., there's no need
for it to be a compatible string: a property is just fine.  So, actually
it may be better.


Because this is a new binding, we can also describe _exactly_ the meaning
of this property being absent.  In other words, we can say that if the
am,v2m-memory-map property is absent, the memory map is the legacy
memory map.  If the property is present with value "rs1", we have the
rs1 memory map.  We can easily add extra values in the future as needed,
without any risk of incompatibility with the legacy case.

So how about:

// Legacy case
/ {
	motherboard {
		// arm,v2m-memory-map absent


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

If you prefer an explicit = "legacy" for the legacy case though,
I'm fine with that too.


When we remove compatible strings from the compatible set on the other
hand, the device described by the node just becomes more and more
general which isn't what we want: legacy is not a generalisation of rs1,
nor vice versa; they are just plain different.  Because properties
don't have these implies generalisation/specialisation semantics, we
don't have this problem when using a property.

Cheers
---Dave

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Dave Martin]

On Wed, Nov 16, 2011 at 05:07:51PM +0000, Pawel Moll wrote:
> On Wed, 2011-11-16 at 16:59 +0000, Rob Herring wrote:
> > 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 ;-)

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:

/ {
	compatible = "arm,vexpress-ca9x4"


> 
> > >> 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
> > > 
> > >>> 	compatible = "arm,vexpress-<model>", "arm-vexpress-rs1", "arm-vexpress";
> > > 
> > > and
> > > 
> > >>> +	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ł
> 
> 
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Dave Martin]

On Wed, Nov 16, 2011 at 05:07:51PM +0000, Pawel Moll wrote:
> On Wed, 2011-11-16 at 16:59 +0000, Rob Herring wrote:
> > 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 ;-)

(excuse the previous, half-written mail...)

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";
	};
};


I'm not commenting on the "legacy" name here -- it is, for better
or worse, the official name for the old memory map.  I don't feel
that calling that "arm,vexpress" is correct either, however.  There
is no such thing as a vanilla vexpress board, because the board
is not fully specified at all without _something_ in the IOFPGA.
Nor are there any prior upstream DT bindings for this platform which
we would be referring back to.

The motherboard node, including the compatible property would be defined
once per memory map, in the relevant .dtsi file.


If we don't have a generic compatible string "arm,vexpress", then we'd
start having to list every possible "arm,vexpress-v2p-*" in the
compatible lists in the code.  I'm not clear on whether that's right
or wrong, but it could certainly get cumbersome.


Thoughts?

> > > 
> > > and
> > > 
> > >>> +	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.

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

Cheers
---Dave

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Pawel Moll]

On Wed, 2011-11-16 at 17:50 +0000, Dave Martin wrote:
> 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.

Yep, that works for me :-) Probably even easier to implement than custom
property.

I'll get it into the code tomorrow.

Cheers!

Paweł



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Russell King - ARM Linux]

On Wed, Nov 16, 2011 at 05:07:51PM +0000, Pawel Moll wrote:
> On Wed, 2011-11-16 at 16:59 +0000, Rob Herring wrote:
> > 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.

If DT support was created at this point, it would have been called
something without 'legacy' in its name.

A year or so on, it's been superseded by something else, and is now called
legacy.  The way DT works, you don't go around renaming stuff because it's
become legacy.  So, the original DT support still stands using whatever
named properties would have been invented at the time.

So, when we create support for it *now*, we should not be creating a DT
file with properties named as 'legacy'.  The fact that it has become
legacy _today_ is irrelevant to DT.

If we follow through the argument that we _should_ name things legacy,
then it follows that we need to constantly patch DT files to rename stuff
to 'legacy' when it's no longer the latest and greatest - and then you
need the right DT file corresponding to the right kernel version.

Again, this is _not_ how DT is supposed to work.  DT is about describing
the hardware, not describing the state of something.  A DT description
of a hardware system created 10 years ago should still work _today_ even
though the hardware system may now be 'legacy'.

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.

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Pawel Moll]

On Thu, 2011-11-17 at 15:53 +0000, Russell King - ARM Linux wrote:
> You're totally failing to understand the point.

That's your opinion, I think otherwise, won't try to convince you.

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

That's right. And then when the new variant was created *hardware
designers* decided to call the previous one "legacy". I couldn't care
less what word was it, could be even "bublebee" from my point of view.

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

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

As the compatible values will be limited to the tile name and the
information about the motherboard "mode" will be passed through private
node property, I will follow Dave's suggestion of using "empty value"
for legacy/original/whatever variant. Hopefully this will end this topic
which is already too long.

Thanks for all comments!

Pawel

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Russell King - ARM Linux]

On Fri, Nov 18, 2011 at 12:20:49PM +0000, Pawel Moll wrote:
> On Thu, 2011-11-17 at 15:53 +0000, Russell King - ARM Linux wrote:
> > 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.

Again, you're totally FAILING to get the point.

Had DT support been created before the new version came out there would
be no 'legacy' in terminology what so ever.  _That's_ the whole point.

What you're using is *todays* terminology, not the terminology used at
the point of *creation*.

Why can't you get it through your thick skull that at some day, the
entire Cortex-A5 series of CPUs is going to be called 'legacy'.  And
at that point, will you be wanting to rename all the properties because
of that change?  No.  That's insane.  So why the fuck do it now to the
CA9x5?

Try thinking.  It's good for the mind.

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Pawel Moll]

On Wed, 2011-11-16 at 15:44 +0000, Dave Martin wrote:
> > +  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" ?

Ok.

> > +  - 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?

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

> > diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
> > index 9311484..4c11e90 100644
> > --- a/arch/arm/mach-vexpress/Kconfig
> > +++ b/arch/arm/mach-vexpress/Kconfig
> > @@ -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.

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

> > diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
> 
> [...]
> 
> > +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));
> > +		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?

Cheers!

Paweł





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Dave Martin]

On Wed, Nov 16, 2011 at 04:35:15PM +0000, Pawel Moll wrote:

[...]

> > > +  - 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?
> 
> 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.

> > > diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
> > > index 9311484..4c11e90 100644
> > > --- a/arch/arm/mach-vexpress/Kconfig
> > > +++ b/arch/arm/mach-vexpress/Kconfig
> > > @@ -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.
> 
> 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.

> 
> > > diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
> > 
> > [...]
> > 
> > > +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));
> > > +		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?

I don't have a strong feeling about it.  Printing a warning is certainly
helpful; by definition we don't really know what to do beyond this
point.

So, the choices are: try to do something reasonable, or panic.

There's no right answer.  "Try to do something reasonable" seems as good
as any other option ... and there's always the possibility that it might
work.

Cheers
---Dave

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Pawel Moll]

On Wed, 2011-11-16 at 17:57 +0000, Dave Martin wrote:
> > > 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).

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

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

Cheers!

Paweł



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Dave Martin]

On Thu, Nov 17, 2011 at 01:50:13PM +0000, Pawel Moll wrote:
> On Wed, 2011-11-16 at 17:57 +0000, Dave Martin wrote:
> > > > 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).
> 
> > 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.

OK, it makes sense to follow other boards in that case.  This could be
a topic for separate discussion, but I agree it's probably out of scope
here.

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

Cheers
---Dave

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Russell King - ARM Linux]

On Fri, Nov 11, 2011 at 06:27:04PM +0000, Pawel Moll wrote:
> +#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);
> +		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'.

> +#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?

> @@ -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.

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Dave Martin]

On Thu, Nov 17, 2011 at 04:05:37PM +0000, Russell King - ARM Linux wrote:

[...]

> > @@ -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.

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.  Hardware features which can be probed can still
be described in the DT and sometimes this is advantageous -- nonetheless
it does break the one-definition principle and may lead to problems.
One scenario where this does make sense is if the bootloader discovers
that value and injects it into the DT.  However, that would broaden the
interface between the bootloader and kernel in a platform-specific way
and may cause problems itself.

Cheers
---Dave

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Russell King - ARM Linux]

On Thu, Nov 17, 2011 at 06:37:26PM +0000, Dave Martin wrote:
> On Thu, Nov 17, 2011 at 04:05:37PM +0000, Russell King - ARM Linux wrote:
> > 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.

That is exactly my point - but we don't want V2M_SYS_ID controlling what
hardware is there because then you'll end up having _drivers_ having to
be aware of the system revision.

Instead, the differences in device IP - when they're not discoverable
from the device itself - should be encoded in DT to allow the device IP
to be reused on different platforms which may not even have a V2M_SYS_ID
register.

That's what my objection is about: nothing should be using the overall
system revision to determine anything about it.

Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m

[Pawel Moll]

On Thu, 2011-11-17 at 16:05 +0000, Russell King - ARM Linux wrote:
> On Fri, Nov 11, 2011 at 06:27:04PM +0000, Pawel Moll wrote:
> > +#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);
> > +		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?  

Yes, just enough, thank you.

> It's well worth reading
> Linus' various messages on the use of BUG(), which can be found:
> 
> 	http://yarchive.net/comp/linux/BUG.html

It was worth reading indeed. I will adapt to the policy.

> 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'.
> 
> > +#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?

No, you are right.

> > @@ -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.

SYS_ID encodes the motherboard revision (SYS_ID >> 28) and the FPGA
build (SYS_ID & 0xf) - those two together constitute the system
revision. As as 4 < 5 < 6, SYS_IDs of Rev A < Rev B < Rev C. I should
have masked out the non-relevant bits.

> 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 change had nothing to do DT or making choices - I was simply asked
to expose this information to the userspace for testing purposes and the
system_rev, available via /proc/cpuinfo seemed the best choice.

But as this change has nothing to do with DT it shouldn't be included in
this patch in the first place. I will split it out.

All comments appreciated as always, thanks!

Pawel

[PATCH 4/5] ARM: vexpress: Initial RS1 memory map support

[Pawel Moll]

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 <pawel.moll@arm.com>
---
 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

diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
new file mode 100644
index 0000000..6f9a822
--- /dev/null
+++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
@@ -0,0 +1,190 @@
+/*
+ * ARM Ltd. Versatile Express
+ *
+ * Motherboard Express uATX
+ * V2M-P1
+ *
+ * HBI-0190D
+ *
+ * RS1 memory map (a.k.a. ARM Cortex-A Series memory map)
+ *
+ * WARNING! The hardware described in this file is independent from the
+ * legacy variant (vexpress-v2m-legacy.dtsi), but there is a strong
+ * correspondence between the two configurations.
+ *
+ * TAKE CARE WHEN MAINTAINING THIS FILE TO PROPAGATE ANY RELEVANT
+ * CHANGES TO vexpress-v2m-legacy.dtsi!
+ */
+
+/ {
+	motherboard {
+		compatible = "simple-bus";
+		#address-cells = <2>; /* SMB chipselect number and offset */
+		#size-cells = <1>;
+		#interrupt-cells = <1>;
+
+		flash@0,00000000 {
+			compatible = "arm,vexpress-flash", "cfi-flash";
+			reg = <0 0x00000000 0x04000000>,
+			      <4 0x00000000 0x04000000>;
+			bank-width = <4>;
+		};
+
+		psram@1,00000000 {
+			compatible = "mtd-ram";
+			reg = <1 0x00000000 0x02000000>;
+			bank-width = <4>;
+		};
+
+		ethernet@2,02000000 {
+			compatible = "smsc,lan9118", "smsc,lan9115";
+			reg = <2 0x02000000 0x10000>;
+			interrupts = <15>;
+			phy-mode = "mii";
+			reg-io-width = <4>;
+			smsc,irq-active-high;
+			smsc,irq-push-pull;
+		};
+
+		usb@2,03000000 {
+			compatible = "nxp,usb-isp1761";
+			reg = <2 0x03000000 0x20000>;
+			interrupts = <16>;
+			port1-otg;
+		};
+
+		iofpga@3,00000000 {
+			compatible = "arm,amba-bus", "simple-bus";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0 3 0 0x200000>;
+
+			sysreg@010000 {
+				compatible = "arm,vexpress-sysreg";
+				reg = <0x010000 0x1000>;
+			};
+
+			sysctl@020000 {
+				compatible = "arm,sp810", "arm,primecell";
+				reg = <0x020000 0x1000>;
+			};
+
+			/* PCI-E I2C bus */
+			mb_i2c_pcie: i2c@030000 {
+				compatible = "arm,versatile-i2c";
+				reg = <0x030000 0x1000>;
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				pcie-switch@60 {
+					compatible = "idt,89hpes32h8";
+					reg = <0x60>;
+				};
+			};
+
+			aaci@040000 {
+				compatible = "arm,pl041", "arm,primecell";
+				reg = <0x040000 0x1000>;
+				interrupts = <11>;
+			};
+
+			mmci@050000 {
+				compatible = "arm,pl180", "arm,primecell";
+				reg = <0x050000 0x1000>;
+				interrupts = <9 10>;
+			};
+
+			kmi@060000 {
+				compatible = "arm,pl050", "arm,primecell";
+				reg = <0x060000 0x1000>;
+				interrupts = <12>;
+			};
+
+			kmi@070000 {
+				compatible = "arm,pl050", "arm,primecell";
+				reg = <0x070000 0x1000>;
+				interrupts = <13>;
+			};
+
+			mb_serial0: uart@090000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x090000 0x1000>;
+				interrupts = <5>;
+			};
+
+			mb_serial1: uart@0a0000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x0a0000 0x1000>;
+				interrupts = <6>;
+			};
+
+			mb_serial2: uart@0b0000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x0b0000 0x1000>;
+				interrupts = <7>;
+			};
+
+			mb_serial3: uart@0c0000 {
+				compatible = "arm,pl011", "arm,primecell";
+				reg = <0x0c0000 0x1000>;
+				interrupts = <8>;
+			};
+
+			wdt@0f0000 {
+				compatible = "arm,sp805", "arm,primecell";
+				reg = <0x0f0000 0x1000>;
+				interrupts = <0>;
+			};
+
+			mb_timer01: timer@110000 {
+				compatible = "arm,sp804", "arm,primecell";
+				reg = <0x110000 0x1000>;
+				interrupts = <2>;
+			};
+
+			mb_timer23: timer@120000 {
+				compatible = "arm,sp804", "arm,primecell";
+				reg = <0x120000 0x1000>;
+			};
+
+			/* DVI I2C bus */
+			mb_i2c_dvi: i2c@160000 {
+				compatible = "arm,versatile-i2c";
+				reg = <0x160000 0x1000>;
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				dvi-transmitter@39 {
+					compatible = "sil,sii9022-tpi", "sil,sii9022";
+					reg = <0x39>;
+				};
+
+				dvi-transmitter@60 {
+					compatible = "sil,sii9022-cpi", "sil,sii9022";
+					reg = <0x60>;
+				};
+			};
+
+			rtc@170000 {
+				compatible = "arm,pl031", "arm,primecell";
+				reg = <0x170000 0x1000>;
+				interrupts = <4>;
+			};
+
+			compact-flash@1a0000 {
+				compatible = "ata-generic";
+				reg = <0x1a0000 0x100
+				       0x1a0100 0xf00>;
+				reg-shift = <2>;
+			};
+
+			clcd@1f0000 {
+				compatible = "arm,pl111", "arm,primecell";
+				reg = <0x1f0000 0x1000>;
+				interrupts = <14>;
+			};
+		};
+	};
+};
diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
index 4c11e90..d1b0f35 100644
--- a/arch/arm/mach-vexpress/Kconfig
+++ b/arch/arm/mach-vexpress/Kconfig
@@ -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
@@ -8,6 +16,7 @@ config ARCH_VEXPRESS_CA9X4
 	select ARM_ERRATA_720789
 	select ARM_ERRATA_751472
 	select ARM_ERRATA_753970
+	select ARCH_VEXPRESS_LEGACY
 
 config ARCH_VEXPRESS_DT
 	bool
diff --git a/arch/arm/mach-vexpress/Makefile.boot b/arch/arm/mach-vexpress/Makefile.boot
index 8630b3d..3278615 100644
--- a/arch/arm/mach-vexpress/Makefile.boot
+++ b/arch/arm/mach-vexpress/Makefile.boot
@@ -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
diff --git a/arch/arm/mach-vexpress/include/mach/debug-macro.S b/arch/arm/mach-vexpress/include/mach/debug-macro.S
index fd9e6c7..adc94ce 100644
--- a/arch/arm/mach-vexpress/include/mach/debug-macro.S
+++ b/arch/arm/mach-vexpress/include/mach/debug-macro.S
@@ -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
+		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
+
 		.endm
 
 #include <asm/hardware/debug-pl01x.S>
diff --git a/arch/arm/mach-vexpress/include/mach/uncompress.h b/arch/arm/mach-vexpress/include/mach/uncompress.h
index 7972c57..0ac6ba5 100644
--- a/arch/arm/mach-vexpress/include/mach/uncompress.h
+++ b/arch/arm/mach-vexpress/include/mach/uncompress.h
@@ -22,7 +22,18 @@
 #define AMBA_UART_CR(base)	(*(volatile unsigned char *)((base) + 0x30))
 #define AMBA_UART_FR(base)	(*(volatile unsigned char *)((base) + 0x18))
 
-#define get_uart_base()	(0x10000000 + 0x00009000)
+#define AMBA_PERIPH_ID0(base)	(*(volatile unsigned char *)((base) + 0xfe0))
+
+#define UART_BASE_LEGACY	0x10009000
+#define UART_BASE_RS1		0x1c090000
+
+static unsigned long get_uart_base(void)
+{
+	if (AMBA_PERIPH_ID0(UART_BASE_LEGACY) == 0x11)
+		return UART_BASE_LEGACY;
+	else
+		return UART_BASE_RS1;
+}
 
 /*
  * This does not append a newline
diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
index 9ad772d..d96bde1 100644
--- a/arch/arm/mach-vexpress/v2m.c
+++ b/arch/arm/mach-vexpress/v2m.c
@@ -513,8 +513,21 @@ MACHINE_END
 
 #if defined(CONFIG_ARCH_VEXPRESS_DT)
 
+static struct map_desc v2m_rs1_io_desc[] __initdata = {
+	{
+		.virtual	= V2M_PERIPH,
+		.pfn		= __phys_to_pfn(0x1c000000),
+		.length		= SZ_2M,
+		.type		= MT_DEVICE,
+	},
+};
+
 void __init v2m_dt_map_io(void)
 {
+	if (of_flat_dt_is_compatible(of_get_flat_dt_root(),
+			"arm,vexpress-legacy"))
+		iotable_init(v2m_io_desc, ARRAY_SIZE(v2m_io_desc));
+	else
 		iotable_init(v2m_rs1_io_desc, ARRAY_SIZE(v2m_rs1_io_desc));
 }
 
@@ -560,6 +573,35 @@ static struct clk_lookup v2m_dt_lookups[] = {
 		.dev_id		= "1001f000.clcd",
 		.clk		= &osc1_clk,
 	},
+	/* RS1 memory map */
+	{	/* PL180 MMCI */
+		.dev_id		= "mb:mmci", /* 1c050000.mmci */
+		.clk		= &osc2_clk,
+	}, {	/* PL050 KMI0 */
+		.dev_id		= "1c060000.kmi",
+		.clk		= &osc2_clk,
+	}, {	/* PL050 KMI1 */
+		.dev_id		= "1c070000.kmi",
+		.clk		= &osc2_clk,
+	}, {	/* PL011 UART0 */
+		.dev_id		= "1c090000.uart",
+		.clk		= &osc2_clk,
+	}, {	/* PL011 UART1 */
+		.dev_id		= "1c0a0000.uart",
+		.clk		= &osc2_clk,
+	}, {	/* PL011 UART2 */
+		.dev_id		= "1c0b0000.uart",
+		.clk		= &osc2_clk,
+	}, {	/* PL011 UART3 */
+		.dev_id		= "1c0c0000.uart",
+		.clk		= &osc2_clk,
+	}, {	/* SP805 WDT */
+		.dev_id		= "1c0f0000.wdt",
+		.clk		= &v2m_ref_clk,
+	}, {	/* PL111 CLCD */
+		.dev_id		= "1c1f0000.clcd",
+		.clk		= &osc1_clk,
+	},
 };
 
 void __init v2m_dt_init_early(void)
@@ -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),
 	{}
 };
 
-- 
1.6.3.3

Re: [PATCH 4/5] ARM: vexpress: Initial RS1 memory map support

[Dave Martin]

On Fri, Nov 11, 2011 at 06:27:05PM +0000, Pawel Moll wrote:
> 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 <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> ---
>  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

[...]

> diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
> index 4c11e90..d1b0f35 100644
> --- a/arch/arm/mach-vexpress/Kconfig
> +++ b/arch/arm/mach-vexpress/Kconfig
> @@ -1,6 +1,14 @@
>  menu "Versatile Express platform type"
>  	depends on ARCH_VEXPRESS
> +config ARCH_VEXPRESS_LEGACY

Brief comment explaining what this is?

> +	bool
> +	select AUTO_ZRELADDR
> +	select ARM_PATCH_PHYS_VIRT
> +
> +config ARCH_VEXPRESS_RS1

Ditto

> +	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.

[...]

> diff --git a/arch/arm/mach-vexpress/include/mach/debug-macro.S b/arch/arm/mach-vexpress/include/mach/debug-macro.S
> index fd9e6c7..adc94ce 100644
> --- a/arch/arm/mach-vexpress/include/mach/debug-macro.S
> +++ b/arch/arm/mach-vexpress/include/mach/debug-macro.S
> @@ -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
> +		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.

[...]

> diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c

[...]

>  void __init v2m_dt_init_early(void)
> @@ -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?

Cheers
---Dave

Re: [PATCH 4/5] ARM: vexpress: Initial RS1 memory map support

[Pawel Moll]

On Wed, 2011-11-16 at 15:42 +0000, Dave Martin wrote:
> > +config ARCH_VEXPRESS_LEGACY
> Brief comment explaining what this is?
> > +config ARCH_VEXPRESS_RS1
> Ditto

Ok.

> > +	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.

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

> 
> [...]
> 
> > diff --git a/arch/arm/mach-vexpress/include/mach/debug-macro.S b/arch/arm/mach-vexpress/include/mach/debug-macro.S
> > index fd9e6c7..adc94ce 100644
> > --- a/arch/arm/mach-vexpress/include/mach/debug-macro.S
> > +++ b/arch/arm/mach-vexpress/include/mach/debug-macro.S
> > @@ -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
> > +		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.

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

> [...]
> 
> > diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
> 
> [...]
> 
> >  void __init v2m_dt_init_early(void)
> > @@ -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...

Cheers!

Paweł



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/5] ARM: vexpress: Initial RS1 memory map support

[Dave Martin]

On Wed, Nov 16, 2011 at 04:28:17PM +0000, Pawel Moll wrote:
> On Wed, 2011-11-16 at 15:42 +0000, Dave Martin wrote:
> > > +config ARCH_VEXPRESS_LEGACY
> > Brief comment explaining what this is?
> > > +config ARCH_VEXPRESS_RS1
> > Ditto
> 
> Ok.
> 
> > > +	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.
> 
> No probs. All suggestions and "templates" welcomed :-)

There were some instances in my previous series.  I think they were
mostly one-liners.

> 
> > 
> > [...]
> > 
> > > diff --git a/arch/arm/mach-vexpress/include/mach/debug-macro.S b/arch/arm/mach-vexpress/include/mach/debug-macro.S
> > > index fd9e6c7..adc94ce 100644
> > > --- a/arch/arm/mach-vexpress/include/mach/debug-macro.S
> > > +++ b/arch/arm/mach-vexpress/include/mach/debug-macro.S
> > > @@ -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
> > > +		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.
> 
> > 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.

I would hope that the LL debug stuff can eventually be tidied up so that
we can choose the right UART via the DT.  But that's really a wishlist
thing.  Until (or unless) that happens, I guess code such as the above
should be fine.

> 
> > [...]
> > 
> > > diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
> > 
> > [...]
> > 
> > >  void __init v2m_dt_init_early(void)
> > > @@ -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...

If as you rightly point out, this stuff is only here temporarily, then
I don't have a strong view on it.

If there will be no reason every to have macros for those addresses,
then it's true that adding them only to remove them later will cause
needless churn.

Cheers
---Dave

Re: [PATCH 4/5] ARM: vexpress: Initial RS1 memory map support

[Russell King - ARM Linux]

On Fri, Nov 11, 2011 at 06:27:05PM +0000, Pawel Moll wrote:
> diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
> index 4c11e90..d1b0f35 100644
> --- a/arch/arm/mach-vexpress/Kconfig
> +++ b/arch/arm/mach-vexpress/Kconfig
> @@ -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
> @@ -8,6 +16,7 @@ config ARCH_VEXPRESS_CA9X4
>  	select ARM_ERRATA_720789
>  	select ARM_ERRATA_751472
>  	select ARM_ERRATA_753970
> +	select ARCH_VEXPRESS_LEGACY

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.

>  
>  config ARCH_VEXPRESS_DT
>  	bool
> diff --git a/arch/arm/mach-vexpress/Makefile.boot b/arch/arm/mach-vexpress/Makefile.boot
> index 8630b3d..3278615 100644
> --- a/arch/arm/mach-vexpress/Makefile.boot
> +++ b/arch/arm/mach-vexpress/Makefile.boot
> @@ -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?

Re: [PATCH 4/5] ARM: vexpress: Initial RS1 memory map support

[Pawel Moll]

On Thu, 2011-11-17 at 15:36 +0000, Russell King - ARM Linux wrote:
> > +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.
> 
> > @@ -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? 

Yes, exactly.

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

All comments appreciated as always, thanks!

Pawel

Re: [PATCH 4/5] ARM: vexpress: Initial RS1 memory map support

[Russell King - ARM Linux]

On Fri, Nov 18, 2011 at 12:20:50PM +0000, Pawel Moll wrote:
> On Thu, 2011-11-17 at 15:36 +0000, Russell King - ARM Linux wrote:
> >  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?

Not with uboot - uboot has been broken for many years in regard of
being able to load kernel images inteligently - it has to be told
explicitly where to load the image in system memory, so if the uImage
is built for 0x80008000, uboot will load it at that address no matter
if there's RAM there or not.

That interacts with the P2V stuff and makes PHYS_OFFSET be 0x80000000
rather than 0x60000000 - and my guess is that it'll still pass in
memory information describing the RAM at 0x60000000 which may result
in the kernel failing to boot.

There's no way around this with uboot at the moment, and I don't
expect the uboots in the field already on devices to be updated to
solve this.

[PATCH 5/5] ARM: vexpress: DT-based support for CoreTiles Express A5x2 and A9x4

[Pawel Moll]

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 <pawel.moll@arm.com>
---
 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

diff --git a/arch/arm/boot/dts/vexpress-v2p-ca5s.dts b/arch/arm/boot/dts/vexpress-v2p-ca5s.dts
new file mode 100644
index 0000000..d88aa0a
--- /dev/null
+++ b/arch/arm/boot/dts/vexpress-v2p-ca5s.dts
@@ -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@80000000 {
+		device_type = "memory";
+		reg = <0x80000000 0x40000000>;
+	};
+
+	hdlcd@2a110000 {
+		compatible = "arm,hdlcd";
+		reg = <0x2a110000 0x1000>;
+		interrupts = <0 85 4>;
+	};
+	
+	dmc@2a150000 {
+		compatible = "arm,pl341", "arm,primecell";
+		reg = <0x2a150000 0x1000>;
+	};
+
+	smc@2a190000 {
+		compatible = "arm,pl354", "arm,primecell";
+		reg = <0x2a190000 0x1000>;
+		interrupts = <0 86 4>,
+			     <0 87 4>;
+	};
+
+	gic: interrupt-controller@2c001000 {
+		compatible = "arm,cortex-a9-gic";
+		#interrupt-cells = <3>;
+		#address-cells = <0>;
+		interrupt-controller;
+		reg = <0x2c001000 0x1000>,
+		      <0x2c000100 0x100>;
+	};
+
+	L2: cache-controller@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"
diff --git a/arch/arm/boot/dts/vexpress-v2p-ca9.dts b/arch/arm/boot/dts/vexpress-v2p-ca9.dts
new file mode 100644
index 0000000..0f26cba
--- /dev/null
+++ b/arch/arm/boot/dts/vexpress-v2p-ca9.dts
@@ -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@60000000 {
+		device_type = "memory";
+		reg = <0x60000000 0x40000000>;
+	};
+
+	clcd@10020000 {
+		compatible = "arm,pl111", "arm,primecell";
+		reg = <0x10020000 0x1000>;
+		interrupts = <0 44 4>;
+	};
+
+	dmc@100e0000 {
+		compatible = "arm,pl341", "arm,primecell";
+		reg = <0x100e0000 0x1000>;
+	};
+
+	smc@100e1000 {
+		compatible = "arm,pl354", "arm,primecell";
+		reg = <0x100e1000 0x1000>;
+		interrupts = <0 45 4>,
+			     <0 46 4>;
+	};
+
+	timer@100e4000 {
+		compatible = "arm,sp804", "arm,primecell";
+		reg = <0x100e4000 0x1000>;
+		interrupts = <0 48 4>,
+			     <0 49 4>;
+	};
+
+	watchdog@100e5000 {
+		compatible = "arm,sp805", "arm,primecell";
+		reg = <0x100e5000 0x1000>;
+		interrupts = <0 51 4>;
+	};
+
+	gic: interrupt-controller@1e001000 {
+		compatible = "arm,cortex-a9-gic";
+		#interrupt-cells = <3>;
+		#address-cells = <0>;
+		interrupt-controller;
+		reg = <0x1e001000 0x1000>,
+		      <0x1e000100 0x100>;
+	};
+
+	L2: cache-controller@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"
diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
index d1b0f35..6b79400 100644
--- a/arch/arm/mach-vexpress/Kconfig
+++ b/arch/arm/mach-vexpress/Kconfig
@@ -22,4 +22,19 @@ config ARCH_VEXPRESS_DT
 	bool
 	select OF
 
+config ARCH_VEXPRESS_V2P_CA5S_CA9
+	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
diff --git a/arch/arm/mach-vexpress/Makefile b/arch/arm/mach-vexpress/Makefile
index 90551b9..06e3687 100644
--- a/arch/arm/mach-vexpress/Makefile
+++ b/arch/arm/mach-vexpress/Makefile
@@ -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
diff --git a/arch/arm/mach-vexpress/v2p-ca5s_ca9.c b/arch/arm/mach-vexpress/v2p-ca5s_ca9.c
new file mode 100644
index 0000000..0f1cd9b
--- /dev/null
+++ b/arch/arm/mach-vexpress/v2p-ca5s_ca9.c
@@ -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
-- 
1.6.3.3

Re: [PATCH 5/5] ARM: vexpress: DT-based support for CoreTiles Express A5x2 and A9x4

[Rob Herring]

Pawel,

Just a few comments:

On 11/11/2011 12:27 PM, Pawel Moll wrote:
> 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 <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> ---
>  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
> 
> diff --git a/arch/arm/boot/dts/vexpress-v2p-ca5s.dts b/arch/arm/boot/dts/vexpress-v2p-ca5s.dts
> new file mode 100644
> index 0000000..d88aa0a
> --- /dev/null
> +++ b/arch/arm/boot/dts/vexpress-v2p-ca5s.dts
> @@ -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@80000000 {
> +		device_type = "memory";
> +		reg = <0x80000000 0x40000000>;
> +	};
> +
> +	hdlcd@2a110000 {
> +		compatible = "arm,hdlcd";
> +		reg = <0x2a110000 0x1000>;
> +		interrupts = <0 85 4>;
> +	};
> +	
> +	dmc@2a150000 {

I missed this earlier, but the preferred generic name is "memory-controller"
> +		compatible = "arm,pl341", "arm,primecell";
> +		reg = <0x2a150000 0x1000>;
> +	};
> +
> +	smc@2a190000 {

same here.

> +		compatible = "arm,pl354", "arm,primecell";
> +		reg = <0x2a190000 0x1000>;
> +		interrupts = <0 86 4>,
> +			     <0 87 4>;
> +	};
> +
> +	gic: interrupt-controller@2c001000 {
> +		compatible = "arm,cortex-a9-gic";
> +		#interrupt-cells = <3>;
> +		#address-cells = <0>;
> +		interrupt-controller;
> +		reg = <0x2c001000 0x1000>,
> +		      <0x2c000100 0x100>;
> +	};
> +
> +	L2: cache-controller@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"
> diff --git a/arch/arm/boot/dts/vexpress-v2p-ca9.dts b/arch/arm/boot/dts/vexpress-v2p-ca9.dts
> new file mode 100644
> index 0000000..0f26cba
> --- /dev/null
> +++ b/arch/arm/boot/dts/vexpress-v2p-ca9.dts
> @@ -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@60000000 {
> +		device_type = "memory";
> +		reg = <0x60000000 0x40000000>;
> +	};
> +
> +	clcd@10020000 {
> +		compatible = "arm,pl111", "arm,primecell";
> +		reg = <0x10020000 0x1000>;
> +		interrupts = <0 44 4>;
> +	};
> +
> +	dmc@100e0000 {
> +		compatible = "arm,pl341", "arm,primecell";
> +		reg = <0x100e0000 0x1000>;
> +	};
> +
> +	smc@100e1000 {
> +		compatible = "arm,pl354", "arm,primecell";
> +		reg = <0x100e1000 0x1000>;
> +		interrupts = <0 45 4>,
> +			     <0 46 4>;
> +	};
> +
> +	timer@100e4000 {
> +		compatible = "arm,sp804", "arm,primecell";
> +		reg = <0x100e4000 0x1000>;
> +		interrupts = <0 48 4>,
> +			     <0 49 4>;
> +	};
> +
> +	watchdog@100e5000 {
> +		compatible = "arm,sp805", "arm,primecell";
> +		reg = <0x100e5000 0x1000>;
> +		interrupts = <0 51 4>;
> +	};
> +
> +	gic: interrupt-controller@1e001000 {
> +		compatible = "arm,cortex-a9-gic";
> +		#interrupt-cells = <3>;
> +		#address-cells = <0>;
> +		interrupt-controller;
> +		reg = <0x1e001000 0x1000>,
> +		      <0x1e000100 0x100>;
> +	};
> +
> +	L2: cache-controller@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"
> diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
> index d1b0f35..6b79400 100644
> --- a/arch/arm/mach-vexpress/Kconfig
> +++ b/arch/arm/mach-vexpress/Kconfig
> @@ -22,4 +22,19 @@ config ARCH_VEXPRESS_DT
>  	bool
>  	select OF
>  
> +config ARCH_VEXPRESS_V2P_CA5S_CA9

You don't expect this to expand to A7 or A15? Why is ARCH_VEXPRESS_DT
not sufficient?

Rob

> +	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
> diff --git a/arch/arm/mach-vexpress/Makefile b/arch/arm/mach-vexpress/Makefile
> index 90551b9..06e3687 100644
> --- a/arch/arm/mach-vexpress/Makefile
> +++ b/arch/arm/mach-vexpress/Makefile
> @@ -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
> diff --git a/arch/arm/mach-vexpress/v2p-ca5s_ca9.c b/arch/arm/mach-vexpress/v2p-ca5s_ca9.c
> new file mode 100644
> index 0000000..0f1cd9b
> --- /dev/null
> +++ b/arch/arm/mach-vexpress/v2p-ca5s_ca9.c
> @@ -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

Re: [PATCH 5/5] ARM: vexpress: DT-based support for CoreTiles Express A5x2 and A9x4

[Pawel Moll]

> > +     dmc@2a150000 {
> I missed this earlier, but the preferred generic name is
> "memory-controller"
> > +     smc@2a190000 {
> same here.
>
Ok.

> > +config ARCH_VEXPRESS_V2P_CA5S_CA9
>
> You don't expect this to expand to A7 or A15?

A7/A15's SCUs are CP15 devices, not memory mapped ones. The same with
timers - A9-like TWD is no more, "taken over" by architected timers.
It's a different initialisation and SMP code, essentially. I wish it
wasn't... Also, in future I may be working on adding support for
MMU-less R-Class processors, which are going to be very very
different ;-)

Cheers!

Paweł

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/5] ARM: vexpress: DT-based support for CoreTiles Express A5x2 and A9x4

[Dave Martin]

On Fri, Nov 11, 2011 at 06:27:06PM +0000, Pawel Moll wrote:
> 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 <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> ---
>  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
> 
> diff --git a/arch/arm/boot/dts/vexpress-v2p-ca5s.dts b/arch/arm/boot/dts/vexpress-v2p-ca5s.dts

[...]

> +/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.

[...]

> diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
> index d1b0f35..6b79400 100644
> --- a/arch/arm/mach-vexpress/Kconfig
> +++ b/arch/arm/mach-vexpress/Kconfig
> @@ -22,4 +22,19 @@ config ARCH_VEXPRESS_DT
>  	bool
>  	select OF
>  
> +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"

> +	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.

> +
>  endmenu
> diff --git a/arch/arm/mach-vexpress/Makefile b/arch/arm/mach-vexpress/Makefile
> index 90551b9..06e3687 100644
> --- a/arch/arm/mach-vexpress/Makefile
> +++ b/arch/arm/mach-vexpress/Makefile
> @@ -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
> diff --git a/arch/arm/mach-vexpress/v2p-ca5s_ca9.c b/arch/arm/mach-vexpress/v2p-ca5s_ca9.c

[...]

> +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.

Cheers
---Dave

Re: [PATCH 5/5] ARM: vexpress: DT-based support for CoreTiles Express A5x2 and A9x4

[Pawel Moll]

On Wed, 2011-11-16 at 15:36 +0000, Dave Martin wrote:
> > +/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

> > +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.

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

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

> > +	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.

Same here - will copy this :-)

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

> >  endmenu
> > diff --git a/arch/arm/mach-vexpress/Makefile b/arch/arm/mach-vexpress/Makefile
> > index 90551b9..06e3687 100644
> > --- a/arch/arm/mach-vexpress/Makefile
> > +++ b/arch/arm/mach-vexpress/Makefile
> > @@ -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
> > diff --git a/arch/arm/mach-vexpress/v2p-ca5s_ca9.c b/arch/arm/mach-vexpress/v2p-ca5s_ca9.c
> 
> [...]
> 
> > +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.

Cheers!

Paweł



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/5] ARM: vexpress: DT-based support for CoreTiles Express A5x2 and A9x4

[Dave Martin]

On Wed, Nov 16, 2011 at 04:22:39PM +0000, Pawel Moll wrote:
> On Wed, 2011-11-16 at 15:36 +0000, Dave Martin wrote:
> > > +/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

Ok, I guess that's correct, then.

> > > +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.

As a newcomer to the Linux kernel a while ago, I found the plethora of
rather descriptive config help text really helpful.


These days, there are too many items (particulary from vendor trees)
where the help text, if present at all, is something like:

config BLAG_SLAD_BUP
	help
	  Enable the BLAG slad bup


or

config SCARY_UNSAFE_ONLY_ENABLE_IF_YOU_KNOW_WHAT_IT_DOES

	(no help to explain what the option does)

or examples such as the RCU subsystem, where although the invididual
items are reasonably documented, there is nowhere a statement of
what RCU does, why you want it, or even what it stands for.  Of
course, it's obvious to people who already know what it is, but
those aren't the people who documentation is written for.



I think we should do our best to sustain the quality, especially for
early examples of particular features which may get used as exemplars.

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

I know -- I did say it was a pedantic point.  Change that or not, as you
wish.
 
> > 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 :-)
> 
> > > +	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.
> 
> Same here - will copy this :-)
> 
> > 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 ;-)
> 
> > >  endmenu
> > > diff --git a/arch/arm/mach-vexpress/Makefile b/arch/arm/mach-vexpress/Makefile
> > > index 90551b9..06e3687 100644
> > > --- a/arch/arm/mach-vexpress/Makefile
> > > +++ b/arch/arm/mach-vexpress/Makefile
> > > @@ -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
> > > diff --git a/arch/arm/mach-vexpress/v2p-ca5s_ca9.c b/arch/arm/mach-vexpress/v2p-ca5s_ca9.c
> > 
> > [...]
> > 
> > > +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.

Happy to go with your judgment on that.

Cheers
---Dave

Re: [PATCH 0/5] Versatile Express DT support, take 2

[Dave Martin]

On Fri, Nov 11, 2011 at 06:27:01PM +0000, Pawel Moll wrote:
> 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 ;-)

I haven't actually tried this code out yet, but I'm broadly in
agreement.

I'm happy for this series to replace my previously posted
series, since it builds on common changes.

I have a few comments on the detail -- see separate mails.

Cheers
---Dave