[03/16] tty: serial: 8250_core: read only RX if there is something in the FIFO

Commit Message

Sebastian Andrzej Siewior Feb. 13, 2015, 6:51 p.m. UTC

Something like this maybe?

Subject: [PATCH] serial: 8250: skip empty RX-read only on OMAP

The conditional RX-FIFO read seems to cause spurious interrupts and we
see just:
|serial8250: too much work for irq29

The previous behaviour was "default" for decades and Marvell's 88f6282 SoC
might not be the only that relies on it. Therefore this patch moves this
special OMAP3630 behaviour befind a BUG field.

Fixes: 0aa525d11859 ("tty: serial: 8250_core: read only RX if there is
       something in the FIFO")

Reported-By: Nicolas Schichan <[e-mail]>
Debuged-By: Peter Hurley <[e-mail]>
Signed-off-by: Sebastian Andrzej Siewior <[e-mail]>
---
 drivers/tty/serial/8250/8250.h      |  1 +
 drivers/tty/serial/8250/8250_core.c | 10 ++++++++--
 drivers/tty/serial/8250/8250_omap.c |  1 +
 3 files changed, 10 insertions(+), 2 deletions(-)

Comments

Russell King - ARM Linux Feb. 13, 2015, 11:15 p.m. UTC | #1

On Fri, Feb 13, 2015 at 07:51:16PM +0100, Sebastian Andrzej Siewior wrote:
> Something like this maybe?

My personal feeling is that as 0aa525d11859 was wrong, it should be
reverted and this should be another attempt to fix the problem.  In
other words, there should be two patches, one a revert of the previously
known bad commit and this one having another go at it.

I feel that would be a better approach, since then we don't end up
with this change building on a previously know buggy change.  It
would also make the changes to this solution from the previous,
known-to-work-for-decades code more obvious.

Patch

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index b00836851061..b6899fd69c7e 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -84,6 +84,7 @@  struct serial8250_config {
 #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 UART_BUG_PARITY	(1 << 4)	/* UART mishandles parity if FIFO enabled */
+#define UART_BUG_RXEMPT	(1 << 5)	/* UART can not read empty FIFO */
 
 #define PROBE_RSA	(1 << 0)
 #define PROBE_ANY	(~0)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index e3b9570a1eff..185386178023 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2137,8 +2137,13 @@  int serial8250_do_startup(struct uart_port *port)
 
 	/*
 	 * Clear the interrupt registers.
+	 *
+	 * This (and later) unsolicied read of the RX FIFO seems to clear the
+	 * RX timeout condition which otherwise generates spurious interrupt.
+	 * This behaviour has been observed on Marvell's 88f6282 SoC.
 	 */
-	if (serial_port_in(port, UART_LSR) & UART_LSR_DR)
+	if (!(up->bugs & UART_BUG_RXEMPT) ||
+	    (serial_port_in(port, UART_LSR) & UART_LSR_DR))
 		serial_port_in(port, UART_RX);
 	serial_port_in(port, UART_IIR);
 	serial_port_in(port, UART_MSR);
@@ -2300,7 +2305,8 @@  int serial8250_do_startup(struct uart_port *port)
 	 * saved flags to avoid getting false values from polling
 	 * routines or the previous session.
 	 */
-	if (serial_port_in(port, UART_LSR) & UART_LSR_DR)
+	if (!(up->bugs & UART_BUG_RXEMPT) ||
+	    (serial_port_in(port, UART_LSR) & UART_LSR_DR))
 		serial_port_in(port, UART_RX);
 	serial_port_in(port, UART_IIR);
 	serial_port_in(port, UART_MSR);
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index fe6d2e51da09..a7ead2fa6a32 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1021,6 +1021,7 @@  static int omap8250_probe(struct platform_device *pdev)
 	up.port.fifosize = 64;
 	up.tx_loadsz = 64;
 	up.capabilities = UART_CAP_FIFO;
+	up.bugs = UART_BUG_RXEMPT;
 #ifdef CONFIG_PM
 	/*
 	 * Runtime PM is mostly transparent. However to do it right we need to a