| Message ID | 1350393709-23546-2-git-send-email-lorenzo.pieralisi@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/16/2012 08:21 AM, Lorenzo Pieralisi wrote: > When booting through a device tree, the kernel cpu logical id map can be > initialized using device tree data passed by FW or through an embedded blob. > > This patch adds a function that parses device tree "cpu" nodes and > retrieves the corresponding CPUs hardware identifiers (MPIDR). > It sets the possible cpus and the cpu logical map values according to > the number of CPUs defined in the device tree and respective properties. > > The primary CPU is assigned cpu logical number 0 to keep the current > convention valid. > > Current bindings documentation is included in the patch: > > Documentation/devicetree/bindings/arm/cpus.txt > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > --- > Documentation/devicetree/bindings/arm/cpus.txt | 42 +++++++++++++++++++++++ > arch/arm/include/asm/prom.h | 2 ++ > arch/arm/kernel/devtree.c | 47 ++++++++++++++++++++++++++ > 3 files changed, 91 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/cpus.txt > > diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt > new file mode 100644 > index 0000000..897f3b4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/cpus.txt > @@ -0,0 +1,42 @@ > +* ARM CPUs binding description > + > +The device tree allows to describe the layout of CPUs in a system through > +the "cpus" node, which in turn contains a number of subnodes (ie "cpu") > +defining properties for every cpu. > + > +Bindings for CPU nodes follow the ePAPR standard, available from: > + > +http://devicetree.org > + > +For the ARM architecture every CPU node must contain the following property: > + > +- reg : property defining the CPU MPIDR[23:0] register bits defining or matching the MPIDR? > + > +Every cpu node is required to set its device_type to "cpu". This is a bit questionable as device_type is deprecated for FDT. However, since the ePAPR defines using it You should add a compatible property for the cpu model. > + > +Example: > + > + cpus { > + #size-cells = <0>; > + #address-cells = <1>; > + > + CPU0: cpu@0 { > + device_type = "cpu"; > + reg = <0x0>; > + }; > + > + CPU1: cpu@1 { > + device_type = "cpu"; > + reg = <0x1>; > + }; > + > + CPU2: cpu@100 { > + device_type = "cpu"; > + reg = <0x100>; > + }; > + > + CPU3: cpu@101 { > + device_type = "cpu"; > + reg = <0x101>; > + }; > + }; > diff --git a/arch/arm/include/asm/prom.h b/arch/arm/include/asm/prom.h > index aeae9c6..8dd51dc 100644 > --- a/arch/arm/include/asm/prom.h > +++ b/arch/arm/include/asm/prom.h > @@ -15,6 +15,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); > > #else /* CONFIG_OF */ > > @@ -24,6 +25,7 @@ static inline struct machine_desc *setup_machine_fdt(unsigned int dt_phys) > } > > static inline void arm_dt_memblock_reserve(void) { } > +static inline void arm_dt_init_cpu_maps(void) { } > > #endif /* CONFIG_OF */ > #endif /* ASMARM_PROM_H */ > diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c > index bee7f9d..c86e414 100644 > --- a/arch/arm/kernel/devtree.c > +++ b/arch/arm/kernel/devtree.c > @@ -19,8 +19,10 @@ > #include <linux/of_irq.h> > #include <linux/of_platform.h> > > +#include <asm/cputype.h> > #include <asm/setup.h> > #include <asm/page.h> > +#include <asm/smp_plat.h> > #include <asm/mach/arch.h> > #include <asm/mach-types.h> > > @@ -61,6 +63,51 @@ void __init arm_dt_memblock_reserve(void) > } > } > > +/* > + * arm_dt_init_cpu_maps - Function retrieves cpu nodes from the device tree > + * and builds the cpu logical map array containing MPIDR values related to > + * logical cpus > + * > + * Updates the cpu possible mask with the number of parsed cpu nodes > + */ > +void __init arm_dt_init_cpu_maps(void) > +{ > + struct device_node *dn = NULL; > + int i, cpu = 1; > + > + while ((dn = of_find_node_by_type(dn, "cpu")) && cpu <= nr_cpu_ids) { I think all /cpu nodes would have the right type. You could use for_each_child_of_node here. > + const u32 *hwid; > + int len; > + > + pr_debug(" * %s...\n", dn->full_name); > + > + hwid = of_get_property(dn, "reg", &len); > + Use of_property_read_u32. > + if (!hwid || len != 4) { > + pr_err(" * %s missing reg property\n", dn->full_name); > + continue; > + } > + /* > + * We want to assign the boot CPU logical id 0. > + * Boot CPU checks its own MPIDR and if matches the current > + * cpu node "reg" value it sets the logical cpu index to 0 > + * and stores the physical id accordingly. > + * If MPIDR does not match, assign sequential cpu logical > + * id (starting from 1) and increments it. > + */ > + i = (be32_to_cpup(hwid) == (read_cpuid_mpidr() & 0xffffff)) > + ? 0 : cpu++; > + > + if (!i) > + printk(KERN_INFO "Booting Linux on CPU HWID 0x%x\n", > + be32_to_cpup(hwid)); > + > + cpu_logical_map(i) = be32_to_cpup(hwid); > + > + set_cpu_possible(i, true); > + } > +} > + > /** > * setup_machine_fdt - Machine setup when an dtb was passed to the kernel > * @dt_phys: physical address of dt blob >
Hi Rob, thanks for having a look. On Tue, Oct 16, 2012 at 09:42:43PM +0100, Rob Herring wrote: > On 10/16/2012 08:21 AM, Lorenzo Pieralisi wrote: > > When booting through a device tree, the kernel cpu logical id map can be > > initialized using device tree data passed by FW or through an embedded blob. > > > > This patch adds a function that parses device tree "cpu" nodes and > > retrieves the corresponding CPUs hardware identifiers (MPIDR). > > It sets the possible cpus and the cpu logical map values according to > > the number of CPUs defined in the device tree and respective properties. > > > > The primary CPU is assigned cpu logical number 0 to keep the current > > convention valid. > > > > Current bindings documentation is included in the patch: > > > > Documentation/devicetree/bindings/arm/cpus.txt > > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > --- > > Documentation/devicetree/bindings/arm/cpus.txt | 42 +++++++++++++++++++++++ > > arch/arm/include/asm/prom.h | 2 ++ > > arch/arm/kernel/devtree.c | 47 ++++++++++++++++++++++++++ > > 3 files changed, 91 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/arm/cpus.txt > > > > diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt > > new file mode 100644 > > index 0000000..897f3b4 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/arm/cpus.txt > > @@ -0,0 +1,42 @@ > > +* ARM CPUs binding description > > + > > +The device tree allows to describe the layout of CPUs in a system through > > +the "cpus" node, which in turn contains a number of subnodes (ie "cpu") > > +defining properties for every cpu. > > + > > +Bindings for CPU nodes follow the ePAPR standard, available from: > > + > > +http://devicetree.org > > + > > +For the ARM architecture every CPU node must contain the following property: > > + > > +- reg : property defining the CPU MPIDR[23:0] register bits > > defining or matching the MPIDR? I would say matching, otherwise it reads as if the MPIDR were writable. > > + > > +Every cpu node is required to set its device_type to "cpu". > > This is a bit questionable as device_type is deprecated for FDT. > However, since the ePAPR defines using it > > You should add a compatible property for the cpu model. Ok, I will. > > + > > +Example: > > + > > + cpus { > > + #size-cells = <0>; > > + #address-cells = <1>; > > + > > + CPU0: cpu@0 { > > + device_type = "cpu"; > > + reg = <0x0>; > > + }; > > + > > + CPU1: cpu@1 { > > + device_type = "cpu"; > > + reg = <0x1>; > > + }; > > + > > + CPU2: cpu@100 { > > + device_type = "cpu"; > > + reg = <0x100>; > > + }; > > + > > + CPU3: cpu@101 { > > + device_type = "cpu"; > > + reg = <0x101>; > > + }; > > + }; > > diff --git a/arch/arm/include/asm/prom.h b/arch/arm/include/asm/prom.h > > index aeae9c6..8dd51dc 100644 > > --- a/arch/arm/include/asm/prom.h > > +++ b/arch/arm/include/asm/prom.h > > @@ -15,6 +15,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); > > > > #else /* CONFIG_OF */ > > > > @@ -24,6 +25,7 @@ static inline struct machine_desc *setup_machine_fdt(unsigned int dt_phys) > > } > > > > static inline void arm_dt_memblock_reserve(void) { } > > +static inline void arm_dt_init_cpu_maps(void) { } > > > > #endif /* CONFIG_OF */ > > #endif /* ASMARM_PROM_H */ > > diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c > > index bee7f9d..c86e414 100644 > > --- a/arch/arm/kernel/devtree.c > > +++ b/arch/arm/kernel/devtree.c > > @@ -19,8 +19,10 @@ > > #include <linux/of_irq.h> > > #include <linux/of_platform.h> > > > > +#include <asm/cputype.h> > > #include <asm/setup.h> > > #include <asm/page.h> > > +#include <asm/smp_plat.h> > > #include <asm/mach/arch.h> > > #include <asm/mach-types.h> > > > > @@ -61,6 +63,51 @@ void __init arm_dt_memblock_reserve(void) > > } > > } > > > > +/* > > + * arm_dt_init_cpu_maps - Function retrieves cpu nodes from the device tree > > + * and builds the cpu logical map array containing MPIDR values related to > > + * logical cpus > > + * > > + * Updates the cpu possible mask with the number of parsed cpu nodes > > + */ > > +void __init arm_dt_init_cpu_maps(void) > > +{ > > + struct device_node *dn = NULL; > > + int i, cpu = 1; > > + > > + while ((dn = of_find_node_by_type(dn, "cpu")) && cpu <= nr_cpu_ids) { > > I think all /cpu nodes would have the right type. You could use > for_each_child_of_node here. Starting from /cpus, looked-up by path, right ? > > + const u32 *hwid; > > + int len; > > + > > + pr_debug(" * %s...\n", dn->full_name); > > + > > + hwid = of_get_property(dn, "reg", &len); > > + > > Use of_property_read_u32. Ok. Thanks, Lorenzo
On Tue, Oct 16, 2012 at 02:21:45PM +0100, Lorenzo Pieralisi wrote: > When booting through a device tree, the kernel cpu logical id map can be > initialized using device tree data passed by FW or through an embedded blob. > > This patch adds a function that parses device tree "cpu" nodes and > retrieves the corresponding CPUs hardware identifiers (MPIDR). > It sets the possible cpus and the cpu logical map values according to > the number of CPUs defined in the device tree and respective properties. > > The primary CPU is assigned cpu logical number 0 to keep the current > convention valid. > > Current bindings documentation is included in the patch: > > Documentation/devicetree/bindings/arm/cpus.txt > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> [...] > +void __init arm_dt_init_cpu_maps(void) > +{ > + struct device_node *dn = NULL; > + int i, cpu = 1; > + > + while ((dn = of_find_node_by_type(dn, "cpu")) && cpu <= nr_cpu_ids) { > + const u32 *hwid; > + int len; > + > + pr_debug(" * %s...\n", dn->full_name); > + > + hwid = of_get_property(dn, "reg", &len); > + > + if (!hwid || len != 4) { > + pr_err(" * %s missing reg property\n", dn->full_name); > + continue; > + } > + /* > + * We want to assign the boot CPU logical id 0. > + * Boot CPU checks its own MPIDR and if matches the current > + * cpu node "reg" value it sets the logical cpu index to 0 > + * and stores the physical id accordingly. > + * If MPIDR does not match, assign sequential cpu logical > + * id (starting from 1) and increments it. > + */ > + i = (be32_to_cpup(hwid) == (read_cpuid_mpidr() & 0xffffff)) > + ? 0 : cpu++; > + > + if (!i) > + printk(KERN_INFO "Booting Linux on CPU HWID 0x%x\n", > + be32_to_cpup(hwid)); I don't think we should bother with this print -- we already print something in smp_setup_processor_id, which cannot differ for the boot CPU, If you want the full mpidr, we could change that code to include it as well. We might also want some sanity checking that we do indeed end up with logical id 0 for the boot CPU. If not, I think we should scream and fall back on the simple mapping created earlier. Will
Hi Will, thanks for the review on the series. On Tue, Nov 06, 2012 at 09:50:44PM +0000, Will Deacon wrote: > On Tue, Oct 16, 2012 at 02:21:45PM +0100, Lorenzo Pieralisi wrote: > > When booting through a device tree, the kernel cpu logical id map can be > > initialized using device tree data passed by FW or through an embedded blob. > > > > This patch adds a function that parses device tree "cpu" nodes and > > retrieves the corresponding CPUs hardware identifiers (MPIDR). > > It sets the possible cpus and the cpu logical map values according to > > the number of CPUs defined in the device tree and respective properties. > > > > The primary CPU is assigned cpu logical number 0 to keep the current > > convention valid. > > > > Current bindings documentation is included in the patch: > > > > Documentation/devicetree/bindings/arm/cpus.txt > > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > [...] > > > +void __init arm_dt_init_cpu_maps(void) > > +{ > > + struct device_node *dn = NULL; > > + int i, cpu = 1; > > + > > + while ((dn = of_find_node_by_type(dn, "cpu")) && cpu <= nr_cpu_ids) { > > + const u32 *hwid; > > + int len; > > + > > + pr_debug(" * %s...\n", dn->full_name); > > + > > + hwid = of_get_property(dn, "reg", &len); > > + > > + if (!hwid || len != 4) { > > + pr_err(" * %s missing reg property\n", dn->full_name); > > + continue; > > + } > > + /* > > + * We want to assign the boot CPU logical id 0. > > + * Boot CPU checks its own MPIDR and if matches the current > > + * cpu node "reg" value it sets the logical cpu index to 0 > > + * and stores the physical id accordingly. > > + * If MPIDR does not match, assign sequential cpu logical > > + * id (starting from 1) and increments it. > > + */ > > + i = (be32_to_cpup(hwid) == (read_cpuid_mpidr() & 0xffffff)) > > + ? 0 : cpu++; > > + > > + if (!i) > > + printk(KERN_INFO "Booting Linux on CPU HWID 0x%x\n", > > + be32_to_cpup(hwid)); > > I don't think we should bother with this print -- we already print something > in smp_setup_processor_id, which cannot differ for the boot CPU, If you want > the full mpidr, we could change that code to include it as well. Yes, it is duplicate code, I will remove it. Extending the printk in smp_setup_processor_id() to include the entire MPIDR should be done though, otherwise we might have printk aliases on system with multiple clusters. > > We might also want some sanity checking that we do indeed end up with > logical id 0 for the boot CPU. If not, I think we should scream and fall > back on the simple mapping created earlier. Well, this basically means that we have to make sure this function is executed on the boot CPU (and it is as the code stands now), right ? Since we are reading the MPIDR of the CPU parsing the tree and assign logical cpu 0 accordingly I think we have this check carried out automatically, unless for any given reason someone calls this function on a CPU that is not the boot one (Why ?). Basically I could add a check like: if (WARN_ON(smp_processor_id()) return; to fall back to the previous mapping, but that's overkill IMHO. Thanks, Lorenzo
On Wed, Nov 07, 2012 at 10:23:49AM +0000, Lorenzo Pieralisi wrote: > On Tue, Nov 06, 2012 at 09:50:44PM +0000, Will Deacon wrote: > > > + while ((dn = of_find_node_by_type(dn, "cpu")) && cpu <= nr_cpu_ids) { > > > + const u32 *hwid; > > > + int len; > > > + > > > + pr_debug(" * %s...\n", dn->full_name); > > > + > > > + hwid = of_get_property(dn, "reg", &len); > > > + > > > + if (!hwid || len != 4) { > > > + pr_err(" * %s missing reg property\n", dn->full_name); > > > + continue; > > > + } > > > + /* > > > + * We want to assign the boot CPU logical id 0. > > > + * Boot CPU checks its own MPIDR and if matches the current > > > + * cpu node "reg" value it sets the logical cpu index to 0 > > > + * and stores the physical id accordingly. > > > + * If MPIDR does not match, assign sequential cpu logical > > > + * id (starting from 1) and increments it. > > > + */ > > > + i = (be32_to_cpup(hwid) == (read_cpuid_mpidr() & 0xffffff)) > > > + ? 0 : cpu++; > > > + > > > + if (!i) > > > + printk(KERN_INFO "Booting Linux on CPU HWID 0x%x\n", > > > + be32_to_cpup(hwid)); > > > > I don't think we should bother with this print -- we already print something > > in smp_setup_processor_id, which cannot differ for the boot CPU, If you want > > the full mpidr, we could change that code to include it as well. > > Yes, it is duplicate code, I will remove it. Extending the printk in > smp_setup_processor_id() to include the entire MPIDR should be done > though, otherwise we might have printk aliases on system with multiple > clusters. Feel free to make that change. You could also replace NR_CPUS in smp_setup_processor_id with nr_cpu_ids for consistency (they'll be the same this early on). > > > > We might also want some sanity checking that we do indeed end up with > > logical id 0 for the boot CPU. If not, I think we should scream and fall > > back on the simple mapping created earlier. > > Well, this basically means that we have to make sure this function is > executed on the boot CPU (and it is as the code stands now), right ? Yes, smp is not up yet which is why we're allowed to play with the logical map. > Since we are reading the MPIDR of the CPU parsing the tree and assign logical > cpu 0 accordingly I think we have this check carried out automatically, > unless for any given reason someone calls this function on a CPU that is > not the boot one (Why ?). > > Basically I could add a check like: > > if (WARN_ON(smp_processor_id()) > return; > > to fall back to the previous mapping, but that's overkill IMHO. No, I was thinking about what happens if the devicetree doesn't contain an mpidr property that matches the boot cpu. In this case, we will fail to assign logical ID 0, right? If this happens, we should complain about an invalid devicetree and try to fall back on the logical_map that was generated earlier on. Will
On Wed, Nov 07, 2012 at 11:05:42AM +0000, Will Deacon wrote: > On Wed, Nov 07, 2012 at 10:23:49AM +0000, Lorenzo Pieralisi wrote: > > On Tue, Nov 06, 2012 at 09:50:44PM +0000, Will Deacon wrote: > > > > + while ((dn = of_find_node_by_type(dn, "cpu")) && cpu <= nr_cpu_ids) { > > > > + const u32 *hwid; > > > > + int len; > > > > + > > > > + pr_debug(" * %s...\n", dn->full_name); > > > > + > > > > + hwid = of_get_property(dn, "reg", &len); > > > > + > > > > + if (!hwid || len != 4) { > > > > + pr_err(" * %s missing reg property\n", dn->full_name); > > > > + continue; > > > > + } > > > > + /* > > > > + * We want to assign the boot CPU logical id 0. > > > > + * Boot CPU checks its own MPIDR and if matches the current > > > > + * cpu node "reg" value it sets the logical cpu index to 0 > > > > + * and stores the physical id accordingly. > > > > + * If MPIDR does not match, assign sequential cpu logical > > > > + * id (starting from 1) and increments it. > > > > + */ > > > > + i = (be32_to_cpup(hwid) == (read_cpuid_mpidr() & 0xffffff)) > > > > + ? 0 : cpu++; > > > > + > > > > + if (!i) > > > > + printk(KERN_INFO "Booting Linux on CPU HWID 0x%x\n", > > > > + be32_to_cpup(hwid)); > > > > > > I don't think we should bother with this print -- we already print something > > > in smp_setup_processor_id, which cannot differ for the boot CPU, If you want > > > the full mpidr, we could change that code to include it as well. > > > > Yes, it is duplicate code, I will remove it. Extending the printk in > > smp_setup_processor_id() to include the entire MPIDR should be done > > though, otherwise we might have printk aliases on system with multiple > > clusters. > > Feel free to make that change. You could also replace NR_CPUS in > smp_setup_processor_id with nr_cpu_ids for consistency (they'll be the same > this early on). Ok. > > > We might also want some sanity checking that we do indeed end up with > > > logical id 0 for the boot CPU. If not, I think we should scream and fall > > > back on the simple mapping created earlier. > > > > Well, this basically means that we have to make sure this function is > > executed on the boot CPU (and it is as the code stands now), right ? > > Yes, smp is not up yet which is why we're allowed to play with the logical > map. > > > Since we are reading the MPIDR of the CPU parsing the tree and assign logical > > cpu 0 accordingly I think we have this check carried out automatically, > > unless for any given reason someone calls this function on a CPU that is > > not the boot one (Why ?). > > > > Basically I could add a check like: > > > > if (WARN_ON(smp_processor_id()) > > return; > > > > to fall back to the previous mapping, but that's overkill IMHO. > > No, I was thinking about what happens if the devicetree doesn't contain an > mpidr property that matches the boot cpu. In this case, we will fail to > assign logical ID 0, right? If this happens, we should complain about an > invalid devicetree and try to fall back on the logical_map that was > generated earlier on. Good point. What I could do, I can assign the MPIDR of the boot CPU to the logical index 0 before even starting to parse the DT (that's what it is done in smp_setup_processor_id(), with a couple of twists). Then, if I find a node that matches the boot CPU mpidr I just skip over it. This way the boot CPU MPIDR will always be correct the only difference with the current approach will be that instead of generating the secondaries MPIDRs we will read them from DT. The problem with this approach is that if we need a pointer (phandle) to the boot CPU DT node through the MPIDR and the boot CPU node is botched or missing we still behave as if the DT CPU nodes were ok. I think I'd better stick a warning condition in there if the boot CPU node is not present or botched (from a MPIDR perspective at least). Thoughts ? Lorenzo
On Wed, Nov 07, 2012 at 12:00:52PM +0000, Lorenzo Pieralisi wrote: > On Wed, Nov 07, 2012 at 11:05:42AM +0000, Will Deacon wrote: > > No, I was thinking about what happens if the devicetree doesn't contain an > > mpidr property that matches the boot cpu. In this case, we will fail to > > assign logical ID 0, right? If this happens, we should complain about an > > invalid devicetree and try to fall back on the logical_map that was > > generated earlier on. > > Good point. What I could do, I can assign the MPIDR of the boot CPU to > the logical index 0 before even starting to parse the DT (that's what it > is done in smp_setup_processor_id(), with a couple of twists). Then, if I > find a node that matches the boot CPU mpidr I just skip over it. This > way the boot CPU MPIDR will always be correct the only difference with > the current approach will be that instead of generating the secondaries > MPIDRs we will read them from DT. That should work, although I'm not sure why you can't just give up altogether and use the initial mapping from smp_setup_processor_id? > The problem with this approach is that if we need a pointer (phandle) to the > boot CPU DT node through the MPIDR and the boot CPU node is botched or missing > we still behave as if the DT CPU nodes were ok. Does any code do this? Wouldn't it be much better to lookup logical CPU 0 if you want to find anything out about the boot CPU? > I think I'd better stick a warning condition in there if the boot CPU > node is not present or botched (from a MPIDR perspective at least). Definitely! Will
On Wed, Nov 07, 2012 at 03:35:30PM +0000, Will Deacon wrote: > On Wed, Nov 07, 2012 at 12:00:52PM +0000, Lorenzo Pieralisi wrote: > > On Wed, Nov 07, 2012 at 11:05:42AM +0000, Will Deacon wrote: > > > No, I was thinking about what happens if the devicetree doesn't contain an > > > mpidr property that matches the boot cpu. In this case, we will fail to > > > assign logical ID 0, right? If this happens, we should complain about an > > > invalid devicetree and try to fall back on the logical_map that was > > > generated earlier on. > > > > Good point. What I could do, I can assign the MPIDR of the boot CPU to > > the logical index 0 before even starting to parse the DT (that's what it > > is done in smp_setup_processor_id(), with a couple of twists). Then, if I > > find a node that matches the boot CPU mpidr I just skip over it. This > > way the boot CPU MPIDR will always be correct the only difference with > > the current approach will be that instead of generating the secondaries > > MPIDRs we will read them from DT. > > That should work, although I'm not sure why you can't just give up > altogether and use the initial mapping from smp_setup_processor_id? Since I need to either stash the values to avoid reparsing the tree or at first I just parse to check for correctness, second pass I update the map. I will stash the reg values, and if the boot CPU MPIDR is correct I will copy the stashed map to the cpu_logical_map. If that's unacceptable we will change it. > > The problem with this approach is that if we need a pointer (phandle) to the > > boot CPU DT node through the MPIDR and the boot CPU node is botched or missing > > we still behave as if the DT CPU nodes were ok. > > Does any code do this? Wouldn't it be much better to lookup logical CPU 0 if > you want to find anything out about the boot CPU? Formulated my point in a horrible way sorry. In order to retrieve the logical id of a CPU (eg IRQ affinity list) we need its MPIDR for the reverse look-up and for that to work the reg property in the /cpu nodes must be correct. Let's gloss over this for now. > > I think I'd better stick a warning condition in there if the boot CPU > > node is not present or botched (from a MPIDR perspective at least). Done, IMHO I wrote some code that is too convoluted, I will post it anyway for review to get further feeback. Thanks !! Lorenzo
diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt new file mode 100644 index 0000000..897f3b4 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/cpus.txt @@ -0,0 +1,42 @@ +* ARM CPUs binding description + +The device tree allows to describe the layout of CPUs in a system through +the "cpus" node, which in turn contains a number of subnodes (ie "cpu") +defining properties for every cpu. + +Bindings for CPU nodes follow the ePAPR standard, available from: + +http://devicetree.org + +For the ARM architecture every CPU node must contain the following property: + +- reg : property defining the CPU MPIDR[23:0] register bits + +Every cpu node is required to set its device_type to "cpu". + +Example: + + cpus { + #size-cells = <0>; + #address-cells = <1>; + + CPU0: cpu@0 { + device_type = "cpu"; + reg = <0x0>; + }; + + CPU1: cpu@1 { + device_type = "cpu"; + reg = <0x1>; + }; + + CPU2: cpu@100 { + device_type = "cpu"; + reg = <0x100>; + }; + + CPU3: cpu@101 { + device_type = "cpu"; + reg = <0x101>; + }; + }; diff --git a/arch/arm/include/asm/prom.h b/arch/arm/include/asm/prom.h index aeae9c6..8dd51dc 100644 --- a/arch/arm/include/asm/prom.h +++ b/arch/arm/include/asm/prom.h @@ -15,6 +15,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); #else /* CONFIG_OF */ @@ -24,6 +25,7 @@ static inline struct machine_desc *setup_machine_fdt(unsigned int dt_phys) } static inline void arm_dt_memblock_reserve(void) { } +static inline void arm_dt_init_cpu_maps(void) { } #endif /* CONFIG_OF */ #endif /* ASMARM_PROM_H */ diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c index bee7f9d..c86e414 100644 --- a/arch/arm/kernel/devtree.c +++ b/arch/arm/kernel/devtree.c @@ -19,8 +19,10 @@ #include <linux/of_irq.h> #include <linux/of_platform.h> +#include <asm/cputype.h> #include <asm/setup.h> #include <asm/page.h> +#include <asm/smp_plat.h> #include <asm/mach/arch.h> #include <asm/mach-types.h> @@ -61,6 +63,51 @@ void __init arm_dt_memblock_reserve(void) } } +/* + * arm_dt_init_cpu_maps - Function retrieves cpu nodes from the device tree + * and builds the cpu logical map array containing MPIDR values related to + * logical cpus + * + * Updates the cpu possible mask with the number of parsed cpu nodes + */ +void __init arm_dt_init_cpu_maps(void) +{ + struct device_node *dn = NULL; + int i, cpu = 1; + + while ((dn = of_find_node_by_type(dn, "cpu")) && cpu <= nr_cpu_ids) { + const u32 *hwid; + int len; + + pr_debug(" * %s...\n", dn->full_name); + + hwid = of_get_property(dn, "reg", &len); + + if (!hwid || len != 4) { + pr_err(" * %s missing reg property\n", dn->full_name); + continue; + } + /* + * We want to assign the boot CPU logical id 0. + * Boot CPU checks its own MPIDR and if matches the current + * cpu node "reg" value it sets the logical cpu index to 0 + * and stores the physical id accordingly. + * If MPIDR does not match, assign sequential cpu logical + * id (starting from 1) and increments it. + */ + i = (be32_to_cpup(hwid) == (read_cpuid_mpidr() & 0xffffff)) + ? 0 : cpu++; + + if (!i) + printk(KERN_INFO "Booting Linux on CPU HWID 0x%x\n", + be32_to_cpup(hwid)); + + cpu_logical_map(i) = be32_to_cpup(hwid); + + set_cpu_possible(i, true); + } +} + /** * setup_machine_fdt - Machine setup when an dtb was passed to the kernel * @dt_phys: physical address of dt blob
When booting through a device tree, the kernel cpu logical id map can be initialized using device tree data passed by FW or through an embedded blob. This patch adds a function that parses device tree "cpu" nodes and retrieves the corresponding CPUs hardware identifiers (MPIDR). It sets the possible cpus and the cpu logical map values according to the number of CPUs defined in the device tree and respective properties. The primary CPU is assigned cpu logical number 0 to keep the current convention valid. Current bindings documentation is included in the patch: Documentation/devicetree/bindings/arm/cpus.txt Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> --- Documentation/devicetree/bindings/arm/cpus.txt | 42 +++++++++++++++++++++++ arch/arm/include/asm/prom.h | 2 ++ arch/arm/kernel/devtree.c | 47 ++++++++++++++++++++++++++ 3 files changed, 91 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/cpus.txt