| Message ID | 1412602320-22896-1-git-send-email-dbaryshkov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> writes: > This reverts commit 2111667b4677 ("ARM: pxa: call debug_ll_io_init for > earlyprintk") > > This commit gives the following backtrace on PXA/tosa early in the boot > time when DEBUG_LL is enabled. It is due to overlap between uart memory > and PERIPH_ memory block mapped by pxa_map_io. Indeed, all available UARTs IO mappings are already covered by pxa_map_io(), so the revert will remove the bug and keep the DEBUG_LL functionality. I suppose you did the test with the revert and the lowlevel debugging works, right ? Haojian, if you are still taking patches for v3.18-rcs, please have my : Reviewed-by: Robert Jarzmik <robert.jarzmik@free.fr> If you want me to begin the gathering, tell me. Cheers. -- Robert
Robert Jarzmik <robert.jarzmik@free.fr> writes: > Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> writes: > >> This reverts commit 2111667b4677 ("ARM: pxa: call debug_ll_io_init for >> earlyprintk") >> >> This commit gives the following backtrace on PXA/tosa early in the boot >> time when DEBUG_LL is enabled. It is due to overlap between uart memory >> and PERIPH_ memory block mapped by pxa_map_io. > > Indeed, all available UARTs IO mappings are already covered by pxa_map_io(), so > the revert will remove the bug and keep the DEBUG_LL functionality. I suppose > you did the test with the revert and the lowlevel debugging works, right ? Actually, I have a question for Andrew : was your commit aimed at the 3 or 4 available UARTs (ie. in peripheral address space), or is it a case where an external UART is mapped on the system bus (if that is possible) ?
On Monday 06 October 2014 20:59:28 Robert Jarzmik wrote: > Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> writes: > > > This reverts commit 2111667b4677 ("ARM: pxa: call debug_ll_io_init for > > earlyprintk") > > > > This commit gives the following backtrace on PXA/tosa early in the boot > > time when DEBUG_LL is enabled. It is due to overlap between uart memory > > and PERIPH_ memory block mapped by pxa_map_io. > > Indeed, all available UARTs IO mappings are already covered by pxa_map_io(), so > the revert will remove the bug and keep the DEBUG_LL functionality. I suppose > you did the test with the revert and the lowlevel debugging works, right ? > > Haojian, if you are still taking patches for v3.18-rcs, please have my : > Reviewed-by: Robert Jarzmik <robert.jarzmik@free.fr> This is a regression fix, so it should go into 3.18 and 3.17-stable once everybody agrees that it is the correct solution. Arnd
On Mon, Oct 06, 2014 at 09:29:36PM +0200, Robert Jarzmik wrote: > Actually, I have a question for Andrew : was your commit aimed at the 3 or 4 > available UARTs (ie. in peripheral address space), or is it a case where an > external UART is mapped on the system bus (if that is possible) ? My apologies! I'm actually on a really long-term project of getting my board (similar to zeus board already in the kernel) fully running off of devicetree. For this particular board, all of the UARTS are on the system bus and not the built in ones. But yes - I do see how the built-in UARTS would overlap and hit the BUG_ON on other boards. Any thoughts on a better way of solving this than just reverting the patch back into only working on the built-in UARTs? Thanks, Andrew Ruder
On Monday 06 October 2014 16:02:09 Andrew Ruder wrote: > On Mon, Oct 06, 2014 at 09:29:36PM +0200, Robert Jarzmik wrote: > > Actually, I have a question for Andrew : was your commit aimed at the 3 or 4 > > available UARTs (ie. in peripheral address space), or is it a case where an > > external UART is mapped on the system bus (if that is possible) ? > > My apologies! I'm actually on a really long-term project of getting my > board (similar to zeus board already in the kernel) fully running off of > devicetree. For this particular board, all of the UARTS are on the > system bus and not the built in ones. But yes - I do see how the > built-in UARTS would overlap and hit the BUG_ON on other boards. Any > thoughts on a better way of solving this than just reverting the patch > back into only working on the built-in UARTs? I think the best way forward is to make the built-in UARTs work with debug_ll_io_init and then apply your patch again. On a more general note, my personal opinion is that in case of PXA, we should not (yet) block additional board files because of missing DT support. I think it's still a long way before it will work, and most of the work is done by hobbyists, so I would allow new board files for PXA to be added. Of course if the PXA maintainers think differently, they can still ask you do to a DT port. Arnd
Arnd Bergmann <arnd@arndb.de> writes: > On Monday 06 October 2014 16:02:09 Andrew Ruder wrote: >> On Mon, Oct 06, 2014 at 09:29:36PM +0200, Robert Jarzmik wrote: >> > Actually, I have a question for Andrew : was your commit aimed at the 3 or 4 >> > available UARTs (ie. in peripheral address space), or is it a case where an >> > external UART is mapped on the system bus (if that is possible) ? >> >> My apologies! I'm actually on a really long-term project of getting my >> board (similar to zeus board already in the kernel) fully running off of >> devicetree. For this particular board, all of the UARTS are on the >> system bus and not the built in ones. But yes - I do see how the >> built-in UARTS would overlap and hit the BUG_ON on other boards. Any >> thoughts on a better way of solving this than just reverting the patch >> back into only working on the built-in UARTs? > > I think the best way forward is to make the built-in UARTs work with > debug_ll_io_init and then apply your patch again. Yes, and that means revert, right ? The best approach I'd see for pxa_map_io() would be to call debug_ll_io_init() conditionally, when the CONFIG_DEBUG_UART_PHYS is defined and is not within the peripheral bus range, ie. within [ PERIPH_PHYS .. PERIPH_PHYS + PERIPH_SIZE ]. I don't see how to do it without ugly ifdefery though such as : #if defined(CONFIG_DEBUG_UART_PHYS) && \ ((CONFIG_DEBUG_UART_PHYS < PERIPH_PHYS) || (CONFIG_DEBUG_UART_PHYS >= PERIPH_PHYS + PERIPH_SIZE)) debug_ll_io_init(); #endif But that's awfull, there should be another better way ...
On Mon, Oct 06, 2014 at 11:18:43PM +0200, Arnd Bergmann wrote: > I think the best way forward is to make the built-in UARTs work with > debug_ll_io_init and then apply your patch again. Agreed, when I get some time I'll take a look at this again. I now have another PXA based board using built-in UARTs for the debug console so I can test a little more thoroughly. So for now: Acked: Andrew Ruder <andrew.ruder@elecsyscorp.com> on the revert. Sorry again for the churn - dropped the ball on this one. > On a more general note, my personal opinion is that in case of PXA, > we should not (yet) block additional board files because of missing DT > support. I think it's still a long way before it will work, and most > of the work is done by hobbyists, so I would allow new board files for > PXA to be added. Of course if the PXA maintainers think differently, > they can still ask you do to a DT port. I'm more of the opinion that we aren't that far from being able to block board files - I am 100% using device-tree and now have a single kernel that will boot on 3 different Elecsys boards. Not to mention the fact that I can't imagine there's a whole lot of new hardware work in the PXA realm. I have posted the work to my github account (http://github.com/aeruder/linux) but it is tough work getting things cleaned up to not break anyone not using DT (HAH, see this thread), documented, and submitted. I was really hoping the big mmc/DMA rewrite would drop into mainline at some point since that is one of the more terrible hacks I'm still maintaining over here. Cheers, Andy
On Mon, Oct 06, 2014 at 11:44:52PM +0200, Robert Jarzmik wrote: > Arnd Bergmann <arnd@arndb.de> writes: > > > On Monday 06 October 2014 16:02:09 Andrew Ruder wrote: > >> On Mon, Oct 06, 2014 at 09:29:36PM +0200, Robert Jarzmik wrote: > >> > Actually, I have a question for Andrew : was your commit aimed at the 3 or 4 > >> > available UARTs (ie. in peripheral address space), or is it a case where an > >> > external UART is mapped on the system bus (if that is possible) ? > >> > >> My apologies! I'm actually on a really long-term project of getting my > >> board (similar to zeus board already in the kernel) fully running off of > >> devicetree. For this particular board, all of the UARTS are on the > >> system bus and not the built in ones. But yes - I do see how the > >> built-in UARTS would overlap and hit the BUG_ON on other boards. Any > >> thoughts on a better way of solving this than just reverting the patch > >> back into only working on the built-in UARTs? > > > > I think the best way forward is to make the built-in UARTs work with > > debug_ll_io_init and then apply your patch again. > Yes, and that means revert, right ? I think so, yes. See my response to Arnd. > I don't see how to do it without ugly ifdefery though such as : > [...] > But that's awfull, there should be another better way ... I agree, surely other ports are getting around this problem in another way? Cheers, Andy
On Monday 06 October 2014 23:44:52 Robert Jarzmik wrote: > Arnd Bergmann <arnd@arndb.de> writes: > > > On Monday 06 October 2014 16:02:09 Andrew Ruder wrote: > >> On Mon, Oct 06, 2014 at 09:29:36PM +0200, Robert Jarzmik wrote: > >> > Actually, I have a question for Andrew : was your commit aimed at the 3 or 4 > >> > available UARTs (ie. in peripheral address space), or is it a case where an > >> > external UART is mapped on the system bus (if that is possible) ? > >> > >> My apologies! I'm actually on a really long-term project of getting my > >> board (similar to zeus board already in the kernel) fully running off of > >> devicetree. For this particular board, all of the UARTS are on the > >> system bus and not the built in ones. But yes - I do see how the > >> built-in UARTS would overlap and hit the BUG_ON on other boards. Any > >> thoughts on a better way of solving this than just reverting the patch > >> back into only working on the built-in UARTs? > > > > I think the best way forward is to make the built-in UARTs work with > > debug_ll_io_init and then apply your patch again. > Yes, and that means revert, right ? > > The best approach I'd see for pxa_map_io() would be to call debug_ll_io_init() > conditionally, when the CONFIG_DEBUG_UART_PHYS is defined and is not within the > peripheral bus range, ie. within [ PERIPH_PHYS .. PERIPH_PHYS + PERIPH_SIZE ]. > > I don't see how to do it without ugly ifdefery though such as : > #if defined(CONFIG_DEBUG_UART_PHYS) && \ > ((CONFIG_DEBUG_UART_PHYS < PERIPH_PHYS) || (CONFIG_DEBUG_UART_PHYS >= > PERIPH_PHYS + PERIPH_SIZE)) > debug_ll_io_init(); > #endif > > But that's awfull, there should be another better way ... Can't you just define CONFIG_DEBUG_UART_VIRT outside of the existing mappings? It doesn't have to use the same mapping as PERIPH_PHYS. Arnd
diff --git a/arch/arm/mach-pxa/generic.c b/arch/arm/mach-pxa/generic.c index 04b013f..d862607 100644 --- a/arch/arm/mach-pxa/generic.c +++ b/arch/arm/mach-pxa/generic.c @@ -99,6 +99,5 @@ static struct map_desc common_io_desc[] __initdata = { void __init pxa_map_io(void) { - debug_ll_io_init(); iotable_init(ARRAY_AND_SIZE(common_io_desc)); }
This reverts commit 2111667b4677 ("ARM: pxa: call debug_ll_io_init for earlyprintk") This commit gives the following backtrace on PXA/tosa early in the boot time when DEBUG_LL is enabled. It is due to overlap between uart memory and PERIPH_ memory block mapped by pxa_map_io. ------------[ cut here ]------------ kernel BUG at /home/lumag/linux/mm/vmalloc.c:1143! Internal error: Oops - BUG: 0 [#1] PREEMPT ARM Modules linked in: CPU: 0 PID: 0 Comm: swapper Not tainted 3.17.0-00032-g8e0d202-dirty #23 task: c062a5a8 ti: c0620000 task.ti: c0620000 PC is at vm_area_add_early+0x54/0x84 LR is at add_static_vm_early+0xc/0x60 pc : [<c03e1100>] lr : [<c03d9ef4>] psr: 800001d3 sp : c0621f04 ip : c03efa74 fp : c03edf84 r10: c0637e98 r9 : 40000001 r8 : c03da57c r7 : c3ffcfb0 r6 : 00000000 r5 : c3ffcfb0 r4 : 02000000 r3 : c3ffcfd8 r2 : f2100000 r1 : f4000000 r0 : c3ffcfb0 Flags: Nzcv IRQs off FIQs off Mode SVC_32 ISA ARM Segment kernel Control: 00007977 Table: a0004000 DAC: 00000017 Process swapper (pid: 0, stack limit = 0xc06201c8) Stack: (0xc0621f04 to 0xc0622000) 1f00: c3ffcfd8 40000001 c3ffcfd8 c03ee08c c03da570 c03db90c c0637d24 1f20: 00000000 c03ec7cc c066e654 a0700000 000a0700 c03db914 c03db90c c03daf84 1f40: 00000000 000a0000 c0000000 c03ec7cc 000a0700 c0700000 ffff1000 000a3fff 1f60: 00001000 00000007 00000000 c03ec7cc c0008000 c03ed748 c0621fd4 c03d5d18 1f80: 69052d00 a03ec48c 00000000 c03d8ad0 0000006c 00007977 c036c6e8 00000001 1fa0: c0621fd4 c03ed744 c0628000 a0004000 69052d00 a03ec48c 00000000 c03d68d4 1fc0: 00000000 00000000 00000000 00000000 00000000 c03ed748 c0649894 c062801c 1fe0: c03ed744 c062b2f0 a0004000 69052d00 a03ec48c a0008040 00000000 00000000 [<c03e1100>] (vm_area_add_early) from [<c03d9ef4>] (add_static_vm_early+0xc/0x60) [<c03d9ef4>] (add_static_vm_early) from [<c03da570>] (iotable_init.part.6+0xa8/0xb4) [<c03da570>] (iotable_init.part.6) from [<c03db914>] (pxa25x_map_io+0x8/0x24) [<c03db914>] (pxa25x_map_io) from [<c03daf84>] (paging_init+0x744/0x8d8) [<c03daf84>] (paging_init) from [<c03d8ad0>] (setup_arch+0x354/0x608) [<c03d8ad0>] (setup_arch) from [<c03d68d4>] (start_kernel+0xa8/0x3dc) [<c03d68d4>] (start_kernel) from [<a0008040>] (0xa0008040) Code: e5904008 e0811004 e1520001 2a000005 (e7f001f2) ---[ end trace f24b6c88ae00fa9a ]--- Kernel panic - not syncing: Attempted to kill the idle task! ---[ end Kernel panic - not syncing: Attempted to kill the idle task! Cc: <stable@vger.kernel.org> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> --- arch/arm/mach-pxa/generic.c | 1 - 1 file changed, 1 deletion(-)