Revert "ARM: pxa: call debug_ll_io_init for earlyprintk"

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.

Quoted 3 lines · Dmitry Eremin-Solenikov

This reverts commit 2111667b4677 ("ARM: pxa: call debug_ll_io_init for earlyprintk")

Quoted 3 lines

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) ?

Quoted 3 lines · Robert Jarzmik

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?

Quoted 5 lines · On Monday 06 October 2014 16:02:09 Andrew Ruder wrote:

Quoted 3 lines · Robert Jarzmik

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) ?

Quoted 7 lines

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.

Quoted 14 lines · Arnd Bergmann

Quoted 12 lines · On Monday 06 October 2014 16:02:09 Andrew Ruder wrote:

Quoted 3 lines · Robert Jarzmik

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?

Quoted 2 lines

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 don't see how to do it without ugly ifdefery though such as :
[...]
But that's awfull, there should be another better way ...

Quoted 14 lines · Arnd Bergmann

Quoted 5 lines · On Monday 06 October 2014 16:02:09 Andrew Ruder wrote:

Quoted 3 lines · Robert Jarzmik

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) ?

Quoted 7 lines

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?

Quoted 2 lines

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

From 93239cf3015997d45fec8fee97059758cac1fab3 Mon Sep 17 00:00:00 2001

Quoted 4 lines · Arnd Bergmann

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.

Quoted 1 line

Arnd

Yep, that looks better.
Would something like that be appropriate ? I tested that on pxa27x, but not on pxa25x, even if there seems to be no reason it wouldn't work for it too ...

And I have not yet thought over if there is any impact on vmalloc memory or if the double virtual mapping can trigger something bad.

---<8---
From 93239cf3015997d45fec8fee97059758cac1fab3 Mon Sep 17 00:00:00 2001
From: Robert Jarzmik <[email protected]>
Date: Tue, 7 Oct 2014 21:18:14 +0200
Subject: [PATCH] ARM: pxa: fix hang on startup with DEBUG_LL

The commit 2111667b4677 ("ARM: pxa: call debug_ll_io_init for earlyprintk") triggers in the current kernel the attached backtrace on PXA/tosa early in the boot time when DEBUG_LL is enabled.

It is due to overlap between uart virtual memory defined in DEBUG_UART_VIRT and mapped by debug_ll_io_init() and peripheral bus mapped by pxa_map_io at the same address, 0xf2100000.

As hinted by Arnd, map early virtual memory for low level debug on address 0xf6200000, even if that means 2 virtual mappings will give access to the pxa internal UARTs (FFUART, BTUART, STUART, ...).

------------[ 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 <[email protected]>

Reported-by Dmitry Eremin-Solenikov <[email protected]>

Signed-off-by Robert Jarzmik <[email protected]>

Ah, you mean doing it now? That would work too.

I was thinking of reverting the patch for now and fixing it properly later, but your patch also looks simple enough that we could probably just do it.

Acked-by Arnd Bergmann <[email protected]>

A small test would be welcome, as I don't have a pxa25x board, and my board doesn't have an available UART for debugging anyway.

Quoted 2 lines · Robert Jarzmik

A small test would be welcome, as I don't have a pxa25x board, and my board doesn't have an available UART for debugging anyway.

I should be able to give it a test on a pxa270 board in the next 24 hours or so but I don't have any pxa25x boards to test with.

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.

Quoted 6 lines · Dmitry Eremin-Solenikov

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 <[email protected]>