tty: serial: 8250_omap: fix TX with DMA for am33xx
authorJiri Slaby (SUSE) <jirislaby@kernel.org>
Thu, 22 May 2025 05:38:35 +0000 (07:38 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 22 May 2025 05:50:45 +0000 (07:50 +0200)
Commit 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
introduced an error in the TX DMA handling for 8250_omap.

When the OMAP_DMA_TX_KICK flag is set, the "skip_byte" is pulled from
the kfifo and emitted directly in order to start the DMA. While the
kfifo is updated, dma->tx_size is not decreased. This leads to
uart_xmit_advance() called in omap_8250_dma_tx_complete() 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 the problem by moving all of the dma setup after the
OMAP_DMA_TX_KICK handling and using kfifo_len() instead of the DMA size
for the 4-byte cutoff check. This slightly changes the behaviour at
buffer wraparound, but it still transmits the correct bytes somehow.

Now, the "skip_byte" would no longer be accounted to the stats. As
previously, dma->tx_size included also this skip byte, up->icount.tx was
updated by aforementioned uart_xmit_advance() in
omap_8250_dma_tx_complete(). Fix this by using the uart_fifo_out()
helper instead of bare kfifo_get().

Based on patch by Mans Rullgard <mans@mansr.com>

Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Fixes: 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
Link: https://lore.kernel.org/all/20250506150748.3162-1-mans@mansr.com/
Reported-by: Mans Rullgard <mans@mansr.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20250522053835.3495975-1-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/tty/serial/8250/8250_omap.c

index 2a0ce11f405d2d23950c7e75132c609bdcc61859..72ae08d6204ff4c3a060140e7f4690175228b6bb 100644 (file)
@@ -1173,16 +1173,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,18 +1197,22 @@ 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;
                }
-               if (!kfifo_get(&tport->xmit_fifo, &c)) {
+               if (!uart_fifo_out(&p->port, &c, 1)) {
                        ret = -EINVAL;
                        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);
+       }
+
+       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) {
+               ret = -EINVAL;
+               goto err;
        }
 
        desc = dmaengine_prep_slave_sg(dma->txchan, &sg, 1, DMA_MEM_TO_DEV,
@@ -1228,6 +1222,7 @@ static int omap_8250_tx_dma(struct uart_8250_port *p)
                goto err;
        }
 
+       dma->tx_size = sg_dma_len(&sg);
        dma->tx_running = 1;
 
        desc->callback = omap_8250_dma_tx_complete;