ARM: iMX: lockup with irqs disabled in drivers/tty/imx.c

ARM: iMX: lockup with irqs disabled in drivers/tty/imx.c

[Michael McTernan]

Hi All,

I've found a lockup (interrupts disabled, infinite loop) in 
drivers/tty/imx.c.  This is on i.MX53 (Karo module), using the Karo BSP, 
but having checked the arm-soc git tree, I think the bug is here too. 
The Linaro tree(s) look the same.

I see the problem when the UART is setup to not have rtscts, so 
port->have_rtscts = 0 in the driver.  Additionally there is nothing 
hanging on the serial port (in my case the peripheral isn't yet powered 
up) so the rx line is low.

I can then trigger the lockup by simply booting the unit and issuing 
"echo > /dev/ttymxc1".  The same can be done on the other ports too. 
Once locked, magic sys-req appears not to work, pinging the target is 
dead etc...

The sequence of events in the kernel goes something like this:

1) imx_startup()
2) imx_int()
       \-> imx_rxint()
             |-> URXD0 returns 0xd800
             \-> tty_insert_flip_char(tty, 0, TTY_BREAK)
...
3) imx_start_tx()
       \-> imx_transmit_buffer()
              Writes '^' '@'
4) imx_set_termios()
       \-> hangs when waiting for USR2_TXDC

The problem is that UCR2_IRTS isn't yet set (that comes later in 
imx_set_termios()), so the transmitter is waiting for the pin to be 
asserted before it can serialise out from its FIFO and set the USR2_TXDC 
bit.  Since CTS/RTS will never come on this port, we end up spinning in 
the following bit of code in imx_set_termios():948, having earlier 
called spin_lock_irqsave():

	while ( !(readl(sport->port.membase + USR2) & USR2_TXDC))
		barrier();

The tx was a surprise to me; it looks like the receiver sees the low rx 
pin as a break which then, I think, causes a local echo sending back 
'^@', as per the following stack:

(imx_start_tx+0x60/0x12c)       from (__uart_start+0x44/0x48)
(__uart_start+0x44/0x48)        from (uart_start+0x20/0x4c)
(uart_start+0x20/0x4c)          from (process_echoes+0x25c/0x260)
(process_echoes+0x25c/0x260)    from (n_tty_receive_buf+0xc90/0xf04)
(n_tty_receive_buf+0xc90/0xf04) from (flush_to_ldisc+0xfc/0x1b0)
(flush_to_ldisc+0xfc/0x1b0)     from (process_one_work+0x1fc/0x330)
(process_one_work+0x1fc/0x330)  from (worker_thread+0x1d0/0x2f8)
(worker_thread+0x1d0/0x2f8)     from (kthread+0x7c/0x84)
(kthread+0x7c/0x84)             from (kernel_thread_exit+0x0/0x8)

Had imx_set_termios() been called prior to the imx_transmit_buffer(), 
UCR2_IRTS would have been set and disaster avoided.

So one workaround is to set UCR2_IRTS in imx_startup() as per the patch 
on the end of this email.  This still wouldn't cater for other cases 
where RTS/CTS is in use but not asserted by a peripheral (or the pin 
mux/pad isn't correctly setup/routed), or that termios() is called while 
data is blocked waiting for CTS/RTS.  The drain loop should probably 
implement a timeout and WARN(), but I'm not sure if it should also use a 
completion and interrupt to be correct - other serial drivers don't 
appear to do this though?

Anyway, the lockup is nasty, so hopefully can be triaged?

Regards,

Mike

Signed-off-by: Michael McTernan <Michael.McTernan.2001@cs.bris.ac.uk>
diff -Naurp orig/imx.c fixed/imx.c
--- orig/imx.c  2012-05-20 10:37:20.320041462 +0100
+++ fixed/imx.c 2012-05-20 11:18:28.512657377 +0100
@@ -741,6 +741,9 @@ static int imx_startup(struct uart_port
         writel(temp, sport->port.membase + UCR1);

         temp = readl(sport->port.membase + UCR2);
+       if (!sport->have_rtscts) {
+                       temp |= UCR2_IRTS;
+       }
         temp |= (UCR2_RXEN | UCR2_TXEN);
         writel(temp, sport->port.membase + UCR2);

ARM: iMX: lockup with irqs disabled in drivers/tty/imx.c

[manfred.schlaegl at gmx.at]

Hi!

Try to contact Sascha Hauer <sascha@saschahauer.de>.
see: http://www.pengutronix.de/software/linux-i.MX/index_de.html

Best Regards,

Manfred

ARM: iMX: lockup with irqs disabled in drivers/tty/imx.c

[Fabio Estevam]

Hi Michael,

On Sun, May 20, 2012 at 8:04 AM, Michael McTernan
<Michael.McTernan.2001@cs.bris.ac.uk> wrote:

> Signed-off-by: Michael McTernan <Michael.McTernan.2001@cs.bris.ac.uk>
> diff -Naurp orig/imx.c fixed/imx.c
> --- orig/imx.c ?2012-05-20 10:37:20.320041462 +0100
> +++ fixed/imx.c 2012-05-20 11:18:28.512657377 +0100
> @@ -741,6 +741,9 @@ static int imx_startup(struct uart_port
> ? ? ? ?writel(temp, sport->port.membase + UCR1);
>
> ? ? ? ?temp = readl(sport->port.membase + UCR2);
> + ? ? ? if (!sport->have_rtscts) {
> + ? ? ? ? ? ? ? ? ? ? ? temp |= UCR2_IRTS;
> + ? ? ? }
> ? ? ? ?temp |= (UCR2_RXEN | UCR2_TXEN);
> ? ? ? ?writel(temp, sport->port.membase + UCR2);

Could you please re-submit this patch via git send-email and copy the
relevant lists and maintainers? (Hint: use ./scripts/get_maintainer.pl
)

You should also add kernel at pengutronix.de in Cc.

Regards,

Fabio Estevam

ARM: iMX: lockup with irqs disabled in drivers/tty/imx.c

[Fabio Estevam]

Michael,

On Sun, May 20, 2012 at 8:04 AM, Michael McTernan
<Michael.McTernan.2001@cs.bris.ac.uk> wrote:

> I can then trigger the lockup by simply booting the unit and issuing "echo >
> /dev/ttymxc1". ?The same can be done on the other ports too. Once locked,
> magic sys-req appears not to work, pinging the target is dead etc...

I did a 'echo "hello" > /dev/ttymxc1' and I am not able to see the
lockup on a mx31pdk running 3.5-rc1.

ARM: iMX: lockup with irqs disabled in drivers/tty/imx.c

[Michael McTernan]

On 06/12/2012 07:53 PM, Fabio Estevam wrote:
>> I can then trigger the lockup by simply booting the unit and issuing "echo>
>> /dev/ttymxc1".  The same can be done on the other ports too. Once locked,
>> magic sys-req appears not to work, pinging the target is dead etc...
>
> I did a 'echo "hello">  /dev/ttymxc1' and I am not able to see the
> lockup on a mx31pdk running 3.5-rc1.

Depending on the setup and board, you may find that the RTS pin is 
asserted so you don't hit the lockup, or more likely, that the Rx pin is 
being driven by a RS232 level shifter so you don't get the early break 
character which triggers the problem.

On my board the RTS will never be set and the Rx pin is pulled low so 
generates a break condition and lockup reliably (prior to patching).

I don't have a i.MX31 reference manual or PDK schematic, but you may be 
able to change the IO mux config to move the serial port Rx and RTS to 
free pins with pull downs in order to reproduce the bug.

This bug is very much a corner case, and arguably the hardware 
setup/board config isn't right, but for the sake of a few lines of code 
it would seem better to avoid such as nasty lockup which isn't 
straightforward to debug.

Regards,

Mike