linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] tty: xuartps: Fix lock ups
@ 2015-11-04  0:20 Soren Brinkmann
  2015-11-04  0:20 ` [PATCH 1/9] tty: xuartps: Beautify read-modify writes Soren Brinkmann
                   ` (8 more replies)
  0 siblings, 9 replies; 11+ messages in thread
From: Soren Brinkmann @ 2015-11-04  0:20 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

I recently found my system locking up under some conditions. I dug
through the code a bit and the result is this collections of various
changes. Some of it may not be required and has just been created out of
half-baked theories and re-reading the documentation. The actual failing
scenarios seem to be:
 - RX IRQ conditions are handled while the receiver is disabled, leaving
   the driver in an infinite loop
 - console_put_char seems to be interrupted by uart_shutdown disabling
   the transmitter while/just before printing

I.e. overall I tried to serialize all operations using the port lock to
avoid such interaction.

	S?ren

S?ren Brinkmann (9):
  tty: xuartps: Beautify read-modify writes
  tty: xuartps: Use spinlock to serialize HW access
  tty: xuartps: Always enable transmitter in start_tx
  tty: xuartps: Clear interrupt status register in shutdown
  tty: xuartps: Improve startup function
  tty: xuartps: Keep lock for whole ISR
  tty: xuartps: Acquire port lock for shutdown
  tty: xuartps: Move RX path into helper function
  tty: xuartps: Only handle RX IRQs when RX is enabled

 drivers/tty/serial/xilinx_uartps.c | 129 ++++++++++++++++++++++---------------
 1 file changed, 78 insertions(+), 51 deletions(-)

-- 
2.6.2.3.ga463a5b

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/9] tty: xuartps: Beautify read-modify writes
  2015-11-04  0:20 [PATCH 0/9] tty: xuartps: Fix lock ups Soren Brinkmann
@ 2015-11-04  0:20 ` Soren Brinkmann
  2015-11-04  0:20 ` [PATCH 2/9] tty: xuartps: Use spinlock to serialize HW access Soren Brinkmann
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Soren Brinkmann @ 2015-11-04  0:20 UTC (permalink / raw)
  To: linux-arm-kernel

Non-functional, formatting changes to ease reading the code.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 009e0dbc12d2..50d4082d2354 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -515,12 +515,14 @@ static void cdns_uart_start_tx(struct uart_port *port)
 	if (uart_circ_empty(&port->state->xmit) || uart_tx_stopped(port))
 		return;
 
-	status = readl(port->membase + CDNS_UART_CR_OFFSET);
-	/* Set the TX enable bit and clear the TX disable bit to enable the
+	/*
+	 * Set the TX enable bit and clear the TX disable bit to enable the
 	 * transmitter.
 	 */
-	writel((status & ~CDNS_UART_CR_TX_DIS) | CDNS_UART_CR_TX_EN,
-			port->membase + CDNS_UART_CR_OFFSET);
+	status = readl(port->membase + CDNS_UART_CR_OFFSET);
+	status &= ~CDNS_UART_CR_TX_DIS;
+	status |= CDNS_UART_CR_TX_EN;
+	writel(status, port->membase + CDNS_UART_CR_OFFSET);
 
 	while (numbytes-- && ((readl(port->membase + CDNS_UART_SR_OFFSET) &
 				CDNS_UART_SR_TXFULL)) != CDNS_UART_SR_TXFULL) {
@@ -1123,8 +1125,9 @@ static void cdns_uart_console_write(struct console *co, const char *s,
 	 * clear the TX disable bit to enable the transmitter.
 	 */
 	ctrl = readl(port->membase + CDNS_UART_CR_OFFSET);
-	writel((ctrl & ~CDNS_UART_CR_TX_DIS) | CDNS_UART_CR_TX_EN,
-			port->membase + CDNS_UART_CR_OFFSET);
+	ctrl &= ~CDNS_UART_CR_TX_DIS;
+	ctrl |= CDNS_UART_CR_TX_EN;
+	writel(ctrl, port->membase + CDNS_UART_CR_OFFSET);
 
 	uart_console_write(port, s, count, cdns_uart_console_putchar);
 	cdns_uart_console_wait_tx(port);
-- 
2.6.2.3.ga463a5b

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/9] tty: xuartps: Use spinlock to serialize HW access
  2015-11-04  0:20 [PATCH 0/9] tty: xuartps: Fix lock ups Soren Brinkmann
  2015-11-04  0:20 ` [PATCH 1/9] tty: xuartps: Beautify read-modify writes Soren Brinkmann
@ 2015-11-04  0:20 ` Soren Brinkmann
  2015-11-04  0:52   ` kbuild test robot
  2015-11-04  0:20 ` [PATCH 3/9] tty: xuartps: Always enable transmitter in start_tx Soren Brinkmann
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 11+ messages in thread
From: Soren Brinkmann @ 2015-11-04  0:20 UTC (permalink / raw)
  To: linux-arm-kernel

Instead of disabling the IRQ, use the spin lock to serialize accesses to
the HW. This protects the driver from interference of non-IRQ callbacks
with each other and makes the driver more consistent in its
serialization method.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 50d4082d2354..dc1987f3dc0a 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -947,10 +947,9 @@ static int cdns_uart_poll_get_char(struct uart_port *port)
 {
 	u32 imr;
 	int c;
+	unsigned long flags;
 
-	/* Disable all interrupts */
-	imr = readl(port->membase + CDNS_UART_IMR_OFFSET);
-	writel(imr, port->membase + CDNS_UART_IDR_OFFSET);
+	spin_lock_irqsave(&port->lock, flags);
 
 	/* Check if FIFO is empty */
 	if (readl(port->membase + CDNS_UART_SR_OFFSET) & CDNS_UART_SR_RXEMPTY)
@@ -959,8 +958,7 @@ static int cdns_uart_poll_get_char(struct uart_port *port)
 		c = (unsigned char) readl(
 					port->membase + CDNS_UART_FIFO_OFFSET);
 
-	/* Enable interrupts */
-	writel(imr, port->membase + CDNS_UART_IER_OFFSET);
+	spin_unlock_irqrestore(&port->lock, flags);
 
 	return c;
 }
@@ -968,10 +966,9 @@ static int cdns_uart_poll_get_char(struct uart_port *port)
 static void cdns_uart_poll_put_char(struct uart_port *port, unsigned char c)
 {
 	u32 imr;
+	unsigned long flags;
 
-	/* Disable all interrupts */
-	imr = readl(port->membase + CDNS_UART_IMR_OFFSET);
-	writel(imr, port->membase + CDNS_UART_IDR_OFFSET);
+	spin_lock_irqsave(&port->lock, flags);
 
 	/* Wait until FIFO is empty */
 	while (!(readl(port->membase + CDNS_UART_SR_OFFSET) &
@@ -986,8 +983,7 @@ static void cdns_uart_poll_put_char(struct uart_port *port, unsigned char c)
 				CDNS_UART_SR_TXEMPTY))
 		cpu_relax();
 
-	/* Enable interrupts */
-	writel(imr, port->membase + CDNS_UART_IER_OFFSET);
+	spin_unlock_irqrestore(&port->lock, flags);
 
 	return;
 }
-- 
2.6.2.3.ga463a5b

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/9] tty: xuartps: Always enable transmitter in start_tx
  2015-11-04  0:20 [PATCH 0/9] tty: xuartps: Fix lock ups Soren Brinkmann
  2015-11-04  0:20 ` [PATCH 1/9] tty: xuartps: Beautify read-modify writes Soren Brinkmann
  2015-11-04  0:20 ` [PATCH 2/9] tty: xuartps: Use spinlock to serialize HW access Soren Brinkmann
@ 2015-11-04  0:20 ` Soren Brinkmann
  2015-11-04  0:20 ` [PATCH 4/9] tty: xuartps: Clear interrupt status register in shutdown Soren Brinkmann
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Soren Brinkmann @ 2015-11-04  0:20 UTC (permalink / raw)
  To: linux-arm-kernel

start_tx must start transmitting characters. Regardless of the state of
the circular buffer, always enable the transmitter hardware.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index dc1987f3dc0a..5edd1efca015 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -512,9 +512,6 @@ static void cdns_uart_start_tx(struct uart_port *port)
 {
 	unsigned int status, numbytes = port->fifosize;
 
-	if (uart_circ_empty(&port->state->xmit) || uart_tx_stopped(port))
-		return;
-
 	/*
 	 * Set the TX enable bit and clear the TX disable bit to enable the
 	 * transmitter.
@@ -524,6 +521,9 @@ static void cdns_uart_start_tx(struct uart_port *port)
 	status |= CDNS_UART_CR_TX_EN;
 	writel(status, port->membase + CDNS_UART_CR_OFFSET);
 
+	if (uart_circ_empty(&port->state->xmit) || uart_tx_stopped(port))
+		return;
+
 	while (numbytes-- && ((readl(port->membase + CDNS_UART_SR_OFFSET) &
 				CDNS_UART_SR_TXFULL)) != CDNS_UART_SR_TXFULL) {
 		/* Break if no more data available in the UART buffer */
-- 
2.6.2.3.ga463a5b

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/9] tty: xuartps: Clear interrupt status register in shutdown
  2015-11-04  0:20 [PATCH 0/9] tty: xuartps: Fix lock ups Soren Brinkmann
                   ` (2 preceding siblings ...)
  2015-11-04  0:20 ` [PATCH 3/9] tty: xuartps: Always enable transmitter in start_tx Soren Brinkmann
@ 2015-11-04  0:20 ` Soren Brinkmann
  2015-11-04  0:20 ` [PATCH 5/9] tty: xuartps: Improve startup function Soren Brinkmann
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Soren Brinkmann @ 2015-11-04  0:20 UTC (permalink / raw)
  To: linux-arm-kernel

When shutting down the UART, clear the interrupt status register.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 5edd1efca015..738df6bb2646 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -825,6 +825,7 @@ static void cdns_uart_shutdown(struct uart_port *port)
 	/* Disable interrupts */
 	status = readl(port->membase + CDNS_UART_IMR_OFFSET);
 	writel(status, port->membase + CDNS_UART_IDR_OFFSET);
+	writel(0xffffffff, port->membase + CDNS_UART_ISR_OFFSET);
 
 	/* Disable the TX and RX */
 	writel(CDNS_UART_CR_TX_DIS | CDNS_UART_CR_RX_DIS,
-- 
2.6.2.3.ga463a5b

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/9] tty: xuartps: Improve startup function
  2015-11-04  0:20 [PATCH 0/9] tty: xuartps: Fix lock ups Soren Brinkmann
                   ` (3 preceding siblings ...)
  2015-11-04  0:20 ` [PATCH 4/9] tty: xuartps: Clear interrupt status register in shutdown Soren Brinkmann
@ 2015-11-04  0:20 ` Soren Brinkmann
  2015-11-04  0:20 ` [PATCH 6/9] tty: xuartps: Keep lock for whole ISR Soren Brinkmann
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Soren Brinkmann @ 2015-11-04  0:20 UTC (permalink / raw)
  To: linux-arm-kernel

The startup function is supposed to initialize the UART for receiving.
Hence, don't enable the TX part. Also, protect HW accesses with the port
lock.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 738df6bb2646..b189e600d4ab 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -755,6 +755,7 @@ static void cdns_uart_set_termios(struct uart_port *port,
  */
 static int cdns_uart_startup(struct uart_port *port)
 {
+	unsigned long flags;
 	unsigned int retval = 0, status = 0;
 
 	retval = request_irq(port->irq, cdns_uart_isr, 0, CDNS_UART_NAME,
@@ -762,6 +763,8 @@ static int cdns_uart_startup(struct uart_port *port)
 	if (retval)
 		return retval;
 
+	spin_lock_irqsave(&port->lock, flags);
+
 	/* Disable the TX and RX */
 	writel(CDNS_UART_CR_TX_DIS | CDNS_UART_CR_RX_DIS,
 			port->membase + CDNS_UART_CR_OFFSET);
@@ -772,15 +775,14 @@ static int cdns_uart_startup(struct uart_port *port)
 	writel(CDNS_UART_CR_TXRST | CDNS_UART_CR_RXRST,
 			port->membase + CDNS_UART_CR_OFFSET);
 
-	status = readl(port->membase + CDNS_UART_CR_OFFSET);
-
-	/* Clear the RX disable and TX disable bits and then set the TX enable
-	 * bit and RX enable bit to enable the transmitter and receiver.
+	/*
+	 * Clear the RX disable bit and then set the RX enable bit to enable
+	 * the receiver.
 	 */
-	writel((status & ~(CDNS_UART_CR_TX_DIS | CDNS_UART_CR_RX_DIS))
-			| (CDNS_UART_CR_TX_EN | CDNS_UART_CR_RX_EN |
-			CDNS_UART_CR_STOPBRK),
-			port->membase + CDNS_UART_CR_OFFSET);
+	status = readl(port->membase + CDNS_UART_CR_OFFSET);
+	status &= CDNS_UART_CR_RX_DIS;
+	status |= CDNS_UART_CR_RX_EN;
+	writel(status, port->membase + CDNS_UART_CR_OFFSET);
 
 	/* Set the Mode Register with normal mode,8 data bits,1 stop bit,
 	 * no parity.
@@ -811,6 +813,8 @@ static int cdns_uart_startup(struct uart_port *port)
 		CDNS_UART_IXR_RXTRIG | CDNS_UART_IXR_TOUT,
 		port->membase + CDNS_UART_IER_OFFSET);
 
+	spin_unlock_irqrestore(&port->lock, flags);
+
 	return retval;
 }
 
-- 
2.6.2.3.ga463a5b

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/9] tty: xuartps: Keep lock for whole ISR
  2015-11-04  0:20 [PATCH 0/9] tty: xuartps: Fix lock ups Soren Brinkmann
                   ` (4 preceding siblings ...)
  2015-11-04  0:20 ` [PATCH 5/9] tty: xuartps: Improve startup function Soren Brinkmann
@ 2015-11-04  0:20 ` Soren Brinkmann
  2015-11-04  0:20 ` [PATCH 7/9] tty: xuartps: Acquire port lock for shutdown Soren Brinkmann
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Soren Brinkmann @ 2015-11-04  0:20 UTC (permalink / raw)
  To: linux-arm-kernel

The RX path in the interrupt handler released a lock unnecessarily.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index b189e600d4ab..544ea13e2e93 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -265,9 +265,7 @@ static irqreturn_t cdns_uart_isr(int irq, void *dev_id)
 			uart_insert_char(port, isrstatus, CDNS_UART_IXR_OVERRUN,
 					data, status);
 		}
-		spin_unlock(&port->lock);
 		tty_flip_buffer_push(&port->state->port);
-		spin_lock(&port->lock);
 	}
 
 	/* Dispatch an appropriate handler */
-- 
2.6.2.3.ga463a5b

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 7/9] tty: xuartps: Acquire port lock for shutdown
  2015-11-04  0:20 [PATCH 0/9] tty: xuartps: Fix lock ups Soren Brinkmann
                   ` (5 preceding siblings ...)
  2015-11-04  0:20 ` [PATCH 6/9] tty: xuartps: Keep lock for whole ISR Soren Brinkmann
@ 2015-11-04  0:20 ` Soren Brinkmann
  2015-11-04  0:20 ` [PATCH 8/9] tty: xuartps: Move RX path into helper function Soren Brinkmann
  2015-11-04  0:20 ` [PATCH 9/9] tty: xuartps: Only handle RX IRQs when RX is enabled Soren Brinkmann
  8 siblings, 0 replies; 11+ messages in thread
From: Soren Brinkmann @ 2015-11-04  0:20 UTC (permalink / raw)
  To: linux-arm-kernel

Shutting down the UART port can happen while console operations are in
progress. Holding the port lock serializes these operations and avoids
the UART HW to be disabled in the middle of console prints.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 544ea13e2e93..4904eec37621 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -823,6 +823,9 @@ static int cdns_uart_startup(struct uart_port *port)
 static void cdns_uart_shutdown(struct uart_port *port)
 {
 	int status;
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
 
 	/* Disable interrupts */
 	status = readl(port->membase + CDNS_UART_IMR_OFFSET);
@@ -832,6 +835,9 @@ static void cdns_uart_shutdown(struct uart_port *port)
 	/* Disable the TX and RX */
 	writel(CDNS_UART_CR_TX_DIS | CDNS_UART_CR_RX_DIS,
 			port->membase + CDNS_UART_CR_OFFSET);
+
+	spin_unlock_irqrestore(&port->lock, flags);
+
 	free_irq(port->irq, port);
 }
 
-- 
2.6.2.3.ga463a5b

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 8/9] tty: xuartps: Move RX path into helper function
  2015-11-04  0:20 [PATCH 0/9] tty: xuartps: Fix lock ups Soren Brinkmann
                   ` (6 preceding siblings ...)
  2015-11-04  0:20 ` [PATCH 7/9] tty: xuartps: Acquire port lock for shutdown Soren Brinkmann
@ 2015-11-04  0:20 ` Soren Brinkmann
  2015-11-04  0:20 ` [PATCH 9/9] tty: xuartps: Only handle RX IRQs when RX is enabled Soren Brinkmann
  8 siblings, 0 replies; 11+ messages in thread
From: Soren Brinkmann @ 2015-11-04  0:20 UTC (permalink / raw)
  To: linux-arm-kernel

Move RX-related IRQ handling into a helper function.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 50 +++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 4904eec37621..149868cc003c 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -172,28 +172,8 @@ struct cdns_uart {
 #define to_cdns_uart(_nb) container_of(_nb, struct cdns_uart, \
 		clk_rate_change_nb);
 
-/**
- * cdns_uart_isr - Interrupt handler
- * @irq: Irq number
- * @dev_id: Id of the port
- *
- * Return: IRQHANDLED
- */
-static irqreturn_t cdns_uart_isr(int irq, void *dev_id)
+static void cdns_uart_handle_rx(struct uart_port *port, unsigned int isrstatus)
 {
-	struct uart_port *port = (struct uart_port *)dev_id;
-	unsigned long flags;
-	unsigned int isrstatus, numbytes;
-	unsigned int data;
-	char status = TTY_NORMAL;
-
-	spin_lock_irqsave(&port->lock, flags);
-
-	/* Read the interrupt status register to determine which
-	 * interrupt(s) is/are active.
-	 */
-	isrstatus = readl(port->membase + CDNS_UART_ISR_OFFSET);
-
 	/*
 	 * There is no hardware break detection, so we interpret framing
 	 * error with all-zeros data as a break sequence. Most of the time,
@@ -223,6 +203,9 @@ static irqreturn_t cdns_uart_isr(int irq, void *dev_id)
 		/* Receive Timeout Interrupt */
 		while (!(readl(port->membase + CDNS_UART_SR_OFFSET) &
 					CDNS_UART_SR_RXEMPTY)) {
+			u32 data;
+			char status = TTY_NORMAL;
+
 			data = readl(port->membase + CDNS_UART_FIFO_OFFSET);
 
 			/* Non-NULL byte after BREAK is garbage (99%) */
@@ -263,10 +246,33 @@ static irqreturn_t cdns_uart_isr(int irq, void *dev_id)
 			}
 
 			uart_insert_char(port, isrstatus, CDNS_UART_IXR_OVERRUN,
-					data, status);
+					 data, status);
 		}
 		tty_flip_buffer_push(&port->state->port);
 	}
+}
+
+/**
+ * cdns_uart_isr - Interrupt handler
+ * @irq: Irq number
+ * @dev_id: Id of the port
+ *
+ * Return: IRQHANDLED
+ */
+static irqreturn_t cdns_uart_isr(int irq, void *dev_id)
+{
+	struct uart_port *port = (struct uart_port *)dev_id;
+	unsigned long flags;
+	unsigned int isrstatus, numbytes;
+
+	spin_lock_irqsave(&port->lock, flags);
+
+	/* Read the interrupt status register to determine which
+	 * interrupt(s) is/are active.
+	 */
+	isrstatus = readl(port->membase + CDNS_UART_ISR_OFFSET);
+
+	cdns_uart_handle_rx(port, isrstatus);
 
 	/* Dispatch an appropriate handler */
 	if ((isrstatus & CDNS_UART_IXR_TXEMPTY) == CDNS_UART_IXR_TXEMPTY) {
-- 
2.6.2.3.ga463a5b

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 9/9] tty: xuartps: Only handle RX IRQs when RX is enabled
  2015-11-04  0:20 [PATCH 0/9] tty: xuartps: Fix lock ups Soren Brinkmann
                   ` (7 preceding siblings ...)
  2015-11-04  0:20 ` [PATCH 8/9] tty: xuartps: Move RX path into helper function Soren Brinkmann
@ 2015-11-04  0:20 ` Soren Brinkmann
  8 siblings, 0 replies; 11+ messages in thread
From: Soren Brinkmann @ 2015-11-04  0:20 UTC (permalink / raw)
  To: linux-arm-kernel

Ignore RX-related interrupts if RX is not enabled.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 149868cc003c..76a060a68934 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -161,6 +161,7 @@ MODULE_PARM_DESC(rx_timeout, "Rx timeout, 1-255");
  * @pclk:		APB clock
  * @baud:		Current baud rate
  * @clk_rate_change_nb:	Notifier block for clock changes
+ * @flags:		Driver flags
  */
 struct cdns_uart {
 	struct uart_port	*port;
@@ -168,7 +169,11 @@ struct cdns_uart {
 	struct clk		*pclk;
 	unsigned int		baud;
 	struct notifier_block	clk_rate_change_nb;
+	u32			flags;
 };
+
+#define CDNS_FLAG_RX_ENABLED	BIT(0)
+
 #define to_cdns_uart(_nb) container_of(_nb, struct cdns_uart, \
 		clk_rate_change_nb);
 
@@ -262,6 +267,7 @@ static void cdns_uart_handle_rx(struct uart_port *port, unsigned int isrstatus)
 static irqreturn_t cdns_uart_isr(int irq, void *dev_id)
 {
 	struct uart_port *port = (struct uart_port *)dev_id;
+	struct cdns_uart *cdns_uart = port->private_data;
 	unsigned long flags;
 	unsigned int isrstatus, numbytes;
 
@@ -272,7 +278,8 @@ static irqreturn_t cdns_uart_isr(int irq, void *dev_id)
 	 */
 	isrstatus = readl(port->membase + CDNS_UART_ISR_OFFSET);
 
-	cdns_uart_handle_rx(port, isrstatus);
+	if (cdns_uart->flags & CDNS_FLAG_RX_ENABLED)
+		cdns_uart_handle_rx(port, isrstatus);
 
 	/* Dispatch an appropriate handler */
 	if ((isrstatus & CDNS_UART_IXR_TXEMPTY) == CDNS_UART_IXR_TXEMPTY) {
@@ -576,11 +583,13 @@ static void cdns_uart_stop_tx(struct uart_port *port)
 static void cdns_uart_stop_rx(struct uart_port *port)
 {
 	unsigned int regval;
+	struct cdns_uart *cdns_uart = port->private_data;
 
 	regval = readl(port->membase + CDNS_UART_CR_OFFSET);
 	regval |= CDNS_UART_CR_RX_DIS;
 	/* Disable the receiver */
 	writel(regval, port->membase + CDNS_UART_CR_OFFSET);
+	cdns_uart->flags &= ~CDNS_FLAG_RX_ENABLED;
 }
 
 /**
@@ -761,6 +770,7 @@ static int cdns_uart_startup(struct uart_port *port)
 {
 	unsigned long flags;
 	unsigned int retval = 0, status = 0;
+	struct cdns_uart *cdns_uart = port->private_data;
 
 	retval = request_irq(port->irq, cdns_uart_isr, 0, CDNS_UART_NAME,
 								(void *)port);
@@ -787,6 +797,7 @@ static int cdns_uart_startup(struct uart_port *port)
 	status &= CDNS_UART_CR_RX_DIS;
 	status |= CDNS_UART_CR_RX_EN;
 	writel(status, port->membase + CDNS_UART_CR_OFFSET);
+	cdns_uart->flags |= CDNS_FLAG_RX_ENABLED;
 
 	/* Set the Mode Register with normal mode,8 data bits,1 stop bit,
 	 * no parity.
@@ -830,6 +841,7 @@ static void cdns_uart_shutdown(struct uart_port *port)
 {
 	int status;
 	unsigned long flags;
+	struct cdns_uart *cdns_uart = port->private_data;
 
 	spin_lock_irqsave(&port->lock, flags);
 
@@ -841,6 +853,7 @@ static void cdns_uart_shutdown(struct uart_port *port)
 	/* Disable the TX and RX */
 	writel(CDNS_UART_CR_TX_DIS | CDNS_UART_CR_RX_DIS,
 			port->membase + CDNS_UART_CR_OFFSET);
+	cdns_uart->flags &= ~CDNS_FLAG_RX_ENABLED;
 
 	spin_unlock_irqrestore(&port->lock, flags);
 
-- 
2.6.2.3.ga463a5b

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/9] tty: xuartps: Use spinlock to serialize HW access
  2015-11-04  0:20 ` [PATCH 2/9] tty: xuartps: Use spinlock to serialize HW access Soren Brinkmann
@ 2015-11-04  0:52   ` kbuild test robot
  0 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2015-11-04  0:52 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Soren,

[auto build test WARNING on v4.3-rc7]
[also WARNING on: next-20151103]

url:    https://github.com/0day-ci/linux/commits/Soren-Brinkmann/tty-xuartps-Beautify-read-modify-writes/20151104-082546
base:   https://github.com/0day-ci/linux Soren-Brinkmann/tty-xuartps-Beautify-read-modify-writes/20151104-082546
config: i386-allmodconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/tty/serial/xilinx_uartps.c: In function 'cdns_uart_poll_get_char':
>> drivers/tty/serial/xilinx_uartps.c:948:6: warning: unused variable 'imr' [-Wunused-variable]
     u32 imr;
         ^
   drivers/tty/serial/xilinx_uartps.c: In function 'cdns_uart_poll_put_char':
   drivers/tty/serial/xilinx_uartps.c:968:6: warning: unused variable 'imr' [-Wunused-variable]
     u32 imr;
         ^

vim +/imr +948 drivers/tty/serial/xilinx_uartps.c

19038ad9f Lars-Peter Clausen 2014-11-05  932  
19f22efdb Thomas Betker      2015-03-12  933  	val = readl(port->membase + CDNS_UART_MODEMCR_OFFSET);
19038ad9f Lars-Peter Clausen 2014-11-05  934  
19038ad9f Lars-Peter Clausen 2014-11-05  935  	val &= ~(CDNS_UART_MODEMCR_RTS | CDNS_UART_MODEMCR_DTR);
19038ad9f Lars-Peter Clausen 2014-11-05  936  
19038ad9f Lars-Peter Clausen 2014-11-05  937  	if (mctrl & TIOCM_RTS)
19038ad9f Lars-Peter Clausen 2014-11-05  938  		val |= CDNS_UART_MODEMCR_RTS;
19038ad9f Lars-Peter Clausen 2014-11-05  939  	if (mctrl & TIOCM_DTR)
19038ad9f Lars-Peter Clausen 2014-11-05  940  		val |= CDNS_UART_MODEMCR_DTR;
19038ad9f Lars-Peter Clausen 2014-11-05  941  
19f22efdb Thomas Betker      2015-03-12  942  	writel(val, port->membase + CDNS_UART_MODEMCR_OFFSET);
61ec90169 John Linn          2011-04-30  943  }
61ec90169 John Linn          2011-04-30  944  
6ee04c6c5 Vlad Lungu         2013-10-17  945  #ifdef CONFIG_CONSOLE_POLL
d9bb3fb12 Soren Brinkmann    2014-04-04  946  static int cdns_uart_poll_get_char(struct uart_port *port)
6ee04c6c5 Vlad Lungu         2013-10-17  947  {
6ee04c6c5 Vlad Lungu         2013-10-17 @948  	u32 imr;
6ee04c6c5 Vlad Lungu         2013-10-17  949  	int c;
c2fd7b9d5 Soren Brinkmann    2015-11-03  950  	unsigned long flags;
6ee04c6c5 Vlad Lungu         2013-10-17  951  
c2fd7b9d5 Soren Brinkmann    2015-11-03  952  	spin_lock_irqsave(&port->lock, flags);
6ee04c6c5 Vlad Lungu         2013-10-17  953  
6ee04c6c5 Vlad Lungu         2013-10-17  954  	/* Check if FIFO is empty */
19f22efdb Thomas Betker      2015-03-12  955  	if (readl(port->membase + CDNS_UART_SR_OFFSET) & CDNS_UART_SR_RXEMPTY)
6ee04c6c5 Vlad Lungu         2013-10-17  956  		c = NO_POLL_CHAR;

:::::: The code at line 948 was first introduced by commit
:::::: 6ee04c6c5488b2b7fdfa22c771c127411f86e610 tty: xuartps: Add polled mode support for xuartps

:::::: TO: Vlad Lungu <vlad.lungu@windriver.com>
:::::: CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 51591 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151104/4b67ff78/attachment-0001.obj>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-11-04  0:52 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-04  0:20 [PATCH 0/9] tty: xuartps: Fix lock ups Soren Brinkmann
2015-11-04  0:20 ` [PATCH 1/9] tty: xuartps: Beautify read-modify writes Soren Brinkmann
2015-11-04  0:20 ` [PATCH 2/9] tty: xuartps: Use spinlock to serialize HW access Soren Brinkmann
2015-11-04  0:52   ` kbuild test robot
2015-11-04  0:20 ` [PATCH 3/9] tty: xuartps: Always enable transmitter in start_tx Soren Brinkmann
2015-11-04  0:20 ` [PATCH 4/9] tty: xuartps: Clear interrupt status register in shutdown Soren Brinkmann
2015-11-04  0:20 ` [PATCH 5/9] tty: xuartps: Improve startup function Soren Brinkmann
2015-11-04  0:20 ` [PATCH 6/9] tty: xuartps: Keep lock for whole ISR Soren Brinkmann
2015-11-04  0:20 ` [PATCH 7/9] tty: xuartps: Acquire port lock for shutdown Soren Brinkmann
2015-11-04  0:20 ` [PATCH 8/9] tty: xuartps: Move RX path into helper function Soren Brinkmann
2015-11-04  0:20 ` [PATCH 9/9] tty: xuartps: Only handle RX IRQs when RX is enabled Soren Brinkmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).