linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nava kishore Manne <nava.manne@xilinx.com>
To: jslaby@suse.com, michal.simek@xilinx.com,
	soren.brinkmann@xilinx.com, anirudh@xilinx.com
Cc: Nava kishore Manne <navam@xilinx.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2 2/3] serial: xuartps: Rewrite the interrupt handling logic
Date: Fri, 20 Nov 2015 19:04:37 +0530	[thread overview]
Message-ID: <1448026478-22131-2-git-send-email-navam@xilinx.com> (raw)
In-Reply-To: <1448026478-22131-1-git-send-email-navam@xilinx.com>

The existing interrupt handling logic has followins issues.
- Upon a parity error with default configuration, the control
  never comes out of the ISR thereby hanging Linux.
- The error handling logic around framing and parity error are buggy.
  There are chances that the errors will never be captured.
This patch fixes all these concerns.

Signed-off-by: Nava kishore Manne <navam@xilinx.com>
---
Changes for v2:
	--Add required changes after spliting the ISR as suggested by
	Peter Hurley.

 drivers/tty/serial/xilinx_uartps.c | 64 ++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 33 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 2e1b0a8..4d71478 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -191,9 +191,10 @@ static irqreturn_t cdns_uart_isr(int irq, void *dev_id)
 	spin_lock_irqsave(&port->lock, flags);
 
 	/* Read the interrupt status register to determine which
-	 * interrupt(s) is/are active.
+	 * interrupt(s) is/are active and clear them.
 	 */
 	isrstatus = readl(port->membase + CDNS_UART_ISR_OFFSET);
+	writel(isrstatus, port->membase + CDNS_UART_ISR_OFFSET);
 
 	if (isrstatus & CDNS_UART_IXR_TXEMPTY) {
 		cdns_uart_handle_tx(dev_id);
@@ -202,8 +203,6 @@ static irqreturn_t cdns_uart_isr(int irq, void *dev_id)
 	if (isrstatus & CDNS_UART_IXR_MASK)
 		cdns_uart_handle_rx(dev_id, isrstatus);
 
-	writel(isrstatus, port->membase + CDNS_UART_ISR_OFFSET);
-
 	/* be sure to release the lock and tty before leaving */
 	spin_unlock_irqrestore(&port->lock, flags);
 
@@ -354,37 +353,32 @@ static void cdns_uart_handle_rx(void *dev_id, unsigned int isrstatus)
 {
 	struct uart_port *port = (struct uart_port *)dev_id;
 	unsigned int data;
+	unsigned int framerrprocessed = 0;
 	char status = TTY_NORMAL;
 
-	/*
-	 * There is no hardware break detection, so we interpret framing
-	 * error with all-zeros data as a break sequence. Most of the time,
-	 * there's another non-zero byte at the end of the sequence.
-	 */
-	if (isrstatus & CDNS_UART_IXR_FRAMING) {
-		while (!(readl(port->membase + CDNS_UART_SR_OFFSET) &
-			CDNS_UART_SR_RXEMPTY)) {
-			if (!readl(port->membase + CDNS_UART_FIFO_OFFSET)) {
+	while ((readl(port->membase + CDNS_UART_SR_OFFSET) &
+		CDNS_UART_SR_RXEMPTY) != CDNS_UART_SR_RXEMPTY) {
+		data = readl(port->membase + CDNS_UART_FIFO_OFFSET);
+		port->icount.rx++;
+
+		/*
+		 * There is no hardware break detection, so we interpret framing
+		 * error with all-zeros data as a break sequence. Most of the
+		 * time, there's another non-zero byte at the end of the
+		 * sequence.
+		 */
+		if (isrstatus & CDNS_UART_IXR_FRAMING) {
+			if (!data) {
 				port->read_status_mask |= CDNS_UART_IXR_BRK;
-				isrstatus &= ~CDNS_UART_IXR_FRAMING;
+				framerrprocessed = 1;
+				continue;
 			}
 		}
-		writel(CDNS_UART_IXR_FRAMING,
-			port->membase + CDNS_UART_ISR_OFFSET);
-	}
 
-	/* drop byte with parity error if IGNPAR specified */
-	if (isrstatus & port->ignore_status_mask & CDNS_UART_IXR_PARITY)
-		isrstatus &= ~(CDNS_UART_IXR_RXTRIG | CDNS_UART_IXR_TOUT);
-
-	isrstatus &= port->read_status_mask;
-	isrstatus &= ~port->ignore_status_mask;
-	if ((isrstatus & CDNS_UART_IXR_TOUT) ||
-		(isrstatus & CDNS_UART_IXR_RXTRIG)) {
-		/* Receive Timeout Interrupt */
-		while ((readl(port->membase + CDNS_UART_SR_OFFSET) &
-			CDNS_UART_SR_RXEMPTY) != CDNS_UART_SR_RXEMPTY) {
-			data = readl(port->membase + CDNS_UART_FIFO_OFFSET);
+		isrstatus &= port->read_status_mask;
+		isrstatus &= ~port->ignore_status_mask;
+		if ((isrstatus & CDNS_UART_IXR_TOUT) ||
+			(isrstatus & CDNS_UART_IXR_RXTRIG)) {
 
 			/* Non-NULL byte after BREAK is garbage (99%) */
 			if (data && (port->read_status_mask
@@ -416,21 +410,25 @@ static void cdns_uart_handle_rx(void *dev_id, unsigned int isrstatus)
 			if (isrstatus & CDNS_UART_IXR_PARITY) {
 				port->icount.parity++;
 				status = TTY_PARITY;
-			} else if (isrstatus & CDNS_UART_IXR_FRAMING) {
+			}
+			if (isrstatus & CDNS_UART_IXR_FRAMING) {
 				port->icount.frame++;
 				status = TTY_FRAME;
-			} else if (isrstatus & CDNS_UART_IXR_OVERRUN) {
+			}
+			if (isrstatus & CDNS_UART_IXR_OVERRUN) {
 				port->icount.overrun++;
+				tty_insert_flip_char(&port->state->port, 0,
+								TTY_OVERRUN);
 			}
 
-			uart_insert_char(port, isrstatus, CDNS_UART_IXR_OVERRUN,
-								data, status);
+			tty_insert_flip_char(&port->state->port, data, status);
 		}
+	}
 		spin_unlock(&port->lock);
 		tty_flip_buffer_push(&port->state->port);
 		spin_lock(&port->lock);
 }
-}
+
 #ifdef CONFIG_COMMON_CLK
 /**
  * cdns_uart_clk_notitifer_cb - Clock notifier callback
-- 
2.1.2

  reply	other threads:[~2015-11-20 13:34 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-20 13:34 [PATCH v2 1/3] serial: xuartps: Spliting the ISR into smaller routines Nava kishore Manne
2015-11-20 13:34 ` Nava kishore Manne [this message]
2015-11-20 13:34 ` [PATCH v2 3/3] serial: xuartps: Do not enable parity error interrupt Nava kishore Manne
2015-11-20 14:43 ` [PATCH v2 1/3] serial: xuartps: Spliting the ISR into smaller routines Sören Brinkmann
2015-12-02  5:32   ` Nava kishore Manne
2015-12-02 23:04     ` Peter Hurley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1448026478-22131-2-git-send-email-navam@xilinx.com \
    --to=nava.manne@xilinx.com \
    --cc=anirudh@xilinx.com \
    --cc=jslaby@suse.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=michal.simek@xilinx.com \
    --cc=navam@xilinx.com \
    --cc=soren.brinkmann@xilinx.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).