diff mbox

[5/12] : sparc64: Refactor OBP cpu scanning code using an iterator.

Message ID 20090408.223738.63750701.davem@davemloft.net
State Accepted
Delegated to: David Miller
Headers show

Commit Message

David Miller April 9, 2009, 5:37 a.m. UTC 
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 arch/sparc/include/asm/prom.h |    1 +
 arch/sparc/kernel/prom_64.c   |  235 ++++++++++++++++++++++-------------------
 2 files changed, 127 insertions(+), 109 deletions(-)

Comments

Sam Ravnborg April 9, 2009, 6:29 a.m. UTC | #1 
On Wed, Apr 08, 2009 at 10:37:38PM -0700, David Miller wrote:
> 
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
>  arch/sparc/include/asm/prom.h |    1 +
>  arch/sparc/kernel/prom_64.c   |  235 ++++++++++++++++++++++-------------------
>  2 files changed, 127 insertions(+), 109 deletions(-)
> 
> diff --git a/arch/sparc/include/asm/prom.h b/arch/sparc/include/asm/prom.h
> index 900d447..1b15212 100644
> --- a/arch/sparc/include/asm/prom.h
> +++ b/arch/sparc/include/asm/prom.h
> @@ -86,6 +86,7 @@ extern int of_node_to_nid(struct device_node *dp);
>  #endif
>  
>  extern void prom_build_devicetree(void);
> +extern void of_populate_present_mask(void);
>  
>  /* Dummy ref counting routines - to be implemented later */
>  static inline struct device_node *of_node_get(struct device_node *node)
> diff --git a/arch/sparc/kernel/prom_64.c b/arch/sparc/kernel/prom_64.c
> index ca55c70..04b518d 100644
> --- a/arch/sparc/kernel/prom_64.c
> +++ b/arch/sparc/kernel/prom_64.c
> @@ -374,75 +374,26 @@ static const char *get_mid_prop(void)
>  	return (tlb_type == spitfire ? "upa-portid" : "portid");
>  }
>  
> -struct device_node *of_find_node_by_cpuid(int cpuid)
> -{
> -	struct device_node *dp;
> -	const char *mid_prop = get_mid_prop();
> -
> -	for_each_node_by_type(dp, "cpu") {
> -		int id = of_getintprop_default(dp, mid_prop, -1);
> -		const char *this_mid_prop = mid_prop;
> -
> -		if (id < 0) {
> -			this_mid_prop = "cpuid";
> -			id = of_getintprop_default(dp, this_mid_prop, -1);
> -		}
> -
> -		if (id < 0) {
> -			prom_printf("OF: Serious problem, cpu lacks "
> -				    "%s property", this_mid_prop);
> -			prom_halt();
> -		}
> -		if (cpuid == id)
> -			return dp;
> -	}
> -	return NULL;
> -}
> -
> -void __init of_fill_in_cpu_data(void)
> +static void *of_iterate_over_cpus(void *(*func)(struct device_node *, int, void *), void *arg)

You never pass a "void *" as argument to this iterator.
It seems cleaner to use a long or even an int as this is closer
to the typical usage, and one can always pass a '0' when the
argument is not used.

I have not looked through all patches yet so it may come later.

	Sam
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller May 27, 2009, 5:35 a.m. UTC | #2 
From: Sam Ravnborg <sam@ravnborg.org>
Date: Thu, 9 Apr 2009 08:29:24 +0200

> You never pass a "void *" as argument to this iterator.
> It seems cleaner to use a long or even an int as this is closer
> to the typical usage, and one can always pass a '0' when the
> argument is not used.

I've changed this to an 'int' while respinning this patch.

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/sparc/include/asm/prom.h b/arch/sparc/include/asm/prom.h
index 900d447..1b15212 100644
--- a/arch/sparc/include/asm/prom.h
+++ b/arch/sparc/include/asm/prom.h
@@ -86,6 +86,7 @@  extern int of_node_to_nid(struct device_node *dp);
 #endif
 
 extern void prom_build_devicetree(void);
+extern void of_populate_present_mask(void);
 
 /* Dummy ref counting routines - to be implemented later */
 static inline struct device_node *of_node_get(struct device_node *node)
diff --git a/arch/sparc/kernel/prom_64.c b/arch/sparc/kernel/prom_64.c
index ca55c70..04b518d 100644
--- a/arch/sparc/kernel/prom_64.c
+++ b/arch/sparc/kernel/prom_64.c
@@ -374,75 +374,26 @@  static const char *get_mid_prop(void)
 	return (tlb_type == spitfire ? "upa-portid" : "portid");
 }
 
-struct device_node *of_find_node_by_cpuid(int cpuid)
-{
-	struct device_node *dp;
-	const char *mid_prop = get_mid_prop();
-
-	for_each_node_by_type(dp, "cpu") {
-		int id = of_getintprop_default(dp, mid_prop, -1);
-		const char *this_mid_prop = mid_prop;
-
-		if (id < 0) {
-			this_mid_prop = "cpuid";
-			id = of_getintprop_default(dp, this_mid_prop, -1);
-		}
-
-		if (id < 0) {
-			prom_printf("OF: Serious problem, cpu lacks "
-				    "%s property", this_mid_prop);
-			prom_halt();
-		}
-		if (cpuid == id)
-			return dp;
-	}
-	return NULL;
-}
-
-void __init of_fill_in_cpu_data(void)
+static void *of_iterate_over_cpus(void *(*func)(struct device_node *, int, void *), void *arg)
 {
 	struct device_node *dp;
 	const char *mid_prop;
 
-	if (tlb_type == hypervisor)
-		return;
-
 	mid_prop = get_mid_prop();
-	ncpus_probed = 0;
 	for_each_node_by_type(dp, "cpu") {
 		int cpuid = of_getintprop_default(dp, mid_prop, -1);
 		const char *this_mid_prop = mid_prop;
-		struct device_node *portid_parent;
-		int portid = -1;
+		void *ret;
 
-		portid_parent = NULL;
 		if (cpuid < 0) {
 			this_mid_prop = "cpuid";
 			cpuid = of_getintprop_default(dp, this_mid_prop, -1);
-			if (cpuid >= 0) {
-				int limit = 2;
-
-				portid_parent = dp;
-				while (limit--) {
-					portid_parent = portid_parent->parent;
-					if (!portid_parent)
-						break;
-					portid = of_getintprop_default(portid_parent,
-								       "portid", -1);
-					if (portid >= 0)
-						break;
-				}
-			}
 		}
-
 		if (cpuid < 0) {
 			prom_printf("OF: Serious problem, cpu lacks "
 				    "%s property", this_mid_prop);
 			prom_halt();
 		}
-
-		ncpus_probed++;
-
 #ifdef CONFIG_SMP
 		if (cpuid >= NR_CPUS) {
 			printk(KERN_WARNING "Ignoring CPU %d which is "
@@ -450,79 +401,145 @@  void __init of_fill_in_cpu_data(void)
 			       cpuid, NR_CPUS);
 			continue;
 		}
-#else
-		/* On uniprocessor we only want the values for the
-		 * real physical cpu the kernel booted onto, however
-		 * cpu_data() only has one entry at index 0.
-		 */
-		if (cpuid != real_hard_smp_processor_id())
-			continue;
-		cpuid = 0;
 #endif
+		ret = func(dp, cpuid, arg);
+		if (ret)
+			return ret;
+	}
+	return NULL;
+}
 
-		cpu_data(cpuid).clock_tick =
-			of_getintprop_default(dp, "clock-frequency", 0);
-
-		if (portid_parent) {
-			cpu_data(cpuid).dcache_size =
-				of_getintprop_default(dp, "l1-dcache-size",
-						      16 * 1024);
-			cpu_data(cpuid).dcache_line_size =
-				of_getintprop_default(dp, "l1-dcache-line-size",
-						      32);
-			cpu_data(cpuid).icache_size =
-				of_getintprop_default(dp, "l1-icache-size",
-						      8 * 1024);
-			cpu_data(cpuid).icache_line_size =
-				of_getintprop_default(dp, "l1-icache-line-size",
-						      32);
-			cpu_data(cpuid).ecache_size =
-				of_getintprop_default(dp, "l2-cache-size", 0);
-			cpu_data(cpuid).ecache_line_size =
-				of_getintprop_default(dp, "l2-cache-line-size", 0);
-			if (!cpu_data(cpuid).ecache_size ||
-			    !cpu_data(cpuid).ecache_line_size) {
-				cpu_data(cpuid).ecache_size =
-					of_getintprop_default(portid_parent,
-							      "l2-cache-size",
-							      (4 * 1024 * 1024));
-				cpu_data(cpuid).ecache_line_size =
-					of_getintprop_default(portid_parent,
-							      "l2-cache-line-size", 64);
-			}
-
-			cpu_data(cpuid).core_id = portid + 1;
-			cpu_data(cpuid).proc_id = portid;
+static void *check_cpu_node(struct device_node *dp, int cpuid, void *arg)
+{
+	int id = (int) (long) arg;
+
+	if (id == cpuid)
+		return dp;
+	return NULL;
+}
+
+struct device_node *of_find_node_by_cpuid(int cpuid)
+{
+	return of_iterate_over_cpus(check_cpu_node, (void *) (long) cpuid);
+}
+
+static void *record_one_cpu(struct device_node *dp, int cpuid, void *arg)
+{
+	ncpus_probed++;
 #ifdef CONFIG_SMP
-			sparc64_multi_core = 1;
+	set_cpu_present(cpuid, true);
+	set_cpu_possible(cpuid, true);
 #endif
-		} else {
-			cpu_data(cpuid).dcache_size =
-				of_getintprop_default(dp, "dcache-size", 16 * 1024);
-			cpu_data(cpuid).dcache_line_size =
-				of_getintprop_default(dp, "dcache-line-size", 32);
+	return NULL;
+}
+
+void __init of_populate_present_mask(void)
+{
+	if (tlb_type == hypervisor)
+		return;
+
+	ncpus_probed = 0;
+	of_iterate_over_cpus(record_one_cpu, NULL);
+}
 
-			cpu_data(cpuid).icache_size =
-				of_getintprop_default(dp, "icache-size", 16 * 1024);
-			cpu_data(cpuid).icache_line_size =
-				of_getintprop_default(dp, "icache-line-size", 32);
+static void *fill_in_one_cpu(struct device_node *dp, int cpuid, void *arg)
+{
+	struct device_node *portid_parent = NULL;
+	int portid = -1;
+
+	if (of_find_property(dp, "cpuid", NULL)) {
+		int limit = 2;
+
+		portid_parent = dp;
+		while (limit--) {
+			portid_parent = portid_parent->parent;
+			if (!portid_parent)
+				break;
+			portid = of_getintprop_default(portid_parent,
+						       "portid", -1);
+			if (portid >= 0)
+				break;
+		}
+	}
 
+#ifndef CONFIG_SMP
+	/* On uniprocessor we only want the values for the
+	 * real physical cpu the kernel booted onto, however
+	 * cpu_data() only has one entry at index 0.
+	 */
+	if (cpuid != real_hard_smp_processor_id())
+		return NULL;
+	cpuid = 0;
+#endif
+
+	cpu_data(cpuid).clock_tick =
+		of_getintprop_default(dp, "clock-frequency", 0);
+
+	if (portid_parent) {
+		cpu_data(cpuid).dcache_size =
+			of_getintprop_default(dp, "l1-dcache-size",
+					      16 * 1024);
+		cpu_data(cpuid).dcache_line_size =
+			of_getintprop_default(dp, "l1-dcache-line-size",
+					      32);
+		cpu_data(cpuid).icache_size =
+			of_getintprop_default(dp, "l1-icache-size",
+					      8 * 1024);
+		cpu_data(cpuid).icache_line_size =
+			of_getintprop_default(dp, "l1-icache-line-size",
+					      32);
+		cpu_data(cpuid).ecache_size =
+			of_getintprop_default(dp, "l2-cache-size", 0);
+		cpu_data(cpuid).ecache_line_size =
+			of_getintprop_default(dp, "l2-cache-line-size", 0);
+		if (!cpu_data(cpuid).ecache_size ||
+		    !cpu_data(cpuid).ecache_line_size) {
 			cpu_data(cpuid).ecache_size =
-				of_getintprop_default(dp, "ecache-size",
+				of_getintprop_default(portid_parent,
+						      "l2-cache-size",
 						      (4 * 1024 * 1024));
 			cpu_data(cpuid).ecache_line_size =
-				of_getintprop_default(dp, "ecache-line-size", 64);
-
-			cpu_data(cpuid).core_id = 0;
-			cpu_data(cpuid).proc_id = -1;
+				of_getintprop_default(portid_parent,
+						      "l2-cache-line-size", 64);
 		}
 
+		cpu_data(cpuid).core_id = portid + 1;
+		cpu_data(cpuid).proc_id = portid;
 #ifdef CONFIG_SMP
-		set_cpu_present(cpuid, true);
-		set_cpu_possible(cpuid, true);
+		sparc64_multi_core = 1;
 #endif
+	} else {
+		cpu_data(cpuid).dcache_size =
+			of_getintprop_default(dp, "dcache-size", 16 * 1024);
+		cpu_data(cpuid).dcache_line_size =
+			of_getintprop_default(dp, "dcache-line-size", 32);
+
+		cpu_data(cpuid).icache_size =
+			of_getintprop_default(dp, "icache-size", 16 * 1024);
+		cpu_data(cpuid).icache_line_size =
+			of_getintprop_default(dp, "icache-line-size", 32);
+
+		cpu_data(cpuid).ecache_size =
+			of_getintprop_default(dp, "ecache-size",
+					      (4 * 1024 * 1024));
+		cpu_data(cpuid).ecache_line_size =
+			of_getintprop_default(dp, "ecache-line-size", 64);
+
+		cpu_data(cpuid).core_id = 0;
+		cpu_data(cpuid).proc_id = -1;
 	}
 
+	return NULL;
+}
+
+void __init of_fill_in_cpu_data(void)
+{
+	if (tlb_type == hypervisor)
+		return;
+
+	of_populate_present_mask();
+	of_iterate_over_cpus(fill_in_one_cpu, NULL);
+
 	smp_fill_in_sib_core_maps();
 }