| Message ID | 1323957761-13553-1-git-send-email-pawel.moll@arm.com |
|---|---|
| State | New |
| Headers | show |
On 12/15/2011 08:02 AM, Pawel Moll wrote: > 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 <pawel.moll@arm.com> > --- > arch/arm/plat-versatile/localtimer.c | 12 ++++++++++++ > 1 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/plat-versatile/localtimer.c b/arch/arm/plat-versatile/localtimer.c > index 0fb3961..8f0dc10 100644 > --- a/arch/arm/plat-versatile/localtimer.c > +++ b/arch/arm/plat-versatile/localtimer.c > @@ -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> > @@ -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"); > + > + 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. Rob
On Thu, 2011-12-15 at 14:53 +0000, Rob Herring wrote: > > @@ -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"); > > + > > + 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. Right, how about that, then: @@ -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); + + dt_node_probed = 1; + } +#endif + if (!twd_base) + return -ENXIO; + evt->irq = IRQ_LOCALTIMER; twd_timer_setup(evt); return 0; Cheers! Paweł
On Thu, 2011-12-15 at 14:02 +0000, Pawel Moll wrote: > This patch adds generic Versatile Express DT machine description, > Device Tree description for the motherboard and documentation for > the bindings. [...] > diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c [...] > +DT_MACHINE_START(VEXPRESS_DT, "ARM Versatile Express") > + .map_io = v2m_dt_map_io, > + .init_early = v2m_dt_init_early, > + .init_irq = v2m_dt_init_irq, > + .timer = &v2m_dt_timer, > + .init_machine = v2m_dt_init, > + .dt_compat = v2m_dt_match, > +MACHINE_END The machine name here is missing a '-' that is in the original non device-tree machine description, which is "ARM-Versatile Express". The hyphen is also in Integrator and Realview machine names, so this seems to be a convention with ARM boards. (I found this because Android parses machine name for configuration purposes.)
Hi, Sorry about loooong delay - I've been on holiday. On Wed, 2012-01-04 at 16:35 +0000, David Vrabel wrote: > On 15/12/11 14:02, Pawel Moll wrote: > > This patch adds support for RS1 memory map based Versatile Express > > motherboard. > > > [...] > > --- a/arch/arm/mach-vexpress/include/mach/debug-macro.S > > +++ b/arch/arm/mach-vexpress/include/mach/debug-macro.S > > @@ -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 > > +#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? > __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. Thanks for trying this out! Paweł
On Tue, 2012-01-10 at 14:21 +0000, David Vrabel wrote: > On 15/12/11 14:02, Pawel Moll wrote: > > 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. > > As the chip's GIC has 160 interrupt inputs and equivalent SMM > > (FPGA) has GIC synthesised with 256 interrupts, NR_IRQS is > > increased. > > > [...] > > --- /dev/null > > +++ b/arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts > [...] > > + memory@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@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... Cheers! Paweł
On 01/19/2012 07:27 AM, Pawel Moll wrote: > On Tue, 2012-01-10 at 14:21 +0000, David Vrabel wrote: >> On 15/12/11 14:02, Pawel Moll wrote: >>> 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. > >>> As the chip's GIC has 160 interrupt inputs and equivalent SMM >>> (FPGA) has GIC synthesised with 256 interrupts, NR_IRQS is >>> increased. >>> >> [...] >>> --- /dev/null >>> +++ b/arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts >> [...] >>> + memory@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@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... > Hummm... I guess you should just use "memory" if you are using skeleton.dtsi. Rob
On Thu, 2012-01-19 at 13:34 +0000, Rob Herring wrote: > > 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. Well, I don't mind _not_ using skeleton, but I had an impression the general policy was to use it? Paweł
On 01/19/2012 07:43 AM, Pawel Moll wrote: > On Thu, 2012-01-19 at 13:34 +0000, Rob Herring wrote: >>> 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. > > 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. Rob
On Thu, 2012-01-19 at 14:01 +0000, Rob Herring wrote: > On 01/19/2012 07:43 AM, Pawel Moll wrote: > > On Thu, 2012-01-19 at 13:34 +0000, Rob Herring wrote: > >>> 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. > > > > 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 :-) Cheers! Paweł
On 19/01/12 13:21, Pawel Moll wrote: > Hi, > > Sorry about loooong delay - I've been on holiday. > > On Wed, 2012-01-04 at 16:35 +0000, David Vrabel wrote: >> On 15/12/11 14:02, Pawel Moll wrote: >>> This patch adds support for RS1 memory map based Versatile Express >>> motherboard. >>> >> [...] >>> --- a/arch/arm/mach-vexpress/include/mach/debug-macro.S >>> +++ b/arch/arm/mach-vexpress/include/mach/debug-macro.S >>> @@ -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 >>> +#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 > # earlyprintk needs to be on the kernel command line to enable it. Without this option it will work fine. > 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? The only output is from the zImage decompressor. It's a synchronous data fault and DFAR is 0xf8009fe0. >> __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. 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. David
On 19/01/12 14:51, Pawel Moll wrote: > On Thu, 2012-01-19 at 14:01 +0000, Rob Herring wrote: >> On 01/19/2012 07:43 AM, Pawel Moll wrote: >>> On Thu, 2012-01-19 at 13:34 +0000, Rob Herring wrote: >>>>> 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. >>> >>> 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 :-) 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. I don't expect any real production vexpress system to use this config options -- we're using it now when running as a guest under Xen because Xen doesn't (yet) support device tree and we're using ATAGs to tell the guest how much RAM it's been allocated. David
On Thu, Jan 19, 2012 at 05:00:56PM +0000, David Vrabel wrote: > I don't expect any real production vexpress system to use this config > options I do - _if_ I decide to try DT on my Versatile Express. That'll be with the existing boot setup, which will be ATAG based.
On Thu, 2012-01-19 at 17:00 +0000, David Vrabel wrote: > 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. > 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. The "memory@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... One way or the other, I'll get this fixed. Thanks for letting me know! Paweł
On Thu, 2012-01-19 at 16:46 +0000, David Vrabel wrote: > > 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. # cat /proc/cmdline console=ttyAMA0,38400 earlyprintk rootwait root=/dev/mmcblk0p2 mmci.fmax=12000000 debug I'll investigate the matter to the bottom, though. Your kernel config would be helpful, thanks! > 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. Yess... The DT+earlyprintk problem was discussed several times already, without any happy resolution so far... Paweł
On Thu, Jan 19, 2012 at 05:27:15PM +0000, Pawel Moll wrote: > On Thu, 2012-01-19 at 17:00 +0000, David Vrabel wrote: > > 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. > > > 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. > > The "memory@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.
On Thu, Jan 19, 2012 at 10:50 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Thu, Jan 19, 2012 at 05:27:15PM +0000, Pawel Moll wrote: >> On Thu, 2012-01-19 at 17:00 +0000, David Vrabel wrote: >> > 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. >> >> > 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. >> >> The "memory@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? 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. > 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. :-) > 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. +1
On Thu, 19 Jan 2012, Grant Likely wrote: > On Thu, Jan 19, 2012 at 10:50 AM, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: > > On Thu, Jan 19, 2012 at 05:27:15PM +0000, Pawel Moll wrote: > >> On Thu, 2012-01-19 at 17:00 +0000, David Vrabel wrote: > >> > 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. > >> > >> > 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. > >> > >> The "memory@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? > > 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. > > > 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. Nicolas
On Thu, Jan 19, 2012 at 11:09 AM, Nicolas Pitre <nico@fluxnic.net> wrote: > On Thu, 19 Jan 2012, Grant Likely wrote: > >> On Thu, Jan 19, 2012 at 10:50 AM, Russell King - ARM Linux >> <linux@arm.linux.org.uk> wrote: >> > On Thu, Jan 19, 2012 at 05:27:15PM +0000, Pawel Moll wrote: >> >> On Thu, 2012-01-19 at 17:00 +0000, David Vrabel wrote: >> >> > 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. >> >> >> >> > 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. >> >> >> >> The "memory@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? >> >> 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. >> >> > 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. Hahaha. Having said that though, I still strongly agree with what we have. Giving the kernel only one or the other avoids any weirdness about which the kernel should choose when both are present. The bootwrapper atag-to-dt conversion is a migration and development tool, and it should not do any DT manipulation must be kept to a bare minimum. g.
On Thu, 2012-01-19 at 16:46 +0000, David Vrabel wrote: > >> __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! Paweł
On Thu, Jan 19, 2012 at 01:21:06PM +0000, Pawel Moll wrote: > Hi, > > Sorry about loooong delay - I've been on holiday. > > On Wed, 2012-01-04 at 16:35 +0000, David Vrabel wrote: > > On 15/12/11 14:02, Pawel Moll wrote: > > > This patch adds support for RS1 memory map based Versatile Express > > > motherboard. > > > > > [...] > > > --- a/arch/arm/mach-vexpress/include/mach/debug-macro.S > > > +++ b/arch/arm/mach-vexpress/include/mach/debug-macro.S > > > @@ -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 > > > +#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? I had to disable this when running on the ARM fast model with your vexpress DT series. I didn't debug into exactly why I had to disable it, but I think I was getting aborts when the kernel was probing for nonexistent legacy memory map location of the UART. I don't know whether this will affect real hardware though... and maybe I was hitting the problem due to something else. Anyway unless I've misunderstood something, AMBA devices are only really probable if there is some real bus slave at the probed address, which is free from nasty side-effects. Really we should have a way for getting this info from the device tree, yada yada. (Repeating myself here, I know) In the meantime, I don't see a practical alternative to adding distinct lowlevel debug UART selection options for the two memory maps ...? Cheers ---Dave
On Fri, Jan 27, 2012 at 02:02:40PM +0000, Pawel Moll wrote: > On Thu, 2012-01-19 at 16:46 +0000, David Vrabel wrote: > > >> __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! I would be interested in that too. However, I would still prefer the explicit debug UART selection route, since we already have to do that for many boards and I'm not sure how much I trust the memory maps of future tiles and models to "evolve" relative to what currently exists. Are you sure it won't change again? If we rely on lucky arrangement of slaves on the bus in order to have a workaround, sooner or later our luck is liable to run out. Cheers ---Dave
Hi All, This is the last (at least this year) version of the patches. I've added Tested-by: Tixy to the patches that hasn't change since v5. I didn't add Arnd's and Rob's Acked-bys as the code changed significantly since. If still applicable, I'll add then on the first opportunity. Arnd, Russell, if you think that the code is ready enough for 3.3, please pull from this branch based on today's tip (v3.2-rc5+): 8<--------------------------------------------------------------------- The following changes since commit 55b02d2f4445ad625213817a1736bf2884d32547: Merge branch 'drm-fixes' of git://people.freedesktop.org/~airlied/linux (2011-12-14 19:45:40 -0800) are available in the git repository at: git://git.linaro.org/people/pawelmoll/linux.git vexpress-dt 8<--------------------------------------------------------------------- I also have a "vexpress-dt-rmk-devel-stable" branch there, based on today's Russell's devel-stable. The only difference is "handle_irq" added to DT machine description. If there are still some issues I'll address them once I'm back from holiday (today it's the last day before I loose all access to the Internet for a month). Changes since v5: * As suggested by Russell, DT-based local timers take precedence over statically defined ones (so the twd_base if overwritten). * Minor redactorial changes in DT-based SMP initialization (the logic stays the same). * Added last missing device node to the V2M trees: "arm,vexpress-vram". * Similarly to "arm,vexpress-cf" added "arm,vexpress-psram" to the relevant node, just in case we need to detect it in future. Tested on: - V2P-CA9 with ATAGs (both with a ATAGs-only and ATAGs+DT kernels). - V2P-CA9 with DT - V2P-CA5s with DT - V2P-CA15 with DT - V2F-2XV6 Cortex-A7 SMM with DT Thanks to all involved for your help! 8<--------------------------------------------------------------------- Pawel Moll (9): ARM: versatile: Add missing ENDPROC to headsmp.S ARM: vexpress: Get rid of MMIO_P2V ARM: versatile: Map local timers using Device Tree when possible ARM: vexpress: Use FDT data in platform SMP calls ARM: vexpress: Add Device Tree support ARM: vexpress: Motherboard RS1 memory map support ARM: vexpress: Add Device Tree for V2P-CA5s core tile ARM: vexpress: Add Device Tree for V2P-CA9 core tile ARM: vexpress: Add Device Tree for V2P-CA15 core tile (TC1 variant) Documentation/devicetree/bindings/arm/vexpress.txt | 144 ++++++++++ arch/arm/Kconfig | 2 +- arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 201 ++++++++++++++ arch/arm/boot/dts/vexpress-v2m.dtsi | 200 ++++++++++++++ arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts | 155 +++++++++++ arch/arm/boot/dts/vexpress-v2p-ca5s.dts | 160 +++++++++++ arch/arm/boot/dts/vexpress-v2p-ca9.dts | 190 +++++++++++++ arch/arm/include/asm/hardware/arm_timer.h | 5 + arch/arm/mach-realview/platsmp.c | 3 +- arch/arm/mach-vexpress/Kconfig | 45 +++- arch/arm/mach-vexpress/Makefile.boot | 6 + arch/arm/mach-vexpress/core.h | 9 +- arch/arm/mach-vexpress/ct-ca9x4.c | 48 +--- arch/arm/mach-vexpress/include/mach/ct-ca9x4.h | 13 +- arch/arm/mach-vexpress/include/mach/debug-macro.S | 37 +++- arch/arm/mach-vexpress/include/mach/irqs.h | 2 +- arch/arm/mach-vexpress/include/mach/motherboard.h | 58 +++-- arch/arm/mach-vexpress/include/mach/uncompress.h | 13 +- arch/arm/mach-vexpress/platsmp.c | 153 ++++++++++- arch/arm/mach-vexpress/v2m.c | 282 ++++++++++++++++++-- arch/arm/mm/Kconfig | 2 +- arch/arm/plat-versatile/headsmp.S | 1 + arch/arm/plat-versatile/localtimer.c | 12 + 23 files changed, 1621 insertions(+), 120 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/vexpress.txt create mode 100644 arch/arm/boot/dts/vexpress-v2m-rs1.dtsi create mode 100644 arch/arm/boot/dts/vexpress-v2m.dtsi create mode 100644 arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts create mode 100644 arch/arm/boot/dts/vexpress-v2p-ca5s.dts create mode 100644 arch/arm/boot/dts/vexpress-v2p-ca9.dts