[PATCH v2 00/12] Add Hisilicon HiP04 SoC

[PATCH v2 00/12] Add Hisilicon HiP04 SoC

[Haojian Zhuang]

Changelog:
v2:
  * Append ARCH_MULTI_V7_LPAE configuration. Define ARCH_MULTI_V7 to only
    support non-LPAE platform.
  * Append document of DT supporting.
  * Append ARCH_HISI in hi3xxx_defconfig.
  * Enable errata 798181 for HiP04 SoC.
  * Add PMU support.

Haojian Zhuang (10):
  ARM: debug: add HiP04 debug uart
  ARM: append ARCH_MULTI_V7_LPAE
  ARM: hisi: add ARCH_HISI
  irq: gic: use mask field in GICC_IAR
  irq: gic: extends the cpu interface to 16
  ARM: mcpm: change max clusters to 4
  ARM: hisi: add hip04 SoC support
  ARM: dts: add hip04-d01 dts file
  ARM: config: append hip04_defconfig
  ARM: config: select ARCH_HISI in hi3xxx_defconfig

Kefeng Wang (2):
  ARM: hisi: enable erratum 798181 of A15 on HiP04
  ARM: dts: Add PMU support in HiP04

 Documentation/devicetree/bindings/arm/gic.txt      |   1 +
 .../bindings/arm/hisilicon/hisilicon.txt           |  12 +
 .../devicetree/bindings/clock/hip04-clock.txt      |  20 ++
 arch/arm/Kconfig                                   |  11 +-
 arch/arm/Kconfig.debug                             |  10 +
 arch/arm/Makefile                                  |   2 +-
 arch/arm/boot/dts/Makefile                         |   1 +
 arch/arm/boot/dts/hip04-d01.dts                    |  74 ++++++
 arch/arm/boot/dts/hip04.dtsi                       | 260 +++++++++++++++++++++
 arch/arm/configs/hi3xxx_defconfig                  |   1 +
 arch/arm/configs/hip04_defconfig                   |  67 ++++++
 arch/arm/include/asm/mcpm.h                        |   2 +-
 arch/arm/mach-hisi/Kconfig                         |  24 +-
 arch/arm/mach-hisi/core.h                          |   6 +
 arch/arm/mach-hisi/hisilicon.c                     |  19 ++
 arch/arm/mach-hisi/platsmp.c                       | 259 ++++++++++++++++++++
 drivers/irqchip/irq-gic.c                          |  80 ++++---
 include/linux/irqchip/arm-gic.h                    |   2 +
 18 files changed, 813 insertions(+), 38 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/hip04-clock.txt
 create mode 100644 arch/arm/boot/dts/hip04-d01.dts
 create mode 100644 arch/arm/boot/dts/hip04.dtsi
 create mode 100644 arch/arm/configs/hip04_defconfig

-- 
1.8.3.2

[PATCH v2 01/12] ARM: debug: add HiP04 debug uart

[Haojian Zhuang]

Add the support of Hisilicon HiP04 debug uart.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 arch/arm/Kconfig.debug | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 7289853..fa1ebd5 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -223,6 +223,14 @@ choice
 		  Say Y here if you want kernel low-level debugging support
 		  on HI3716 UART.
 
+	config DEBUG_HIP04_UART
+		bool "Hisilicon HiP04 Debug UART"
+		depends on ARCH_HIP04
+		select DEBUG_UART_8250
+		help
+		  Say Y here if you want kernel low-level debugging support
+		  on HIP04 UART.
+
 	config DEBUG_HIGHBANK_UART
 		bool "Kernel low-level debugging messages via Highbank UART"
 		depends on ARCH_HIGHBANK
@@ -1053,6 +1061,7 @@ config DEBUG_UART_PHYS
 	default 0xd4017000 if DEBUG_MMP_UART2
 	default 0xd4018000 if DEBUG_MMP_UART3
 	default 0xe0000000 if ARCH_SPEAR13XX
+	default 0xe4007000 if DEBUG_HIP04_UART
 	default 0xf0000be0 if ARCH_EBSA110
 	default 0xf1012000 if DEBUG_MVEBU_UART_ALTERNATE
 	default 0xf1012000 if ARCH_DOVE || ARCH_KIRKWOOD || ARCH_MV78XX0 || \
@@ -1085,6 +1094,7 @@ config DEBUG_UART_VIRT
 	default 0xf4090000 if ARCH_LPC32XX
 	default 0xf4200000 if ARCH_GEMINI
 	default 0xf7fc9000 if DEBUG_BERLIN_UART
+	default 0xf8007000 if DEBUG_HIP04_UART
 	default 0xf8009000 if DEBUG_VEXPRESS_UART0_CA9
 	default 0xf8090000 if DEBUG_VEXPRESS_UART0_RS1
 	default 0xfb009000 if DEBUG_REALVIEW_STD_PORT
-- 
1.8.3.2

[PATCH v2 02/12] ARM: append ARCH_MULTI_V7_LPAE

[Haojian Zhuang]

Add new ARCH_MULTI_V7_LPAE config. It will select ARM_LPAE only both
ARCH_MULTI_V6 & ARCH_MULTI_V7 is disabled.

ARCH_MULTI_V7 means non-LPAE platform.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 arch/arm/Kconfig | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 5248923..a8b2b45 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -918,12 +918,19 @@ config ARCH_MULTI_V6
 	select CPU_V6K
 
 config ARCH_MULTI_V7
-	bool "ARMv7 based platforms (Cortex-A, PJ4, Scorpion, Krait)"
+	bool "ARMv7 non-LPAE based platforms (Cortex-A, PJ4, Scorpion, Krait)"
 	default y
 	select ARCH_MULTI_V6_V7
 	select CPU_V7
 	select HAVE_SMP
 
+config ARCH_MULTI_V7_LPAE
+	bool "ARMv7 LPAE based platforms (HIP04)"
+	select ARCH_MULTI_V6_V7
+	select CPU_V7
+	select HAVE_SMP
+	select ARM_LPAE if !(ARCH_MULTI_V6 || ARCH_MULTI_V7)
+
 config ARCH_MULTI_V6_V7
 	bool
 	select MIGHT_HAVE_CACHE_L2X0
-- 
1.8.3.2

[PATCH v2 03/12] ARM: hisi: add ARCH_HISI

[Haojian Zhuang]

Since multiple ARCH configuration will be appended into mach-hisi
directory, add ARCH_HISI as common configuration for different ARCH in
mach-hisi.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 arch/arm/Makefile          |  2 +-
 arch/arm/mach-hisi/Kconfig | 16 ++++++++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 41c1931..4c2798a 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -154,7 +154,7 @@ machine-$(CONFIG_ARCH_EP93XX)		+= ep93xx
 machine-$(CONFIG_ARCH_EXYNOS)		+= exynos
 machine-$(CONFIG_ARCH_GEMINI)		+= gemini
 machine-$(CONFIG_ARCH_HIGHBANK)		+= highbank
-machine-$(CONFIG_ARCH_HI3xxx)		+= hisi
+machine-$(CONFIG_ARCH_HISI)		+= hisi
 machine-$(CONFIG_ARCH_INTEGRATOR)	+= integrator
 machine-$(CONFIG_ARCH_IOP13XX)		+= iop13xx
 machine-$(CONFIG_ARCH_IOP32X)		+= iop32x
diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
index feee4db..da16efd 100644
--- a/arch/arm/mach-hisi/Kconfig
+++ b/arch/arm/mach-hisi/Kconfig
@@ -1,8 +1,16 @@
-config ARCH_HI3xxx
-	bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
+config ARCH_HISI
+	bool "Hisilicon SoC Support"
+	depends on ARCH_MULTIPLATFORM
 	select ARM_AMBA
 	select ARM_GIC
 	select ARM_TIMER_SP804
+
+if ARCH_HISI
+
+menu "Hisilicon platform type"
+
+config ARCH_HI3xxx
+	bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
 	select CACHE_L2X0
 	select HAVE_ARM_SCU if SMP
 	select HAVE_ARM_TWD if SMP
@@ -10,3 +18,7 @@ config ARCH_HI3xxx
 	select PINCTRL_SINGLE
 	help
 	  Support for Hisilicon Hi36xx/Hi37xx processor family
+
+endmenu
+
+endif
-- 
1.8.3.2

[PATCH v2 04/12] irq: gic: use mask field in GICC_IAR

[Haojian Zhuang]

Bit[9:0] is interrupt ID field in GICC_IAR. Bit[12:10] is CPU ID field,
and others are reserved.

So we should use GICC_IAR_INTID to get interrupt ID. It's not a good way
to use ~0x1c00 (CPU ID field) to get interrupt ID.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 drivers/irqchip/irq-gic.c       | 2 +-
 include/linux/irqchip/arm-gic.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 4300b66..8fd27bf 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -287,7 +287,7 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
 
 	do {
 		irqstat = readl_relaxed(cpu_base + GIC_CPU_INTACK);
-		irqnr = irqstat & ~0x1c00;
+		irqnr = irqstat & GICC_IAR_INTID;
 
 		if (likely(irqnr > 15 && irqnr < 1021)) {
 			irqnr = irq_find_mapping(gic->domain, irqnr);
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 7ed92d0..55933aa 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -21,6 +21,8 @@
 #define GIC_CPU_ACTIVEPRIO		0xd0
 #define GIC_CPU_IDENT			0xfc
 
+#define GICC_IAR_INTID			0x3ff
+
 #define GIC_DIST_CTRL			0x000
 #define GIC_DIST_CTR			0x004
 #define GIC_DIST_IGROUP			0x080
-- 
1.8.3.2

[PATCH v2 05/12] irq: gic: extends the cpu interface to 16

[Haojian Zhuang]

In order to support 16 CPUs, Hisilicon extends the GIC to support the
number of CPU interfaces from 8 to 16.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 drivers/irqchip/irq-gic.c | 78 ++++++++++++++++++++++++++++-------------------
 1 file changed, 47 insertions(+), 31 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 8fd27bf..44eff46 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -76,9 +76,12 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock);
  * The GIC mapping of CPU interfaces does not necessarily match
  * the logical CPU numbering.  Let's use a mapping as returned
  * by the GIC itself.
+ *
+ * Hisilicon HiP04 extends the number of CPU interface from 8 to 16.
  */
-#define NR_GIC_CPU_IF 8
-static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
+#define MAX_NR_GIC_CPU_IF 16
+static u16 gic_cpu_map[MAX_NR_GIC_CPU_IF] __read_mostly;
+static int nr_gic_cpu_if = 8;	/* The standard GIC supports 8 CPUs */
 
 /*
  * Supported arch specific GIC irq extension.
@@ -245,16 +248,19 @@ static int gic_retrigger(struct irq_data *d)
 static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
 			    bool force)
 {
-	void __iomem *reg = gic_dist_base(d) + GIC_DIST_TARGET + (gic_irq(d) & ~3);
-	unsigned int shift = (gic_irq(d) % 4) * 8;
+	void __iomem *reg;
+	unsigned int shift, step;
 	unsigned int cpu = cpumask_any_and(mask_val, cpu_online_mask);
 	u32 val, mask, bit;
-
-	if (cpu >= NR_GIC_CPU_IF || cpu >= nr_cpu_ids)
+	if (cpu >= nr_gic_cpu_if || cpu >= nr_cpu_ids)
 		return -EINVAL;
 
+	step = BITS_PER_LONG / nr_gic_cpu_if;
+	shift = (gic_irq(d) % step) * nr_gic_cpu_if;
+	reg = gic_dist_base(d) + GIC_DIST_TARGET + (gic_irq(d) / step * 4);
+
 	raw_spin_lock(&irq_controller_lock);
-	mask = 0xff << shift;
+	mask = ((1 << nr_gic_cpu_if) - 1) << shift;
 	bit = gic_cpu_map[cpu] << shift;
 	val = readl_relaxed(reg) & ~mask;
 	writel_relaxed(val | bit, reg);
@@ -354,15 +360,17 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
 	irq_set_chained_handler(irq, gic_handle_cascade_irq);
 }
 
-static u8 gic_get_cpumask(struct gic_chip_data *gic)
+static u16 gic_get_cpumask(struct gic_chip_data *gic)
 {
 	void __iomem *base = gic_data_dist_base(gic);
-	u32 mask, i;
-
-	for (i = mask = 0; i < 32; i += 4) {
-		mask = readl_relaxed(base + GIC_DIST_TARGET + i);
-		mask |= mask >> 16;
-		mask |= mask >> 8;
+	u32 mask, i, j, step;
+
+	/* get the number of CPU fields in GIC_DIST_TARGET register */
+	step = BITS_PER_LONG / nr_gic_cpu_if;
+	for (i = mask = 0; i < 32; i += step) {
+		mask = readl_relaxed(base + GIC_DIST_TARGET + i / step * 4);
+		for (j = BITS_PER_LONG >> 1; j >= nr_gic_cpu_if; j >>= 1)
+			mask |= mask >> j;
 		if (mask)
 			break;
 	}
@@ -375,7 +383,7 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic)
 
 static void __init gic_dist_init(struct gic_chip_data *gic)
 {
-	unsigned int i;
+	unsigned int i, step;
 	u32 cpumask;
 	unsigned int gic_irqs = gic->gic_irqs;
 	void __iomem *base = gic_data_dist_base(gic);
@@ -392,10 +400,11 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
 	 * Set all global interrupts to this CPU only.
 	 */
 	cpumask = gic_get_cpumask(gic);
-	cpumask |= cpumask << 8;
-	cpumask |= cpumask << 16;
-	for (i = 32; i < gic_irqs; i += 4)
-		writel_relaxed(cpumask, base + GIC_DIST_TARGET + i * 4 / 4);
+	for (i = nr_gic_cpu_if; i < BITS_PER_LONG; i <<= 1)
+		cpumask |= cpumask << i;
+	step = BITS_PER_LONG / nr_gic_cpu_if;
+	for (i = 32; i < gic_irqs; i += step)
+		writel_relaxed(cpumask, base + GIC_DIST_TARGET + i / step * 4);
 
 	/*
 	 * Set priority on all global interrupts.
@@ -423,7 +432,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 	/*
 	 * Get what the GIC says our CPU mask is.
 	 */
-	BUG_ON(cpu >= NR_GIC_CPU_IF);
+	BUG_ON(cpu >= nr_gic_cpu_if);
 	cpu_mask = gic_get_cpumask(gic);
 	gic_cpu_map[cpu] = cpu_mask;
 
@@ -431,7 +440,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 	 * Clear our mask from the other map entries in case they're
 	 * still undefined.
 	 */
-	for (i = 0; i < NR_GIC_CPU_IF; i++)
+	for (i = 0; i < nr_gic_cpu_if; i++)
 		if (i != cpu)
 			gic_cpu_map[i] &= ~cpu_mask;
 
@@ -469,7 +478,7 @@ static void gic_dist_save(unsigned int gic_nr)
 {
 	unsigned int gic_irqs;
 	void __iomem *dist_base;
-	int i;
+	int i, step;
 
 	if (gic_nr >= MAX_GIC_NR)
 		BUG();
@@ -484,7 +493,8 @@ static void gic_dist_save(unsigned int gic_nr)
 		gic_data[gic_nr].saved_spi_conf[i] =
 			readl_relaxed(dist_base + GIC_DIST_CONFIG + i * 4);
 
-	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
+	step = BITS_PER_LONG / nr_gic_cpu_if;
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, step); i++)
 		gic_data[gic_nr].saved_spi_target[i] =
 			readl_relaxed(dist_base + GIC_DIST_TARGET + i * 4);
 
@@ -503,7 +513,7 @@ static void gic_dist_save(unsigned int gic_nr)
 static void gic_dist_restore(unsigned int gic_nr)
 {
 	unsigned int gic_irqs;
-	unsigned int i;
+	unsigned int i, step;
 	void __iomem *dist_base;
 
 	if (gic_nr >= MAX_GIC_NR)
@@ -525,7 +535,8 @@ static void gic_dist_restore(unsigned int gic_nr)
 		writel_relaxed(0xa0a0a0a0,
 			dist_base + GIC_DIST_PRI + i * 4);
 
-	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
+	step = BITS_PER_LONG / nr_gic_cpu_if;
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, step); i++)
 		writel_relaxed(gic_data[gic_nr].saved_spi_target[i],
 			dist_base + GIC_DIST_TARGET + i * 4);
 
@@ -666,8 +677,8 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 	dmb(ishst);
 
 	/* this always happens on GIC0 */
-	writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
-
+	writel_relaxed(map << (8 + 16 - nr_gic_cpu_if) | irq,
+		       gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
 	raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
 }
 #endif
@@ -681,7 +692,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
  */
 void gic_send_sgi(unsigned int cpu_id, unsigned int irq)
 {
-	BUG_ON(cpu_id >= NR_GIC_CPU_IF);
+	BUG_ON(cpu_id >= nr_gic_cpu_if);
 	cpu_id = 1 << cpu_id;
 	/* this always happens on GIC0 */
 	writel_relaxed((cpu_id << 16) | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
@@ -700,7 +711,7 @@ int gic_get_cpu_id(unsigned int cpu)
 {
 	unsigned int cpu_bit;
 
-	if (cpu >= NR_GIC_CPU_IF)
+	if (cpu >= nr_gic_cpu_if)
 		return -1;
 	cpu_bit = gic_cpu_map[cpu];
 	if (cpu_bit & (cpu_bit - 1))
@@ -971,8 +982,8 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
 	 * Initialize the CPU interface map to all CPUs.
 	 * It will be refined as each CPU probes its ID.
 	 */
-	for (i = 0; i < NR_GIC_CPU_IF; i++)
-		gic_cpu_map[i] = 0xff;
+	for (i = 0; i < nr_gic_cpu_if; i++)
+		gic_cpu_map[i] = (1 << MAX_NR_GIC_CPU_IF) - 1;
 
 	/*
 	 * For primary GICs, skip over SGIs.
@@ -1047,6 +1058,10 @@ gic_of_init(struct device_node *node, struct device_node *parent)
 	if (WARN_ON(!node))
 		return -ENODEV;
 
+	/* HiP04 supports 16 CPUs@most */
+	if (of_device_is_compatible(node, "hisilicon,hip04-gic"))
+		nr_gic_cpu_if = 16;
+
 	dist_base = of_iomap(node, 0);
 	WARN(!dist_base, "unable to map gic dist registers\n");
 
@@ -1069,6 +1084,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
 }
 IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
 IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
+IRQCHIP_DECLARE(hip04_gic, "hisilicon,hip04-gic", gic_of_init);
 IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
 IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
 
-- 
1.8.3.2

[PATCH v2 06/12] ARM: mcpm: change max clusters to 4

[Haojian Zhuang]

In order to support 4 clusters with 4 Cortex A15 Cores in each cluster,
enlarge maximum clusters from 2 to 4 in MCPM.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 arch/arm/include/asm/mcpm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/mcpm.h b/arch/arm/include/asm/mcpm.h
index 608516e..68f82cf 100644
--- a/arch/arm/include/asm/mcpm.h
+++ b/arch/arm/include/asm/mcpm.h
@@ -20,7 +20,7 @@
  * to consider dynamic allocation.
  */
 #define MAX_CPUS_PER_CLUSTER	4
-#define MAX_NR_CLUSTERS		2
+#define MAX_NR_CLUSTERS		4
 
 #ifndef __ASSEMBLY__
 
-- 
1.8.3.2

[PATCH v2 07/12] ARM: hisi: add hip04 SoC support

[Haojian Zhuang]

Hisilicon Hi3xxx is based on Cortex A9 Core. Now HiP04 SoC is based on
Cortex A15 Core. Since multiple clusters is used in HiP04 SoC, it could
be based on MCPM.

And HiP04 supports LPAE to support large memory.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 arch/arm/Kconfig               |   2 +-
 arch/arm/mach-hisi/Kconfig     |   7 ++
 arch/arm/mach-hisi/core.h      |   6 +
 arch/arm/mach-hisi/hisilicon.c |  19 +++
 arch/arm/mach-hisi/platsmp.c   | 259 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 292 insertions(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a8b2b45..6af6609 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1113,7 +1113,7 @@ source arch/arm/mm/Kconfig
 
 config ARM_NR_BANKS
 	int
-	default 16 if ARCH_EP93XX
+	default 16 if ARCH_EP93XX || ARCH_HIP04
 	default 8
 
 config IWMMXT
diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
index da16efd..4dd966a 100644
--- a/arch/arm/mach-hisi/Kconfig
+++ b/arch/arm/mach-hisi/Kconfig
@@ -19,6 +19,13 @@ config ARCH_HI3xxx
 	help
 	  Support for Hisilicon Hi36xx/Hi37xx processor family
 
+config ARCH_HIP04
+	bool "Hisilicon HiP04 Cortex A15 family" if ARCH_MULTI_V7_LPAE
+	select HAVE_ARM_ARCH_TIMER
+	select MCPM if SMP
+	help
+	  Support for Hisilicon HiP04 processor family
+
 endmenu
 
 endif
diff --git a/arch/arm/mach-hisi/core.h b/arch/arm/mach-hisi/core.h
index af23ec2..e008c7a 100644
--- a/arch/arm/mach-hisi/core.h
+++ b/arch/arm/mach-hisi/core.h
@@ -12,4 +12,10 @@ extern void hi3xxx_cpu_die(unsigned int cpu);
 extern int hi3xxx_cpu_kill(unsigned int cpu);
 extern void hi3xxx_set_cpu(int cpu, bool enable);
 
+#define HIP04_BOOTWRAPPER_PHYS		0x10c00000
+#define HIP04_BOOTWRAPPER_MAGIC		0xa5a5a5a5
+#define HIP04_BOOTWRAPPER_SIZE		0x00010000
+
+extern bool __init hip04_smp_init_ops(void);
+
 #endif
diff --git a/arch/arm/mach-hisi/hisilicon.c b/arch/arm/mach-hisi/hisilicon.c
index 741faf3..10a605f 100644
--- a/arch/arm/mach-hisi/hisilicon.c
+++ b/arch/arm/mach-hisi/hisilicon.c
@@ -14,6 +14,7 @@
 #include <linux/clk-provider.h>
 #include <linux/clocksource.h>
 #include <linux/irqchip.h>
+#include <linux/memblock.h>
 #include <linux/of_address.h>
 #include <linux/of_platform.h>
 
@@ -88,3 +89,21 @@ DT_MACHINE_START(HI3620, "Hisilicon Hi3620 (Flattened Device Tree)")
 	.smp		= smp_ops(hi3xxx_smp_ops),
 	.restart	= hi3xxx_restart,
 MACHINE_END
+
+static const char *hip04_compat[] __initconst = {
+	"hisilicon,hip04-d01",
+	NULL,
+};
+
+static void __init hip04_reserve(void)
+{
+	memblock_reserve(HIP04_BOOTWRAPPER_PHYS, HIP04_BOOTWRAPPER_SIZE);
+}
+
+DT_MACHINE_START(HIP04, "Hisilicon HiP04 (Flattened Device Tree)")
+	.dt_compat	= hip04_compat,
+#ifdef CONFIG_MCPM
+	.smp_init	= smp_init_ops(hip04_smp_init_ops),
+#endif
+	.reserve	= hip04_reserve,
+MACHINE_END
diff --git a/arch/arm/mach-hisi/platsmp.c b/arch/arm/mach-hisi/platsmp.c
index 471f1ee..3a5833f 100644
--- a/arch/arm/mach-hisi/platsmp.c
+++ b/arch/arm/mach-hisi/platsmp.c
@@ -9,9 +9,13 @@
  */
 #include <linux/smp.h>
 #include <linux/io.h>
+#include <linux/irqchip/arm-gic.h>
 #include <linux/of_address.h>
 
 #include <asm/cacheflush.h>
+#include <asm/cp15.h>
+#include <asm/cputype.h>
+#include <asm/mcpm.h>
 #include <asm/smp_plat.h>
 #include <asm/smp_scu.h>
 
@@ -87,3 +91,258 @@ struct smp_operations hi3xxx_smp_ops __initdata = {
 	.cpu_kill		= hi3xxx_cpu_kill,
 #endif
 };
+
+#ifdef CONFIG_MCPM
+/* bits definition in SC_CPU_RESET_REQ[x]/SC_CPU_RESET_DREQ[x]
+ * 1 -- unreset; 0 -- reset
+ */
+#define CORE_RESET_BIT(x)		(1 << x)
+#define NEON_RESET_BIT(x)		(1 << (x + 4))
+#define CORE_DEBUG_RESET_BIT(x)		(1 << (x + 9))
+#define CLUSTER_L2_RESET_BIT		(1 << 8)
+#define CLUSTER_DEBUG_RESET_BIT		(1 << 13)
+
+/*
+ * bits definition in SC_CPU_RESET_STATUS[x]
+ * 1 -- reset status; 0 -- unreset status
+ */
+#define CORE_RESET_STATUS(x)		(1 << x)
+#define NEON_RESET_STATUS(x)		(1 << (x + 4))
+#define CORE_DEBUG_RESET_STATUS(x)	(1 << (x + 9))
+#define CLUSTER_L2_RESET_STATUS		(1 << 8)
+#define CLUSTER_DEBUG_RESET_STATUS	(1 << 13)
+#define CORE_WFI_STATUS(x)		(1 << (x + 16))
+#define CORE_WFE_STATUS(x)		(1 << (x + 20))
+#define CORE_DEBUG_ACK(x)		(1 << (x + 24))
+
+#define SC_CPU_RESET_REQ(x)		(0x520 + (x << 3))	/* reset */
+#define SC_CPU_RESET_DREQ(x)		(0x524 + (x << 3))	/* unreset */
+#define SC_CPU_RESET_STATUS(x)		(0x1520 + (x << 3))
+
+#define FAB_SF_MODE			0x0c
+#define FAB_SF_INVLD			0x10
+
+/* bits definition in FB_SF_INVLD */
+#define FB_SF_INVLD_START		(1 << 8)
+
+#define HIP04_MAX_CLUSTERS		4
+#define HIP04_MAX_CPUS_PER_CLUSTER	4
+
+static void __iomem *relocation, *sysctrl, *fabric;
+static int hip04_cpu_table[HIP04_MAX_CLUSTERS][HIP04_MAX_CPUS_PER_CLUSTER];
+static DEFINE_SPINLOCK(boot_lock);
+
+static bool hip04_cluster_down(unsigned int cluster)
+{
+	int i;
+
+	for (i = 0; i < HIP04_MAX_CPUS_PER_CLUSTER; i++)
+		if (hip04_cpu_table[cluster][i])
+			return false;
+	return true;
+}
+
+static void hip04_set_snoop_filter(unsigned int cluster, unsigned int on)
+{
+	unsigned long data;
+
+	if (!fabric)
+		return;
+	data = readl_relaxed(fabric + FAB_SF_MODE);
+	if (on)
+		data |= 1 << cluster;
+	else
+		data &= ~(1 << cluster);
+	writel_relaxed(data, fabric + FAB_SF_MODE);
+	while (1) {
+		if (data == readl_relaxed(fabric + FAB_SF_MODE))
+			break;
+	}
+}
+
+static int hip04_mcpm_power_up(unsigned int cpu, unsigned int cluster)
+{
+	unsigned long data, mask;
+
+	if (!relocation || !sysctrl)
+		return -ENODEV;
+	if (cluster >= HIP04_MAX_CLUSTERS || cpu >= HIP04_MAX_CPUS_PER_CLUSTER)
+		return -EINVAL;
+
+	spin_lock(&boot_lock);
+	writel_relaxed(HIP04_BOOTWRAPPER_PHYS, relocation);
+	writel_relaxed(HIP04_BOOTWRAPPER_MAGIC, relocation + 4);
+	writel_relaxed(virt_to_phys(mcpm_entry_point), relocation + 8);
+	writel_relaxed(0, relocation + 12);
+
+	if (hip04_cluster_down(cluster)) {
+		data = CLUSTER_L2_RESET_BIT | CLUSTER_DEBUG_RESET_BIT;
+		writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
+		do {
+			mask = CLUSTER_L2_RESET_STATUS | \
+			       CLUSTER_DEBUG_RESET_STATUS;
+			data = readl_relaxed(sysctrl + \
+					     SC_CPU_RESET_STATUS(cluster));
+		} while (data & mask);
+		hip04_set_snoop_filter(cluster, 1);
+	}
+
+	hip04_cpu_table[cluster][cpu]++;
+
+	data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
+	       CORE_DEBUG_RESET_BIT(cpu);
+	writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
+	spin_unlock(&boot_lock);
+
+	return 0;
+}
+
+static void hip04_mcpm_power_down(void)
+{
+	unsigned int mpidr, cpu, cluster;
+	unsigned int v;
+
+	spin_lock(&boot_lock);
+	spin_unlock(&boot_lock);
+
+	mpidr = read_cpuid_mpidr();
+	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+	local_irq_disable();
+	gic_cpu_if_down();
+
+	__mcpm_cpu_down(cpu, cluster);
+
+	asm volatile(
+	"	mrc	p15, 0, %0, c1, c0, 0\n"
+	"	bic	%0, %0, %1\n"
+	"	mcr	p15, 0, %0, c1, c0, 0\n"
+	  : "=&r" (v)
+	  : "Ir" (CR_C)
+	  : "cc");
+
+	flush_cache_louis();
+
+	asm volatile(
+	/*
+	* Turn off coherency
+	*/
+	"	mrc	p15, 0, %0, c1, c0, 1\n"
+	"	bic	%0, %0, %1\n"
+	"	mcr	p15, 0, %0, c1, c0, 1\n"
+	: "=&r" (v)
+	: "Ir" (0x40)
+	: "cc");
+
+	isb();
+	dsb();
+}
+
+static int hip04_mcpm_power_down_finish(unsigned int cpu, unsigned int cluster)
+{
+	int ret = -EBUSY;
+
+	spin_lock(&boot_lock);
+	BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
+	__mcpm_cpu_going_down(cpu, cluster);
+
+	hip04_cpu_table[cluster][cpu]--;
+	if (hip04_cpu_table[cluster][cpu]) {
+		pr_err("Cluster %d CPU%d is still running\n", cluster, cpu);
+		goto out;
+	}
+	ret = 0;
+out:
+	spin_unlock(&boot_lock);
+	return ret;
+}
+
+static void hip04_mcpm_powered_up(void)
+{
+	if (!relocation)
+		return;
+	spin_lock(&boot_lock);
+	writel_relaxed(0, relocation);
+	writel_relaxed(0, relocation + 4);
+	writel_relaxed(0, relocation + 8);
+	writel_relaxed(0, relocation + 12);
+	spin_unlock(&boot_lock);
+}
+
+static const struct mcpm_platform_ops hip04_mcpm_ops = {
+	.power_up		= hip04_mcpm_power_up,
+	.power_down		= hip04_mcpm_power_down,
+	.power_down_finish	= hip04_mcpm_power_down_finish,
+	.powered_up		= hip04_mcpm_powered_up,
+};
+
+static bool __init hip04_cpu_table_init(void)
+{
+	unsigned int mpidr, cpu, cluster;
+
+	mpidr = read_cpuid_mpidr();
+	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+	if (cluster >= HIP04_MAX_CLUSTERS ||
+	    cpu >= HIP04_MAX_CPUS_PER_CLUSTER) {
+		pr_err("%s: boot CPU is out of bound!\n", __func__);
+		return false;
+	}
+	hip04_set_snoop_filter(cluster, 1);
+	hip04_cpu_table[cluster][cpu] = 1;
+	return true;
+}
+
+static int __init hip04_mcpm_init(void)
+{
+	struct device_node *np;
+	int ret = -ENODEV;
+
+	np = of_find_compatible_node(NULL, NULL, "hisilicon,hip04-mcpm");
+	if (!np) {
+		pr_err("failed to find hisilicon,hip04-mcpm node\n");
+		goto err;
+	}
+	relocation = of_iomap(np, 0);
+	if (!relocation) {
+		pr_err("failed to get relocation space\n");
+		ret = -ENOMEM;
+		goto err;
+	}
+	sysctrl = of_iomap(np, 1);
+	if (!sysctrl) {
+		pr_err("failed to get sysctrl base\n");
+		ret = -ENOMEM;
+		goto err_sysctrl;
+	}
+	fabric = of_iomap(np, 2);
+	if (!fabric) {
+		pr_err("failed to get fabric base\n");
+		ret = -ENOMEM;
+		goto err_fabric;
+	}
+	if (!hip04_cpu_table_init())
+		return -EINVAL;
+	ret = mcpm_platform_register(&hip04_mcpm_ops);
+	if (!ret) {
+		mcpm_sync_init(NULL);
+		pr_info("HiP04 MCPM initialized\n");
+	}
+	return ret;
+err_fabric:
+	iounmap(sysctrl);
+err_sysctrl:
+	iounmap(relocation);
+err:
+	return ret;
+}
+early_initcall(hip04_mcpm_init);
+
+bool __init hip04_smp_init_ops(void)
+{
+	mcpm_smp_set_ops();
+	return true;
+}
+#endif	/* CONFIG_MCPM */
-- 
1.8.3.2

[PATCH v2 08/12] ARM: dts: add hip04-d01 dts file

[Haojian Zhuang]

Add hip04.dtsi & hip04-d01.dts file to support HiP04 SoC platform.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 Documentation/devicetree/bindings/arm/gic.txt      |   1 +
 .../bindings/arm/hisilicon/hisilicon.txt           |  12 ++
 .../devicetree/bindings/clock/hip04-clock.txt      |  20 ++
 arch/arm/boot/dts/Makefile                         |   1 +
 arch/arm/boot/dts/hip04-d01.dts                    |  74 +++++++
 arch/arm/boot/dts/hip04.dtsi                       | 240 +++++++++++++++++++++
 6 files changed, 348 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/hip04-clock.txt
 create mode 100644 arch/arm/boot/dts/hip04-d01.dts
 create mode 100644 arch/arm/boot/dts/hip04.dtsi

diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
index 5573c08..150f7d6 100644
--- a/Documentation/devicetree/bindings/arm/gic.txt
+++ b/Documentation/devicetree/bindings/arm/gic.txt
@@ -16,6 +16,7 @@ Main node required properties:
 	"arm,cortex-a9-gic"
 	"arm,cortex-a7-gic"
 	"arm,arm11mp-gic"
+	"hisilicon,hip04-gic"
 - interrupt-controller : Identifies the node as an interrupt controller
 - #interrupt-cells : Specifies the number of cells needed to encode an
   interrupt source.  The type shall be a <u32> and the value shall be 3.
diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
index df0a452..47c0a13 100644
--- a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
+++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
@@ -4,6 +4,10 @@ Hisilicon Platforms Device Tree Bindings
 Hi4511 Board
 Required root node properties:
 	- compatible = "hisilicon,hi3620-hi4511";
+HiP04 D01 Board
+Required root node properties:
+	- compatible = "hisilicon,hip04-d01";
+
 
 Hisilicon system controller
 
@@ -31,6 +35,14 @@ Example:
 		reboot-offset = <0x4>;
 	};
 
+
+Hisilicon MCPM Implementation
+
+Required Properties:
+- compatible: "hisilicon,hip04-mcpm"
+- reg: Register address and size.
+
+
 PCTRL: Peripheral misc control register
 
 Required Properties:
diff --git a/Documentation/devicetree/bindings/clock/hip04-clock.txt b/Documentation/devicetree/bindings/clock/hip04-clock.txt
new file mode 100644
index 0000000..4d31ae3
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/hip04-clock.txt
@@ -0,0 +1,20 @@
+* Hisilicon HiP04 Clock Controller
+
+The HiP04 clock controller generates and supplies clock to various
+controllers within the HiP04 SoC.
+
+Required Properties:
+
+- compatible: should be one of the following.
+  - "hisilicon,hip04-clock" - controller compatible with HiP04 SoC.
+
+- reg: physical base address of the controller and length of memory mapped
+  region.
+
+- #clock-cells: should be 1.
+
+
+Each clock is assigned an identifier and client nodes use this identifier
+to specify the clock which they consume.
+
+All these identifier could be found in <dt-bindings/clock/hip04-clock.h>.
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 2145af6..d0cbc939 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -80,6 +80,7 @@ dtb-$(CONFIG_ARCH_EXYNOS) += exynos4210-origen.dtb \
 dtb-$(CONFIG_ARCH_HI3xxx) += hi3620-hi4511.dtb
 dtb-$(CONFIG_ARCH_HIGHBANK) += highbank.dtb \
 	ecx-2000.dtb
+dtb-$(CONFIG_ARCH_HIP04) += hip04-d01.dtb
 dtb-$(CONFIG_ARCH_INTEGRATOR) += integratorap.dtb \
 	integratorcp.dtb
 dtb-$(CONFIG_ARCH_KEYSTONE) += k2hk-evm.dtb \
diff --git a/arch/arm/boot/dts/hip04-d01.dts b/arch/arm/boot/dts/hip04-d01.dts
new file mode 100644
index 0000000..a10dcf3
--- /dev/null
+++ b/arch/arm/boot/dts/hip04-d01.dts
@@ -0,0 +1,74 @@
+/*
+ *  Copyright (C) 2013-2014 Linaro Ltd.
+ *  Author: Haojian Zhuang <haojian.zhuang@linaro.org>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  publishhed by the Free Software Foundation.
+ */
+
+/dts-v1/;
+
+#include "hip04.dtsi"
+
+/ {
+	/* memory bus is 64-bit */
+	#address-cells = <2>;
+	#size-cells = <1>;
+	model = "Hisilicon D01 Development Board";
+	compatible = "hisilicon,hip04-d01";
+
+	memory at 0 {
+		device_type = "memory";
+		/*
+		 * Bootloader loads kernel image into 0x1000_0000 region,
+		 * so disables the region between [0000_0000 - 1000_0000]
+		 * temporarily.
+		 * Because the PHYS_TO_VIRT_OFFSET is calculated based on
+		 * the original region that kenrel is loaded.
+		 * This workaround will be removed only after UEFI updated.
+		 */
+		reg = <0x00000000 0x10000000 0xc0000000>;
+	};
+
+	memory at 00000004c0000000 {
+		device_type = "memory";
+		reg = <0x00000004 0xc0000000 0x40000000>;
+	};
+
+	memory at 0000000500000000 {
+		device_type = "memory";
+		reg = <0x00000005 0x00000000 0x80000000>;
+	};
+
+	memory at 0000000580000000 {
+		device_type = "memory";
+		reg = <0x00000005 0x80000000 0x80000000>;
+	};
+
+	memory at 0000000600000000 {
+		device_type = "memory";
+		reg = <0x00000006 0x00000000 0x80000000>;
+	};
+
+	memory at 0000000680000000 {
+		device_type = "memory";
+		reg = <0x00000006 0x80000000 0x80000000>;
+	};
+
+	memory at 0000000700000000 {
+		device_type = "memory";
+		reg = <0x00000007 0x00000000 0x80000000>;
+	};
+
+	memory at 0000000780000000 {
+		device_type = "memory";
+		reg = <0x00000007 0x80000000 0x80000000>;
+	};
+
+	soc {
+		uart0: uart at 4007000 {
+			status = "ok";
+		};
+	};
+};
diff --git a/arch/arm/boot/dts/hip04.dtsi b/arch/arm/boot/dts/hip04.dtsi
new file mode 100644
index 0000000..eb5e5a2
--- /dev/null
+++ b/arch/arm/boot/dts/hip04.dtsi
@@ -0,0 +1,240 @@
+/*
+ * Hisilicon Ltd. HiP01 SoC
+ *
+ * Copyright (C) 2013-2014 Hisilicon Ltd.
+ * Copyright (C) 2013-2014 Linaro Ltd.
+ *
+ * Author: Haojian Zhuang <haojian.zhuang@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * publishhed by the Free Software Foundation.
+ */
+
+#include <dt-bindings/clock/hip04-clock.h>
+
+/ {
+	/* memory bus is 64-bit */
+	#address-cells = <2>;
+	#size-cells = <1>;
+
+	aliases {
+		serial0 = &uart0;
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&CPU0>;
+				};
+				core1 {
+					cpu = <&CPU1>;
+				};
+				core2 {
+					cpu = <&CPU2>;
+				};
+				core3 {
+					cpu = <&CPU3>;
+				};
+			};
+			cluster1 {
+				core0 {
+					cpu = <&CPU4>;
+				};
+				core1 {
+					cpu = <&CPU5>;
+				};
+				core2 {
+					cpu = <&CPU6>;
+				};
+				core3 {
+					cpu = <&CPU7>;
+				};
+			};
+			cluster2 {
+				core0 {
+					cpu = <&CPU8>;
+				};
+				core1 {
+					cpu = <&CPU9>;
+				};
+				core2 {
+					cpu = <&CPU10>;
+				};
+				core3 {
+					cpu = <&CPU11>;
+				};
+			};
+			cluster3 {
+				core0 {
+					cpu = <&CPU12>;
+				};
+				core1 {
+					cpu = <&CPU13>;
+				};
+				core2 {
+					cpu = <&CPU14>;
+				};
+				core3 {
+					cpu = <&CPU15>;
+				};
+			};
+		};
+		CPU0: cpu at 0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <0>;
+		};
+		CPU1: cpu at 1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <1>;
+		};
+		CPU2: cpu at 2 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <2>;
+		};
+		CPU3: cpu at 3 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <3>;
+		};
+		CPU4: cpu at 100 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <0x100>;
+			clock-frequency = <1350000000>;
+		};
+		CPU5: cpu at 101 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <0x101>;
+			clock-frequency = <1350000000>;
+		};
+		CPU6: cpu at 102 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <0x102>;
+			clock-frequency = <1350000000>;
+		};
+		CPU7: cpu at 103 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <0x103>;
+			clock-frequency = <1350000000>;
+		};
+		CPU8: cpu at 200 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <0x200>;
+			clock-frequency = <1350000000>;
+		};
+		CPU9: cpu at 201 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <0x201>;
+			clock-frequency = <1350000000>;
+		};
+		CPU10: cpu at 202 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <0x202>;
+			clock-frequency = <1350000000>;
+		};
+		CPU11: cpu at 203 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <0x203>;
+			clock-frequency = <1350000000>;
+		};
+		CPU12: cpu at 300 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <0x300>;
+			clock-frequency = <1350000000>;
+		};
+		CPU13: cpu at 301 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <0x301>;
+			clock-frequency = <1350000000>;
+		};
+		CPU14: cpu at 302 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <0x302>;
+			clock-frequency = <1350000000>;
+		};
+		CPU15: cpu at 303 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <0x303>;
+			clock-frequency = <1350000000>;
+		};
+	};
+
+	soc {
+		/* It's a 32-bit SoC. */
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "arm,amba-bus", "simple-bus";
+		device_type = "soc";
+		interrupt-parent = <&gic>;
+		ranges = <0 0 0xe0000000 0x10000000>;
+
+		gic: interrupt-controller at c01000 {
+			compatible = "hisilicon,hip04-gic";
+			#interrupt-cells = <3>;
+			#address-cells = <0>;
+			interrupt-controller;
+
+			/* gic dist base, gic cpu base */
+			reg = <0xc01000 0x1000>, <0xc02000 0x1000>;
+		};
+
+		mcpm: mcpm {
+			compatible = "hisilicon,hip04-mcpm";
+			reg = <0x100 0x1000>, <0x3e00000 0x00100000>,
+			      <0x302a000 0x1000>;
+		};
+
+		clock: clock {
+			compatible = "hisilicon,hip04-clock";
+			/* FIXME: the base of clock controller */
+			reg = <0 0x1000>;
+			#clock-cells = <1>;
+		};
+
+		dual_timer0: dual_timer at 3000000 {
+			compatible = "arm,sp804", "arm,primecell";
+			reg = <0x3000000 0x1000>;
+			interrupts = <0 224 4>;
+			clocks = <&clock HIP04_CLK_50M>;
+			clock-names = "apb_pclk";
+			status = "ok";
+		};
+
+		timer {
+			compatible = "arm,armv7-timer";
+			interrupts = <1 13 0xf08>,
+				     <1 14 0xf08>,
+				     <1 11 0xf08>,
+				     <1 10 0xf08>;
+		};
+
+		uart0: uart at 4007000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x4007000 0x1000>;
+			interrupts = <0 381 4>;
+			clocks = <&clock HIP04_CLK_168M>;
+			clock-names = "uartclk";
+			reg-shift = <2>;
+			status = "disabled";
+		};
+	};
+};
-- 
1.8.3.2

[PATCH v2 09/12] ARM: config: append hip04_defconfig

[Haojian Zhuang]

Select HiP04 SoC configuration by hip04_defconfig.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 arch/arm/configs/hip04_defconfig | 67 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 arch/arm/configs/hip04_defconfig

diff --git a/arch/arm/configs/hip04_defconfig b/arch/arm/configs/hip04_defconfig
new file mode 100644
index 0000000..b6fa635
--- /dev/null
+++ b/arch/arm/configs/hip04_defconfig
@@ -0,0 +1,67 @@
+CONFIG_SYSVIPC=y
+CONFIG_POSIX_MQUEUE=y
+CONFIG_IRQ_DOMAIN_DEBUG=y
+CONFIG_NO_HZ=y
+CONFIG_HIGH_RES_TIMERS=y
+CONFIG_BSD_PROCESS_ACCT=y
+CONFIG_BLK_DEV_INITRD=y
+CONFIG_RD_GZIP=y
+# CONFIG_ARCH_MULTI_V6 is not set
+# CONFIG_ARCH_MULTI_V7 is not set
+CONFIG_ARCH_MULTI_V7_LPAE=y
+CONFIG_ARCH_HISI=y
+CONFIG_ARCH_HIP04=y
+CONFIG_SMP=y
+CONFIG_NR_CPUS=16
+CONFIG_PREEMPT=y
+CONFIG_AEABI=y
+CONFIG_ARM_APPENDED_DTB=y
+CONFIG_ARM_ATAG_DTB_COMPAT=y
+CONFIG_ARM_ATAG_DTB_COMPAT_CMDLINE_FROM_BOOTLOADER=y
+CONFIG_HIGHMEM=y
+CONFIG_VFP=y
+CONFIG_NEON=y
+CONFIG_NET=y
+CONFIG_UNIX=y
+CONFIG_INET=y
+CONFIG_IP_PNP=y
+CONFIG_IP_PNP_DHCP=y
+CONFIG_DEVTMPFS=y
+CONFIG_DEVTMPFS_MOUNT=y
+CONFIG_BLK_DEV=y
+CONFIG_BLK_DEV_LOOP=y
+CONFIG_BLK_DEV_LOOP_MIN_COUNT=8
+CONFIG_BLK_DEV_RAM=y
+CONFIG_BLK_DEV_RAM_COUNT=16
+CONFIG_BLK_DEV_RAM_SIZE=4096
+CONFIG_BLK_DEV_SD=y
+CONFIG_ATA=y
+CONFIG_SATA_AHCI_PLATFORM=y
+CONFIG_NETDEVICES=y
+CONFIG_SERIAL_8250=y
+CONFIG_SERIAL_8250_CONSOLE=y
+CONFIG_SERIAL_8250_NR_UARTS=2
+CONFIG_SERIAL_8250_RUNTIME_UARTS=2
+CONFIG_SERIAL_8250_DW=y
+CONFIG_SERIAL_OF_PLATFORM=y
+CONFIG_I2C_DESIGNWARE_PLATFORM=y
+CONFIG_PINCTRL_SINGLE=y
+CONFIG_GPIO_GENERIC_PLATFORM=y
+CONFIG_REGULATOR_GPIO=y
+CONFIG_DRM=y
+CONFIG_FB_SIMPLE=y
+CONFIG_RTC_CLASS=y
+CONFIG_EXT4_FS=y
+CONFIG_TMPFS=y
+CONFIG_NFS_FS=y
+CONFIG_NFS_V3_ACL=y
+CONFIG_NFS_V4=y
+CONFIG_ROOT_NFS=y
+CONFIG_PRINTK_TIME=y
+CONFIG_DEBUG_FS=y
+CONFIG_DEBUG_KERNEL=y
+CONFIG_DEBUG_LL=y
+CONFIG_DEBUG_UART_8250=y
+CONFIG_EARLY_PRINTK=y
+CONFIG_LOCKUP_DETECTOR=y
+CONFIG_DEBUG_USER=y
-- 
1.8.3.2

[PATCH v2 10/12] ARM: config: select ARCH_HISI in hi3xxx_defconfig

[Haojian Zhuang]

Since ARCH_HISI is added as common configuration of both ARCH_HI3xxx and
ARCH_HIP04, update it into hi3xxx_defconfig.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 arch/arm/configs/hi3xxx_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/hi3xxx_defconfig b/arch/arm/configs/hi3xxx_defconfig
index f186bdf..f8c78ac 100644
--- a/arch/arm/configs/hi3xxx_defconfig
+++ b/arch/arm/configs/hi3xxx_defconfig
@@ -3,6 +3,7 @@ CONFIG_NO_HZ=y
 CONFIG_HIGH_RES_TIMERS=y
 CONFIG_BLK_DEV_INITRD=y
 CONFIG_RD_LZMA=y
+CONFIG_ARCH_HISI=y
 CONFIG_ARCH_HI3xxx=y
 CONFIG_SMP=y
 CONFIG_PREEMPT=y
-- 
1.8.3.2

[PATCH v2 11/12] ARM: hisi: enable erratum 798181 of A15 on HiP04

[Haojian Zhuang]

From: Kefeng Wang <kefeng.wang@linaro.org>

The commit 93dc688 (ARM: 7684/1: errata: Workaround for Cortex-A15
erratum 798181 (TLBI/DSB operations)) introduced a workaround for
Cortex-A15 erratum 798181. Enable it for HIP04(Cortex-a15 r3p2).

Signed-off-by: Kefeng Wang <kefeng.wang@linaro.org>
Singed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 arch/arm/mach-hisi/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
index 4dd966a..c976bdb 100644
--- a/arch/arm/mach-hisi/Kconfig
+++ b/arch/arm/mach-hisi/Kconfig
@@ -21,6 +21,7 @@ config ARCH_HI3xxx
 
 config ARCH_HIP04
 	bool "Hisilicon HiP04 Cortex A15 family" if ARCH_MULTI_V7_LPAE
+	select ARM_ERRATA_798181 if SMP
 	select HAVE_ARM_ARCH_TIMER
 	select MCPM if SMP
 	help
-- 
1.8.3.2

[PATCH v2 12/12] ARM: dts: Add PMU support in HiP04

[Haojian Zhuang]

From: Kefeng Wang <kefeng.wang@linaro.org>

ARM Performance Monitor Units are available on the HiP04.
So add the support in the dtsi.

Simply tested with perf.

Signed-off-by: Kefeng Wang <kefeng.wang@linaro.org>
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 arch/arm/boot/dts/hip04.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/hip04.dtsi b/arch/arm/boot/dts/hip04.dtsi
index eb5e5a2..0ad1e7b 100644
--- a/arch/arm/boot/dts/hip04.dtsi
+++ b/arch/arm/boot/dts/hip04.dtsi
@@ -219,6 +219,26 @@
 			status = "ok";
 		};
 
+		arm-pmu {
+			compatible = "arm,cortex-a15-pmu";
+			interrupts = <0 64 4>,
+				     <0 65 4>,
+				     <0 66 4>,
+				     <0 67 4>,
+				     <0 68 4>,
+				     <0 69 4>,
+				     <0 70 4>,
+				     <0 71 4>,
+				     <0 72 4>,
+				     <0 73 4>,
+				     <0 74 4>,
+				     <0 75 4>,
+				     <0 76 4>,
+				     <0 77 4>,
+				     <0 78 4>,
+				     <0 79 4>;
+		};
+
 		timer {
 			compatible = "arm,armv7-timer";
 			interrupts = <1 13 0xf08>,
-- 
1.8.3.2

[PATCH v2 02/12] ARM: append ARCH_MULTI_V7_LPAE

[Arnd Bergmann]

On Tuesday 08 April 2014 16:00:42 Haojian Zhuang wrote:
> Add new ARCH_MULTI_V7_LPAE config. It will select ARM_LPAE only both
> ARCH_MULTI_V6 & ARCH_MULTI_V7 is disabled.
> 
> ARCH_MULTI_V7 means non-LPAE platform.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

I think it's good to have this option, but it should be added more carefully.

>  arch/arm/Kconfig | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 5248923..a8b2b45 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -918,12 +918,19 @@ config ARCH_MULTI_V6
>  	select CPU_V6K
>  
>  config ARCH_MULTI_V7
> -	bool "ARMv7 based platforms (Cortex-A, PJ4, Scorpion, Krait)"
> +	bool "ARMv7 non-LPAE based platforms (Cortex-A, PJ4, Scorpion, Krait)"
>  	default y
>  	select ARCH_MULTI_V6_V7
>  	select CPU_V7
>  	select HAVE_SMP
>  
> +config ARCH_MULTI_V7_LPAE
> +	bool "ARMv7 LPAE based platforms (HIP04)"
> +	select ARCH_MULTI_V6_V7
> +	select CPU_V7
> +	select HAVE_SMP
> +	select ARM_LPAE if !(ARCH_MULTI_V6 || ARCH_MULTI_V7)
> +
>  config ARCH_MULTI_V6_V7
>  	bool
>  	select MIGHT_HAVE_CACHE_L2X0

Two points here:

a) The other options all list the CPU types (Cortex-A, PJ4, ...),
   while the one you introduce lists the platform name. I think they
   should all list the CPU cores, like

  bool "ARMv7 non-LPAE based platforms (Cortex-A5/A8/A9, PJ4, Scorpion)"

  bool "ARMv7 non-LPAE based platforms (Cortex-A7/A12/A15/A17, Brahma-B15, PJ4B, Krait)"
  (I'm not completely sure about Krait, need to check that).

b) When you introduce this, please change the other platforms over to use
the symbol correctly. We already have a number of platforms based on A7, A15,
PJ4B and Krait.

	Arnd

[PATCH v2 03/12] ARM: hisi: add ARCH_HISI

[Arnd Bergmann]

On Tuesday 08 April 2014 16:00:43 Haojian Zhuang wrote:
> diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
> index feee4db..da16efd 100644
> --- a/arch/arm/mach-hisi/Kconfig
> +++ b/arch/arm/mach-hisi/Kconfig
> @@ -1,8 +1,16 @@
> -config ARCH_HI3xxx
> -       bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
> +config ARCH_HISI
> +       bool "Hisilicon SoC Support"
> +       depends on ARCH_MULTIPLATFORM
>         select ARM_AMBA
>         select ARM_GIC
>         select ARM_TIMER_SP804
> +
> +if ARCH_HISI
> +
> +menu "Hisilicon platform type"
> +
> +config ARCH_HI3xxx
> +       bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
>         select CACHE_L2X0
>         select HAVE_ARM_SCU if SMP
>         select HAVE_ARM_TWD if SMP
> @@ -10,3 +18,7 @@ config ARCH_HI3xxx
>         select PINCTRL_SINGLE
>         help
>           Support for Hisilicon Hi36xx/Hi37xx processor family
> +
> +endmenu
> +
> +endif

I'd rather keep them as separate top-level options. The two platforms
are not really related technically, other than being designed by the
same company. Also, I'd prefer the use of 'menuconfig' here, like:

menuconfig ARCH_HI3xxx
	bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7

if ARCH_HI3xxx

...

endif


menuconfig ARCH_HIP04
	bool "Hisilicon HiP04 family" if ARCH_MULTI_V7_LPAE

if ARCH_HIP04

...

endif


	Arnd

[PATCH v2 07/12] ARM: hisi: add hip04 SoC support

[Arnd Bergmann]

On Tuesday 08 April 2014 16:00:47 Haojian Zhuang wrote:
> diff --git a/arch/arm/mach-hisi/hisilicon.c b/arch/arm/mach-hisi/hisilicon.c
> index 741faf3..10a605f 100644
> --- a/arch/arm/mach-hisi/hisilicon.c
> +++ b/arch/arm/mach-hisi/hisilicon.c
> @@ -14,6 +14,7 @@
>  #include <linux/clk-provider.h>
>  #include <linux/clocksource.h>
>  #include <linux/irqchip.h>
> +#include <linux/memblock.h>
>  #include <linux/of_address.h>
>  #include <linux/of_platform.h>
>  
> @@ -88,3 +89,21 @@ DT_MACHINE_START(HI3620, "Hisilicon Hi3620 (Flattened Device Tree)")
>  	.smp		= smp_ops(hi3xxx_smp_ops),
>  	.restart	= hi3xxx_restart,
>  MACHINE_END
> +
> +static const char *hip04_compat[] __initconst = {
> +	"hisilicon,hip04-d01",
> +	NULL,
> +};
> +
> +static void __init hip04_reserve(void)
> +{
> +	memblock_reserve(HIP04_BOOTWRAPPER_PHYS, HIP04_BOOTWRAPPER_SIZE);
> +}

Can you explain why you do this? Shouldn't that memory just be listed
in the DT?

> +DT_MACHINE_START(HIP04, "Hisilicon HiP04 (Flattened Device Tree)")
> +	.dt_compat	= hip04_compat,
> +#ifdef CONFIG_MCPM
> +	.smp_init	= smp_init_ops(hip04_smp_init_ops),
> +#endif
> +	.reserve	= hip04_reserve,
> +MACHINE_END

I think the #ifdef is not needed here, you already hide hip04_smp_init_ops
if CONFIG_SMP is disabled, and SMP implies MCPM based on your Kconfig.

> diff --git a/arch/arm/mach-hisi/platsmp.c b/arch/arm/mach-hisi/platsmp.c
> index 471f1ee..3a5833f 100644
> --- a/arch/arm/mach-hisi/platsmp.c
> +++ b/arch/arm/mach-hisi/platsmp.c


>  };
> +
> +#ifdef CONFIG_MCPM
> +/* bits definition in SC_CPU_RESET_REQ[x]/SC_CPU_RESET_DREQ[x]
> + * 1 -- unreset; 0 -- reset
> + */
> ...

It would seem appropriate to put all of this into a separate
source file -- there is no shared code between the two SMP
implementations.

> +static int __init hip04_mcpm_init(void)
> +{
> +	struct device_node *np;
> +	int ret = -ENODEV;
> +
> +	np = of_find_compatible_node(NULL, NULL, "hisilicon,hip04-mcpm");
> +	if (!np) {
> +		pr_err("failed to find hisilicon,hip04-mcpm node\n");
> +		goto err;
> +	}

This shouldn't be a fatal error: if you run the kernel on a platform
other than hip04, the code will still be executed here but it won't
find the node and should just ignore the device silently.

> +	relocation = of_iomap(np, 0);
> +	if (!relocation) {
> +		pr_err("failed to get relocation space\n");
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +	sysctrl = of_iomap(np, 1);
> +	if (!sysctrl) {
> +		pr_err("failed to get sysctrl base\n");
> +		ret = -ENOMEM;
> +		goto err_sysctrl;
> +	}

sysctrl sounds like a shared device that you probably don't want
to map here, but rather use a "syscon" node.

> +}
> +early_initcall(hip04_mcpm_init);

Actually, I guess it would be better to do this from one of the
various calls you already have for SMP initialization than
doing an initcall.

	Arnd

[PATCH v2 03/12] ARM: hisi: add ARCH_HISI

[Arnd Bergmann]

On Tuesday 08 April 2014 16:00:43 Haojian Zhuang wrote:
> diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
> index feee4db..da16efd 100644
> --- a/arch/arm/mach-hisi/Kconfig
> +++ b/arch/arm/mach-hisi/Kconfig
> @@ -1,8 +1,16 @@
> -config ARCH_HI3xxx
> -       bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
> +config ARCH_HISI
> +       bool "Hisilicon SoC Support"
> +       depends on ARCH_MULTIPLATFORM
>         select ARM_AMBA
>         select ARM_GIC
>         select ARM_TIMER_SP804
> +
> +if ARCH_HISI
> +
> +menu "Hisilicon platform type"
> +
> +config ARCH_HI3xxx
> +       bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
>         select CACHE_L2X0
>         select HAVE_ARM_SCU if SMP
>         select HAVE_ARM_TWD if SMP
> @@ -10,3 +18,7 @@ config ARCH_HI3xxx
>         select PINCTRL_SINGLE
>         help
>           Support for Hisilicon Hi36xx/Hi37xx processor family
> +
> +endmenu

On second thought: You don't actually have any sub-options under
the main SoC specific option, so using 'menuconfig' as I suggested
first makes no sense.

Just leave ARCH_HI3xxx and ARCH_HIP04 as top-level options and
drop this patch. There is no real need for a new menu here.
That also means you can drop patch 9.

	Arnd

[PATCH v2 09/12] ARM: config: append hip04_defconfig

[Arnd Bergmann]

On Tuesday 08 April 2014 16:00:49 Haojian Zhuang wrote:
> Select HiP04 SoC configuration by hip04_defconfig.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  arch/arm/configs/hip04_defconfig | 67 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 arch/arm/configs/hip04_defconfig

Is it necessary to have a separate defconfig for this?

Since this is a fairly generic platform, I'd prefer to only enable
it in the standard defconfigs like multi_v7_defconfig.

It's probably time now to add a multi_v7_lpae_defconfig though.
Maybe someone has other ideas, we have talked about the split
between the defconfigs before. I think the latest suggestion
was to remove multi_v7_defconfig and replace it with one that
enables both v6 and v7, and another one that only enables
v7 with LPAE (omap5, exynos5, mvebu, sunxi, hip04, vexpress,
shmobile, highbank, keystone, qcom)

	Arnd

[PATCH v2 05/12] irq: gic: extends the cpu interface to 16

[Marc Zyngier]

On Tue, Apr 08 2014 at  9:00:45 am BST, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:

Hi Haojian,

> In order to support 16 CPUs, Hisilicon extends the GIC to support the
> number of CPU interfaces from 8 to 16.

Yet another Frankein-GIC! Hooray! I missed that fuzzy feeling! ;-)

First, it would be good to have an exact description of what are the
changes HiSilicon made in the GIC distributor. Here, I can only make
educated guesses (which I'd like to avoid).

> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  drivers/irqchip/irq-gic.c | 78 ++++++++++++++++++++++++++++-------------------
>  1 file changed, 47 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 8fd27bf..44eff46 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -76,9 +76,12 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock);
>   * The GIC mapping of CPU interfaces does not necessarily match
>   * the logical CPU numbering.  Let's use a mapping as returned
>   * by the GIC itself.
> + *
> + * Hisilicon HiP04 extends the number of CPU interface from 8 to 16.
>   */
> -#define NR_GIC_CPU_IF 8
> -static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
> +#define MAX_NR_GIC_CPU_IF 16
> +static u16 gic_cpu_map[MAX_NR_GIC_CPU_IF] __read_mostly;
> +static int nr_gic_cpu_if = 8;  /* The standard GIC supports 8 CPUs */

How do you deal with a situation where you have a standard GIC cascaded
into one of these super-charged GIC? This is really a per-GIC property,
not a global one.

>  /*
>   * Supported arch specific GIC irq extension.
> @@ -245,16 +248,19 @@ static int gic_retrigger(struct irq_data *d)
>  static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
>                             bool force)
>  {
> -       void __iomem *reg = gic_dist_base(d) + GIC_DIST_TARGET + (gic_irq(d) & ~3);
> -       unsigned int shift = (gic_irq(d) % 4) * 8;
> +       void __iomem *reg;
> +       unsigned int shift, step;
>         unsigned int cpu = cpumask_any_and(mask_val, cpu_online_mask);
>         u32 val, mask, bit;
> -
> -       if (cpu >= NR_GIC_CPU_IF || cpu >= nr_cpu_ids)
> +       if (cpu >= nr_gic_cpu_if || cpu >= nr_cpu_ids)
>                 return -EINVAL;
>
> +       step = BITS_PER_LONG / nr_gic_cpu_if;

NAK. This breaks arm64 in a very bad way. The GICD registers are always
32bit, nothing to do with the size of a long on a given CPU.

> +       shift = (gic_irq(d) % step) * nr_gic_cpu_if;
> +       reg = gic_dist_base(d) + GIC_DIST_TARGET + (gic_irq(d) / step * 4);
> +
>         raw_spin_lock(&irq_controller_lock);
> -       mask = 0xff << shift;
> +       mask = ((1 << nr_gic_cpu_if) - 1) << shift;
>         bit = gic_cpu_map[cpu] << shift;
>         val = readl_relaxed(reg) & ~mask;
>         writel_relaxed(val | bit, reg);

So, in the absence of any form of documentation, I'll guess:

GICD_ITARGETSRn has been modified to have 16bits per interrupt. Assuming
the GICD memory map stays the same, this means we now have a limit of
510 interrupts. Please reflect this limit in the init function.

> @@ -354,15 +360,17 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
>         irq_set_chained_handler(irq, gic_handle_cascade_irq);
>  }
>
> -static u8 gic_get_cpumask(struct gic_chip_data *gic)
> +static u16 gic_get_cpumask(struct gic_chip_data *gic)
>  {
>         void __iomem *base = gic_data_dist_base(gic);
> -       u32 mask, i;
> -
> -       for (i = mask = 0; i < 32; i += 4) {
> -               mask = readl_relaxed(base + GIC_DIST_TARGET + i);
> -               mask |= mask >> 16;
> -               mask |= mask >> 8;
> +       u32 mask, i, j, step;
> +
> +       /* get the number of CPU fields in GIC_DIST_TARGET register */
> +       step = BITS_PER_LONG / nr_gic_cpu_if;
> +       for (i = mask = 0; i < 32; i += step) {
> +               mask = readl_relaxed(base + GIC_DIST_TARGET + i / step * 4);
> +               for (j = BITS_PER_LONG >> 1; j >= nr_gic_cpu_if; j >>= 1)
> +                       mask |= mask >> j;

Again. All the uses of BITS_PER_LONG in this driver are completely
wrong. I'll stop commenting on that.

>                 if (mask)
>                         break;
>         }
> @@ -375,7 +383,7 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic)
>
>  static void __init gic_dist_init(struct gic_chip_data *gic)
>  {
> -       unsigned int i;
> +       unsigned int i, step;
>         u32 cpumask;
>         unsigned int gic_irqs = gic->gic_irqs;
>         void __iomem *base = gic_data_dist_base(gic);
> @@ -392,10 +400,11 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
>          * Set all global interrupts to this CPU only.
>          */
>         cpumask = gic_get_cpumask(gic);
> -       cpumask |= cpumask << 8;
> -       cpumask |= cpumask << 16;
> -       for (i = 32; i < gic_irqs; i += 4)
> -               writel_relaxed(cpumask, base + GIC_DIST_TARGET + i * 4 / 4);
> +       for (i = nr_gic_cpu_if; i < BITS_PER_LONG; i <<= 1)
> +               cpumask |= cpumask << i;
> +       step = BITS_PER_LONG / nr_gic_cpu_if;
> +       for (i = 32; i < gic_irqs; i += step)
> +               writel_relaxed(cpumask, base + GIC_DIST_TARGET + i / step * 4);
>
>         /*
>          * Set priority on all global interrupts.
> @@ -423,7 +432,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>         /*
>          * Get what the GIC says our CPU mask is.
>          */
> -       BUG_ON(cpu >= NR_GIC_CPU_IF);
> +       BUG_ON(cpu >= nr_gic_cpu_if);
>         cpu_mask = gic_get_cpumask(gic);
>         gic_cpu_map[cpu] = cpu_mask;
>
> @@ -431,7 +440,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>          * Clear our mask from the other map entries in case they're
>          * still undefined.
>          */
> -       for (i = 0; i < NR_GIC_CPU_IF; i++)
> +       for (i = 0; i < nr_gic_cpu_if; i++)
>                 if (i != cpu)
>                         gic_cpu_map[i] &= ~cpu_mask;
>
> @@ -469,7 +478,7 @@ static void gic_dist_save(unsigned int gic_nr)
>  {
>         unsigned int gic_irqs;
>         void __iomem *dist_base;
> -       int i;
> +       int i, step;
>
>         if (gic_nr >= MAX_GIC_NR)
>                 BUG();
> @@ -484,7 +493,8 @@ static void gic_dist_save(unsigned int gic_nr)
>                 gic_data[gic_nr].saved_spi_conf[i] =
>                         readl_relaxed(dist_base + GIC_DIST_CONFIG + i * 4);
>
> -       for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
> +       step = BITS_PER_LONG / nr_gic_cpu_if;
> +       for (i = 0; i < DIV_ROUND_UP(gic_irqs, step); i++)
>                 gic_data[gic_nr].saved_spi_target[i] =
>                         readl_relaxed(dist_base + GIC_DIST_TARGET + i * 4);
>
> @@ -503,7 +513,7 @@ static void gic_dist_save(unsigned int gic_nr)
>  static void gic_dist_restore(unsigned int gic_nr)
>  {
>         unsigned int gic_irqs;
> -       unsigned int i;
> +       unsigned int i, step;
>         void __iomem *dist_base;
>
>         if (gic_nr >= MAX_GIC_NR)
> @@ -525,7 +535,8 @@ static void gic_dist_restore(unsigned int gic_nr)
>                 writel_relaxed(0xa0a0a0a0,
>                         dist_base + GIC_DIST_PRI + i * 4);
>
> -       for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
> +       step = BITS_PER_LONG / nr_gic_cpu_if;
> +       for (i = 0; i < DIV_ROUND_UP(gic_irqs, step); i++)
>                 writel_relaxed(gic_data[gic_nr].saved_spi_target[i],
>                         dist_base + GIC_DIST_TARGET + i * 4);
>
> @@ -666,8 +677,8 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
>         dmb(ishst);
>
>         /* this always happens on GIC0 */
> -       writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
> -
> +       writel_relaxed(map << (8 + 16 - nr_gic_cpu_if) | irq,
> +                      gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);

So you're writing the CPU target map into GICD_SGIR[23:8]. What happens
to the NSATT bit?

>         raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
>  }
>  #endif
> @@ -681,7 +692,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
>   */
>  void gic_send_sgi(unsigned int cpu_id, unsigned int irq)
>  {
> -       BUG_ON(cpu_id >= NR_GIC_CPU_IF);
> +       BUG_ON(cpu_id >= nr_gic_cpu_if);
>         cpu_id = 1 << cpu_id;
>         /* this always happens on GIC0 */
>         writel_relaxed((cpu_id << 16) | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
> @@ -700,7 +711,7 @@ int gic_get_cpu_id(unsigned int cpu)
>  {
>         unsigned int cpu_bit;
>
> -       if (cpu >= NR_GIC_CPU_IF)
> +       if (cpu >= nr_gic_cpu_if)
>                 return -1;
>         cpu_bit = gic_cpu_map[cpu];
>         if (cpu_bit & (cpu_bit - 1))
> @@ -971,8 +982,8 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>          * Initialize the CPU interface map to all CPUs.
>          * It will be refined as each CPU probes its ID.
>          */
> -       for (i = 0; i < NR_GIC_CPU_IF; i++)
> -               gic_cpu_map[i] = 0xff;
> +       for (i = 0; i < nr_gic_cpu_if; i++)
> +               gic_cpu_map[i] = (1 << MAX_NR_GIC_CPU_IF) - 1;
>
>         /*
>          * For primary GICs, skip over SGIs.
> @@ -1047,6 +1058,10 @@ gic_of_init(struct device_node *node, struct device_node *parent)
>         if (WARN_ON(!node))
>                 return -ENODEV;
>
> +       /* HiP04 supports 16 CPUs at most */
> +       if (of_device_is_compatible(node, "hisilicon,hip04-gic"))
> +               nr_gic_cpu_if = 16;
> +

Consider using a separate init function that will simply override this
value, rather than having a test in the generic code.

>         dist_base = of_iomap(node, 0);
>         WARN(!dist_base, "unable to map gic dist registers\n");
>
> @@ -1069,6 +1084,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
>  }
>  IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
>  IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
> +IRQCHIP_DECLARE(hip04_gic, "hisilicon,hip04-gic", gic_of_init);
>  IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
>  IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);

General question: How does this work with virtualisation?
- Does your GICV interface also refect the changes you've made to GICC?
- Do your GICH_LRn registers reflect the changes to GICD_SGIR (can't
really see where you'd shove all the bits, but asking anyway)?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

[PATCH v2 07/12] ARM: hisi: add hip04 SoC support

[Mark Rutland]

On Tue, Apr 08, 2014 at 09:00:47AM +0100, Haojian Zhuang wrote:
> Hisilicon Hi3xxx is based on Cortex A9 Core. Now HiP04 SoC is based on
> Cortex A15 Core. Since multiple clusters is used in HiP04 SoC, it could
> be based on MCPM.
> 
> And HiP04 supports LPAE to support large memory.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  arch/arm/Kconfig               |   2 +-
>  arch/arm/mach-hisi/Kconfig     |   7 ++
>  arch/arm/mach-hisi/core.h      |   6 +
>  arch/arm/mach-hisi/hisilicon.c |  19 +++
>  arch/arm/mach-hisi/platsmp.c   | 259 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 292 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index a8b2b45..6af6609 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1113,7 +1113,7 @@ source arch/arm/mm/Kconfig
> 
>  config ARM_NR_BANKS
>         int
> -       default 16 if ARCH_EP93XX
> +       default 16 if ARCH_EP93XX || ARCH_HIP04
>         default 8
> 
>  config IWMMXT
> diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
> index da16efd..4dd966a 100644
> --- a/arch/arm/mach-hisi/Kconfig
> +++ b/arch/arm/mach-hisi/Kconfig
> @@ -19,6 +19,13 @@ config ARCH_HI3xxx
>         help
>           Support for Hisilicon Hi36xx/Hi37xx processor family
> 
> +config ARCH_HIP04
> +       bool "Hisilicon HiP04 Cortex A15 family" if ARCH_MULTI_V7_LPAE
> +       select HAVE_ARM_ARCH_TIMER
> +       select MCPM if SMP
> +       help
> +         Support for Hisilicon HiP04 processor family

Nit: Surely this is an SoC family? The processor is Cortex-A15, as you
state above.

> +
>  endmenu
> 
>  endif
> diff --git a/arch/arm/mach-hisi/core.h b/arch/arm/mach-hisi/core.h
> index af23ec2..e008c7a 100644
> --- a/arch/arm/mach-hisi/core.h
> +++ b/arch/arm/mach-hisi/core.h
> @@ -12,4 +12,10 @@ extern void hi3xxx_cpu_die(unsigned int cpu);
>  extern int hi3xxx_cpu_kill(unsigned int cpu);
>  extern void hi3xxx_set_cpu(int cpu, bool enable);
> 
> +#define HIP04_BOOTWRAPPER_PHYS         0x10c00000
> +#define HIP04_BOOTWRAPPER_MAGIC                0xa5a5a5a5
> +#define HIP04_BOOTWRAPPER_SIZE         0x00010000

The address and size should come from DT.

What is the magic, exactly? How is it used by the kernel and
bootwrapper?

[...]

> +static void __init hip04_reserve(void)
> +{
> +       memblock_reserve(HIP04_BOOTWRAPPER_PHYS, HIP04_BOOTWRAPPER_SIZE);
> +}

As Arnd said, do this in your DT. This is not a hardware property the
kernel needs to statically know, but rather a system-specific property
that needs to be in the DT such that it can vary.

[...]

> +static int hip04_mcpm_power_up(unsigned int cpu, unsigned int cluster)
> +{
> +       unsigned long data, mask;
> +
> +       if (!relocation || !sysctrl)
> +               return -ENODEV;
> +       if (cluster >= HIP04_MAX_CLUSTERS || cpu >= HIP04_MAX_CPUS_PER_CLUSTER)
> +               return -EINVAL;
> +
> +       spin_lock(&boot_lock);
> +       writel_relaxed(HIP04_BOOTWRAPPER_PHYS, relocation);
> +       writel_relaxed(HIP04_BOOTWRAPPER_MAGIC, relocation + 4);
> +       writel_relaxed(virt_to_phys(mcpm_entry_point), relocation + 8);

So the kernel is poking the hardware directly to throw CPUs into a
bootwrapper that it pokes to throw the CPUs into the kernel? That does
not strike me as fantastic.

It's a shame we have to manage coherency here at all and don't have a
PSCI implementation to abstract this. That would mean you wouldn't need
a machine descriptor at all, could reuse existing infrastructure, and
you'd be able to use Hyp.

[...]

> +static void hip04_mcpm_power_down(void)
> +{
> +       unsigned int mpidr, cpu, cluster;
> +       unsigned int v;
> +
> +       spin_lock(&boot_lock);
> +       spin_unlock(&boot_lock);

Huh? What's this for?

> +       asm volatile(
> +       "       mrc     p15, 0, %0, c1, c0, 0\n"
> +       "       bic     %0, %0, %1\n"
> +       "       mcr     p15, 0, %0, c1, c0, 0\n"
> +         : "=&r" (v)
> +         : "Ir" (CR_C)
> +         : "cc");

I don't think that cc clobber is necessary, none of these instructions
set the flags.

> +       asm volatile(
> +       /*
> +       * Turn off coherency
> +       */
> +       "       mrc     p15, 0, %0, c1, c0, 1\n"
> +       "       bic     %0, %0, %1\n"
> +       "       mcr     p15, 0, %0, c1, c0, 1\n"
> +       : "=&r" (v)
> +       : "Ir" (0x40)

0x40?

> +       : "cc");

Likewise I don't think this clobber is necessary.

[...]

> +static int __init hip04_mcpm_init(void)
> +{
> +       struct device_node *np;
> +       int ret = -ENODEV;
> +
> +       np = of_find_compatible_node(NULL, NULL, "hisilicon,hip04-mcpm");
> +       if (!np) {
> +               pr_err("failed to find hisilicon,hip04-mcpm node\n");
> +               goto err;
> +       }

MCPM is a Linux-specific software construct. It is not a piece of
hardware, and not a generic interface name. Please come up with a better
name for this that makes it clear exactly what you are describing.

> +       relocation = of_iomap(np, 0);
> +       if (!relocation) {
> +               pr_err("failed to get relocation space\n");
> +               ret = -ENOMEM;
> +               goto err;
> +       }
> +       sysctrl = of_iomap(np, 1);
> +       if (!sysctrl) {
> +               pr_err("failed to get sysctrl base\n");
> +               ret = -ENOMEM;
> +               goto err_sysctrl;
> +       }
> +       fabric = of_iomap(np, 2);
> +       if (!fabric) {
> +               pr_err("failed to get fabric base\n");
> +               ret = -ENOMEM;
> +               goto err_fabric;
> +       }

These sounds like they would be better described by separate nodes.

Thanks,
Mark.

[PATCH v2 08/12] ARM: dts: add hip04-d01 dts file

[Mark Rutland]

On Tue, Apr 08, 2014 at 09:00:48AM +0100, Haojian Zhuang wrote:
> Add hip04.dtsi & hip04-d01.dts file to support HiP04 SoC platform.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  Documentation/devicetree/bindings/arm/gic.txt      |   1 +
>  .../bindings/arm/hisilicon/hisilicon.txt           |  12 ++
>  .../devicetree/bindings/clock/hip04-clock.txt      |  20 ++
>  arch/arm/boot/dts/Makefile                         |   1 +
>  arch/arm/boot/dts/hip04-d01.dts                    |  74 +++++++
>  arch/arm/boot/dts/hip04.dtsi                       | 240 +++++++++++++++++++++
>  6 files changed, 348 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/hip04-clock.txt
>  create mode 100644 arch/arm/boot/dts/hip04-d01.dts
>  create mode 100644 arch/arm/boot/dts/hip04.dtsi
> 
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> index 5573c08..150f7d6 100644
> --- a/Documentation/devicetree/bindings/arm/gic.txt
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -16,6 +16,7 @@ Main node required properties:
>         "arm,cortex-a9-gic"
>         "arm,cortex-a7-gic"
>         "arm,arm11mp-gic"
> +       "hisilicon,hip04-gic"
>  - interrupt-controller : Identifies the node as an interrupt controller
>  - #interrupt-cells : Specifies the number of cells needed to encode an
>    interrupt source.  The type shall be a <u32> and the value shall be 3.
> diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
> index df0a452..47c0a13 100644
> --- a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
> +++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
> @@ -4,6 +4,10 @@ Hisilicon Platforms Device Tree Bindings
>  Hi4511 Board
>  Required root node properties:
>         - compatible = "hisilicon,hi3620-hi4511";
> +HiP04 D01 Board
> +Required root node properties:
> +       - compatible = "hisilicon,hip04-d01";
> +
> 
>  Hisilicon system controller
> 
> @@ -31,6 +35,14 @@ Example:
>                 reboot-offset = <0x4>;
>         };
> 
> +
> +Hisilicon MCPM Implementation

This is _NOT_ a hardware or system property. It is a Linux
implementation detail. This does not belong here as-is.

> +
> +Required Properties:
> +- compatible: "hisilicon,hip04-mcpm"
> +- reg: Register address and size.
> +
> +

The code was looking for several entries in the reg, and you don't
describe what any of them are.

> diff --git a/Documentation/devicetree/bindings/clock/hip04-clock.txt b/Documentation/devicetree/bindings/clock/hip04-clock.txt
> new file mode 100644
> index 0000000..4d31ae3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/hip04-clock.txt
> @@ -0,0 +1,20 @@
> +* Hisilicon HiP04 Clock Controller
> +
> +The HiP04 clock controller generates and supplies clock to various
> +controllers within the HiP04 SoC.
> +
> +Required Properties:
> +
> +- compatible: should be one of the following.
> +  - "hisilicon,hip04-clock" - controller compatible with HiP04 SoC.
> +
> +- reg: physical base address of the controller and length of memory mapped
> +  region.
> +
> +- #clock-cells: should be 1.
> +
> +
> +Each clock is assigned an identifier and client nodes use this identifier
> +to specify the clock which they consume.

Delete this sentence -- that's a well understood facet of the clock
bindings.

What does the single clock cell represent? Is it a simple linear index?
Is it sparse? Are there a set of well-known IDs?

> +
> +All these identifier could be found in <dt-bindings/clock/hip04-clock.h>.

I'm really not a fan of dt includes, but I understand that others are.
Please move this sentence into the #clock-cells description to make it
clearer.

> diff --git a/arch/arm/boot/dts/hip04-d01.dts b/arch/arm/boot/dts/hip04-d01.dts
> new file mode 100644
> index 0000000..a10dcf3
> --- /dev/null
> +++ b/arch/arm/boot/dts/hip04-d01.dts
> @@ -0,0 +1,74 @@
> +/*
> + *  Copyright (C) 2013-2014 Linaro Ltd.
> + *  Author: Haojian Zhuang <haojian.zhuang@linaro.org>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  publishhed by the Free Software Foundation.
> + */
> +
> +/dts-v1/;
> +
> +#include "hip04.dtsi"
> +
> +/ {
> +       /* memory bus is 64-bit */
> +       #address-cells = <2>;
> +       #size-cells = <1>;
> +       model = "Hisilicon D01 Development Board";
> +       compatible = "hisilicon,hip04-d01";
> +

I would expect commented /memreserve/ statements here.

> +       memory at 0 {

This memory doesn't start at 0, as your comment below explains. The
unit-address (the bit after the '@') should math the address cells of
the first reg entry.

> +               device_type = "memory";
> +               /*
> +                * Bootloader loads kernel image into 0x1000_0000 region,
> +                * so disables the region between [0000_0000 - 1000_0000]
> +                * temporarily.

How does it "disable" this region, and why?

> +                * Because the PHYS_TO_VIRT_OFFSET is calculated based on
> +                * the original region that kenrel is loaded.
> +                * This workaround will be removed only after UEFI updated.

What problem exactly does this work around?

> +                */
> +               reg = <0x00000000 0x10000000 0xc0000000>;
> +       };
> +
> +       memory at 00000004c0000000 {

Nit: Place a comma between 32-bit wide entries in the unit-address for
clarity, e.g. memory at 00000004,c0000000

> +               device_type = "memory";
> +               reg = <0x00000004 0xc0000000 0x40000000>;
> +       };
> +
> +       memory at 0000000500000000 {
> +               device_type = "memory";
> +               reg = <0x00000005 0x00000000 0x80000000>;
> +       };
> +
> +       memory at 0000000580000000 {
> +               device_type = "memory";
> +               reg = <0x00000005 0x80000000 0x80000000>;
> +       };
> +
> +       memory at 0000000600000000 {
> +               device_type = "memory";
> +               reg = <0x00000006 0x00000000 0x80000000>;
> +       };
> +
> +       memory at 0000000680000000 {
> +               device_type = "memory";
> +               reg = <0x00000006 0x80000000 0x80000000>;
> +       };
> +
> +       memory at 0000000700000000 {
> +               device_type = "memory";
> +               reg = <0x00000007 0x00000000 0x80000000>;
> +       };
> +
> +       memory at 0000000780000000 {
> +               device_type = "memory";
> +               reg = <0x00000007 0x80000000 0x80000000>;
> +       };

Please fold these into a single memory node with multiple reg entries.
Is there a good reason to keep these separate that I'm not aware of?

> +
> +       soc {
> +               uart0: uart at 4007000 {
> +                       status = "ok";
> +               };
> +       };
> +};
> diff --git a/arch/arm/boot/dts/hip04.dtsi b/arch/arm/boot/dts/hip04.dtsi
> new file mode 100644
> index 0000000..eb5e5a2
> --- /dev/null
> +++ b/arch/arm/boot/dts/hip04.dtsi
> @@ -0,0 +1,240 @@
> +/*
> + * Hisilicon Ltd. HiP01 SoC
> + *
> + * Copyright (C) 2013-2014 Hisilicon Ltd.
> + * Copyright (C) 2013-2014 Linaro Ltd.
> + *
> + * Author: Haojian Zhuang <haojian.zhuang@linaro.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * publishhed by the Free Software Foundation.
> + */
> +
> +#include <dt-bindings/clock/hip04-clock.h>
> +
> +/ {
> +       /* memory bus is 64-bit */
> +       #address-cells = <2>;
> +       #size-cells = <1>;

[...]

> +               CPU3: cpu at 3 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a15";
> +                       reg = <3>;
> +               };
> +               CPU4: cpu at 100 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a15";
> +                       reg = <0x100>;
> +                       clock-frequency = <1350000000>;

Why is there a clock-frequency property in some CPU nodes but not
others? Please be consistent.

[...]

> +       soc {
> +               /* It's a 32-bit SoC. */
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               compatible = "arm,amba-bus", "simple-bus";

You don't need both here. Either this is an AMBA bus or another MMIO
bus, please choose. Existing dts are misleading in having both.

> +               device_type = "soc";

This is completely unnecessary.

> +               interrupt-parent = <&gic>;
> +               ranges = <0 0 0xe0000000 0x10000000>;
> +
> +               gic: interrupt-controller at c01000 {
> +                       compatible = "hisilicon,hip04-gic";
> +                       #interrupt-cells = <3>;
> +                       #address-cells = <0>;
> +                       interrupt-controller;
> +
> +                       /* gic dist base, gic cpu base */
> +                       reg = <0xc01000 0x1000>, <0xc02000 0x1000>;

No GICH or GICV?

> +               };
> +
> +               mcpm: mcpm {
> +                       compatible = "hisilicon,hip04-mcpm";
> +                       reg = <0x100 0x1000>, <0x3e00000 0x00100000>,
> +                             <0x302a000 0x1000>;
> +               };

As mentioned elsewhere, this is meaningless. Please come up with a
better name and/or split this into the actual components.

> +
> +               clock: clock {
> +                       compatible = "hisilicon,hip04-clock";
> +                       /* FIXME: the base of clock controller */
> +                       reg = <0 0x1000>;
> +                       #clock-cells = <1>;
> +               };

Fix this or get rid of it.

Cheers,
Mark.

[PATCH v2 06/12] ARM: mcpm: change max clusters to 4

[Dave Martin]

On Tue, Apr 08, 2014 at 04:00:46PM +0800, Haojian Zhuang wrote:
> In order to support 4 clusters with 4 Cortex A15 Cores in each cluster,
> enlarge maximum clusters from 2 to 4 in MCPM.

CC Nico on mcpm patches please.  I'm happy to be CC'd as well.

> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  arch/arm/include/asm/mcpm.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/mcpm.h b/arch/arm/include/asm/mcpm.h
> index 608516e..68f82cf 100644
> --- a/arch/arm/include/asm/mcpm.h
> +++ b/arch/arm/include/asm/mcpm.h
> @@ -20,7 +20,7 @@
>   * to consider dynamic allocation.
>   */
>  #define MAX_CPUS_PER_CLUSTER	4
> -#define MAX_NR_CLUSTERS		2
> +#define MAX_NR_CLUSTERS		4

Because of the need for alignment to the biggest cacheline size in the
system, the MCPM low-level locking structures consume a non-trivial
amount of memory.

Therefore, I'm not keen on the idea of simply increasing this #define
every time a platform appears with a larger number of clusters.
If hip04 is not built into the kernel, this just wastes memory.

I'll leave it to Nico to decide whether we can increase the #define
to 4 or whether this needs a proper fix now.  Ideally, we would have
a way of choosing the maximum value required by the set of boards built
into the kernel, or switch to dynamic allocation.

Cheers
---Dave

[PATCH v2 08/12] ARM: dts: add hip04-d01 dts file

[Dave Martin]

On Tue, Apr 08, 2014 at 04:00:48PM +0800, Haojian Zhuang wrote:
> Add hip04.dtsi & hip04-d01.dts file to support HiP04 SoC platform.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

[...]

> diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
> index df0a452..47c0a13 100644
> --- a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
> +++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt

[...]

> @@ -31,6 +35,14 @@ Example:
>  		reboot-offset = <0x4>;
>  	};
>  
> +
> +Hisilicon MCPM Implementation
> +
> +Required Properties:
> +- compatible: "hisilicon,hip04-mcpm"
> +- reg: Register address and size.
> +
> +

Unless you have a component in your hardware called "MCPM", this is
describing a power controller peripheral.  If so, what is it?

Cheers
---Dave

[PATCH v2 07/12] ARM: hisi: add hip04 SoC support

[Dave Martin]

On Tue, Apr 08, 2014 at 04:00:47PM +0800, Haojian Zhuang wrote:
> Hisilicon Hi3xxx is based on Cortex A9 Core. Now HiP04 SoC is based on
> Cortex A15 Core. Since multiple clusters is used in HiP04 SoC, it could
> be based on MCPM.

The mcpm backend implementation here looks strange.

Before commenting on implementation details, can you explain the
hardware config, and what this code is trying to achieve?

At the moment, I'm confused:

  * There is no obvious code here to actually power CPUs or clusters up
    and down.  There is no wfi(), so I can't see why a CPU will even
    halt when mcpm_power_down() is called.

  * _power_down_finish() looks odd.  It is intended for serialisation
    only, i.e., to confirm that a CPU is physically powered down or
    parked somehow in hardware.  There is no guarantee that the
    _power_down_finish() is called at all.

  * __mcpu_cpu_going_down() and __mcpm_cpu_down() are called in the
    wrong order (if __mcpu_cpu_going_down() is called at all).

  * ... etc.


Cheers
---Dave

> 
> And HiP04 supports LPAE to support large memory.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  arch/arm/Kconfig               |   2 +-
>  arch/arm/mach-hisi/Kconfig     |   7 ++
>  arch/arm/mach-hisi/core.h      |   6 +
>  arch/arm/mach-hisi/hisilicon.c |  19 +++
>  arch/arm/mach-hisi/platsmp.c   | 259 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 292 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index a8b2b45..6af6609 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1113,7 +1113,7 @@ source arch/arm/mm/Kconfig
>  
>  config ARM_NR_BANKS
>  	int
> -	default 16 if ARCH_EP93XX
> +	default 16 if ARCH_EP93XX || ARCH_HIP04
>  	default 8
>  
>  config IWMMXT
> diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
> index da16efd..4dd966a 100644
> --- a/arch/arm/mach-hisi/Kconfig
> +++ b/arch/arm/mach-hisi/Kconfig
> @@ -19,6 +19,13 @@ config ARCH_HI3xxx
>  	help
>  	  Support for Hisilicon Hi36xx/Hi37xx processor family
>  
> +config ARCH_HIP04
> +	bool "Hisilicon HiP04 Cortex A15 family" if ARCH_MULTI_V7_LPAE
> +	select HAVE_ARM_ARCH_TIMER
> +	select MCPM if SMP
> +	help
> +	  Support for Hisilicon HiP04 processor family
> +
>  endmenu
>  
>  endif
> diff --git a/arch/arm/mach-hisi/core.h b/arch/arm/mach-hisi/core.h
> index af23ec2..e008c7a 100644
> --- a/arch/arm/mach-hisi/core.h
> +++ b/arch/arm/mach-hisi/core.h
> @@ -12,4 +12,10 @@ extern void hi3xxx_cpu_die(unsigned int cpu);
>  extern int hi3xxx_cpu_kill(unsigned int cpu);
>  extern void hi3xxx_set_cpu(int cpu, bool enable);
>  
> +#define HIP04_BOOTWRAPPER_PHYS		0x10c00000
> +#define HIP04_BOOTWRAPPER_MAGIC		0xa5a5a5a5
> +#define HIP04_BOOTWRAPPER_SIZE		0x00010000
> +
> +extern bool __init hip04_smp_init_ops(void);
> +
>  #endif
> diff --git a/arch/arm/mach-hisi/hisilicon.c b/arch/arm/mach-hisi/hisilicon.c
> index 741faf3..10a605f 100644
> --- a/arch/arm/mach-hisi/hisilicon.c
> +++ b/arch/arm/mach-hisi/hisilicon.c
> @@ -14,6 +14,7 @@
>  #include <linux/clk-provider.h>
>  #include <linux/clocksource.h>
>  #include <linux/irqchip.h>
> +#include <linux/memblock.h>
>  #include <linux/of_address.h>
>  #include <linux/of_platform.h>
>  
> @@ -88,3 +89,21 @@ DT_MACHINE_START(HI3620, "Hisilicon Hi3620 (Flattened Device Tree)")
>  	.smp		= smp_ops(hi3xxx_smp_ops),
>  	.restart	= hi3xxx_restart,
>  MACHINE_END
> +
> +static const char *hip04_compat[] __initconst = {
> +	"hisilicon,hip04-d01",
> +	NULL,
> +};
> +
> +static void __init hip04_reserve(void)
> +{
> +	memblock_reserve(HIP04_BOOTWRAPPER_PHYS, HIP04_BOOTWRAPPER_SIZE);
> +}
> +
> +DT_MACHINE_START(HIP04, "Hisilicon HiP04 (Flattened Device Tree)")
> +	.dt_compat	= hip04_compat,
> +#ifdef CONFIG_MCPM
> +	.smp_init	= smp_init_ops(hip04_smp_init_ops),
> +#endif
> +	.reserve	= hip04_reserve,
> +MACHINE_END
> diff --git a/arch/arm/mach-hisi/platsmp.c b/arch/arm/mach-hisi/platsmp.c
> index 471f1ee..3a5833f 100644
> --- a/arch/arm/mach-hisi/platsmp.c
> +++ b/arch/arm/mach-hisi/platsmp.c
> @@ -9,9 +9,13 @@
>   */
>  #include <linux/smp.h>
>  #include <linux/io.h>
> +#include <linux/irqchip/arm-gic.h>
>  #include <linux/of_address.h>
>  
>  #include <asm/cacheflush.h>
> +#include <asm/cp15.h>
> +#include <asm/cputype.h>
> +#include <asm/mcpm.h>
>  #include <asm/smp_plat.h>
>  #include <asm/smp_scu.h>
>  
> @@ -87,3 +91,258 @@ struct smp_operations hi3xxx_smp_ops __initdata = {
>  	.cpu_kill		= hi3xxx_cpu_kill,
>  #endif
>  };
> +
> +#ifdef CONFIG_MCPM

It would be cleaner to split the mcpm code out into a separate file
and lose the #ifdefs.  There doesn't seem to be any interdependency
between the mcpm code and platsmp code here.

> +/* bits definition in SC_CPU_RESET_REQ[x]/SC_CPU_RESET_DREQ[x]
> + * 1 -- unreset; 0 -- reset
> + */
> +#define CORE_RESET_BIT(x)		(1 << x)
> +#define NEON_RESET_BIT(x)		(1 << (x + 4))
> +#define CORE_DEBUG_RESET_BIT(x)		(1 << (x + 9))
> +#define CLUSTER_L2_RESET_BIT		(1 << 8)
> +#define CLUSTER_DEBUG_RESET_BIT		(1 << 13)
> +
> +/*
> + * bits definition in SC_CPU_RESET_STATUS[x]
> + * 1 -- reset status; 0 -- unreset status
> + */
> +#define CORE_RESET_STATUS(x)		(1 << x)
> +#define NEON_RESET_STATUS(x)		(1 << (x + 4))
> +#define CORE_DEBUG_RESET_STATUS(x)	(1 << (x + 9))
> +#define CLUSTER_L2_RESET_STATUS		(1 << 8)
> +#define CLUSTER_DEBUG_RESET_STATUS	(1 << 13)
> +#define CORE_WFI_STATUS(x)		(1 << (x + 16))
> +#define CORE_WFE_STATUS(x)		(1 << (x + 20))
> +#define CORE_DEBUG_ACK(x)		(1 << (x + 24))
> +
> +#define SC_CPU_RESET_REQ(x)		(0x520 + (x << 3))	/* reset */
> +#define SC_CPU_RESET_DREQ(x)		(0x524 + (x << 3))	/* unreset */
> +#define SC_CPU_RESET_STATUS(x)		(0x1520 + (x << 3))
> +
> +#define FAB_SF_MODE			0x0c
> +#define FAB_SF_INVLD			0x10
> +
> +/* bits definition in FB_SF_INVLD */
> +#define FB_SF_INVLD_START		(1 << 8)
> +
> +#define HIP04_MAX_CLUSTERS		4
> +#define HIP04_MAX_CPUS_PER_CLUSTER	4
> +
> +static void __iomem *relocation, *sysctrl, *fabric;
> +static int hip04_cpu_table[HIP04_MAX_CLUSTERS][HIP04_MAX_CPUS_PER_CLUSTER];
> +static DEFINE_SPINLOCK(boot_lock);
> +
> +static bool hip04_cluster_down(unsigned int cluster)
> +{
> +	int i;
> +
> +	for (i = 0; i < HIP04_MAX_CPUS_PER_CLUSTER; i++)
> +		if (hip04_cpu_table[cluster][i])
> +			return false;
> +	return true;
> +}
> +
> +static void hip04_set_snoop_filter(unsigned int cluster, unsigned int on)
> +{
> +	unsigned long data;
> +
> +	if (!fabric)
> +		return;
> +	data = readl_relaxed(fabric + FAB_SF_MODE);
> +	if (on)
> +		data |= 1 << cluster;
> +	else
> +		data &= ~(1 << cluster);
> +	writel_relaxed(data, fabric + FAB_SF_MODE);
> +	while (1) {
> +		if (data == readl_relaxed(fabric + FAB_SF_MODE))
> +			break;
> +	}
> +}
> +
> +static int hip04_mcpm_power_up(unsigned int cpu, unsigned int cluster)
> +{
> +	unsigned long data, mask;
> +
> +	if (!relocation || !sysctrl)
> +		return -ENODEV;
> +	if (cluster >= HIP04_MAX_CLUSTERS || cpu >= HIP04_MAX_CPUS_PER_CLUSTER)
> +		return -EINVAL;
> +
> +	spin_lock(&boot_lock);
> +	writel_relaxed(HIP04_BOOTWRAPPER_PHYS, relocation);
> +	writel_relaxed(HIP04_BOOTWRAPPER_MAGIC, relocation + 4);
> +	writel_relaxed(virt_to_phys(mcpm_entry_point), relocation + 8);
> +	writel_relaxed(0, relocation + 12);
> +
> +	if (hip04_cluster_down(cluster)) {
> +		data = CLUSTER_L2_RESET_BIT | CLUSTER_DEBUG_RESET_BIT;
> +		writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
> +		do {
> +			mask = CLUSTER_L2_RESET_STATUS | \
> +			       CLUSTER_DEBUG_RESET_STATUS;
> +			data = readl_relaxed(sysctrl + \
> +					     SC_CPU_RESET_STATUS(cluster));
> +		} while (data & mask);
> +		hip04_set_snoop_filter(cluster, 1);
> +	}
> +
> +	hip04_cpu_table[cluster][cpu]++;
> +
> +	data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
> +	       CORE_DEBUG_RESET_BIT(cpu);
> +	writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
> +	spin_unlock(&boot_lock);
> +
> +	return 0;
> +}
> +
> +static void hip04_mcpm_power_down(void)
> +{
> +	unsigned int mpidr, cpu, cluster;
> +	unsigned int v;
> +
> +	spin_lock(&boot_lock);
> +	spin_unlock(&boot_lock);
> +
> +	mpidr = read_cpuid_mpidr();
> +	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> +	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> +
> +	local_irq_disable();
> +	gic_cpu_if_down();
> +
> +	__mcpm_cpu_down(cpu, cluster);
> +
> +	asm volatile(
> +	"	mrc	p15, 0, %0, c1, c0, 0\n"
> +	"	bic	%0, %0, %1\n"
> +	"	mcr	p15, 0, %0, c1, c0, 0\n"
> +	  : "=&r" (v)
> +	  : "Ir" (CR_C)
> +	  : "cc");
> +
> +	flush_cache_louis();
> +
> +	asm volatile(
> +	/*
> +	* Turn off coherency
> +	*/
> +	"	mrc	p15, 0, %0, c1, c0, 1\n"
> +	"	bic	%0, %0, %1\n"
> +	"	mcr	p15, 0, %0, c1, c0, 1\n"
> +	: "=&r" (v)
> +	: "Ir" (0x40)
> +	: "cc");
> +
> +	isb();
> +	dsb();
> +}
> +
> +static int hip04_mcpm_power_down_finish(unsigned int cpu, unsigned int cluster)
> +{
> +	int ret = -EBUSY;
> +
> +	spin_lock(&boot_lock);
> +	BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
> +	__mcpm_cpu_going_down(cpu, cluster);
> +
> +	hip04_cpu_table[cluster][cpu]--;
> +	if (hip04_cpu_table[cluster][cpu]) {
> +		pr_err("Cluster %d CPU%d is still running\n", cluster, cpu);
> +		goto out;
> +	}
> +	ret = 0;
> +out:
> +	spin_unlock(&boot_lock);
> +	return ret;
> +}
> +
> +static void hip04_mcpm_powered_up(void)
> +{
> +	if (!relocation)
> +		return;
> +	spin_lock(&boot_lock);
> +	writel_relaxed(0, relocation);
> +	writel_relaxed(0, relocation + 4);
> +	writel_relaxed(0, relocation + 8);
> +	writel_relaxed(0, relocation + 12);
> +	spin_unlock(&boot_lock);
> +}
> +
> +static const struct mcpm_platform_ops hip04_mcpm_ops = {
> +	.power_up		= hip04_mcpm_power_up,
> +	.power_down		= hip04_mcpm_power_down,
> +	.power_down_finish	= hip04_mcpm_power_down_finish,
> +	.powered_up		= hip04_mcpm_powered_up,
> +};
> +
> +static bool __init hip04_cpu_table_init(void)
> +{
> +	unsigned int mpidr, cpu, cluster;
> +
> +	mpidr = read_cpuid_mpidr();
> +	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> +	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> +
> +	if (cluster >= HIP04_MAX_CLUSTERS ||
> +	    cpu >= HIP04_MAX_CPUS_PER_CLUSTER) {
> +		pr_err("%s: boot CPU is out of bound!\n", __func__);
> +		return false;
> +	}
> +	hip04_set_snoop_filter(cluster, 1);
> +	hip04_cpu_table[cluster][cpu] = 1;
> +	return true;
> +}
> +
> +static int __init hip04_mcpm_init(void)
> +{
> +	struct device_node *np;
> +	int ret = -ENODEV;
> +
> +	np = of_find_compatible_node(NULL, NULL, "hisilicon,hip04-mcpm");
> +	if (!np) {
> +		pr_err("failed to find hisilicon,hip04-mcpm node\n");
> +		goto err;
> +	}
> +	relocation = of_iomap(np, 0);
> +	if (!relocation) {
> +		pr_err("failed to get relocation space\n");
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +	sysctrl = of_iomap(np, 1);
> +	if (!sysctrl) {
> +		pr_err("failed to get sysctrl base\n");
> +		ret = -ENOMEM;
> +		goto err_sysctrl;
> +	}
> +	fabric = of_iomap(np, 2);
> +	if (!fabric) {
> +		pr_err("failed to get fabric base\n");
> +		ret = -ENOMEM;
> +		goto err_fabric;
> +	}
> +	if (!hip04_cpu_table_init())
> +		return -EINVAL;
> +	ret = mcpm_platform_register(&hip04_mcpm_ops);
> +	if (!ret) {
> +		mcpm_sync_init(NULL);
> +		pr_info("HiP04 MCPM initialized\n");
> +	}
> +	return ret;
> +err_fabric:
> +	iounmap(sysctrl);
> +err_sysctrl:
> +	iounmap(relocation);
> +err:
> +	return ret;
> +}
> +early_initcall(hip04_mcpm_init);
> +
> +bool __init hip04_smp_init_ops(void)
> +{
> +	mcpm_smp_set_ops();
> +	return true;
> +}
> +#endif	/* CONFIG_MCPM */
> -- 
> 1.8.3.2
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 06/12] ARM: mcpm: change max clusters to 4

[Nicolas Pitre]

On Thu, 10 Apr 2014, Dave Martin wrote:

> On Tue, Apr 08, 2014 at 04:00:46PM +0800, Haojian Zhuang wrote:
> > In order to support 4 clusters with 4 Cortex A15 Cores in each cluster,
> > enlarge maximum clusters from 2 to 4 in MCPM.
> 
> CC Nico on mcpm patches please.  I'm happy to be CC'd as well.

Indeed.

> > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> > ---
> >  arch/arm/include/asm/mcpm.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/include/asm/mcpm.h b/arch/arm/include/asm/mcpm.h
> > index 608516e..68f82cf 100644
> > --- a/arch/arm/include/asm/mcpm.h
> > +++ b/arch/arm/include/asm/mcpm.h
> > @@ -20,7 +20,7 @@
> >   * to consider dynamic allocation.
> >   */
> >  #define MAX_CPUS_PER_CLUSTER	4
> > -#define MAX_NR_CLUSTERS		2
> > +#define MAX_NR_CLUSTERS		4
> 
> Because of the need for alignment to the biggest cacheline size in the
> system, the MCPM low-level locking structures consume a non-trivial
> amount of memory.
> 
> Therefore, I'm not keen on the idea of simply increasing this #define
> every time a platform appears with a larger number of clusters.
> If hip04 is not built into the kernel, this just wastes memory.
> 
> I'll leave it to Nico to decide whether we can increase the #define
> to 4 or whether this needs a proper fix now.  Ideally, we would have
> a way of choosing the maximum value required by the set of boards built
> into the kernel, or switch to dynamic allocation.

I think we should go with the ability to select a maximum based on the 
configured platforms. That could be as simple as having a 
CONFIG_MCPM_QUAD_CLUSTER symbol to be selected by those platforms that 
need it.

The memory usage is still relatively low: 4 clusters containing 6 
independent cache lines each, so for a 64-byte cache line this means 
1536 bytes.  That is rather insignificant compared to the amount of 
memory fitted to typical multi-cluster systems, especially quad cluster 
systems.  Unless there is yet more expansion of clusters and/or CPUs per 
cluster to come amongst MCPM users, I don't think we've yet reached the 
tipping point where the complexity of dynamic memory allocation for this 
is worth it.

However, changing MAX_NR_CLUSTERS cannot be done without also modifying 
the code in bL_switcher_init() or the b.L switcher won't work anymore on 
dual cluster systems as soon as a quad cluster system is configured in. 
Simply removing the test against MAX_NR_CLUSTERS should be sufficient as 
there is already a runtime validation test in bL_switcher_halve_cpus().


Nicolas

[PATCH v2 06/12] ARM: mcpm: change max clusters to 4

[Dave Martin]

On Thu, Apr 10, 2014 at 10:39:04PM -0400, Nicolas Pitre wrote:
> On Thu, 10 Apr 2014, Dave Martin wrote:
> 
> > On Tue, Apr 08, 2014 at 04:00:46PM +0800, Haojian Zhuang wrote:
> > > In order to support 4 clusters with 4 Cortex A15 Cores in each cluster,
> > > enlarge maximum clusters from 2 to 4 in MCPM.
> > 
> > CC Nico on mcpm patches please.  I'm happy to be CC'd as well.
> 
> Indeed.
> 
> > > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> > > ---
> > >  arch/arm/include/asm/mcpm.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm/include/asm/mcpm.h b/arch/arm/include/asm/mcpm.h
> > > index 608516e..68f82cf 100644
> > > --- a/arch/arm/include/asm/mcpm.h
> > > +++ b/arch/arm/include/asm/mcpm.h
> > > @@ -20,7 +20,7 @@
> > >   * to consider dynamic allocation.
> > >   */
> > >  #define MAX_CPUS_PER_CLUSTER	4
> > > -#define MAX_NR_CLUSTERS		2
> > > +#define MAX_NR_CLUSTERS		4
> > 
> > Because of the need for alignment to the biggest cacheline size in the
> > system, the MCPM low-level locking structures consume a non-trivial
> > amount of memory.
> > 
> > Therefore, I'm not keen on the idea of simply increasing this #define
> > every time a platform appears with a larger number of clusters.
> > If hip04 is not built into the kernel, this just wastes memory.
> > 
> > I'll leave it to Nico to decide whether we can increase the #define
> > to 4 or whether this needs a proper fix now.  Ideally, we would have
> > a way of choosing the maximum value required by the set of boards built
> > into the kernel, or switch to dynamic allocation.
> 
> I think we should go with the ability to select a maximum based on the 
> configured platforms. That could be as simple as having a 
> CONFIG_MCPM_QUAD_CLUSTER symbol to be selected by those platforms that 
> need it.

Yes, that could work.

We could use a similar approach for __CACHE_WRITEBACK_ORDER, but at
the moment we are not aware of a platform that contains caches with
lines larger than 64 bytes.  We shouldn't address that until/unless
it's needed.

> The memory usage is still relatively low: 4 clusters containing 6 
> independent cache lines each, so for a 64-byte cache line this means 
> 1536 bytes.  That is rather insignificant compared to the amount of 
> memory fitted to typical multi-cluster systems, especially quad cluster 
> systems.  Unless there is yet more expansion of clusters and/or CPUs per 
> cluster to come amongst MCPM users, I don't think we've yet reached the 
> tipping point where the complexity of dynamic memory allocation for this 
> is worth it.

I agree with this.  If we se 16 clusters somewhere, it might be time to
rethink, but dynamic allocation seems like overkill for 4.

> However, changing MAX_NR_CLUSTERS cannot be done without also modifying 
> the code in bL_switcher_init() or the b.L switcher won't work anymore on 
> dual cluster systems as soon as a quad cluster system is configured in. 
> Simply removing the test against MAX_NR_CLUSTERS should be sufficient as 
> there is already a runtime validation test in bL_switcher_halve_cpus().

Agreed, that will need some attention, but it doesn't sound too painful.

Cheers
---Dave

[PATCH v2 02/12] ARM: append ARCH_MULTI_V7_LPAE

[Haojian Zhuang]

On 8 April 2014 18:59, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 08 April 2014 16:00:42 Haojian Zhuang wrote:
>> Add new ARCH_MULTI_V7_LPAE config. It will select ARM_LPAE only both
>> ARCH_MULTI_V6 & ARCH_MULTI_V7 is disabled.
>>
>> ARCH_MULTI_V7 means non-LPAE platform.
>>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>
> I think it's good to have this option, but it should be added more carefully.
>
>>  arch/arm/Kconfig | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 5248923..a8b2b45 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -918,12 +918,19 @@ config ARCH_MULTI_V6
>>       select CPU_V6K
>>
>>  config ARCH_MULTI_V7
>> -     bool "ARMv7 based platforms (Cortex-A, PJ4, Scorpion, Krait)"
>> +     bool "ARMv7 non-LPAE based platforms (Cortex-A, PJ4, Scorpion, Krait)"
>>       default y
>>       select ARCH_MULTI_V6_V7
>>       select CPU_V7
>>       select HAVE_SMP
>>
>> +config ARCH_MULTI_V7_LPAE
>> +     bool "ARMv7 LPAE based platforms (HIP04)"
>> +     select ARCH_MULTI_V6_V7
>> +     select CPU_V7
>> +     select HAVE_SMP
>> +     select ARM_LPAE if !(ARCH_MULTI_V6 || ARCH_MULTI_V7)
>> +
>>  config ARCH_MULTI_V6_V7
>>       bool
>>       select MIGHT_HAVE_CACHE_L2X0
>
> Two points here:
>
> a) The other options all list the CPU types (Cortex-A, PJ4, ...),
>    while the one you introduce lists the platform name. I think they
>    should all list the CPU cores, like
>
>   bool "ARMv7 non-LPAE based platforms (Cortex-A5/A8/A9, PJ4, Scorpion)"
>
>   bool "ARMv7 non-LPAE based platforms (Cortex-A7/A12/A15/A17, Brahma-B15, PJ4B, Krait)"
>   (I'm not completely sure about Krait, need to check that).
>
I think that you're saying ARMv7 LPAE based platforms (Cortex-A7/...) at here.

> b) When you introduce this, please change the other platforms over to use
> the symbol correctly. We already have a number of platforms based on A7, A15,
> PJ4B and Krait.
>
Sure, I'll update all the platforms.

Regards
Haojian

[PATCH v2 03/12] ARM: hisi: add ARCH_HISI

[Haojian Zhuang]

On 8 April 2014 19:13, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 08 April 2014 16:00:43 Haojian Zhuang wrote:
>> diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
>> index feee4db..da16efd 100644
>> --- a/arch/arm/mach-hisi/Kconfig
>> +++ b/arch/arm/mach-hisi/Kconfig
>> @@ -1,8 +1,16 @@
>> -config ARCH_HI3xxx
>> -       bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
>> +config ARCH_HISI
>> +       bool "Hisilicon SoC Support"
>> +       depends on ARCH_MULTIPLATFORM
>>         select ARM_AMBA
>>         select ARM_GIC
>>         select ARM_TIMER_SP804
>> +
>> +if ARCH_HISI
>> +
>> +menu "Hisilicon platform type"
>> +
>> +config ARCH_HI3xxx
>> +       bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
>>         select CACHE_L2X0
>>         select HAVE_ARM_SCU if SMP
>>         select HAVE_ARM_TWD if SMP
>> @@ -10,3 +18,7 @@ config ARCH_HI3xxx
>>         select PINCTRL_SINGLE
>>         help
>>           Support for Hisilicon Hi36xx/Hi37xx processor family
>> +
>> +endmenu
>
> On second thought: You don't actually have any sub-options under
> the main SoC specific option, so using 'menuconfig' as I suggested
> first makes no sense.
>
> Just leave ARCH_HI3xxx and ARCH_HIP04 as top-level options and
> drop this patch. There is no real need for a new menu here.
> That also means you can drop patch 9.
>
>         Arnd

Since they're two archs. In arch/arm/Kconfig, I have to write as below.

machine-$(CONFIG_ARCH_HI3xxx)            += mach-hisi
machine-$(CONFIG_ARCH_HIP04)             += mach-hisi

Then mach-hisi is duplicated in Kconfig. I'll get build error in below.

Makefile:877: target `arch/arm/mach-hisi' given more than once in the same rule.

Regards
Haojian

[PATCH v2 03/12] ARM: hisi: add ARCH_HISI

[Arnd Bergmann]

On Monday 14 April 2014 15:57:08 Haojian Zhuang wrote:
> On 8 April 2014 19:13, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 08 April 2014 16:00:43 Haojian Zhuang wrote:
> >> diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
> >> index feee4db..da16efd 100644
> >> --- a/arch/arm/mach-hisi/Kconfig
> >> +++ b/arch/arm/mach-hisi/Kconfig
> >> @@ -1,8 +1,16 @@
> >> -config ARCH_HI3xxx
> >> -       bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
> >> +config ARCH_HISI
> >> +       bool "Hisilicon SoC Support"
> >> +       depends on ARCH_MULTIPLATFORM
> >>         select ARM_AMBA
> >>         select ARM_GIC
> >>         select ARM_TIMER_SP804
> >> +
> >> +if ARCH_HISI
> >> +
> >> +menu "Hisilicon platform type"
> >> +
> >> +config ARCH_HI3xxx
> >> +       bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
> >>         select CACHE_L2X0
> >>         select HAVE_ARM_SCU if SMP
> >>         select HAVE_ARM_TWD if SMP
> >> @@ -10,3 +18,7 @@ config ARCH_HI3xxx
> >>         select PINCTRL_SINGLE
> >>         help
> >>           Support for Hisilicon Hi36xx/Hi37xx processor family
> >> +
> >> +endmenu
> >
> > On second thought: You don't actually have any sub-options under
> > the main SoC specific option, so using 'menuconfig' as I suggested
> > first makes no sense.
> >
> > Just leave ARCH_HI3xxx and ARCH_HIP04 as top-level options and
> > drop this patch. There is no real need for a new menu here.
> > That also means you can drop patch 9.
> >
> >         Arnd
> 
> Since they're two archs. In arch/arm/Kconfig, I have to write as below.
> 
> machine-$(CONFIG_ARCH_HI3xxx)            += mach-hisi
> machine-$(CONFIG_ARCH_HIP04)             += mach-hisi
> 
> Then mach-hisi is duplicated in Kconfig. I'll get build error in below.
> 
> Makefile:877: target `arch/arm/mach-hisi' given more than once in the same rule.

Ah, you are right.

You can use a top-level option then, but please use 'menuconfig for it'
Alternatively, you could make ARCH_HISI a hidden symbol like

config ARCH_HISI
	def_bool ARCH_HI3xxx || ARCH_HIP04

and use that one in the Makefile.

	Arnd

[PATCH v2 06/12] ARM: mcpm: change max clusters to 4

[Haojian Zhuang]

On 11 April 2014 10:39, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Thu, 10 Apr 2014, Dave Martin wrote:
>
>> On Tue, Apr 08, 2014 at 04:00:46PM +0800, Haojian Zhuang wrote:
>> > In order to support 4 clusters with 4 Cortex A15 Cores in each cluster,
>> > enlarge maximum clusters from 2 to 4 in MCPM.
>>
>> CC Nico on mcpm patches please.  I'm happy to be CC'd as well.
>
> Indeed.
>
>> > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>> > ---
>> >  arch/arm/include/asm/mcpm.h | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/arch/arm/include/asm/mcpm.h b/arch/arm/include/asm/mcpm.h
>> > index 608516e..68f82cf 100644
>> > --- a/arch/arm/include/asm/mcpm.h
>> > +++ b/arch/arm/include/asm/mcpm.h
>> > @@ -20,7 +20,7 @@
>> >   * to consider dynamic allocation.
>> >   */
>> >  #define MAX_CPUS_PER_CLUSTER       4
>> > -#define MAX_NR_CLUSTERS            2
>> > +#define MAX_NR_CLUSTERS            4
>>
>> Because of the need for alignment to the biggest cacheline size in the
>> system, the MCPM low-level locking structures consume a non-trivial
>> amount of memory.
>>
>> Therefore, I'm not keen on the idea of simply increasing this #define
>> every time a platform appears with a larger number of clusters.
>> If hip04 is not built into the kernel, this just wastes memory.
>>
>> I'll leave it to Nico to decide whether we can increase the #define
>> to 4 or whether this needs a proper fix now.  Ideally, we would have
>> a way of choosing the maximum value required by the set of boards built
>> into the kernel, or switch to dynamic allocation.
>
> I think we should go with the ability to select a maximum based on the
> configured platforms. That could be as simple as having a
> CONFIG_MCPM_QUAD_CLUSTER symbol to be selected by those platforms that
> need it.
>

I'll set MAX_NR_CLUSTERS from Kconfig.

If BIG_LITTLE isn't enabled & HIP04 is enabled, MAX_NR_CLUSTERS could
be set to 4.
Otherwise, it's always 2. Since there're only 16 A15 cores in HiP04.

Regards
Haojian

[PATCH v2 07/12] ARM: hisi: add hip04 SoC support

[Haojian Zhuang]

On 8 April 2014 19:10, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 08 April 2014 16:00:47 Haojian Zhuang wrote:
>> diff --git a/arch/arm/mach-hisi/hisilicon.c b/arch/arm/mach-hisi/hisilicon.c
>> index 741faf3..10a605f 100644
>> --- a/arch/arm/mach-hisi/hisilicon.c
>> +++ b/arch/arm/mach-hisi/hisilicon.c
>> @@ -14,6 +14,7 @@
>>  #include <linux/clk-provider.h>
>>  #include <linux/clocksource.h>
>>  #include <linux/irqchip.h>
>> +#include <linux/memblock.h>
>>  #include <linux/of_address.h>
>>  #include <linux/of_platform.h>
>>
>> @@ -88,3 +89,21 @@ DT_MACHINE_START(HI3620, "Hisilicon Hi3620 (Flattened Device Tree)")
>>       .smp            = smp_ops(hi3xxx_smp_ops),
>>       .restart        = hi3xxx_restart,
>>  MACHINE_END
>> +
>> +static const char *hip04_compat[] __initconst = {
>> +     "hisilicon,hip04-d01",
>> +     NULL,
>> +};
>> +
>> +static void __init hip04_reserve(void)
>> +{
>> +     memblock_reserve(HIP04_BOOTWRAPPER_PHYS, HIP04_BOOTWRAPPER_SIZE);
>> +}
>
> Can you explain why you do this? Shouldn't that memory just be listed
> in the DT?

In current implementation, boot code of secondary CPUs are stored in the memory
of HIP04_BOOTWRAPPER_PHYS.

If the bootwrapper could be removed, we need to refresh the code on booting
up secondary CPU.
>
>> +DT_MACHINE_START(HIP04, "Hisilicon HiP04 (Flattened Device Tree)")
>> +     .dt_compat      = hip04_compat,
>> +#ifdef CONFIG_MCPM
>> +     .smp_init       = smp_init_ops(hip04_smp_init_ops),
>> +#endif
>> +     .reserve        = hip04_reserve,
>> +MACHINE_END
>
> I think the #ifdef is not needed here, you already hide hip04_smp_init_ops
> if CONFIG_SMP is disabled, and SMP implies MCPM based on your Kconfig.
>

If I build only ARCH_HI3xxx without ARCH_HIP04, CONFIG_MCPM won't be
selected. Then I'll meet the build error.

>> diff --git a/arch/arm/mach-hisi/platsmp.c b/arch/arm/mach-hisi/platsmp.c
>> index 471f1ee..3a5833f 100644
>> --- a/arch/arm/mach-hisi/platsmp.c
>> +++ b/arch/arm/mach-hisi/platsmp.c
>
>
>>  };
>> +
>> +#ifdef CONFIG_MCPM
>> +/* bits definition in SC_CPU_RESET_REQ[x]/SC_CPU_RESET_DREQ[x]
>> + * 1 -- unreset; 0 -- reset
>> + */
>> ...
>
> It would seem appropriate to put all of this into a separate
> source file -- there is no shared code between the two SMP
> implementations.
>

Sure. I'll move the contents into platmcpm.c in mach-hisi directory.

>> +static int __init hip04_mcpm_init(void)
>> +{
>> +     struct device_node *np;
>> +     int ret = -ENODEV;
>> +
>> +     np = of_find_compatible_node(NULL, NULL, "hisilicon,hip04-mcpm");
>> +     if (!np) {
>> +             pr_err("failed to find hisilicon,hip04-mcpm node\n");
>> +             goto err;
>> +     }
>
> This shouldn't be a fatal error: if you run the kernel on a platform
> other than hip04, the code will still be executed here but it won't
> find the node and should just ignore the device silently.
>

OK. I'll fix it.

>> +     relocation = of_iomap(np, 0);
>> +     if (!relocation) {
>> +             pr_err("failed to get relocation space\n");
>> +             ret = -ENOMEM;
>> +             goto err;
>> +     }
>> +     sysctrl = of_iomap(np, 1);
>> +     if (!sysctrl) {
>> +             pr_err("failed to get sysctrl base\n");
>> +             ret = -ENOMEM;
>> +             goto err_sysctrl;
>> +     }
>
> sysctrl sounds like a shared device that you probably don't want
> to map here, but rather use a "syscon" node.
>
Up to now, only this driver is using it.

>> +}
>> +early_initcall(hip04_mcpm_init);
>
> Actually, I guess it would be better to do this from one of the
> various calls you already have for SMP initialization than
> doing an initcall.
>
>         Arnd

[PATCH v2 07/12] ARM: hisi: add hip04 SoC support

[Haojian Zhuang]

On 10 April 2014 16:50, Mark Rutland <mark.rutland@arm.com> wrote:
> On Tue, Apr 08, 2014 at 09:00:47AM +0100, Haojian Zhuang wrote:
>> Hisilicon Hi3xxx is based on Cortex A9 Core. Now HiP04 SoC is based on
>> Cortex A15 Core. Since multiple clusters is used in HiP04 SoC, it could
>> be based on MCPM.
>>
>>
>> +config ARCH_HIP04
>> +       bool "Hisilicon HiP04 Cortex A15 family" if ARCH_MULTI_V7_LPAE
>> +       select HAVE_ARM_ARCH_TIMER
>> +       select MCPM if SMP
>> +       help
>> +         Support for Hisilicon HiP04 processor family
>
> Nit: Surely this is an SoC family? The processor is Cortex-A15, as you
> state above.
>
Yes, it's SoC, not processor. I'll fix it.

>> +
>>  endmenu
>>
>>  endif
>> diff --git a/arch/arm/mach-hisi/core.h b/arch/arm/mach-hisi/core.h
>> index af23ec2..e008c7a 100644
>> --- a/arch/arm/mach-hisi/core.h
>> +++ b/arch/arm/mach-hisi/core.h
>> @@ -12,4 +12,10 @@ extern void hi3xxx_cpu_die(unsigned int cpu);
>>  extern int hi3xxx_cpu_kill(unsigned int cpu);
>>  extern void hi3xxx_set_cpu(int cpu, bool enable);
>>
>> +#define HIP04_BOOTWRAPPER_PHYS         0x10c00000
>> +#define HIP04_BOOTWRAPPER_MAGIC                0xa5a5a5a5
>> +#define HIP04_BOOTWRAPPER_SIZE         0x00010000
>
> The address and size should come from DT.
>
> What is the magic, exactly? How is it used by the kernel and
> bootwrapper?
>
Of course, I can move it into DT.

>
>> +static void __init hip04_reserve(void)
>> +{
>> +       memblock_reserve(HIP04_BOOTWRAPPER_PHYS, HIP04_BOOTWRAPPER_SIZE);
>> +}
>
> As Arnd said, do this in your DT. This is not a hardware property the
> kernel needs to statically know, but rather a system-specific property
> that needs to be in the DT such that it can vary.
>
Sure. I'll update it.

>
>> +static int hip04_mcpm_power_up(unsigned int cpu, unsigned int cluster)
>> +{
>> +       unsigned long data, mask;
>> +
>> +       if (!relocation || !sysctrl)
>> +               return -ENODEV;
>> +       if (cluster >= HIP04_MAX_CLUSTERS || cpu >= HIP04_MAX_CPUS_PER_CLUSTER)
>> +               return -EINVAL;
>> +
>> +       spin_lock(&boot_lock);
>> +       writel_relaxed(HIP04_BOOTWRAPPER_PHYS, relocation);
>> +       writel_relaxed(HIP04_BOOTWRAPPER_MAGIC, relocation + 4);
>> +       writel_relaxed(virt_to_phys(mcpm_entry_point), relocation + 8);
>
> So the kernel is poking the hardware directly to throw CPUs into a
> bootwrapper that it pokes to throw the CPUs into the kernel? That does
> not strike me as fantastic.
>
> It's a shame we have to manage coherency here at all and don't have a
> PSCI implementation to abstract this. That would mean you wouldn't need
> a machine descriptor at all, could reuse existing infrastructure, and
> you'd be able to use Hyp.
>

Do we need a firmware to support PSCI first? Up to now, I don't have
this kind of firmware.

>
>> +static void hip04_mcpm_power_down(void)
>> +{
>> +       unsigned int mpidr, cpu, cluster;
>> +       unsigned int v;
>> +
>> +       spin_lock(&boot_lock);
>> +       spin_unlock(&boot_lock);
>
> Huh? What's this for?

Actually it could be removed. Since secondary CPU is always booted up
one by one.
>
>> +       asm volatile(
>> +       "       mrc     p15, 0, %0, c1, c0, 0\n"
>> +       "       bic     %0, %0, %1\n"
>> +       "       mcr     p15, 0, %0, c1, c0, 0\n"
>> +         : "=&r" (v)
>> +         : "Ir" (CR_C)
>> +         : "cc");
>
> I don't think that cc clobber is necessary, none of these instructions
> set the flags.

Yes, I'll remove it.
>
>> +       asm volatile(
>> +       /*
>> +       * Turn off coherency
>> +       */
>> +       "       mrc     p15, 0, %0, c1, c0, 1\n"
>> +       "       bic     %0, %0, %1\n"
>> +       "       mcr     p15, 0, %0, c1, c0, 1\n"
>> +       : "=&r" (v)
>> +       : "Ir" (0x40)
>
> 0x40?

Disable SMP bit in ACTLR register.

>
>> +       : "cc");
>
> Likewise I don't think this clobber is necessary.

Yes, I'll remove it.
>
>> +static int __init hip04_mcpm_init(void)
>> +{
>> +       struct device_node *np;
>> +       int ret = -ENODEV;
>> +
>> +       np = of_find_compatible_node(NULL, NULL, "hisilicon,hip04-mcpm");
>> +       if (!np) {
>> +               pr_err("failed to find hisilicon,hip04-mcpm node\n");
>> +               goto err;
>> +       }
>
> MCPM is a Linux-specific software construct. It is not a piece of
> hardware, and not a generic interface name. Please come up with a better
> name for this that makes it clear exactly what you are describing.
>

OK. I'll use "hisilicon,core-power-control" instead.

>> +       relocation = of_iomap(np, 0);
>> +       if (!relocation) {
>> +               pr_err("failed to get relocation space\n");
>> +               ret = -ENOMEM;
>> +               goto err;
>> +       }
>> +       sysctrl = of_iomap(np, 1);
>> +       if (!sysctrl) {
>> +               pr_err("failed to get sysctrl base\n");
>> +               ret = -ENOMEM;
>> +               goto err_sysctrl;
>> +       }
>> +       fabric = of_iomap(np, 2);
>> +       if (!fabric) {
>> +               pr_err("failed to get fabric base\n");
>> +               ret = -ENOMEM;
>> +               goto err_fabric;
>> +       }
>
> These sounds like they would be better described by separate nodes.
>
OK

[PATCH v2 07/12] ARM: hisi: add hip04 SoC support

[Arnd Bergmann]

On Tuesday 15 April 2014 15:02:10 Haojian Zhuang wrote:
> On 8 April 2014 19:10, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 08 April 2014 16:00:47 Haojian Zhuang wrote:
> >> diff --git a/arch/arm/mach-hisi/hisilicon.c b/arch/arm/mach-hisi/hisilicon.c
> >> index 741faf3..10a605f 100644
> >> --- a/arch/arm/mach-hisi/hisilicon.c
> >> +++ b/arch/arm/mach-hisi/hisilicon.c
> >> @@ -14,6 +14,7 @@
> >>  #include <linux/clk-provider.h>
> >>  #include <linux/clocksource.h>
> >>  #include <linux/irqchip.h>
> >> +#include <linux/memblock.h>
> >>  #include <linux/of_address.h>
> >>  #include <linux/of_platform.h>
> >>
> >> @@ -88,3 +89,21 @@ DT_MACHINE_START(HI3620, "Hisilicon Hi3620 (Flattened Device Tree)")
> >>       .smp            = smp_ops(hi3xxx_smp_ops),
> >>       .restart        = hi3xxx_restart,
> >>  MACHINE_END
> >> +
> >> +static const char *hip04_compat[] __initconst = {
> >> +     "hisilicon,hip04-d01",
> >> +     NULL,
> >> +};
> >> +
> >> +static void __init hip04_reserve(void)
> >> +{
> >> +     memblock_reserve(HIP04_BOOTWRAPPER_PHYS, HIP04_BOOTWRAPPER_SIZE);
> >> +}
> >
> > Can you explain why you do this? Shouldn't that memory just be listed
> > in the DT?
> 
> In current implementation, boot code of secondary CPUs are stored in the memory
> of HIP04_BOOTWRAPPER_PHYS.
> 
> If the bootwrapper could be removed, we need to refresh the code on booting
> up secondary CPU.

What I meant was that you should not call memblock_reserve() from an early
function here, but instead pass correct data through the DT so you don't
have to. You can reserve the memory in DT.

If changing the boot code is an option, you might consider just using the
standard PSCI interface. That is definitely the recommended approach, although
I don't know how that interacts with MCPM

> >> +DT_MACHINE_START(HIP04, "Hisilicon HiP04 (Flattened Device Tree)")
> >> +     .dt_compat      = hip04_compat,
> >> +#ifdef CONFIG_MCPM
> >> +     .smp_init       = smp_init_ops(hip04_smp_init_ops),
> >> +#endif
> >> +     .reserve        = hip04_reserve,
> >> +MACHINE_END
> >
> > I think the #ifdef is not needed here, you already hide hip04_smp_init_ops
> > if CONFIG_SMP is disabled, and SMP implies MCPM based on your Kconfig.
> >
> 
> If I build only ARCH_HI3xxx without ARCH_HIP04, CONFIG_MCPM won't be
> selected. Then I'll meet the build error.

I would rather see an #ifdef ARCH_HIP04 around the HIP04 code then.
You could also just use separate files for HI3xxx and HIP04, since
there is nothing shared between the two anyway.

	Arnd

[PATCH v2 06/12] ARM: mcpm: change max clusters to 4

[Dave Martin]

On Tue, Apr 15, 2014 at 02:45:05PM +0800, Haojian Zhuang wrote:
> On 11 April 2014 10:39, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > On Thu, 10 Apr 2014, Dave Martin wrote:
> >
> >> On Tue, Apr 08, 2014 at 04:00:46PM +0800, Haojian Zhuang wrote:
> >> > In order to support 4 clusters with 4 Cortex A15 Cores in each cluster,
> >> > enlarge maximum clusters from 2 to 4 in MCPM.
> >>
> >> CC Nico on mcpm patches please.  I'm happy to be CC'd as well.
> >
> > Indeed.
> >
> >> > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> >> > ---
> >> >  arch/arm/include/asm/mcpm.h | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/arch/arm/include/asm/mcpm.h b/arch/arm/include/asm/mcpm.h
> >> > index 608516e..68f82cf 100644
> >> > --- a/arch/arm/include/asm/mcpm.h
> >> > +++ b/arch/arm/include/asm/mcpm.h
> >> > @@ -20,7 +20,7 @@
> >> >   * to consider dynamic allocation.
> >> >   */
> >> >  #define MAX_CPUS_PER_CLUSTER       4
> >> > -#define MAX_NR_CLUSTERS            2
> >> > +#define MAX_NR_CLUSTERS            4
> >>
> >> Because of the need for alignment to the biggest cacheline size in the
> >> system, the MCPM low-level locking structures consume a non-trivial
> >> amount of memory.
> >>
> >> Therefore, I'm not keen on the idea of simply increasing this #define
> >> every time a platform appears with a larger number of clusters.
> >> If hip04 is not built into the kernel, this just wastes memory.
> >>
> >> I'll leave it to Nico to decide whether we can increase the #define
> >> to 4 or whether this needs a proper fix now.  Ideally, we would have
> >> a way of choosing the maximum value required by the set of boards built
> >> into the kernel, or switch to dynamic allocation.
> >
> > I think we should go with the ability to select a maximum based on the
> > configured platforms. That could be as simple as having a
> > CONFIG_MCPM_QUAD_CLUSTER symbol to be selected by those platforms that
> > need it.
> >
> 
> I'll set MAX_NR_CLUSTERS from Kconfig.
> 
> If BIG_LITTLE isn't enabled & HIP04 is enabled, MAX_NR_CLUSTERS could
> be set to 4.

Perhaps the bL switcher should simply refuse to enable itself if there
are more than 2 clusters, or 2 clusters that are identical.

There's no reason why HIP04 and b.L switcher support shouldn't be built
into the same kernel image, since the decision about whether to enable
the switcher is made at runtime.

Cheers
---Dave

[PATCH v2 06/12] ARM: mcpm: change max clusters to 4

[Nicolas Pitre]

On Tue, 15 Apr 2014, Haojian Zhuang wrote:

> On 11 April 2014 10:39, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > On Thu, 10 Apr 2014, Dave Martin wrote:
> >
> >> On Tue, Apr 08, 2014 at 04:00:46PM +0800, Haojian Zhuang wrote:
> >> > In order to support 4 clusters with 4 Cortex A15 Cores in each cluster,
> >> > enlarge maximum clusters from 2 to 4 in MCPM.
> >>
> >> CC Nico on mcpm patches please.  I'm happy to be CC'd as well.
> >
> > Indeed.
> >
> >> > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> >> > ---
> >> >  arch/arm/include/asm/mcpm.h | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/arch/arm/include/asm/mcpm.h b/arch/arm/include/asm/mcpm.h
> >> > index 608516e..68f82cf 100644
> >> > --- a/arch/arm/include/asm/mcpm.h
> >> > +++ b/arch/arm/include/asm/mcpm.h
> >> > @@ -20,7 +20,7 @@
> >> >   * to consider dynamic allocation.
> >> >   */
> >> >  #define MAX_CPUS_PER_CLUSTER       4
> >> > -#define MAX_NR_CLUSTERS            2
> >> > +#define MAX_NR_CLUSTERS            4
> >>
> >> Because of the need for alignment to the biggest cacheline size in the
> >> system, the MCPM low-level locking structures consume a non-trivial
> >> amount of memory.
> >>
> >> Therefore, I'm not keen on the idea of simply increasing this #define
> >> every time a platform appears with a larger number of clusters.
> >> If hip04 is not built into the kernel, this just wastes memory.
> >>
> >> I'll leave it to Nico to decide whether we can increase the #define
> >> to 4 or whether this needs a proper fix now.  Ideally, we would have
> >> a way of choosing the maximum value required by the set of boards built
> >> into the kernel, or switch to dynamic allocation.
> >
> > I think we should go with the ability to select a maximum based on the
> > configured platforms. That could be as simple as having a
> > CONFIG_MCPM_QUAD_CLUSTER symbol to be selected by those platforms that
> > need it.
> >
> 
> I'll set MAX_NR_CLUSTERS from Kconfig.
> 
> If BIG_LITTLE isn't enabled & HIP04 is enabled, MAX_NR_CLUSTERS could
> be set to 4.
> Otherwise, it's always 2. Since there're only 16 A15 cores in HiP04.

No.

Please let's avoid cross dependencies for things that are conceptually 
unrelated.

Like I suggested, just create a CONFIG_MCPM_QUAD_CLUSTER symbol and 
select it from the HIP04 config.

For multi-platform kernels we must be able to support both BIG_LITTLE 
and HIP04 in the same config.  Whether or not the big.LITTLE code is 
activated can be decided at run time.


Nicolas