[PATCH] 8250: yet another attempt at a serial console fix

[Georg Nikodym]

Various people have reported a problem in which the serial console  
output gets stalled / stuck until a receive interrupt is received  
(typically somebody hitting return a couple of times in an attempt to  
see if the machine is still alive).

I'm working on an LSI based ARM SoC and as of 2.6.16, I too  
experience this.  For me the change that appears to have broken  
things is where Alan Cox changed the following line in  
serial8250_console_write() from:

	serial_out(up, UART_IER, ier);

to:

	serial_out(up, UART_IER, ier | UART_IER_THRI);

The documentation I have for my SoC, AMBA Multilayer Reference Design  
Rev 2.0 TECHNICAL MANUAL (cw001202_MLRD2.0_TechManual.pdf) covers the  
UART in chapter 10.  There it describes the UART as 16550D compatible  
with several listed behavioural changes.  Specific to the THRE  
interrupt, it says the following:

	The previous version of the ApUart would trigger the THRE interrupt if
	the THRE bit was set and the Enable Transmitter Holding Register Empty
	Interrupt bit was changed from 0 to 1.

	The new version does this only if the interrupt handler has not already
	reported this condition (by a read of the IIR when a THRE interrupt  
is the
	highest priority pending interrupt). A new interrupt will only be  
reported
	by the new version if one or more characters are written to, and
	eventually emptied out of, the THR or the Transmitter FIFO.

This appears to describe the behaviour observed by Alex Williamson  
and addressed in his backup-timer-for-uarts-that-lose-interrupts- 
take-3.patch (http://lkml.org/lkml/2006/1/21/93)

Since the old code worked I had trouble swallowing the backup timer  
idea.  But the detection logic worked a charm so I lifted that and  
offer up the attached patch for evisceration.

-g

===== 8250.h 1.27 vs edited =====
--- 1.27/drivers/serial/8250.h	2006-01-04 14:43:24 -05:00
+++ edited/8250.h	2006-03-30 16:26:56 -05:00
@@ -50,6 +50,7 @@ struct serial8250_config {
 #define UART_BUG_QUOT	(1 << 0)	/* UART has buggy quot LSB */
 #define UART_BUG_TXEN	(1 << 1)	/* UART has buggy TX IIR status */
 #define UART_BUG_NOMSR	(1 << 2)	/* UART has buggy MSR status bits  
(Au1x00) */
+#define UART_BUG_THRI	(1 << 3)	/* UART has revised THRE int.  
semantics */

 #define PROBE_RSA	(1 << 0)
 #define PROBE_ANY	(~0)
===== 8250.c 1.140 vs edited =====
--- 1.140/drivers/serial/8250.c	2006-03-30 14:34:11 -05:00
+++ edited/8250.c	2006-03-30 18:36:35 -05:00
@@ -298,6 +298,10 @@ static inline int map_8250_out_reg(struc

 #endif

+#ifdef CONFIG_SERIAL_8250_CONSOLE
+static void thre_bug_test(struct uart_8250_port *up);
+#endif
+
 static unsigned int serial_in(struct uart_8250_port *up, int offset)
 {
 	offset = map_8250_in_reg(up, offset) << up->port.regshift;
@@ -1389,6 +1393,7 @@ static int serial_link_irq_chain(struct
 				  irq_flags, "serial", i);
 		if (ret < 0)
 			serial_do_unlink(i, up);
+
 	}

 	return ret;
@@ -1623,6 +1628,10 @@ static int serial8250_startup(struct uar
 		up->bugs &= ~UART_BUG_TXEN;
 	}

+#ifdef CONFIG_SERIAL_8250_CONSOLE
+	thre_bug_test(up);
+#endif
+
 	spin_unlock_irqrestore(&up->port.lock, flags);

 	/*
@@ -2182,6 +2191,37 @@ static inline void wait_for_xmitr(struct
 	}
 }

+static void thre_bug_test(struct uart_8250_port *up)
+{
+	unsigned char iir;
+
+	if (is_real_interrupt(up->port.irq) && !timer_pending(&up->timer)) {
+		/*
+		 * Test for UARTs that do not reassert THRE when the
+		 * transmitter is idle and the interrupt has already
+		 * been cleared.  Real 16550s should always reassert
+		 * this interrupt whenever the transmitter is idle and
+		 * the interrupt is enabled.
+		 */
+		wait_for_xmitr(up, BOTH_EMPTY);
+		serial_outp(up, UART_IER, UART_IER_THRI);
+		(void)serial_in(up, UART_IIR);
+		serial_outp(up, UART_IER, 0);
+		serial_outp(up, UART_IER, UART_IER_THRI);
+		iir = serial_in(up, UART_IIR);
+		serial_outp(up, UART_IER, 0);
+
+		/*
+		 * If the interrupt is not reasserted, setup a timer to
+		 * kick the UART on a regular basis.
+		 */
+		if (iir & UART_IIR_NO_INT) {
+			pr_debug("ttyS%d - enabling THRI workaround\n",
+				 up->port.line);
+		}
+	}
+}
+
 /*
  *	Print a string to the serial port trying not to disturb
  *	any possible real use of the port...
@@ -2229,9 +2269,12 @@ serial8250_console_write(struct console
 	 *	and restore the IER
 	 */
 	wait_for_xmitr(up, BOTH_EMPTY);
-	// up->ier |= UART_IER_THRI;
-	// serial_out(up, UART_IER, ier | UART_IER_THRI);
-	serial_out(up, UART_IER, ier);
+	if (up->bugs & UART_BUG_THRI) {
+		serial_out(up, UART_IER, ier);
+	} else {
+		up->ier |= UART_IER_THRI;
+		serial_out(up, UART_IER, ier | UART_IER_THRI);
+	}
 }

 static int serial8250_console_setup(struct console *co, char *options)

Attachment: PGP.sig
Description: This is a digitally signed message part