[PATCH v3 0/4] serial: samsung: Fix UART status handling and other fixes

[PATCH v3 0/4] serial: samsung: Fix UART status handling and other fixes

[Robert Baldyga]

Hello,

This patch set contains four patches: two minor fixes and two patches
fixing quite importatn bug which was missing UART status handling in
DMA mode. It fixes, among others, 'break' contition handling, which is
necessary if we want to use Magic SysRq. So this patch fixes Magic SysRq
handling for serial consoles using UART in DMA mode,

Best regards,
Robert Baldyga

Changelog:

v3:
- better describe change in patch "Fix UART status handling in DMA mode"

v2: https://lkml.org/lkml/2015/9/10/336
- address comments from Krzysztof Kozlowski:
  - add comment in patch removing 'ignore_char' label
  - split patch "Fix UART status handling in DMA mode"
    into two patches for better code readability

v1: https://lkml.org/lkml/2015/9/8/190

Robert Baldyga (4):
  serial: samsung: remove unused 'irq' parameter
  serial: samsung: remove unneded 'ignore_char' label
  serial: samsung: introduce s3c24xx_serial_rx_drain_fifo() function
  serial: samsung: Fix UART status handling in DMA mode

 drivers/tty/serial/samsung.c | 66 ++++++++++++++------------------------------
 1 file changed, 21 insertions(+), 45 deletions(-)

-- 
1.9.1

[PATCH v3 1/4] serial: samsung: remove unused 'irq' parameter

[Robert Baldyga]

This parameter is not used anywhere, so we can get rid of it.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/tty/serial/samsung.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 856686d..fee764c 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -573,7 +573,7 @@ static void enable_rx_pio(struct s3c24xx_uart_port *ourport)
 	ourport->rx_mode = S3C24XX_RX_PIO;
 }
 
-static irqreturn_t s3c24xx_serial_rx_chars_dma(int irq, void *dev_id)
+static irqreturn_t s3c24xx_serial_rx_chars_dma(void *dev_id)
 {
 	unsigned int utrstat, ufstat, received;
 	struct s3c24xx_uart_port *ourport = dev_id;
@@ -621,7 +621,7 @@ finish:
 	return IRQ_HANDLED;
 }
 
-static irqreturn_t s3c24xx_serial_rx_chars_pio(int irq, void *dev_id)
+static irqreturn_t s3c24xx_serial_rx_chars_pio(void *dev_id)
 {
 	struct s3c24xx_uart_port *ourport = dev_id;
 	struct uart_port *port = &ourport->port;
@@ -718,8 +718,8 @@ static irqreturn_t s3c24xx_serial_rx_chars(int irq, void *dev_id)
 	struct s3c24xx_uart_port *ourport = dev_id;
 
 	if (ourport->dma && ourport->dma->rx_chan)
-		return s3c24xx_serial_rx_chars_dma(irq, dev_id);
-	return s3c24xx_serial_rx_chars_pio(irq, dev_id);
+		return s3c24xx_serial_rx_chars_dma(dev_id);
+	return s3c24xx_serial_rx_chars_pio(dev_id);
 }
 
 static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
-- 
1.9.1

[PATCH v3 2/4] serial: samsung: remove unneded 'ignore_char' label

[Robert Baldyga]

This label does nothing special and we don't need to have it anymore.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/tty/serial/samsung.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index fee764c..dc4be54 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -676,7 +676,7 @@ static irqreturn_t s3c24xx_serial_rx_chars_pio(void *dev_id)
 				dbg("break!\n");
 				port->icount.brk++;
 				if (uart_handle_break(port))
-					goto ignore_char;
+					continue; /* Ignore character */
 			}
 
 			if (uerstat & S3C2410_UERSTAT_FRAME)
@@ -696,13 +696,10 @@ static irqreturn_t s3c24xx_serial_rx_chars_pio(void *dev_id)
 		}
 
 		if (uart_handle_sysrq_char(port, ch))
-			goto ignore_char;
+			continue; /* Ignore character */
 
 		uart_insert_char(port, uerstat, S3C2410_UERSTAT_OVERRUN,
 				 ch, flag);
-
-ignore_char:
-		continue;
 	}
 
 	spin_unlock_irqrestore(&port->lock, flags);
-- 
1.9.1

[PATCH v3 3/4] serial: samsung: introduce s3c24xx_serial_rx_drain_fifo() function

[Robert Baldyga]

This patch introduces s3c24xx_serial_rx_drain_fifo() which reads data
from RX FIFO and writes it to tty buffer. It also checks for special
conditions (such as 'break') and handles it. This function has been
separated from s3c24xx_serial_rx_chars_pio() as it contains code which
can be used also in DMA mode.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/tty/serial/samsung.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index dc4be54..1d7dd86 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -621,16 +621,12 @@ finish:
 	return IRQ_HANDLED;
 }
 
-static irqreturn_t s3c24xx_serial_rx_chars_pio(void *dev_id)
+static void s3c24xx_serial_rx_drain_fifo(struct s3c24xx_uart_port *ourport)
 {
-	struct s3c24xx_uart_port *ourport = dev_id;
 	struct uart_port *port = &ourport->port;
 	unsigned int ufcon, ch, flag, ufstat, uerstat;
-	unsigned long flags;
 	int max_count = port->fifosize;
 
-	spin_lock_irqsave(&port->lock, flags);
-
 	while (max_count-- > 0) {
 		ufcon = rd_regl(port, S3C2410_UFCON);
 		ufstat = rd_regl(port, S3C2410_UFSTAT);
@@ -654,9 +650,7 @@ static irqreturn_t s3c24xx_serial_rx_chars_pio(void *dev_id)
 					ufcon |= S3C2410_UFCON_RESETRX;
 					wr_regl(port, S3C2410_UFCON, ufcon);
 					rx_enabled(port) = 1;
-					spin_unlock_irqrestore(&port->lock,
-							flags);
-					goto out;
+					return;
 				}
 				continue;
 			}
@@ -702,10 +696,19 @@ static irqreturn_t s3c24xx_serial_rx_chars_pio(void *dev_id)
 				 ch, flag);
 	}
 
-	spin_unlock_irqrestore(&port->lock, flags);
 	tty_flip_buffer_push(&port->state->port);
+}
+
+static irqreturn_t s3c24xx_serial_rx_chars_pio(void *dev_id)
+{
+	struct s3c24xx_uart_port *ourport = dev_id;
+	struct uart_port *port = &ourport->port;
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+	s3c24xx_serial_rx_drain_fifo(ourport);
+	spin_unlock_irqrestore(&port->lock, flags);
 
-out:
 	return IRQ_HANDLED;
 }
 
-- 
1.9.1

[PATCH v3 4/4] serial: samsung: Fix UART status handling in DMA mode

[Robert Baldyga]

So far, when interrupt occured in DMA mode, it was handled by terminating
DMA transfer and draining data remaining in RX FIFO. It worked well
until interrupt was caused by timeout, but the same interrupt can be
alse caused by special condition (eg. 'break'), which requires special
handling. In such case handling mechanism was the same - DMA transaction
was terminated and FIFO was drained, but any special conditions were
ingnored. Because of this in DMA mode there was no ability to use,
for example, Magic SysRq.

This patch fixes this problem by using s3c24xx_serial_rx_drain_fifo()
function instead of uart_rx_drain_fifo(), which does the same thing
(drains RX FIFO) plus checks UART status to detect special conditions
(such as 'break'). Thanks to this we have exactly the same UART status
handling in both DMA and PIO mode.

This change additionally simplifies RX handling code, as we no longer
need uart_rx_drain_fifo() function, so we can remove it.

Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/tty/serial/samsung.c | 30 +++---------------------------
 1 file changed, 3 insertions(+), 27 deletions(-)

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 1d7dd86..d72cd73 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -385,32 +385,6 @@ static void s3c24xx_uart_copy_rx_to_tty(struct s3c24xx_uart_port *ourport,
 	}
 }
 
-static int s3c24xx_serial_rx_fifocnt(struct s3c24xx_uart_port *ourport,
-				     unsigned long ufstat);
-
-static void uart_rx_drain_fifo(struct s3c24xx_uart_port *ourport)
-{
-	struct uart_port *port = &ourport->port;
-	struct tty_port *tty = &port->state->port;
-	unsigned int ch, ufstat;
-	unsigned int count;
-
-	ufstat = rd_regl(port, S3C2410_UFSTAT);
-	count = s3c24xx_serial_rx_fifocnt(ourport, ufstat);
-
-	if (!count)
-		return;
-
-	while (count-- > 0) {
-		ch = rd_regb(port, S3C2410_URXH);
-
-		ourport->port.icount.rx++;
-		tty_insert_flip_char(tty, ch, TTY_NORMAL);
-	}
-
-	tty_flip_buffer_push(tty);
-}
-
 static void s3c24xx_serial_stop_rx(struct uart_port *port)
 {
 	struct s3c24xx_uart_port *ourport = to_ourport(port);
@@ -573,6 +547,8 @@ static void enable_rx_pio(struct s3c24xx_uart_port *ourport)
 	ourport->rx_mode = S3C24XX_RX_PIO;
 }
 
+static void s3c24xx_serial_rx_drain_fifo(struct s3c24xx_uart_port *ourport);
+
 static irqreturn_t s3c24xx_serial_rx_chars_dma(void *dev_id)
 {
 	unsigned int utrstat, ufstat, received;
@@ -606,7 +582,7 @@ static irqreturn_t s3c24xx_serial_rx_chars_dma(void *dev_id)
 		enable_rx_pio(ourport);
 	}
 
-	uart_rx_drain_fifo(ourport);
+	s3c24xx_serial_rx_drain_fifo(ourport);
 
 	if (tty) {
 		tty_flip_buffer_push(t);
-- 
1.9.1

Re: [PATCH v3 3/4] serial: samsung: introduce s3c24xx_serial_rx_drain_fifo() function

[Krzysztof Kozlowski]

On 15.09.2015 21:48, Robert Baldyga wrote:
> This patch introduces s3c24xx_serial_rx_drain_fifo() which reads data
> from RX FIFO and writes it to tty buffer. It also checks for special
> conditions (such as 'break') and handles it. This function has been
> separated from s3c24xx_serial_rx_chars_pio() as it contains code which
> can be used also in DMA mode.
> 
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
> ---
>  drivers/tty/serial/samsung.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

Re: [PATCH v3 3/4] serial: samsung: introduce s3c24xx_serial_rx_drain_fifo() function

[Krzysztof Kozlowski]

On 15.09.2015 21:48, Robert Baldyga wrote:
> This patch introduces s3c24xx_serial_rx_drain_fifo() which reads data
> from RX FIFO and writes it to tty buffer. It also checks for special
> conditions (such as 'break') and handles it. This function has been
> separated from s3c24xx_serial_rx_chars_pio() as it contains code which
> can be used also in DMA mode.
> 
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
> ---
>  drivers/tty/serial/samsung.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)

Thanks for splitting the code. It looks good but I don't feel
experienced enough in the driver internals to give a review tag. :)

Best regards,
Krzysztof