[PATCH] Re: Serial: UART_BUG_TXEN race conditions
From: Ingo van Lil
Date: Wed Jun 28 2006 - 03:59:00 EST
On Mon, Jun 26, 2006 at 09:25:36PM +0100, Russell King wrote:
> > 1. What if the IIR actually equals UART_IIR_THRI at that point? The
> > read access will clear the interrupt condition and the workaround
> > will effect the actual opposite of its intention: Neither
> > serial8250_start_tx() nor the interrupt handler will start
> > transmitting characters for the ring buffer.
>
> Gah, looks like you're right - reading the IIR will clear the transmit
> pending interrupt, so we should probably just load the transmitter up
> with characters anyway if the TEMT bit is set.
Proposed patch. It's my first submission, I hope it meets this list's
standards.
The patch drops the buggy UART detection and always writes a first load
of characters into the transmitter FIFO if it's currently idle. This
should be fine for both well-behaved and broken UARTs.
Cheers,
Ingo
Signed-off-by: Ingo van Lil <inguin@xxxxxx>
---
diff -u linux-2.6.17.1/drivers/serial/8250.c.orig linux-2.6.17.1/drivers/serial/8250.c
--- linux-2.6.17.1/drivers/serial/8250.c.orig 2006-06-27 21:37:41.000000000 +0200
+++ linux-2.6.17.1/drivers/serial/8250.c 2006-06-27 21:56:20.000000000 +0200
@@ -1119,18 +1119,13 @@
static void serial8250_start_tx(struct uart_port *port)
{
struct uart_8250_port *up = (struct uart_8250_port *)port;
+ unsigned char lsr;
if (!(up->ier & UART_IER_THRI)) {
up->ier |= UART_IER_THRI;
serial_out(up, UART_IER, up->ier);
-
- if (up->bugs & UART_BUG_TXEN) {
- unsigned char lsr, iir;
- lsr = serial_in(up, UART_LSR);
- iir = serial_in(up, UART_IIR);
- if (lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT)
- transmit_chars(up);
- }
+ lsr = serial_in(up, UART_LSR);
+ if (lsr & UART_LSR_TEMT) transmit_chars(up);
}
/*
@@ -1529,7 +1524,6 @@
{
struct uart_8250_port *up = (struct uart_8250_port *)port;
unsigned long flags;
- unsigned char lsr, iir;
int retval;
up->capabilities = uart_config[up->port.type].flags;
@@ -1634,25 +1628,6 @@
serial8250_set_mctrl(&up->port, up->port.mctrl);
- /*
- * Do a quick test to see if we receive an
- * interrupt when we enable the TX irq.
- */
- serial_outp(up, UART_IER, UART_IER_THRI);
- lsr = serial_in(up, UART_LSR);
- iir = serial_in(up, UART_IIR);
- serial_outp(up, UART_IER, 0);
-
- if (lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT) {
- if (!(up->bugs & UART_BUG_TXEN)) {
- up->bugs |= UART_BUG_TXEN;
- pr_debug("ttyS%d - enabling bad tx status workarounds\n",
- port->line);
- }
- } else {
- up->bugs &= ~UART_BUG_TXEN;
- }
-
spin_unlock_irqrestore(&up->port.lock, flags);
/*
diff -u linux-2.6.17.1/drivers/serial/8250.h.orig linux-2.6.17.1/drivers/serial/8250.h
--- linux-2.6.17.1/drivers/serial/8250.h.orig 2006-06-27 21:46:13.000000000 +0200
+++ linux-2.6.17.1/drivers/serial/8250.h 2006-06-27 21:46:35.000000000 +0200
@@ -48,8 +48,7 @@
#define UART_CAP_UUE (1 << 12) /* UART needs IER bit 6 set (Xscale) */
#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_NOMSR (1 << 1) /* UART has buggy MSR status bits (Au1x00) */
#define PROBE_RSA (1 << 0)
#define PROBE_ANY (~0)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/