On 1 July 2011 23:09, Russell King - ARM Linux linux@arm.linux.org.uk wrote:
On Fri, Jul 01, 2011 at 07:43:30AM +0200, Vincent Guittot wrote:
Changes since v3 :
- Update the format of printk message
- Remove blank line
Can I trouble you to check the patch for more instances of the 'blank line at end of function' thing... Also, let's get rid of unnecessary parens.
ok, I will update the patch with your comments
diff --git a/arch/arm/include/asm/topology.h b/arch/arm/include/asm/topology.h index accbd7c..63a7454 100644 --- a/arch/arm/include/asm/topology.h +++ b/arch/arm/include/asm/topology.h @@ -1,6 +1,39 @@ #ifndef _ASM_ARM_TOPOLOGY_H #define _ASM_ARM_TOPOLOGY_H
+#ifdef CONFIG_ARM_CPU_TOPOLOGY
+#include <linux/cpumask.h>
+struct cputopo_arm {
- int thread_id;
- int core_id;
- int socket_id;
- cpumask_t thread_sibling;
- cpumask_t core_sibling;
+};
+extern struct cputopo_arm cpu_topology[NR_CPUS];
+#define topology_physical_package_id(cpu) (cpu_topology[cpu].socket_id) +#define topology_core_id(cpu) (cpu_topology[cpu].core_id) +#define topology_core_cpumask(cpu) (&(cpu_topology[cpu].core_sibling)) +#define topology_thread_cpumask(cpu) (&(cpu_topology[cpu].thread_sibling))
The inner-most parens aren't required.
+#define mc_capable() (cpu_topology[0].socket_id != -1) +#define smt_capable() (cpu_topology[0].thread_id != -1)
+void init_cpu_topology(void); +void store_cpu_topology(unsigned int cpuid); +const struct cpumask *cpu_coregroup_mask(unsigned int cpu);
+#else
+static inline void init_cpu_topology(void) { }; +static inline void store_cpu_topology(unsigned int cpuid) { };
Functions don't need a ; after the }
+#endif
#include <asm-generic/topology.h>
#endif /* _ASM_ARM_TOPOLOGY_H */ diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile index a5b31af..816a481 100644 --- a/arch/arm/kernel/Makefile +++ b/arch/arm/kernel/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_IWMMXT) += iwmmxt.o obj-$(CONFIG_CPU_HAS_PMU) += pmu.o obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o AFLAGS_iwmmxt.o := -Wa,-mcpu=iwmmxt +obj-$(CONFIG_ARM_CPU_TOPOLOGY) += topology.o
ifneq ($(CONFIG_ARCH_EBSA110),y) obj-y += io.o diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 344e52b..3e8dc3b 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -31,6 +31,7 @@ #include <asm/cacheflush.h> #include <asm/cpu.h> #include <asm/cputype.h> +#include <asm/topology.h> #include <asm/mmu_context.h> #include <asm/pgtable.h> #include <asm/pgalloc.h> @@ -268,6 +269,9 @@ static void __cpuinit smp_store_cpu_info(unsigned int cpuid) struct cpuinfo_arm *cpu_info = &per_cpu(cpu_data, cpuid);
cpu_info->loops_per_jiffy = loops_per_jiffy;
- store_cpu_topology(cpuid);
Don't need this blank line.
}
/* @@ -354,6 +358,8 @@ void __init smp_prepare_cpus(unsigned int max_cpus) { unsigned int ncores = num_possible_cpus();
- init_cpu_topology();
smp_store_cpu_info(smp_processor_id());
/* diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c new file mode 100644 index 0000000..e8f3b95 --- /dev/null +++ b/arch/arm/kernel/topology.c @@ -0,0 +1,149 @@ +/*
- arch/arm/kernel/topology.c
- Copyright (C) 2011 Linaro Limited.
- Written by: Vincent Guittot
- based on arch/sh/kernel/topology.c
- This file is subject to the terms and conditions of the GNU General Public
- License. See the file "COPYING" in the main directory of this archive
- for more details.
- */
+#include <linux/cpu.h> +#include <linux/cpumask.h> +#include <linux/init.h> +#include <linux/percpu.h> +#include <linux/node.h> +#include <linux/nodemask.h> +#include <linux/sched.h>
+#include <asm/cputype.h> +#include <asm/topology.h>
+#define MPIDR_SMP_BITMASK (0x3 << 30) +#define MPIDR_SMP_VALUE (0x2 << 30)
+#define MPIDR_MT_BITMASK (0x1 << 24)
+/*
- These masks reflect the current use of the affinity levels.
- The affinity level can be up to 16 bits according to ARM ARM
- */
+#define MPIDR_LEVEL0_MASK 0x3 +#define MPIDR_LEVEL0_SHIFT 0
+#define MPIDR_LEVEL1_MASK 0xF +#define MPIDR_LEVEL1_SHIFT 8
+#define MPIDR_LEVEL2_MASK 0xFF +#define MPIDR_LEVEL2_SHIFT 16
+struct cputopo_arm cpu_topology[NR_CPUS];
+const struct cpumask *cpu_coregroup_mask(unsigned int cpu) +{
- return &(cpu_topology[cpu].core_sibling);
These parens aren't required.
+}
+/*
- store_cpu_topology is called at boot when only one cpu is running
- and with the mutex cpu_hotplug.lock locked, when several cpus have booted,
- which prevents simultaneous write access to cpu_topology array
- */
+void store_cpu_topology(unsigned int cpuid) +{
- struct cputopo_arm *cpuid_topo = &(cpu_topology[cpuid]);
Neither are these.
- unsigned int mpidr;
- unsigned int cpu;
- /* If the cpu topology has been already set, just return */
- if (cpuid_topo->core_id != -1)
- return;
- mpidr = read_cpuid_mpidr();
- /* create cpu topology mapping */
- if ((mpidr & MPIDR_SMP_BITMASK) == MPIDR_SMP_VALUE) {
- /*
- * This is a multiprocessor system
- * multiprocessor format & multiprocessor mode field are set
- */
- if (mpidr & MPIDR_MT_BITMASK) {
- /* core performance interdependency */
- cpuid_topo->thread_id = ((mpidr >> MPIDR_LEVEL0_SHIFT)
- & MPIDR_LEVEL0_MASK);
- cpuid_topo->core_id = ((mpidr >> MPIDR_LEVEL1_SHIFT)
- & MPIDR_LEVEL1_MASK);
- cpuid_topo->socket_id = ((mpidr >> MPIDR_LEVEL2_SHIFT)
- & MPIDR_LEVEL2_MASK);
Outer set aren't required.
- } else {
- /* largely independent cores */
- cpuid_topo->thread_id = -1;
- cpuid_topo->core_id = ((mpidr >> MPIDR_LEVEL0_SHIFT)
- & MPIDR_LEVEL0_MASK);
- cpuid_topo->socket_id = ((mpidr >> MPIDR_LEVEL1_SHIFT)
- & MPIDR_LEVEL1_MASK);
Outer set aren't required.
- }
- } else {
- /*
- * This is an uniprocessor system
- * we are in multiprocessor format but uniprocessor system
- * or in the old uniprocessor format
- */
This blank line isn't necessary.
- cpuid_topo->thread_id = -1;
- cpuid_topo->core_id = 0;
- cpuid_topo->socket_id = -1;
- }
- /* update core and thread sibling masks */
- for_each_possible_cpu(cpu) {
- struct cputopo_arm *cpu_topo = &(cpu_topology[cpu]);
Parens not required.
- if (cpuid_topo->socket_id == cpu_topo->socket_id) {
- cpumask_set_cpu(cpuid, &cpu_topo->core_sibling);
- if (cpu != cpuid)
- cpumask_set_cpu(cpu,
- &cpuid_topo->core_sibling);
- if (cpuid_topo->core_id == cpu_topo->core_id) {
- cpumask_set_cpu(cpuid,
- &cpu_topo->thread_sibling);
- if (cpu != cpuid)
- cpumask_set_cpu(cpu,
- &cpuid_topo->thread_sibling);
- }
- }
- }
- smp_wmb();
- printk(KERN_INFO "CPU%u: thread %d, cpu %d, socket %d, mpidr %x\n",
- cpuid, cpu_topology[cpuid].thread_id,
- cpu_topology[cpuid].core_id,
- cpu_topology[cpuid].socket_id, mpidr);
Thanks for changing that.
+}
+/*
- init_cpu_topology is called at boot when only one cpu is running
- which prevent simultaneous write access to cpu_topology array
- */
+void init_cpu_topology(void) +{
- unsigned int cpu;
- /* init core mask */
- for_each_possible_cpu(cpu) {
- struct cputopo_arm *cpu_topo = &(cpu_topology[cpu]);
- cpu_topo->thread_id = -1;
- cpu_topo->core_id = -1;
- cpu_topo->socket_id = -1;
- cpumask_clear(&cpu_topo->core_sibling);
- cpumask_clear(&cpu_topo->thread_sibling);
- }
- smp_wmb();
+}