Re: [PATCH 11/15] tty: msm_serial: use dmaengine_prep_slave_sg()

From: Marek Szyprowski
Date: Wed Apr 17 2024 - 06:15:42 EST


Hi Jiri,

On 16.04.2024 12:23, Jiri Slaby wrote:
> On 15. 04. 24, 23:17, Marek Szyprowski wrote:
>> On 05.04.2024 08:08, Jiri Slaby (SUSE) wrote:
>>> This is a preparatory for the serial-to-kfifo switch. kfifo understands
>>> only scatter-gatter approach, so switch to that.
>>>
>>> No functional change intended, it's just dmaengine_prep_slave_single()
>>> inline expanded.
>>>
>>> And in this case, switch from dma_map_single() to dma_map_sg() too.
>>> This
>>> needs struct msm_dma changes. I split the rx and tx parts into an
>>> union.
>>> TX is now struct scatterlist, RX remains the old good phys-virt-count
>>> triple.
>>>
>>> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@xxxxxxxxxx>
>>> Cc: Bjorn Andersson <andersson@xxxxxxxxxx>
>>> Cc: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
>>> Cc: linux-arm-msm@xxxxxxxxxxxxxxx
>>
>> I've just found that this patch broke UART operation on DragonBoard
>> 410c. I briefly checked and didn't notice anything obviously wrong here,
>> but the board stops transmitting any data from its serial port after the
>> first message. I will try to analyze this issue a bit more tomorrow.
>
> I double checked, but I see no immediate issues in the patch too. So
> please, if you can analyze this more…

I've spent some time digging into this issue and frankly speaking I
still have no idea WHY it doesn't work (or I seriously mixed something
in the scatterlist principles). However I found a workaround to make it
working. Maybe it will help a bit guessing what happens there.

Here is a workaround:

diff --git a/drivers/tty/serial/msm_serial.c
b/drivers/tty/serial/msm_serial.c
index ae7a8e3cf467..fd3f3bf03f33 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -169,6 +169,7 @@ struct msm_dma {
                } rx;
                struct scatterlist tx_sg;
        };
+       int                     mapped;
        dma_cookie_t            cookie;
        u32                     enable_bit;
        struct dma_async_tx_descriptor  *desc;
@@ -278,6 +279,7 @@ static void msm_stop_dma(struct uart_port *port,
struct msm_dma *dma)
                if (dma->dir == DMA_TO_DEVICE) {
                        dma_unmap_sg(dev, &dma->tx_sg, 1, dma->dir);
                        sg_init_table(&dma->tx_sg, 1);
+                       dma->mapped = 0;
                } else
                        dma_unmap_single(dev, dma->rx.phys, mapped,
dma->dir);
        }
@@ -434,7 +436,7 @@ static void msm_start_tx(struct uart_port *port)
        struct msm_dma *dma = &msm_port->tx_dma;

        /* Already started in DMA mode */
-       if (sg_dma_len(&dma->tx_sg))
+       if (dma->mapped)
                return;

        msm_port->imr |= MSM_UART_IMR_TXLEV;
@@ -462,7 +464,7 @@ static void msm_complete_tx_dma(void *args)
        uart_port_lock_irqsave(port, &flags);

        /* Already stopped */
-       if (!sg_dma_len(&dma->tx_sg))
+       if (!dma->mapped)
                goto done;

        dmaengine_tx_status(dma->chan, dma->cookie, &state);
@@ -481,6 +483,7 @@ static void msm_complete_tx_dma(void *args)
        count = sg_dma_len(&dma->tx_sg) - state.residue;
        uart_xmit_advance(port, count);
        sg_init_table(&dma->tx_sg, 1);
+       dma->mapped = 0;

        /* Restore "Tx FIFO below watermark" interrupt */
        msm_port->imr |= MSM_UART_IMR_TXLEV;
@@ -522,6 +525,7 @@ static int msm_handle_tx_dma(struct msm_port
*msm_port, unsigned int count)
        dma->desc->callback_param = msm_port;

        dma->cookie = dmaengine_submit(dma->desc);
+       dma->mapped = 1;
        ret = dma_submit_error(dma->cookie);
        if (ret)
                goto unmap;
@@ -549,6 +553,7 @@ static int msm_handle_tx_dma(struct msm_port
*msm_port, unsigned int count)
 unmap:
        dma_unmap_sg(port->dev, &dma->tx_sg, 1, dma->dir);
        sg_init_table(&dma->tx_sg, 1);
+       dma->mapped = 0;
        return ret;
 }


Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland