On Mon, Aug 11, 2008 at 10:32 PM, Andrew Morton
[off-list ref] wrote:

On Wed, 6 Aug 2008 11:53:13 +0100
"Will Newton" [off-list ref] wrote:

quoted

quoted

From 36ac82a231498247ada098d31e8d12e735eb34f2 Mon Sep 17 00:00:00 2001

From: Will Newton <redacted>
Date: Wed, 6 Aug 2008 11:48:29 +0100
Subject: [PATCH] 8250: Improve workaround for UARTs that don't
re-assert THRE correctly.

Recent changes to tighten the check for UARTs that don't correctly
re-assert THRE caused problems when such a UART was opened for the second
time - the bug could only successfully be detected at first initialization.
This patch stores the information about the bug in the bugs field of the
port structure when the port is first started up so subsequent opens can
check this bit even if the test for the bug fails.

Signed-off-by: Will Newton <redacted>
Acked-by: Alex Williamson <redacted>

What are the "recent changes" to which you refer?  I had a quick look
and didn't spot any THRE-related changes this year?

Sorry, I should have been more specific. Commit
01c194d9278efc15d4785ff205643e9c0bdcef53.

quoted

---
 drivers/serial/8250.c |   16 ++++++++++++----
 drivers/serial/8250.h |    1 +
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 342e12f..9ccc563 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c

@@ -1908,15 +1908,23 @@ static int serial8250_startup(struct uart_port *port)
               * kick the UART on a regular basis.
               */
              if (!(iir1 & UART_IIR_NO_INT) && (iir & UART_IIR_NO_INT)) {
+                     up->bugs |= UART_BUG_THRE;
                      pr_debug("ttyS%d - using backup timer\n", port->line);
-                     up->timer.function = serial8250_backup_timeout;
-                     up->timer.data = (unsigned long)up;
-                     mod_timer(&up->timer, jiffies +
-                             poll_timeout(up->port.timeout) + HZ / 5);
              }
      }

      /*
+      * The above check will only give an accurate result the first time
+      * the port is opened so this value needs to be preserved.
+      */
+     if (up->bugs & UART_BUG_THRE) {
+             up->timer.function = serial8250_backup_timeout;
+             up->timer.data = (unsigned long)up;
+             mod_timer(&up->timer, jiffies +
+                       poll_timeout(up->port.timeout) + HZ / 5);
+     }
+
+     /*
       * If the "interrupt" for this port doesn't correspond with any
       * hardware interrupt, we use a timer-based system.  The original
       * driver used to do this with IRQ0.

diff --git a/drivers/serial/8250.h b/drivers/serial/8250.h
index 78c0016..5202603 100644
--- a/drivers/serial/8250.h
+++ b/drivers/serial/8250.h

@@ -47,6 +47,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_THRE        (1 << 3)        /* UART has buggy THRE reassertion */

 #define PROBE_RSA    (1 << 0)
 #define PROBE_ANY    (~0)

Also, how serious is the problem which is being fixed here?  It
_sounds_ like it's of the "fatal for people who have that hardware"
variety, in which case we should get this into 2.6.27 and probably
2.6.26.x.  Not sure about 2.5.26.x though - the patch doesn't apply
there, but I didn't check whether this is due to functional changes.

For users of this version of this particular UART IP it is fatal. From
looking at the git history it looks like the original patch went into
2.6.26 so it might also affect that kernel.

I'll look into it. Some of the weirdness comes from the way the timer
is used for this bug workaround but also for irq-less ports.