multi-cluster power management

CPUs in cluster based systems, such as big.LITTLE, have special needs when entering the kernel due to a hotplug event, or when resuming from a deep sleep mode.

This is vectorized so multiple CPUs can enter the kernel in parallel without serialization.

The mcpm prefix stands for "multi cluster power management", however this is usable on single cluster systems as well. Only the basic structure is introduced here. This will be extended with later patches.

In order not to complexify things more than they currently have to, the planned work to make runtime adjusted MPIDR based indexing and dynamic memory allocation for cluster states is postponed to a later cycle. The MAX_NR_CLUSTERS and MAX_CPUS_PER_CLUSTER static definitions should be sufficient for those systems expected to be available in the near future.

Signed-off-by Nicolas Pitre <[email protected]>

---
  arch/arm/Kconfig                  |  8 ++++
  arch/arm/common/Makefile          |  1 +
  arch/arm/common/mcpm_entry.c      | 29 +++++++++++++
  arch/arm/common/mcpm_head.S       | 86 +++++++++++++++++++++++++++++++++++++++
  arch/arm/include/asm/mcpm_entry.h | 35 ++++++++++++++++
  5 files changed, 159 insertions(+)

create mode 100644 arch/arm/common/mcpm_entry.c create mode 100644 arch/arm/common/mcpm_head.S create mode 100644 arch/arm/include/asm/mcpm_entry.h

arch/arm/Kconfig +8 -0

index 67874b82a4..200f559c1c 100644
@@ -1584,6 +1584,14 @@ config HAVE_ARM_TWD
  	help
  	  This options enables support for the ARM timer and watchdog unit
 
+config CLUSTER_PM
+	bool "Cluster Power Management Infrastructure"
+	depends on CPU_V7 && SMP
+	help
+	  This option provides the common power management infrastructure
+	  for (multi-)cluster based systems, such as big.LITTLE based
+	  systems.
+
  choice
  	prompt "Memory split"
  	default VMSPLIT_3G

arch/arm/common/Makefile +1 -0

index e8a4e58f1b..23e85b1fae 100644
@@ -13,3 +13,4 @@ obj-$(CONFIG_SHARP_PARAM)	+= sharpsl_param.o
  obj-$(CONFIG_SHARP_SCOOP)	+= scoop.o
  obj-$(CONFIG_PCI_HOST_ITE8152)  += it8152.o
  obj-$(CONFIG_ARM_TIMER_SP804)	+= timer-sp.o
+obj-$(CONFIG_CLUSTER_PM)	+= mcpm_head.o mcpm_entry.o

arch/arm/common/mcpm_entry.c new file +14 -0

new file mode 100644
index 0000000000..3a6d7e70fd
@@ -0,0 +1,29 @@
+/*
+ * arch/arm/common/mcpm_entry.c -- entry point for multi-cluster PM
+ *
+ * Created by:  Nicolas Pitre, March 2012
+ * Copyright:   (C) 2012-2013  Linaro Limited
+ *
+ * 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
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+

This is the basic API used to handle the powering up/down of individual CPUs in a (multi-)cluster system. The platform specific backend implementation has the responsibility to also handle the cluster level power as well when the first/last CPU in a cluster is brought up/down.

Signed-off-by Nicolas Pitre <[email protected]>

---

From: Dave Martin <[email protected]>

This provides helper methods to coordinate between CPUs coming down and CPUs going up, as well as documentation on the used algorithms, so that cluster teardown and setup
operations are not done for a cluster simultaneously.

For use in the power_down() implementation:
* __mcpm_cpu_going_down(unsigned int cluster, unsigned int cpu)
* __mcpm_outbound_enter_critical(unsigned int cluster)
* __mcpm_outbound_leave_critical(unsigned int cluster)
* __mcpm_cpu_down(unsigned int cluster, unsigned int cpu)

The power_up_setup() helper should do platform-specific setup in preparation for turning the CPU on, such as invalidating local caches or entering coherency. It must be assembler for now, since it must run before the MMU can be switched on. It is passed the affinity level which should be initialized.

Because the mcpm_sync_struct content is looked-up and modified with the cache enabled or disabled depending on the code path, it is crucial to always ensure proper cache maintenance to update main memory right away. Therefore, any cached write must be followed by a cache clean operation and any cached read must be preceded by a cache invalidate operation (actually a cache flush i.e. clean+invalidate to avoid discarding possible concurrent writes) on the accessed memory.

Also, in order to prevent a cached writer from interfering with an adjacent non-cached writer, we ensure each state variable is located to a separate cache line.

Thanks to Nicolas Pitre and Achin Gupta for the help with this patch.

Signed-off-by Dave Martin <[email protected]>

---

arch/arm/common/mcpm_entry.c +1 -0

index c8c0e2113e..2b83121966 100644
@@ -18,6 +18,7 @@
  #include <asm/proc-fns.h>
  #include <asm/cacheflush.h>
  #include <asm/idmap.h>
+#include <asm/cputype.h>
 
  extern volatile unsigned long mcpm_entry_vectors[MAX_NR_CLUSTERS][MAX_CPUS_PER_CLUSTER];
 

Quoted 36 lines · Nicolas Pitre

From: Dave Martin <[email protected]>

This provides helper methods to coordinate between CPUs coming down and CPUs going up, as well as documentation on the used algorithms, so that cluster teardown and setup
operations are not done for a cluster simultaneously.

For use in the power_down() implementation:
* __mcpm_cpu_going_down(unsigned int cluster, unsigned int cpu)
* __mcpm_outbound_enter_critical(unsigned int cluster)
* __mcpm_outbound_leave_critical(unsigned int cluster)
* __mcpm_cpu_down(unsigned int cluster, unsigned int cpu)

The power_up_setup() helper should do platform-specific setup in preparation for turning the CPU on, such as invalidating local caches or entering coherency. It must be assembler for now, since it must run before the MMU can be switched on. It is passed the affinity level which should be initialized.

Because the mcpm_sync_struct content is looked-up and modified with the cache enabled or disabled depending on the code path, it is crucial to always ensure proper cache maintenance to update main memory right away. Therefore, any cached write must be followed by a cache clean operation and any cached read must be preceded by a cache invalidate operation (actually a cache flush i.e. clean+invalidate to avoid discarding possible concurrent writes) on the accessed memory.

Also, in order to prevent a cached writer from interfering with an adjacent non-cached writer, we ensure each state variable is located to a separate cache line.

Thanks to Nicolas Pitre and Achin Gupta for the help with this patch.

Signed-off-by Dave Martin <[email protected]>

---

[..]

Quoted 13 lines

arch/arm/common/mcpm_entry.c +1 -0

index c8c0e2113e..2b83121966 100644
@@ -18,6 +18,7 @@
  #include <asm/proc-fns.h>
  #include <asm/cacheflush.h>
  #include <asm/idmap.h>
+#include <asm/cputype.h>
 
  extern volatile unsigned long
 mcpm_entry_vectors[MAX_NR_CLUSTERS][MAX_CPUS_PER_CLUSTER];

[...]

arch/arm/common/mcpm_entry.c int mcpm_cpu_powered_up(void) · line 157

Quoted 13 lines

arch/arm/common/mcpm_entry.c +13 -0

+/*
+ * Ensure preceding writes to *p by other CPUs are visible to
+ * subsequent reads by this CPU.  We must be careful not to
+ * discard data simultaneously written by another CPU, hence the
+ * usage of flush rather than invalidate operations.
+ */
+static void __sync_range_r(volatile void *p, size_t size)
+{
+	char *_p = (char *)p;
+
+#ifdef CONFIG_OUTER_CACHE
+	if (outer_cache.flush_range) {
+

You don't need above #ifdef. In case of non-outer
cache the function pointer is null anyways.

		/*

arch/arm/common/mcpm_entry.c int mcpm_cpu_powered_up(void) · line 164

Quoted 12 lines

arch/arm/common/mcpm_entry.c +12 -0

+		 * Ensure dirty data migrated from other CPUs into our cache
+		 * are cleaned out safely before the outer cache is cleaned:
+		 */
+		__cpuc_clean_dcache_area(_p, size);
+
+		/* Clean and invalidate stale data for *p from outer ... */
+		outer_flush_range(__pa(_p), __pa(_p + size));
+	}
+#endif
+
+	/* ... and inner cache: */
+	__cpuc_flush_dcache_area(_p, size);

This will be un-necessary when inner cache is available, no ? May be you can re-arrange the code like below, unless and until you would like to invalidate any speculative fetches during the outer_flush_range()

	__cpuc_clean_dcache_area(_p, size);
	if (outer_cache.flush_range)
		outer_flush_range(__pa(_p), __pa(_p + size));

Quoted 54 lines · Santosh Shilimkar

Quoted 36 lines · Nicolas Pitre

From: Dave Martin <[email protected]>

This provides helper methods to coordinate between CPUs coming down and CPUs going up, as well as documentation on the used algorithms, so that cluster teardown and setup
operations are not done for a cluster simultaneously.

For use in the power_down() implementation:
* __mcpm_cpu_going_down(unsigned int cluster, unsigned int cpu)
* __mcpm_outbound_enter_critical(unsigned int cluster)
* __mcpm_outbound_leave_critical(unsigned int cluster)
* __mcpm_cpu_down(unsigned int cluster, unsigned int cpu)

The power_up_setup() helper should do platform-specific setup in preparation for turning the CPU on, such as invalidating local caches or entering coherency. It must be assembler for now, since it must run before the MMU can be switched on. It is passed the affinity level which should be initialized.

Because the mcpm_sync_struct content is looked-up and modified with the cache enabled or disabled depending on the code path, it is crucial to always ensure proper cache maintenance to update main memory right away. Therefore, any cached write must be followed by a cache clean operation and any cached read must be preceded by a cache invalidate operation (actually a cache flush i.e. clean+invalidate to avoid discarding possible concurrent writes) on the accessed memory.

Also, in order to prevent a cached writer from interfering with an adjacent non-cached writer, we ensure each state variable is located to a separate cache line.

Thanks to Nicolas Pitre and Achin Gupta for the help with this patch.

Signed-off-by Dave Martin <[email protected]>

---

[..]

Quoted 13 lines

arch/arm/common/mcpm_entry.c +1 -0

index c8c0e2113e..2b83121966 100644
@@ -18,6 +18,7 @@
   #include <asm/proc-fns.h>
   #include <asm/cacheflush.h>
   #include <asm/idmap.h>
+#include <asm/cputype.h>
 
   extern volatile unsigned long
 mcpm_entry_vectors[MAX_NR_CLUSTERS][MAX_CPUS_PER_CLUSTER];

[...]

Quoted 15 lines

arch/arm/common/mcpm_entry.c int mcpm_cpu_powered_up(void) · line 157

Quoted 13 lines

arch/arm/common/mcpm_entry.c +13 -0

+/*
+ * Ensure preceding writes to *p by other CPUs are visible to
+ * subsequent reads by this CPU.  We must be careful not to
+ * discard data simultaneously written by another CPU, hence the
+ * usage of flush rather than invalidate operations.
+ */
+static void __sync_range_r(volatile void *p, size_t size)
+{
+	char *_p = (char *)p;
+
+#ifdef CONFIG_OUTER_CACHE
+	if (outer_cache.flush_range) {
+

You don't need above #ifdef. In case of non-outer
cache the function pointer is null anyways.

We do need the #ifdef, because if CONFIG_OUTER_CACHE is not selected then the outer_cache structure simply doesn't exist.

Quoted 18 lines

		/*

arch/arm/common/mcpm_entry.c int mcpm_cpu_powered_up(void) · line 164

Quoted 12 lines

arch/arm/common/mcpm_entry.c +12 -0

+		 * Ensure dirty data migrated from other CPUs into our cache
+		 * are cleaned out safely before the outer cache is cleaned:
+		 */
+		__cpuc_clean_dcache_area(_p, size);
+
+		/* Clean and invalidate stale data for *p from outer ... */
+		outer_flush_range(__pa(_p), __pa(_p + size));
+	}
+#endif
+
+	/* ... and inner cache: */
+	__cpuc_flush_dcache_area(_p, size);

This will be un-necessary when inner cache is available, no ? May be you can re-arrange the code like below, unless and until you would like to invalidate any speculative fetches during the outer_flush_range()

Quoted 3 lines

	__cpuc_clean_dcache_area(_p, size);
	if (outer_cache.flush_range)
		outer_flush_range(__pa(_p), __pa(_p + size));

As you said, the code is sequenced that way to get rid of potential speculative fetch that could happen right before L2 is flushed.

Quoted 72 lines · Nicolas Pitre

Quoted 54 lines · Santosh Shilimkar

Quoted 36 lines · Nicolas Pitre

From: Dave Martin <[email protected]>

This provides helper methods to coordinate between CPUs coming down and CPUs going up, as well as documentation on the used algorithms, so that cluster teardown and setup
operations are not done for a cluster simultaneously.

For use in the power_down() implementation:
* __mcpm_cpu_going_down(unsigned int cluster, unsigned int cpu)
* __mcpm_outbound_enter_critical(unsigned int cluster)
* __mcpm_outbound_leave_critical(unsigned int cluster)
* __mcpm_cpu_down(unsigned int cluster, unsigned int cpu)

The power_up_setup() helper should do platform-specific setup in preparation for turning the CPU on, such as invalidating local caches or entering coherency. It must be assembler for now, since it must run before the MMU can be switched on. It is passed the affinity level which should be initialized.

Because the mcpm_sync_struct content is looked-up and modified with the cache enabled or disabled depending on the code path, it is crucial to always ensure proper cache maintenance to update main memory right away. Therefore, any cached write must be followed by a cache clean operation and any cached read must be preceded by a cache invalidate operation (actually a cache flush i.e. clean+invalidate to avoid discarding possible concurrent writes) on the accessed memory.

Also, in order to prevent a cached writer from interfering with an adjacent non-cached writer, we ensure each state variable is located to a separate cache line.

Thanks to Nicolas Pitre and Achin Gupta for the help with this patch.

Signed-off-by Dave Martin <[email protected]>

---

[..]

Quoted 13 lines

arch/arm/common/mcpm_entry.c +1 -0

index c8c0e2113e..2b83121966 100644
@@ -18,6 +18,7 @@
   #include <asm/proc-fns.h>
   #include <asm/cacheflush.h>
   #include <asm/idmap.h>
+#include <asm/cputype.h>
 
   extern volatile unsigned long
 mcpm_entry_vectors[MAX_NR_CLUSTERS][MAX_CPUS_PER_CLUSTER];

[...]

Quoted 15 lines

arch/arm/common/mcpm_entry.c int mcpm_cpu_powered_up(void) · line 157

Quoted 13 lines

arch/arm/common/mcpm_entry.c +13 -0

+/*
+ * Ensure preceding writes to *p by other CPUs are visible to
+ * subsequent reads by this CPU.  We must be careful not to
+ * discard data simultaneously written by another CPU, hence the
+ * usage of flush rather than invalidate operations.
+ */
+static void __sync_range_r(volatile void *p, size_t size)
+{
+	char *_p = (char *)p;
+
+#ifdef CONFIG_OUTER_CACHE
+	if (outer_cache.flush_range) {
+

You don't need above #ifdef. In case of non-outer
cache the function pointer is null anyways.

Quoted 3 lines

We do need the #ifdef, because if CONFIG_OUTER_CACHE is not selected then the outer_cache structure simply doesn't exist.

You are right. #ifdef in middle of the code looks bit ugly and hence was thinking to avoid it.

Quoted 18 lines

Quoted 18 lines

		/*

arch/arm/common/mcpm_entry.c int mcpm_cpu_powered_up(void) · line 164

Quoted 12 lines

arch/arm/common/mcpm_entry.c +12 -0

+		 * Ensure dirty data migrated from other CPUs into our cache
+		 * are cleaned out safely before the outer cache is cleaned:
+		 */
+		__cpuc_clean_dcache_area(_p, size);
+
+		/* Clean and invalidate stale data for *p from outer ... */
+		outer_flush_range(__pa(_p), __pa(_p + size));
+	}
+#endif
+
+	/* ... and inner cache: */
+	__cpuc_flush_dcache_area(_p, size);

This will be un-necessary when inner cache is available, no ? May be you can re-arrange the code like below, unless and until you would like to invalidate any speculative fetches during the outer_flush_range()

Quoted 7 lines

Quoted 3 lines

	__cpuc_clean_dcache_area(_p, size);
	if (outer_cache.flush_range)
		outer_flush_range(__pa(_p), __pa(_p + size));

As you said, the code is sequenced that way to get rid of potential speculative fetch that could happen right before L2 is flushed.

Thanks for clarifying it. It makes sense.

From: Dave Martin <[email protected]>

This patch adds a simple low-level voting mutex implementation to be used to arbitrate during first man selection when no load/store exclusive instructions are usable.

For want of a better name, these are called "vlocks". (I was tempted to call them ballot locks, but "block" is way too confusing an abbreviation...)

There is no function to wait for the lock to be released, and no vlock_lock() function since we don't need these at the moment. These could straightforwardly be added if vlocks get used for other purposes.

For architectural correctness even Strongly-Ordered memory accesses require barriers in order to guarantee that multiple CPUs have a coherent view of the ordering of memory accesses. Whether or not this matters depends on hardware implementation details of the memory system. Since the purpose of this code is to provide a clean, generic locking mechanism with no platform-specific dependencies the barriers should be present to avoid unpleasant surprises on future platforms.

Note:

* When taking the lock, we don't care about implicit background memory operations and other signalling which may be pending, because those are not part of the critical section anyway.

A DMB is sufficient to ensure correctly observed ordering if the explicit memory accesses in vlock_trylock.

* No barrier is required after checking the election result, because the result is determined by the store to VLOCK_OWNER_OFFSET and is already globally observed due to the barriers in voting_end. This means that global agreement on the winner is guaranteed, even before the winner is known locally.

Signed-off-by Dave Martin <[email protected]>

Signed-off-by Nicolas Pitre <[email protected]>

---

From: Dave Martin <[email protected]>

Instead of requiring the first man to be elected in advance (which can be suboptimal in some situations), this patch uses a per- cluster mutex to co-ordinate selection of the first man.

This should also make it more feasible to reuse this code path for asynchronous cluster resume (as in CPUidle scenarios).

We must ensure that the vlock data doesn't share a cacheline with anything else, or dirty cache eviction could corrupt it.

Signed-off-by Dave Martin <[email protected]>

Signed-off-by Nicolas Pitre <[email protected]>

---

Now that the cluster power API is in place, we can use it for SMP secondary bringup and CPU hotplug in a generic fashion.

Signed-off-by Nicolas Pitre <[email protected]>

---
 arch/arm/common/Makefile       |  2 +-
 arch/arm/common/mcpm_platsmp.c | 85 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/common/mcpm_platsmp.c

arch/arm/common/Makefile +1 -1

index c901a38c59..e1c9db45de 100644
@@ -13,4 +13,4 @@ obj-$(CONFIG_SHARP_PARAM)	+= sharpsl_param.o
 obj-$(CONFIG_SHARP_SCOOP)	+= scoop.o
 obj-$(CONFIG_PCI_HOST_ITE8152)  += it8152.o
 obj-$(CONFIG_ARM_TIMER_SP804)	+= timer-sp.o
-obj-$(CONFIG_CLUSTER_PM)	+= mcpm_head.o mcpm_entry.o vlock.o
+obj-$(CONFIG_CLUSTER_PM)	+= mcpm_head.o mcpm_entry.o mcpm_platsmp.o vlock.o

arch/arm/common/mcpm_platsmp.c new file +13 -0

new file mode 100644
index 0000000000..401298f5ee
@@ -0,0 +1,85 @@
+/*
+ * linux/arch/arm/mach-vexpress/mcpm_platsmp.c
+ *
+ * Created by:  Nicolas Pitre, November 2012
+ * Copyright:   (C) 2012-2013  Linaro Limited
+ *
+ * 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
+ * published by the Free Software Foundation.
+ *
+ * Code to handle secondary CPU bringup and hotplug for the cluster power API.
+ */
+

arch/arm/common/mcpm_platsmp.c new file +9 -0

+}
+
+static int __cpuinit mcpm_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+	unsigned int mpidr, pcpu, pcluster, ret;
+	extern void secondary_startup(void);
+
+	mpidr = cpu_logical_map(cpu);
+	pcpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);

arch/arm/common/mcpm_platsmp.c new file +35 -0

+}
+
+#ifdef CONFIG_HOTPLUG_CPU
+
+static int mcpm_cpu_disable(unsigned int cpu)
+{
+	/*
+	 * We assume all CPUs may be shut down.
+	 * This would be the hook to use for eventual Secure
+	 * OS migration requests as described in the PSCI spec.
+	 */
+	return 0;
+}
+
+static void mcpm_cpu_die(unsigned int cpu)
+{
+	unsigned int mpidr, pcpu, pcluster;
+	mpidr = read_cpuid_mpidr();
+	pcpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+	pcluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+	mcpm_set_entry_vector(pcpu, pcluster, NULL);
+	mcpm_cpu_power_down();
+}
+
+#endif
+
+struct smp_operations __initdata mcpm_smp_ops = {
+	.smp_init_cpus		= simple_smp_init_cpus,
+	.smp_boot_secondary	= mcpm_boot_secondary,
+	.smp_secondary_init	= mcpm_secondary_init,
+#ifdef CONFIG_HOTPLUG_CPU
+	.cpu_disable		= mcpm_cpu_disable,
+	.cpu_die		= mcpm_cpu_die,
+#endif
+};

Now that the cluster power API is in place, we can use it for SMP secondary bringup and CPU hotplug in a generic fashion.

Signed-off-by Nicolas Pitre <[email protected]>

---
  arch/arm/common/Makefile       |  2 +-
  arch/arm/common/mcpm_platsmp.c | 85 ++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 86 insertions(+), 1 deletion(-)

create mode 100644 arch/arm/common/mcpm_platsmp.c

arch/arm/common/Makefile +1 -1

index c901a38c59..e1c9db45de 100644
@@ -13,4 +13,4 @@ obj-$(CONFIG_SHARP_PARAM)	+= sharpsl_param.o
  obj-$(CONFIG_SHARP_SCOOP)	+= scoop.o
  obj-$(CONFIG_PCI_HOST_ITE8152)  += it8152.o
  obj-$(CONFIG_ARM_TIMER_SP804)	+= timer-sp.o
-obj-$(CONFIG_CLUSTER_PM)	+= mcpm_head.o mcpm_entry.o vlock.o
+obj-$(CONFIG_CLUSTER_PM)	+= mcpm_head.o mcpm_entry.o mcpm_platsmp.o vlock.o

arch/arm/common/mcpm_platsmp.c new file +40 -0

new file mode 100644
index 0000000000..401298f5ee
@@ -0,0 +1,85 @@
+/*
+ * linux/arch/arm/mach-vexpress/mcpm_platsmp.c
+ *
+ * Created by:  Nicolas Pitre, November 2012
+ * Copyright:   (C) 2012-2013  Linaro Limited
+ *
+ * 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
+ * published by the Free Software Foundation.
+ *
+ * Code to handle secondary CPU bringup and hotplug for the cluster power API.
+ */
+
+#include <linux/init.h>
+#include <linux/smp.h>
+
+#include <asm/mcpm_entry.h>
+#include <asm/smp_plat.h>
+#include <asm/hardware/gic.h>
+
+static void __init simple_smp_init_cpus(void)
+{
+	set_smp_cross_call(gic_raise_softirq);
+}
+
+static int __cpuinit mcpm_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+	unsigned int mpidr, pcpu, pcluster, ret;
+	extern void secondary_startup(void);
+
+	mpidr = cpu_logical_map(cpu);
+	pcpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+	pcluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+	pr_debug("%s: logical CPU %d is physical CPU %d cluster %d\n",
+		 __func__, cpu, pcpu, pcluster);
+
+	mcpm_set_entry_vector(pcpu, pcluster, NULL);
+	ret = mcpm_cpu_power_up(pcpu, pcluster);
+	if (ret)
+		return ret;

From: Jon Medhurst <[email protected]>

Signed-off-by Jon Medhurst <[email protected]>

---

 arch/arm/include/asm/mach/arch.h |  3 +++
 arch/arm/kernel/setup.c          |  5 ++++-
 arch/arm/mach-vexpress/core.h    |  2 ++
 arch/arm/mach-vexpress/platsmp.c | 12 ++++++++++++
 arch/arm/mach-vexpress/v2m.c     |  2 +-
 5 files changed, 22 insertions(+), 2 deletions(-)

arch/arm/include/asm/mach/arch.h +3 -0

index 917d4fcfd9..3d01c6d6c3 100644
@@ -17,8 +17,10 @@ struct pt_regs;
 struct smp_operations;
 #ifdef CONFIG_SMP
 #define smp_ops(ops) (&(ops))
+#define smp_init_ops(ops) (&(ops))
 #else
 #define smp_ops(ops) (struct smp_operations *)NULL
+#define smp_init_ops(ops) (void (*)(void))NULL
 #endif
 
 struct machine_desc {

@@ -42,6 +44,7 @@ struct machine_desc {
 	unsigned char		reserve_lp2 :1;	/* never has lp2	*/
 	char			restart_mode;	/* default restart mode	*/
 	struct smp_operations	*smp;		/* SMP operations	*/
+	void			(*smp_init)(void);
 	void			(*fixup)(struct tag *, char **,
 					 struct meminfo *);
 	void			(*reserve)(void);/* reserve mem blocks	*/

arch/arm/kernel/setup.c +4 -1

index 3f6cbb2e3e..41edca8582 100644
@@ -768,7 +768,10 @@ void __init setup_arch(char **cmdline_p)
 	arm_dt_init_cpu_maps();
 #ifdef CONFIG_SMP
 	if (is_smp()) {
-		smp_set_ops(mdesc->smp);
+		if(mdesc->smp_init)
+			(*mdesc->smp_init)();
+		else
+			smp_set_ops(mdesc->smp);
 		smp_init_cpus();
 	}
 #endif

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

index f134cd4a85..3a761fd76c 100644
@@ -6,6 +6,8 @@
 
 void vexpress_dt_smp_map_io(void);
 
+void vexpress_smp_init_ops(void);
+
 extern struct smp_operations	vexpress_smp_ops;
 
 extern void vexpress_cpu_die(unsigned int cpu);

arch/arm/mach-vexpress/platsmp.c +12 -0

index c5d70de9bb..667344b479 100644
@@ -12,6 +12,7 @@
 #include <linux/errno.h>
 #include <linux/smp.h>
 #include <linux/io.h>
+#include <linux/of.h>
 #include <linux/of_fdt.h>
 #include <linux/vexpress.h>
 

@@ -206,3 +207,14 @@ struct smp_operations __initdata vexpress_smp_ops = {
 	.cpu_die		= vexpress_cpu_die,
 #endif
 };
+
+void __init vexpress_smp_init_ops(void)
+{
+	struct smp_operations *ops = &vexpress_smp_ops;
+#ifdef CONFIG_CLUSTER_PM
+	extern struct smp_operations mcpm_smp_ops;
+	if(of_find_compatible_node(NULL, NULL, "arm,cci"))
+		ops = &mcpm_smp_ops;
+#endif
+	smp_set_ops(ops);
+}

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

index 011661a6c5..34172bd504 100644
@@ -494,7 +494,7 @@ static const char * const v2m_dt_match[] __initconst = {
 
 DT_MACHINE_START(VEXPRESS_DT, "ARM-Versatile Express")
 	.dt_compat	= v2m_dt_match,
-	.smp		= smp_ops(vexpress_smp_ops),
+	.smp_init	= smp_init_ops(vexpress_smp_init_ops),
 	.map_io		= v2m_dt_map_io,
 	.init_early	= v2m_dt_init_early,
 	.init_irq	= v2m_dt_init_irq,

Quoted 4 lines · Nicolas Pitre

From: Jon Medhurst <[email protected]>

Signed-off-by Jon Medhurst <[email protected]>

---

Should this patch be split into two. One to introduce the new smp_init hook into the generic ARM kernel code, and one to make vexpress use it? With, descriptions something like:

-----------------------------------------------------------------------
ARM: kernel: Enable selection of SMP operations at boot time

Add a new 'smp_init' hook to machine_desc so platforms can specify a function to be used to setup smp ops instead of having a statically defined value.
-----------------------------------------------------------------------
ARM: vexpress: Select multi-cluster SMP operation if required
-----------------------------------------------------------------------

From: Jon Medhurst <[email protected]>

Signed-off-by Jon Medhurst <[email protected]>

---
  arch/arm/include/asm/mach/arch.h |  3 +++
  arch/arm/kernel/setup.c          |  5 ++++-
  arch/arm/mach-vexpress/core.h    |  2 ++
  arch/arm/mach-vexpress/platsmp.c | 12 ++++++++++++
  arch/arm/mach-vexpress/v2m.c     |  2 +-
  5 files changed, 22 insertions(+), 2 deletions(-)

arch/arm/include/asm/mach/arch.h +3 -0

index 917d4fcfd9..3d01c6d6c3 100644
@@ -17,8 +17,10 @@ struct pt_regs;
  struct smp_operations;
  #ifdef CONFIG_SMP
  #define smp_ops(ops) (&(ops))
+#define smp_init_ops(ops) (&(ops))
  #else
  #define smp_ops(ops) (struct smp_operations *)NULL
+#define smp_init_ops(ops) (void (*)(void))NULL
  #endif
 
  struct machine_desc {

@@ -42,6 +44,7 @@ struct machine_desc {
  	unsigned char		reserve_lp2 :1;	/* never has lp2	*/
  	char			restart_mode;	/* default restart mode	*/
  	struct smp_operations	*smp;		/* SMP operations	*/
+	void			(*smp_init)(void);
  	void			(*fixup)(struct tag *, char **,
  					 struct meminfo *);
  	void			(*reserve)(void);/* reserve mem blocks	*/

arch/arm/kernel/setup.c +4 -1

index 3f6cbb2e3e..41edca8582 100644
@@ -768,7 +768,10 @@ void __init setup_arch(char **cmdline_p)
  	arm_dt_init_cpu_maps();
  #ifdef CONFIG_SMP
  	if (is_smp()) {
-		smp_set_ops(mdesc->smp);
+		if(mdesc->smp_init)
+			(*mdesc->smp_init)();
+		else
+			smp_set_ops(mdesc->smp);
  		smp_init_cpus();
  	}
  #endif

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

index f134cd4a85..3a761fd76c 100644
@@ -6,6 +6,8 @@
 
  void vexpress_dt_smp_map_io(void);
 
+void vexpress_smp_init_ops(void);
+
  extern struct smp_operations	vexpress_smp_ops;
 
  extern void vexpress_cpu_die(unsigned int cpu);

arch/arm/mach-vexpress/platsmp.c +1 -0

index c5d70de9bb..667344b479 100644
@@ -12,6 +12,7 @@
  #include <linux/errno.h>
  #include <linux/smp.h>
  #include <linux/io.h>
+#include <linux/of.h>
  #include <linux/of_fdt.h>
  #include <linux/vexpress.h>
 

Quoted 2 lines · Nicolas Pitre

From: Jon Medhurst <[email protected]>

The patch deserves couple of lines of description here.

Quoted 77 lines

Signed-off-by Jon Medhurst <[email protected]>

---
  arch/arm/include/asm/mach/arch.h |  3 +++
  arch/arm/kernel/setup.c          |  5 ++++-
  arch/arm/mach-vexpress/core.h    |  2 ++
  arch/arm/mach-vexpress/platsmp.c | 12 ++++++++++++
  arch/arm/mach-vexpress/v2m.c     |  2 +-
  5 files changed, 22 insertions(+), 2 deletions(-)

arch/arm/include/asm/mach/arch.h +3 -0

index 917d4fcfd9..3d01c6d6c3 100644
@@ -17,8 +17,10 @@ struct pt_regs;
  struct smp_operations;
  #ifdef CONFIG_SMP
  #define smp_ops(ops) (&(ops))
+#define smp_init_ops(ops) (&(ops))
  #else
  #define smp_ops(ops) (struct smp_operations *)NULL
+#define smp_init_ops(ops) (void (*)(void))NULL
  #endif
 
  struct machine_desc {

@@ -42,6 +44,7 @@ struct machine_desc {
  	unsigned char		reserve_lp2 :1;	/* never has lp2	*/
  	char			restart_mode;	/* default restart mode	*/
  	struct smp_operations	*smp;		/* SMP operations	*/
+	void			(*smp_init)(void);
  	void			(*fixup)(struct tag *, char **,
  					 struct meminfo *);
  	void			(*reserve)(void);/* reserve mem blocks	*/

arch/arm/kernel/setup.c +4 -1

index 3f6cbb2e3e..41edca8582 100644
@@ -768,7 +768,10 @@ void __init setup_arch(char **cmdline_p)
  	arm_dt_init_cpu_maps();
  #ifdef CONFIG_SMP
  	if (is_smp()) {
-		smp_set_ops(mdesc->smp);
+		if(mdesc->smp_init)
+			(*mdesc->smp_init)();
+		else
+			smp_set_ops(mdesc->smp);
  		smp_init_cpus();
  	}
  #endif

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

index f134cd4a85..3a761fd76c 100644
@@ -6,6 +6,8 @@
 
  void vexpress_dt_smp_map_io(void);
 
+void vexpress_smp_init_ops(void);
+
  extern struct smp_operations	vexpress_smp_ops;
 
  extern void vexpress_cpu_die(unsigned int cpu);

arch/arm/mach-vexpress/platsmp.c +1 -0

index c5d70de9bb..667344b479 100644
@@ -12,6 +12,7 @@
  #include <linux/errno.h>
  #include <linux/smp.h>
  #include <linux/io.h>
+#include <linux/of.h>
  #include <linux/of_fdt.h>
  #include <linux/vexpress.h>
 

arch/arm/mach-vexpress/platsmp.c struct smp_operations __initdata vexpress_smp_ops = { · line 214

Quoted 8 lines

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

@@ -206,3 +207,14 @@ struct smp_operations __initdata vexpress_smp_ops = {
  	.cpu_die		= vexpress_cpu_die,
  #endif
  };
+
+void __init vexpress_smp_init_ops(void)
+{
+	struct smp_operations *ops = &vexpress_smp_ops;
+#ifdef CONFIG_CLUSTER_PM

See if you can avoid this #ifdef in the middle of function.

Reviewed-by Santosh Shilimkar<[email protected]>

From: Rob Herring <[email protected]>

Move the private set_auxcr/get_auxcr functions from drivers/cpuidle/cpuidle-calxeda.c so they can be used across platforms.

Signed-off-by Rob Herring <[email protected]>

Cc Russell King <[email protected]>

Acked-by Tony Lindgren <[email protected]>

Signed-off-by Nicolas Pitre <[email protected]>

---

This adds basic CPU and cluster reset controls on RTSM for the A15x4-A7x4 model configuration using the Dual Cluster System Configuration Block (DCSCB).

The cache coherency interconnect (CCI) is not handled yet.

Signed-off-by Nicolas Pitre <[email protected]>

---
  arch/arm/mach-vexpress/Kconfig  |   8 ++
  arch/arm/mach-vexpress/Makefile |   1 +
  arch/arm/mach-vexpress/dcscb.c  | 159 ++++++++++++++++++++++++++++++++++++++++
  3 files changed, 168 insertions(+)

create mode 100644 arch/arm/mach-vexpress/dcscb.c

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

index 52d315b792..f3f92b120a 100644
@@ -52,4 +52,12 @@ config ARCH_VEXPRESS_CORTEX_A5_A9_ERRATA
  config ARCH_VEXPRESS_CA9X4
  	bool "Versatile Express Cortex-A9x4 tile"
 
+config ARCH_VEXPRESS_DCSCB
+	bool "Dual Cluster System Control Block (DCSCB) support"
+	depends on CLUSTER_PM
+	help
+	  Support for the Dual Cluster System Configuration Block (DCSCB).
+	  This is needed to provide CPU and cluster power management
+	  on RTSM.
+
  endmenu

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

index 80b64971fb..2253644054 100644
@@ -6,5 +6,6 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include \
 
  obj-y					:= v2m.o reset.o
  obj-$(CONFIG_ARCH_VEXPRESS_CA9X4)	+= ct-ca9x4.o
+obj-$(CONFIG_ARCH_VEXPRESS_DCSCB)	+= dcscb.o
  obj-$(CONFIG_SMP)			+= platsmp.o
  obj-$(CONFIG_HOTPLUG_CPU)		+= hotplug.o

arch/arm/mach-vexpress/dcscb.c new file +44 -0

new file mode 100644
index 0000000000..677ced9efc
@@ -0,0 +1,159 @@
+/*
+ * arch/arm/mach-vexpress/dcscb.c - Dual Cluster System Control Block
+ *
+ * Created by:	Nicolas Pitre, May 2012
+ * Copyright:	(C) 2012-2013  Linaro Limited
+ *
+ * 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
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/io.h>
+#include <linux/spinlock.h>
+#include <linux/errno.h>
+#include <linux/vexpress.h>
+
+#include <asm/mcpm_entry.h>
+#include <asm/proc-fns.h>
+#include <asm/cacheflush.h>
+#include <asm/cputype.h>
+#include <asm/cp15.h>
+
+
+#define DCSCB_PHYS_BASE	0x60000000
+
+#define RST_HOLD0	0x0
+#define RST_HOLD1	0x4
+#define SYS_SWRESET	0x8
+#define RST_STAT0	0xc
+#define RST_STAT1	0x10
+#define EAG_CFG_R	0x20
+#define EAG_CFG_W	0x24
+#define KFC_CFG_R	0x28
+#define KFC_CFG_W	0x2c
+#define DCS_CFG_R	0x30
+
+/*
+ * We can't use regular spinlocks. In the switcher case, it is possible
+ * for an outbound CPU to call power_down() after its inbound counterpart
+ * is already live using the same logical CPU number which trips lockdep
+ * debugging.
+ */

arch/arm/mach-vexpress/dcscb.c new file +40 -0

+		return -EINVAL;
+
+	/*
+	 * Since this is called with IRQs enabled, and no arch_spin_lock_irq
+	 * variant exists, we need to disable IRQs manually here.
+	 */
+	local_irq_disable();
+	arch_spin_lock(&dcscb_lock);
+
+	rst_hold = readl_relaxed(dcscb_base + RST_HOLD0 + cluster * 4);
+	if (rst_hold & (1 << 8)) {
+		/* remove cluster reset and add individual CPU's reset */
+		rst_hold &= ~(1 << 8);
+		rst_hold |= 0xf;
+	}
+	rst_hold &= ~(cpumask | (cpumask << 4));
+	writel(rst_hold, dcscb_base + RST_HOLD0 + cluster * 4);
+
+	arch_spin_unlock(&dcscb_lock);
+	local_irq_enable();
+
+	return 0;
+}
+
+static void dcscb_power_down(void)
+{
+	unsigned int mpidr, cpu, cluster, rst_hold, cpumask, last_man;
+
+	mpidr = read_cpuid_mpidr();
+	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+	cpumask = (1 << cpu);
+
+	pr_debug("%s: cpu %u cluster %u\n", __func__, cpu, cluster);
+	BUG_ON(cpu >= 4 || cluster >= 2);
+
+	arch_spin_lock(&dcscb_lock);
+	rst_hold = readl_relaxed(dcscb_base + RST_HOLD0 + cluster * 4);
+	rst_hold |= cpumask;
+	if (((rst_hold | (rst_hold >> 4)) & 0xf) == 0xf)

It is possible for a CPU to be told to power up before it managed to power itself down. Solve this race with a usage count as mandated by the API definition.

Signed-off-by nicolas Pitre <[email protected]>

---
  arch/arm/mach-vexpress/dcscb.c | 77 +++++++++++++++++++++++++++++++++---------
  1 file changed, 61 insertions(+), 16 deletions(-)

arch/arm/mach-vexpress/dcscb.c +11 -5

index 677ced9efc..f993608944 100644
@@ -45,6 +45,7 @@
  static arch_spinlock_t dcscb_lock = __ARCH_SPIN_LOCK_UNLOCKED;
 
  static void __iomem *dcscb_base;
+static int dcscb_use_count[4][2];
 
  static int dcscb_power_up(unsigned int cpu, unsigned int cluster)
  {

@@ -61,14 +62,27 @@ static int dcscb_power_up(unsigned int cpu, unsigned int cluster)
  	local_irq_disable();
  	arch_spin_lock(&dcscb_lock);
 
-	rst_hold = readl_relaxed(dcscb_base + RST_HOLD0 + cluster * 4);
-	if (rst_hold & (1 << 8)) {
-		/* remove cluster reset and add individual CPU's reset */
-		rst_hold &= ~(1 << 8);
-		rst_hold |= 0xf;
+	dcscb_use_count[cpu][cluster]++;
+	if (dcscb_use_count[cpu][cluster] == 1) {
+		rst_hold = readl_relaxed(dcscb_base + RST_HOLD0 + cluster * 4);
+		if (rst_hold & (1 << 8)) {
+			/* remove cluster reset and add individual CPU's reset */
+			rst_hold &= ~(1 << 8);
+			rst_hold |= 0xf;
+		}
+		rst_hold &= ~(cpumask | (cpumask << 4));
+		writel(rst_hold, dcscb_base + RST_HOLD0 + cluster * 4);

arch/arm/mach-vexpress/dcscb.c +11 -8

  	}
-	rst_hold &= ~(cpumask | (cpumask << 4));
-	writel(rst_hold, dcscb_base + RST_HOLD0 + cluster * 4);
 
  	arch_spin_unlock(&dcscb_lock);
  	local_irq_enable();

@@ -78,7 +92,8 @@ static int dcscb_power_up(unsigned int cpu, unsigned int cluster)
 
  static void dcscb_power_down(void)
  {
-	unsigned int mpidr, cpu, cluster, rst_hold, cpumask, last_man;
+	unsigned int mpidr, cpu, cluster, rst_hold, cpumask;
+	bool last_man = false, skip_wfi = false;
 
  	mpidr = read_cpuid_mpidr();
  	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);

@@ -89,13 +104,26 @@ static void dcscb_power_down(void)
  	BUG_ON(cpu >= 4 || cluster >= 2);
 
  	arch_spin_lock(&dcscb_lock);
-	rst_hold = readl_relaxed(dcscb_base + RST_HOLD0 + cluster * 4);
-	rst_hold |= cpumask;
-	if (((rst_hold | (rst_hold >> 4)) & 0xf) == 0xf)
-		rst_hold |= (1 << 8);
-	writel(rst_hold, dcscb_base + RST_HOLD0 + cluster * 4);
+	dcscb_use_count[cpu][cluster]--;
+	if (dcscb_use_count[cpu][cluster] == 0) {
+		rst_hold = readl_relaxed(dcscb_base + RST_HOLD0 + cluster * 4);
+		rst_hold |= cpumask;
+		if (((rst_hold | (rst_hold >> 4)) & 0xf) == 0xf) {
+			rst_hold |= (1 << 8);
+			last_man = true;
+		}
+		writel(rst_hold, dcscb_base + RST_HOLD0 + cluster * 4);

If 4 CPUs are assumed, the A15x1-A7x1 model configuration would never shut down the initial cluster as the 0xf reset bit mask will never be observed. Let's construct this mask based on the provided information in the DCSCB config register for the number of CPUs per cluster.

Signed-off-by Nicolas Pitre <[email protected]>

---
  arch/arm/mach-vexpress/dcscb.c | 14 ++++++++++----
  1 file changed, 10 insertions(+), 4 deletions(-)

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

index f993608944..8d363357ef 100644

Quoted 17 lines · Nicolas Pitre

If 4 CPUs are assumed, the A15x1-A7x1 model configuration would never shut down the initial cluster as the 0xf reset bit mask will never be observed. Let's construct this mask based on the provided information in the DCSCB config register for the number of CPUs per cluster.

Signed-off-by Nicolas Pitre <[email protected]>

---
  arch/arm/mach-vexpress/dcscb.c | 14 ++++++++++----
  1 file changed, 10 insertions(+), 4 deletions(-)

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

index f993608944..8d363357ef 100644
@@ -46,10 +46,12 @@ static arch_spinlock_t dcscb_lock =
 __ARCH_SPIN_LOCK_UNLOCKED;
 

arch/arm/mach-vexpress/dcscb.c static arch_spinlock_t dcscb_lock = __ARCH_SPIN_LOCK_UNLOCKED; · line 49

Quoted 3 lines

arch/arm/mach-vexpress/dcscb.c +1 -0

  static void __iomem *dcscb_base;
  static int dcscb_use_count[4][2];
+static int dcscb_mcpm_cpu_mask[2];

s/2/MAX_CLUSTERS ?

Apart from above minor question,

Reviewed-by Santosh Shilimkar <[email protected]>

Quoted 18 lines · Santosh Shilimkar

Quoted 17 lines · Nicolas Pitre

If 4 CPUs are assumed, the A15x1-A7x1 model configuration would never shut down the initial cluster as the 0xf reset bit mask will never be observed. Let's construct this mask based on the provided information in the DCSCB config register for the number of CPUs per cluster.

Signed-off-by Nicolas Pitre <[email protected]>

---
   arch/arm/mach-vexpress/dcscb.c | 14 ++++++++++----
   1 file changed, 10 insertions(+), 4 deletions(-)

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

index f993608944..8d363357ef 100644
@@ -46,10 +46,12 @@ static arch_spinlock_t dcscb_lock =
 __ARCH_SPIN_LOCK_UNLOCKED;
 

Quoted 4 lines

arch/arm/mach-vexpress/dcscb.c static arch_spinlock_t dcscb_lock = __ARCH_SPIN_LOCK_UNLOCKED; · line 49

Quoted 3 lines

arch/arm/mach-vexpress/dcscb.c +1 -0

   static void __iomem *dcscb_base;
   static int dcscb_use_count[4][2];
+static int dcscb_mcpm_cpu_mask[2];

s/2/MAX_CLUSTERS ?

No. The DCSCB (*dual* cluster system control block) does manage only 2 clusters, regardless of the MAX_CLUSTERS definition which might increase in the future.

From: Lorenzo Pieralisi <[email protected]>

On ARM multi-cluster systems coherency between cores running on different clusters is managed by the cache-coherent interconnect (CCI). It allows broadcasting of TLB invalidates and memory barriers and it guarantees cache coherency at system level.

This patch enables the basic infrastructure required in Linux to handle and programme the CCI component. The first implementation is based on a platform device, its relative DT compatible property and a simple programming interface.

Signed-off-by Nicolas Pitre <[email protected]>

---
  drivers/bus/Kconfig     |   4 ++
  drivers/bus/Makefile    |   2 +
  drivers/bus/arm-cci.c   | 107 ++++++++++++++++++++++++++++++++++++++++++++++++
  include/linux/arm-cci.h |  30 ++++++++++++++
  4 files changed, 143 insertions(+)

create mode 100644 drivers/bus/arm-cci.c
create mode 100644 include/linux/arm-cci.h

drivers/bus/Kconfig +4 -0

index 0f51ed687d..d032f74ff2 100644
@@ -19,4 +19,8 @@ config OMAP_INTERCONNECT
 
  	help
  	  Driver to enable OMAP interconnect error handling driver.
+
+config ARM_CCI
+       bool "ARM CCI driver support"
+
  endmenu

drivers/bus/Makefile +2 -0

index 45d997c854..55aac809e5 100644
@@ -6,3 +6,5 @@ obj-$(CONFIG_OMAP_OCP2SCP)	+= omap-ocp2scp.o
 
  # Interconnect bus driver for OMAP SoCs.
  obj-$(CONFIG_OMAP_INTERCONNECT)	+= omap_l3_smx.o omap_l3_noc.o
+
+obj-$(CONFIG_ARM_CCI)		+= arm-cci.o

drivers/bus/arm-cci.c new file +30 -0

new file mode 100644
index 0000000000..25ae156924
@@ -0,0 +1,107 @@
+/*
+ * ARM Cache Coherency Interconnect (CCI400) support
+ *
+ * Copyright (C) 2012-2013 ARM Ltd.
+ * Author: Lorenzo Pieralisi <[email protected]>
+ *
+ * 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
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/arm-cci.h>
+
+#define CCI400_EAG_OFFSET       0x4000
+#define CCI400_KF_OFFSET        0x5000
+
+#define DRIVER_NAME	"CCI"
+struct cci_drvdata {
+	void __iomem *baseaddr;
+	spinlock_t lock;

From: Dave Martin <[email protected]>

Non-local variables used by the CCI management function called after disabling the cache must be flushed out to main memory in advance, otherwise incoherency of those values may occur if they are sitting in the cache of some other CPU when cci_disable() executes.

This patch adds the appropriate flushing to the CCI driver to ensure that the relevant data is available in RAM ahead of time.

Because this creates a dependency on arch-specific cacheflushing functions, this patch also makes ARM_CCI depend on ARM.

Signed-off-by Dave Martin <[email protected]>

Signed-off-by Nicolas Pitre <[email protected]>

---

Quoted 7 lines · Nicolas Pitre

From: Dave Martin <[email protected]>

Non-local variables used by the CCI management function called after disabling the cache must be flushed out to main memory in advance, otherwise incoherency of those values may occur if they are sitting in the cache of some other CPU when cci_disable() executes.

Any CPU calling cci_disable() would have already cleaned its local cache and the snoop unit should take care of syncing the shared data before hand from other CPU local caches for shared accesses. May be I am unable to visualize the issue here or missing some key point.

Quoted 6 lines

This patch adds the appropriate flushing to the CCI driver to ensure that the relevant data is available in RAM ahead of time.

Because this creates a dependency on arch-specific cacheflushing functions, this patch also makes ARM_CCI depend on ARM.

You should do that otherwise to avoid other arch building this driver for random builds and breaking their builds.

Quoted 3 lines

Signed-off-by Dave Martin <[email protected]>

Signed-off-by Nicolas Pitre <[email protected]>

---

Patch is fine apart from the question.

Reviewed-by Santosh Shilimkar <[email protected]>

Quoted 8 lines · Santosh Shilimkar

Quoted 7 lines · Nicolas Pitre

From: Dave Martin <[email protected]>

Non-local variables used by the CCI management function called after disabling the cache must be flushed out to main memory in advance, otherwise incoherency of those values may occur if they are sitting in the cache of some other CPU when cci_disable() executes.

Quoted 5 lines

Any CPU calling cci_disable() would have already cleaned its local cache and the snoop unit should take care of syncing the shared data before hand from other CPU local caches for shared accesses. May be I am unable to visualize the issue here or missing some key point.

Let's suppose CPU0 initializes the CCI. Without this patch, the CCI base address might be sitting in CPU0's cache.

The last CPU in a cluster to shut itself down is responsible for calling cci_disable(). And being the last, it is also responsible for flushing out its L1 and L2 caches before doing that. If CPU0 went down before that, it did flush its L1 already. So the base address will be flushed to RAM in that case.

But if it is a CPU in _another_ cluster which is shutting down and becoming the last man _there_. It will flush its L1 and L2 cache before calling cci_disable(). And
because the cache is disabled at that point, that CPU won't send any snoop request across to the other cluster where CPU0 holds the base address in its L1 or even L2 cache.

This is why we must push out that value out to RAM before cci_disable() is used.

Quoted 3 lines

Quoted 3 lines

This patch adds the appropriate flushing to the CCI driver to ensure that the relevant data is available in RAM ahead of time.

Quoted 5 lines

Quoted 3 lines

Because this creates a dependency on arch-specific cacheflushing functions, this patch also makes ARM_CCI depend on ARM.

You should do that otherwise to avoid other arch building this driver for random builds and breaking their builds.

Before this patch the driver was buildable on any architecture. That's why this dependency is added only in this patch.

Quoted 16 lines · Nicolas Pitre

Quoted 8 lines · Santosh Shilimkar

Quoted 7 lines · Nicolas Pitre

From: Dave Martin <[email protected]>

Non-local variables used by the CCI management function called after disabling the cache must be flushed out to main memory in advance, otherwise incoherency of those values may occur if they are sitting in the cache of some other CPU when cci_disable() executes.

Quoted 5 lines

Any CPU calling cci_disable() would have already cleaned its local cache and the snoop unit should take care of syncing the shared data before hand from other CPU local caches for shared accesses. May be I am unable to visualize the issue here or missing some key point.

Quoted 9 lines

Let's suppose CPU0 initializes the CCI. Without this patch, the CCI base address might be sitting in CPU0's cache.

The last CPU in a cluster to shut itself down is responsible for calling cci_disable(). And being the last, it is also responsible for flushing out its L1 and L2 caches before doing that. If CPU0 went down before that, it did flush its L1 already. So the base address will be flushed to RAM in that case.

Yes. This is valid case. Thanks for description.

Quoted 19 lines

But if it is a CPU in _another_ cluster which is shutting down and becoming the last man _there_. It will flush its L1 and L2 cache before calling cci_disable(). And
because the cache is disabled at that point, that CPU won't send any snoop request across to the other cluster where CPU0 holds the base address in its L1 or even L2 cache.

This is why we must push out that value out to RAM before cci_disable() is used.

Quoted 8 lines

Quoted 6 lines

This patch adds the appropriate flushing to the CCI driver to ensure that the relevant data is available in RAM ahead of time.

Because this creates a dependency on arch-specific cacheflushing functions, this patch also makes ARM_CCI depend on ARM.

You should do that otherwise to avoid other arch building this driver for random builds and breaking their builds.

Quoted 3 lines

Before this patch the driver was buildable on any architecture. That's why this dependency is added only in this patch.

I was just trying to counter the reasoning in the changelog which says dependency is added because of arch specific cache flushing function. Meaning even without that ARM dependency should be in place to avoid driver getting build for other archs.

Quoted 5 lines · Santosh Shilimkar

Quoted 4 lines · Nicolas Pitre

Quoted 2 lines · Santosh Shilimkar

Quoted 1 line · Nicolas Pitre

From: Dave Martin <[email protected]>

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

Quoted 6 lines

This patch adds the appropriate flushing to the CCI driver to ensure that the relevant data is available in RAM ahead of time.

Because this creates a dependency on arch-specific cacheflushing functions, this patch also makes ARM_CCI depend on ARM.

You should do that otherwise to avoid other arch building this driver for random builds and breaking their builds.

Quoted 7 lines

Quoted 3 lines

Before this patch the driver was buildable on any architecture. That's why this dependency is added only in this patch.

I was just trying to counter the reasoning in the changelog which says dependency is added because of arch specific cache flushing function. Meaning even without that ARM dependency should be in place to avoid driver getting build for other archs.

Well, some upstream maintainers' opinion is that you should not put artificial dependencies on a specific architecture if a driver is buildable on any architecture, even if the built driver is of no use to those other architectures.

arch/arm/mach-vexpress/dcscb.c +9 -2

+		flush_cache_louis();
+
+		/* Disable local coherency by clearing the ACTLR "SMP" bit: */
+		set_auxcr(get_auxcr() & ~(1 << 6));
 	}
 
-	/* Disable local coherency by clearing the ACTLR "SMP" bit: */
-	set_auxcr(get_auxcr() & ~(1 << 6));
+	__mcpm_cpu_down(cpu, cluster);
 
 	/* Now we are prepared for power-down, do it: */
 	if (!skip_wfi) {

@@ -179,6 +211,8 @@ static void __init dcscb_usage_count_init(void)
 	dcscb_use_count[cpu][cluster] = 1;
 }
 
+extern void dcscb_power_up_setup(unsigned int affinity_level);
+
 static int __init dcscb_init(void)
 {
 	unsigned int cfg;

@@ -193,6 +227,8 @@ static int __init dcscb_init(void)
 	dcscb_usage_count_init();
 
 	ret = mcpm_platform_register(&dcscb_power_ops);
+	if (!ret)
+		ret = mcpm_sync_init(dcscb_power_up_setup);
 	if (ret) {
 		iounmap(dcscb_base);
 		return ret;

arch/arm/mach-vexpress/dcscb_setup.S new file +18 -0

new file mode 100644
index 0000000000..cac033b982
@@ -0,0 +1,80 @@
+/*
+ * arch/arm/include/asm/dcscb_setup.S
+ *
+ * Created by:  Dave Martin, 2012-06-22
+ * Copyright:   (C) 2012-2013  Linaro Limited
+ *
+ * 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
+ * published by the Free Software Foundation.
+ */
+
+
+#include <linux/linkage.h>
+#include <asm/mcpm_entry.h>
+
+
+#define SLAVE_SNOOPCTL_OFFSET	0
+#define SNOOPCTL_SNOOP_ENABLE	(1 << 0)

On Tue, Jan 29, 2013 at 07:51:09AM +0000, Nicolas Pitre wrote:

[...]

arch/arm/mach-vexpress/dcscb.c static void dcscb_power_down(void) · line 177

Quoted 9 lines

arch/arm/mach-vexpress/dcscb.c +9 -0

+		/*
+		 * Flush the local CPU cache.
+		 *
+		 * A15/A7 can hit in the cache with SCTLR.C=0, so we don't need
+		 * a preliminary flush here for those CPUs.  At least, that's
+		 * the theory -- without the extra flush, Linux explodes on
+		 * RTSM (maybe not needed anymore, to be investigated).
+		 */
+		flush_cache_louis();

This is not needed. If it is, that is a model bug and should be flagged up as such.

Review

Quoted 1 line

(quoted diff) +1 -0

+		cpu_proc_fin(); /* disable allocation into internal caches*/

This code disables the I-cache causing following instruction fetches from DRAM; that is extremely slow and should be avoided, there is no point in disabling the I-cache here, that is not required.
On fast-models that's a non-issue, but I really want to prevent copy'n'paste of this sequence as it stands.

Quoted 30 lines

arch/arm/mach-vexpress/dcscb_setup.S new file +24 -0

new file mode 100644
index 0000000000..cac033b982
@@ -0,0 +1,80 @@
+/*
+ * arch/arm/include/asm/dcscb_setup.S
+ *
+ * Created by:  Dave Martin, 2012-06-22
+ * Copyright:   (C) 2012-2013  Linaro Limited
+ *
+ * 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
+ * published by the Free Software Foundation.
+ */
+
+
+#include <linux/linkage.h>
+#include <asm/mcpm_entry.h>
+
+
+#define SLAVE_SNOOPCTL_OFFSET	0
+#define SNOOPCTL_SNOOP_ENABLE	(1 << 0)
+#define SNOOPCTL_DVM_ENABLE	(1 << 1)
+
+#define CCI_STATUS_OFFSET	0xc
+#define STATUS_CHANGE_PENDING	(1 << 0)
+
+#define CCI_SLAVE_OFFSET(n)	(0x1000 + 0x1000 * (n))

arch/arm/mach-vexpress/dcscb_setup.S line 28

Quoted 4 lines

arch/arm/mach-vexpress/dcscb_setup.S new file +4 -0

+
+#define RTSM_CCI_PHYS_BASE	0x2c090000
+#define RTSM_CCI_SLAVE_A15	3
+#define RTSM_CCI_SLAVE_A7	4

We need to remove these hardcoded values in due course as you know, I am working on new code that allows us to match the CCI port address to MPIDR on resume.

Quoted 4 lines · Lorenzo Pieralisi

On Tue, Jan 29, 2013 at 07:51:09AM +0000, Nicolas Pitre wrote:

[...]

Quoted 12 lines

arch/arm/mach-vexpress/dcscb.c static void dcscb_power_down(void) · line 177

Quoted 9 lines

arch/arm/mach-vexpress/dcscb.c +9 -0

+		/*
+		 * Flush the local CPU cache.
+		 *
+		 * A15/A7 can hit in the cache with SCTLR.C=0, so we don't need
+		 * a preliminary flush here for those CPUs.  At least, that's
+		 * the theory -- without the extra flush, Linux explodes on
+		 * RTSM (maybe not needed anymore, to be investigated).
+		 */
+		flush_cache_louis();

This is not needed. If it is, that is a model bug and should be flagged up as such.

Could someone at ARM do that?

I just confirmed that this is still the case by commenting out the preliminary flush calls and hot-plugging CPUs out and back. Result is a non-sensical kernel oops which has the looks of serious memory corruption. This is with RTSM version 7.1.42.

Quoted 7 lines

Review

Quoted 1 line

(quoted diff) +1 -0

+		cpu_proc_fin(); /* disable allocation into internal caches*/

This code disables the I-cache causing following instruction fetches from DRAM; that is extremely slow and should be avoided, there is no point in disabling the I-cache here, that is not required.
On fast-models that's a non-issue, but I really want to prevent copy'n'paste of this sequence as it stands.

Agreed, I'll change that. The (not included in this series) TC2 backend does leave the I-cache active already.

From: Dave Martin <[email protected]>

Add the required code to properly handle race free platform coherency exit to the DCSCB power down method.

The power_up_setup callback is used to enable the CCI interface for the cluster being brought up. This must be done in assembly before the kernel environment is entered.

Thanks to Achin Gupta and Nicolas Pitre for their help and contributions.

Signed-off-by Dave Martin <[email protected]>

Signed-off-by Nicolas Pitre <[email protected]>

---

This allows for the DCSCB support to be compiled in and selected at run time.

Signed-off-by Nicolas Pitre <[email protected]>

---
 arch/arm/mach-vexpress/dcscb.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

arch/arm/mach-vexpress/dcscb.c +1 -2

index 58051ffafb..a724507cbc 100644
@@ -14,6 +14,7 @@
 #include <linux/io.h>
 #include <linux/spinlock.h>
 #include <linux/errno.h>
+#include <linux/of_address.h>
 #include <linux/vexpress.h>
 #include <linux/arm-cci.h>
 

@@ -24,8 +25,6 @@
 #include <asm/cp15.h>
 
 
-#define DCSCB_PHYS_BASE	0x60000000
-
 #define RST_HOLD0	0x0
 #define RST_HOLD1	0x4
 #define SYS_SWRESET	0x8

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

+	if (!node)
+		return -ENODEV;
+	dcscb_base= of_iomap(node, 0);
 	if (!dcscb_base)
 		return -EADDRNOTAVAIL;
 	cfg = readl_relaxed(dcscb_base + DCS_CFG_R);

Quoted 2 lines · Nicolas Pitre

This allows for the DCSCB support to be compiled in and selected at run time.

Shouldn't this just be rolled into the commit creating dcscb.c?

Quoted 34 lines

Signed-off-by Nicolas Pitre <[email protected]>

---
 arch/arm/mach-vexpress/dcscb.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

arch/arm/mach-vexpress/dcscb.c +2 -2

index 58051ffafb..a724507cbc 100644
@@ -14,6 +14,7 @@
 #include <linux/io.h>
 #include <linux/spinlock.h>
 #include <linux/errno.h>
+#include <linux/of_address.h>
 #include <linux/vexpress.h>
 #include <linux/arm-cci.h>
 

@@ -24,8 +25,6 @@
 #include <asm/cp15.h>
 
 
-#define DCSCB_PHYS_BASE	0x60000000
-
 #define RST_HOLD0	0x0
 #define RST_HOLD1	0x4
 #define SYS_SWRESET	0x8

@@ -215,10 +214,14 @@ extern void dcscb_power_up_setup(unsigned int affinity_level);
 
 static int __init dcscb_init(void)
 {
+	struct device_node *node;
 	unsigned int cfg;
 	int ret;
 

arch/arm/mach-vexpress/dcscb.c extern void dcscb_power_up_setup(unsigned int affinity_level); · line 221

Quoted 2 lines

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

-	dcscb_base = ioremap(DCSCB_PHYS_BASE, 0x1000);
+	node = of_find_compatible_node(NULL, NULL, "arm,dcscb");

This needs binding documentation and should be a more specific name. Not knowing what dcscb is, I don't have a suggestion.

Perhaps should include vexpress or specific core tile name it is part of.

Quoted 5 lines · Rob Herring

Quoted 2 lines · Nicolas Pitre

This allows for the DCSCB support to be compiled in and selected at run time.

Shouldn't this just be rolled into the commit creating dcscb.c?

Probably, yes.

Quoted 34 lines

Quoted 34 lines

Signed-off-by Nicolas Pitre <[email protected]>

---
 arch/arm/mach-vexpress/dcscb.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

arch/arm/mach-vexpress/dcscb.c +2 -2

index 58051ffafb..a724507cbc 100644
@@ -14,6 +14,7 @@
 #include <linux/io.h>
 #include <linux/spinlock.h>
 #include <linux/errno.h>
+#include <linux/of_address.h>
 #include <linux/vexpress.h>
 #include <linux/arm-cci.h>
 

@@ -24,8 +25,6 @@
 #include <asm/cp15.h>
 
 
-#define DCSCB_PHYS_BASE    0x60000000
-
 #define RST_HOLD0  0x0
 #define RST_HOLD1  0x4
 #define SYS_SWRESET        0x8

@@ -215,10 +214,14 @@ extern void dcscb_power_up_setup(unsigned int affinity_level);
 
 static int __init dcscb_init(void)
 {
+   struct device_node *node;
    unsigned int cfg;
    int ret;
 

Quoted 5 lines

arch/arm/mach-vexpress/dcscb.c extern void dcscb_power_up_setup(unsigned int affinity_level); · line 221

Quoted 2 lines

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

-   dcscb_base = ioremap(DCSCB_PHYS_BASE, 0x1000);
+   node = of_find_compatible_node(NULL, NULL, "arm,dcscb");

This needs binding documentation and should be a more specific name. Not knowing what dcscb is, I don't have a suggestion.

Yes, I mentioned in the cover page that DT bindings are not yet documented.

DCSCB stands for "Dual Cluster System Control Block". This is in fact a set of miscellaneous registers, mainly for reset control of individual CPUs and clusters.

Quoted 2 lines

Perhaps should include vexpress or specific core tile name it is part of.

/me hopes for some ARM dude more acquainted with their nomenclature to chime in with suggestions.