| Message ID | 1373883732-26303-3-git-send-email-Sudeep.KarkadaNagesha@arm.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
On 07/15/2013 05:22 AM, Sudeep.KarkadaNagesha@arm.com wrote: > From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com> > > CPU subsystem now provides architecture specific hook to retrieve the > of_node. Most of the cpu DT node parsing and initialisation is contained > in devtree.c. It's better to contain all CPU device node parsing there. > > arch_of_get_cpu_node is mainly used to assign cpu->of_node when CPUs get > registered. This patch overrides the defination of the same. It can also > act as the helper function in pre-SMP/early initialisation stages to > retrieve CPU device node pointers in logical ordering. > > This mainly helps to avoid replication of the code doing CPU node parsing > and physical(MPIDR) to logical mapping. > > Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com> [snip] > +struct device_node * __init arch_of_get_cpu_node(int cpu) > +{ > + struct device_node *cpun, *cpus; > + const u32 *cell; > + u64 hwid; > + int ac; > + > + cpus = of_find_node_by_path("/cpus"); > + if (WARN(!cpus, "Missing cpus node, bailing out\n")) > + return NULL; > + > + if (WARN_ON(of_property_read_u32(cpus, "#address-cells", &ac))) > + ac = of_n_addr_cells(cpus); > + > + for_each_child_of_node(cpus, cpun) { > + if (of_node_cmp(cpun->type, "cpu")) > + continue; > + cell = of_get_property(cpun, "reg", NULL); > + if (WARN(!cell, "%s: missing reg property\n", cpun->full_name)) > + continue; > + > + hwid = of_read_number(cell, ac); > + if ((hwid & MPIDR_HWID_BITMASK) == cpu_logical_map(cpu)) Most of this function is not ARM specific, so it would be nice if we could shrink the arch specific part down to just this match. A default match of reg == logical cpu number might be useful. The powermac cpufreq driver could probably also be converted (arch/powerpc/platforms/powermac/cpufreq_64.c). Other than that, this series looks good to me. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 16 July 2013 00:40, Rob Herring <robherring2@gmail.com> wrote: > The powermac cpufreq driver could probably also be converted > (arch/powerpc/platforms/powermac/cpufreq_64.c). Its called: drivers/cpufreq/pmac64-cpufreq.c now. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15/07/13 20:10, Rob Herring wrote: > On 07/15/2013 05:22 AM, Sudeep.KarkadaNagesha@arm.com wrote: >> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com> >> >> CPU subsystem now provides architecture specific hook to retrieve the >> of_node. Most of the cpu DT node parsing and initialisation is contained >> in devtree.c. It's better to contain all CPU device node parsing there. >> >> arch_of_get_cpu_node is mainly used to assign cpu->of_node when CPUs get >> registered. This patch overrides the defination of the same. It can also >> act as the helper function in pre-SMP/early initialisation stages to >> retrieve CPU device node pointers in logical ordering. >> >> This mainly helps to avoid replication of the code doing CPU node parsing >> and physical(MPIDR) to logical mapping. >> >> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com> > > [snip] > >> +struct device_node * __init arch_of_get_cpu_node(int cpu) >> +{ >> + struct device_node *cpun, *cpus; >> + const u32 *cell; >> + u64 hwid; >> + int ac; >> + >> + cpus = of_find_node_by_path("/cpus"); >> + if (WARN(!cpus, "Missing cpus node, bailing out\n")) >> + return NULL; >> + >> + if (WARN_ON(of_property_read_u32(cpus, "#address-cells", &ac))) >> + ac = of_n_addr_cells(cpus); >> + >> + for_each_child_of_node(cpus, cpun) { >> + if (of_node_cmp(cpun->type, "cpu")) >> + continue; >> + cell = of_get_property(cpun, "reg", NULL); >> + if (WARN(!cell, "%s: missing reg property\n", cpun->full_name)) >> + continue; >> + >> + hwid = of_read_number(cell, ac); >> + if ((hwid & MPIDR_HWID_BITMASK) == cpu_logical_map(cpu)) > > Most of this function is not ARM specific, so it would be nice if we > could shrink the arch specific part down to just this match. A default > match of reg == logical cpu number might be useful. > I completely agree, in fact that was my initial idea too. But when I had a look at powerpc implementation of "of_get_cpu_node" in arch/powerpc/kernel/prom.c, it looked like PPC is using some compatibles(e.g. ibm,ppc-interrupt-server#s) which are not specified in ePAPR. I am not sure is that's allowed or not, if allowed then we can't have generic of_get_cpu_node with just arch specific hwid matching function. Let me know how would you prefer me to proceed on this. Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 16/07/13 10:03, Sudeep KarkadaNagesha wrote: > On 15/07/13 20:10, Rob Herring wrote: >> On 07/15/2013 05:22 AM, Sudeep.KarkadaNagesha@arm.com wrote: >>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com> >>> >>> CPU subsystem now provides architecture specific hook to retrieve the >>> of_node. Most of the cpu DT node parsing and initialisation is contained >>> in devtree.c. It's better to contain all CPU device node parsing there. >>> >>> arch_of_get_cpu_node is mainly used to assign cpu->of_node when CPUs get >>> registered. This patch overrides the defination of the same. It can also >>> act as the helper function in pre-SMP/early initialisation stages to >>> retrieve CPU device node pointers in logical ordering. >>> >>> This mainly helps to avoid replication of the code doing CPU node parsing >>> and physical(MPIDR) to logical mapping. >>> >>> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com> >> >> [snip] >> >>> +struct device_node * __init arch_of_get_cpu_node(int cpu) >>> +{ >>> + struct device_node *cpun, *cpus; >>> + const u32 *cell; >>> + u64 hwid; >>> + int ac; >>> + >>> + cpus = of_find_node_by_path("/cpus"); >>> + if (WARN(!cpus, "Missing cpus node, bailing out\n")) >>> + return NULL; >>> + >>> + if (WARN_ON(of_property_read_u32(cpus, "#address-cells", &ac))) >>> + ac = of_n_addr_cells(cpus); >>> + >>> + for_each_child_of_node(cpus, cpun) { >>> + if (of_node_cmp(cpun->type, "cpu")) >>> + continue; >>> + cell = of_get_property(cpun, "reg", NULL); >>> + if (WARN(!cell, "%s: missing reg property\n", cpun->full_name)) >>> + continue; >>> + >>> + hwid = of_read_number(cell, ac); >>> + if ((hwid & MPIDR_HWID_BITMASK) == cpu_logical_map(cpu)) >> >> Most of this function is not ARM specific, so it would be nice if we >> could shrink the arch specific part down to just this match. A default >> match of reg == logical cpu number might be useful. >> > I completely agree, in fact that was my initial idea too. > > But when I had a look at powerpc implementation of "of_get_cpu_node" in > arch/powerpc/kernel/prom.c, it looked like PPC is using some > compatibles(e.g. ibm,ppc-interrupt-server#s) which are not specified in > ePAPR. I am not sure is that's allowed or not, if allowed then we can't > have generic of_get_cpu_node with just arch specific hwid matching function. I meant property names not compatibles. Looks like PPC and SPARC seem to use non-standard property names like "cpuid", "ibm,ppc-interrupt-server#s" instead of single "reg" property for all cpus/threads Since the cpufreq driver doesn't depend on those properties, I moved arch_of_get_cpu_node to OF/DT core in v2. Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/include/asm/prom.h b/arch/arm/include/asm/prom.h index a219227..96f1682 100644 --- a/arch/arm/include/asm/prom.h +++ b/arch/arm/include/asm/prom.h @@ -18,6 +18,7 @@ extern struct machine_desc *setup_machine_fdt(unsigned int dt_phys); extern void arm_dt_memblock_reserve(void); extern void __init arm_dt_init_cpu_maps(void); +extern struct device_node *arch_of_get_cpu_node(int cpu); #else /* CONFIG_OF */ diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c index 5859c8b..3431aa9 100644 --- a/arch/arm/kernel/devtree.c +++ b/arch/arm/kernel/devtree.c @@ -169,6 +169,35 @@ void __init arm_dt_init_cpu_maps(void) } } +struct device_node * __init arch_of_get_cpu_node(int cpu) +{ + struct device_node *cpun, *cpus; + const u32 *cell; + u64 hwid; + int ac; + + cpus = of_find_node_by_path("/cpus"); + if (WARN(!cpus, "Missing cpus node, bailing out\n")) + return NULL; + + if (WARN_ON(of_property_read_u32(cpus, "#address-cells", &ac))) + ac = of_n_addr_cells(cpus); + + for_each_child_of_node(cpus, cpun) { + if (of_node_cmp(cpun->type, "cpu")) + continue; + cell = of_get_property(cpun, "reg", NULL); + if (WARN(!cell, "%s: missing reg property\n", cpun->full_name)) + continue; + + hwid = of_read_number(cell, ac); + if ((hwid & MPIDR_HWID_BITMASK) == cpu_logical_map(cpu)) + return cpun; + } + + return NULL; +} + /** * setup_machine_fdt - Machine setup when an dtb was passed to the kernel * @dt_phys: physical address of dt blob