[PATCH v4 0/6] add basic rk3288 smp support

[PATCH v4 0/6] add basic rk3288 smp support

[Kever Yang]

rk3288 is qual-core CPU Soc, we enable the smp in this patch.

In version 3 we use regmap and pmu syscon for cpu power on/off.
This should be work after Pankaj Dubey's patch applied:
"syscon: Decouple syscon interface from platform devices"
(https://lkml.org/lkml/2014/9/30/156)

Changes in v4:
- add rockchip,pmu property into cpus.txt
- merge patch "fix up rk3288 smp cpu hotplug" into this patch
- remove "rockchip,rk3288-pmu-sram" because we don't use it here
- remove "[PATCH v3 2/6] ARM: dts: rockchip: use the same pmu node"

Changes in v3:
- add pmu regmap patches in version 3
- use one ops and secondary_starup for all rockchip SOCs
- pick back the power domain operation for cpu hotplug
- remove 'enable-method' from this patch
- add compitable name "rockchip,rk3288-pmu-sram" for pmu-intmem

Changes in v2:
- use rk3288_boot_secondary instead ofsmp_boot_secondary
- discards the power domain operation
- handle the per cpu starup when actived by 'sev'
- adjust the alignment

Kever Yang (6):
  ARM: rockchip: convert to regmap and use pmu syscon if available
  ARM: rockchip: add option to access the pmu via a phandle in
    smp_operations
  ARM: dts: rockchip: add pmu references to cpus nodes
  ARM: rockchip: add basic smp support for rk3288
  ARM: dts: rockchip: add intmem node for rk3288 smp support
  ARM: dts: rockchip: add reset for CPU nodes

 Documentation/devicetree/bindings/arm/cpus.txt |   9 +
 arch/arm/boot/dts/rk3288.dtsi                  |  18 ++
 arch/arm/mach-rockchip/headsmp.S               |   5 +-
 arch/arm/mach-rockchip/platsmp.c               | 223 ++++++++++++++++++++-----
 4 files changed, 211 insertions(+), 44 deletions(-)

-- 
1.9.1

[PATCH v4 1/6] ARM: rockchip: convert to regmap and use pmu syscon if available

[Kever Yang]

From: Heiko Stuebner <heiko@sntech.de>

The pmu register space is - like the GRF - shared by quite some peripherals.
On the rk3188 and rk3288 even parts of the pinctrl are living there.
Therefore we normally shouldn't map it a second time when the syscon
does this already.

Therefore convert the cpu power-domain handling to access the pmu via a
regmap and at first try to get it via the syscon interface.
Getting this syscon will only fail if the pmu node does not have the
"syscon" compatible and thus does not get shared with other drivers.

In this case we map it like before and create the necessary regmap on
top of it.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

Changes in v4: None
Changes in v3:
- add this patch in version 3

Changes in v2: None

 arch/arm/mach-rockchip/platsmp.c | 104 +++++++++++++++++++++++++++++----------
 1 file changed, 78 insertions(+), 26 deletions(-)

diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
index 189684f..4c36fbf 100644
--- a/arch/arm/mach-rockchip/platsmp.c
+++ b/arch/arm/mach-rockchip/platsmp.c
@@ -19,6 +19,8 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
 
 #include <asm/cacheflush.h>
 #include <asm/cp15.h>
@@ -37,23 +39,42 @@ static int ncores;
 
 #define PMU_PWRDN_SCU		4
 
-static void __iomem *pmu_base_addr;
+static struct regmap *pmu;
 
-static inline bool pmu_power_domain_is_on(int pd)
+static int pmu_power_domain_is_on(int pd)
 {
-	return !(readl_relaxed(pmu_base_addr + PMU_PWRDN_ST) & BIT(pd));
+	u32 val;
+	int ret;
+
+	ret = regmap_read(pmu, PMU_PWRDN_ST, &val);
+	if (ret < 0)
+		return ret;
+
+	return !(val & BIT(pd));
 }
 
-static void pmu_set_power_domain(int pd, bool on)
+static int pmu_set_power_domain(int pd, bool on)
 {
-	u32 val = readl_relaxed(pmu_base_addr + PMU_PWRDN_CON);
-	if (on)
-		val &= ~BIT(pd);
-	else
-		val |=  BIT(pd);
-	writel(val, pmu_base_addr + PMU_PWRDN_CON);
-
-	while (pmu_power_domain_is_on(pd) != on) { }
+	u32 val = (on) ? 0 : BIT(pd);
+	int ret;
+
+	ret = regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), val);
+	if (ret < 0) {
+		pr_err("%s: could not update power domain\n", __func__);
+		return ret;
+	}
+
+	ret = -1;
+	while (ret != on) {
+		ret = pmu_power_domain_is_on(pd);
+		if (ret < 0) {
+			pr_err("%s: could not read power domain state\n",
+				 __func__);
+			return ret;
+		}
+	}
+
+	return 0;
 }
 
 /*
@@ -63,7 +84,7 @@ static void pmu_set_power_domain(int pd, bool on)
 static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
 					     struct task_struct *idle)
 {
-	if (!sram_base_addr || !pmu_base_addr) {
+	if (!sram_base_addr || !pmu) {
 		pr_err("%s: sram or pmu missing for cpu boot\n", __func__);
 		return -ENXIO;
 	}
@@ -75,9 +96,7 @@ static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
 	}
 
 	/* start the core */
-	pmu_set_power_domain(0 + cpu, true);
-
-	return 0;
+	return pmu_set_power_domain(0 + cpu, true);
 }
 
 /**
@@ -125,6 +144,48 @@ static int __init rockchip_smp_prepare_sram(struct device_node *node)
 	return 0;
 }
 
+static struct regmap_config rockchip_pmu_regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
+static int __init rockchip_smp_prepare_pmu(void)
+{
+	struct device_node *node;
+	void __iomem *pmu_base;
+
+	pmu = syscon_regmap_lookup_by_compatible("rockchip,rk3066-pmu");
+	if (!IS_ERR(pmu))
+		return 0;
+
+	/* fallback, create our own regmap for the pmu area */
+	pmu = NULL;
+	node = of_find_compatible_node(NULL, NULL, "rockchip,rk3066-pmu");
+	if (!node) {
+		pr_err("%s: could not find pmu dt node\n", __func__);
+		return -ENODEV;
+	}
+
+	pmu_base = of_iomap(node, 0);
+	if (!pmu_base) {
+		pr_err("%s: could not map pmu registers\n", __func__);
+		return -ENOMEM;
+	}
+
+	pmu = regmap_init_mmio(NULL, pmu_base, &rockchip_pmu_regmap_config);
+	if (IS_ERR(pmu)) {
+		int ret = PTR_ERR(pmu);
+
+		iounmap(pmu_base);
+		pmu = NULL;
+		pr_err("%s: regmap init failed\n", __func__);
+		return ret;
+	}
+
+	return 0;
+}
+
 static void __init rockchip_smp_prepare_cpus(unsigned int max_cpus)
 {
 	struct device_node *node;
@@ -151,17 +212,8 @@ static void __init rockchip_smp_prepare_cpus(unsigned int max_cpus)
 	if (rockchip_smp_prepare_sram(node))
 		return;
 
-	node = of_find_compatible_node(NULL, NULL, "rockchip,rk3066-pmu");
-	if (!node) {
-		pr_err("%s: could not find pmu dt node\n", __func__);
+	if (rockchip_smp_prepare_pmu())
 		return;
-	}
-
-	pmu_base_addr = of_iomap(node, 0);
-	if (!pmu_base_addr) {
-		pr_err("%s: could not map pmu registers\n", __func__);
-		return;
-	}
 
 	/* enable the SCU power domain */
 	pmu_set_power_domain(PMU_PWRDN_SCU, true);
-- 
1.9.1

[PATCH v4 2/6] ARM: rockchip: add option to access the pmu via a phandle in smp_operations

[Kever Yang]

From: Heiko Stuebner <heiko@sntech.de>

Makes it possible to define a rockchip,pmu phandle in the cpus node directly
referencing the pmu syscon instead of searching for specific compatible.

The old way of finding the pmu stays of course available.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

Changes in v4:
- add rockchip,pmu property into cpus.txt

Changes in v3:
- add this patch

Changes in v2: None

 Documentation/devicetree/bindings/arm/cpus.txt |  9 +++++++++
 arch/arm/mach-rockchip/platsmp.c               | 13 +++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
index fc44634..b2aacbe 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -227,6 +227,15 @@ nodes to be present and contain the properties described below.
 			# List of phandles to idle state nodes supported
 			  by this cpu [3].
 
+	- rockchip,pmu
+		Usage: optional for systems that have an "enable-method"
+		       property value of "rockchip,rk3066-smp"
+		       While optional, it is the preferred way to get access to
+		       the cpu-core power-domains.
+		Value type: <phandle>
+		Definition: Specifies the syscon node controlling the cpu core
+			    power domains.
+
 Example 1 (dual-cluster big.LITTLE system 32-bit):
 
 	cpus {
diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
index 4c36fbf..57b53b3 100644
--- a/arch/arm/mach-rockchip/platsmp.c
+++ b/arch/arm/mach-rockchip/platsmp.c
@@ -155,6 +155,19 @@ static int __init rockchip_smp_prepare_pmu(void)
 	struct device_node *node;
 	void __iomem *pmu_base;
 
+	/*
+	 * This function is only called via smp_ops->smp_prepare_cpu().
+	 * That only happens if a "/cpus" device tree node exists
+	 * and has an "enable-method" property that selects the SMP
+	 * operations defined herein.
+	 */
+	node = of_find_node_by_path("/cpus");
+
+	pmu = syscon_regmap_lookup_by_phandle(node, "rockchip,pmu");
+	of_node_put(node);
+	if (!IS_ERR(pmu))
+		return 0;
+
 	pmu = syscon_regmap_lookup_by_compatible("rockchip,rk3066-pmu");
 	if (!IS_ERR(pmu))
 		return 0;
-- 
1.9.1

[PATCH v4 3/6] ARM: dts: rockchip: add pmu references to cpus nodes

[Kever Yang]

This patch add pmu reference and enable-method for smp

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

Changes in v4: None
Changes in v3:
- add this patch

Changes in v2: None

 arch/arm/boot/dts/rk3288.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 874e66d..bcfde19 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -46,6 +46,8 @@
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
+		enable-method = "rockchip,rk3066-smp";
+		rockchip,pmu = <&pmu>;
 
 		cpu at 500 {
 			device_type = "cpu";
-- 
1.9.1

[PATCH v4 4/6] ARM: rockchip: add basic smp support for rk3288

[Kever Yang]

This patch add basic rk3288 smp support.

Only cortex-A9 need invalid L1, A7/A12/A15/A17 should not invalid L1, since
for A7/A12/A15, the invalidation would be taken as clean and invalidate.

If you use the software manual invalidation instead of hardware invalidation
(assert l1/l2rstdisable during reset) after reset, there is tiny change that
some cachelines would be in dirty and valid state after reset(since the ram
content would be random value after reset), then the unexpected clean might
lead to system crash.

It is a known issue for the A12/A17 MPCore multiprocessor that the active
processors might be stalled when the individual processor is powered down,
we can avoid this prolbem by softreset the processor before power it down.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

Changes in v4:
- merge patch "fix up rk3288 smp cpu hotplug" into this patch

Changes in v3:
- use one ops and secondary_starup for all rockchip SOCs
- pick back the power domain operation for cpu hotplug

Changes in v2:
- use rk3288_boot_secondary instead ofsmp_boot_secondary
- discards the power domain operation
- handle the per cpu starup when actived by 'sev'

 arch/arm/mach-rockchip/headsmp.S |   5 +-
 arch/arm/mach-rockchip/platsmp.c | 120 +++++++++++++++++++++++++++++++--------
 2 files changed, 100 insertions(+), 25 deletions(-)

diff --git a/arch/arm/mach-rockchip/headsmp.S b/arch/arm/mach-rockchip/headsmp.S
index 73206e3..46c22de 100644
--- a/arch/arm/mach-rockchip/headsmp.S
+++ b/arch/arm/mach-rockchip/headsmp.S
@@ -16,7 +16,10 @@
 #include <linux/init.h>
 
 ENTRY(rockchip_secondary_startup)
-	bl	v7_invalidate_l1
+	mrc	p15, 0, r0, c0, c0, 0	@ read main ID register
+	ldr	r1, =0x00000c09		@ Cortex-A9 primary part number
+	teq	r0, r1
+	beq	v7_invalidate_l1
 	b	secondary_startup
 ENDPROC(rockchip_secondary_startup)
 
diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
index 57b53b3..866579e 100644
--- a/arch/arm/mach-rockchip/platsmp.c
+++ b/arch/arm/mach-rockchip/platsmp.c
@@ -22,6 +22,8 @@
 #include <linux/regmap.h>
 #include <linux/mfd/syscon.h>
 
+#include <linux/reset.h>
+#include <linux/cpu.h>
 #include <asm/cacheflush.h>
 #include <asm/cp15.h>
 #include <asm/smp_scu.h>
@@ -53,11 +55,47 @@ static int pmu_power_domain_is_on(int pd)
 	return !(val & BIT(pd));
 }
 
+struct reset_control *rockchip_get_core_reset(int cpu)
+{
+	struct device *dev = get_cpu_device(cpu);
+	struct device_node *np;
+
+	/* The cpu device is only available after the initial core bringup */
+	if (dev)
+		np = dev->of_node;
+	else
+		np = of_get_cpu_node(cpu, 0);
+
+	return of_reset_control_get(np, NULL);
+}
+
 static int pmu_set_power_domain(int pd, bool on)
 {
 	u32 val = (on) ? 0 : BIT(pd);
 	int ret;
 
+	/*
+	 * We need to soft reset the cpu when we turn off the cpu power domain,
+	 * or else the active processors might be stalled when the individual
+	 * processor is powered down.
+	 */
+	if (read_cpuid_part_number() != ARM_CPU_PART_CORTEX_A9) {
+		struct reset_control *rstc = rockchip_get_core_reset(pd);
+
+		if (IS_ERR(rstc)) {
+			pr_err("%s: could not get reset control for core %d\n",
+			       __func__, pd);
+			return PTR_ERR(rstc);
+		}
+
+		if (on)
+			reset_control_deassert(rstc);
+		else
+			reset_control_assert(rstc);
+
+		reset_control_put(rstc);
+	}
+
 	ret = regmap_update_bits(pmu, PMU_PWRDN_CON, BIT(pd), val);
 	if (ret < 0) {
 		pr_err("%s: could not update power domain\n", __func__);
@@ -84,6 +122,8 @@ static int pmu_set_power_domain(int pd, bool on)
 static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
 					     struct task_struct *idle)
 {
+	int ret;
+
 	if (!sram_base_addr || !pmu) {
 		pr_err("%s: sram or pmu missing for cpu boot\n", __func__);
 		return -ENXIO;
@@ -96,7 +136,26 @@ static int __cpuinit rockchip_boot_secondary(unsigned int cpu,
 	}
 
 	/* start the core */
-	return pmu_set_power_domain(0 + cpu, true);
+	ret = pmu_set_power_domain(0 + cpu, true);
+	if (ret < 0)
+		return ret;
+
+	if (read_cpuid_part() != ARM_CPU_PART_CORTEX_A9) {
+		/* We communicate with the bootrom to active the cpus other
+		 * than cpu0, after a blob of initialize code, they will
+		 * stay@wfe state, once they are actived, they will check
+		 * the mailbox:
+		 * sram_base_addr + 4: 0xdeadbeaf
+		 * sram_base_addr + 8: start address for pc
+		 * */
+		udelay(10);
+		writel(virt_to_phys(rockchip_secondary_startup),
+			sram_base_addr + 8);
+		writel(0xDEADBEAF, sram_base_addr + 4);
+		dsb_sev();
+	}
+
+	return 0;
 }
 
 /**
@@ -129,8 +188,6 @@ static int __init rockchip_smp_prepare_sram(struct device_node *node)
 		return -EINVAL;
 	}
 
-	sram_base_addr = of_iomap(node, 0);
-
 	/* set the boot function for the sram code */
 	rockchip_boot_fn = virt_to_phys(rockchip_secondary_startup);
 
@@ -204,40 +261,55 @@ static void __init rockchip_smp_prepare_cpus(unsigned int max_cpus)
 	struct device_node *node;
 	unsigned int i;
 
-	node = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
-	if (!node) {
-		pr_err("%s: missing scu\n", __func__);
-		return;
-	}
-
-	scu_base_addr = of_iomap(node, 0);
-	if (!scu_base_addr) {
-		pr_err("%s: could not map scu registers\n", __func__);
-		return;
-	}
-
 	node = of_find_compatible_node(NULL, NULL, "rockchip,rk3066-smp-sram");
 	if (!node) {
 		pr_err("%s: could not find sram dt node\n", __func__);
 		return;
 	}
 
-	if (rockchip_smp_prepare_sram(node))
+	sram_base_addr = of_iomap(node, 0);
+	if (!sram_base_addr) {
+		pr_err("%s: could not map sram registers\n", __func__);
 		return;
+	}
 
 	if (rockchip_smp_prepare_pmu())
 		return;
 
-	/* enable the SCU power domain */
-	pmu_set_power_domain(PMU_PWRDN_SCU, true);
+	if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
+		if (rockchip_smp_prepare_sram(node))
+			return;
 
-	/*
-	 * While the number of cpus is gathered from dt, also get the number
-	 * of cores from the scu to verify this value when booting the cores.
-	 */
-	ncores = scu_get_core_count(scu_base_addr);
+		/* enable the SCU power domain */
+		pmu_set_power_domain(PMU_PWRDN_SCU, true);
+
+		node = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
+		if (!node) {
+			pr_err("%s: missing scu\n", __func__);
+			return;
+		}
 
-	scu_enable(scu_base_addr);
+		scu_base_addr = of_iomap(node, 0);
+		if (!scu_base_addr) {
+			pr_err("%s: could not map scu registers\n", __func__);
+			return;
+		}
+
+		/*
+		 * While the number of cpus is gathered from dt, also get the
+		 * number of cores from the scu to verify this value when
+		 * booting the cores.
+		 */
+		ncores = scu_get_core_count(scu_base_addr);
+		pr_err("%s: ncores %d\n", __func__, ncores);
+
+		scu_enable(scu_base_addr);
+	} else {
+		unsigned int l2ctlr;
+
+		asm ("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
+		ncores = ((l2ctlr >> 24) & 0x3) + 1;
+	}
 
 	/* Make sure that all cores except the first are really off */
 	for (i = 1; i < ncores; i++)
-- 
1.9.1

[PATCH v4 4/6] ARM: rockchip: add basic smp support for rk3288

[Doug Anderson]

Kever,

On Mon, Oct 13, 2014 at 1:12 PM, Kever Yang <kever.yang@rock-chips.com> wrote:
> +       /*
> +        * We need to soft reset the cpu when we turn off the cpu power domain,
> +        * or else the active processors might be stalled when the individual
> +        * processor is powered down.
> +        */
> +       if (read_cpuid_part_number() != ARM_CPU_PART_CORTEX_A9) {

I haven't done a full review of this patch, but it seems unlikely that
your uses of read_cpuid_part_number() and read_cpuid_part() in this
patch are correct.  You use both functions and in both cases compare
the results to ARM_CPU_PART_CORTEX_A9.

[PATCH v4 4/6] ARM: rockchip: add basic smp support for rk3288

[Heiko Stübner]

Am Dienstag, 14. Oktober 2014, 13:24:03 schrieb Doug Anderson:
> Kever,
> 
> On Mon, Oct 13, 2014 at 1:12 PM, Kever Yang <kever.yang@rock-chips.com> 
wrote:
> > +       /*
> > +        * We need to soft reset the cpu when we turn off the cpu power
> > domain, +        * or else the active processors might be stalled when
> > the individual +        * processor is powered down.
> > +        */
> > +       if (read_cpuid_part_number() != ARM_CPU_PART_CORTEX_A9) {
> 
> I haven't done a full review of this patch, but it seems unlikely that
> your uses of read_cpuid_part_number() and read_cpuid_part() in this
> patch are correct.  You use both functions and in both cases compare
> the results to ARM_CPU_PART_CORTEX_A9.

I think read_cpuid_part() would be the correct one, as it does

	read_cpuid_id() & ARM_CPU_PART_MASK

which in turn should mask the correct parts of the cpuid to match against 
ARM_CPU_PART_CORTEX_A9 [0].



[0] http://lxr.free-electrons.com/source/arch/arm/include/asm/cputype.h#L77

[PATCH v4 4/6] ARM: rockchip: add basic smp support for rk3288

[Kever Yang]

Heiko,

On 10/14/2014 02:23 PM, Heiko St?bner wrote:
> Am Dienstag, 14. Oktober 2014, 13:24:03 schrieb Doug Anderson:
>> Kever,
>>
>> On Mon, Oct 13, 2014 at 1:12 PM, Kever Yang <kever.yang@rock-chips.com>
> wrote:
>>> +       /*
>>> +        * We need to soft reset the cpu when we turn off the cpu power
>>> domain, +        * or else the active processors might be stalled when
>>> the individual +        * processor is powered down.
>>> +        */
>>> +       if (read_cpuid_part_number() != ARM_CPU_PART_CORTEX_A9) {
>> I haven't done a full review of this patch, but it seems unlikely that
>> your uses of read_cpuid_part_number() and read_cpuid_part() in this
>> patch are correct.  You use both functions and in both cases compare
>> the results to ARM_CPU_PART_CORTEX_A9.
> I think read_cpuid_part() would be the correct one, as it does
You are right, read_cpuid_part() is correct one on kernel next,
I mix up 3.14 kernel and next tree, only read_cpuid_part_number() is 
available
in 3.14 kernel.

I will correct it in my next version, any other changes needed for new 
version?

- Kever.

[PATCH v4 4/6] ARM: rockchip: add basic smp support for rk3288

[Russell King - ARM Linux]

On Tue, Oct 14, 2014 at 02:50:07PM -0700, Kever Yang wrote:
> Heiko,
> 
> On 10/14/2014 02:23 PM, Heiko St?bner wrote:
> >Am Dienstag, 14. Oktober 2014, 13:24:03 schrieb Doug Anderson:
> >>Kever,
> >>
> >>On Mon, Oct 13, 2014 at 1:12 PM, Kever Yang <kever.yang@rock-chips.com>
> >wrote:
> >>>+       /*
> >>>+        * We need to soft reset the cpu when we turn off the cpu power
> >>>domain, +        * or else the active processors might be stalled when
> >>>the individual +        * processor is powered down.
> >>>+        */
> >>>+       if (read_cpuid_part_number() != ARM_CPU_PART_CORTEX_A9) {
> >>I haven't done a full review of this patch, but it seems unlikely that
> >>your uses of read_cpuid_part_number() and read_cpuid_part() in this
> >>patch are correct.  You use both functions and in both cases compare
> >>the results to ARM_CPU_PART_CORTEX_A9.
> >I think read_cpuid_part() would be the correct one, as it does
> You are right, read_cpuid_part() is correct one on kernel next,
> I mix up 3.14 kernel and next tree, only read_cpuid_part_number() is
> available
> in 3.14 kernel.
> 
> I will correct it in my next version, any other changes needed for new
> version?

You need to at the _very_ _minimum_ build test your code against a
recent kernel, and preferably test it to make sure that it works as
you intend.

Developing on such an old kernel version, and hoping that the code is
going to be correct for later kernels isn't a nice idea.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

[PATCH v4 4/6] ARM: rockchip: add basic smp support for rk3288

[Kever Yang]

Russell,

On 10/14/2014 04:37 PM, Russell King - ARM Linux wrote:
> On Tue, Oct 14, 2014 at 02:50:07PM -0700, Kever Yang wrote:
>> Heiko,
>>
>> On 10/14/2014 02:23 PM, Heiko St?bner wrote:
>>> Am Dienstag, 14. Oktober 2014, 13:24:03 schrieb Doug Anderson:
>>>> Kever,
>>>>
>>>> On Mon, Oct 13, 2014 at 1:12 PM, Kever Yang <kever.yang@rock-chips.com>
>>> wrote:
>>>>> +       /*
>>>>> +        * We need to soft reset the cpu when we turn off the cpu power
>>>>> domain, +        * or else the active processors might be stalled when
>>>>> the individual +        * processor is powered down.
>>>>> +        */
>>>>> +       if (read_cpuid_part_number() != ARM_CPU_PART_CORTEX_A9) {
>>>> I haven't done a full review of this patch, but it seems unlikely that
>>>> your uses of read_cpuid_part_number() and read_cpuid_part() in this
>>>> patch are correct.  You use both functions and in both cases compare
>>>> the results to ARM_CPU_PART_CORTEX_A9.
>>> I think read_cpuid_part() would be the correct one, as it does
>> You are right, read_cpuid_part() is correct one on kernel next,
>> I mix up 3.14 kernel and next tree, only read_cpuid_part_number() is
>> available
>> in 3.14 kernel.
>>
>> I will correct it in my next version, any other changes needed for new
>> version?
> You need to at the _very_ _minimum_ build test your code against a
> recent kernel, and preferably test it to make sure that it works as
> you intend.
Thanks for you advice, I'll be more careful next time.

I'm sorry for not test this patch with rk3188(A9 based) board,
I do have test it on top of kernel next with my rk3288 evb(not A9 based),
and I didn't get any warning for read_cpuid_part_number()
during building the image.

- Kever
>
> Developing on such an old kernel version, and hoping that the code is
> going to be correct for later kernels isn't a nice idea.
>

[PATCH v4 4/6] ARM: rockchip: add basic smp support for rk3288

[Russell King - ARM Linux]

On Tue, Oct 14, 2014 at 11:23:40PM +0200, Heiko St?bner wrote:
> Am Dienstag, 14. Oktober 2014, 13:24:03 schrieb Doug Anderson:
> > Kever,
> > 
> > On Mon, Oct 13, 2014 at 1:12 PM, Kever Yang <kever.yang@rock-chips.com> 
> wrote:
> > > +       /*
> > > +        * We need to soft reset the cpu when we turn off the cpu power
> > > domain, +        * or else the active processors might be stalled when
> > > the individual +        * processor is powered down.
> > > +        */
> > > +       if (read_cpuid_part_number() != ARM_CPU_PART_CORTEX_A9) {
> > 
> > I haven't done a full review of this patch, but it seems unlikely that
> > your uses of read_cpuid_part_number() and read_cpuid_part() in this
> > patch are correct.  You use both functions and in both cases compare
> > the results to ARM_CPU_PART_CORTEX_A9.
> 
> I think read_cpuid_part() would be the correct one, as it does
> 
> 	read_cpuid_id() & ARM_CPU_PART_MASK
> 
> which in turn should mask the correct parts of the cpuid to match against 
> ARM_CPU_PART_CORTEX_A9 [0].

The clue is the deprecated message.  Clearly the original author did not
even build test their code, or if they did, they ignored the warnings.
Either way, that's a fairly worrying issue which raises the question:
how well was this code tested in the first place?

Note that the above quoted if() evaluates to always-false.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

[PATCH v4 5/6] ARM: dts: rockchip: add intmem node for rk3288 smp support

[Kever Yang]

This patch add intmem node des which is needed by platsmp.c

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

Changes in v4:
- remove "rockchip,rk3288-pmu-sram" because we don't use it here

Changes in v3:
- remove 'enable-method' from this patch
- add compitable name "rockchip,rk3288-pmu-sram" for pmu-intmem

Changes in v2:
- adjust the alignment

 arch/arm/boot/dts/rk3288.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index bcfde19..726c5c7 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -441,6 +441,18 @@
 		status = "disabled";
 	};
 
+	bus_intmem at ff700000 {
+		compatible = "mmio-sram";
+		reg = <0xff700000 0x18000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0xff700000 0x18000>;
+		smp-sram at 0 {
+			compatible = "rockchip,rk3066-smp-sram";
+			reg = <0x00 0x10>;
+		};
+	};
+
 	pmu: power-management at ff730000 {
 		compatible = "rockchip,rk3288-pmu", "syscon";
 		reg = <0xff730000 0x100>;
-- 
1.9.1

[PATCH v4 6/6] ARM: dts: rockchip: add reset for CPU nodes

[Kever Yang]

This patch add reset for CPU nodes to use the reset controller.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/arm/boot/dts/rk3288.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 726c5c7..cb614fc 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -53,21 +53,25 @@
 			device_type = "cpu";
 			compatible = "arm,cortex-a12";
 			reg = <0x500>;
+			resets = <&cru SRST_CORE0>;
 		};
 		cpu at 501 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a12";
 			reg = <0x501>;
+			resets = <&cru SRST_CORE1>;
 		};
 		cpu at 502 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a12";
 			reg = <0x502>;
+			resets = <&cru SRST_CORE2>;
 		};
 		cpu at 503 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a12";
 			reg = <0x503>;
+			resets = <&cru SRST_CORE3>;
 		};
 	};
 
-- 
1.9.1