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