Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1274782 > unrolled thread

[PATCH LINUX v3 00/10] tty: xuartps: Fix lock ups

Started bySoren Brinkmann <soren.brinkmann@xilinx.com>
First post2015-11-22 04:00 +0100
Last post2015-11-22 04:10 +0100
Articles 7 — 1 participant

Back to article view | Back to linux.kernel

Contents

  [PATCH LINUX v3 00/10] tty: xuartps: Fix lock ups Soren Brinkmann <soren.brinkmann@xilinx.com> - 2015-11-22 04:00 +0100
    [PATCH LINUX v3 07/10] tty: xuartps: Acquire port lock for shutdown Soren Brinkmann <soren.brinkmann@xilinx.com> - 2015-11-22 04:00 +0100
    [PATCH LINUX v3 01/10] tty: xuartps: Beautify read-modify writes Soren Brinkmann <soren.brinkmann@xilinx.com> - 2015-11-22 04:10 +0100
    [PATCH LINUX v3 09/10] tty: xuartps: Only handle RX IRQs when RX is enabled Soren Brinkmann <soren.brinkmann@xilinx.com> - 2015-11-22 04:10 +0100
    [PATCH LINUX v3 03/10] tty: xuartps: Don't consider circular buffer when enabling transmitter Soren Brinkmann <soren.brinkmann@xilinx.com> - 2015-11-22 04:10 +0100
    [PATCH LINUX v3 04/10] tty: xuartps: Clear interrupt status register in shutdown Soren Brinkmann <soren.brinkmann@xilinx.com> - 2015-11-22 04:10 +0100
    [PATCH LINUX v3 05/10] tty: xuartps: Improve startup function Soren Brinkmann <soren.brinkmann@xilinx.com> - 2015-11-22 04:10 +0100

#1274782 — [PATCH LINUX v3 00/10] tty: xuartps: Fix lock ups

FromSoren Brinkmann <soren.brinkmann@xilinx.com>
Date2015-11-22 04:00 +0100
Subject[PATCH LINUX v3 00/10] tty: xuartps: Fix lock ups
Message-ID<qxt2F-5Ri-3@gated-at.bofh.it>
Hi,

here is v3 of this series. It's largely the same as before, but I
adjusted 'tty: xuartps: Don't consider circular buffer when enabling
transmitter' according to Peter's suggestions.

I also spent some time trying to get Peter's test for flow control and
xchar running. The xchar thing fails and will need some more work, but I
think in general it should be possible to get it to work.
The flow control test passes:

root@Xilinx-ZC1751-DC1:~# ./flow /dev/ttyPS0
Test flow control on /dev/ttyPS0
begin test1
patterns sent: 223  recvd: 208
read distribution: 1   = 0
                   2+  = 0
                   4+  = 0
                   8+  = 0
                  16+  = 0
                  32+  = 643
                  64+  = 0
                 128+  = 0
                 256+  = 0
                 512+  = 0
PASSED

root@Xilinx-ZC1751-DC1:~# ./flow /dev/ttyPS1
Test flow control on /dev/ttyPS1
begin test1
patterns sent: 223  recvd: 208
read distribution: 1   = 0
                   2+  = 0
                   4+  = 0
                   8+  = 0
                  16+  = 0
                  32+  = 643
                  64+  = 0
                 128+  = 0
                 256+  = 0
                 512+  = 0
PASSED

	Sören

Sören Brinkmann (10):
  tty: xuartps: Beautify read-modify writes
  tty: xuartps: Use spinlock to serialize HW access
  tty: xuartps: Don't consider circular buffer when enabling transmitter
  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
  tty: xuartps: Cleanup: Reformat if-else

 drivers/tty/serial/xilinx_uartps.c | 246 +++++++++++++++++++++----------------
 1 file changed, 137 insertions(+), 109 deletions(-)

-- 
2.6.3.3.g9bb996a

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1274783 — [PATCH LINUX v3 07/10] tty: xuartps: Acquire port lock for shutdown

FromSoren Brinkmann <soren.brinkmann@xilinx.com>
Date2015-11-22 04:00 +0100
Subject[PATCH LINUX v3 07/10] tty: xuartps: Acquire port lock for shutdown
Message-ID<qxt2G-5Ri-13@gated-at.bofh.it>
In reply to#1274782
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 ab3995d00973..f3ac69387b0a 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -826,6 +826,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);
@@ -835,6 +838,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.3.3.g9bb996a

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1274784 — [PATCH LINUX v3 01/10] tty: xuartps: Beautify read-modify writes

FromSoren Brinkmann <soren.brinkmann@xilinx.com>
Date2015-11-22 04:10 +0100
Subject[PATCH LINUX v3 01/10] tty: xuartps: Beautify read-modify writes
Message-ID<qxtcl-6aY-1@gated-at.bofh.it>
In reply to#1274782
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.3.3.g9bb996a

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1274785 — [PATCH LINUX v3 09/10] tty: xuartps: Only handle RX IRQs when RX is enabled

FromSoren Brinkmann <soren.brinkmann@xilinx.com>
Date2015-11-22 04:10 +0100
Subject[PATCH LINUX v3 09/10] tty: xuartps: Only handle RX IRQs when RX is enabled
Message-ID<qxtcl-6aY-3@gated-at.bofh.it>
In reply to#1274782
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 db9e23eaf300..3a1e508926c1 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) {
@@ -579,11 +586,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;
 }
 
 /**
@@ -764,6 +773,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);
@@ -790,6 +800,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.
@@ -833,6 +844,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);
 
@@ -844,6 +856,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.3.3.g9bb996a

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1274786 — [PATCH LINUX v3 03/10] tty: xuartps: Don't consider circular buffer when enabling transmitter

FromSoren Brinkmann <soren.brinkmann@xilinx.com>
Date2015-11-22 04:10 +0100
Subject[PATCH LINUX v3 03/10] tty: xuartps: Don't consider circular buffer when enabling transmitter
Message-ID<qxtcl-6aY-5@gated-at.bofh.it>
In reply to#1274782
Restarting the transmitter even if the circ buffer is empty may be
necessary to push out remaining data when the port is restarted after
being stopped.

Cc: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
v3:
 - changed this patch to not always enable the transmitter, but keep the
   check for uart_tx_stopped() in place, as suggested by Peter, whose
   explanation I also stole for the new commit message (thx)
---
 drivers/tty/serial/xilinx_uartps.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 2c98c357d9a0..6a7cd4e057ae 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -512,7 +512,7 @@ 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))
+	if (uart_tx_stopped(port))
 		return;
 
 	/*
@@ -524,6 +524,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))
+		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.3.3.g9bb996a

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1274787 — [PATCH LINUX v3 04/10] tty: xuartps: Clear interrupt status register in shutdown

FromSoren Brinkmann <soren.brinkmann@xilinx.com>
Date2015-11-22 04:10 +0100
Subject[PATCH LINUX v3 04/10] tty: xuartps: Clear interrupt status register in shutdown
Message-ID<qxtcm-6aY-11@gated-at.bofh.it>
In reply to#1274782
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 6a7cd4e057ae..ef114d7a0623 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -828,6 +828,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.3.3.g9bb996a

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1274789 — [PATCH LINUX v3 05/10] tty: xuartps: Improve startup function

FromSoren Brinkmann <soren.brinkmann@xilinx.com>
Date2015-11-22 04:10 +0100
Subject[PATCH LINUX v3 05/10] tty: xuartps: Improve startup function
Message-ID<qxtcm-6aY-17@gated-at.bofh.it>
In reply to#1274782
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 ef114d7a0623..6ffd3bbe3e18 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -758,6 +758,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,
@@ -765,6 +766,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);
@@ -775,15 +778,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.
@@ -814,6 +816,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.3.3.g9bb996a

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web