[RFC PATCH 0/2] ARM: DT cpu bindings updates

[RFC PATCH 0/2] ARM: DT cpu bindings updates

[Lorenzo Pieralisi]

The introduction of DT cpus/cpu bindings for ARM requires well established
rules to enforce the reg property definition for 32-bit and 64-bit ARM
processors, inclusive of legacy and current uniprocessor/SMP systems.

ARM 64 bit architecture also requires dtb compiled for 64-bit configurations
to be reused for kernels running in 32 bit mode, so the cpus/cpu bindings
specification must be made compliant to cope with this configuration.

Patch #1 of this series is a fix and is included to have a clean patch
series and should get reviewed and merged separately.

Patch #2, along with some kernel fixes related to DT parsing function,
updates the cpu node bindings to cope with legacy, current and upcoming
ARM systems.

In-kernel device tree source files are updated to comply with the latest
specification, so thorough testing is required in order to validate all
changes on all affected ARM systems, in particular systems with exotic
MPIDR configurations that are likely to break with the changes provided.

Code relying on the reg property size to be 4-bytes will break when
dtb compiled for 64-bit kernels are used to boot a 32-bit system so
kernel code relying on that (bogus) assumption must be updated properly.

Lorenzo Pieralisi (2):
  ARM: DT: kernel: move temporary cpu map stack array to static data
  ARM: DT: kernel: DT cpu node bindings update

 Documentation/devicetree/bindings/arm/cpus.txt | 335 +++++++++++++++++++++----
 arch/arm/boot/dts/exynos5440.dtsi              |   7 +
 arch/arm/boot/dts/omap3.dtsi                   |   4 +
 arch/arm/boot/dts/omap4.dtsi                   |   5 +
 arch/arm/boot/dts/omap5.dtsi                   |   5 +
 arch/arm/boot/dts/r8a7740.dtsi                 |   3 +
 arch/arm/boot/dts/sh7372.dtsi                  |   4 +
 arch/arm/boot/dts/sunxi.dtsi                   |   4 +
 arch/arm/kernel/devtree.c                      |  85 ++++---
 9 files changed, 379 insertions(+), 73 deletions(-)

-- 
1.7.12

[RFC PATCH 1/2] ARM: DT: kernel: move temporary cpu map stack array to static data

[Lorenzo Pieralisi]

As the number of CPUs increase, the temporary array allocated on the
stack in arm_dt_init_cpu_maps() can become too big and trigger stack
issues.
This patch moves the allocated memory to static __initdata so that stack
data is not used anymore to allocate the temporary array.
Memory is marked as __initdata since it need not be persistent after boot.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 arch/arm/kernel/devtree.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
index 70f1bde..f149217 100644
--- a/arch/arm/kernel/devtree.c
+++ b/arch/arm/kernel/devtree.c
@@ -78,11 +78,12 @@ void __init arm_dt_init_cpu_maps(void)
 	 * contain a list of MPIDR[23:0] values where MPIDR[31:24] must
 	 * read as 0.
 	 */
+	static u32 tmp_map[NR_CPUS] __initdata = {
+				[0 ... NR_CPUS-1] = UINT_MAX };
 	struct device_node *cpu, *cpus;
 	u32 i, j, cpuidx = 1;
 	u32 mpidr = is_smp() ? read_cpuid_mpidr() & MPIDR_HWID_BITMASK : 0;
 
-	u32 tmp_map[NR_CPUS] = { [0 ... NR_CPUS-1] = UINT_MAX };
 	bool bootcpu_valid = false;
 	cpus = of_find_node_by_path("/cpus");
 
-- 
1.7.12

[RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Lorenzo Pieralisi]

In order to extend the current cpu nodes bindings to newer CPUs
inclusive of AArch64 and to update support for older ARM CPUs this
patch updates device tree documentation for the cpu nodes bindings.

Main changes:

- adds 64-bit bindings (inclusive of cpus node #address-cells updates)
- defines behaviour on pre and post v7 uniprocessor systems
- adds ARM 11MPcore specific reg property definition

DT cpu map parsing code must be made compliant with the latest bindings
updates, hence this patch also updates the arm_dt_init_cpu_maps() function
with checks and additional parsing rules.

Uniprocessor systems predating v7 do not parse the cpus node at all
since the reg property is meaningless on those systems.

Device trees for 64-bit systems can be taken as device tree input also
for 64-bit CPUs running in 32-bit mode. The code checks that the reg entries
are zeroed as required in the respective fields and detects automatically
the cpus node #address-cells value so that device tree written for
64-bit ARM platforms (cpus #address-cells == 2) can still be taken as
input. The correct device tree entries are to be set up by the boot
loader, kernel code just checks that device tree entries in the cpus
node are as expected for a 32-bit CPU (reg[63:24] == 0).

cpu node entries with invalid reg property or containing duplicates are
ignored and the device tree parsing is not stopped anymore when such
entries are encountered, the device tree cpu entry is just skipped.

A device tree with cpu nodes missing the boot CPU MPIDR is considered a
hard error and the kernel flags this up as a bug to force firmware updates.

The updates to cpus/cpu node bindings for ARM require all in-kernel dts files
present in the mainline to be made compliant to the latest specification.

This patch also updates ARM in-kernel non-compliant dts files to the latest
cpus/cpu nodes bindings specification.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
---
 Documentation/devicetree/bindings/arm/cpus.txt | 335 +++++++++++++++++++++----
 arch/arm/boot/dts/exynos5440.dtsi              |   7 +
 arch/arm/boot/dts/omap3.dtsi                   |   4 +
 arch/arm/boot/dts/omap4.dtsi                   |   5 +
 arch/arm/boot/dts/omap5.dtsi                   |   5 +
 arch/arm/boot/dts/r8a7740.dtsi                 |   3 +
 arch/arm/boot/dts/sh7372.dtsi                  |   4 +
 arch/arm/boot/dts/sunxi.dtsi                   |   4 +
 arch/arm/kernel/devtree.c                      |  82 +++---
 9 files changed, 377 insertions(+), 72 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
index f32494d..b1ef3d8 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -1,4 +1,6 @@
-* ARM CPUs binding description
+=============================
+ARM CPUs bindings 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")
@@ -8,70 +10,317 @@ 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:
-
-- device_type:	must be "cpu"
-- reg:		property matching the CPU MPIDR[23:0] register bits
-		reg[31:24] bits must be set to 0
-- compatible:	should be one of:
-		"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,cortex-a5"
-		"arm,cortex-a7"
-		"arm,cortex-a8"
-		"arm,cortex-a9"
-		"arm,cortex-a15"
-		"arm,arm1136"
-		"arm,arm1156"
-		"arm,arm1176"
-		"arm,arm11mpcore"
-		"faraday,fa526"
-		"intel,sa110"
-		"intel,sa1100"
-		"marvell,feroceon"
-		"marvell,mohawk"
-		"marvell,xsc3"
-		"marvell,xscale"
-
-Example:
+with updates for 32-bit and 64-bit ARM systems provided in this document.
+
+In the bindings below:
+
+- square brackets define bitfields, eg reg[7:0] value of the bitfield in
+  the reg property contained in bits 7 down to 0
+
+- cpus node
+
+	Description: Container of cpu nodes
+
+	The node name must be "cpus".
+
+	A cpus node must define the following properties:
+
+	- #address-cells
+		Usage: required
+		Value type: <u32>
+		Definition: must be set to 1 for 32-bit systems and 2 for
+			    64-bit systems
+	- #size-cells
+		Usage: required
+		Value type: <u32>
+		Definition: must be set to 0
+
+- cpu node
+
+	Description: Describes a CPU in an ARM based system
+
+	PROPERTIES
+
+	- device_type
+		Usage: required
+		Value type: <string>
+		Definition: must be "cpu"
+	- reg
+		Usage and definition depend on ARM architecture version and
+		configuration:
+
+			# On uniprocessor ARM architectures previous to v7
+			  this property is optional since they do not define
+			  any register that provides a CPU identifier.
+			  Any value set in the reg property for these CPUs
+			  should be ignored.
+
+			# On ARM 11 MPcore based systems this property is
+			  required and matches the CPUID[11:0] register bits.
+
+			  Bits [11:0] in the reg cell must be set to
+			  bits [11:0] in CPU ID register.
+
+			  All other bits in the reg cell must be set to 0.
+
+			# On ARM architecture versions >= 7 based 32-bit
+			  systems this property is required and matches the
+			  CPU MPIDR[23:0] register bits.
+
+			  Bits [23:0] in the reg cell must be set to
+			  bits [23:0] in MPIDR.
+
+			  All other bits in the reg cell must be set to 0.
+
+			# On ARM v8 64-bit systems, where the reg property
+			  is made up of two cells to accomodate the 64-bit
+			  MPDIR_EL1 register this property is required and
+			  matches:
+
+			  The first reg cell bits [7:0] must be set to
+			  bits [39:32] of MPIDR_EL1.
+
+			  The second reg cell bits [23:0] must be set to
+			  bits [23:0] of MPIDR_EL1.
+
+			  All other bits in the reg cells must be set to 0.
+	- compatible:
+		Usage: required
+		Value type: <string>
+		Definition: should be one of:
+			    "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,cortex-a5"
+			    "arm,cortex-a7"
+			    "arm,cortex-a8"
+			    "arm,cortex-a9"
+			    "arm,cortex-a15"
+			    "arm,cortex-a57"
+			    "arm,arm1136"
+			    "arm,arm1156"
+			    "arm,arm1176"
+			    "arm,arm11mpcore"
+			    "faraday,fa526"
+			    "intel,sa110"
+			    "intel,sa1100"
+			    "marvell,feroceon"
+			    "marvell,mohawk"
+			    "marvell,xsc3"
+			    "marvell,xscale"
+	- enable-method
+		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
+		       systems
+		Value type: <string>
+		Definition: On ARM 64-bit systems must be "spin-table" [1].
+
+	- cpu-release-addr
+		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
+		       systems
+		Value type: <prop-encoded-array>
+		Definition: On ARM 64-bit systems must be a two cell
+			    property identifying a 64-bit zero-initialised
+			    memory location [1].
+
+Example 1 (dual-cluster big.LITTLE system 32-bit):
 
 	cpus {
 		#size-cells = <0>;
 		#address-cells = <1>;
 
-		CPU0: cpu@0 {
+		cpu@0 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a15";
 			reg = <0x0>;
 		};
 
-		CPU1: cpu@1 {
+		cpu@1 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a15";
 			reg = <0x1>;
 		};
 
-		CPU2: cpu@100 {
+		cpu@100 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a7";
 			reg = <0x100>;
 		};
 
-		CPU3: cpu@101 {
+		cpu@101 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a7";
 			reg = <0x101>;
 		};
 	};
+
+Example 2 (Cortex-A8 uniprocessor 32-bit system):
+
+	cpus {
+		#size-cells = <0>;
+		#address-cells = <1>;
+
+		cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a8";
+			reg = <0x0>;
+		};
+
+	};
+
+Example 3 (ARM 1176 uniprocessor 32-bit system):
+
+	cpus {
+		cpu {
+			device_type = "cpu";
+			compatible = "arm,arm1176";
+		};
+	};
+
+Example 4 (ARM Cortex-A57 64-bit system):
+
+cpus {
+	#size-cells = <0>;
+	#address-cells = <2>;
+
+	cpu@0 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x0 0x0>;
+		enable-method = "spin-table";
+		cpu-release-addr = <0 0x20000000>;
+	};
+
+	cpu@1 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x0 0x1>;
+		enable-method = "spin-table";
+		cpu-release-addr = <0 0x20000000>;
+	};
+
+	cpu@100 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x0 0x100>;
+		enable-method = "spin-table";
+		cpu-release-addr = <0 0x20000000>;
+	};
+
+	cpu@101 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x0 0x101>;
+		enable-method = "spin-table";
+		cpu-release-addr = <0 0x20000000>;
+	};
+
+	cpu@10000 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x0 0x10000>;
+		enable-method = "spin-table";
+		cpu-release-addr = <0 0x20000000>;
+	};
+
+	cpu@10001 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x0 0x10001>;
+		enable-method = "spin-table";
+		cpu-release-addr = <0 0x20000000>;
+	};
+
+	cpu@10100 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x0 0x10100>;
+		enable-method = "spin-table";
+		cpu-release-addr = <0 0x20000000>;
+	};
+
+	cpu@10101 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x0 0x10101>;
+		enable-method = "spin-table";
+		cpu-release-addr = <0 0x20000000>;
+	};
+
+	cpu@100000000 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x1 0x0>;
+		enable-method = "spin-table";
+		cpu-release-addr = <0 0x20000000>;
+	};
+
+	cpu@100000001 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x1 0x1>;
+		enable-method = "spin-table";
+		cpu-release-addr = <0 0x20000000>;
+	};
+
+	cpu@100000100 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x1 0x100>;
+		enable-method = "spin-table";
+		cpu-release-addr = <0 0x20000000>;
+	};
+
+	cpu@100000101 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x1 0x101>;
+		enable-method = "spin-table";
+		cpu-release-addr = <0 0x20000000>;
+	};
+
+	cpu@100010000 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x1 0x10000>;
+		enable-method = "spin-table";
+		cpu-release-addr = <0 0x20000000>;
+	};
+
+	cpu@100010001 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x1 0x10001>;
+		enable-method = "spin-table";
+		cpu-release-addr = <0 0x20000000>;
+	};
+
+	cpu@100010100 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x1 0x10100>;
+		enable-method = "spin-table";
+		cpu-release-addr = <0 0x20000000>;
+	};
+
+	cpu@100010101 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a57";
+		reg = <0x1 0x10101>;
+		enable-method = "spin-table";
+		cpu-release-addr = <0 0x20000000>;
+	};
+};
+===============================================================================
+[1] ARM Linux kernel documentation
+    Documentation/devicetree/bindings/arm64/booting.txt
diff --git a/arch/arm/boot/dts/exynos5440.dtsi b/arch/arm/boot/dts/exynos5440.dtsi
index 5f3562a..eda21d5 100644
--- a/arch/arm/boot/dts/exynos5440.dtsi
+++ b/arch/arm/boot/dts/exynos5440.dtsi
@@ -24,8 +24,12 @@
 	};
 
 	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
 		cpu@0 {
 			compatible = "arm,cortex-a15";
+			reg = <0x0>;
 			timer {
 				compatible = "arm,armv7-timer";
 				interrupts = <1 13 0xf08>;
@@ -34,6 +38,7 @@
 		};
 		cpu@1 {
 			compatible = "arm,cortex-a15";
+			reg = <0x1>;
 			timer {
 				compatible = "arm,armv7-timer";
 				interrupts = <1 14 0xf08>;
@@ -42,6 +47,7 @@
 		};
 		cpu@2 {
 			compatible = "arm,cortex-a15";
+			reg = <0x2>;
 			timer {
 				compatible = "arm,armv7-timer";
 				interrupts = <1 14 0xf08>;
@@ -50,6 +56,7 @@
 		};
 		cpu@3 {
 			compatible = "arm,cortex-a15";
+			reg = <0x3>;
 			timer {
 				compatible = "arm,armv7-timer";
 				interrupts = <1 14 0xf08>;
diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index 1acc261..b4e2b8d 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -21,8 +21,12 @@
 	};
 
 	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
 		cpu@0 {
 			compatible = "arm,cortex-a8";
+			reg = <0x0>;
 		};
 	};
 
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 739bb79..9c5f7c2 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -28,13 +28,18 @@
 	};
 
 	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
 		cpu@0 {
 			compatible = "arm,cortex-a9";
 			next-level-cache = <&L2>;
+			reg = <0x0>;
 		};
 		cpu@1 {
 			compatible = "arm,cortex-a9";
 			next-level-cache = <&L2>;
+			reg = <0x1>;
 		};
 	};
 
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index 790bb2a..d2106b6 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -31,8 +31,12 @@
 	};
 
 	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
 		cpu@0 {
 			compatible = "arm,cortex-a15";
+			reg = <0x0>;
 			timer {
 				compatible = "arm,armv7-timer";
 				/* 14th PPI IRQ, active low level-sensitive */
@@ -42,6 +46,7 @@
 		};
 		cpu@1 {
 			compatible = "arm,cortex-a15";
+			reg = <0x1>;
 			timer {
 				compatible = "arm,armv7-timer";
 				/* 14th PPI IRQ, active low level-sensitive */
diff --git a/arch/arm/boot/dts/r8a7740.dtsi b/arch/arm/boot/dts/r8a7740.dtsi
index 798fa35..b47bb36 100644
--- a/arch/arm/boot/dts/r8a7740.dtsi
+++ b/arch/arm/boot/dts/r8a7740.dtsi
@@ -14,8 +14,11 @@
 	compatible = "renesas,r8a7740";
 
 	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
 		cpu@0 {
 			compatible = "arm,cortex-a9";
+			reg = <0x0>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/sh7372.dtsi b/arch/arm/boot/dts/sh7372.dtsi
index 677fc60..78478af 100644
--- a/arch/arm/boot/dts/sh7372.dtsi
+++ b/arch/arm/boot/dts/sh7372.dtsi
@@ -14,8 +14,12 @@
 	compatible = "renesas,sh7372";
 
 	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
 		cpu@0 {
 			compatible = "arm,cortex-a8";
+			reg = <0x0>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/sunxi.dtsi b/arch/arm/boot/dts/sunxi.dtsi
index 8b36abe..447a0f0 100644
--- a/arch/arm/boot/dts/sunxi.dtsi
+++ b/arch/arm/boot/dts/sunxi.dtsi
@@ -17,8 +17,12 @@
 	interrupt-parent = <&intc>;
 
 	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
 		cpu@0 {
 			compatible = "arm,cortex-a8";
+			reg = <0x0>;
 		};
 	};
 
diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
index f149217..4eda2dc 100644
--- a/arch/arm/kernel/devtree.c
+++ b/arch/arm/kernel/devtree.c
@@ -23,6 +23,7 @@
 #include <asm/setup.h>
 #include <asm/page.h>
 #include <asm/smp_plat.h>
+#include <asm/system_info.h>
 #include <asm/mach/arch.h>
 #include <asm/mach-types.h>
 
@@ -81,48 +82,68 @@ void __init arm_dt_init_cpu_maps(void)
 	static u32 tmp_map[NR_CPUS] __initdata = {
 				[0 ... NR_CPUS-1] = UINT_MAX };
 	struct device_node *cpu, *cpus;
-	u32 i, j, cpuidx = 1;
+	u32 i, j, ac, cpuidx = 1;
 	u32 mpidr = is_smp() ? read_cpuid_mpidr() & MPIDR_HWID_BITMASK : 0;
-
+	int len;
 	bool bootcpu_valid = false;
+
 	cpus = of_find_node_by_path("/cpus");
 
-	if (!cpus)
+	if (!cpus || ((cpu_architecture() < CPU_ARCH_ARMv7) && !is_smp()))
 		return;
 
+	if (WARN_ON(of_property_read_u32(cpus, "#address-cells", &ac)))
+		ac = of_n_addr_cells(cpus);
+
 	for_each_child_of_node(cpus, cpu) {
-		u32 hwid;
+		u64 hwid64;
+		u32 hwid32;
+		const __be32 *prop;
 
 		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.
+		 * A CPU node with missing or wrong "reg" property is
+		 * considered invalid to build a cpu_logical_map entry.
 		 */
-		if (of_property_read_u32(cpu, "reg", &hwid)) {
-			pr_debug(" * %s missing reg property\n",
-				     cpu->full_name);
-			return;
+		prop = of_get_property(cpu, "reg", &len);
+		if (!prop || len < (ac * sizeof(*prop))) {
+				WARN(1, " * %s node missing/wrong reg"
+					 " property, skipped\n",
+					 cpu->full_name);
+				goto next;
 		}
-
 		/*
-		 * 8 MSBs must be set to 0 in the DT since the reg property
-		 * defines the MPIDR[23:0].
+		 * Always read reg as u64 value.
+		 * For dts with #address-cells == 1 hwid64[63:32]
+		 * will be set to 0 by of_read_number.
+		 * Toss away the top 32 bits and store value in hwid32.
+		 */
+		hwid32 = hwid64 = of_read_number(prop, ac);
+		/*
+		 * hwid64[63:24] must be always be 0 since the reg
+		 * property defines the MPIDR[23:0] bits regardless
+		 * of the cpus node #address-cells value.
 		 */
-		if (hwid & ~MPIDR_HWID_BITMASK)
-			return;
+		if (hwid64 & ~MPIDR_HWID_BITMASK) {
+			WARN(1, " * %s node reg[63:24] must be 0 on"
+			       " 32-bit dts, got %#016llx, skipped\n",
+			       cpu->full_name, hwid64);
+			goto next;
+		}
 
 		/*
 		 * Duplicate MPIDRs are a recipe for disaster.
 		 * Scan all initialized entries and check for
-		 * duplicates. If any is found just bail out.
+		 * duplicates. If any is found just ignore the CPU.
 		 * temp values were initialized to UINT_MAX
 		 * to avoid matching valid MPIDR[23:0] values.
 		 */
 		for (j = 0; j < cpuidx; j++)
-			if (WARN(tmp_map[j] == hwid, "Duplicate /cpu reg "
-						     "properties in the DT\n"))
-				return;
+			if (WARN(tmp_map[j] == hwid32, " * %s node "
+						"duplicate cpu reg "
+						"property, skipped\n",
+						cpu->full_name))
+				goto next;
 
 		/*
 		 * Build a stashed array of MPIDR values. Numbering scheme
@@ -133,26 +154,29 @@ void __init arm_dt_init_cpu_maps(void)
 		 * logical map built from DT is validated and can be used
 		 * to override the map created in smp_setup_processor_id().
 		 */
-		if (hwid == mpidr) {
+		if (hwid32 == mpidr) {
 			i = 0;
 			bootcpu_valid = true;
 		} else {
 			i = cpuidx++;
 		}
 
-		if (WARN(cpuidx > nr_cpu_ids, "DT /cpu %u nodes greater than "
-					       "max cores %u, capping them\n",
+		if (WARN_ONCE(cpuidx > nr_cpu_ids, "DT cpu %u nodes greater"
+						   " than max cores %u,"
+						   " capping them\n",
 					       cpuidx, nr_cpu_ids)) {
 			cpuidx = nr_cpu_ids;
-			break;
+			goto next;
 		}
 
-		tmp_map[i] = hwid;
+		tmp_map[i] = hwid32;
+next:	;
 	}
-
-	if (WARN(!bootcpu_valid, "DT missing boot CPU MPIDR[23:0], "
-				 "fall back to default cpu_logical_map\n"))
-		return;
+	/*
+	 * A DT missing the boot CPU MPIDR is a really bad omen
+	 * Flag it up as such and force firmware/dtb updates
+	 */
+	BUG_ON(!bootcpu_valid);
 
 	/*
 	 * Since the boot CPU node contains proper data, and all nodes have
-- 
1.7.12

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Stephen Warren]

On 04/15/2013 10:13 AM, Lorenzo Pieralisi wrote:
> In order to extend the current cpu nodes bindings to newer CPUs
> inclusive of AArch64 and to update support for older ARM CPUs this
> patch updates device tree documentation for the cpu nodes bindings.
> 

> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt

>  http://devicetree.org
>  
> -For the ARM architecture every CPU node must contain the following properties:
> -
...
> +with updates for 32-bit and 64-bit ARM systems provided in this document.
> +
> +In the bindings below:

That's a slightly odd change, since it removes the statement that a cpus
node must exist, and "in the bindings below" is not idiomatic for DT
binding definitions.

Perhaps replace that last list with:

The ARM architecture requires the following properties in the cpus and
cpu nodes contain the properties described below.

> +- square brackets define bitfields, eg reg[7:0] value of the bitfield in
> +  the reg property contained in bits 7 down to 0

Isn't that standard enough it's not even worth mentioning? If it is,
it's certainly not something that should be mentioned in the part of the
document that describes which properties are requried.

> +	- #address-cells
> +		Usage: required

"Usage" sounds more like what it's used for. "Presence" seems better to me.

> +			# On ARM architecture versions >= 7 based 32-bit
> +			  systems this property is required and matches the

Perhaps "On 32-bit ARMv7 or later systems, this property ..."

> +			# On ARM v8 64-bit systems, where the reg property

Should there be an explicit note here re: 32-bit SW running on a 64-bit
system?

Perhaps "on ARMv8 systems running 32-bit or 64- bit software, the reg
property ..."

> +			  is made up of two cells to accomodate the 64-bit
> +			  MPDIR_EL1 register this property is required and
> +			  matches:

s/matches/must contain/

> +	- enable-method
> +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
> +		       systems
> +		Value type: <string>
> +		Definition: On ARM 64-bit systems must be "spin-table" [1].

Can that be an integer instead? with dtc+cpp support, that shouldn't
hurt the eyes too much any more.

> +	- cpu-release-addr
> +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
> +		       systems
> +		Value type: <prop-encoded-array>
> +		Definition: On ARM 64-bit systems must be a two cell
> +			    property identifying a 64-bit zero-initialised
> +			    memory location [1].

Presumably that property is required, or not, based on the value of
enable-method, not based on the ARM architecture or bit-size?

> +[1] ARM Linux kernel documentation
> +    Documentation/devicetree/bindings/arm64/booting.txt

Is referencing Linux-specific documentation from a supposedly
OS-agnostic DT binding definition a good idea?

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Lorenzo Pieralisi]

Thanks Stephen for the review.

On Mon, Apr 15, 2013 at 08:26:02PM +0100, Stephen Warren wrote:
> On 04/15/2013 10:13 AM, Lorenzo Pieralisi wrote:
> > In order to extend the current cpu nodes bindings to newer CPUs
> > inclusive of AArch64 and to update support for older ARM CPUs this
> > patch updates device tree documentation for the cpu nodes bindings.
> > 
> 
> > diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> 
> >  http://devicetree.org
> >  
> > -For the ARM architecture every CPU node must contain the following properties:
> > -
> ...
> > +with updates for 32-bit and 64-bit ARM systems provided in this document.
> > +
> > +In the bindings below:
> 
> That's a slightly odd change, since it removes the statement that a cpus
> node must exist, and "in the bindings below" is not idiomatic for DT
> binding definitions.

I beg to differ.

"Bindings for CPU nodes follow the ePAPR standard...."

ePAPR v1.1

3.6 CPUS node properties

"A cpus node is required for all device trees".

> Perhaps replace that last list with:
> 
> The ARM architecture requires the following properties in the cpus and
> cpu nodes contain the properties described below.
> 
> > +- square brackets define bitfields, eg reg[7:0] value of the bitfield in
> > +  the reg property contained in bits 7 down to 0
> 
> Isn't that standard enough it's not even worth mentioning? If it is,
> it's certainly not something that should be mentioned in the part of the
> document that describes which properties are requried.

It is mentioned before cpus node and cpu node descriptions start.

I think you have a point though and it can be omitted, I certainly would
like to understand better what's "standard enough" in DT world to write
these bindings, I took ePAPR as a reference and tried to assume nothing.

> 
> > +	- #address-cells
> > +		Usage: required
> 
> "Usage" sounds more like what it's used for. "Presence" seems better to me.

I have not reinvented the wheel, just had a look at powerPC bindings and
tried to comply. If "Usage" is not proper we also have to patch a number
of in-kernel DT bindings and update the ePAPR.

> > +			# On ARM architecture versions >= 7 based 32-bit
> > +			  systems this property is required and matches the
> 
> Perhaps "On 32-bit ARMv7 or later systems, this property ..."
> 
> > +			# On ARM v8 64-bit systems, where the reg property
> 
> Should there be an explicit note here re: 32-bit SW running on a 64-bit
> system?

Yes, I think I should add an explicit note.

> Perhaps "on ARMv8 systems running 32-bit or 64- bit software, the reg
> property ..."
> 
> > +			  is made up of two cells to accomodate the 64-bit
> > +			  MPDIR_EL1 register this property is required and
> > +			  matches:
> 
> s/matches/must contain/

Ok, I will reword it.

> > +	- enable-method
> > +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
> > +		       systems
> > +		Value type: <string>
> > +		Definition: On ARM 64-bit systems must be "spin-table" [1].
> 
> Can that be an integer instead? with dtc+cpp support, that shouldn't
> hurt the eyes too much any more.

Mmm, I need to read more on dtc+cpp, I do not think that leaving it
as a string would hurt though, am I wrong ? Can we assume that all dts
are preprocessed before being compiled and passed to the kernel ?

I need to catch up on dtc+cpp before commenting further though, thanks
for the hint.

> > +	- cpu-release-addr
> > +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
> > +		       systems
> > +		Value type: <prop-encoded-array>
> > +		Definition: On ARM 64-bit systems must be a two cell
> > +			    property identifying a 64-bit zero-initialised
> > +			    memory location [1].
> 
> Presumably that property is required, or not, based on the value of
> enable-method, not based on the ARM architecture or bit-size?

Yes, you have a point even though the bit-size implicitly plays a role
since we have to decouple 32-bit vs 64-bit boot methods.

> > +[1] ARM Linux kernel documentation
> > +    Documentation/devicetree/bindings/arm64/booting.txt
> 
> Is referencing Linux-specific documentation from a supposedly
> OS-agnostic DT binding definition a good idea?

Well, an OS-agnostic DT binding definition in the Linux kernel Documentation
directory. I would like to hear more on this to understand where these
bindings should be published, for now I have no choice but to point
people at Linux kernel documentation (and the ePAPR does too BTW, even
though it is not for explicit bindings).

Thank you !
Lorenzo

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Stephen Warren]

On 04/16/2013 04:45 AM, Lorenzo Pieralisi wrote:
> Thanks Stephen for the review.
> 
> On Mon, Apr 15, 2013 at 08:26:02PM +0100, Stephen Warren wrote:
>> On 04/15/2013 10:13 AM, Lorenzo Pieralisi wrote:
>>> In order to extend the current cpu nodes bindings to newer CPUs
>>> inclusive of AArch64 and to update support for older ARM CPUs this
>>> patch updates device tree documentation for the cpu nodes bindings.
>>>
>>
>>> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
>>
>>>  http://devicetree.org
>>>  
>>> -For the ARM architecture every CPU node must contain the following properties:
>>> -
>> ...
>>> +with updates for 32-bit and 64-bit ARM systems provided in this document.
>>> +
>>> +In the bindings below:
>>
>> That's a slightly odd change, since it removes the statement that a cpus
>> node must exist, and "in the bindings below" is not idiomatic for DT
>> binding definitions.
> 
> I beg to differ.
> 
> "Bindings for CPU nodes follow the ePAPR standard...."
> 
> ePAPR v1.1
> 
> 3.6 CPUS node properties
> 
> "A cpus node is required for all device trees".

Hmm, OK, I guess that's true. Still, I think this binding was pretty
much complete without having to read that document before wasn't it, yet
now it isn't? I suppose that's OK.

>> Perhaps replace that last list with:
>>
>> The ARM architecture requires the following properties in the cpus and
>> cpu nodes contain the properties described below.
>>
>>> +- square brackets define bitfields, eg reg[7:0] value of the bitfield in
>>> +  the reg property contained in bits 7 down to 0
>>
>> Isn't that standard enough it's not even worth mentioning? If it is,
>> it's certainly not something that should be mentioned in the part of the
>> document that describes which properties are requried.
> 
> It is mentioned before cpus node and cpu node descriptions start.

It's in the list "In the bindings below", which kinda lumps together
some syntax definitions for the text, and the actual semantic
definitions of the node contents.

>>> +	- #address-cells
>>> +		Usage: required
>>
>> "Usage" sounds more like what it's used for. "Presence" seems better to me.
> 
> I have not reinvented the wheel, just had a look at powerPC bindings and
> tried to comply. If "Usage" is not proper we also have to patch a number
> of in-kernel DT bindings and update the ePAPR.

The existing language seems unfortunate. I suppose it makes sense to
follow it since this document is defining a "sub class" of it, but
still, I'd still be tempted just to use the right word.

>>> +	- enable-method
>>> +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
>>> +		       systems
>>> +		Value type: <string>
>>> +		Definition: On ARM 64-bit systems must be "spin-table" [1].
>>
>> Can that be an integer instead? with dtc+cpp support, that shouldn't
>> hurt the eyes too much any more.
> 
> Mmm, I need to read more on dtc+cpp, I do not think that leaving it
> as a string would hurt though, am I wrong ? Can we assume that all dts
> are preprocessed before being compiled and passed to the kernel ?

It wouldn't hurt too much, but representing enums as strings when
there's a capability to simply represent it as a integer just feels
pretty gross to me.

>>> +[1] ARM Linux kernel documentation
>>> +    Documentation/devicetree/bindings/arm64/booting.txt
>>
>> Is referencing Linux-specific documentation from a supposedly
>> OS-agnostic DT binding definition a good idea?
> 
> Well, an OS-agnostic DT binding definition in the Linux kernel Documentation
> directory.

I think that's mainly a historical "accident". There is perpetual talk
of moving the bindings directory, and even the .dts files I believe, to
a separate repository to make this distinction clear.

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Benjamin Herrenschmidt]

On Tue, 2013-04-16 at 09:57 -0600, Stephen Warren wrote:
> > Mmm, I need to read more on dtc+cpp, I do not think that leaving it
> > as a string would hurt though, am I wrong ? Can we assume that all
> dts
> > are preprocessed before being compiled and passed to the kernel ?
> 
> It wouldn't hurt too much, but representing enums as strings when
> there's a capability to simply represent it as a integer just feels
> pretty gross to me.

It's the other way around...

Unless there's a direct semantic meaning of the numbers, things like
that (ie. "methods") should be represented as strings.

Else we fall back to the realm of "magic numbers" which sucks and causes
all sort of pains when several new methods get added by different
people, etc...

The tree is meant to be generally human readable and we should stick to
that. The tiny memory usage is irrelevant on any modern setup.

Cheers,
Ben.

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Dave Martin]

On Mon, Apr 15, 2013 at 01:26:02PM -0600, Stephen Warren wrote:
> On 04/15/2013 10:13 AM, Lorenzo Pieralisi wrote:

[...]

> > +	- enable-method
> > +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
> > +		       systems

If we define no meaningful enumerations for 32-bit systems (for example,
the spin-table boot method doesn't exist), then arguably this property
should be prohibited for 32-bit systems in this version of the binding.

The binding should be extended later when we have enable methods to add?

(enable-method = "psci" is one plausible addition)

> > +		Value type: <string>
> > +		Definition: On ARM 64-bit systems must be "spin-table" [1].
> 
> Can that be an integer instead? with dtc+cpp support, that shouldn't
> hurt the eyes too much any more.

That doesn't feel very DT-like, and it makes the standalone FDT excessively
cryptic without the headers.  Does it really matter that much?  DT parsing
should not be on any performance-critical path.

Mostly, DT convention seems to be to use legible string names for
enumerations like this.

Anything which tends to encrypt the DT content starts us on a slippery
slope IMHO.  Converting the DT back to a vaguely human-readable form
becomes impossible without the headers.

Cheers
---Dave

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Mark Rutland]

Hi Stephen,

> > +	- enable-method
> > +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
> > +		       systems
> > +		Value type: <string>
> > +		Definition: On ARM 64-bit systems must be "spin-table" [1].
> 
> Can that be an integer instead? with dtc+cpp support, that shouldn't
> hurt the eyes too much any more.

The "enable-method" property is described as a stringlist by ePAPR, and is
currently in use on arm64 as such. It *must* remain a string(list) for arm64.

Having it as an integer for arm is only going to cause us additional work,
makes it impossible to share a common dt between 64bit and 32bit, and goes
against the standard. I think it should be a stringlist for arm.

> 
> > +	- cpu-release-addr
> > +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
> > +		       systems
> > +		Value type: <prop-encoded-array>
> > +		Definition: On ARM 64-bit systems must be a two cell
> > +			    property identifying a 64-bit zero-initialised
> > +			    memory location [1].
> 
> Presumably that property is required, or not, based on the value of
> enable-method, not based on the ARM architecture or bit-size?

This should be required if the enable-method depends on it. In ePAPR, it's
specified it should be there if the enable-method is "spin-table". We have
(undocumented) PSCI support on arm64 which doesn't require a cpu-release-addr.

> 
> > +[1] ARM Linux kernel documentation
> > +    Documentation/devicetree/bindings/arm64/booting.txt
> 
> Is referencing Linux-specific documentation from a supposedly
> OS-agnostic DT binding definition a good idea?

Once we have the enable-method(s) and their required properties here, we can
invert the dependency and refer to the binding from the Linux documentation.

Mark.

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Stephen Warren]

On 04/17/2013 03:14 AM, Mark Rutland wrote:
> Hi Stephen,
> 
>>> +	- enable-method
>>> +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
>>> +		       systems
>>> +		Value type: <string>
>>> +		Definition: On ARM 64-bit systems must be "spin-table" [1].
>>
>> Can that be an integer instead? with dtc+cpp support, that shouldn't
>> hurt the eyes too much any more.
> 
> The "enable-method" property is described as a stringlist by ePAPR, and is
> currently in use on arm64 as such. It *must* remain a string(list) for arm64.
> 
> Having it as an integer for arm is only going to cause us additional work,
> makes it impossible to share a common dt between 64bit and 32bit, and goes
> against the standard. I think it should be a stringlist for arm.

OK, that's a great reason for this case.

I hope we don't introduce any more standards that use strings, but that
may just be my personal preference...

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Nicolas Pitre]

On Wed, 17 Apr 2013, Stephen Warren wrote:

> On 04/17/2013 03:14 AM, Mark Rutland wrote:
> > Hi Stephen,
> > 
> >>> +	- enable-method
> >>> +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
> >>> +		       systems
> >>> +		Value type: <string>
> >>> +		Definition: On ARM 64-bit systems must be "spin-table" [1].
> >>
> >> Can that be an integer instead? with dtc+cpp support, that shouldn't
> >> hurt the eyes too much any more.
> > 
> > The "enable-method" property is described as a stringlist by ePAPR, and is
> > currently in use on arm64 as such. It *must* remain a string(list) for arm64.
> > 
> > Having it as an integer for arm is only going to cause us additional work,
> > makes it impossible to share a common dt between 64bit and 32bit, and goes
> > against the standard. I think it should be a stringlist for arm.
> 
> OK, that's a great reason for this case.
> 
> I hope we don't introduce any more standards that use strings, but that
> may just be my personal preference...

I think in any standard, strings are far easier to deal with.  
Especially with config stuff which is far from being performance 
critical. Strings are much less prone to conflicts.  It is too easy to 
"extend" a standard by assigning meanings to free numerical values just 
to discover that someone else did use the same numbers for other 
meanings.

In order to avoid this issue, a central authority has to be established 
to assign numbers out while strings are fine without that most of the 
time.

Not that I advocate for everyone to go wild and create their own 
strings, but if they do it anyway this won't be a catastrophy like 
conflicting numbers are.


Nicolas

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Stephen Warren]

On 04/17/2013 10:02 AM, Nicolas Pitre wrote:
> On Wed, 17 Apr 2013, Stephen Warren wrote:
> 
>> On 04/17/2013 03:14 AM, Mark Rutland wrote:
>>> Hi Stephen,
>>>
>>>>> +	- enable-method
>>>>> +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
>>>>> +		       systems
>>>>> +		Value type: <string>
>>>>> +		Definition: On ARM 64-bit systems must be "spin-table" [1].
>>>>
>>>> Can that be an integer instead? with dtc+cpp support, that shouldn't
>>>> hurt the eyes too much any more.
>>>
>>> The "enable-method" property is described as a stringlist by ePAPR, and is
>>> currently in use on arm64 as such. It *must* remain a string(list) for arm64.
>>>
>>> Having it as an integer for arm is only going to cause us additional work,
>>> makes it impossible to share a common dt between 64bit and 32bit, and goes
>>> against the standard. I think it should be a stringlist for arm.
>>
>> OK, that's a great reason for this case.
>>
>> I hope we don't introduce any more standards that use strings, but that
>> may just be my personal preference...
> 
> I think in any standard, strings are far easier to deal with.  
> Especially with config stuff which is far from being performance 
> critical. Strings are much less prone to conflicts.  It is too easy to 
> "extend" a standard by assigning meanings to free numerical values just 
> to discover that someone else did use the same numbers for other 
> meanings.
> 
> In order to avoid this issue, a central authority has to be established 
> to assign numbers out while strings are fine without that most of the 
> time.

For DT, all strings or numbers must always be documented in the DT
binding, so there's no risk of conflict there.

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Nicolas Pitre]

On Wed, 17 Apr 2013, Stephen Warren wrote:

> On 04/17/2013 10:02 AM, Nicolas Pitre wrote:
> > On Wed, 17 Apr 2013, Stephen Warren wrote:
> > 
> >> On 04/17/2013 03:14 AM, Mark Rutland wrote:
> >>> Hi Stephen,
> >>>
> >>>>> +	- enable-method
> >>>>> +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
> >>>>> +		       systems
> >>>>> +		Value type: <string>
> >>>>> +		Definition: On ARM 64-bit systems must be "spin-table" [1].
> >>>>
> >>>> Can that be an integer instead? with dtc+cpp support, that shouldn't
> >>>> hurt the eyes too much any more.
> >>>
> >>> The "enable-method" property is described as a stringlist by ePAPR, and is
> >>> currently in use on arm64 as such. It *must* remain a string(list) for arm64.
> >>>
> >>> Having it as an integer for arm is only going to cause us additional work,
> >>> makes it impossible to share a common dt between 64bit and 32bit, and goes
> >>> against the standard. I think it should be a stringlist for arm.
> >>
> >> OK, that's a great reason for this case.
> >>
> >> I hope we don't introduce any more standards that use strings, but that
> >> may just be my personal preference...
> > 
> > I think in any standard, strings are far easier to deal with.  
> > Especially with config stuff which is far from being performance 
> > critical. Strings are much less prone to conflicts.  It is too easy to 
> > "extend" a standard by assigning meanings to free numerical values just 
> > to discover that someone else did use the same numbers for other 
> > meanings.
> > 
> > In order to avoid this issue, a central authority has to be established 
> > to assign numbers out while strings are fine without that most of the 
> > time.
> 
> For DT, all strings or numbers must always be documented in the DT
> binding, so there's no risk of conflict there.

No, that's not good enough.  Most of the time, DT properties are created 
during development way earlier than any publication of them.  Picking a 
random number is just creating trouble whereas a string might just be 
right from the start.

Furthermore, what's the point to obfuscate a property name behind a 
numerical indirection?  If what you want to describe is a number then 
just use a number.  But if you want to describe a property, a method, or 
any other attribute with no direct relation to a number except for the 
only purpose of enumeration then there is just no point not to use a 
string up front.


Nicolas

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Dave Martin]

On Wed, Apr 17, 2013 at 12:36:08PM -0400, Nicolas Pitre wrote:
> On Wed, 17 Apr 2013, Stephen Warren wrote:
> 
> > On 04/17/2013 10:02 AM, Nicolas Pitre wrote:
> > > On Wed, 17 Apr 2013, Stephen Warren wrote:
> > > 
> > >> On 04/17/2013 03:14 AM, Mark Rutland wrote:
> > >>> Hi Stephen,
> > >>>
> > >>>>> +	- enable-method
> > >>>>> +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
> > >>>>> +		       systems
> > >>>>> +		Value type: <string>
> > >>>>> +		Definition: On ARM 64-bit systems must be "spin-table" [1].
> > >>>>
> > >>>> Can that be an integer instead? with dtc+cpp support, that shouldn't
> > >>>> hurt the eyes too much any more.
> > >>>
> > >>> The "enable-method" property is described as a stringlist by ePAPR, and is
> > >>> currently in use on arm64 as such. It *must* remain a string(list) for arm64.
> > >>>
> > >>> Having it as an integer for arm is only going to cause us additional work,
> > >>> makes it impossible to share a common dt between 64bit and 32bit, and goes
> > >>> against the standard. I think it should be a stringlist for arm.
> > >>
> > >> OK, that's a great reason for this case.
> > >>
> > >> I hope we don't introduce any more standards that use strings, but that
> > >> may just be my personal preference...
> > > 
> > > I think in any standard, strings are far easier to deal with.  
> > > Especially with config stuff which is far from being performance 
> > > critical. Strings are much less prone to conflicts.  It is too easy to 
> > > "extend" a standard by assigning meanings to free numerical values just 
> > > to discover that someone else did use the same numbers for other 
> > > meanings.
> > > 
> > > In order to avoid this issue, a central authority has to be established 
> > > to assign numbers out while strings are fine without that most of the 
> > > time.
> > 
> > For DT, all strings or numbers must always be documented in the DT
> > binding, so there's no risk of conflict there.
> 
> No, that's not good enough.  Most of the time, DT properties are created 
> during development way earlier than any publication of them.  Picking a 
> random number is just creating trouble whereas a string might just be 
> right from the start.
> 
> Furthermore, what's the point to obfuscate a property name behind a 
> numerical indirection?  If what you want to describe is a number then 
> just use a number.  But if you want to describe a property, a method, or 
> any other attribute with no direct relation to a number except for the 
> only purpose of enumeration then there is just no point not to use a 
> string up front.

I also fear that vendors will follow the precedent and avoid publishing
any symbolic names ... or bindings, or headers ... for enumerated
choices in their non-upstream product trees.  Using the source -- if
you can find it -- will be the only hope for guessing what such entries
in an FDT could possibly mean.  This just makes life unnecessarily hard
for developers.

There is no foolproof way to prevent this, but conventions which
encourage transparency over crypticness at least discourage that
sort of behaviour.

Cheers
---Dave

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Benjamin Herrenschmidt]

On Wed, 2013-04-17 at 09:14 -0600, Stephen Warren wrote:
> On 04/17/2013 03:14 AM, Mark Rutland wrote:
> > Hi Stephen,
> > 
> >>> +	- enable-method
> >>> +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
> >>> +		       systems
> >>> +		Value type: <string>
> >>> +		Definition: On ARM 64-bit systems must be "spin-table" [1].
> >>
> >> Can that be an integer instead? with dtc+cpp support, that shouldn't
> >> hurt the eyes too much any more.
> > 
> > The "enable-method" property is described as a stringlist by ePAPR, and is
> > currently in use on arm64 as such. It *must* remain a string(list) for arm64.
> > 
> > Having it as an integer for arm is only going to cause us additional work,
> > makes it impossible to share a common dt between 64bit and 32bit, and goes
> > against the standard. I think it should be a stringlist for arm.
> 
> OK, that's a great reason for this case.
> 
> I hope we don't introduce any more standards that use strings, but that
> may just be my personal preference...

I happen to have the exact opposite opinion :-)

Cheers,
Ben.

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Grant Likely]

On Wed, 17 Apr 2013 09:14:46 -0600, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> On 04/17/2013 03:14 AM, Mark Rutland wrote:
> > Hi Stephen,
> > 
> >>> +	- enable-method
> >>> +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
> >>> +		       systems
> >>> +		Value type: <string>
> >>> +		Definition: On ARM 64-bit systems must be "spin-table" [1].
> >>
> >> Can that be an integer instead? with dtc+cpp support, that shouldn't
> >> hurt the eyes too much any more.
> > 
> > The "enable-method" property is described as a stringlist by ePAPR, and is
> > currently in use on arm64 as such. It *must* remain a string(list) for arm64.
> > 
> > Having it as an integer for arm is only going to cause us additional work,
> > makes it impossible to share a common dt between 64bit and 32bit, and goes
> > against the standard. I think it should be a stringlist for arm.
> 
> OK, that's a great reason for this case.
> 
> I hope we don't introduce any more standards that use strings, but that
> may just be my personal preference...

We've got lots of bindings using strings like this. I don't really see any problem with it.

g.

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Simon Horman]

On Mon, Apr 15, 2013 at 05:13:22PM +0100, Lorenzo Pieralisi wrote:
> In order to extend the current cpu nodes bindings to newer CPUs
> inclusive of AArch64 and to update support for older ARM CPUs this
> patch updates device tree documentation for the cpu nodes bindings.
> 
> Main changes:
> 
> - adds 64-bit bindings (inclusive of cpus node #address-cells updates)
> - defines behaviour on pre and post v7 uniprocessor systems
> - adds ARM 11MPcore specific reg property definition
> 
> DT cpu map parsing code must be made compliant with the latest bindings
> updates, hence this patch also updates the arm_dt_init_cpu_maps() function
> with checks and additional parsing rules.
> 
> Uniprocessor systems predating v7 do not parse the cpus node at all
> since the reg property is meaningless on those systems.
> 
> Device trees for 64-bit systems can be taken as device tree input also
> for 64-bit CPUs running in 32-bit mode. The code checks that the reg entries
> are zeroed as required in the respective fields and detects automatically
> the cpus node #address-cells value so that device tree written for
> 64-bit ARM platforms (cpus #address-cells == 2) can still be taken as
> input. The correct device tree entries are to be set up by the boot
> loader, kernel code just checks that device tree entries in the cpus
> node are as expected for a 32-bit CPU (reg[63:24] == 0).
> 
> cpu node entries with invalid reg property or containing duplicates are
> ignored and the device tree parsing is not stopped anymore when such
> entries are encountered, the device tree cpu entry is just skipped.
> 
> A device tree with cpu nodes missing the boot CPU MPIDR is considered a
> hard error and the kernel flags this up as a bug to force firmware updates.
> 
> The updates to cpus/cpu node bindings for ARM require all in-kernel dts files
> present in the mainline to be made compliant to the latest specification.
> 
> This patch also updates ARM in-kernel non-compliant dts files to the latest
> cpus/cpu nodes bindings specification.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/arm/cpus.txt | 335 +++++++++++++++++++++----
>  arch/arm/boot/dts/exynos5440.dtsi              |   7 +
>  arch/arm/boot/dts/omap3.dtsi                   |   4 +
>  arch/arm/boot/dts/omap4.dtsi                   |   5 +
>  arch/arm/boot/dts/omap5.dtsi                   |   5 +
>  arch/arm/boot/dts/r8a7740.dtsi                 |   3 +
>  arch/arm/boot/dts/sh7372.dtsi                  |   4 +

I would prefer if the SoC updates, in particular the sh7372 portion,
was a separate patch.

>  arch/arm/boot/dts/sunxi.dtsi                   |   4 +
>  arch/arm/kernel/devtree.c                      |  82 +++---
>  9 files changed, 377 insertions(+), 72 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> index f32494d..b1ef3d8 100644
> --- a/Documentation/devicetree/bindings/arm/cpus.txt
> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> @@ -1,4 +1,6 @@
> -* ARM CPUs binding description
> +=============================
> +ARM CPUs bindings 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")
> @@ -8,70 +10,317 @@ 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:
> -
> -- device_type:	must be "cpu"
> -- reg:		property matching the CPU MPIDR[23:0] register bits
> -		reg[31:24] bits must be set to 0
> -- compatible:	should be one of:
> -		"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,cortex-a5"
> -		"arm,cortex-a7"
> -		"arm,cortex-a8"
> -		"arm,cortex-a9"
> -		"arm,cortex-a15"
> -		"arm,arm1136"
> -		"arm,arm1156"
> -		"arm,arm1176"
> -		"arm,arm11mpcore"
> -		"faraday,fa526"
> -		"intel,sa110"
> -		"intel,sa1100"
> -		"marvell,feroceon"
> -		"marvell,mohawk"
> -		"marvell,xsc3"
> -		"marvell,xscale"
> -
> -Example:
> +with updates for 32-bit and 64-bit ARM systems provided in this document.
> +
> +In the bindings below:
> +
> +- square brackets define bitfields, eg reg[7:0] value of the bitfield in
> +  the reg property contained in bits 7 down to 0
> +
> +- cpus node
> +
> +	Description: Container of cpu nodes
> +
> +	The node name must be "cpus".
> +
> +	A cpus node must define the following properties:
> +
> +	- #address-cells
> +		Usage: required
> +		Value type: <u32>
> +		Definition: must be set to 1 for 32-bit systems and 2 for
> +			    64-bit systems
> +	- #size-cells
> +		Usage: required
> +		Value type: <u32>
> +		Definition: must be set to 0
> +
> +- cpu node
> +
> +	Description: Describes a CPU in an ARM based system
> +
> +	PROPERTIES
> +
> +	- device_type
> +		Usage: required
> +		Value type: <string>
> +		Definition: must be "cpu"
> +	- reg
> +		Usage and definition depend on ARM architecture version and
> +		configuration:
> +
> +			# On uniprocessor ARM architectures previous to v7
> +			  this property is optional since they do not define
> +			  any register that provides a CPU identifier.
> +			  Any value set in the reg property for these CPUs
> +			  should be ignored.
> +
> +			# On ARM 11 MPcore based systems this property is
> +			  required and matches the CPUID[11:0] register bits.
> +
> +			  Bits [11:0] in the reg cell must be set to
> +			  bits [11:0] in CPU ID register.
> +
> +			  All other bits in the reg cell must be set to 0.
> +
> +			# On ARM architecture versions >= 7 based 32-bit
> +			  systems this property is required and matches the
> +			  CPU MPIDR[23:0] register bits.
> +
> +			  Bits [23:0] in the reg cell must be set to
> +			  bits [23:0] in MPIDR.
> +
> +			  All other bits in the reg cell must be set to 0.
> +
> +			# On ARM v8 64-bit systems, where the reg property
> +			  is made up of two cells to accomodate the 64-bit
> +			  MPDIR_EL1 register this property is required and
> +			  matches:
> +
> +			  The first reg cell bits [7:0] must be set to
> +			  bits [39:32] of MPIDR_EL1.
> +
> +			  The second reg cell bits [23:0] must be set to
> +			  bits [23:0] of MPIDR_EL1.
> +
> +			  All other bits in the reg cells must be set to 0.
> +	- compatible:
> +		Usage: required
> +		Value type: <string>
> +		Definition: should be one of:
> +			    "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,cortex-a5"
> +			    "arm,cortex-a7"
> +			    "arm,cortex-a8"
> +			    "arm,cortex-a9"
> +			    "arm,cortex-a15"
> +			    "arm,cortex-a57"
> +			    "arm,arm1136"
> +			    "arm,arm1156"
> +			    "arm,arm1176"
> +			    "arm,arm11mpcore"
> +			    "faraday,fa526"
> +			    "intel,sa110"
> +			    "intel,sa1100"
> +			    "marvell,feroceon"
> +			    "marvell,mohawk"
> +			    "marvell,xsc3"
> +			    "marvell,xscale"
> +	- enable-method
> +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
> +		       systems
> +		Value type: <string>
> +		Definition: On ARM 64-bit systems must be "spin-table" [1].
> +
> +	- cpu-release-addr
> +		Usage: required on ARM 64-bit systems, optional on ARM 32-bit
> +		       systems
> +		Value type: <prop-encoded-array>
> +		Definition: On ARM 64-bit systems must be a two cell
> +			    property identifying a 64-bit zero-initialised
> +			    memory location [1].
> +
> +Example 1 (dual-cluster big.LITTLE system 32-bit):
>  
>  	cpus {
>  		#size-cells = <0>;
>  		#address-cells = <1>;
>  
> -		CPU0: cpu@0 {
> +		cpu@0 {
>  			device_type = "cpu";
>  			compatible = "arm,cortex-a15";
>  			reg = <0x0>;
>  		};
>  
> -		CPU1: cpu@1 {
> +		cpu@1 {
>  			device_type = "cpu";
>  			compatible = "arm,cortex-a15";
>  			reg = <0x1>;
>  		};
>  
> -		CPU2: cpu@100 {
> +		cpu@100 {
>  			device_type = "cpu";
>  			compatible = "arm,cortex-a7";
>  			reg = <0x100>;
>  		};
>  
> -		CPU3: cpu@101 {
> +		cpu@101 {
>  			device_type = "cpu";
>  			compatible = "arm,cortex-a7";
>  			reg = <0x101>;
>  		};
>  	};
> +
> +Example 2 (Cortex-A8 uniprocessor 32-bit system):
> +
> +	cpus {
> +		#size-cells = <0>;
> +		#address-cells = <1>;
> +
> +		cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a8";
> +			reg = <0x0>;
> +		};
> +
> +	};
> +
> +Example 3 (ARM 1176 uniprocessor 32-bit system):
> +
> +	cpus {
> +		cpu {
> +			device_type = "cpu";
> +			compatible = "arm,arm1176";
> +		};
> +	};
> +
> +Example 4 (ARM Cortex-A57 64-bit system):
> +
> +cpus {
> +	#size-cells = <0>;
> +	#address-cells = <2>;
> +
> +	cpu@0 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a57";
> +		reg = <0x0 0x0>;
> +		enable-method = "spin-table";
> +		cpu-release-addr = <0 0x20000000>;
> +	};
> +
> +	cpu@1 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a57";
> +		reg = <0x0 0x1>;
> +		enable-method = "spin-table";
> +		cpu-release-addr = <0 0x20000000>;
> +	};
> +
> +	cpu@100 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a57";
> +		reg = <0x0 0x100>;
> +		enable-method = "spin-table";
> +		cpu-release-addr = <0 0x20000000>;
> +	};
> +
> +	cpu@101 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a57";
> +		reg = <0x0 0x101>;
> +		enable-method = "spin-table";
> +		cpu-release-addr = <0 0x20000000>;
> +	};
> +
> +	cpu@10000 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a57";
> +		reg = <0x0 0x10000>;
> +		enable-method = "spin-table";
> +		cpu-release-addr = <0 0x20000000>;
> +	};
> +
> +	cpu@10001 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a57";
> +		reg = <0x0 0x10001>;
> +		enable-method = "spin-table";
> +		cpu-release-addr = <0 0x20000000>;
> +	};
> +
> +	cpu@10100 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a57";
> +		reg = <0x0 0x10100>;
> +		enable-method = "spin-table";
> +		cpu-release-addr = <0 0x20000000>;
> +	};
> +
> +	cpu@10101 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a57";
> +		reg = <0x0 0x10101>;
> +		enable-method = "spin-table";
> +		cpu-release-addr = <0 0x20000000>;
> +	};
> +
> +	cpu@100000000 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a57";
> +		reg = <0x1 0x0>;
> +		enable-method = "spin-table";
> +		cpu-release-addr = <0 0x20000000>;
> +	};
> +
> +	cpu@100000001 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a57";
> +		reg = <0x1 0x1>;
> +		enable-method = "spin-table";
> +		cpu-release-addr = <0 0x20000000>;
> +	};
> +
> +	cpu@100000100 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a57";
> +		reg = <0x1 0x100>;
> +		enable-method = "spin-table";
> +		cpu-release-addr = <0 0x20000000>;
> +	};
> +
> +	cpu@100000101 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a57";
> +		reg = <0x1 0x101>;
> +		enable-method = "spin-table";
> +		cpu-release-addr = <0 0x20000000>;
> +	};
> +
> +	cpu@100010000 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a57";
> +		reg = <0x1 0x10000>;
> +		enable-method = "spin-table";
> +		cpu-release-addr = <0 0x20000000>;
> +	};
> +
> +	cpu@100010001 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a57";
> +		reg = <0x1 0x10001>;
> +		enable-method = "spin-table";
> +		cpu-release-addr = <0 0x20000000>;
> +	};
> +
> +	cpu@100010100 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a57";
> +		reg = <0x1 0x10100>;
> +		enable-method = "spin-table";
> +		cpu-release-addr = <0 0x20000000>;
> +	};
> +
> +	cpu@100010101 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a57";
> +		reg = <0x1 0x10101>;
> +		enable-method = "spin-table";
> +		cpu-release-addr = <0 0x20000000>;
> +	};
> +};
> +===============================================================================
> +[1] ARM Linux kernel documentation
> +    Documentation/devicetree/bindings/arm64/booting.txt
> diff --git a/arch/arm/boot/dts/exynos5440.dtsi b/arch/arm/boot/dts/exynos5440.dtsi
> index 5f3562a..eda21d5 100644
> --- a/arch/arm/boot/dts/exynos5440.dtsi
> +++ b/arch/arm/boot/dts/exynos5440.dtsi
> @@ -24,8 +24,12 @@
>  	};
>  
>  	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
>  		cpu@0 {
>  			compatible = "arm,cortex-a15";
> +			reg = <0x0>;
>  			timer {
>  				compatible = "arm,armv7-timer";
>  				interrupts = <1 13 0xf08>;
> @@ -34,6 +38,7 @@
>  		};
>  		cpu@1 {
>  			compatible = "arm,cortex-a15";
> +			reg = <0x1>;
>  			timer {
>  				compatible = "arm,armv7-timer";
>  				interrupts = <1 14 0xf08>;
> @@ -42,6 +47,7 @@
>  		};
>  		cpu@2 {
>  			compatible = "arm,cortex-a15";
> +			reg = <0x2>;
>  			timer {
>  				compatible = "arm,armv7-timer";
>  				interrupts = <1 14 0xf08>;
> @@ -50,6 +56,7 @@
>  		};
>  		cpu@3 {
>  			compatible = "arm,cortex-a15";
> +			reg = <0x3>;
>  			timer {
>  				compatible = "arm,armv7-timer";
>  				interrupts = <1 14 0xf08>;
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index 1acc261..b4e2b8d 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -21,8 +21,12 @@
>  	};
>  
>  	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
>  		cpu@0 {
>  			compatible = "arm,cortex-a8";
> +			reg = <0x0>;
>  		};
>  	};
>  
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 739bb79..9c5f7c2 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -28,13 +28,18 @@
>  	};
>  
>  	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
>  		cpu@0 {
>  			compatible = "arm,cortex-a9";
>  			next-level-cache = <&L2>;
> +			reg = <0x0>;
>  		};
>  		cpu@1 {
>  			compatible = "arm,cortex-a9";
>  			next-level-cache = <&L2>;
> +			reg = <0x1>;
>  		};
>  	};
>  
> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
> index 790bb2a..d2106b6 100644
> --- a/arch/arm/boot/dts/omap5.dtsi
> +++ b/arch/arm/boot/dts/omap5.dtsi
> @@ -31,8 +31,12 @@
>  	};
>  
>  	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
>  		cpu@0 {
>  			compatible = "arm,cortex-a15";
> +			reg = <0x0>;
>  			timer {
>  				compatible = "arm,armv7-timer";
>  				/* 14th PPI IRQ, active low level-sensitive */
> @@ -42,6 +46,7 @@
>  		};
>  		cpu@1 {
>  			compatible = "arm,cortex-a15";
> +			reg = <0x1>;
>  			timer {
>  				compatible = "arm,armv7-timer";
>  				/* 14th PPI IRQ, active low level-sensitive */
> diff --git a/arch/arm/boot/dts/r8a7740.dtsi b/arch/arm/boot/dts/r8a7740.dtsi
> index 798fa35..b47bb36 100644
> --- a/arch/arm/boot/dts/r8a7740.dtsi
> +++ b/arch/arm/boot/dts/r8a7740.dtsi
> @@ -14,8 +14,11 @@
>  	compatible = "renesas,r8a7740";
>  
>  	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
>  		cpu@0 {
>  			compatible = "arm,cortex-a9";
> +			reg = <0x0>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/sh7372.dtsi b/arch/arm/boot/dts/sh7372.dtsi
> index 677fc60..78478af 100644
> --- a/arch/arm/boot/dts/sh7372.dtsi
> +++ b/arch/arm/boot/dts/sh7372.dtsi
> @@ -14,8 +14,12 @@
>  	compatible = "renesas,sh7372";
>  
>  	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
>  		cpu@0 {
>  			compatible = "arm,cortex-a8";
> +			reg = <0x0>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/sunxi.dtsi b/arch/arm/boot/dts/sunxi.dtsi
> index 8b36abe..447a0f0 100644
> --- a/arch/arm/boot/dts/sunxi.dtsi
> +++ b/arch/arm/boot/dts/sunxi.dtsi
> @@ -17,8 +17,12 @@
>  	interrupt-parent = <&intc>;
>  
>  	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
>  		cpu@0 {
>  			compatible = "arm,cortex-a8";
> +			reg = <0x0>;
>  		};
>  	};
>  
> diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> index f149217..4eda2dc 100644
> --- a/arch/arm/kernel/devtree.c
> +++ b/arch/arm/kernel/devtree.c
> @@ -23,6 +23,7 @@
>  #include <asm/setup.h>
>  #include <asm/page.h>
>  #include <asm/smp_plat.h>
> +#include <asm/system_info.h>
>  #include <asm/mach/arch.h>
>  #include <asm/mach-types.h>
>  
> @@ -81,48 +82,68 @@ void __init arm_dt_init_cpu_maps(void)
>  	static u32 tmp_map[NR_CPUS] __initdata = {
>  				[0 ... NR_CPUS-1] = UINT_MAX };
>  	struct device_node *cpu, *cpus;
> -	u32 i, j, cpuidx = 1;
> +	u32 i, j, ac, cpuidx = 1;
>  	u32 mpidr = is_smp() ? read_cpuid_mpidr() & MPIDR_HWID_BITMASK : 0;
> -
> +	int len;
>  	bool bootcpu_valid = false;
> +
>  	cpus = of_find_node_by_path("/cpus");
>  
> -	if (!cpus)
> +	if (!cpus || ((cpu_architecture() < CPU_ARCH_ARMv7) && !is_smp()))
>  		return;
>  
> +	if (WARN_ON(of_property_read_u32(cpus, "#address-cells", &ac)))
> +		ac = of_n_addr_cells(cpus);
> +
>  	for_each_child_of_node(cpus, cpu) {
> -		u32 hwid;
> +		u64 hwid64;
> +		u32 hwid32;
> +		const __be32 *prop;
>  
>  		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.
> +		 * A CPU node with missing or wrong "reg" property is
> +		 * considered invalid to build a cpu_logical_map entry.
>  		 */
> -		if (of_property_read_u32(cpu, "reg", &hwid)) {
> -			pr_debug(" * %s missing reg property\n",
> -				     cpu->full_name);
> -			return;
> +		prop = of_get_property(cpu, "reg", &len);
> +		if (!prop || len < (ac * sizeof(*prop))) {
> +				WARN(1, " * %s node missing/wrong reg"
> +					 " property, skipped\n",
> +					 cpu->full_name);
> +				goto next;
>  		}
> -
>  		/*
> -		 * 8 MSBs must be set to 0 in the DT since the reg property
> -		 * defines the MPIDR[23:0].
> +		 * Always read reg as u64 value.
> +		 * For dts with #address-cells == 1 hwid64[63:32]
> +		 * will be set to 0 by of_read_number.
> +		 * Toss away the top 32 bits and store value in hwid32.
> +		 */
> +		hwid32 = hwid64 = of_read_number(prop, ac);
> +		/*
> +		 * hwid64[63:24] must be always be 0 since the reg
> +		 * property defines the MPIDR[23:0] bits regardless
> +		 * of the cpus node #address-cells value.
>  		 */
> -		if (hwid & ~MPIDR_HWID_BITMASK)
> -			return;
> +		if (hwid64 & ~MPIDR_HWID_BITMASK) {
> +			WARN(1, " * %s node reg[63:24] must be 0 on"
> +			       " 32-bit dts, got %#016llx, skipped\n",
> +			       cpu->full_name, hwid64);
> +			goto next;
> +		}
>  
>  		/*
>  		 * Duplicate MPIDRs are a recipe for disaster.
>  		 * Scan all initialized entries and check for
> -		 * duplicates. If any is found just bail out.
> +		 * duplicates. If any is found just ignore the CPU.
>  		 * temp values were initialized to UINT_MAX
>  		 * to avoid matching valid MPIDR[23:0] values.
>  		 */
>  		for (j = 0; j < cpuidx; j++)
> -			if (WARN(tmp_map[j] == hwid, "Duplicate /cpu reg "
> -						     "properties in the DT\n"))
> -				return;
> +			if (WARN(tmp_map[j] == hwid32, " * %s node "
> +						"duplicate cpu reg "
> +						"property, skipped\n",
> +						cpu->full_name))
> +				goto next;
>  
>  		/*
>  		 * Build a stashed array of MPIDR values. Numbering scheme
> @@ -133,26 +154,29 @@ void __init arm_dt_init_cpu_maps(void)
>  		 * logical map built from DT is validated and can be used
>  		 * to override the map created in smp_setup_processor_id().
>  		 */
> -		if (hwid == mpidr) {
> +		if (hwid32 == mpidr) {
>  			i = 0;
>  			bootcpu_valid = true;
>  		} else {
>  			i = cpuidx++;
>  		}
>  
> -		if (WARN(cpuidx > nr_cpu_ids, "DT /cpu %u nodes greater than "
> -					       "max cores %u, capping them\n",
> +		if (WARN_ONCE(cpuidx > nr_cpu_ids, "DT cpu %u nodes greater"
> +						   " than max cores %u,"
> +						   " capping them\n",
>  					       cpuidx, nr_cpu_ids)) {
>  			cpuidx = nr_cpu_ids;
> -			break;
> +			goto next;
>  		}
>  
> -		tmp_map[i] = hwid;
> +		tmp_map[i] = hwid32;
> +next:	;
>  	}
> -
> -	if (WARN(!bootcpu_valid, "DT missing boot CPU MPIDR[23:0], "
> -				 "fall back to default cpu_logical_map\n"))
> -		return;
> +	/*
> +	 * A DT missing the boot CPU MPIDR is a really bad omen
> +	 * Flag it up as such and force firmware/dtb updates
> +	 */
> +	BUG_ON(!bootcpu_valid);
>  
>  	/*
>  	 * Since the boot CPU node contains proper data, and all nodes have
> -- 
> 1.7.12
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Lorenzo Pieralisi]

On Tue, Apr 16, 2013 at 03:41:46AM +0100, Simon Horman wrote:
> On Mon, Apr 15, 2013 at 05:13:22PM +0100, Lorenzo Pieralisi wrote:
> > In order to extend the current cpu nodes bindings to newer CPUs
> > inclusive of AArch64 and to update support for older ARM CPUs this
> > patch updates device tree documentation for the cpu nodes bindings.
> >
> > Main changes:
> >
> > - adds 64-bit bindings (inclusive of cpus node #address-cells updates)
> > - defines behaviour on pre and post v7 uniprocessor systems
> > - adds ARM 11MPcore specific reg property definition
> >
> > DT cpu map parsing code must be made compliant with the latest bindings
> > updates, hence this patch also updates the arm_dt_init_cpu_maps() function
> > with checks and additional parsing rules.
> >
> > Uniprocessor systems predating v7 do not parse the cpus node at all
> > since the reg property is meaningless on those systems.
> >
> > Device trees for 64-bit systems can be taken as device tree input also
> > for 64-bit CPUs running in 32-bit mode. The code checks that the reg entries
> > are zeroed as required in the respective fields and detects automatically
> > the cpus node #address-cells value so that device tree written for
> > 64-bit ARM platforms (cpus #address-cells == 2) can still be taken as
> > input. The correct device tree entries are to be set up by the boot
> > loader, kernel code just checks that device tree entries in the cpus
> > node are as expected for a 32-bit CPU (reg[63:24] == 0).
> >
> > cpu node entries with invalid reg property or containing duplicates are
> > ignored and the device tree parsing is not stopped anymore when such
> > entries are encountered, the device tree cpu entry is just skipped.
> >
> > A device tree with cpu nodes missing the boot CPU MPIDR is considered a
> > hard error and the kernel flags this up as a bug to force firmware updates.
> >
> > The updates to cpus/cpu node bindings for ARM require all in-kernel dts files
> > present in the mainline to be made compliant to the latest specification.
> >
> > This patch also updates ARM in-kernel non-compliant dts files to the latest
> > cpus/cpu nodes bindings specification.
> >
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > ---
> >  Documentation/devicetree/bindings/arm/cpus.txt | 335 +++++++++++++++++++++----
> >  arch/arm/boot/dts/exynos5440.dtsi              |   7 +
> >  arch/arm/boot/dts/omap3.dtsi                   |   4 +
> >  arch/arm/boot/dts/omap4.dtsi                   |   5 +
> >  arch/arm/boot/dts/omap5.dtsi                   |   5 +
> >  arch/arm/boot/dts/r8a7740.dtsi                 |   3 +
> >  arch/arm/boot/dts/sh7372.dtsi                  |   4 +
> 
> I would prefer if the SoC updates, in particular the sh7372 portion,
> was a separate patch.

Ok, I will split the patch accordingly.

Thanks,
Lorenzo

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Nicolas Ferre]

On 04/15/2013 06:13 PM, Lorenzo Pieralisi :
> In order to extend the current cpu nodes bindings to newer CPUs
> inclusive of AArch64 and to update support for older ARM CPUs this
> patch updates device tree documentation for the cpu nodes bindings.
> 
> Main changes:
> 
> - adds 64-bit bindings (inclusive of cpus node #address-cells updates)
> - defines behaviour on pre and post v7 uniprocessor systems
> - adds ARM 11MPcore specific reg property definition
> 
> DT cpu map parsing code must be made compliant with the latest bindings
> updates, hence this patch also updates the arm_dt_init_cpu_maps() function
> with checks and additional parsing rules.
> 
> Uniprocessor systems predating v7 do not parse the cpus node at all
> since the reg property is meaningless on those systems.
> 
> Device trees for 64-bit systems can be taken as device tree input also
> for 64-bit CPUs running in 32-bit mode. The code checks that the reg entries
> are zeroed as required in the respective fields and detects automatically
> the cpus node #address-cells value so that device tree written for
> 64-bit ARM platforms (cpus #address-cells == 2) can still be taken as
> input. The correct device tree entries are to be set up by the boot
> loader, kernel code just checks that device tree entries in the cpus
> node are as expected for a 32-bit CPU (reg[63:24] == 0).
> 
> cpu node entries with invalid reg property or containing duplicates are
> ignored and the device tree parsing is not stopped anymore when such
> entries are encountered, the device tree cpu entry is just skipped.
> 
> A device tree with cpu nodes missing the boot CPU MPIDR is considered a
> hard error and the kernel flags this up as a bug to force firmware updates.
> 
> The updates to cpus/cpu node bindings for ARM require all in-kernel dts files
> present in the mainline to be made compliant to the latest specification.
> 
> This patch also updates ARM in-kernel non-compliant dts files to the latest
> cpus/cpu nodes bindings specification.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> ---
>  Documentation/devicetree/bindings/arm/cpus.txt | 335 +++++++++++++++++++++----
>  arch/arm/boot/dts/exynos5440.dtsi              |   7 +
>  arch/arm/boot/dts/omap3.dtsi                   |   4 +
>  arch/arm/boot/dts/omap4.dtsi                   |   5 +
>  arch/arm/boot/dts/omap5.dtsi                   |   5 +
>  arch/arm/boot/dts/r8a7740.dtsi                 |   3 +
>  arch/arm/boot/dts/sh7372.dtsi                  |   4 +
>  arch/arm/boot/dts/sunxi.dtsi                   |   4 +

Hi,

If it can be done easily, can you please consider the upcoming (3.10)
cortex-a5 SoC:

arch/arm/boot/dts/sama5d3.dtsi

It is in linux-next through arm-soc/at91/at91/soc


>  arch/arm/kernel/devtree.c                      |  82 +++---
>  9 files changed, 377 insertions(+), 72 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> index f32494d..b1ef3d8 100644
> --- a/Documentation/devicetree/bindings/arm/cpus.txt
> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> @@ -1,4 +1,6 @@
> -* ARM CPUs binding description
> +=============================
> +ARM CPUs bindings 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")
> @@ -8,70 +10,317 @@ 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:
> -
> -- device_type:	must be "cpu"
> -- reg:		property matching the CPU MPIDR[23:0] register bits
> -		reg[31:24] bits must be set to 0
> -- compatible:	should be one of:
> -		"arm,arm1020"
> -		"arm,arm1020e"
> -		"arm,arm1022"
> -		"arm,arm1026"
> -		"arm,arm720"
> -		"arm,arm740"
> -		"arm,arm7tdmi"
> -		"arm,arm920"

Here we have: "arm,arm920t" for at91rm9200.dtsi

> -		"arm,arm922"
> -		"arm,arm925"
> -		"arm,arm926"

Oh, I missed this one: the strange thing is that *all* arm926 variants
have a compatible string of this form: "arm,arm926ejs"

(using: git grep "arm,arm9" next/master)

Should we change? Is there a rule for this naming?

> -		"arm,arm940"
> -		"arm,arm946"
> -		"arm,arm9tdmi"
> -		"arm,cortex-a5"
> -		"arm,cortex-a7"
> -		"arm,cortex-a8"
> -		"arm,cortex-a9"
> -		"arm,cortex-a15"
> -		"arm,arm1136"
> -		"arm,arm1156"
> -		"arm,arm1176"
> -		"arm,arm11mpcore"
> -		"faraday,fa526"
> -		"intel,sa110"
> -		"intel,sa1100"
> -		"marvell,feroceon"
> -		"marvell,mohawk"
> -		"marvell,xsc3"
> -		"marvell,xscale"

[..]

Thanks, best regards,
-- 
Nicolas Ferre

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Lorenzo Pieralisi]

On Wed, Apr 17, 2013 at 10:35:24AM +0100, Nicolas Ferre wrote:
> On 04/15/2013 06:13 PM, Lorenzo Pieralisi :
> > In order to extend the current cpu nodes bindings to newer CPUs
> > inclusive of AArch64 and to update support for older ARM CPUs this
> > patch updates device tree documentation for the cpu nodes bindings.
> > 
> > Main changes:
> > 
> > - adds 64-bit bindings (inclusive of cpus node #address-cells updates)
> > - defines behaviour on pre and post v7 uniprocessor systems
> > - adds ARM 11MPcore specific reg property definition
> > 
> > DT cpu map parsing code must be made compliant with the latest bindings
> > updates, hence this patch also updates the arm_dt_init_cpu_maps() function
> > with checks and additional parsing rules.
> > 
> > Uniprocessor systems predating v7 do not parse the cpus node at all
> > since the reg property is meaningless on those systems.
> > 
> > Device trees for 64-bit systems can be taken as device tree input also
> > for 64-bit CPUs running in 32-bit mode. The code checks that the reg entries
> > are zeroed as required in the respective fields and detects automatically
> > the cpus node #address-cells value so that device tree written for
> > 64-bit ARM platforms (cpus #address-cells == 2) can still be taken as
> > input. The correct device tree entries are to be set up by the boot
> > loader, kernel code just checks that device tree entries in the cpus
> > node are as expected for a 32-bit CPU (reg[63:24] == 0).
> > 
> > cpu node entries with invalid reg property or containing duplicates are
> > ignored and the device tree parsing is not stopped anymore when such
> > entries are encountered, the device tree cpu entry is just skipped.
> > 
> > A device tree with cpu nodes missing the boot CPU MPIDR is considered a
> > hard error and the kernel flags this up as a bug to force firmware updates.
> > 
> > The updates to cpus/cpu node bindings for ARM require all in-kernel dts files
> > present in the mainline to be made compliant to the latest specification.
> > 
> > This patch also updates ARM in-kernel non-compliant dts files to the latest
> > cpus/cpu nodes bindings specification.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > ---
> >  Documentation/devicetree/bindings/arm/cpus.txt | 335 +++++++++++++++++++++----
> >  arch/arm/boot/dts/exynos5440.dtsi              |   7 +
> >  arch/arm/boot/dts/omap3.dtsi                   |   4 +
> >  arch/arm/boot/dts/omap4.dtsi                   |   5 +
> >  arch/arm/boot/dts/omap5.dtsi                   |   5 +
> >  arch/arm/boot/dts/r8a7740.dtsi                 |   3 +
> >  arch/arm/boot/dts/sh7372.dtsi                  |   4 +
> >  arch/arm/boot/dts/sunxi.dtsi                   |   4 +
> 
> Hi,
> 
> If it can be done easily, can you please consider the upcoming (3.10)
> cortex-a5 SoC:
> 
> arch/arm/boot/dts/sama5d3.dtsi
> 
> It is in linux-next through arm-soc/at91/at91/soc

I will do that at the same time this set gets merged.

> >  arch/arm/kernel/devtree.c                      |  82 +++---
> >  9 files changed, 377 insertions(+), 72 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> > index f32494d..b1ef3d8 100644
> > --- a/Documentation/devicetree/bindings/arm/cpus.txt
> > +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> > @@ -1,4 +1,6 @@
> > -* ARM CPUs binding description
> > +=============================
> > +ARM CPUs bindings 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")
> > @@ -8,70 +10,317 @@ 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:
> > -
> > -- device_type:	must be "cpu"
> > -- reg:		property matching the CPU MPIDR[23:0] register bits
> > -		reg[31:24] bits must be set to 0
> > -- compatible:	should be one of:
> > -		"arm,arm1020"
> > -		"arm,arm1020e"
> > -		"arm,arm1022"
> > -		"arm,arm1026"
> > -		"arm,arm720"
> > -		"arm,arm740"
> > -		"arm,arm7tdmi"
> > -		"arm,arm920"
> 
> Here we have: "arm,arm920t" for at91rm9200.dtsi

I will update it.

> > -		"arm,arm922"
> > -		"arm,arm925"
> > -		"arm,arm926"
> 
> Oh, I missed this one: the strange thing is that *all* arm926 variants
> have a compatible string of this form: "arm,arm926ejs"
> 
> (using: git grep "arm,arm9" next/master)
> 
> Should we change? Is there a rule for this naming?

We decided to keep the compatible property as the base processor
names, leaving out variants, so I think answer is: the rule for the
naming is this document, and yes we should change it, if we all agree
on the approach.

Lorenzo

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Arnd Bergmann]

On Monday 15 April 2013, Lorenzo Pieralisi wrote:
> -               "arm,cortex-a9"
> -               "arm,cortex-a15"
> -               "arm,arm1136"
> -               "arm,arm1156"
> -               "arm,arm1176"
> -               "arm,arm11mpcore"
> -               "faraday,fa526"
> -               "intel,sa110"
> -               "intel,sa1100"
> -               "marvell,feroceon"
> -               "marvell,mohawk"
> -               "marvell,xsc3"
> -               "marvell,xscale"

> +                           "arm,cortex-a9"
> +                           "arm,cortex-a15"
> +                           "arm,cortex-a57"
> +                           "arm,arm1136"
> +                           "arm,arm1156"
> +                           "arm,arm1176"
> +                           "arm,arm11mpcore"
> +                           "faraday,fa526"
> +                           "intel,sa110"
> +                           "intel,sa1100"
> +                           "marvell,feroceon"
> +                           "marvell,mohawk"
> +                           "marvell,xsc3"
> +                           "marvell,xscale"

I see you are adding cortex-a57 here, but there are actually a couple
of other cores that we could add here. It also makes sense to sort these
alphanumerically I think, and move arm10 after arm9, as well as arm11
before cortex.

The ones that I find missing but actually used are:

	"arm,cortex-a53"
	"arm,cortex-m0"
	"arm,cortex-m0+"
	"arm,cortex-m1"
	"arm,cortex-m3"
	"arm,cortex-m4"
	"arm,cortex-r4"
	"arm,cortex-r5"
	"arm,cortex-r7"
	"marvell,pj4"
	"qcom,scorpion"
	"qcom,krait"

There are also a number of faraday cores that we don't support in
Linux (fa626, fa626te, fa726te, ...), not sure if we should attempt to
list them all here. The only one that is being worked on at the moment
is based on fa526.

There are also a number of 64 bit cores that other companies have announced,
but I'm not sure if the names are final yet.

	Arnd

Re: [RFC PATCH 2/2] ARM: DT: kernel: DT cpu node bindings update

[Lorenzo Pieralisi]

On Wed, Apr 17, 2013 at 10:48:56AM +0100, Arnd Bergmann wrote:
> On Monday 15 April 2013, Lorenzo Pieralisi wrote:
> > -               "arm,cortex-a9"
> > -               "arm,cortex-a15"
> > -               "arm,arm1136"
> > -               "arm,arm1156"
> > -               "arm,arm1176"
> > -               "arm,arm11mpcore"
> > -               "faraday,fa526"
> > -               "intel,sa110"
> > -               "intel,sa1100"
> > -               "marvell,feroceon"
> > -               "marvell,mohawk"
> > -               "marvell,xsc3"
> > -               "marvell,xscale"
> 
> > +                           "arm,cortex-a9"
> > +                           "arm,cortex-a15"
> > +                           "arm,cortex-a57"
> > +                           "arm,arm1136"
> > +                           "arm,arm1156"
> > +                           "arm,arm1176"
> > +                           "arm,arm11mpcore"
> > +                           "faraday,fa526"
> > +                           "intel,sa110"
> > +                           "intel,sa1100"
> > +                           "marvell,feroceon"
> > +                           "marvell,mohawk"
> > +                           "marvell,xsc3"
> > +                           "marvell,xscale"
> 
> I see you are adding cortex-a57 here, but there are actually a couple
> of other cores that we could add here. It also makes sense to sort these
> alphanumerically I think, and move arm10 after arm9, as well as arm11
> before cortex.

Ok, I will do that.

> The ones that I find missing but actually used are:
> 
> 	"arm,cortex-a53"
> 	"arm,cortex-m0"
> 	"arm,cortex-m0+"
> 	"arm,cortex-m1"
> 	"arm,cortex-m3"
> 	"arm,cortex-m4"
> 	"arm,cortex-r4"
> 	"arm,cortex-r5"
> 	"arm,cortex-r7"
> 	"marvell,pj4"
> 	"qcom,scorpion"
> 	"qcom,krait"

Ok, I will add them, but we should probably enforce a policy regulating
how this binding should be updated in the future.

> 
> There are also a number of faraday cores that we don't support in
> Linux (fa626, fa626te, fa726te, ...), not sure if we should attempt to
> list them all here. The only one that is being worked on at the moment
> is based on fa526.

That's why it is the only one in the list, not sure what we should do either.

> There are also a number of 64 bit cores that other companies have announced,
> but I'm not sure if the names are final yet.

On a side note, as far as the in-kernel dts updates are concerned, how would
you like me to split them ? per-mach ? per-SoC ?

Thank you very much,
Lorenzo