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@kernel.org>
Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
Cc: linux-arm-msm@vger.kernel.org
---
 drivers/tty/serial/msm_serial.c | 86 +++++++++++++++++++--------------
 1 file changed, 49 insertions(+), 37 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index d27c4c8c84e1..7bf30e632313 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -161,11 +161,16 @@ enum {
 struct msm_dma {
 	struct dma_chan		*chan;
 	enum dma_data_direction dir;
-	dma_addr_t		phys;
-	unsigned char		*virt;
+	union {
+		struct {
+			dma_addr_t		phys;
+			unsigned char		*virt;
+			unsigned int		count;
+		} rx;
+		struct scatterlist tx_sg;
+	};
 	dma_cookie_t		cookie;
 	u32			enable_bit;
-	unsigned int		count;
 	struct dma_async_tx_descriptor	*desc;
 };
 
@@ -249,8 +254,12 @@ static void msm_stop_dma(struct uart_port *port, struct msm_dma *dma)
 	unsigned int mapped;
 	u32 val;
 
-	mapped = dma->count;
-	dma->count = 0;
+	if (dma->dir == DMA_TO_DEVICE) {
+		mapped = sg_dma_len(&dma->tx_sg);
+	} else {
+		mapped = dma->rx.count;
+		dma->rx.count = 0;
+	}
 
 	dmaengine_terminate_all(dma->chan);
 
@@ -265,8 +274,13 @@ static void msm_stop_dma(struct uart_port *port, struct msm_dma *dma)
 	val &= ~dma->enable_bit;
 	msm_write(port, val, UARTDM_DMEN);
 
-	if (mapped)
-		dma_unmap_single(dev, dma->phys, mapped, dma->dir);
+	if (mapped) {
+		if (dma->dir == DMA_TO_DEVICE) {
+			dma_unmap_sg(dev, &dma->tx_sg, 1, dma->dir);
+			sg_init_table(&dma->tx_sg, 1);
+		} else
+			dma_unmap_single(dev, dma->rx.phys, mapped, dma->dir);
+	}
 }
 
 static void msm_release_dma(struct msm_port *msm_port)
@@ -285,7 +299,7 @@ static void msm_release_dma(struct msm_port *msm_port)
 	if (dma->chan) {
 		msm_stop_dma(&msm_port->uart, dma);
 		dma_release_channel(dma->chan);
-		kfree(dma->virt);
+		kfree(dma->rx.virt);
 	}
 
 	memset(dma, 0, sizeof(*dma));
@@ -357,8 +371,8 @@ static void msm_request_rx_dma(struct msm_port *msm_port, resource_size_t base)
 
 	of_property_read_u32(dev->of_node, "qcom,rx-crci", &crci);
 
-	dma->virt = kzalloc(UARTDM_RX_SIZE, GFP_KERNEL);
-	if (!dma->virt)
+	dma->rx.virt = kzalloc(UARTDM_RX_SIZE, GFP_KERNEL);
+	if (!dma->rx.virt)
 		goto rel_rx;
 
 	memset(&conf, 0, sizeof(conf));
@@ -385,7 +399,7 @@ static void msm_request_rx_dma(struct msm_port *msm_port, resource_size_t base)
 
 	return;
 err:
-	kfree(dma->virt);
+	kfree(dma->rx.virt);
 rel_rx:
 	dma_release_channel(dma->chan);
 no_rx:
@@ -420,7 +434,7 @@ static void msm_start_tx(struct uart_port *port)
 	struct msm_dma *dma = &msm_port->tx_dma;
 
 	/* Already started in DMA mode */
-	if (dma->count)
+	if (sg_dma_len(&dma->tx_sg))
 		return;
 
 	msm_port->imr |= MSM_UART_IMR_TXLEV;
@@ -448,12 +462,12 @@ static void msm_complete_tx_dma(void *args)
 	uart_port_lock_irqsave(port, &flags);
 
 	/* Already stopped */
-	if (!dma->count)
+	if (!sg_dma_len(&dma->tx_sg))
 		goto done;
 
 	dmaengine_tx_status(dma->chan, dma->cookie, &state);
 
-	dma_unmap_single(port->dev, dma->phys, dma->count, dma->dir);
+	dma_unmap_sg(port->dev, &dma->tx_sg, 1, dma->dir);
 
 	val = msm_read(port, UARTDM_DMEN);
 	val &= ~dma->enable_bit;
@@ -464,9 +478,9 @@ static void msm_complete_tx_dma(void *args)
 		msm_write(port, MSM_UART_CR_TX_ENABLE, MSM_UART_CR);
 	}
 
-	count = dma->count - state.residue;
+	count = sg_dma_len(&dma->tx_sg) - state.residue;
 	uart_xmit_advance(port, count);
-	dma->count = 0;
+	sg_init_table(&dma->tx_sg, 1);
 
 	/* Restore "Tx FIFO below watermark" interrupt */
 	msm_port->imr |= MSM_UART_IMR_TXLEV;
@@ -485,19 +499,18 @@ static int msm_handle_tx_dma(struct msm_port *msm_port, unsigned int count)
 	struct circ_buf *xmit = &msm_port->uart.state->xmit;
 	struct uart_port *port = &msm_port->uart;
 	struct msm_dma *dma = &msm_port->tx_dma;
-	void *cpu_addr;
 	int ret;
 	u32 val;
 
-	cpu_addr = &xmit->buf[xmit->tail];
+	sg_init_table(&dma->tx_sg, 1);
+	sg_set_buf(&dma->tx_sg, &xmit->buf[xmit->tail], count);
 
-	dma->phys = dma_map_single(port->dev, cpu_addr, count, dma->dir);
-	ret = dma_mapping_error(port->dev, dma->phys);
+	ret = dma_map_sg(port->dev, &dma->tx_sg, 1, dma->dir);
 	if (ret)
 		return ret;
 
-	dma->desc = dmaengine_prep_slave_single(dma->chan, dma->phys,
-						count, DMA_MEM_TO_DEV,
+	dma->desc = dmaengine_prep_slave_sg(dma->chan, &dma->tx_sg, 1,
+						DMA_MEM_TO_DEV,
 						DMA_PREP_INTERRUPT |
 						DMA_PREP_FENCE);
 	if (!dma->desc) {
@@ -520,8 +533,6 @@ static int msm_handle_tx_dma(struct msm_port *msm_port, unsigned int count)
 	msm_port->imr &= ~MSM_UART_IMR_TXLEV;
 	msm_write(port, msm_port->imr, MSM_UART_IMR);
 
-	dma->count = count;
-
 	val = msm_read(port, UARTDM_DMEN);
 	val |= dma->enable_bit;
 
@@ -536,7 +547,8 @@ static int msm_handle_tx_dma(struct msm_port *msm_port, unsigned int count)
 	dma_async_issue_pending(dma->chan);
 	return 0;
 unmap:
-	dma_unmap_single(port->dev, dma->phys, count, dma->dir);
+	dma_unmap_sg(port->dev, &dma->tx_sg, 1, dma->dir);
+	sg_init_table(&dma->tx_sg, 1);
 	return ret;
 }
 
@@ -553,7 +565,7 @@ static void msm_complete_rx_dma(void *args)
 	uart_port_lock_irqsave(port, &flags);
 
 	/* Already stopped */
-	if (!dma->count)
+	if (!dma->rx.count)
 		goto done;
 
 	val = msm_read(port, UARTDM_DMEN);
@@ -570,14 +582,14 @@ static void msm_complete_rx_dma(void *args)
 
 	port->icount.rx += count;
 
-	dma->count = 0;
+	dma->rx.count = 0;
 
-	dma_unmap_single(port->dev, dma->phys, UARTDM_RX_SIZE, dma->dir);
+	dma_unmap_single(port->dev, dma->rx.phys, UARTDM_RX_SIZE, dma->dir);
 
 	for (i = 0; i < count; i++) {
 		char flag = TTY_NORMAL;
 
-		if (msm_port->break_detected && dma->virt[i] == 0) {
+		if (msm_port->break_detected && dma->rx.virt[i] == 0) {
 			port->icount.brk++;
 			flag = TTY_BREAK;
 			msm_port->break_detected = false;
@@ -588,9 +600,9 @@ static void msm_complete_rx_dma(void *args)
 		if (!(port->read_status_mask & MSM_UART_SR_RX_BREAK))
 			flag = TTY_NORMAL;
 
-		sysrq = uart_prepare_sysrq_char(port, dma->virt[i]);
+		sysrq = uart_prepare_sysrq_char(port, dma->rx.virt[i]);
 		if (!sysrq)
-			tty_insert_flip_char(tport, dma->virt[i], flag);
+			tty_insert_flip_char(tport, dma->rx.virt[i], flag);
 	}
 
 	msm_start_rx_dma(msm_port);
@@ -614,13 +626,13 @@ static void msm_start_rx_dma(struct msm_port *msm_port)
 	if (!dma->chan)
 		return;
 
-	dma->phys = dma_map_single(uart->dev, dma->virt,
+	dma->rx.phys = dma_map_single(uart->dev, dma->rx.virt,
 				   UARTDM_RX_SIZE, dma->dir);
-	ret = dma_mapping_error(uart->dev, dma->phys);
+	ret = dma_mapping_error(uart->dev, dma->rx.phys);
 	if (ret)
 		goto sw_mode;
 
-	dma->desc = dmaengine_prep_slave_single(dma->chan, dma->phys,
+	dma->desc = dmaengine_prep_slave_single(dma->chan, dma->rx.phys,
 						UARTDM_RX_SIZE, DMA_DEV_TO_MEM,
 						DMA_PREP_INTERRUPT);
 	if (!dma->desc)
@@ -648,7 +660,7 @@ static void msm_start_rx_dma(struct msm_port *msm_port)
 
 	msm_write(uart, msm_port->imr, MSM_UART_IMR);
 
-	dma->count = UARTDM_RX_SIZE;
+	dma->rx.count = UARTDM_RX_SIZE;
 
 	dma_async_issue_pending(dma->chan);
 
@@ -668,7 +680,7 @@ static void msm_start_rx_dma(struct msm_port *msm_port)
 
 	return;
 unmap:
-	dma_unmap_single(uart->dev, dma->phys, UARTDM_RX_SIZE, dma->dir);
+	dma_unmap_single(uart->dev, dma->rx.phys, UARTDM_RX_SIZE, dma->dir);
 
 sw_mode:
 	/*
@@ -955,7 +967,7 @@ static irqreturn_t msm_uart_irq(int irq, void *dev_id)
 	}
 
 	if (misr & (MSM_UART_IMR_RXLEV | MSM_UART_IMR_RXSTALE)) {
-		if (dma->count) {
+		if (dma->rx.count) {
 			val = MSM_UART_CR_CMD_STALE_EVENT_DISABLE;
 			msm_write(port, val, MSM_UART_CR);
 			val = MSM_UART_CR_CMD_RESET_STALE_INT;
-- 
2.44.0

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@kernel.org>
> Cc: Bjorn Andersson <andersson@kernel.org>
> Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
> Cc: linux-arm-msm@vger.kernel.org

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.

> ---
>   drivers/tty/serial/msm_serial.c | 86 +++++++++++++++++++--------------
>   1 file changed, 49 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
> index d27c4c8c84e1..7bf30e632313 100644
> --- a/drivers/tty/serial/msm_serial.c
> +++ b/drivers/tty/serial/msm_serial.c
> @@ -161,11 +161,16 @@ enum {
>   struct msm_dma {
>   	struct dma_chan		*chan;
>   	enum dma_data_direction dir;
> -	dma_addr_t		phys;
> -	unsigned char		*virt;
> +	union {
> +		struct {
> +			dma_addr_t		phys;
> +			unsigned char		*virt;
> +			unsigned int		count;
> +		} rx;
> +		struct scatterlist tx_sg;
> +	};
>   	dma_cookie_t		cookie;
>   	u32			enable_bit;
> -	unsigned int		count;
>   	struct dma_async_tx_descriptor	*desc;
>   };
>   
> @@ -249,8 +254,12 @@ static void msm_stop_dma(struct uart_port *port, struct msm_dma *dma)
>   	unsigned int mapped;
>   	u32 val;
>   
> -	mapped = dma->count;
> -	dma->count = 0;
> +	if (dma->dir == DMA_TO_DEVICE) {
> +		mapped = sg_dma_len(&dma->tx_sg);
> +	} else {
> +		mapped = dma->rx.count;
> +		dma->rx.count = 0;
> +	}
>   
>   	dmaengine_terminate_all(dma->chan);
>   
> @@ -265,8 +274,13 @@ static void msm_stop_dma(struct uart_port *port, struct msm_dma *dma)
>   	val &= ~dma->enable_bit;
>   	msm_write(port, val, UARTDM_DMEN);
>   
> -	if (mapped)
> -		dma_unmap_single(dev, dma->phys, mapped, dma->dir);
> +	if (mapped) {
> +		if (dma->dir == DMA_TO_DEVICE) {
> +			dma_unmap_sg(dev, &dma->tx_sg, 1, dma->dir);
> +			sg_init_table(&dma->tx_sg, 1);
> +		} else
> +			dma_unmap_single(dev, dma->rx.phys, mapped, dma->dir);
> +	}
>   }
>   
>   static void msm_release_dma(struct msm_port *msm_port)
> @@ -285,7 +299,7 @@ static void msm_release_dma(struct msm_port *msm_port)
>   	if (dma->chan) {
>   		msm_stop_dma(&msm_port->uart, dma);
>   		dma_release_channel(dma->chan);
> -		kfree(dma->virt);
> +		kfree(dma->rx.virt);
>   	}
>   
>   	memset(dma, 0, sizeof(*dma));
> @@ -357,8 +371,8 @@ static void msm_request_rx_dma(struct msm_port *msm_port, resource_size_t base)
>   
>   	of_property_read_u32(dev->of_node, "qcom,rx-crci", &crci);
>   
> -	dma->virt = kzalloc(UARTDM_RX_SIZE, GFP_KERNEL);
> -	if (!dma->virt)
> +	dma->rx.virt = kzalloc(UARTDM_RX_SIZE, GFP_KERNEL);
> +	if (!dma->rx.virt)
>   		goto rel_rx;
>   
>   	memset(&conf, 0, sizeof(conf));
> @@ -385,7 +399,7 @@ static void msm_request_rx_dma(struct msm_port *msm_port, resource_size_t base)
>   
>   	return;
>   err:
> -	kfree(dma->virt);
> +	kfree(dma->rx.virt);
>   rel_rx:
>   	dma_release_channel(dma->chan);
>   no_rx:
> @@ -420,7 +434,7 @@ static void msm_start_tx(struct uart_port *port)
>   	struct msm_dma *dma = &msm_port->tx_dma;
>   
>   	/* Already started in DMA mode */
> -	if (dma->count)
> +	if (sg_dma_len(&dma->tx_sg))
>   		return;
>   
>   	msm_port->imr |= MSM_UART_IMR_TXLEV;
> @@ -448,12 +462,12 @@ static void msm_complete_tx_dma(void *args)
>   	uart_port_lock_irqsave(port, &flags);
>   
>   	/* Already stopped */
> -	if (!dma->count)
> +	if (!sg_dma_len(&dma->tx_sg))
>   		goto done;
>   
>   	dmaengine_tx_status(dma->chan, dma->cookie, &state);
>   
> -	dma_unmap_single(port->dev, dma->phys, dma->count, dma->dir);
> +	dma_unmap_sg(port->dev, &dma->tx_sg, 1, dma->dir);
>   
>   	val = msm_read(port, UARTDM_DMEN);
>   	val &= ~dma->enable_bit;
> @@ -464,9 +478,9 @@ static void msm_complete_tx_dma(void *args)
>   		msm_write(port, MSM_UART_CR_TX_ENABLE, MSM_UART_CR);
>   	}
>   
> -	count = dma->count - state.residue;
> +	count = sg_dma_len(&dma->tx_sg) - state.residue;
>   	uart_xmit_advance(port, count);
> -	dma->count = 0;
> +	sg_init_table(&dma->tx_sg, 1);
>   
>   	/* Restore "Tx FIFO below watermark" interrupt */
>   	msm_port->imr |= MSM_UART_IMR_TXLEV;
> @@ -485,19 +499,18 @@ static int msm_handle_tx_dma(struct msm_port *msm_port, unsigned int count)
>   	struct circ_buf *xmit = &msm_port->uart.state->xmit;
>   	struct uart_port *port = &msm_port->uart;
>   	struct msm_dma *dma = &msm_port->tx_dma;
> -	void *cpu_addr;
>   	int ret;
>   	u32 val;
>   
> -	cpu_addr = &xmit->buf[xmit->tail];
> +	sg_init_table(&dma->tx_sg, 1);
> +	sg_set_buf(&dma->tx_sg, &xmit->buf[xmit->tail], count);
>   
> -	dma->phys = dma_map_single(port->dev, cpu_addr, count, dma->dir);
> -	ret = dma_mapping_error(port->dev, dma->phys);
> +	ret = dma_map_sg(port->dev, &dma->tx_sg, 1, dma->dir);
>   	if (ret)
>   		return ret;
>   
> -	dma->desc = dmaengine_prep_slave_single(dma->chan, dma->phys,
> -						count, DMA_MEM_TO_DEV,
> +	dma->desc = dmaengine_prep_slave_sg(dma->chan, &dma->tx_sg, 1,
> +						DMA_MEM_TO_DEV,
>   						DMA_PREP_INTERRUPT |
>   						DMA_PREP_FENCE);
>   	if (!dma->desc) {
> @@ -520,8 +533,6 @@ static int msm_handle_tx_dma(struct msm_port *msm_port, unsigned int count)
>   	msm_port->imr &= ~MSM_UART_IMR_TXLEV;
>   	msm_write(port, msm_port->imr, MSM_UART_IMR);
>   
> -	dma->count = count;
> -
>   	val = msm_read(port, UARTDM_DMEN);
>   	val |= dma->enable_bit;
>   
> @@ -536,7 +547,8 @@ static int msm_handle_tx_dma(struct msm_port *msm_port, unsigned int count)
>   	dma_async_issue_pending(dma->chan);
>   	return 0;
>   unmap:
> -	dma_unmap_single(port->dev, dma->phys, count, dma->dir);
> +	dma_unmap_sg(port->dev, &dma->tx_sg, 1, dma->dir);
> +	sg_init_table(&dma->tx_sg, 1);
>   	return ret;
>   }
>   
> @@ -553,7 +565,7 @@ static void msm_complete_rx_dma(void *args)
>   	uart_port_lock_irqsave(port, &flags);
>   
>   	/* Already stopped */
> -	if (!dma->count)
> +	if (!dma->rx.count)
>   		goto done;
>   
>   	val = msm_read(port, UARTDM_DMEN);
> @@ -570,14 +582,14 @@ static void msm_complete_rx_dma(void *args)
>   
>   	port->icount.rx += count;
>   
> -	dma->count = 0;
> +	dma->rx.count = 0;
>   
> -	dma_unmap_single(port->dev, dma->phys, UARTDM_RX_SIZE, dma->dir);
> +	dma_unmap_single(port->dev, dma->rx.phys, UARTDM_RX_SIZE, dma->dir);
>   
>   	for (i = 0; i < count; i++) {
>   		char flag = TTY_NORMAL;
>   
> -		if (msm_port->break_detected && dma->virt[i] == 0) {
> +		if (msm_port->break_detected && dma->rx.virt[i] == 0) {
>   			port->icount.brk++;
>   			flag = TTY_BREAK;
>   			msm_port->break_detected = false;
> @@ -588,9 +600,9 @@ static void msm_complete_rx_dma(void *args)
>   		if (!(port->read_status_mask & MSM_UART_SR_RX_BREAK))
>   			flag = TTY_NORMAL;
>   
> -		sysrq = uart_prepare_sysrq_char(port, dma->virt[i]);
> +		sysrq = uart_prepare_sysrq_char(port, dma->rx.virt[i]);
>   		if (!sysrq)
> -			tty_insert_flip_char(tport, dma->virt[i], flag);
> +			tty_insert_flip_char(tport, dma->rx.virt[i], flag);
>   	}
>   
>   	msm_start_rx_dma(msm_port);
> @@ -614,13 +626,13 @@ static void msm_start_rx_dma(struct msm_port *msm_port)
>   	if (!dma->chan)
>   		return;
>   
> -	dma->phys = dma_map_single(uart->dev, dma->virt,
> +	dma->rx.phys = dma_map_single(uart->dev, dma->rx.virt,
>   				   UARTDM_RX_SIZE, dma->dir);
> -	ret = dma_mapping_error(uart->dev, dma->phys);
> +	ret = dma_mapping_error(uart->dev, dma->rx.phys);
>   	if (ret)
>   		goto sw_mode;
>   
> -	dma->desc = dmaengine_prep_slave_single(dma->chan, dma->phys,
> +	dma->desc = dmaengine_prep_slave_single(dma->chan, dma->rx.phys,
>   						UARTDM_RX_SIZE, DMA_DEV_TO_MEM,
>   						DMA_PREP_INTERRUPT);
>   	if (!dma->desc)
> @@ -648,7 +660,7 @@ static void msm_start_rx_dma(struct msm_port *msm_port)
>   
>   	msm_write(uart, msm_port->imr, MSM_UART_IMR);
>   
> -	dma->count = UARTDM_RX_SIZE;
> +	dma->rx.count = UARTDM_RX_SIZE;
>   
>   	dma_async_issue_pending(dma->chan);
>   
> @@ -668,7 +680,7 @@ static void msm_start_rx_dma(struct msm_port *msm_port)
>   
>   	return;
>   unmap:
> -	dma_unmap_single(uart->dev, dma->phys, UARTDM_RX_SIZE, dma->dir);
> +	dma_unmap_single(uart->dev, dma->rx.phys, UARTDM_RX_SIZE, dma->dir);
>   
>   sw_mode:
>   	/*
> @@ -955,7 +967,7 @@ static irqreturn_t msm_uart_irq(int irq, void *dev_id)
>   	}
>   
>   	if (misr & (MSM_UART_IMR_RXLEV | MSM_UART_IMR_RXSTALE)) {
> -		if (dma->count) {
> +		if (dma->rx.count) {
>   			val = MSM_UART_CR_CMD_STALE_EVENT_DISABLE;
>   			msm_write(port, val, MSM_UART_CR);
>   			val = MSM_UART_CR_CMD_RESET_STALE_INT;

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

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@kernel.org>
>> Cc: Bjorn Andersson <andersson@kernel.org>
>> Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
>> Cc: linux-arm-msm@vger.kernel.org
> 
> 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…

thanks,
-- 
js
suse labs

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@kernel.org>
>>> Cc: Bjorn Andersson <andersson@kernel.org>
>>> Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
>>> Cc: linux-arm-msm@vger.kernel.org
>>
>> 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

On 17. 04. 24, 12:15, Marek Szyprowski wrote:
> 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@kernel.org>
>>>> Cc: Bjorn Andersson <andersson@kernel.org>
>>>> Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>> Cc: linux-arm-msm@vger.kernel.org
>>>
>>> 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.
...
> @@ -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)

Thanks for looking into this.

I was hesitant if I should use a flag. I should have, apparently.

Quick question:
What's value of CONFIG_NEED_SG_DMA_LENGTH in your .config?

thanks,
-- 
js
suse labs

On 17.04.2024 12:50, Jiri Slaby wrote:
> On 17. 04. 24, 12:15, Marek Szyprowski wrote:
>> 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@kernel.org>
>>>>> Cc: Bjorn Andersson <andersson@kernel.org>
>>>>> Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>>> Cc: linux-arm-msm@vger.kernel.org
>>>>
>>>> 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.
> ...
>> @@ -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)
>
> Thanks for looking into this.
>
> I was hesitant if I should use a flag. I should have, apparently.
>
> Quick question:
> What's value of CONFIG_NEED_SG_DMA_LENGTH in your .config?


CONFIG_NEED_SG_DMA_LENGTH=y


I alse tried to change the "if (dma->mapped)" check in msm_start_tx() to:

1. if (dma->tx_sg.length)

2. if (dma->tx_sg.page_link & ~SG_PAGE_LINK_MASK)

but none of the above worked what is really strange and incomprehensible 
for me.


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

On 17. 04. 24, 14:45, Marek Szyprowski wrote:
> I alse tried to change the "if (dma->mapped)" check in msm_start_tx() to:
> 
> 1. if (dma->tx_sg.length)
> 
> 2. if (dma->tx_sg.page_link & ~SG_PAGE_LINK_MASK)
> 
> but none of the above worked what is really strange and incomprehensible
> for me.
> 

Aaaah, nevermind, what about this?

Two bugs:
1) dma_map_sg() returns the number of mapped entries. Not zero!
2) And I forgot to zero tx_sg in case of error.

--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -506,8 +506,8 @@ static int msm_handle_tx_dma(struct msm_port 
*msm_port, unsigned int count)
         kfifo_dma_out_prepare(&tport->xmit_fifo, &dma->tx_sg, 1, count);

         ret = dma_map_sg(port->dev, &dma->tx_sg, 1, dma->dir);
-       if (ret)
-               return ret;
+       if (!ret)
+               goto zero_out;

         dma->desc = dmaengine_prep_slave_sg(dma->chan, &dma->tx_sg, 1,
                                                 DMA_MEM_TO_DEV,
@@ -548,6 +548,7 @@ static int msm_handle_tx_dma(struct msm_port 
*msm_port, unsigned int count)
         return 0;
  unmap:
         dma_unmap_sg(port->dev, &dma->tx_sg, 1, dma->dir);
+zero_out:
         sg_init_table(&dma->tx_sg, 1);
         return ret;
  }


thanks,
-- 
js
suse labs

On 19. 04. 24, 9:43, Jiri Slaby wrote:
> On 17. 04. 24, 14:45, Marek Szyprowski wrote:
>> I alse tried to change the "if (dma->mapped)" check in msm_start_tx() to:
>>
>> 1. if (dma->tx_sg.length)
>>
>> 2. if (dma->tx_sg.page_link & ~SG_PAGE_LINK_MASK)
>>
>> but none of the above worked what is really strange and incomprehensible
>> for me.
>>
> 
> Aaaah, nevermind, what about this?
> 
> Two bugs:
> 1) dma_map_sg() returns the number of mapped entries. Not zero!
> 2) And I forgot to zero tx_sg in case of error.
> 
> --- a/drivers/tty/serial/msm_serial.c
> +++ b/drivers/tty/serial/msm_serial.c
> @@ -506,8 +506,8 @@ static int msm_handle_tx_dma(struct msm_port 
> *msm_port, unsigned int count)
>          kfifo_dma_out_prepare(&tport->xmit_fifo, &dma->tx_sg, 1, count);
> 
>          ret = dma_map_sg(port->dev, &dma->tx_sg, 1, dma->dir);
> -       if (ret)
> -               return ret;
> +       if (!ret)

Still wrong, ret = -EIO missing in here.

> +               goto zero_out;
> 
>          dma->desc = dmaengine_prep_slave_sg(dma->chan, &dma->tx_sg, 1,
>                                                  DMA_MEM_TO_DEV,
> @@ -548,6 +548,7 @@ static int msm_handle_tx_dma(struct msm_port 
> *msm_port, unsigned int count)
>          return 0;
>   unmap:
>          dma_unmap_sg(port->dev, &dma->tx_sg, 1, dma->dir);
> +zero_out:
>          sg_init_table(&dma->tx_sg, 1);
>          return ret;
>   }
> 
> 
> thanks,

-- 
js
suse labs

On 19.04.2024 09:53, Jiri Slaby wrote:
> On 19. 04. 24, 9:43, Jiri Slaby wrote:
>> On 17. 04. 24, 14:45, Marek Szyprowski wrote:
>>> I alse tried to change the "if (dma->mapped)" check in 
>>> msm_start_tx() to:
>>>
>>> 1. if (dma->tx_sg.length)
>>>
>>> 2. if (dma->tx_sg.page_link & ~SG_PAGE_LINK_MASK)
>>>
>>> but none of the above worked what is really strange and 
>>> incomprehensible
>>> for me.
>>>
>>
>> Aaaah, nevermind, what about this?
>>
>> Two bugs:
>> 1) dma_map_sg() returns the number of mapped entries. Not zero!
>> 2) And I forgot to zero tx_sg in case of error.
>>
>> --- a/drivers/tty/serial/msm_serial.c
>> +++ b/drivers/tty/serial/msm_serial.c
>> @@ -506,8 +506,8 @@ static int msm_handle_tx_dma(struct msm_port 
>> *msm_port, unsigned int count)
>>          kfifo_dma_out_prepare(&tport->xmit_fifo, &dma->tx_sg, 1, 
>> count);
>>
>>          ret = dma_map_sg(port->dev, &dma->tx_sg, 1, dma->dir);
>> -       if (ret)
>> -               return ret;
>> +       if (!ret)
>
> Still wrong, ret = -EIO missing in here.

"if (ret <= 0)" seems to be better here.

Indeed this fixed the issue. I checked the code many times, but I missed 
this dma_map_sg() return value issue.

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

>
>> +               goto zero_out;
>>
>>          dma->desc = dmaengine_prep_slave_sg(dma->chan, &dma->tx_sg, 1,
>>                                                  DMA_MEM_TO_DEV,
>> @@ -548,6 +548,7 @@ static int msm_handle_tx_dma(struct msm_port 
>> *msm_port, unsigned int count)
>>          return 0;
>>   unmap:
>>          dma_unmap_sg(port->dev, &dma->tx_sg, 1, dma->dir);
>> +zero_out:
>>          sg_init_table(&dma->tx_sg, 1);
>>          return ret;
>>   }
>>
>>
>> thanks,
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

On 19. 04. 24, 10:00, Marek Szyprowski wrote:
> On 19.04.2024 09:53, Jiri Slaby wrote:
>> On 19. 04. 24, 9:43, Jiri Slaby wrote:
>>> On 17. 04. 24, 14:45, Marek Szyprowski wrote:
>>>> I alse tried to change the "if (dma->mapped)" check in
>>>> msm_start_tx() to:
>>>>
>>>> 1. if (dma->tx_sg.length)
>>>>
>>>> 2. if (dma->tx_sg.page_link & ~SG_PAGE_LINK_MASK)
>>>>
>>>> but none of the above worked what is really strange and
>>>> incomprehensible
>>>> for me.
>>>>
>>>
>>> Aaaah, nevermind, what about this?
>>>
>>> Two bugs:
>>> 1) dma_map_sg() returns the number of mapped entries. Not zero!
>>> 2) And I forgot to zero tx_sg in case of error.
>>>
>>> --- a/drivers/tty/serial/msm_serial.c
>>> +++ b/drivers/tty/serial/msm_serial.c
>>> @@ -506,8 +506,8 @@ static int msm_handle_tx_dma(struct msm_port
>>> *msm_port, unsigned int count)
>>>           kfifo_dma_out_prepare(&tport->xmit_fifo, &dma->tx_sg, 1,
>>> count);
>>>
>>>           ret = dma_map_sg(port->dev, &dma->tx_sg, 1, dma->dir);
>>> -       if (ret)
>>> -               return ret;
>>> +       if (!ret)
>>
>> Still wrong, ret = -EIO missing in here.
> 
> "if (ret <= 0)" seems to be better here.

It returns unsigned, so I have a better patch. Will send in a second.

> Indeed this fixed the issue. I checked the code many times, but I missed
> this dma_map_sg() return value issue.

Perfect!

> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

Could you test that one too :)?

thanks,
-- 
js
suse labs

The -next commit f8fef2fa419f (tty: msm_serial: use
dmaengine_prep_slave_sg()), switched to using dma_map_sg(). But the
return value of dma_map_sg() is special: it returns number of elements
mapped. And not a standard error value.

The commit also forgot to reset dma->tx_sg in case of this failure.

Fix both these mistakes.

Thanks to Marek who helped debugging this.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/tty/serial/msm_serial.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index ae7a8e3cf467..0a9c5219df88 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -499,15 +499,18 @@ static int msm_handle_tx_dma(struct msm_port *msm_port, unsigned int count)
 	struct uart_port *port = &msm_port->uart;
 	struct tty_port *tport = &port->state->port;
 	struct msm_dma *dma = &msm_port->tx_dma;
+	unsigned int mapped;
 	int ret;
 	u32 val;
 
 	sg_init_table(&dma->tx_sg, 1);
 	kfifo_dma_out_prepare(&tport->xmit_fifo, &dma->tx_sg, 1, count);
 
-	ret = dma_map_sg(port->dev, &dma->tx_sg, 1, dma->dir);
-	if (ret)
-		return ret;
+	mapped = dma_map_sg(port->dev, &dma->tx_sg, 1, dma->dir);
+	if (!mapped) {
+		ret = -EIO;
+		goto zero_sg;
+	}
 
 	dma->desc = dmaengine_prep_slave_sg(dma->chan, &dma->tx_sg, 1,
 						DMA_MEM_TO_DEV,
@@ -548,6 +551,7 @@ static int msm_handle_tx_dma(struct msm_port *msm_port, unsigned int count)
 	return 0;
 unmap:
 	dma_unmap_sg(port->dev, &dma->tx_sg, 1, dma->dir);
+zero_sg:
 	sg_init_table(&dma->tx_sg, 1);
 	return ret;
 }
-- 
2.44.0

On 19.04.2024 10:09, Jiri Slaby (SUSE) wrote:
> The -next commit f8fef2fa419f (tty: msm_serial: use
> dmaengine_prep_slave_sg()), switched to using dma_map_sg(). But the
> return value of dma_map_sg() is special: it returns number of elements
> mapped. And not a standard error value.
>
> The commit also forgot to reset dma->tx_sg in case of this failure.
>
> Fix both these mistakes.
>
> Thanks to Marek who helped debugging this.
>
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
>   drivers/tty/serial/msm_serial.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
> index ae7a8e3cf467..0a9c5219df88 100644
> --- a/drivers/tty/serial/msm_serial.c
> +++ b/drivers/tty/serial/msm_serial.c
> @@ -499,15 +499,18 @@ static int msm_handle_tx_dma(struct msm_port *msm_port, unsigned int count)
>   	struct uart_port *port = &msm_port->uart;
>   	struct tty_port *tport = &port->state->port;
>   	struct msm_dma *dma = &msm_port->tx_dma;
> +	unsigned int mapped;
>   	int ret;
>   	u32 val;
>   
>   	sg_init_table(&dma->tx_sg, 1);
>   	kfifo_dma_out_prepare(&tport->xmit_fifo, &dma->tx_sg, 1, count);
>   
> -	ret = dma_map_sg(port->dev, &dma->tx_sg, 1, dma->dir);
> -	if (ret)
> -		return ret;
> +	mapped = dma_map_sg(port->dev, &dma->tx_sg, 1, dma->dir);
> +	if (!mapped) {
> +		ret = -EIO;
> +		goto zero_sg;
> +	}
>   
>   	dma->desc = dmaengine_prep_slave_sg(dma->chan, &dma->tx_sg, 1,
>   						DMA_MEM_TO_DEV,
> @@ -548,6 +551,7 @@ static int msm_handle_tx_dma(struct msm_port *msm_port, unsigned int count)
>   	return 0;
>   unmap:
>   	dma_unmap_sg(port->dev, &dma->tx_sg, 1, dma->dir);
> +zero_sg:
>   	sg_init_table(&dma->tx_sg, 1);
>   	return ret;
>   }

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

On 17. 04. 24, 14:45, Marek Szyprowski wrote:
> On 17.04.2024 12:50, Jiri Slaby wrote:
>> On 17. 04. 24, 12:15, Marek Szyprowski wrote:
>>> 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@kernel.org>
>>>>>> Cc: Bjorn Andersson <andersson@kernel.org>
>>>>>> Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>>>> Cc: linux-arm-msm@vger.kernel.org
>>>>>
>>>>> 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.
>> ...
>>> @@ -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)
>>
>> Thanks for looking into this.
>>
>> I was hesitant if I should use a flag. I should have, apparently.
>>
>> Quick question:
>> What's value of CONFIG_NEED_SG_DMA_LENGTH in your .config?
> 
> 
> CONFIG_NEED_SG_DMA_LENGTH=y
> 
> 
> I alse tried to change the "if (dma->mapped)" check in msm_start_tx() to:
> 
> 1. if (dma->tx_sg.length)
> 
> 2. if (dma->tx_sg.page_link & ~SG_PAGE_LINK_MASK)
> 
> but none of the above worked what is really strange and incomprehensible
> for me.

Thanks. Neither for me. Could you add:
         {
                 static DEFINE_RATELIMIT_STATE(rs, 
DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST);
                 if (dma->mapped != !!sg_dma_len(&dma->tx_sg) && 
__ratelimit(&rs))
                         printk_deferred(KERN_DEBUG "%s (%d): mapped=%u 
dma_len=%u\n",
                                         __func__, __LINE__, 
dma->mapped, sg_dma_len(&dma->tx_sg));
         }

before each of your 'if (dma->mapped)' to see where sg_dma_len() is 
wrong and what is its value in the bad case. I hope I did the logic right.

thanks,
-- 
js
suse labs