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

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

---

• Follow-Ups:
  • Re: [PATCH] 8250: yet another attempt at a serial console fix
    • From: Georg Nikodym <[email protected]>

• Prev by Date: Re: [patch] PI-futex patchset: -V4
• Next by Date: Re: [PATCH 2/4] locks: don't unnecessarily fail posix lock operations
• Previous by thread: ACPI Error (Method parse/execution failed) with 2.6.16 kernel
• Next by thread: Re: [PATCH] 8250: yet another attempt at a serial console fix
• Index(es):
  • Date
  • Thread

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]