public inbox for linux-serial@vger.kernel.org
 help / color / mirror / Atom feed
From: Mans Rullgard <mans@mansr.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org
Subject: [PATCH] tty: serial: 8250_omap: fix tx with dma
Date: Wed, 30 Apr 2025 17:37:09 +0100	[thread overview]
Message-ID: <20250430163709.15850-1-mans@mansr.com> (raw)

Commit 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
introduced two errors in the TX DMA handling for 8250_omap.

Firstly, kfifo_dma_out_prepare_mapped() needs a scatterlist with two
entries whereas only one is provided.  The same error was fixed for
8250_dma in 59449c9dbdaa ("tty: serial: 8250_dma: use sgl with 2 nents
to take care of buffer wrap").

Secondly, when the OMAP_DMA_TX_KICK flag is set, one byte is pulled from
the kfifo and emitted directly in order to start the DMA.  This is done
without updating DMA tx_size which leads to uart_xmit_advance() called
in the DMA complete callback advancing the kfifo by one too much.

In practice, transmitting N bytes has been seen to result in the last
N-1 bytes being sent repeatedly.

This change fixes both problems.

Fixes: 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
Signed-off-by: Mans Rullgard <mans@mansr.com>
---
 drivers/tty/serial/8250/8250_omap.c | 35 +++++++++++++++--------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index f1aee915bc02..84a2f013015e 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1152,9 +1152,11 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 	struct omap8250_priv		*priv = p->port.private_data;
 	struct tty_port			*tport = &p->port.state->port;
 	struct dma_async_tx_descriptor	*desc;
-	struct scatterlist sg;
+	struct scatterlist *sg;
+	struct scatterlist sgl[2];
 	int skip_byte = -1;
 	int ret;
+	int i;
 
 	if (dma->tx_running)
 		return 0;
@@ -1173,16 +1175,6 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 		return 0;
 	}
 
-	sg_init_table(&sg, 1);
-	ret = kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
-					   UART_XMIT_SIZE, dma->tx_addr);
-	if (ret != 1) {
-		serial8250_clear_THRI(p);
-		return 0;
-	}
-
-	dma->tx_size = sg_dma_len(&sg);
-
 	if (priv->habit & OMAP_DMA_TX_KICK) {
 		unsigned char c;
 		u8 tx_lvl;
@@ -1207,7 +1199,7 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 			ret = -EBUSY;
 			goto err;
 		}
-		if (dma->tx_size < 4) {
+		if (kfifo_len(&tport->xmit_fifo) < 4) {
 			ret = -EINVAL;
 			goto err;
 		}
@@ -1216,12 +1208,19 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 			goto err;
 		}
 		skip_byte = c;
-		/* now we need to recompute due to kfifo_get */
-		kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, &sg, 1,
-				UART_XMIT_SIZE, dma->tx_addr);
 	}
 
-	desc = dmaengine_prep_slave_sg(dma->txchan, &sg, 1, DMA_MEM_TO_DEV,
+	sg_init_table(sgl, ARRAY_SIZE(sgl));
+
+	ret = kfifo_dma_out_prepare_mapped(&tport->xmit_fifo, sgl, ARRAY_SIZE(sgl),
+					   UART_XMIT_SIZE, dma->tx_addr);
+
+	dma->tx_size = 0;
+
+	for_each_sg(sgl, sg, ret, i)
+		dma->tx_size += sg_dma_len(sg);
+
+	desc = dmaengine_prep_slave_sg(dma->txchan, sgl, ret, DMA_MEM_TO_DEV,
 			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
 	if (!desc) {
 		ret = -EBUSY;
@@ -1248,8 +1247,10 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
 err:
 	dma->tx_err = 1;
 out_skip:
-	if (skip_byte >= 0)
+	if (skip_byte >= 0) {
 		serial_out(p, UART_TX, skip_byte);
+		p->port.icount.tx++;
+	}
 	return ret;
 }
 
-- 
2.49.0


             reply	other threads:[~2025-04-30 16:45 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-30 16:37 Mans Rullgard [this message]
2025-04-30 16:50 ` [PATCH] tty: serial: 8250_omap: fix tx with dma Greg Kroah-Hartman
2025-05-05 11:43 ` Geert Uytterhoeven
2025-05-06  7:32 ` Jiri Slaby

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=20250430163709.15850-1-mans@mansr.com \
    --to=mans@mansr.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    /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