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

[PATCH 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

Marek Szyprowski (1):
  serial: samsung: calculate min_dma_size

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 1/2] serial: samsung: calculate min_dma_size

[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.

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 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.

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 0/2] serial: samsung: fix DMA for small FIFO sizes

[Krzysztof Kozlowski]

2015-07-30 18:14 GMT+09:00 Robert Baldyga <r.baldyga@samsung.com>:
> 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
>
> Marek Szyprowski (1):
>   serial: samsung: calculate min_dma_size
>
> 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(-)

Does this apply for stable backporting? Or at least fixes tag
indicating a bug? I could not figure this out from description of
first patch. For second patch it seems it applies.

Anyway I would appreciate a reported-by credits.

Best regards,
Krzysztof

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

[Robert Baldyga]

On 07/31/2015 10:04 AM, Krzysztof Kozlowski wrote:
> 2015-07-30 18:14 GMT+09:00 Robert Baldyga <r.baldyga@samsung.com>:
>> 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
>>
>> Marek Szyprowski (1):
>>   serial: samsung: calculate min_dma_size
>>
>> 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(-)
> 
> Does this apply for stable backporting? Or at least fixes tag
> indicating a bug? I could not figure this out from description of
> first patch. For second patch it seems it applies.

These patches are two steps of fixing the bug, so they both apply for
stable backporting.

> 
> Anyway I would appreciate a reported-by credits.

Sure ;)

Thanks,
Robert