Versatile Express DT support

Try to map TWD registers basing on a "arm,smp-twd" Device Tree node (compatible value as used in Highbank's DT). This overrides existing twd_base value.

Signed-off-by Pawel Moll <[email protected]>

---
 arch/arm/plat-versatile/localtimer.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

arch/arm/plat-versatile/localtimer.c +2 -0

index 0fb3961..8f0dc10 100644
@@ -11,6 +11,8 @@
 #include <linux/init.h>
 #include <linux/smp.h>
 #include <linux/clockchips.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
 
 #include <asm/smp_twd.h>
 #include <asm/localtimer.h>

Quoted 7 lines

arch/arm/plat-versatile/localtimer.c +3 -0

@@ -21,6 +23,16 @@
  */
 int __cpuinit local_timer_setup(struct clock_event_device *evt)
 {
+#if defined(CONFIG_OF)
+	struct device_node *node = of_find_compatible_node(NULL,
+			NULL, "arm,smp-twd");

arch/arm/plat-versatile/localtimer.c line 32

Quoted 4 lines

arch/arm/plat-versatile/localtimer.c +4 -0

+
+	if (node)
+		twd_base = of_iomap(node, 0);
+#endif

I think your previous version was more correct. This is going to find the node and do ioremap N times where N is the number of cores. It does work though because that is what I did initially too.

Quoted 3 lines

I think your previous version was more correct. This is going to find the node and do ioremap N times where N is the number of cores. It does work though because that is what I did initially too.

Right, how about that, then:

arch/arm/plat-versatile/localtimer.c +9 -0

@@ -21,6 +23,22 @@
  */
 int __cpuinit local_timer_setup(struct clock_event_device *evt)
 {
+#if defined(CONFIG_OF)
+       static int dt_node_probed;
+
+       if (!dt_node_probed) {
+               struct device_node *node = of_find_compatible_node(NULL,
+                               NULL, "arm,smp-twd");
+
+               if (node)
+                       twd_base = of_iomap(node, 0);

This patch adds generic Versatile Express DT machine description, Device Tree description for the motherboard and documentation for the bindings.

arch/arm/mach-vexpress/v2m.c +0 -0

This patch adds support for RS1 memory map based Versatile Express motherboard.

arch/arm/mach-vexpress/include/mach/debug-macro.S +11 -4

@@ -10,12 +10,41 @@
  * published by the Free Software Foundation.
  */
 
-#define DEBUG_LL_UART_OFFSET	0x00009000
+#define DEBUG_LL_PHYS_BASE		0x10000000
+#define DEBUG_LL_UART_OFFSET		0x00009000
+
+#define DEBUG_LL_PHYS_BASE_RS1		0x1c000000
+#define DEBUG_LL_UART_OFFSET_RS1	0x00090000
+
+#define DEBUG_LL_VIRT_BASE		0xf8000000
 
 		.macro	addruart,rp,rv,tmp
-		mov	\rp, #DEBUG_LL_UART_OFFSET
-		orr	\rv, \rp, #0xf8000000	@ virtual base
-		orr	\rp, \rp, #0x10000000	@ physical base
+
+		@ Check the MMU state
+#if defined(CONFIG_MMU)
+		mrc	p15, 0, \tmp, c1, c0	@ SCTRL

arch/arm/mach-vexpress/include/mach/debug-macro.S +5 -0

+		teq	\tmp, #0x11		@ PL011
+
+		@ Original memory map
+		moveq	\rp, #DEBUG_LL_UART_OFFSET
+		orreq	\rv, \rp, #DEBUG_LL_VIRT_BASE

Quoted 3 lines · Pawel Moll

This patch adds support for RS1 memory map based Versatile Express motherboard.

[...]

Quoted 28 lines

arch/arm/mach-vexpress/include/mach/debug-macro.S +16 -4

@@ -10,12 +10,41 @@
  * published by the Free Software Foundation.
  */
 
-#define DEBUG_LL_UART_OFFSET	0x00009000
+#define DEBUG_LL_PHYS_BASE		0x10000000
+#define DEBUG_LL_UART_OFFSET		0x00009000
+
+#define DEBUG_LL_PHYS_BASE_RS1		0x1c000000
+#define DEBUG_LL_UART_OFFSET_RS1	0x00090000
+
+#define DEBUG_LL_VIRT_BASE		0xf8000000
 
 		.macro	addruart,rp,rv,tmp
-		mov	\rp, #DEBUG_LL_UART_OFFSET
-		orr	\rv, \rp, #0xf8000000	@ virtual base
-		orr	\rp, \rp, #0x10000000	@ physical base
+
+		@ Check the MMU state
+#if defined(CONFIG_MMU)
+		mrc	p15, 0, \tmp, c1, c0	@ SCTRL
+		tst	\tmp, #1		@ MMU enabled?
+		moveq	\tmp, #DEBUG_LL_PHYS_BASE
+		movne	\tmp, #DEBUG_LL_VIRT_BASE
+#else
+		mov	\tmp, #DEBUG_LL_PHYS_BASE

arch/arm/mach-vexpress/include/mach/debug-macro.S line 35

Quoted 5 lines

arch/arm/mach-vexpress/include/mach/debug-macro.S +5 -0

+#endif
+
+		@ PL011 present in "original" place?
+		orr	\tmp, \tmp, #DEBUG_LL_UART_OFFSET
+		ldr	\tmp, [\tmp, #0xfe0]	@ PeriphID0

This doesn't work with CONFIG_EARLY_PRINTK=y on a system with the RS1 memory map.

__create_page_tables has only mapped the single physical
page at 0x1c090000 and thus the test for the UART in the other memory map faults.

Hi,

Sorry about loooong delay - I've been on holiday.

Quoted 33 lines · David Vrabel

Quoted 3 lines · Pawel Moll

This patch adds support for RS1 memory map based Versatile Express motherboard.

[...]

Quoted 28 lines

arch/arm/mach-vexpress/include/mach/debug-macro.S +16 -4

@@ -10,12 +10,41 @@
  * published by the Free Software Foundation.
  */
 
-#define DEBUG_LL_UART_OFFSET	0x00009000
+#define DEBUG_LL_PHYS_BASE		0x10000000
+#define DEBUG_LL_UART_OFFSET		0x00009000
+
+#define DEBUG_LL_PHYS_BASE_RS1		0x1c000000
+#define DEBUG_LL_UART_OFFSET_RS1	0x00090000
+
+#define DEBUG_LL_VIRT_BASE		0xf8000000
 
 		.macro	addruart,rp,rv,tmp
-		mov	\rp, #DEBUG_LL_UART_OFFSET
-		orr	\rv, \rp, #0xf8000000	@ virtual base
-		orr	\rp, \rp, #0x10000000	@ physical base
+
+		@ Check the MMU state
+#if defined(CONFIG_MMU)
+		mrc	p15, 0, \tmp, c1, c0	@ SCTRL
+		tst	\tmp, #1		@ MMU enabled?
+		moveq	\tmp, #DEBUG_LL_PHYS_BASE
+		movne	\tmp, #DEBUG_LL_VIRT_BASE
+#else
+		mov	\tmp, #DEBUG_LL_PHYS_BASE

Quoted 8 lines

arch/arm/mach-vexpress/include/mach/debug-macro.S line 35

Quoted 5 lines

arch/arm/mach-vexpress/include/mach/debug-macro.S +5 -0

+#endif
+
+		@ PL011 present in "original" place?
+		orr	\tmp, \tmp, #DEBUG_LL_UART_OFFSET
+		ldr	\tmp, [\tmp, #0xfe0]	@ PeriphID0

This doesn't work with CONFIG_EARLY_PRINTK=y on a system with the RS1 memory map.

It does for me:

# zcat /proc/config.gz | grep EARLY_PRINTK
CONFIG_EARLY_PRINTK=y
# cat /proc/device-tree/motherboard/arm,v2m-memory-map && echo
rs1     
#

Can you tell me what exactly is going wrong in your case? Does it hang without any warning? Do you get at least part of the boot log? Can you send me (privately probably) your kernel config?

Quoted 3 lines

__create_page_tables has only mapped the single physical
page at 0x1c090000 and thus the test for the UART in the other memory map faults.

I investigated this when writing the code and I vaguely remember it was fine, partly by accident. I'll dig in again and let you know my findings.

Quoted 7 lines

It does for me:

# zcat /proc/config.gz | grep EARLY_PRINTK
CONFIG_EARLY_PRINTK=y
# cat /proc/device-tree/motherboard/arm,v2m-memory-map && echo
rs1     
#

earlyprintk needs to be on the kernel command line to enable it. Without this option it will work fine.

It's also a problem when running as a guest under a hypervisor as there won't be any stage 2 translation table entries for non-existent peripherals.

I think there needs to be someway of finding out from the DTB which UART to use.

Quoted 3 lines

Quoted 3 lines

__create_page_tables has only mapped the single physical
page at 0x1c090000 and thus the test for the UART in the other memory map faults.

Quoted 3 lines · David Vrabel

Quoted 3 lines

Quoted 3 lines

__create_page_tables has only mapped the single physical
page at 0x1c090000 and thus the test for the UART in the other memory map faults.

I got to the bottom of the problem... The mapping created by the DEBUG_LL code in __create_page_tables is actually a section (1MB) so covers both possible accesses (0xf8009000 and 0xf8090000). The difference between model and hardware is that "real" RS1 VE has a DAP ROM located between 0xf8000000 and 0xf800ffff so the 0xf8009000 succeeds (returning some irrelevant data), while model is "empty" in that space, so it faults.

Anyway, I have an idea how to solve (or rather work around) the problem and will get if working on models in the next version of the series. I'll send you a modified version of the relevant patch before that so you can test it.

Thanks for your time!

Hi,

Sorry about loooong delay - I've been on holiday.

Quoted 33 lines · David Vrabel

Quoted 3 lines · Pawel Moll

This patch adds support for RS1 memory map based Versatile Express motherboard.

[...]

Quoted 28 lines

arch/arm/mach-vexpress/include/mach/debug-macro.S +16 -4

@@ -10,12 +10,41 @@
  * published by the Free Software Foundation.
  */
 
-#define DEBUG_LL_UART_OFFSET	0x00009000
+#define DEBUG_LL_PHYS_BASE		0x10000000
+#define DEBUG_LL_UART_OFFSET		0x00009000
+
+#define DEBUG_LL_PHYS_BASE_RS1		0x1c000000
+#define DEBUG_LL_UART_OFFSET_RS1	0x00090000
+
+#define DEBUG_LL_VIRT_BASE		0xf8000000
 
 		.macro	addruart,rp,rv,tmp
-		mov	\rp, #DEBUG_LL_UART_OFFSET
-		orr	\rv, \rp, #0xf8000000	@ virtual base
-		orr	\rp, \rp, #0x10000000	@ physical base
+
+		@ Check the MMU state
+#if defined(CONFIG_MMU)
+		mrc	p15, 0, \tmp, c1, c0	@ SCTRL
+		tst	\tmp, #1		@ MMU enabled?
+		moveq	\tmp, #DEBUG_LL_PHYS_BASE
+		movne	\tmp, #DEBUG_LL_VIRT_BASE
+#else
+		mov	\tmp, #DEBUG_LL_PHYS_BASE

Quoted 8 lines

arch/arm/mach-vexpress/include/mach/debug-macro.S line 35

Quoted 5 lines

arch/arm/mach-vexpress/include/mach/debug-macro.S +5 -0

+#endif
+
+		@ PL011 present in "original" place?
+		orr	\tmp, \tmp, #DEBUG_LL_UART_OFFSET
+		ldr	\tmp, [\tmp, #0xfe0]	@ PeriphID0

This doesn't work with CONFIG_EARLY_PRINTK=y on a system with the RS1 memory map.

It does for me:

# zcat /proc/config.gz | grep EARLY_PRINTK
CONFIG_EARLY_PRINTK=y
# cat /proc/device-tree/motherboard/arm,v2m-memory-map && echo
rs1     
#

Can you tell me what exactly is going wrong in your case? Does it hang without any warning? Do you get at least part of the boot log? Can you send me (privately probably) your kernel config?

This patch adds Device Tree file for the CoreTile Express A15x2 (V2P-CA15) with Test Chip 1.

As the chip's GIC has 160 interrupt inputs and equivalent SMM (FPGA) has GIC synthesised with 256 interrupts, NR_IRQS is increased.

arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts new file +0 -0

From 22cc6a3410077b99d81e42c4d7f4e29eab6070e3 Mon Sep 17 00:00:00 2001

Quoted 2 lines · Pawel Moll

This patch adds Device Tree file for the CoreTile Express A15x2 (V2P-CA15) with Test Chip 1.

This doesn't work as-is with the software model as accessing some of the peripherals that aren't modeled will cause an exception. Is it worth having a device tree file suitable for the models? Or are the models too configurable for this to be workable?

Quoted 4 lines

As the chip's GIC has 160 interrupt inputs and equivalent SMM (FPGA) has GIC synthesised with 256 interrupts, NR_IRQS is increased.

[...]

Quoted 2 lines

arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts new file +0 -0

[...]

arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts

Quoted 4 lines

arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts new file +4 -0

+	memory at 80000000 {
+		device_type = "memory";
+		reg = <0x80000000 0x40000000>;
+	};

If CONFIG_ARM_ATAG_DTB_COMPAT is enabled the device tree will end up with two nodes describing the memory ("memory" and "memory at 80000000" in this case).

Quoted 8 lines · David Vrabel

Quoted 2 lines · Pawel Moll

This patch adds Device Tree file for the CoreTile Express A15x2 (V2P-CA15) with Test Chip 1.

This doesn't work as-is with the software model as accessing some of the peripherals that aren't modeled will cause an exception. Is it worth having a device tree file suitable for the models? Or are the models too configurable for this to be workable?

The model as you have it doesn't exactly represent the board for a number of reasons, mainly because there was no hardware design when the model was created, so some of the solution was best-guessed by the model people. Anyway, current A15 model can't be considered a 1-to-1 equivalent of the VE board. The plan is that the models will be shipped with their own DTSes. I'll work on that in the following months, I can keep you updated (and use as a beta tester ;-) if you want.

Quoted 8 lines

Quoted 4 lines

As the chip's GIC has 160 interrupt inputs and equivalent SMM (FPGA) has GIC synthesised with 256 interrupts, NR_IRQS is increased.

[...]

Quoted 2 lines

arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts new file +0 -0

[...]

Quoted 8 lines

arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts

Quoted 4 lines

arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts new file +4 -0

+	memory at 80000000 {
+		device_type = "memory";
+		reg = <0x80000000 0x40000000>;
+	};

If CONFIG_ARM_ATAG_DTB_COMPAT is enabled the device tree will end up with two nodes describing the memory ("memory" and "memory at 80000000" in this case).

You're right - the skeleton.dtsi contains "memory" mode... Funnily enough originally I was using that name, but then Rob Herring suggested changing it to @80000000, which seemed reasonable.

Now I wonder - is the "memory" node special and should not contain "@address", or the skelton shouldn't contain the empty "memory" node...

Quoted 7 lines

You're right - the skeleton.dtsi contains "memory" mode... Funnily enough originally I was using that name, but then Rob Herring suggested changing it to @80000000, which seemed reasonable.

Now I wonder - is the "memory" node special and should not contain "@address", or the skelton shouldn't contain the empty "memory" node...

Hummm... I guess you should just use "memory" if you are using skeleton.dtsi.

Quoted 8 lines · Rob Herring

Quoted 7 lines

You're right - the skeleton.dtsi contains "memory" mode... Funnily enough originally I was using that name, but then Rob Herring suggested changing it to @80000000, which seemed reasonable.

Now I wonder - is the "memory" node special and should not contain "@address", or the skelton shouldn't contain the empty "memory" node...

Quoted 2 lines

Hummm... I guess you should just use "memory" if you are using skeleton.dtsi.

Well, I don't mind _not_ using skeleton, but I had an impression the general policy was to use it?

Quoted 9 lines · Pawel Moll

Quoted 8 lines · Rob Herring

Quoted 7 lines

You're right - the skeleton.dtsi contains "memory" mode... Funnily enough originally I was using that name, but then Rob Herring suggested changing it to @80000000, which seemed reasonable.

Now I wonder - is the "memory" node special and should not contain "@address", or the skelton shouldn't contain the empty "memory" node...

Quoted 5 lines

Quoted 2 lines

Hummm... I guess you should just use "memory" if you are using skeleton.dtsi.

Well, I don't mind _not_ using skeleton, but I had an impression the general policy was to use it?

Either way is fine. I don't really think it buys you much.

Quoted 13 lines · Rob Herring

Quoted 12 lines · Pawel Moll

Quoted 10 lines · Rob Herring

Quoted 7 lines

You're right - the skeleton.dtsi contains "memory" mode... Funnily enough originally I was using that name, but then Rob Herring suggested changing it to @80000000, which seemed reasonable.

Now I wonder - is the "memory" node special and should not contain "@address", or the skelton shouldn't contain the empty "memory" node...

Hummm... I guess you should just use "memory" if you are using skeleton.dtsi.

Quoted 4 lines

Quoted 2 lines

Well, I don't mind _not_ using skeleton, but I had an impression the general policy was to use it?

Either way is fine. I don't really think it buys you much.

Ok, /include/ "skeleton.dtsi" is gone then :-)

I don't expect any real production vexpress system to use this config options

The problem wasn't with including skeleton.dtsi.

With
CONFIG_ARM_ATAG_DTB_COMPAT the zImage decompressor modifies the appended DTB using information from the ATAGs (see atags_to_fdt()).

If there's an ATAG giving the amount of RAM the DTB's "memory" node is replaced with a new one. Since the vexpress DTBs don't have a "memory" node it's added and the DTB ends up with two nodes describing memory.

Quoted 1 line · David Vrabel

The problem wasn't with including skeleton.dtsi.

Including as it is creates two device_type="memory" nodes, one with regs=<0 0>, which is definitely wrong.

Quoted 4 lines

With
CONFIG_ARM_ATAG_DTB_COMPAT the zImage decompressor modifies the appended DTB using information from the ATAGs (see atags_to_fdt()).

Quoted 3 lines

If there's an ATAG giving the amount of RAM the DTB's "memory" node is replaced with a new one. Since the vexpress DTBs don't have a "memory" node it's added and the DTB ends up with two nodes describing memory.

The "memory at address" node name is in my opinion perfectly legal - p. 3.4 of the DT spec says "The name component of the node name (see 2.2.1) shall be memory.". So the decompressor code may be wrong in looking for adress-less "memory" node...

Quoted 10 lines · Pawel Moll

Quoted 1 line · David Vrabel

The problem wasn't with including skeleton.dtsi.

Including as it is creates two device_type="memory" nodes, one with regs=<0 0>, which is definitely wrong.

Quoted 4 lines

With
CONFIG_ARM_ATAG_DTB_COMPAT the zImage decompressor modifies the appended DTB using information from the ATAGs (see atags_to_fdt()).

Quoted 8 lines

Quoted 3 lines

If there's an ATAG giving the amount of RAM the DTB's "memory" node is replaced with a new one. ?Since the vexpress DTBs don't have a "memory" node it's added and the DTB ends up with two nodes describing memory.

The "memory at address" node name is in my opinion perfectly legal - p. 3.4 of the DT spec says "The name component of the node name (see 2.2.1) shall be memory.". So the decompressor code may be wrong in looking for adress-less "memory" node...

I don't think you can expect such early code to properly parse a DT tree with a variability in how memory stuff is declared into that DT tree.

What if you have two memory nodes specified in the DT file, and the ATAG data contains one?

The more I look at this, the more I'm convinced that Grant's idea that DT should entirely override ATAGs all the way to the kernel proper was the wrong solution - at least in the kernel, if we had both available, we could make a choice there, and have the full DT library to be able to manipulate the DT blob.

Unfortunately, that's not so, so we're just going to have to accept that on ARM it should be "memory" if we want the ATAG code to override it. ?Expecting the decompressor to figure out that it needs to delete DT nodes and all that I think is asking far too much.

Quoted 20 lines · Russell King - ARM Linux

Quoted 10 lines · Pawel Moll

Quoted 1 line · David Vrabel

The problem wasn't with including skeleton.dtsi.

Including as it is creates two device_type="memory" nodes, one with regs=<0 0>, which is definitely wrong.

Quoted 4 lines

With
CONFIG_ARM_ATAG_DTB_COMPAT the zImage decompressor modifies the appended DTB using information from the ATAGs (see atags_to_fdt()).

Quoted 8 lines

Quoted 3 lines

If there's an ATAG giving the amount of RAM the DTB's "memory" node is replaced with a new one. ?Since the vexpress DTBs don't have a "memory" node it's added and the DTB ends up with two nodes describing memory.

The "memory at address" node name is in my opinion perfectly legal - p. 3.4 of the DT spec says "The name component of the node name (see 2.2.1) shall be memory.". So the decompressor code may be wrong in looking for adress-less "memory" node...

Quoted 5 lines

I don't think you can expect such early code to properly parse a DT tree with a variability in how memory stuff is declared into that DT tree.

What if you have two memory nodes specified in the DT file, and the ATAG data contains one?

Yes, just because it is technically legal doesn't make it okay. The pragmatic approach here is that the skeleton.dtsi file calls the node "memory", so this .dts file must do the same.

Quoted 5 lines

The more I look at this, the more I'm convinced that Grant's idea that DT should entirely override ATAGs all the way to the kernel proper was the wrong solution - at least in the kernel, if we had both available, we could make a choice there, and have the full DT library to be able to manipulate the DT blob.

Hey! I was originally lobbying for the dt pointer carried by an ATAG.
Nico conviced me otherwise. :-)

Quoted 32 lines · Grant Likely

Quoted 20 lines · Russell King - ARM Linux

Quoted 10 lines · Pawel Moll

Quoted 1 line · David Vrabel

The problem wasn't with including skeleton.dtsi.

Including as it is creates two device_type="memory" nodes, one with regs=<0 0>, which is definitely wrong.

Quoted 4 lines

With
CONFIG_ARM_ATAG_DTB_COMPAT the zImage decompressor modifies the appended DTB using information from the ATAGs (see atags_to_fdt()).

Quoted 8 lines

Quoted 3 lines

If there's an ATAG giving the amount of RAM the DTB's "memory" node is replaced with a new one. ?Since the vexpress DTBs don't have a "memory" node it's added and the DTB ends up with two nodes describing memory.

The "memory at address" node name is in my opinion perfectly legal - p. 3.4 of the DT spec says "The name component of the node name (see 2.2.1) shall be memory.". So the decompressor code may be wrong in looking for adress-less "memory" node...

Quoted 5 lines

I don't think you can expect such early code to properly parse a DT tree with a variability in how memory stuff is declared into that DT tree.

What if you have two memory nodes specified in the DT file, and the ATAG data contains one?

Yes, just because it is technically legal doesn't make it okay. ?The pragmatic approach here is that the skeleton.dtsi file calls the node "memory", so this .dts file must do the same.

Quoted 8 lines

Quoted 5 lines

The more I look at this, the more I'm convinced that Grant's idea that DT should entirely override ATAGs all the way to the kernel proper was the wrong solution - at least in the kernel, if we had both available, we could make a choice there, and have the full DT library to be able to manipulate the DT blob.

Hey! ?I was originally lobbying for the dt pointer carried by an ATAG. ?Nico conviced me otherwise. ?:-)

Hey! ?I was originally lobbying for people to have a fully DT aware bootloader if they wanted to play with DT, otherwise there is no incentive for updated bootloaders. ?But someone else convinced me otherwise. ?:-)

Mixed bags always have loose ends.

Quoted 1 line

Ok, /include/ "skeleton.dtsi" is gone then :-)

The problem wasn't with including skeleton.dtsi. With CONFIG_ARM_ATAG_DTB_COMPAT the zImage decompressor modifies the appended DTB using information from the ATAGs (see atags_to_fdt()).

If there's an ATAG giving the amount of RAM the DTB's "memory" node is replaced with a new one. Since the vexpress DTBs don't have a "memory" node it's added and the DTB ends up with two nodes describing memory.

Quoted 9 lines · David Vrabel

Quoted 1 line

Ok, /include/ "skeleton.dtsi" is gone then :-)

The problem wasn't with including skeleton.dtsi. With CONFIG_ARM_ATAG_DTB_COMPAT the zImage decompressor modifies the appended DTB using information from the ATAGs (see atags_to_fdt()).

If there's an ATAG giving the amount of RAM the DTB's "memory" node is replaced with a new one. Since the vexpress DTBs don't have a "memory" node it's added and the DTB ends up with two nodes describing memory.

As it turned out it was just the "skeleton.dtsi" problem after all - I mean the fact that there where two device_type="memory" nodes in the tree.

The decompressor's setprop() (arch/arm/boot/compressed/atags_to_fdt.c:12) uses libfdt's fdt_setprop(), which correctly ignores the "@00000000" component of the node name and sets the reg property as expected. So as long as there is exactly one "memory[@address]" node in the tree, CONFIG_ARM_ATAG_DTB_COMPAT is happy.

I will remove the /include/ from the dts files for VE (see below) in the v3.3-rc1 based series.

Thanks for spotting this!

Pawe??

arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts +4 -2

index 02cada5..2a690f2 100644
@@ -9,13 +9,15 @@
 
 /dts-v1/;
 
-/include/ "skeleton.dtsi"
-
 / {
 	model = "V2P-CA15";
 	arm,hbi = <0x237>;
 	compatible = "arm,vexpress,v2p-ca15,tc1", "arm,vexpress,v2p-ca15", "arm,vexpress";
 	interrupt-parent = <&gic>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	chosen { };
 
 	aliases {
 		serial0 = &v2m_serial0;

arch/arm/boot/dts/vexpress-v2p-ca5s.dts +4 -2

index da26a13..d4c5322 100644
@@ -9,13 +9,15 @@
 
 /dts-v1/;
 
-/include/ "skeleton.dtsi"
-
 / {
 	model = "V2P-CA5s";
 	arm,hbi = <0x225>;
 	compatible = "arm,vexpress,v2p-ca5s", "arm,vexpress";
 	interrupt-parent = <&gic>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	chosen { };
 
 	aliases {
 		serial0 = &v2m_serial0;

arch/arm/boot/dts/vexpress-v2p-ca9.dts +4 -2

index 84542e7..5d90ce5 100644
@@ -9,13 +9,15 @@
 
 /dts-v1/;
 
-/include/ "skeleton.dtsi"
-
 / {
 	model = "V2P-CA9";
 	arm,hbi = <0x191>;
 	compatible = "arm,vexpress,v2p-ca9", "arm,vexpress";
 	interrupt-parent = <&gic>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	chosen { };
 
 	aliases {
 		serial0 = &v2m_serial0;

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 22 lines · Pawel Moll

Quoted 9 lines · David Vrabel

Quoted 1 line

Ok, /include/ "skeleton.dtsi" is gone then :-)

The problem wasn't with including skeleton.dtsi. With CONFIG_ARM_ATAG_DTB_COMPAT the zImage decompressor modifies the appended DTB using information from the ATAGs (see atags_to_fdt()).

If there's an ATAG giving the amount of RAM the DTB's "memory" node is replaced with a new one. Since the vexpress DTBs don't have a "memory" node it's added and the DTB ends up with two nodes describing memory.

As it turned out it was just the "skeleton.dtsi" problem after all - I mean the fact that there where two device_type="memory" nodes in the tree.

The decompressor's setprop() (arch/arm/boot/compressed/atags_to_fdt.c:12) uses libfdt's fdt_setprop(), which correctly ignores the "@00000000" component of the node name and sets the reg property as expected. So as long as there is exactly one "memory[@address]" node in the tree, CONFIG_ARM_ATAG_DTB_COMPAT is happy.

Quoted 6 lines

I will remove the /include/ from the dts files for VE (see below) in the v3.3-rc1 based series.

Thanks for spotting this!

Pawe??

This carries a significant risk of unintended fragmentation and buggy maintenance. This patch is a good example of the kind of change which could easily go wrong. (I'm not saying that it is wrong -- just using it as an example.)

Since we will end up with a significantly large number of device trees for vexpress, I can foresee that we'll end up with a highly reduncant set of .dts{,i} files (each of which is often rather internally redundant too).

Does anyone have a view on whether it's acceptable to generate device tree sources from another form, instead of having them verbatim in the kernel tree? This could involve a preprocessor, or something more heavyweight.

Quoted 40 lines · Dave Martin

Quoted 22 lines · Pawel Moll

Quoted 9 lines · David Vrabel

Quoted 1 line

Ok, /include/ "skeleton.dtsi" is gone then :-)

The problem wasn't with including skeleton.dtsi. With CONFIG_ARM_ATAG_DTB_COMPAT the zImage decompressor modifies the appended DTB using information from the ATAGs (see atags_to_fdt()).

If there's an ATAG giving the amount of RAM the DTB's "memory" node is replaced with a new one. Since the vexpress DTBs don't have a "memory" node it's added and the DTB ends up with two nodes describing memory.

As it turned out it was just the "skeleton.dtsi" problem after all - I mean the fact that there where two device_type="memory" nodes in the tree.

The decompressor's setprop() (arch/arm/boot/compressed/atags_to_fdt.c:12) uses libfdt's fdt_setprop(), which correctly ignores the "@00000000" component of the node name and sets the reg property as expected. So as long as there is exactly one "memory[@address]" node in the tree, CONFIG_ARM_ATAG_DTB_COMPAT is happy.

Quoted 6 lines

I will remove the /include/ from the dts files for VE (see below) in the v3.3-rc1 based series.

Thanks for spotting this!

Pawe??

This carries a significant risk of unintended fragmentation and buggy maintenance. This patch is a good example of the kind of change which could easily go wrong. (I'm not saying that it is wrong -- just using it as an example.)

Since we will end up with a significantly large number of device trees for vexpress, I can foresee that we'll end up with a highly reduncant set of .dts{,i} files (each of which is often rather internally redundant too).

Quoted 4 lines

Does anyone have a view on whether it's acceptable to generate device tree sources from another form, instead of having them verbatim in the kernel tree? This could involve a preprocessor, or something more heavyweight.

Yes, the xilinx folks have been using a dts generator to create the device tree that matches an FPGA design. This works on ppc and microblaze, and they'll do the same thing for their ARM FPGA SoC.