[PATCH v2 0/2] serial: samsung: fix DMA for small FIFO sizes

[PATCH v2 0/2] serial: samsung: fix DMA for small FIFO sizes

[Robert Baldyga]

Hello,

This patch set fixes bug causing serial hang in DMA mode for FIFO sizes
smaller than cache alignment. The first patch fixes DMA mode entering
condition to avoid starting with buffer smaller than cache line size.
Second patch fixes the serial hang bug, which was caused by unproper
buffer aligning algorithm which assumed that there is always enough
free space in FIFO for excessive bytes of buffer that is being alligned.

Best regards,
Robert Baldyga

Changelog:

v2:
- Add CC to stable
- Add Reported-by: Krzysztof Kozlowski
- Change title of the first patch to more relevant

v1: http://permalink.gmane.org/gmane.linux.kernel/2008281

Marek Szyprowski (1):
  serial: samsung: fix DMA mode enter condition for small FIFO sizes

Robert Baldyga (1):
  serial: samsung: fix DMA for FIFO smaller than cache line size

 drivers/tty/serial/samsung.c | 47 +++++++++++++++++++++++++++++---------------
 drivers/tty/serial/samsung.h |  1 +
 2 files changed, 32 insertions(+), 16 deletions(-)

-- 
1.9.1

[PATCH v2 1/2] serial: samsung: fix DMA mode enter condition for small FIFO sizes

[Robert Baldyga]

From: Marek Szyprowski <m.szyprowski@samsung.com>

Due to some of serial ports can have FIFO size smaller than cache line
size, and because of need to align DMA buffer address to cache line size,
it's necessary to calculate minimum number of bytes for which we want
to start DMA transaction to be at least cache line size. The simplest
way to meet this requirement is to get maximum of cache line size and
FIFO size.

Cc: <stable@vger.kernel.org> # v3.18+
Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/tty/serial/samsung.c | 13 +++++++++++--
 drivers/tty/serial/samsung.h |  1 +
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 67d0c21..08168f9 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -342,7 +342,8 @@ static void s3c24xx_serial_start_next_tx(struct s3c24xx_uart_port *ourport)
 		return;
 	}
 
-	if (!ourport->dma || !ourport->dma->tx_chan || count < port->fifosize)
+	if (!ourport->dma || !ourport->dma->tx_chan ||
+	    count < ourport->min_dma_size)
 		s3c24xx_serial_start_tx_pio(ourport);
 	else
 		s3c24xx_serial_start_tx_dma(ourport, count);
@@ -742,7 +743,8 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
 
 	count = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
 
-	if (ourport->dma && ourport->dma->tx_chan && count >= port->fifosize) {
+	if (ourport->dma && ourport->dma->tx_chan &&
+	    count >= ourport->min_dma_size) {
 		s3c24xx_serial_start_tx_dma(ourport, count);
 		goto out;
 	}
@@ -1838,6 +1840,13 @@ static int s3c24xx_serial_probe(struct platform_device *pdev)
 	else if (ourport->info->fifosize)
 		ourport->port.fifosize = ourport->info->fifosize;
 
+	/*
+	 * DMA transfers must be aligned at least to cache line size,
+	 * so find minimal transfer size suitable for DMA mode
+	 */
+	ourport->min_dma_size = max_t(int, ourport->port.fifosize,
+				    dma_get_cache_alignment());
+
 	probe_index++;
 
 	dbg("%s: initialising port %p...\n", __func__, ourport);
diff --git a/drivers/tty/serial/samsung.h b/drivers/tty/serial/samsung.h
index d275032..fc5deaa 100644
--- a/drivers/tty/serial/samsung.h
+++ b/drivers/tty/serial/samsung.h
@@ -82,6 +82,7 @@ struct s3c24xx_uart_port {
 	unsigned char			tx_claimed;
 	unsigned int			pm_level;
 	unsigned long			baudclk_rate;
+	unsigned int			min_dma_size;
 
 	unsigned int			rx_irq;
 	unsigned int			tx_irq;
-- 
1.9.1

[PATCH v2 2/2] serial: samsung: fix DMA for FIFO smaller than cache line size

[Robert Baldyga]

So far DMA mode were activated when only number of bytes to send was
equal or greater than min_dma_size. Due to requirement that DMA transaction
buffer should be aligned to cache line size, the excessive bytes were
written to FIFO before starting DMA transaction. The problem occurred
when FIFO size were smaller than cache alignment, because writing all
excessive bytes to FIFO would fail. It happened in DMA mode when PIO
interrupts disabled, which caused driver hung.

The solution is to test if buffer is alligned to cache line size before
activating DMA mode, and if it's not, running PIO mode to align buffer
and then starting DMA transaction. In PIO mode, when interrupts are
enabled, lack of space in FIFO isn't the problem, so buffer aligning
will always finish with success.

Cc: <stable@vger.kernel.org> # v3.18+
Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/tty/serial/samsung.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 08168f9..5916311 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -295,15 +295,6 @@ static int s3c24xx_serial_start_tx_dma(struct s3c24xx_uart_port *ourport,
 	if (ourport->tx_mode != S3C24XX_TX_DMA)
 		enable_tx_dma(ourport);
 
-	while (xmit->tail & (dma_get_cache_alignment() - 1)) {
-		if (rd_regl(port, S3C2410_UFSTAT) & ourport->info->tx_fifofull)
-			return 0;
-		wr_regb(port, S3C2410_UTXH, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-		count--;
-	}
-
 	dma->tx_size = count & ~(dma_get_cache_alignment() - 1);
 	dma->tx_transfer_addr = dma->tx_addr + xmit->tail;
 
@@ -343,7 +334,8 @@ static void s3c24xx_serial_start_next_tx(struct s3c24xx_uart_port *ourport)
 	}
 
 	if (!ourport->dma || !ourport->dma->tx_chan ||
-	    count < ourport->min_dma_size)
+	    count < ourport->min_dma_size ||
+	    xmit->tail & (dma_get_cache_alignment() - 1))
 		s3c24xx_serial_start_tx_pio(ourport);
 	else
 		s3c24xx_serial_start_tx_dma(ourport, count);
@@ -737,7 +729,7 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
 	struct uart_port *port = &ourport->port;
 	struct circ_buf *xmit = &port->state->xmit;
 	unsigned long flags;
-	int count;
+	int count, dma_count = 0;
 
 	spin_lock_irqsave(&port->lock, flags);
 
@@ -745,8 +737,12 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
 
 	if (ourport->dma && ourport->dma->tx_chan &&
 	    count >= ourport->min_dma_size) {
-		s3c24xx_serial_start_tx_dma(ourport, count);
-		goto out;
+		int align = dma_get_cache_alignment() -
+			(xmit->tail & (dma_get_cache_alignment() - 1));
+		if (count-align >= ourport->min_dma_size) {
+			dma_count = count-align;
+			count = align;
+		}
 	}
 
 	if (port->x_char) {
@@ -767,14 +763,24 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
 
 	/* try and drain the buffer... */
 
-	count = port->fifosize;
-	while (!uart_circ_empty(xmit) && count-- > 0) {
+	if (count > port->fifosize) {
+		count = port->fifosize;
+		dma_count = 0;
+	}
+
+	while (!uart_circ_empty(xmit) && count > 0) {
 		if (rd_regl(port, S3C2410_UFSTAT) & ourport->info->tx_fifofull)
 			break;
 
 		wr_regb(port, S3C2410_UTXH, xmit->buf[xmit->tail]);
 		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
 		port->icount.tx++;
+		count--;
+	}
+
+	if (!count && dma_count) {
+		s3c24xx_serial_start_tx_dma(ourport, dma_count);
+		goto out;
 	}
 
 	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) {
-- 
1.9.1

Re: [PATCH v2 0/2] serial: samsung: fix DMA for small FIFO sizes

[Krzysztof Kozlowski]

On 31.07.2015 17:58, Robert Baldyga wrote:
> Hello,
> 
> This patch set fixes bug causing serial hang in DMA mode for FIFO sizes
> smaller than cache alignment. The first patch fixes DMA mode entering
> condition to avoid starting with buffer smaller than cache line size.
> Second patch fixes the serial hang bug, which was caused by unproper
> buffer aligning algorithm which assumed that there is always enough
> free space in FIFO for excessive bytes of buffer that is being alligned.
> 
> Best regards,
> Robert Baldyga
> 
> Changelog:
> 
> v2:
> - Add CC to stable
> - Add Reported-by: Krzysztof Kozlowski
> - Change title of the first patch to more relevant
> 
> v1: http://permalink.gmane.org/gmane.linux.kernel/2008281
> 
> Marek Szyprowski (1):
>   serial: samsung: fix DMA mode enter condition for small FIFO sizes
> 
> Robert Baldyga (1):
>   serial: samsung: fix DMA for FIFO smaller than cache line size
> 
>  drivers/tty/serial/samsung.c | 47 +++++++++++++++++++++++++++++---------------
>  drivers/tty/serial/samsung.h |  1 +
>  2 files changed, 32 insertions(+), 16 deletions(-)
> 

Tested on Trats2 (Exynos4412 with pl330, serial output on ttySAC2).
Patchset fixes the reported issue (hang after applying "ARM: dts:
exynos4: add DMA support for serial ports" [0]).

However with [0] dmatest cannot execute tests on dma0 channels. Just
silently passes but does not execute memcpy on any of dma0 channels.
I wonder why... the serial ports use only some of dma0 channels, not all
of them.

Any ideas?

[0] http://www.spinics.net/lists/linux-samsung-soc/msg45700.html

Best regards,
Krzysztof