Skip to contentSkip to conversation

ARM: realview: basic device tree implementation

PATCH Superseded Linus Walleij 7 participants 26 messages May 8, 2014 – May 29, 2014

Subsystems (6)

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

This implements basic device tree boot support for the RealView platforms, with a basic device tree for ARM PB1176 as an example.

The implementation is done with a new DT-specific board file using only pre-existing bindings for the basic IRQ, timer and serial port drivers. A new compatible type is added to the GIC for the ARM1176.

This implementation uses the MFD syscon handle from day one to access the system controller registers, and register the devices using the SoC bus.

Cc Arnd Bergmann <[email protected]>
Cc Rob Herring <[email protected]>

---
ChangeLog v1->v2:
- Adjust timer clock names to be the same as the example in the device tree binding.
- Remove all memory fixup code - this should be handled by the device tree specification of memory areas or by special MM hacks for the RealView PBX.
- Fix the documentation around syscon to specify that it can be in any node, need not be the root node.
- Switch device tree license to the BSD license, taken from arch/powerpc/boot/dts/p1024rdb_32b.dts
- Add a hunk for the new compatible string to Documentation/devicetree/bindings/arm/gic.txt
- Move the clocks out of the SoC node, certainly the xtal is not sitting on the SoC...
- Sort the selects in Kconfig alphabetically
- Use IS_ENABLED() for the l2x0 code snippet
- Instead of checking the board variant in the reset routine to figure out how to tweak the reset controller, have a compatible string for each syscon variant, map it to an enum that provides a unique type ID and that way figure out how to handle it in a maybe more elegant way.
- Open issue: what do to with the l2x0 stuff?

---
 Documentation/devicetree/bindings/arm/arm-boards |  57 ++++++
 Documentation/devicetree/bindings/arm/gic.txt    |   1 +
 arch/arm/boot/dts/Makefile                       |   1 +
 arch/arm/boot/dts/arm-realview-pb1176.dts        | 177 ++++++++++++++++++
 arch/arm/mach-realview/Kconfig                   |  10 +
 arch/arm/mach-realview/Makefile                  |   1 +
 arch/arm/mach-realview/realview-dt.c             | 225 +++++++++++++++++++++++
 arch/arm/mm/Kconfig                              |   2 +-
 drivers/irqchip/irq-gic.c                        |   1 +
 9 files changed, 474 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/arm-realview-pb1176.dts
 create mode 100644 arch/arm/mach-realview/realview-dt.c
Documentation/devicetree/bindings/arm/arm-boards +57 -0
index 3509707f9320..86b36f399d1c 100644
@@ -86,3 +86,60 @@ Interrupt controllers:
 	compatible = "arm,versatile-sic";
 	interrupt-controller;
 	#interrupt-cells = <1>;
+
+
+ARM RealView Boards
+-------------------
+The RealView boards cover tailored evaluation boards that are used to explore
+the ARM11 and Cortex A-8 and Cortex A-9 processors.
+
+Required properties (in root node):
+	/* RealView Emulation Baseboard */
+	compatible = "arm,realview-eb";
+	 /* RealView Platform Baseboard for ARM1176JZF-S */
+	compatible = "arm,realview-pb1176";
+	/* RealView Platform Baseboard for ARM11 MPCore */
+	compatible = "arm,realview-pb11mp";
+	/* RealView Platform Baseboard for Cortex A-8 */
+	compatible = "arm,realview-pba8";
+	/* RealView Platform Baseboard Explore for Cortex A-9 */
+	compatible = "arm,realview-pbx";
+
+Required nodes:
+
+- syscon: some node of the RealView platforms must have a
+  system controller node pointing to the control registers,
+  with the compatible string set to one of these tuples:
+   "arm,realview-eb-syscon", "syscon"
+   "arm,realview-pb1176-syscon", "syscon"
+   "arm,realview-pb11mp-syscon", "syscon"
+   "arm,realview-pba8-syscon", "syscon"
+   "arm,realview-pbx-syscon", "syscon"
+
+  Required properties for the system controller:
+  - regs: the location and size of the system controller registers,
+    one range of 0x1000 bytes.
+
+Example:
+
+/dts-v1/;
+#include <dt-bindings/interrupt-controller/irq.h>
+#include "skeleton.dtsi"
+
+/ {
+	model = "ARM RealView PB1176 with device tree";
+	compatible = "arm,realview-pb1176";
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		ranges;
+
+		syscon: syscon at 10000000 {
+			compatible = "arm,realview-syscon", "syscon";
+			reg = <0x10000000 0x1000>;
+		};
+
+	};
+};
Documentation/devicetree/bindings/arm/gic.txt +1 -0
index 5573c08d3180..539420194efd 100644
@@ -16,6 +16,7 @@ Main node required properties:
 	"arm,cortex-a9-gic"
 	"arm,cortex-a7-gic"
 	"arm,arm11mp-gic"
+	"arm,arm1176jzf-gic"
 - interrupt-controller : Identifies the node as an interrupt controller
 - #interrupt-cells : Specifies the number of cells needed to encode an
   interrupt source.  The type shall be a <u32> and the value shall be 3.
arch/arm/boot/dts/Makefile +1 -0
index 377b7c364033..1419a70026da 100644
@@ -294,6 +294,7 @@ dtb-$(CONFIG_ARCH_PRIMA2) += prima2-evb.dtb
 dtb-$(CONFIG_ARCH_QCOM) += qcom-msm8660-surf.dtb \
 	qcom-msm8960-cdp.dtb \
 	qcom-apq8074-dragonboard.dtb
+dtb-$(CONFIG_ARCH_REALVIEW) += arm-realview-pb1176.dtb
 dtb-$(CONFIG_ARCH_S3C24XX) += s3c2416-smdk2416.dtb
 dtb-$(CONFIG_ARCH_S3C64XX) += s3c6410-mini6410.dtb \
 	s3c6410-smdk6410.dtb
arch/arm/boot/dts/arm-realview-pb1176.dts new file +177 -0

177 lines not on this page · raw message

arch/arm/mach-realview/Kconfig +10 -0
index 9db2029aa632..65a07f5b5225 100644
@@ -1,6 +1,16 @@
 menu "RealView platform type"
 	depends on ARCH_REALVIEW
 
+config REALVIEW_DT
+	bool "Support RealView(R) Device Tree based boot"
+	select ARM_GIC
+	select MFD_SYSCON
+	select SOC_BUS
+	select USE_OF
+	help
+	  Include support for booting the ARM(R) RealView(R) evaluation
+	  boards using a device tree machine description.
+
 config MACH_REALVIEW_EB
 	bool "Support RealView(R) Emulation Baseboard"
 	select ARM_GIC
arch/arm/mach-realview/Makefile +1 -0
index 541fa4c109ef..e07fdf7ae8a7 100644
@@ -3,6 +3,7 @@
 #
 
 obj-y					:= core.o
+obj-$(CONFIG_REALVIEW_DT)		+= realview-dt.o
 obj-$(CONFIG_MACH_REALVIEW_EB)		+= realview_eb.o
 obj-$(CONFIG_MACH_REALVIEW_PB11MP)	+= realview_pb11mp.o
 obj-$(CONFIG_MACH_REALVIEW_PB1176)	+= realview_pb1176.o
arch/arm/mach-realview/realview-dt.c new file +225 -0

225 lines not on this page · raw message

arch/arm/mm/Kconfig +1 -1
index 5bf7c3c3b301..5461c4401ed6 100644
@@ -928,7 +928,7 @@ config ARM_L1_CACHE_SHIFT
 config ARM_DMA_MEM_BUFFERABLE
 	bool "Use non-cacheable memory for DMA" if (CPU_V6 || CPU_V6K) && !CPU_V7
 	depends on !(MACH_REALVIEW_PB1176 || REALVIEW_EB_ARM11MP || \
-		     MACH_REALVIEW_PB11MP)
+		     MACH_REALVIEW_PB11MP || REALVIEW_DT)
 	default y if CPU_V6 || CPU_V6K || CPU_V7
 	help
 	  Historically, the kernel has used strongly ordered mappings to
drivers/irqchip/irq-gic.c +1 -0
index 57d165e026f4..70281b38c73b 100644
@@ -1073,6 +1073,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
 }
 IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
 IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
+IRQCHIP_DECLARE(arm1176jzf_gic, "arm,arm1176jzf-gic", gic_of_init);
 IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
 IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
 
Signature · 1 line
1.9.0
maintainer
#1 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

arch/arm/boot/dts/arm-realview-pb1176.dts

Quoted 5 lines · Linus Walleij
arch/arm/boot/dts/arm-realview-pb1176.dts new file +0 -0
new file mode 100644
index 000000000000..0eea0f4a7b39

Can you split this up into an arm-realview.dtsi file for the common parts and a pb1176 specific file for the rest?

arch/arm/boot/dts/arm-realview-pb1176.dts line 83

Quoted 10 lines
arch/arm/boot/dts/arm-realview-pb1176.dts new file +10 -0
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		ranges;
+
+		syscon: syscon at 10000000 {
+			compatible = "arm,realview-pb1176-syscon", "syscon";
+			reg = <0x10000000 0x1000>;
+		};

Hmm, in order to have a proper reset driver, we probably want a separate node for the reset controller. I believe the x-gene people have just posted a generic reset driver based on syscon. Let's see if we can share that.

arch/arm/boot/dts/arm-realview-pb1176.dts line 148

Quoted 8 lines
arch/arm/boot/dts/arm-realview-pb1176.dts new file +8 -0
+		pb1176_serial0: uart at 1010c000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x1010c000 0x1000>;
+			interrupt-parent = <&intc_dc1176>;
+			interrupts = <0 18 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&uartclk>, <&pclk>;
+			clock-names = "uartclk", "apb_pclk";
+		};

I believe the "official" name for a uart is serial at 1010c000, not uart at 1010c000. I know we are inconsistent with that.

Quoted 4 lines
arch/arm/boot/dts/arm-realview-pb1176.dts new file +4 -0
+
+/* Pointer to the system controller */
+struct regmap *syscon_regmap;
+u8 syscon_type;

arch/arm/boot/dts/arm-realview-pb1176.dts

Quoted 10 lines
arch/arm/boot/dts/arm-realview-pb1176.dts new file +10 -0
+
+static struct map_desc realview_dt_io_desc[] __initdata = {
+	{
+		/* FIXME: static map needed for LED driver */
+		.virtual	= IO_ADDRESS(REALVIEW_SYS_BASE),
+		.pfn		= __phys_to_pfn(REALVIEW_SYS_BASE),
+		.length		= SZ_4K,
+		.type		= MT_DEVICE,
+	},
+};

I've looked at the driver and I don't see why this is required. The driver does a devm_ioremap_resource(), which should work with or without a static mapping.

Quoted 23 lines
arch/arm/boot/dts/arm-realview-pb1176.dts new file +23 -0
+/*
+ * We detect the different syscon types from the compatible strings.
+ */
+enum realview_syscon {
+	REALVIEW_SYSCON_EB,
+	REALVIEW_SYSCON_PB1176,
+	REALVIEW_SYSCON_PB11MP,
+	REALVIEW_SYSCON_PBA8,
+	REALVIEW_SYSCON_PBX,
+};
+
+static const struct of_device_id realview_syscon_match[] = {
+	{
+		.compatible = "arm,realview-eb-syscon",
+		.data = (void *)REALVIEW_SYSCON_EB,
+	},
+	{
+		.compatible = "arm,realview-pb1176-syscon",
+		.data = (void *)REALVIEW_SYSCON_PB1176,
+	},
+	{
+		.compatible = "arm,realview-pb11mp-syscon",
+		.data = (void *)REALVIEW_SYSCON_PB11MP,

arch/arm/boot/dts/arm-realview-pb1176.dts

Quoted 10 lines
arch/arm/boot/dts/arm-realview-pb1176.dts new file +10 -0
+	},
+	{
+		.compatible = "arm,realview-pba8-syscon",
+		.data = (void *)REALVIEW_SYSCON_PBA8,
+	},
+	{
+		.compatible = "arm,realview-pbx-syscon",
+		.data = (void *)REALVIEW_SYSCON_PBX,
+	},
+};

If not the generic syscon reset driver, it should at least live in drivers/power/reset/ rather than here.

Quoted 18 lines
arch/arm/boot/dts/arm-realview-pb1176.dts new file +18 -0
+#if IS_ENABLED(CONFIG_CACHE_L2X0)
+	if (of_machine_is_compatible("arm,realview-eb"))
+		/*
+		 * 1MB (128KB/way), 8-way associativity,
+		 * evmon/parity/share enabled
+		 * Bits:  .... ...0 0111 1001 0000 .... .... ....
+		 */
+		l2x0_of_init(0x00790000, 0xfe000fff);
+	else if (of_machine_is_compatible("arm,realview-pb1176"))
+		/*
+		 * 128Kb (16Kb/way) 8-way associativity.
+		 * evmon/parity/share enabled.
+		 */
+		l2x0_of_init(0x00730000, 0xfe000fff);
+	else if (of_machine_is_compatible("arm,realview-pb11mp"))
+		/*
+		 * 1MB (128KB/way), 8-way associativity,
+		 * evmon/parity/share enabled

arch/arm/boot/dts/arm-realview-pb1176.dts

Quoted 10 lines
arch/arm/boot/dts/arm-realview-pb1176.dts new file +10 -0
+		 * Bits:  .... ...0 0111 1001 0000 .... .... ....
+		 */
+		l2x0_of_init(0x00730000, 0xfe000fff);
+	else if (of_machine_is_compatible("arm,realview-pbx"))
+		/*
+		 * 16KB way size, 8-way associativity, parity disabled
+		 * Bits:  .. 0 0 0 0 1 00 1 0 1 001 0 000 0 .... .... ....
+		 */
+		l2x0_of_init(0x02520000, 0xc0000fff);
+#endif

You wrote that you added the #if IS_ENABLED() here. Why?

l2x0_of_init is already a nop if CONFIG_CACHE_L2X0 is not set. If you want to skip a bit of dead code here, you could make it an inline function and do

	if (IS_ENABLED(CONFIG_CACHE_L2X0))
		realview_setup_l2x0();

Other than that, I still think we should avoid doing anything platform specific here at all, and move it all into the l2x0_of_init function, based on Russell's l2x0 patch series.

Quoted 13 lines
arch/arm/boot/dts/arm-realview-pb1176.dts new file +13 -0
+	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
+	if (!soc_dev_attr)
+		return;
+	ret = of_property_read_string(root, "compatible",
+				      &soc_dev_attr->soc_id);
+	if (ret)
+		return;
+	ret = of_property_read_string(root, "model", &soc_dev_attr->machine);
+        if (ret)
+		return;
+	soc_dev_attr->family = "RealView";
+	soc_dev = soc_device_register(soc_dev_attr);
+	if (IS_ERR(soc_dev)) {

arch/arm/boot/dts/arm-realview-pb1176.dts

Quoted 10 lines
arch/arm/boot/dts/arm-realview-pb1176.dts new file +10 -0
+		kfree(soc_dev_attr);
+		return;
+	}
+	parent = soc_device_to_device(soc_dev);
+	ret = of_platform_populate(root, of_default_bus_match_table,
+				   NULL, parent);
+	if (ret) {
+		pr_crit("could not populate device tree\n");
+		return;
+	}

I wonder if there is a way to do this without having code in the platform. I would hate it if we get to the point where each platform is completely empty but still requires a copy of this code.

We just faced the same discussion on the exynos chip id patches.

I also noticed that you pass the DT root here, which isn't actually the intention of the soc device interface: you should pass the DT node that has the on-chip devices but not the board-level (top-level) nodes such as your fixed clocks.

arch/arm/mm/Kconfig config ARM_L1_CACHE_SHIFT · line 934

Quoted 13 lines
arch/arm/mm/Kconfig +1 -1
index 5bf7c3c3b301..5461c4401ed6 100644
@@ -928,7 +928,7 @@ config ARM_L1_CACHE_SHIFT
 config ARM_DMA_MEM_BUFFERABLE
 	bool "Use non-cacheable memory for DMA" if (CPU_V6 || CPU_V6K) && !CPU_V7
 	depends on !(MACH_REALVIEW_PB1176 || REALVIEW_EB_ARM11MP || \
-		     MACH_REALVIEW_PB11MP)
+		     MACH_REALVIEW_PB11MP || REALVIEW_DT)
 	default y if CPU_V6 || CPU_V6K || CPU_V7
 	help
 	  Historically, the kernel has used strongly ordered mappings to

Do you have an explanation for why this is needed? I don't remember seeing this before, but I guess it gets in the way of multiplatform support. Is this just a performance optimization on realview, or is it actually required?

Arnd

#2 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 4 lines · Arnd Bergmann
Quoted 1 line

+++ b/arch/arm/boot/dts/arm-realview-pb1176.dts

Can you split this up into an arm-realview.dtsi file for the common parts and a pb1176 specific file for the rest?

I wonder if that makes any kind of sense. The RealView platforms does not really share much more than the name, sadly. For example: IP blocks aren't even at the same address.

I could create a .dtsi file but it would be empty :-(

Quoted 11 lines

arch/arm/boot/dts/arm-realview-pb1176.dts line 83

Quoted 10 lines
arch/arm/boot/dts/arm-realview-pb1176.dts new file +10 -0
+     soc {
+             #address-cells = <1>;
+             #size-cells = <1>;
+             compatible = "simple-bus";
+             ranges;
+
+             syscon: syscon at 10000000 {
+                     compatible = "arm,realview-pb1176-syscon", "syscon";
+                     reg = <0x10000000 0x1000>;
+             };
Quoted 4 lines

Hmm, in order to have a proper reset driver, we probably want a separate node for the reset controller. I believe the x-gene people have just posted a generic reset driver based on syscon. Let's see if we can share that.

I have looked at it. Patch titled
"power: reset: Add generic SYSCON register mapped reset"

Sadly it does not work for our case. This is because it assumes that reboot will be triggered by writing one value to one register. The RealViews need you to *first* write to a special unlock register, then write a magic value to a magic register.

So of course we could extend the syscon regmap reset driver by starting to encode a jam table such as a sequence of register writes to a sequence of registers, but we have refused such code in the past because as I recall Grant said, it is the beginning of a byte code interpreter and then we could as well bite the bullet and start implementing open firmware.

This is exactly the type of thing that ACPI AML code usually does by the way so what he says makes a *lot* of sense.

So for now I keep to our special driver.

Quoted 11 lines

arch/arm/boot/dts/arm-realview-pb1176.dts line 148

Quoted 8 lines
arch/arm/boot/dts/arm-realview-pb1176.dts new file +8 -0
+             pb1176_serial0: uart at 1010c000 {
+                     compatible = "arm,pl011", "arm,primecell";
+                     reg = <0x1010c000 0x1000>;
+                     interrupt-parent = <&intc_dc1176>;
+                     interrupts = <0 18 IRQ_TYPE_LEVEL_HIGH>;
+                     clocks = <&uartclk>, <&pclk>;
+                     clock-names = "uartclk", "apb_pclk";
+             };

I believe the "official" name for a uart is serial at 1010c000, not uart at 1010c000. I know we are inconsistent with that.

OK fixed.

Quoted 9 lines
Quoted 9 lines
arch/arm/mach-realview/realview-dt.c new file +9 -0
+/* Pointer to the system controller */
+struct regmap *syscon_regmap;
+u8 syscon_type;
+
+static struct map_desc realview_dt_io_desc[] __initdata = {
+     {
+             /* FIXME: static map needed for LED driver */
+             .virtual        = IO_ADDRESS(REALVIEW_SYS_BASE),
+             .pfn            = __phys_to_pfn(REALVIEW_SYS_BASE),
Quoted 8 lines

arch/arm/mach-realview/realview-dt.c line 45

Quoted 4 lines
arch/arm/mach-realview/realview-dt.c new file +4 -0
+             .length         = SZ_4K,
+             .type           = MT_DEVICE,
+     },
+};

I've looked at the driver and I don't see why this is required. The driver does a devm_ioremap_resource(), which should work with or without a static mapping.

Well yeah, the version of the driver that is in linux-next does this. This was parallel work and I had no idea which would reach mainline first :-)

Anyway, I just deleted because I realized it was better if I came up with the concept of syscon-leds to handle this through the syscon.

Quoted 28 lines
Quoted 28 lines
arch/arm/mach-realview/realview-dt.c new file +28 -0
+/*
+ * We detect the different syscon types from the compatible strings.
+ */
+enum realview_syscon {
+     REALVIEW_SYSCON_EB,
+     REALVIEW_SYSCON_PB1176,
+     REALVIEW_SYSCON_PB11MP,
+     REALVIEW_SYSCON_PBA8,
+     REALVIEW_SYSCON_PBX,
+};
+
+static const struct of_device_id realview_syscon_match[] = {
+     {
+             .compatible = "arm,realview-eb-syscon",
+             .data = (void *)REALVIEW_SYSCON_EB,
+     },
+     {
+             .compatible = "arm,realview-pb1176-syscon",
+             .data = (void *)REALVIEW_SYSCON_PB1176,
+     },
+     {
+             .compatible = "arm,realview-pb11mp-syscon",
+             .data = (void *)REALVIEW_SYSCON_PB11MP,
+     },
+     {
+             .compatible = "arm,realview-pba8-syscon",
+             .data = (void *)REALVIEW_SYSCON_PBA8,
+     },
Quoted 8 lines

arch/arm/mach-realview/realview-dt.c line 84

Quoted 5 lines
arch/arm/mach-realview/realview-dt.c new file +5 -0
+     {
+             .compatible = "arm,realview-pbx-syscon",
+             .data = (void *)REALVIEW_SYSCON_PBX,
+     },
+};

If not the generic syscon reset driver, it should at least live in drivers/power/reset/ rather than here.

This we can fix.

Quoted 22 lines
Quoted 22 lines
arch/arm/mach-realview/realview-dt.c new file +22 -0
+#if IS_ENABLED(CONFIG_CACHE_L2X0)
+     if (of_machine_is_compatible("arm,realview-eb"))
+             /*
+              * 1MB (128KB/way), 8-way associativity,
+              * evmon/parity/share enabled
+              * Bits:  .... ...0 0111 1001 0000 .... .... ....
+              */
+             l2x0_of_init(0x00790000, 0xfe000fff);
+     else if (of_machine_is_compatible("arm,realview-pb1176"))
+             /*
+              * 128Kb (16Kb/way) 8-way associativity.
+              * evmon/parity/share enabled.
+              */
+             l2x0_of_init(0x00730000, 0xfe000fff);
+     else if (of_machine_is_compatible("arm,realview-pb11mp"))
+             /*
+              * 1MB (128KB/way), 8-way associativity,
+              * evmon/parity/share enabled
+              * Bits:  .... ...0 0111 1001 0000 .... .... ....
+              */
+             l2x0_of_init(0x00730000, 0xfe000fff);
+     else if (of_machine_is_compatible("arm,realview-pbx"))
Quoted 8 lines

arch/arm/mach-realview/realview-dt.c line 124

Quoted 6 lines
arch/arm/mach-realview/realview-dt.c new file +6 -0
+             /*
+              * 16KB way size, 8-way associativity, parity disabled
+              * Bits:  .. 0 0 0 0 1 00 1 0 1 001 0 000 0 .... .... ....
+              */
+             l2x0_of_init(0x02520000, 0xc0000fff);
+#endif

You wrote that you added the #if IS_ENABLED() here. Why?

Mainly because I had this other review comment that I should use IS_ENABLED() rather than #ifdef and it seemed better but I don't know anymore...

Quoted 9 lines

l2x0_of_init is already a nop if CONFIG_CACHE_L2X0 is not set. If you want to skip a bit of dead code here, you could make it an inline function and do

        if (IS_ENABLED(CONFIG_CACHE_L2X0))
                realview_setup_l2x0();

Other than that, I still think we should avoid doing anything platform specific here at all, and move it all into the l2x0_of_init function, based on Russell's l2x0 patch series.

I just need a base branch I guess, then I can build on top of Russell's patches, it's just a matter of dependencies.

Quoted 24 lines
Quoted 13 lines
arch/arm/mach-realview/realview-dt.c new file +13 -0
+     soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
+     if (!soc_dev_attr)
+             return;
+     ret = of_property_read_string(root, "compatible",
+                                   &soc_dev_attr->soc_id);
+     if (ret)
+             return;
+     ret = of_property_read_string(root, "model", &soc_dev_attr->machine);
+        if (ret)
+             return;
+     soc_dev_attr->family = "RealView";
+     soc_dev = soc_device_register(soc_dev_attr);
+     if (IS_ERR(soc_dev)) {

arch/arm/mach-realview/realview-dt.c line 158

Quoted 10 lines
arch/arm/mach-realview/realview-dt.c new file +10 -0
+             kfree(soc_dev_attr);
+             return;
+     }
+     parent = soc_device_to_device(soc_dev);
+     ret = of_platform_populate(root, of_default_bus_match_table,
+                                NULL, parent);
+     if (ret) {
+             pr_crit("could not populate device tree\n");
+             return;
+     }
Quoted 3 lines

I wonder if there is a way to do this without having code in the platform. I would hate it if we get to the point where each platform is completely empty but still requires a copy of this code.

The question can be rephrased like this:
Should we create drivers/soc?

The structure of the SoC bus device is maybe a bit irritating but it has been hammered into the device core so now we have to live with it as it is.

Quoted 1 line

We just faced the same discussion on the exynos chip id patches.

Yes, same answer :-)

Quoted 4 lines

I also noticed that you pass the DT root here, which isn't actually the intention of the soc device interface: you should pass the DT node that has the on-chip devices but not the board-level (top-level) nodes such as your fixed clocks.

Hm that's true. I introduced yet another set of compatible tuples like this:
compatible = "simple-bus", "arm-realview-pb1176-soc"

It will, just like the reset driver generate a new batch of identical compatible-strings and documenation sets, but it does have the upside of clean separation.

Quoted 10 lines
Quoted 10 lines
arch/arm/mm/Kconfig +1 -1
index 5bf7c3c3b301..5461c4401ed6 100644
@@ -928,7 +928,7 @@ config ARM_L1_CACHE_SHIFT
 config ARM_DMA_MEM_BUFFERABLE
      bool "Use non-cacheable memory for DMA" if (CPU_V6 || CPU_V6K) && !CPU_V7
      depends on !(MACH_REALVIEW_PB1176 || REALVIEW_EB_ARM11MP || \
-                  MACH_REALVIEW_PB11MP)
+                  MACH_REALVIEW_PB11MP || REALVIEW_DT)
Quoted 8 lines

arch/arm/mm/Kconfig config ARM_L1_CACHE_SHIFT · line 934

Quoted 3 lines
arch/arm/mm/Kconfig +0 -0
      default y if CPU_V6 || CPU_V6K || CPU_V7
      help
        Historically, the kernel has used strongly ordered mappings to

Do you have an explanation for why this is needed? I don't remember seeing this before, but I guess it gets in the way of multiplatform support. Is this just a performance optimization on realview, or is it actually required?

I don't know that actually. And as DMA is likely only used on PCI I have no way of testing it either without such hardware. But I guess I can just leave it out for the time being.

Yours,
Linus Walleij

maintainer
#3 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 7 lines · On Thursday 22 May 2014 09:41:39 Linus Walleij wrote:
Quoted 4 lines · Arnd Bergmann
Quoted 1 line

+++ b/arch/arm/boot/dts/arm-realview-pb1176.dts

Can you split this up into an arm-realview.dtsi file for the common parts and a pb1176 specific file for the rest?

Quoted 5 lines

I wonder if that makes any kind of sense. The RealView platforms does not really share much more than the name, sadly. For example: IP blocks aren't even at the same address.

I could create a .dtsi file but it would be empty :-(

Ok, fair enough.

Quoted 16 lines
Quoted 11 lines

Review

Quoted 10 lines
(quoted diff) +10 -0
+     soc {
+             #address-cells = <1>;
+             #size-cells = <1>;
+             compatible = "simple-bus";
+             ranges;
+
+             syscon: syscon at 10000000 {
+                     compatible = "arm,realview-pb1176-syscon", "syscon";
+                     reg = <0x10000000 0x1000>;
+             };
Quoted 4 lines

Hmm, in order to have a proper reset driver, we probably want a separate node for the reset controller. I believe the x-gene people have just posted a generic reset driver based on syscon. Let's see if we can share that.

Quoted 7 lines

I have looked at it. Patch titled
"power: reset: Add generic SYSCON register mapped reset"

Sadly it does not work for our case. This is because it assumes that reboot will be triggered by writing one value to one register. The RealViews need you to *first* write to a special unlock register, then write a magic value to a magic register.

We've already added a significant number of reset drivers like this, I guess it doesn't hurt to have one more. We have so far resisted adding a register state machine to DT.

Quoted 11 lines

So of course we could extend the syscon regmap reset driver by starting to encode a jam table such as a sequence of register writes to a sequence of registers, but we have refused such code in the past because as I recall Grant said, it is the beginning of a byte code interpreter and then we could as well bite the bullet and start implementing open firmware.

This is exactly the type of thing that ACPI AML code usually does by the way so what he says makes a *lot* of sense.

So for now I keep to our special driver.

Ok.

Quoted 18 lines
Quoted 9 lines
Quoted 9 lines
(quoted diff) +9 -0
+/* Pointer to the system controller */
+struct regmap *syscon_regmap;
+u8 syscon_type;
+
+static struct map_desc realview_dt_io_desc[] __initdata = {
+     {
+             /* FIXME: static map needed for LED driver */
+             .virtual        = IO_ADDRESS(REALVIEW_SYS_BASE),
+             .pfn            = __phys_to_pfn(REALVIEW_SYS_BASE),
Quoted 8 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+             .length         = SZ_4K,
+             .type           = MT_DEVICE,
+     },
+};

I've looked at the driver and I don't see why this is required. The driver does a devm_ioremap_resource(), which should work with or without a static mapping.

Quoted 7 lines

Well yeah, the version of the driver that is in linux-next does this. This was parallel work and I had no idea which would reach mainline first :-)

Anyway, I just deleted because I realized it was better if I came up with the concept of syscon-leds to handle this through the syscon.

Ok.

Quoted 31 lines
Quoted 22 lines
Quoted 22 lines
(quoted diff) +22 -0
+#if IS_ENABLED(CONFIG_CACHE_L2X0)
+     if (of_machine_is_compatible("arm,realview-eb"))
+             /*
+              * 1MB (128KB/way), 8-way associativity,
+              * evmon/parity/share enabled
+              * Bits:  .... ...0 0111 1001 0000 .... .... ....
+              */
+             l2x0_of_init(0x00790000, 0xfe000fff);
+     else if (of_machine_is_compatible("arm,realview-pb1176"))
+             /*
+              * 128Kb (16Kb/way) 8-way associativity.
+              * evmon/parity/share enabled.
+              */
+             l2x0_of_init(0x00730000, 0xfe000fff);
+     else if (of_machine_is_compatible("arm,realview-pb11mp"))
+             /*
+              * 1MB (128KB/way), 8-way associativity,
+              * evmon/parity/share enabled
+              * Bits:  .... ...0 0111 1001 0000 .... .... ....
+              */
+             l2x0_of_init(0x00730000, 0xfe000fff);
+     else if (of_machine_is_compatible("arm,realview-pbx"))
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+             /*
+              * 16KB way size, 8-way associativity, parity disabled
+              * Bits:  .. 0 0 0 0 1 00 1 0 1 001 0 000 0 .... .... ....
+              */
+             l2x0_of_init(0x02520000, 0xc0000fff);
+#endif

You wrote that you added the #if IS_ENABLED() here. Why?

Quoted 3 lines

Mainly because I had this other review comment that I should use IS_ENABLED() rather than #ifdef and it seemed better but I don't know anymore...

There are two reasons for using IS_ENABLED: the first is that it catches both the =y and the =m case in Kconfig, which doesn't apply here since the cache controller driver cannot be a loadable module.

The other is that you can replace the #ifdef with an if(), to improve readability and compile time coverage, but you didn't do that.

Quoted 6 lines
Quoted 6 lines

l2x0_of_init is already a nop if CONFIG_CACHE_L2X0 is not set. If you want to skip a bit of dead code here, you could make it an inline function and do

        if (IS_ENABLED(CONFIG_CACHE_L2X0))
                realview_setup_l2x0();
Quoted 6 lines
Quoted 3 lines

Other than that, I still think we should avoid doing anything platform specific here at all, and move it all into the l2x0_of_init function, based on Russell's l2x0 patch series.

I just need a base branch I guess, then I can build on top of Russell's patches, it's just a matter of dependencies.

Ok.

Quoted 24 lines
Quoted 24 lines
Quoted 23 lines
(quoted diff) +23 -0
+     soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
+     if (!soc_dev_attr)
+             return;
+     ret = of_property_read_string(root, "compatible",
+                                   &soc_dev_attr->soc_id);
+     if (ret)
+             return;
+     ret = of_property_read_string(root, "model", &soc_dev_attr->machine);
+        if (ret)
+             return;
+     soc_dev_attr->family = "RealView";
+     soc_dev = soc_device_register(soc_dev_attr);
+     if (IS_ERR(soc_dev)) {
+             kfree(soc_dev_attr);
+             return;
+     }
+     parent = soc_device_to_device(soc_dev);
+     ret = of_platform_populate(root, of_default_bus_match_table,
+                                NULL, parent);
+     if (ret) {
+             pr_crit("could not populate device tree\n");
+             return;
+     }
Quoted 6 lines
Quoted 3 lines

I wonder if there is a way to do this without having code in the platform. I would hate it if we get to the point where each platform is completely empty but still requires a copy of this code.

The question can be rephrased like this:
Should we create drivers/soc?

I think we just did, not sure if the patches have made it into linux-next already, but there were a couple of other platforms already that want this directory.

Quoted 3 lines

The structure of the SoC bus device is maybe a bit irritating but it has been hammered into the device core so now we have to live with it as it is.

We can always improve the code.

Quoted 5 lines
Quoted 4 lines

I also noticed that you pass the DT root here, which isn't actually the intention of the soc device interface: you should pass the DT node that has the on-chip devices but not the board-level (top-level) nodes such as your fixed clocks.

Quoted 7 lines

Hm that's true. I introduced yet another set of compatible tuples like this:
compatible = "simple-bus", "arm-realview-pb1176-soc"

It will, just like the reset driver generate a new batch of identical compatible-strings and documenation sets, but it does have the upside of clean separation.

Ok

Quoted 19 lines
Quoted 10 lines
Quoted 10 lines
arch/arm/mm/Kconfig +1 -1
index 5bf7c3c3b301..5461c4401ed6 100644
@@ -928,7 +928,7 @@ config ARM_L1_CACHE_SHIFT
 config ARM_DMA_MEM_BUFFERABLE
      bool "Use non-cacheable memory for DMA" if (CPU_V6 || CPU_V6K) && !CPU_V7
      depends on !(MACH_REALVIEW_PB1176 || REALVIEW_EB_ARM11MP || \
-                  MACH_REALVIEW_PB11MP)
+                  MACH_REALVIEW_PB11MP || REALVIEW_DT)
Quoted 8 lines

arch/arm/mm/Kconfig config ARM_L1_CACHE_SHIFT · line 934

Quoted 3 lines
arch/arm/mm/Kconfig +0 -0
      default y if CPU_V6 || CPU_V6K || CPU_V7
      help
        Historically, the kernel has used strongly ordered mappings to

Do you have an explanation for why this is needed? I don't remember seeing this before, but I guess it gets in the way of multiplatform support. Is this just a performance optimization on realview, or is it actually required?

Quoted 3 lines

I don't know that actually. And as DMA is likely only used on PCI I have no way of testing it either without such hardware. But I guess I can just leave it out for the time being.

Ok, let's try that. It would be good to have some insight from somebody who understands the architecture though.

Quoted 1 line

From reading the code, I understand that

* On ARMv7, we always treat memory as bufferable, dma_alloc_coherent() uses L_PTE_MT_BUFFERABLE just like dma_alloc_writecombine does, and the mb/rmb/wmb macros flush the write buffers as needed.
* On ARMv5, we never treat dma_alloc_coherent() memory as bufferable, and the mb/rmb/wmb macros do not need to flush the write buffers.
* On ARMv6-only kernels we allow the user to pick the behavior at compile time.
* The code above for Realview was introduced by this commit:

commit 42c4dafe803dcad82980fd8b0831a89032156f93

Author: Catalin Marinas <[email protected]>
Date:   Thu Jul 1 13:22:48 2010 +0100

ARM: 6202/1: Do not ARM_DMA_MEM_BUFFERABLE on RealView boards with L210/L220

RealView boards with certain revisions of the L210/L220 cache controller may have issues (hardware deadlock) with the mandatory barriers (DSB followed by an L2 cache sync) when ARM_DMA_MEM_BUFFERABLE is enabled. The patch disables ARM_DMA_MEM_BUFFERABLE for these boards.

arch/arm/mm/Kconfig +2 -0
index fc1b2fa..101105e 100644
@@ -813,6 +813,8 @@ config ARM_L1_CACHE_SHIFT
 
 config ARM_DMA_MEM_BUFFERABLE
        bool "Use non-cacheable memory for DMA" if CPU_V6 && !CPU_V7
+       depends on !(MACH_REALVIEW_PB1176 || REALVIEW_EB_ARM11MP || \
+                    MACH_REALVIEW_PB11MP)
        default y if CPU_V6 || CPU_V7
        help
          Historically, the kernel has used strongly ordered mappings to

This means that in a kernel that supports one of these three realview variants, we shouldn't enable this option, but that in turn will break any ARMv7 machine supported by the same kernel. This is bad. Not setting ARM_DMA_MEM_BUFFERABLE will break with MACH_REALVIEW_DT on your PB1176, which is also bad.

Maybe Catalin remembers the exact nature of the deadlock so we can come up with another solution. If I read the original patches correctly, the dsb() wasn't a problem, but the outer_sync() was. Maybe we can do an alternative implementation for outer_sync() for the affected machines that avoids the deadlock?

Arnd

#4 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 8 lines · Arnd Bergmann
Quoted 2 lines · On Thursday 22 May 2014 09:41:39 Linus Walleij wrote:

The question can be rephrased like this:
Should we create drivers/soc?

I think we just did, not sure if the patches have made it into linux-next already, but there were a couple of other platforms already that want this directory.

Awesomeness. Can you give me the title of a patch or branch from ARM SoC I should pull in to build on top of this? Then I can split off all the SoC bus business and have the machine file real small, nice and clean.

Yours,
Linus Walleij

maintainer
#5 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 6 lines · On Thursday 22 May 2014 15:04:07 Linus Walleij wrote:
Quoted 5 lines · Arnd Bergmann
Quoted 2 lines · On Thursday 22 May 2014 09:41:39 Linus Walleij wrote:

The question can be rephrased like this:
Should we create drivers/soc?

Quoted 8 lines
Quoted 3 lines

I think we just did, not sure if the patches have made it into linux-next already, but there were a couple of other platforms already that want this directory.

Awesomeness. Can you give me the title of a patch or branch from ARM SoC I should pull in to build on top of this? Then I can split off all the SoC bus business and have the machine file real small, nice and clean.

I'd have to check again, it seems that as of today, there is no drivers/soc on arm-soc yet. I do remember that there were at least two platforms besides yours that want it though, so we have to coordinate it a bit.

Then again, as I expect all of them to go through arm-soc, we can probably resolve the conflicts in our tree.

Arnd

#6 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

In reply to Arnd Bergmann
Quoted 6 lines · Arnd Bergmann

commit 42c4dafe803dcad82980fd8b0831a89032156f93

Author: Catalin Marinas <[email protected]>
Date:   Thu Jul 1 13:22:48 2010 +0100

ARM: 6202/1: Do not ARM_DMA_MEM_BUFFERABLE on RealView boards with L210/L220

Quoted 8 lines

RealView boards with certain revisions of the L210/L220 cache controller may have issues (hardware deadlock) with the mandatory barriers (DSB followed by an L2 cache sync) when ARM_DMA_MEM_BUFFERABLE is enabled. The patch disables ARM_DMA_MEM_BUFFERABLE for these boards.

Looking through the L220 errata document, I think this relates to 425331 which says that a cache sync can deadlock the system if the write buffer is not empty. By making the DMA memory Strongly Ordered, we avoid the L220 write buffer. In addition to the above commit, we also had 2503a5ecd86c (ARM: 6201/1: RealView: Do not use outer_sync() on ARM11MPCore boards with L220).

The above only solves the issue for coherent DMA allocations. Streaming DMA requires additional changes to cache-l2x0.c but they are all weird and depend on how the L2x0 is wired (for example, using SWP to access the L2x0 registers works on one RealView board but not on another). Anyway, I don't think we have any streaming DMA used on these RealView boards.

If we only use the DMA coherent buffers for CLCD on these boards, we don't really need the outer_sync() either so we could go back to bufferable DMA memory (strangely enough, this L2 cache workaround looks similar to Marvell's PCI+PL310 problem where we want to avoid outer_sync()).

Signature · 1 line
Catalin
maintainer
#7 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 12 lines · On Thursday 22 May 2014 15:45:03 Catalin Marinas wrote:
Quoted 11 lines · Arnd Bergmann

commit 42c4dafe803dcad82980fd8b0831a89032156f93

Author: Catalin Marinas <[email protected]>
Date:   Thu Jul 1 13:22:48 2010 +0100

ARM: 6202/1: Do not ARM_DMA_MEM_BUFFERABLE on RealView boards with L210/L220

RealView boards with certain revisions of the L210/L220 cache controller may have issues (hardware deadlock) with the mandatory barriers (DSB followed by an L2 cache sync) when ARM_DMA_MEM_BUFFERABLE is enabled. The patch disables ARM_DMA_MEM_BUFFERABLE for these boards.

Quoted 7 lines
Quoted 3 lines

Looking through the L220 errata document, I think this relates to 425331 which says that a cache sync can deadlock the system if the write buffer is not empty.

Hmm, but isn't the write buffer empty here because we have just issued a dsb()?

Quoted 4 lines

By making the DMA memory Strongly Ordered, we avoid the L220 write buffer. In addition to the above commit, we also had 2503a5ecd86c (ARM: 6201/1: RealView: Do not use outer_sync() on ARM11MPCore boards with L220).

Ah, so this also stands in the way of multiplatform realview :(

Quoted 6 lines

The above only solves the issue for coherent DMA allocations. Streaming DMA requires additional changes to cache-l2x0.c but they are all weird and depend on how the L2x0 is wired (for example, using SWP to access the L2x0 registers works on one RealView board but not on another). Anyway, I don't think we have any streaming DMA used on these RealView boards.

Yes, I think that is correct. MMCI, SSP and serial could all use DMA but are not configured to do it.

Quoted 5 lines

If we only use the DMA coherent buffers for CLCD on these boards, we don't really need the outer_sync() either so we could go back to bufferable DMA memory (strangely enough, this L2 cache workaround looks similar to Marvell's PCI+PL310 problem where we want to avoid outer_sync()).

Ah, so you think we can add another hook to NULL out outer_cache.sync per platform?

Arnd

#8 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 17 lines · Arnd Bergmann
Quoted 16 lines · On Thursday 22 May 2014 15:45:03 Catalin Marinas wrote:
Quoted 14 lines · Arnd Bergmann

commit 42c4dafe803dcad82980fd8b0831a89032156f93

Author: Catalin Marinas <[email protected]>
Date:   Thu Jul 1 13:22:48 2010 +0100

ARM: 6202/1: Do not ARM_DMA_MEM_BUFFERABLE on RealView boards with L210/L220

RealView boards with certain revisions of the L210/L220 cache controller may have issues (hardware deadlock) with the mandatory barriers (DSB followed by an L2 cache sync) when ARM_DMA_MEM_BUFFERABLE is enabled. The patch disables ARM_DMA_MEM_BUFFERABLE for these boards.

Quoted 6 lines
Quoted 3 lines

Looking through the L220 errata document, I think this relates to 425331 which says that a cache sync can deadlock the system if the write buffer is not empty.

Hmm, but isn't the write buffer empty here because we have just issued a dsb()?

The problem with these outer caches is that DSB doesn't propagate to them (and I hope to never see such caches again ;)). So an mb() on such systems needs to be dsb()+outer_sync(). The dsb() pushes data from the CPU write buffer into the L2x0 write buffer and outer_sync() does the flushing out of the outer cache write buffer (which strangely enough buffers even if the memory access is Normal NonCacheable).

Quoted 8 lines
Quoted 5 lines

If we only use the DMA coherent buffers for CLCD on these boards, we don't really need the outer_sync() either so we could go back to bufferable DMA memory (strangely enough, this L2 cache workaround looks similar to Marvell's PCI+PL310 problem where we want to avoid outer_sync()).

Ah, so you think we can add another hook to NULL out outer_cache.sync per platform?

Yes, with the note that mb() no longer guarantees the write being flushed to RAM but just to the L2 write buffer. If CLCD framebuffer is the only DMA device, we don't need to worry about such barriers since we map it as writecombine anyway (hence bufferable).

Unless Linus or Pawel think there are other DMA capable devices in use here, I'm happy to enable ARM_DMA_MEM_BUFFERABLE for these boards.

Signature · 1 line
Catalin
#9 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

In reply to Arnd Bergmann
Quoted 7 lines · Arnd Bergmann
Quoted 4 lines · On Thursday 22 May 2014 15:45:03 Catalin Marinas wrote:

By making the DMA memory Strongly Ordered, we avoid the L220 write buffer. In addition to the above commit, we also had 2503a5ecd86c (ARM: 6201/1: RealView: Do not use outer_sync() on ARM11MPCore boards with L220).

Ah, so this also stands in the way of multiplatform realview :(

Isn't the real value of a multiplatform realview in the ability to use it with Qemu? Did Qemu go to the extreme of emulating all those cache issues?

Nicolas

maintainer
#10 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 8 lines · On Thursday 22 May 2014 14:09:52 Nicolas Pitre wrote:
Quoted 6 lines · Arnd Bergmann
Quoted 4 lines · On Thursday 22 May 2014 15:45:03 Catalin Marinas wrote:

By making the DMA memory Strongly Ordered, we avoid the L220 write buffer. In addition to the above commit, we also had 2503a5ecd86c (ARM: 6201/1: RealView: Do not use outer_sync() on ARM11MPCore boards with L220).

Quoted 5 lines
Quoted 1 line

Ah, so this also stands in the way of multiplatform realview

Isn't the real value of a multiplatform realview in the ability to use it with Qemu? Did Qemu go to the extreme of emulating all those cache issues?

The qemu developers have in the past always argued strongly that they want to run the same kernel that runs on real hardware. Having a kernel that runs only on the emulated version but not on actual hardware is clearly against that goal, and we should use CONFIG_ARCH_VIRT for that purpose.

Arnd

#11 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 15 lines · Arnd Bergmann
Quoted 8 lines · On Thursday 22 May 2014 14:09:52 Nicolas Pitre wrote:
Quoted 6 lines · Arnd Bergmann
Quoted 4 lines · On Thursday 22 May 2014 15:45:03 Catalin Marinas wrote:

By making the DMA memory Strongly Ordered, we avoid the L220 write buffer. In addition to the above commit, we also had 2503a5ecd86c (ARM: 6201/1: RealView: Do not use outer_sync() on ARM11MPCore boards with L220).

Quoted 5 lines
Quoted 1 line

Ah, so this also stands in the way of multiplatform realview

Isn't the real value of a multiplatform realview in the ability to use it with Qemu? Did Qemu go to the extreme of emulating all those cache issues?

Quoted 5 lines

The qemu developers have in the past always argued strongly that they want to run the same kernel that runs on real hardware. Having a kernel that runs only on the emulated version but not on actual hardware is clearly against that goal, and we should use CONFIG_ARCH_VIRT for that purpose.

But the kernel that actually runs on the real hardware also runs on Qemu already. It is just that, by some coincidence, a multi-platform kernel that pretends to have the same peripherals as a Realview board could also be used on Qemu.

Otherwise I really don't see the point in making the code more complex just to make a multi-platform kernel run on actual Realview hardware that is quite obsolete now and that very few people have access to.

Nicolas

maintainer
#12 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 23 lines · On Thursday 22 May 2014 14:39:20 Nicolas Pitre wrote:
Quoted 15 lines · Arnd Bergmann
Quoted 8 lines · On Thursday 22 May 2014 14:09:52 Nicolas Pitre wrote:
Quoted 6 lines · Arnd Bergmann
Quoted 4 lines · On Thursday 22 May 2014 15:45:03 Catalin Marinas wrote:

By making the DMA memory Strongly Ordered, we avoid the L220 write buffer. In addition to the above commit, we also had 2503a5ecd86c (ARM: 6201/1: RealView: Do not use outer_sync() on ARM11MPCore boards with L220).

Quoted 5 lines
Quoted 1 line

Ah, so this also stands in the way of multiplatform realview

Isn't the real value of a multiplatform realview in the ability to use it with Qemu? Did Qemu go to the extreme of emulating all those cache issues?

Quoted 5 lines

The qemu developers have in the past always argued strongly that they want to run the same kernel that runs on real hardware. Having a kernel that runs only on the emulated version but not on actual hardware is clearly against that goal, and we should use CONFIG_ARCH_VIRT for that purpose.

Quoted 8 lines

But the kernel that actually runs on the real hardware also runs on Qemu already. It is just that, by some coincidence, a multi-platform kernel that pretends to have the same peripherals as a Realview board could also be used on Qemu.

Otherwise I really don't see the point in making the code more complex just to make a multi-platform kernel run on actual Realview hardware that is quite obsolete now and that very few people have access to.

The main reason is that out of principle I want to get to the point where every ARMv6 and ARMv7 machine is part of CONFIG_ARCH_MULTIPLATFORM.

Then again, once we introduce CONFIG_BROKEN_MULTIPLATFORM, we could have an option that depends on that, and turns on both ARM_DMA_MEM_BUFFERABLE as well as the hack to remove the outer_sync from mb() and wmb() for CONFIG_SMP.

Arnd

#13 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

In reply to Arnd Bergmann
Quoted 7 lines · Arnd Bergmann
Quoted 2 lines · On Thursday 22 May 2014 15:45:03 Catalin Marinas wrote:

Anyway, I don't think we have any streaming DMA used on these RealView boards.

Yes, I think that is correct. MMCI, SSP and serial could all use DMA but are not configured to do it.

PB1176 has no working DMA (borked DMA hardware), the PB11MPCore actually has working DMA but only on UART TX and maybe AACI (haven't tested) no clue about the Cortex boards PBA8 and PBX though.

Yours,
Linus Walleij

maintainer
#14 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 6 lines · On Thursday 22 May 2014 20:52:38 Linus Walleij wrote:
Quoted 5 lines · Arnd Bergmann
Quoted 2 lines · On Thursday 22 May 2014 15:45:03 Catalin Marinas wrote:

Anyway, I don't think we have any streaming DMA used on these RealView boards.

Quoted 6 lines
Quoted 2 lines

Yes, I think that is correct. MMCI, SSP and serial could all use DMA but are not configured to do it.

PB1176 has no working DMA (borked DMA hardware), the PB11MPCore actually has working DMA but only on UART TX and maybe AACI (haven't tested) no clue about the Cortex boards PBA8 and PBX though.

I don't see any DMA data passed to the pl011 driver for PB11MPCore or any other realview board, so even if it works, we don't use it.

The AACI driver doesn't seem to support DMA.

PBA8 and PBX don't need the hack because they don't have a broken l2x0.

Arnd

#15 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 10 lines · Arnd Bergmann
Quoted 9 lines · On Thursday 22 May 2014 20:52:38 Linus Walleij wrote:
Quoted 7 lines · Arnd Bergmann
Quoted 2 lines · On Thursday 22 May 2014 15:45:03 Catalin Marinas wrote:

Anyway, I don't think we have any streaming DMA used on these RealView boards.

Yes, I think that is correct. MMCI, SSP and serial could all use DMA but are not configured to do it.

Quoted 6 lines
Quoted 3 lines

PB1176 has no working DMA (borked DMA hardware), the PB11MPCore actually has working DMA but only on UART TX and maybe AACI (haven't tested) no clue about the Cortex boards PBA8 and PBX though.

I don't see any DMA data passed to the pl011 driver for PB11MPCore or any other realview board, so even if it works, we don't use it.

No I was more saying it works in theory... and we actually have patches to enable it for e.g. memcpy on Versatile and PB11MP that I floated at times, Russell also experimented around with it.

Quoted 1 line

The AACI driver doesn't seem to support DMA.

Nope, but the hardware does.

Quoted 2 lines

PBA8 and PBX don't need the hack because they don't have a broken l2x0.

True!

Yours,
Linus Walleij

#16 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

In reply to Linus Walleij
Quoted 43 lines · Linus Walleij
Quoted 4 lines · Arnd Bergmann
Quoted 1 line

+++ b/arch/arm/boot/dts/arm-realview-pb1176.dts

Can you split this up into an arm-realview.dtsi file for the common parts and a pb1176 specific file for the rest?

I wonder if that makes any kind of sense. The RealView platforms does not really share much more than the name, sadly. For example: IP blocks aren't even at the same address.

I could create a .dtsi file but it would be empty :-(

Quoted 11 lines

arch/arm/boot/dts/arm-realview-pb1176.dts line 83

Quoted 10 lines
arch/arm/boot/dts/arm-realview-pb1176.dts new file +10 -0
+     soc {
+             #address-cells = <1>;
+             #size-cells = <1>;
+             compatible = "simple-bus";
+             ranges;
+
+             syscon: syscon at 10000000 {
+                     compatible = "arm,realview-pb1176-syscon", "syscon";
+                     reg = <0x10000000 0x1000>;
+             };
Quoted 4 lines

Hmm, in order to have a proper reset driver, we probably want a separate node for the reset controller. I believe the x-gene people have just posted a generic reset driver based on syscon. Let's see if we can share that.

I have looked at it. Patch titled
"power: reset: Add generic SYSCON register mapped reset"

Sadly it does not work for our case. This is because it assumes that reboot will be triggered by writing one value to one register. The RealViews need you to *first* write to a special unlock register, then write a magic value to a magic register.

So of course we could extend the syscon regmap reset driver by starting to encode a jam table such as a sequence of register writes to a sequence of registers, but we have refused such code in the past because as I recall Grant said, it is the beginning of a byte code interpreter and then we could as well bite the bullet and start implementing open firmware.

Quoted 4 lines

This is exactly the type of thing that ACPI AML code usually does by the way so what he says makes a *lot* of sense.

So for now I keep to our special driver.

+1 for drivers/soc _and_ of_xlate for regmap instead of syscon.

driven by Mike Turquette's request to join mach-berlin clock node into a single node, we currently use pinctrl driver (that must be somewhere in Linus inbox) as a crutch to register a regmap for other drivers.

I also saw that Arnd requested to not chop chip registers into tiny pieces for Linux subsystems but rather have a single DT node and make the drivers use it.

Mike also mentioned he is fine with different subsystem drivers reusing the same node property-wise, i.e. reset and clock in a single node.

With Berlin (and other SoCs for sure) ultimately there will be a bunch of drivers that require the io resource and the node early and non-early. Now, using syscon would still require dummy nodes for each subsystem, as there will be only one driver registered to a single DT node.

With a drivers/soc we'd have a good place for those messy, SoC-specific registers to put a driver that hooks up to a single node and takes care of registering e.g. pinctrl and reset the plain-old platform_device way. It will be a little bit like arm/mach-foo before, but maybe we have to admit that on SoCs there will always be some amount of very specific, non-probable, non-describable registers that simply need special treatment.

Sebastian

#17 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 9 lines · Sebastian Hesselbarth
Quoted 1 line · Linus Walleij

So for now I keep to our special driver.

+1 for drivers/soc _and_ of_xlate for regmap instead of syscon.

driven by Mike Turquette's request to join mach-berlin clock node into a single node, we currently use pinctrl driver (that must be somewhere in Linus inbox) as a crutch to register a regmap for other drivers.

I've merged it actually, I think. Else hit me on any missed patches.

Quoted 7 lines

With a drivers/soc we'd have a good place for those messy, SoC-specific registers to put a driver that hooks up to a single node and takes care of registering e.g. pinctrl and reset the plain-old platform_device way. It will be a little bit like arm/mach-foo before, but maybe we have to admit that on SoCs there will always be some amount of very specific, non-probable, non-describable registers that simply need special treatment.

Hmmmm well I want to see that code before I believe it ... but I get what you mean.

Yours,
Linus Walleij

#18 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 5 lines · Linus Walleij
Quoted 3 lines · Sebastian Hesselbarth
Quoted 1 line · Linus Walleij

So for now I keep to our special driver.

Quoted 7 lines
Quoted 5 lines

+1 for drivers/soc _and_ of_xlate for regmap instead of syscon.

driven by Mike Turquette's request to join mach-berlin clock node into a single node, we currently use pinctrl driver (that must be somewhere in Linus inbox) as a crutch to register a regmap for other drivers.

I've merged it actually, I think. Else hit me on any missed patches.

Yeah, you did. Thanks, no more patches on this one.

Quoted 10 lines
Quoted 7 lines

With a drivers/soc we'd have a good place for those messy, SoC-specific registers to put a driver that hooks up to a single node and takes care of registering e.g. pinctrl and reset the plain-old platform_device way. It will be a little bit like arm/mach-foo before, but maybe we have to admit that on SoCs there will always be some amount of very specific, non-probable, non-describable registers that simply need special treatment.

Hmmmm well I want to see that code before I believe it ... but I get what you mean.

Well, the register set on Berlin I was thinking about tackles pinctrl, padmux, clock, reset and some other freaky stuff I cannot recall. The mess in it is that there is no order you can apply to split it up into separate nodes. Moreover, the next SoC has some registers just shoved into it, moving some other registers.

It is already painful to deal with in code, and I have been through several iterations of the clk driver with both choppy-DT and one-DT node approaches.

At some point, Linux subsystem driven DT node tailoring isn't the answer. Sometimes it fits very well, sometimes you will either end up with gazillions of "clocks"/"clock-names" properties (just think of a pll with 2x8 outputs) or the same amount of "reg" properties to collect e.g. all registers dealing with reset.

The way we will go for it on Berlin is now:

(a) have a single node for the whole chip-ctrl register set
(b) register clk driver on it (early, as it provides timer clk)
(c) register one chip-ctrl driver on it that will
(d) reserve the iomap,
(e) register a regmap-mmio,
(f) register platform_devices for {pinctrl,reset,...} that will use regmap-mmio and read their properties from the single node

The individual drivers should stay in their respective drivers/ folder, but (c) needs a good home. It would have been mach-foo in the past, and could be drivers/soc now. In (f), I tend to just lookup regmap by name, no DT involved at all.

BTW, syscon is of no use here at all, once it registers itself as a driver for the node, there will be no other drivers registered. That would at least require (again subsystem driven) dummy-nodes for pinctrl, clk, reset.

Also, using some of_xlate as we already have for clk, gpio, pinctrl ... would put an end to syscon's syscon_get_regmap_from_{foo,bar,baz,...} that gets a new helper every cycle.

Sebastian

#19 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 10 lines · Sebastian Hesselbarth
Quoted 2 lines · Linus Walleij

Hmmmm well I want to see that code before I believe it ... but I get what you mean.

Well, the register set on Berlin I was thinking about tackles pinctrl, padmux, clock, reset and some other freaky stuff I cannot recall. The mess in it is that there is no order you can apply to split it up into separate nodes. Moreover, the next SoC has some registers just shoved into it, moving some other registers.

Berlin hardware engineers seem to be hellbent on complicating your life.

Quoted 10 lines

The way we will go for it on Berlin is now:

(a) have a single node for the whole chip-ctrl register set
(b) register clk driver on it (early, as it provides timer clk)
(c) register one chip-ctrl driver on it that will
(d) reserve the iomap,
(e) register a regmap-mmio,
(f) register platform_devices for {pinctrl,reset,...} that will use regmap-mmio and read their properties from the single node

OK makes perfect sense actually.

Quoted 4 lines

The individual drivers should stay in their respective drivers/ folder, but (c) needs a good home. It would have been mach-foo in the past, and could be drivers/soc now. In (f), I tend to just lookup regmap by name, no DT involved at all.

OK

Quoted 4 lines

BTW, syscon is of no use here at all, once it registers itself as a driver for the node, there will be no other drivers registered. That would at least require (again subsystem driven) dummy-nodes for pinctrl, clk, reset.

Yeah that sucks, we need something more flexible no question about that.

Will the chipctrl driver be as generic as syscon or a Berlin-specific thing?

Quoted 3 lines

Also, using some of_xlate as we already have for clk, gpio, pinctrl ... would put an end to syscon's syscon_get_regmap_from_{foo,bar,baz,...} that gets a new helper every cycle.

This part I don't understand but I guess I will understand it when I see the code.

Yours,
Linus Walleij

#20 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 6 lines · Linus Walleij
Quoted 4 lines · Sebastian Hesselbarth
Quoted 2 lines · Linus Walleij

Hmmmm well I want to see that code before I believe it ... but I get what you mean.

Quoted 8 lines
Quoted 5 lines

Well, the register set on Berlin I was thinking about tackles pinctrl, padmux, clock, reset and some other freaky stuff I cannot recall. The mess in it is that there is no order you can apply to split it up into separate nodes. Moreover, the next SoC has some registers just shoved into it, moving some other registers.

Berlin hardware engineers seem to be hellbent on complicating your life.

Tell me about it! ;)

Although, I don't think it is exclusive to Berlin but other vendors, too.

Quoted 25 lines
Quoted 10 lines

The way we will go for it on Berlin is now:

(a) have a single node for the whole chip-ctrl register set
(b) register clk driver on it (early, as it provides timer clk)
(c) register one chip-ctrl driver on it that will
(d) reserve the iomap,
(e) register a regmap-mmio,
(f) register platform_devices for {pinctrl,reset,...} that will use regmap-mmio and read their properties from the single node

OK makes perfect sense actually.

Quoted 4 lines

The individual drivers should stay in their respective drivers/ folder, but (c) needs a good home. It would have been mach-foo in the past, and could be drivers/soc now. In (f), I tend to just lookup regmap by name, no DT involved at all.

OK

Quoted 4 lines

BTW, syscon is of no use here at all, once it registers itself as a driver for the node, there will be no other drivers registered. That would at least require (again subsystem driven) dummy-nodes for pinctrl, clk, reset.

Quoted 5 lines

Yeah that sucks, we need something more flexible no question about that.

Will the chipctrl driver be as generic as syscon or a Berlin-specific thing?

There is nothing berlin-specific about it except what regmap(s) and platform_devices are registered. I can think of some generic helpers to ease looping over regmap structs to be registered, IIRC driver core already provides the same for platform_devices.

Actually, syscon is already a good starting point, it just lacks to register platform_devices using the DT-registered regmap.

Quoted 6 lines
Quoted 3 lines

Also, using some of_xlate as we already have for clk, gpio, pinctrl ... would put an end to syscon's syscon_get_regmap_from_{foo,bar,baz,...} that gets a new helper every cycle.

This part I don't understand but I guess I will understand it when I see the code.

git grep 'struct regmap \*syscon' drivers/mfd/syscon.c drivers/mfd/syscon.c:struct regmap *syscon_node_to_regmap(struct device_node *np)
drivers/mfd/syscon.c:struct regmap *syscon_regmap_lookup_by_compatible(const char *s) drivers/mfd/syscon.c:struct regmap *syscon_regmap_lookup_by_pdevname(const char *s) drivers/mfd/syscon.c:struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,

and IIRC I have seen pending patches adding more syscon_regmap_by_foo helpers.

My point is that we should have some regmap-provider to allow us to use phandle+specifier, just like of_clk_get and friends work.

With some more time after v3.16-rc1 drops, I plan to provide a chip-ctrl driver for Berlin. Most likely not too generic or based on extending syscon but definitely something to talk about.

Sebastian

maintainer
#21 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

In reply to Linus Walleij
Quoted 165 lines · Linus Walleij

This implements basic device tree boot support for the RealView platforms, with a basic device tree for ARM PB1176 as an example.

The implementation is done with a new DT-specific board file using only pre-existing bindings for the basic IRQ, timer and serial port drivers. A new compatible type is added to the GIC for the ARM1176.

This implementation uses the MFD syscon handle from day one to access the system controller registers, and register the devices using the SoC bus.

Cc Arnd Bergmann <[email protected]>
Cc Rob Herring <[email protected]>

---
ChangeLog v1->v2:
- Adjust timer clock names to be the same as the example in the device tree binding.
- Remove all memory fixup code - this should be handled by the device tree specification of memory areas or by special MM hacks for the RealView PBX.
- Fix the documentation around syscon to specify that it can be in any node, need not be the root node.
- Switch device tree license to the BSD license, taken from arch/powerpc/boot/dts/p1024rdb_32b.dts
- Add a hunk for the new compatible string to Documentation/devicetree/bindings/arm/gic.txt
- Move the clocks out of the SoC node, certainly the xtal is not sitting on the SoC...
- Sort the selects in Kconfig alphabetically
- Use IS_ENABLED() for the l2x0 code snippet
- Instead of checking the board variant in the reset routine to figure out how to tweak the reset controller, have a compatible string for each syscon variant, map it to an enum that provides a unique type ID and that way figure out how to handle it in a maybe more elegant way.
- Open issue: what do to with the l2x0 stuff?

---
 Documentation/devicetree/bindings/arm/arm-boards |  57 ++++++
 Documentation/devicetree/bindings/arm/gic.txt    |   1 +
 arch/arm/boot/dts/Makefile                       |   1 +
 arch/arm/boot/dts/arm-realview-pb1176.dts        | 177 ++++++++++++++++++
 arch/arm/mach-realview/Kconfig                   |  10 +
 arch/arm/mach-realview/Makefile                  |   1 +
 arch/arm/mach-realview/realview-dt.c             | 225 +++++++++++++++++++++++
 arch/arm/mm/Kconfig                              |   2 +-
 drivers/irqchip/irq-gic.c                        |   1 +
 9 files changed, 474 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/arm-realview-pb1176.dts
 create mode 100644 arch/arm/mach-realview/realview-dt.c
Documentation/devicetree/bindings/arm/arm-boards +57 -0
index 3509707f9320..86b36f399d1c 100644
@@ -86,3 +86,60 @@ Interrupt controllers:
        compatible = "arm,versatile-sic";
        interrupt-controller;
        #interrupt-cells = <1>;
+
+
+ARM RealView Boards
+-------------------
+The RealView boards cover tailored evaluation boards that are used to explore
+the ARM11 and Cortex A-8 and Cortex A-9 processors.
+
+Required properties (in root node):
+       /* RealView Emulation Baseboard */
+       compatible = "arm,realview-eb";
+        /* RealView Platform Baseboard for ARM1176JZF-S */
+       compatible = "arm,realview-pb1176";
+       /* RealView Platform Baseboard for ARM11 MPCore */
+       compatible = "arm,realview-pb11mp";
+       /* RealView Platform Baseboard for Cortex A-8 */
+       compatible = "arm,realview-pba8";
+       /* RealView Platform Baseboard Explore for Cortex A-9 */
+       compatible = "arm,realview-pbx";
+
+Required nodes:
+
+- syscon: some node of the RealView platforms must have a
+  system controller node pointing to the control registers,
+  with the compatible string set to one of these tuples:
+   "arm,realview-eb-syscon", "syscon"
+   "arm,realview-pb1176-syscon", "syscon"
+   "arm,realview-pb11mp-syscon", "syscon"
+   "arm,realview-pba8-syscon", "syscon"
+   "arm,realview-pbx-syscon", "syscon"
+
+  Required properties for the system controller:
+  - regs: the location and size of the system controller registers,
+    one range of 0x1000 bytes.
+
+Example:
+
+/dts-v1/;
+#include <dt-bindings/interrupt-controller/irq.h>
+#include "skeleton.dtsi"
+
+/ {
+       model = "ARM RealView PB1176 with device tree";
+       compatible = "arm,realview-pb1176";
+
+       soc {
+               #address-cells = <1>;
+               #size-cells = <1>;
+               compatible = "simple-bus";
+               ranges;
+
+               syscon: syscon at 10000000 {
+                       compatible = "arm,realview-syscon", "syscon";
+                       reg = <0x10000000 0x1000>;
+               };
+
+       };
+};
Documentation/devicetree/bindings/arm/gic.txt +1 -0
index 5573c08d3180..539420194efd 100644
@@ -16,6 +16,7 @@ Main node required properties:
        "arm,cortex-a9-gic"
        "arm,cortex-a7-gic"
        "arm,arm11mp-gic"
+       "arm,arm1176jzf-gic"
 - interrupt-controller : Identifies the node as an interrupt controller
 - #interrupt-cells : Specifies the number of cells needed to encode an
   interrupt source.  The type shall be a <u32> and the value shall be 3.
arch/arm/boot/dts/Makefile +1 -0
index 377b7c364033..1419a70026da 100644
@@ -294,6 +294,7 @@ dtb-$(CONFIG_ARCH_PRIMA2) += prima2-evb.dtb
 dtb-$(CONFIG_ARCH_QCOM) += qcom-msm8660-surf.dtb \
        qcom-msm8960-cdp.dtb \
        qcom-apq8074-dragonboard.dtb
+dtb-$(CONFIG_ARCH_REALVIEW) += arm-realview-pb1176.dtb
 dtb-$(CONFIG_ARCH_S3C24XX) += s3c2416-smdk2416.dtb
 dtb-$(CONFIG_ARCH_S3C64XX) += s3c6410-mini6410.dtb \
        s3c6410-smdk6410.dtb
arch/arm/boot/dts/arm-realview-pb1176.dts new file +18 -0
new file mode 100644
index 000000000000..0eea0f4a7b39
@@ -0,0 +1,177 @@
+/*
+ * Copyright 2014 Linaro Ltd
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,

arch/arm/boot/dts/arm-realview-pb1176.dts line 28

Quoted 10 lines
arch/arm/boot/dts/arm-realview-pb1176.dts new file +10 -0
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include <dt-bindings/interrupt-controller/irq.h>
+#include "skeleton.dtsi"
+
+/ {
+       model = "ARM RealView PB1176 with device tree";

Isn't "with device tree" redundant?

Quoted 217 lines
arch/arm/boot/dts/arm-realview-pb1176.dts new file +149 -0
+       compatible = "arm,realview-pb1176";
+
+       chosen { };
+
+       aliases {
+               serial0 = &pb1176_serial0;
+               serial1 = &pb1176_serial1;
+               serial2 = &pb1176_serial2;
+               serial3 = &pb1176_serial3;
+       };
+
+       memory {
+               /* 128 MiB memory @ 0x0 */
+               reg = <0x00000000 0x08000000>;
+       };
+
+       xtal24mhz: xtal24mhz at 24M {
+               #clock-cells = <0>;
+               compatible = "fixed-clock";
+               clock-frequency = <24000000>;
+       };
+
+       timclk: timclk at 1M {
+               #clock-cells = <0>;
+               compatible = "fixed-factor-clock";
+               clock-div = <24>;
+               clock-mult = <1>;
+               clocks = <&xtal24mhz>;
+       };
+
+       uartclk: uartclk at 24M {
+               #clock-cells = <0>;
+               compatible = "fixed-factor-clock";
+               clock-div = <1>;
+               clock-mult = <1>;
+               clocks = <&xtal24mhz>;
+       };
+
+       /* FIXME: this actually hangs off the PLL clocks */
+       pclk: pclk at 0 {
+               #clock-cells = <0>;
+               compatible = "fixed-clock";
+               clock-frequency = <0>;
+       };
+
+       soc {
+               #address-cells = <1>;
+               #size-cells = <1>;
+               compatible = "simple-bus";
+               ranges;
+
+               syscon: syscon at 10000000 {
+                       compatible = "arm,realview-pb1176-syscon", "syscon";
+                       reg = <0x10000000 0x1000>;
+               };
+
+               /* Primary DevChip GIC synthesized with the CPU */
+               intc_dc1176: interrupt-controller at 10120000 {
+                       compatible = "arm,arm1176jzf-gic";
+                       #interrupt-cells = <3>;
+                       #address-cells = <1>;
+                       interrupt-controller;
+                       reg = <0x10121000 0x1000>,
+                             <0x10120000 0x100>;
+               };
+
+               /* This GIC on the board is cascaded off the DevChip GIC */
+               intc_pb1176: interrupt-controller at 10040000 {
+                       compatible = "arm,arm1176jzf-gic";
+                       #interrupt-cells = <3>;
+                       #address-cells = <1>;
+                       interrupt-controller;
+                       reg = <0x10041000 0x1000>,
+                             <0x10040000 0x100>;
+                       interrupt-parent = <&intc_dc1176>;
+                       interrupts = <0 31 IRQ_TYPE_LEVEL_HIGH>;
+               };
+
+               L2: l2-cache {
+                       compatible = "arm,l220-cache";
+                       reg = <0x10110000 0x1000>;
+                       interrupt-parent = <&intc_dc1176>;
+                       interrupts = <0 13 IRQ_TYPE_LEVEL_HIGH>;
+                       cache-unified;
+                       cache-level = <2>;
+               };
+
+               pmu {
+                       compatible = "arm,arm1176-pmu";
+                       interrupt-parent = <&intc_dc1176>;
+                       interrupts = <0 7 IRQ_TYPE_LEVEL_HIGH>;
+               };
+
+               timer01: timer at 10104000 {
+                       compatible = "arm,sp804", "arm,primecell";
+                       reg = <0x10104000 0x1000>;
+                       interrupt-parent = <&intc_dc1176>;
+                       interrupts = <0 8 IRQ_TYPE_LEVEL_HIGH>, <0 9 IRQ_TYPE_LEVEL_HIGH>;
+                       clocks = <&timclk>, <&timclk>, <&pclk>;
+                       clock-names = "timer1", "timer2", "apb_pclk";
+               };
+
+               timer23: timer at 10105000 {
+                       compatible = "arm,sp804", "arm,primecell";
+                       reg = <0x10105000 0x1000>;
+                       interrupt-parent = <&intc_dc1176>;
+                       interrupts = <0 10 IRQ_TYPE_LEVEL_HIGH>;
+                       arm,sp804-has-irq = <1>;
+                       clocks = <&timclk>, <&timclk>, <&pclk>;
+                       clock-names = "timer1", "timer2", "apb_pclk";
+               };
+
+               pb1176_serial0: uart at 1010c000 {
+                       compatible = "arm,pl011", "arm,primecell";
+                       reg = <0x1010c000 0x1000>;
+                       interrupt-parent = <&intc_dc1176>;
+                       interrupts = <0 18 IRQ_TYPE_LEVEL_HIGH>;
+                       clocks = <&uartclk>, <&pclk>;
+                       clock-names = "uartclk", "apb_pclk";
+               };
+
+               pb1176_serial1: uart at 1010d000 {
+                       compatible = "arm,pl011", "arm,primecell";
+                       reg = <0x1010d000 0x1000>;
+                       interrupt-parent = <&intc_dc1176>;
+                       interrupts = <0 19 IRQ_TYPE_LEVEL_HIGH>;
+                       clocks = <&uartclk>, <&pclk>;
+                       clock-names = "uartclk", "apb_pclk";
+               };
+
+               pb1176_serial2: uart at 1010e000 {
+                       compatible = "arm,pl011", "arm,primecell";
+                       reg = <0x1010e000 0x1000>;
+                       interrupt-parent = <&intc_dc1176>;
+                       interrupts = <0 20 IRQ_TYPE_LEVEL_HIGH>;
+                       clocks = <&uartclk>, <&pclk>;
+                       clock-names = "uartclk", "apb_pclk";
+               };
+
+               pb1176_serial3: uart at 1010f000 {
+                       compatible = "arm,pl011", "arm,primecell";
+                       reg = <0x1010f000 0x1000>;
+                       interrupt-parent = <&intc_dc1176>;
+                       interrupts = <0 21 IRQ_TYPE_LEVEL_HIGH>;
+                       clocks = <&uartclk>, <&pclk>;
+                       clock-names = "uartclk", "apb_pclk";
+               };
+       };
+};
arch/arm/mach-realview/Kconfig +10 -0
index 9db2029aa632..65a07f5b5225 100644
@@ -1,6 +1,16 @@
 menu "RealView platform type"
        depends on ARCH_REALVIEW
 
+config REALVIEW_DT
+       bool "Support RealView(R) Device Tree based boot"
+       select ARM_GIC
+       select MFD_SYSCON
+       select SOC_BUS
+       select USE_OF
+       help
+         Include support for booting the ARM(R) RealView(R) evaluation
+         boards using a device tree machine description.
+
 config MACH_REALVIEW_EB
        bool "Support RealView(R) Emulation Baseboard"
        select ARM_GIC
arch/arm/mach-realview/Makefile +1 -0
index 541fa4c109ef..e07fdf7ae8a7 100644
@@ -3,6 +3,7 @@
 #
 
 obj-y                                  := core.o
+obj-$(CONFIG_REALVIEW_DT)              += realview-dt.o
 obj-$(CONFIG_MACH_REALVIEW_EB)         += realview_eb.o
 obj-$(CONFIG_MACH_REALVIEW_PB11MP)     += realview_pb11mp.o
 obj-$(CONFIG_MACH_REALVIEW_PB1176)     += realview_pb1176.o
arch/arm/mach-realview/realview-dt.c new file +29 -0
new file mode 100644
index 000000000000..4ba7c5303d09
@@ -0,0 +1,225 @@
+/*
+ * Copyright (C) 2014 Linaro Ltd.
+ *
+ * Author: Linus Walleij <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2, as
+ * published by the Free Software Foundation.
+ *
+ */
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/arm-gic.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <linux/sys_soc.h>
+#include <linux/slab.h>
+#include <asm/mach/map.h>
+#include <asm/mach/arch.h>
+#include <asm/hardware/cache-l2x0.h>
+#include <mach/hardware.h>
+#include <mach/platform.h>
+#include <mach/board-eb.h>
+#include <mach/board-pb1176.h>
+#include <mach/board-pb11mp.h>

arch/arm/mach-realview/realview-dt.c line 39

Quoted 10 lines
arch/arm/mach-realview/realview-dt.c new file +10 -0
+#include <mach/board-pbx.h>
+#include "core.h"
+
+/* Pointer to the system controller */
+struct regmap *syscon_regmap;
+u8 syscon_type;
+
+static struct map_desc realview_dt_io_desc[] __initdata = {
+       {
+               /* FIXME: static map needed for LED driver */

Don't you have patches to fix this? I need them for Versatile conversion as well.

Quoted 51 lines
arch/arm/mach-realview/realview-dt.c new file +51 -0
+               .virtual        = IO_ADDRESS(REALVIEW_SYS_BASE),
+               .pfn            = __phys_to_pfn(REALVIEW_SYS_BASE),
+               .length         = SZ_4K,
+               .type           = MT_DEVICE,
+       },
+};
+
+static void __init realview_dt_map_io(void)
+{
+       iotable_init(realview_dt_io_desc, ARRAY_SIZE(realview_dt_io_desc));
+}
+
+/*
+ * We detect the different syscon types from the compatible strings.
+ */
+enum realview_syscon {
+       REALVIEW_SYSCON_EB,
+       REALVIEW_SYSCON_PB1176,
+       REALVIEW_SYSCON_PB11MP,
+       REALVIEW_SYSCON_PBA8,
+       REALVIEW_SYSCON_PBX,
+};
+
+static const struct of_device_id realview_syscon_match[] = {
+       {
+               .compatible = "arm,realview-eb-syscon",
+               .data = (void *)REALVIEW_SYSCON_EB,
+       },
+       {
+               .compatible = "arm,realview-pb1176-syscon",
+               .data = (void *)REALVIEW_SYSCON_PB1176,
+       },
+       {
+               .compatible = "arm,realview-pb11mp-syscon",
+               .data = (void *)REALVIEW_SYSCON_PB11MP,
+       },
+       {
+               .compatible = "arm,realview-pba8-syscon",
+               .data = (void *)REALVIEW_SYSCON_PBA8,
+       },
+       {
+               .compatible = "arm,realview-pbx-syscon",
+               .data = (void *)REALVIEW_SYSCON_PBX,
+       },
+};
+
+static void __init realview_dt_init_machine(void)
+{
+       struct device_node *root;
+       struct device_node *syscon;
+       const struct of_device_id *syscon_id;

arch/arm/mach-realview/realview-dt.c line 100

Quoted 10 lines
arch/arm/mach-realview/realview-dt.c new file +10 -0
+       struct soc_device *soc_dev;
+       struct soc_device_attribute *soc_dev_attr;
+       struct device *parent;
+       u32 coreid;
+       int ret;
+
+#if IS_ENABLED(CONFIG_CACHE_L2X0)
+       if (of_machine_is_compatible("arm,realview-eb"))
+               /*
+                * 1MB (128KB/way), 8-way associativity,

These should go into DT properties. Cache attributes are already defined in ePAPR.

arch/arm/mach-realview/realview-dt.c line 101

Quoted 1 line
arch/arm/mach-realview/realview-dt.c new file +1 -0
+                * evmon/parity/share enabled

These can probably be unconditionally enabled except for parity which could be a DT property.

Quoted 41 lines
arch/arm/mach-realview/realview-dt.c new file +41 -0
+                * Bits:  .... ...0 0111 1001 0000 .... .... ....
+                */
+               l2x0_of_init(0x00790000, 0xfe000fff);
+       else if (of_machine_is_compatible("arm,realview-pb1176"))
+               /*
+                * 128Kb (16Kb/way) 8-way associativity.
+                * evmon/parity/share enabled.
+                */
+               l2x0_of_init(0x00730000, 0xfe000fff);
+       else if (of_machine_is_compatible("arm,realview-pb11mp"))
+               /*
+                * 1MB (128KB/way), 8-way associativity,
+                * evmon/parity/share enabled
+                * Bits:  .... ...0 0111 1001 0000 .... .... ....
+                */
+               l2x0_of_init(0x00730000, 0xfe000fff);
+       else if (of_machine_is_compatible("arm,realview-pbx"))
+               /*
+                * 16KB way size, 8-way associativity, parity disabled
+                * Bits:  .. 0 0 0 0 1 00 1 0 1 001 0 000 0 .... .... ....
+                */
+               l2x0_of_init(0x02520000, 0xc0000fff);
+#endif
+
+        /* Here we create an SoC device for the root node */
+       root = of_find_node_by_path("/");
+       if (!root)
+               return;
+       syscon = of_find_matching_node_and_match(root, realview_syscon_match,
+                                                &syscon_id);
+       if (!syscon)
+               return;
+       syscon_type = (enum realview_syscon)syscon_id->data;
+
+       soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
+       if (!soc_dev_attr)
+               return;
+       ret = of_property_read_string(root, "compatible",
+                                     &soc_dev_attr->soc_id);
+       if (ret)
+               return;

arch/arm/mach-realview/realview-dt.c line 152

Quoted 10 lines
arch/arm/mach-realview/realview-dt.c new file +10 -0
+       ret = of_property_read_string(root, "model", &soc_dev_attr->machine);
+        if (ret)
+               return;
+       soc_dev_attr->family = "RealView";
+       soc_dev = soc_device_register(soc_dev_attr);
+       if (IS_ERR(soc_dev)) {
+               kfree(soc_dev_attr);
+               return;
+       }
+       parent = soc_device_to_device(soc_dev);

We really need to standardize this soc device stuff so that platforms always have a parent soc device or not.

Quoted 43 lines
arch/arm/mach-realview/realview-dt.c new file +43 -0
+       ret = of_platform_populate(root, of_default_bus_match_table,
+                                  NULL, parent);
+       if (ret) {
+               pr_crit("could not populate device tree\n");
+               return;
+       }
+
+       syscon_regmap = syscon_node_to_regmap(syscon);
+       if (IS_ERR(syscon_regmap)) {
+               pr_crit("could not locate syscon regmap\n");
+               return;
+       }
+       ret = regmap_read(syscon_regmap, REALVIEW_SYS_ID_OFFSET, &coreid);
+       if (ret)
+               return;
+       pr_info("RealView Syscon type %02x, Core ID: 0x%08x\n",
+               syscon_type, coreid);
+       /* FIXME: add attributes for SoC to sysfs */
+}
+
+static void realview_dt_restart(enum reboot_mode mode, const char *cmd)
+{
+       if (IS_ERR(syscon_regmap))
+               return;
+
+       /* Unlock the reset register */
+       regmap_write(syscon_regmap, REALVIEW_SYS_LOCK_OFFSET,
+                    REALVIEW_SYS_LOCK_VAL);
+       /* Then hit reset on the different machines */
+       switch (syscon_type) {
+       case REALVIEW_SYSCON_EB:
+               regmap_write(syscon_regmap,
+                            REALVIEW_SYS_RESETCTL_OFFSET, 0x0008);
+               break;
+       case REALVIEW_SYSCON_PB1176:
+               regmap_write(syscon_regmap,
+                            REALVIEW_SYS_RESETCTL_OFFSET, 0x0100);
+               break;
+       case REALVIEW_SYSCON_PB11MP:
+       case REALVIEW_SYSCON_PBA8:
+               regmap_write(syscon_regmap, REALVIEW_SYS_RESETCTL_OFFSET,
+                            0x0000);
+               regmap_write(syscon_regmap, REALVIEW_SYS_RESETCTL_OFFSET,

arch/arm/mach-realview/realview-dt.c line 205

Quoted 10 lines
arch/arm/mach-realview/realview-dt.c new file +10 -0
+                            0x0004);
+               break;
+       case REALVIEW_SYSCON_PBX:
+               regmap_write(syscon_regmap, REALVIEW_SYS_RESETCTL_OFFSET,
+                            0x00f0);
+               regmap_write(syscon_regmap, REALVIEW_SYS_RESETCTL_OFFSET,
+                            0x00f4);
+               break;
+       }
+       dsb();

Barriers need comments as to why they are needed.

Rob

#22 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 5 lines · Rob Herring
Quoted 1 line · Linus Walleij

+ model = "ARM RealView PB1176 with device tree";

Isn't "with device tree" redundant?

Nuked it.

Quoted 6 lines

arch/arm/mach-realview/realview-dt.c line 39

Quoted 3 lines
arch/arm/mach-realview/realview-dt.c new file +3 -0
+static struct map_desc realview_dt_io_desc[] __initdata = {
+       {
+               /* FIXME: static map needed for LED driver */

Don't you have patches to fix this? I need them for Versatile conversion as well.

Done away with. The legacy driver was moved to drivers/leds/leds-versatile.c and I then contemplated adding DT support to it but it was a *bad* idea.

Instead I have created drivers/leds/leds-syscon.c that can handle any LEDs in a syscon. Will post in next iteration as soon as I get the drivers/soc dependency in place.

Quoted 7 lines

arch/arm/mach-realview/realview-dt.c line 100

Quoted 4 lines
arch/arm/mach-realview/realview-dt.c new file +4 -0
+#if IS_ENABLED(CONFIG_CACHE_L2X0)
+       if (of_machine_is_compatible("arm,realview-eb"))
+               /*
+                * 1MB (128KB/way), 8-way associativity,

These should go into DT properties. Cache attributes are already defined in ePAPR.

Yes, the dependencies to do this with DT is floating around somewhere in the ARM tree or Russells tree I think, maybe I cannot do this until the next kernel cycle.

Quoted 4 lines
Quoted 1 line

+ * evmon/parity/share enabled

These can probably be unconditionally enabled except for parity which could be a DT property.

I'll wait and see what I need to add really.

Quoted 4 lines
Quoted 1 line

+ parent = soc_device_to_device(soc_dev);

We really need to standardize this soc device stuff so that platforms always have a parent soc device or not.

Arnds position is that the entire platform/board should not be a "soc device" and that seems correct since the board is no SoC.

So I will spin this off into a SoC driver and push under drivers/soc and bind to the SoC node in the device tree.

Yours,
Linus Walleij

maintainer
#23 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

Quoted 10 lines · On Thursday 22 May 2014 15:30:39 Linus Walleij wrote:
Quoted 4 lines
Quoted 1 line

+ parent = soc_device_to_device(soc_dev);

We really need to standardize this soc device stuff so that platforms always have a parent soc device or not.

Arnds position is that the entire platform/board should not be a "soc device" and that seems correct since the board is no SoC.

So I will spin this off into a SoC driver and push under drivers/soc and bind to the SoC node in the device tree.

To be clear, this has nothing to do with whether this is strictly a SoC or not. I would still want to see all the internal components of the main chip under the soc node, and probably anything that is attached to one of its interfaces under those nodes. The parts that should not be under /soc are the ones that are not in the device hierarchy, such as external clock generators.

Arnd

maintainer
#24 lore raw
Quoted 12 lines · Arnd Bergmann
Quoted 10 lines · On Thursday 22 May 2014 15:30:39 Linus Walleij wrote:
Quoted 4 lines
Quoted 1 line

+ parent = soc_device_to_device(soc_dev);

We really need to standardize this soc device stuff so that platforms always have a parent soc device or not.

Arnds position is that the entire platform/board should not be a "soc device" and that seems correct since the board is no SoC.

So I will spin this off into a SoC driver and push under drivers/soc and bind to the SoC node in the device tree.

Quoted 6 lines

To be clear, this has nothing to do with whether this is strictly a SoC or not. I would still want to see all the internal components of the main chip under the soc node, and probably anything that is attached to one of its interfaces under those nodes. The parts that should not be under /soc are the ones that are not in the device hierarchy, such as external clock generators.

Pretty much everything is an SOC these days. And there is really no way to tell what is the SOC boundary in DT. Perhaps it is just all devices handled by the top-level of_platform_populate call.

If this is the route we want to go, I don't think it should be left to the whim of the platforms. We should have some sort of default/generic SOC device until a more specific device is added or SOCs just add data to the generic device instead of making their own. With Pawel's vexpress clean-up that just got pulled into arm-soc for 3.16, we can now call of_platform_populate multiple times on the DT root. This means we can get rid of most of_platform_populate calls in platforms.

#25 lore raw

PATCH v2 1/1 ARM: realview: basic device tree implementation

In reply to Linus Walleij
Quoted 52 lines · Linus Walleij

This implements basic device tree boot support for the RealView platforms, with a basic device tree for ARM PB1176 as an example.

The implementation is done with a new DT-specific board file using only pre-existing bindings for the basic IRQ, timer and serial port drivers. A new compatible type is added to the GIC for the ARM1176.

This implementation uses the MFD syscon handle from day one to access the system controller registers, and register the devices using the SoC bus.

Cc Arnd Bergmann <[email protected]>
Cc Rob Herring <[email protected]>

---
ChangeLog v1->v2:
- Adjust timer clock names to be the same as the example in the device tree binding.
- Remove all memory fixup code - this should be handled by the device tree specification of memory areas or by special MM hacks for the RealView PBX.
- Fix the documentation around syscon to specify that it can be in any node, need not be the root node.
- Switch device tree license to the BSD license, taken from arch/powerpc/boot/dts/p1024rdb_32b.dts
- Add a hunk for the new compatible string to Documentation/devicetree/bindings/arm/gic.txt
- Move the clocks out of the SoC node, certainly the xtal is not sitting on the SoC...
- Sort the selects in Kconfig alphabetically
- Use IS_ENABLED() for the l2x0 code snippet
- Instead of checking the board variant in the reset routine to figure out how to tweak the reset controller, have a compatible string for each syscon variant, map it to an enum that provides a unique type ID and that way figure out how to handle it in a maybe more elegant way.
- Open issue: what do to with the l2x0 stuff?

---
 Documentation/devicetree/bindings/arm/arm-boards |  57 ++++++
 Documentation/devicetree/bindings/arm/gic.txt    |   1 +
 arch/arm/boot/dts/Makefile                       |   1 +
 arch/arm/boot/dts/arm-realview-pb1176.dts        | 177 ++++++++++++++++++
 arch/arm/mach-realview/Kconfig                   |  10 +
 arch/arm/mach-realview/Makefile                  |   1 +
 arch/arm/mach-realview/realview-dt.c             | 225 +++++++++++++++++++++++
 arch/arm/mm/Kconfig                              |   2 +-
 drivers/irqchip/irq-gic.c                        |   1 +
 9 files changed, 474 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/arm-realview-pb1176.dts
 create mode 100644 arch/arm/mach-realview/realview-dt.c

...

Quoted 12 lines
Documentation/devicetree/bindings/arm/gic.txt +1 -0
index 5573c08d3180..539420194efd 100644
@@ -16,6 +16,7 @@ Main node required properties:
 	"arm,cortex-a9-gic"
 	"arm,cortex-a7-gic"
 	"arm,arm11mp-gic"
+	"arm,arm1176jzf-gic"
 - interrupt-controller : Identifies the node as an interrupt controller
 - #interrupt-cells : Specifies the number of cells needed to encode an
   interrupt source.  The type shall be a <u32> and the value shall be 3.

...

drivers/irqchip/irq-gic.c gic_of_init(struct device_node *node, struct device_node *parent) · line 1078

Quoted 11 lines
drivers/irqchip/irq-gic.c +1 -0
index 57d165e026f4..70281b38c73b 100644
@@ -1073,6 +1073,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
 }
 IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
 IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
+IRQCHIP_DECLARE(arm1176jzf_gic, "arm,arm1176jzf-gic", gic_of_init);
 IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
 IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);

Please place in alphabetical order. We're starting to collect compatible strings for this driver...

thx,

Jason.