diff mbox

[v2,2/5] ARM: kernel: add device tree init map function

Message ID 1352471654-20207-3-git-send-email-lorenzo.pieralisi@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lorenzo Pieralisi Nov. 9, 2012, 2:34 p.m. UTC
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 device tree HW identifiers are considered valid if all CPU nodes contain
a "reg" property and the DT defines a CPU node that matches the MPIDR[23:0]
of the boot CPU.

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 | 84 ++++++++++++++++++++++++++
 arch/arm/include/asm/prom.h                    |  2 +
 arch/arm/kernel/devtree.c                      | 76 +++++++++++++++++++++++
 3 files changed, 162 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/cpus.txt

Comments

Will Deacon Nov. 9, 2012, 2:42 p.m. UTC | #1
On Fri, Nov 09, 2012 at 02:34:11PM +0000, 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 device tree HW identifiers are considered valid if all CPU nodes contain
> a "reg" property and the DT defines a CPU node that matches the MPIDR[23:0]
> of the boot CPU.
> 
> 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>

[...]

> +List of possible "compatible" string ids:
> +
> +<arm, arm1020>
> +<arm, arm1020e>
> +<arm, arm1022>
> +<arm, arm1026>
> +<arm, arm720>
> +<arm, arm740>
> +<arm, arm7tdmi>
> +<arm, arm920>
> +<arm, arm922>
> +<arm, arm925>
> +<arm, arm926>
> +<arm, arm940>
> +<arm, arm946>
> +<arm, arm9tdmi>
> +<arm, fa526>
> +<arm, feroceon>
> +<arm, mohawk>
> +<arm, sa110>
> +<arm, sa1100>
> +<arm, xsc3>
> +<arm, xscale>
> +<arm, cortex-a5>
> +<arm, cortex-a7>
> +<arm, cortex-a8>
> +<arm, cortex-a9>
> +<arm, cortex-a15>
> +<arm, arm1136>
> +<arm, arm11-mpcore>

Is this supposed to be an exhaustive list? What about 1156 and 1176? Also,
"arm11mpcore" should probably be used instead of "arm11-mpcore".

Will
Lorenzo Pieralisi Nov. 9, 2012, 2:57 p.m. UTC | #2
On Fri, Nov 09, 2012 at 02:42:59PM +0000, Will Deacon wrote:
> On Fri, Nov 09, 2012 at 02:34:11PM +0000, 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 device tree HW identifiers are considered valid if all CPU nodes contain
> > a "reg" property and the DT defines a CPU node that matches the MPIDR[23:0]
> > of the boot CPU.
> > 
> > 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>
> 
> [...]
> 
> > +List of possible "compatible" string ids:
> > +
> > +<arm, arm1020>
> > +<arm, arm1020e>
> > +<arm, arm1022>
> > +<arm, arm1026>
> > +<arm, arm720>
> > +<arm, arm740>
> > +<arm, arm7tdmi>
> > +<arm, arm920>
> > +<arm, arm922>
> > +<arm, arm925>
> > +<arm, arm926>
> > +<arm, arm940>
> > +<arm, arm946>
> > +<arm, arm9tdmi>
> > +<arm, fa526>
> > +<arm, feroceon>
> > +<arm, mohawk>
> > +<arm, sa110>
> > +<arm, sa1100>
> > +<arm, xsc3>
> > +<arm, xscale>
> > +<arm, cortex-a5>
> > +<arm, cortex-a7>
> > +<arm, cortex-a8>
> > +<arm, cortex-a9>
> > +<arm, cortex-a15>
> > +<arm, arm1136>
> > +<arm, arm11-mpcore>
> 
> Is this supposed to be an exhaustive list? What about 1156 and 1176? Also,
> "arm11mpcore" should probably be used instead of "arm11-mpcore".

I added the list since IMHO defining a compatible property and not
defining a list of possible strings can encourage abuse. Happy to do
whatever DT guys think I should do with it.

I will add the processor version strings you suggested.

Thanks,
Lorenzo
Mark Rutland Nov. 12, 2012, 10:38 a.m. UTC | #3
On Fri, Nov 09, 2012 at 02:34:11PM +0000, 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 device tree HW identifiers are considered valid if all CPU nodes contain
> a "reg" property and the DT defines a CPU node that matches the MPIDR[23:0]
> of the boot CPU.
> 
> 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 | 84 ++++++++++++++++++++++++++
>  arch/arm/include/asm/prom.h                    |  2 +
>  arch/arm/kernel/devtree.c                      | 76 +++++++++++++++++++++++
>  3 files changed, 162 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..83cd98a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> @@ -0,0 +1,84 @@
> +* 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 properties:
> +
> +- reg : property matching the CPU MPIDR[23:0] register bits
> +- compatible: must be set to "arm, <cpu-model>"
> +              where <cpu-model> is the full processor name as used in the
> +              processor Technical Reference Manual, eg:
> +              - for a Cortex A9 processor
> +                compatible = <arm, cortex-a9>;
> +              - for a Cortex A15 processor
> +                compatible = <arm, cortex-a15>;
> +
> +List of possible "compatible" string ids:
> +
> +<arm, arm1020>
> +<arm, arm1020e>
> +<arm, arm1022>
> +<arm, arm1026>
> +<arm, arm720>
> +<arm, arm740>
> +<arm, arm7tdmi>
> +<arm, arm920>
> +<arm, arm922>
> +<arm, arm925>
> +<arm, arm926>
> +<arm, arm940>
> +<arm, arm946>
> +<arm, arm9tdmi>
> +<arm, fa526>
> +<arm, feroceon>
> +<arm, mohawk>
> +<arm, sa110>
> +<arm, sa1100>
> +<arm, xsc3>
> +<arm, xscale>
> +<arm, cortex-a5>
> +<arm, cortex-a7>
> +<arm, cortex-a8>
> +<arm, cortex-a9>
> +<arm, cortex-a15>
> +<arm, arm1136>
> +<arm, arm11-mpcore>
> +

Is there any reason for the spaces in the compatible string? No other binding
seems to do this.

Is <STRING> a valid dts format? Should these not be "STRING" instead?

For consistency, it would be nice to have the compatible strings tab-indented
as in Documentation/devicetree/bindings/arm/pmu.txt.

Thanks,
Mark
Lorenzo Pieralisi Nov. 12, 2012, 11:51 a.m. UTC | #4
Hi Mark,

On Mon, Nov 12, 2012 at 10:38:09AM +0000, Mark Rutland wrote:
> On Fri, Nov 09, 2012 at 02:34:11PM +0000, 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 device tree HW identifiers are considered valid if all CPU nodes contain
> > a "reg" property and the DT defines a CPU node that matches the MPIDR[23:0]
> > of the boot CPU.
> > 
> > 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 | 84 ++++++++++++++++++++++++++
> >  arch/arm/include/asm/prom.h                    |  2 +
> >  arch/arm/kernel/devtree.c                      | 76 +++++++++++++++++++++++
> >  3 files changed, 162 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..83cd98a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> > @@ -0,0 +1,84 @@
> > +* 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 properties:
> > +
> > +- reg : property matching the CPU MPIDR[23:0] register bits
> > +- compatible: must be set to "arm, <cpu-model>"
> > +              where <cpu-model> is the full processor name as used in the
> > +              processor Technical Reference Manual, eg:
> > +              - for a Cortex A9 processor
> > +                compatible = <arm, cortex-a9>;
> > +              - for a Cortex A15 processor
> > +                compatible = <arm, cortex-a15>;
> > +
> > +List of possible "compatible" string ids:
> > +
> > +<arm, arm1020>
> > +<arm, arm1020e>
> > +<arm, arm1022>
> > +<arm, arm1026>
> > +<arm, arm720>
> > +<arm, arm740>
> > +<arm, arm7tdmi>
> > +<arm, arm920>
> > +<arm, arm922>
> > +<arm, arm925>
> > +<arm, arm926>
> > +<arm, arm940>
> > +<arm, arm946>
> > +<arm, arm9tdmi>
> > +<arm, fa526>
> > +<arm, feroceon>
> > +<arm, mohawk>
> > +<arm, sa110>
> > +<arm, sa1100>
> > +<arm, xsc3>
> > +<arm, xscale>
> > +<arm, cortex-a5>
> > +<arm, cortex-a7>
> > +<arm, cortex-a8>
> > +<arm, cortex-a9>
> > +<arm, cortex-a15>
> > +<arm, arm1136>
> > +<arm, arm11-mpcore>
> > +
> 
> Is there any reason for the spaces in the compatible string? No other binding
> seems to do this.
> 
> Is <STRING> a valid dts format? Should these not be "STRING" instead?
> 
> For consistency, it would be nice to have the compatible strings tab-indented
> as in Documentation/devicetree/bindings/arm/pmu.txt.

You are absolutely right on all points, I will update the description.

Thanks,
Lorenzo
tip-bot for Dave Martin Nov. 12, 2012, 3:14 p.m. UTC | #5
On Fri, Nov 09, 2012 at 02:34:11PM +0000, 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 device tree HW identifiers are considered valid if all CPU nodes contain
> a "reg" property and the DT defines a CPU node that matches the MPIDR[23:0]
> of the boot CPU.
> 
> 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 | 84 ++++++++++++++++++++++++++
>  arch/arm/include/asm/prom.h                    |  2 +
>  arch/arm/kernel/devtree.c                      | 76 +++++++++++++++++++++++
>  3 files changed, 162 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..83cd98a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> @@ -0,0 +1,84 @@
> +* 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 properties:
> +
> +- reg : property matching the CPU MPIDR[23:0] register bits
> +- compatible: must be set to "arm, <cpu-model>"
> +              where <cpu-model> is the full processor name as used in the
> +              processor Technical Reference Manual, eg:
> +              - for a Cortex A9 processor
> +                compatible = <arm, cortex-a9>;
> +              - for a Cortex A15 processor
> +                compatible = <arm, cortex-a15>;
> +
> +List of possible "compatible" string ids:
> +
> +<arm, arm1020>
> +<arm, arm1020e>
> +<arm, arm1022>
> +<arm, arm1026>
> +<arm, arm720>
> +<arm, arm740>
> +<arm, arm7tdmi>
> +<arm, arm920>
> +<arm, arm922>
> +<arm, arm925>
> +<arm, arm926>
> +<arm, arm940>
> +<arm, arm946>
> +<arm, arm9tdmi>
> +<arm, fa526>
> +<arm, feroceon>
> +<arm, mohawk>
> +<arm, sa110>
> +<arm, sa1100>
> +<arm, xsc3>
> +<arm, xscale>
> +<arm, cortex-a5>
> +<arm, cortex-a7>
> +<arm, cortex-a8>
> +<arm, cortex-a9>
> +<arm, cortex-a15>
> +<arm, arm1136>
> +<arm, arm11-mpcore>

Any views on how we make sure that this list gets maintained?

This binding feels like it probably is the right place to keep the
exhaustive, "official" list of ARM CPU compatible strings, but I would
not be surprised if it gets stale.

Also, do we worry about the "arm," namespace for compatible strings
becoming overcrowded?

Currently we're shovelling a whole load of things in there, some of
which are not ARM products.

Thiry-party implementations like mohawk, sa11*, xsc*, fa526 and feroceon
are not ARM products, and should be in a different, appropriate vendor
namespace.

Cheers
---Dave

> +
> +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";
> +			compatible = <arm, cortex-a15>;
> +			reg = <0x0>;
> +		};
> +
> +		CPU1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = <arm, cortex-a15>;
> +			reg = <0x1>;
> +		};
> +
> +		CPU2: cpu@100 {
> +			device_type = "cpu";
> +			compatible = <arm, cortex-a7>;
> +			reg = <0x100>;
> +		};
> +
> +		CPU3: cpu@101 {
> +			device_type = "cpu";
> +			compatible = <arm, cortex-a7>;
> +			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..d64d222 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,80 @@ 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 *cpu, *cpus;
> +	u32 i, cpuidx = 1, mpidr = read_cpuid_mpidr() & 0xffffff;
> +	u32 tmp_map[NR_CPUS];
> +	bool bootcpu_valid = false;
> +
> +	cpus = of_find_node_by_path("/cpus");
> +
> +	if (!cpus)
> +		return;
> +
> +	memset(tmp_map, 0, sizeof(tmp_map));
> +
> +	for_each_child_of_node(cpus, cpu) {
> +		u32 hwid;
> +
> +		pr_debug(" * %s...\n", cpu->full_name);
> +		/*
> +		 * A device tree containing CPU nodes with missing "reg"
> +		 * properties is considered invalid to build the
> +		 * cpu_logical_map.
> +		 */
> +		if (of_property_read_u32(cpu, "reg", &hwid)) {
> +			pr_debug(" * %s missing reg property\n",
> +				     cpu->full_name);
> +			return;
> +		}
> +
> +		/*
> +		 * Build a stashed array of MPIDR values. Numbering scheme
> +		 * requires that if detected the boot CPU must be assigned
> +		 * logical id 0. Other CPUs get sequential indexes starting
> +		 * from 1. If a CPU node with a reg property matching the
> +		 * boot CPU MPIDR is detected, this is recorded so that the
> +		 * logical map built from DT is validated and can be used
> +		 * to override the map created in smp_setup_processor_id().
> +		 */
> +		if (hwid == mpidr) {
> +			i = 0;
> +			bootcpu_valid = true;
> +		} else {
> +			i = cpuidx++;
> +		}
> +
> +		tmp_map[i] = hwid;
> +
> +		if (cpuidx > nr_cpu_ids)
> +			break;
> +	}
> +
> +	if (WARN(!bootcpu_valid, "DT CPU bindings are not valid,"
> +				 "fall back to default cpu_logical_map\n"))
> +		return;
> +
> +	/*
> +	 * Since the boot CPU node contains proper data, and all nodes have
> +	 * a reg property, the DT CPU list can be considered valid and the
> +	 * logical map created in smp_setup_processor_id() can be overridden
> +	 */
> +	for (i = 0; i < cpuidx; i++) {
> +		set_cpu_possible(i, true);
> +		cpu_logical_map(i) = tmp_map[i];
> +		pr_debug("cpu logical map 0x%x\n", cpu_logical_map(i));
> +	}
> +}
> +
>  /**
>   * setup_machine_fdt - Machine setup when an dtb was passed to the kernel
>   * @dt_phys: physical address of dt blob
> -- 
> 1.7.12
> 
>
Lorenzo Pieralisi Nov. 12, 2012, 3:55 p.m. UTC | #6
On Mon, Nov 12, 2012 at 03:14:01PM +0000, Dave Martin wrote:
> On Fri, Nov 09, 2012 at 02:34:11PM +0000, 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 device tree HW identifiers are considered valid if all CPU nodes contain
> > a "reg" property and the DT defines a CPU node that matches the MPIDR[23:0]
> > of the boot CPU.
> > 
> > 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 | 84 ++++++++++++++++++++++++++
> >  arch/arm/include/asm/prom.h                    |  2 +
> >  arch/arm/kernel/devtree.c                      | 76 +++++++++++++++++++++++
> >  3 files changed, 162 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..83cd98a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> > @@ -0,0 +1,84 @@
> > +* 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 properties:
> > +
> > +- reg : property matching the CPU MPIDR[23:0] register bits
> > +- compatible: must be set to "arm, <cpu-model>"
> > +              where <cpu-model> is the full processor name as used in the
> > +              processor Technical Reference Manual, eg:
> > +              - for a Cortex A9 processor
> > +                compatible = <arm, cortex-a9>;
> > +              - for a Cortex A15 processor
> > +                compatible = <arm, cortex-a15>;
> > +
> > +List of possible "compatible" string ids:
> > +
> > +<arm, arm1020>
> > +<arm, arm1020e>
> > +<arm, arm1022>
> > +<arm, arm1026>
> > +<arm, arm720>
> > +<arm, arm740>
> > +<arm, arm7tdmi>
> > +<arm, arm920>
> > +<arm, arm922>
> > +<arm, arm925>
> > +<arm, arm926>
> > +<arm, arm940>
> > +<arm, arm946>
> > +<arm, arm9tdmi>
> > +<arm, fa526>
> > +<arm, feroceon>
> > +<arm, mohawk>
> > +<arm, sa110>
> > +<arm, sa1100>
> > +<arm, xsc3>
> > +<arm, xscale>
> > +<arm, cortex-a5>
> > +<arm, cortex-a7>
> > +<arm, cortex-a8>
> > +<arm, cortex-a9>
> > +<arm, cortex-a15>
> > +<arm, arm1136>
> > +<arm, arm11-mpcore>
> 
> Any views on how we make sure that this list gets maintained?
> 
> This binding feels like it probably is the right place to keep the
> exhaustive, "official" list of ARM CPU compatible strings, but I would
> not be surprised if it gets stale.
> 
> Also, do we worry about the "arm," namespace for compatible strings
> becoming overcrowded?
> 
> Currently we're shovelling a whole load of things in there, some of
> which are not ARM products.
> 
> Thiry-party implementations like mohawk, sa11*, xsc*, fa526 and feroceon
> are not ARM products, and should be in a different, appropriate vendor
> namespace.

I have nothing to object, all valid points.

Any views/feedback on this please ? I share most of Dave's concerns here.

Thanks,
Lorenzo
tip-bot for Dave Martin Nov. 12, 2012, 5:27 p.m. UTC | #7
On Fri, Nov 09, 2012 at 02:34:11PM +0000, 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 device tree HW identifiers are considered valid if all CPU nodes contain
> a "reg" property and the DT defines a CPU node that matches the MPIDR[23:0]
> of the boot CPU.
> 
> 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 | 84 ++++++++++++++++++++++++++
>  arch/arm/include/asm/prom.h                    |  2 +
>  arch/arm/kernel/devtree.c                      | 76 +++++++++++++++++++++++
>  3 files changed, 162 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..83cd98a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> @@ -0,0 +1,84 @@
> +* 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 properties:
> +
> +- reg : property matching the CPU MPIDR[23:0] register bits
> +- compatible: must be set to "arm, <cpu-model>"
> +              where <cpu-model> is the full processor name as used in the
> +              processor Technical Reference Manual, eg:
> +              - for a Cortex A9 processor
> +                compatible = <arm, cortex-a9>;
> +              - for a Cortex A15 processor
> +                compatible = <arm, cortex-a15>;
> +
> +List of possible "compatible" string ids:
> +
> +<arm, arm1020>
> +<arm, arm1020e>
> +<arm, arm1022>
> +<arm, arm1026>
> +<arm, arm720>
> +<arm, arm740>
> +<arm, arm7tdmi>
> +<arm, arm920>
> +<arm, arm922>
> +<arm, arm925>
> +<arm, arm926>
> +<arm, arm940>
> +<arm, arm946>
> +<arm, arm9tdmi>
> +<arm, fa526>
> +<arm, feroceon>
> +<arm, mohawk>
> +<arm, sa110>
> +<arm, sa1100>
> +<arm, xsc3>
> +<arm, xscale>
> +<arm, cortex-a5>
> +<arm, cortex-a7>
> +<arm, cortex-a8>
> +<arm, cortex-a9>
> +<arm, cortex-a15>
> +<arm, arm1136>
> +<arm, arm11-mpcore>
> +
> +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";
> +			compatible = <arm, cortex-a15>;
> +			reg = <0x0>;
> +		};
> +
> +		CPU1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = <arm, cortex-a15>;
> +			reg = <0x1>;
> +		};
> +
> +		CPU2: cpu@100 {
> +			device_type = "cpu";
> +			compatible = <arm, cortex-a7>;
> +			reg = <0x100>;
> +		};
> +
> +		CPU3: cpu@101 {

Should we document the unit address convention as part of the binding
documentation?

Using the MPIDR value here is a bit cumbersome, but I'm not sure if
there's a better alternative, unless we make a multi-element vector
out of the MPIDR to use as the address -- sounds like overkill.

> +			device_type = "cpu";
> +			compatible = <arm, cortex-a7>;
> +			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..d64d222 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,80 @@ 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
> + */

Can this function sanity-check that we do not assign the same MPIDR
value for multiple logical CPUs?

It turns out to be surprisingly easy to write a DT with duplicate reg
properties in the CPUs node due to careless cut-and-paste.  (i.e., I
did it, but have been getting away with it up to now).

> +void __init arm_dt_init_cpu_maps(void)
> +{

[...]

Cheers
---Dave
Lorenzo Pieralisi Nov. 13, 2012, 10:06 a.m. UTC | #8
On Mon, Nov 12, 2012 at 05:27:53PM +0000, Dave Martin wrote:
> On Fri, Nov 09, 2012 at 02:34:11PM +0000, Lorenzo Pieralisi wrote:

[...]

> > +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";
> > +			compatible = <arm, cortex-a15>;
> > +			reg = <0x0>;
> > +		};
> > +
> > +		CPU1: cpu@1 {
> > +			device_type = "cpu";
> > +			compatible = <arm, cortex-a15>;
> > +			reg = <0x1>;
> > +		};
> > +
> > +		CPU2: cpu@100 {
> > +			device_type = "cpu";
> > +			compatible = <arm, cortex-a7>;
> > +			reg = <0x100>;
> > +		};
> > +
> > +		CPU3: cpu@101 {
> 
> Should we document the unit address convention as part of the binding
> documentation?
> 
> Using the MPIDR value here is a bit cumbersome, but I'm not sure if
> there's a better alternative, unless we make a multi-element vector
> out of the MPIDR to use as the address -- sounds like overkill.

I just followed the booting-without-of.txt convention for /cpu nodes,
where the unit-address is just the reg property stripped of its leading
zeros. I think it is the default way of defining the unit-address I do not
know if I need to add to this, I certainly can.

> > +			device_type = "cpu";
> > +			compatible = <arm, cortex-a7>;
> > +			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..d64d222 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,80 @@ 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
> > + */
> 
> Can this function sanity-check that we do not assign the same MPIDR
> value for multiple logical CPUs?
> 
> It turns out to be surprisingly easy to write a DT with duplicate reg
> properties in the CPUs node due to careless cut-and-paste.  (i.e., I
> did it, but have been getting away with it up to now).

Check added. Waiting for some comments on the compatible list of ids to
post v3.

Thanks a lot,
Lorenzo
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
new file mode 100644
index 0000000..83cd98a
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -0,0 +1,84 @@ 
+* 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 properties:
+
+- reg : property matching the CPU MPIDR[23:0] register bits
+- compatible: must be set to "arm, <cpu-model>"
+              where <cpu-model> is the full processor name as used in the
+              processor Technical Reference Manual, eg:
+              - for a Cortex A9 processor
+                compatible = <arm, cortex-a9>;
+              - for a Cortex A15 processor
+                compatible = <arm, cortex-a15>;
+
+List of possible "compatible" string ids:
+
+<arm, arm1020>
+<arm, arm1020e>
+<arm, arm1022>
+<arm, arm1026>
+<arm, arm720>
+<arm, arm740>
+<arm, arm7tdmi>
+<arm, arm920>
+<arm, arm922>
+<arm, arm925>
+<arm, arm926>
+<arm, arm940>
+<arm, arm946>
+<arm, arm9tdmi>
+<arm, fa526>
+<arm, feroceon>
+<arm, mohawk>
+<arm, sa110>
+<arm, sa1100>
+<arm, xsc3>
+<arm, xscale>
+<arm, cortex-a5>
+<arm, cortex-a7>
+<arm, cortex-a8>
+<arm, cortex-a9>
+<arm, cortex-a15>
+<arm, arm1136>
+<arm, arm11-mpcore>
+
+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";
+			compatible = <arm, cortex-a15>;
+			reg = <0x0>;
+		};
+
+		CPU1: cpu@1 {
+			device_type = "cpu";
+			compatible = <arm, cortex-a15>;
+			reg = <0x1>;
+		};
+
+		CPU2: cpu@100 {
+			device_type = "cpu";
+			compatible = <arm, cortex-a7>;
+			reg = <0x100>;
+		};
+
+		CPU3: cpu@101 {
+			device_type = "cpu";
+			compatible = <arm, cortex-a7>;
+			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..d64d222 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,80 @@  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 *cpu, *cpus;
+	u32 i, cpuidx = 1, mpidr = read_cpuid_mpidr() & 0xffffff;
+	u32 tmp_map[NR_CPUS];
+	bool bootcpu_valid = false;
+
+	cpus = of_find_node_by_path("/cpus");
+
+	if (!cpus)
+		return;
+
+	memset(tmp_map, 0, sizeof(tmp_map));
+
+	for_each_child_of_node(cpus, cpu) {
+		u32 hwid;
+
+		pr_debug(" * %s...\n", cpu->full_name);
+		/*
+		 * A device tree containing CPU nodes with missing "reg"
+		 * properties is considered invalid to build the
+		 * cpu_logical_map.
+		 */
+		if (of_property_read_u32(cpu, "reg", &hwid)) {
+			pr_debug(" * %s missing reg property\n",
+				     cpu->full_name);
+			return;
+		}
+
+		/*
+		 * Build a stashed array of MPIDR values. Numbering scheme
+		 * requires that if detected the boot CPU must be assigned
+		 * logical id 0. Other CPUs get sequential indexes starting
+		 * from 1. If a CPU node with a reg property matching the
+		 * boot CPU MPIDR is detected, this is recorded so that the
+		 * logical map built from DT is validated and can be used
+		 * to override the map created in smp_setup_processor_id().
+		 */
+		if (hwid == mpidr) {
+			i = 0;
+			bootcpu_valid = true;
+		} else {
+			i = cpuidx++;
+		}
+
+		tmp_map[i] = hwid;
+
+		if (cpuidx > nr_cpu_ids)
+			break;
+	}
+
+	if (WARN(!bootcpu_valid, "DT CPU bindings are not valid,"
+				 "fall back to default cpu_logical_map\n"))
+		return;
+
+	/*
+	 * Since the boot CPU node contains proper data, and all nodes have
+	 * a reg property, the DT CPU list can be considered valid and the
+	 * logical map created in smp_setup_processor_id() can be overridden
+	 */
+	for (i = 0; i < cpuidx; i++) {
+		set_cpu_possible(i, true);
+		cpu_logical_map(i) = tmp_map[i];
+		pr_debug("cpu logical map 0x%x\n", cpu_logical_map(i));
+	}
+}
+
 /**
  * setup_machine_fdt - Machine setup when an dtb was passed to the kernel
  * @dt_phys: physical address of dt blob