diff mbox series

[3/3] arm64: dts: allwinner: h616: identify the UART integration

Message ID 20261023-submit-h616-uart-fifo-v1-v1-3-e4a3bae2eba8@gmail.com (mailing list archive)
State New
Headers
Series serial: 8250_dw: describe the Allwinner H616 UART configuration |

Commit Message

James Hilliard Oct. 23, 2026, 7:33 a.m. UTC 
Use the H616 UART compatible for all six controllers so the driver can
select their fixed 256-byte FIFO configuration and register layout.
Retain the generic DesignWare compatible as a fallback for older kernels.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

sashiko-bot@kernel.org Oct. 23, 2026, 8:32 a.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] arm64: dts: allwinner: h616: enabling UART RX DMA on sun6i-dma loses RX data
--

commit 3bfb41f1bb740f4fc20f5b9cb7ef6f35cf6c4504
Author: James Hilliard <james.hilliard1@gmail.com>

arm64: dts: allwinner: h616: identify the UART integration

This commit updates the UART0 through UART5 nodes in sun50i-h616.dtsi to
use the "allwinner,sun50i-h616-uart" compatible string with
"snps,dw-apb-uart" as a fallback so the driver selects the H616 256-byte
FIFO configuration and register layout.

> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> index 1598e86259ab5..6905c9eaa2493 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> @@ -538,7 +538,7 @@ mmc2: mmc@4022000 {
>  		};
>  
>  		uart0: serial@5000000 {
> -			compatible = "snps,dw-apb-uart";
> +			compatible = "allwinner,sun50i-h616-uart", "snps,dw-apb-uart";

[Severity: High]
Could switching uart0 through uart5 to "allwinner,sun50i-h616-uart" cause RX
data loss and wedge RX DMA on non-console H616 UART ports when paired with
their existing dmas properties referencing sun6i-dma?

Previously, matching "snps,dw-apb-uart" caused dw8250_setup_port() to read
DW_UART_CPR as 0, leaving p->fifosize == 0 and up->dma == NULL. Matching
"allwinner,sun50i-h616-uart" selects dw8250_h616_data in dw8250_probe(),
which attaches 8250 DMA to the port:

drivers/tty/serial/8250/8250_dw.c:dw8250_probe() {
	...
	if (data->pdata && data->pdata->type) {
		dw8250_setup_port_fixed(up, data->pdata->type);
		data->data.dma.rxconf.src_maxburst = data->pdata->dma_maxburst;
		data->data.dma.txconf.dst_maxburst = data->pdata->dma_maxburst;
		up->dma = &data->data.dma;
	}
	...
}

Because uart0 through uart5 in arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
already specify dmas = <&dma ...> and dma-names = "tx", "rx" pointing to
"allwinner,sun50i-h616-dma" (handled by drivers/dma/sun6i-dma.c), opening any
non-console UART calls serial8250_request_dma() and enables 8250 RX DMA.

First, when serial8250_rx_dma_flush() runs (for example on
UART_IIR_RX_TIMEOUT, UART_IIR_RDI, UART_IIR_RLSI, or dw8250_idle_enter()), it
pauses the channel, calls __dma_rx_complete(), and then terminates the
channel:

drivers/tty/serial/8250/8250_dma.c:serial8250_rx_dma_flush() {
	...
	dmaengine_pause(dma->rxchan);
	__dma_rx_complete(p);
	dmaengine_terminate_async(dma->rxchan);
}

In __dma_rx_complete(), dmaengine_tx_status() is checked and returns early if
the status is DMA_IN_PROGRESS:

drivers/tty/serial/8250/8250_dma.c:__dma_rx_complete() {
	...
	dma_status = dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state);
	if (dma_status == DMA_IN_PROGRESS)
		return;

	count = dma->rx_size - state.residue;

	tty_insert_flip_string(tty_port, dma->rx_buf, count);
	p->port.icount.rx += count;
	dma->rx_running = 0;
}

However, in drivers/dma/sun6i-dma.c, sun6i_dma_tx_status() returns the value
of dma_cookie_status() (DMA_IN_PROGRESS) directly and never returns
DMA_PAUSED even after sun6i_dma_pause() has paused the channel:

drivers/dma/sun6i-dma.c:sun6i_dma_tx_status() {
	...
	ret = dma_cookie_status(chan, cookie, state);
	if (ret == DMA_COMPLETE || !state)
		return ret;
	...
	dma_set_residue(state, bytes);

	return ret;
}

As a result, __dma_rx_complete() returns early without pushing the
transferred bytes (dma->rx_size - state.residue) to the TTY flip buffer and
without clearing dma->rx_running = 0. Then dmaengine_terminate_async() frees
the descriptor via sun6i_dma_terminate_all() so the completion callback
dma_rx_complete() never runs, discarding the received bytes and leaving
dma->rx_running stuck at 1.

Second, when UART_IIR_RDI triggers handle_rx_dma() -> serial8250_rx_dma() in
hardirq context, sun6i_dma_issue_pending() does not start a physical DMA
channel immediately; it only queues the virtual channel on sdev->pending and
schedules sdev->task:

drivers/dma/sun6i-dma.c:sun6i_dma_issue_pending() {
	...
	if (vchan_issue_pending(&vchan->vc)) {
		spin_lock(&sdev->lock);

		if (!vchan->phy && list_empty(&vchan->node)) {
			list_add_tail(&vchan->node, &sdev->pending);
			tasklet_schedule(&sdev->task);
	...
}

Since serial8250_rx_dma() neither masks UART_IER_RDI nor drains UART_RX, won't
the level-high UART interrupt (IRQ_TYPE_LEVEL_HIGH) remain asserted and
immediately re-enter handle_rx_dma() with UART_IIR_RDI and
up->dma->rx_running == 1 before sun6i_dma_tasklet() can run, flushing and
terminating the newly queued RX DMA transfer?
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
index 1598e86259ab..6905c9eaa249 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
@@ -538,7 +538,7 @@  mmc2: mmc@4022000 {
 		};
 
 		uart0: serial@5000000 {
-			compatible = "snps,dw-apb-uart";
+			compatible = "allwinner,sun50i-h616-uart", "snps,dw-apb-uart";
 			reg = <0x05000000 0x400>;
 			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
 			reg-shift = <2>;
@@ -551,7 +551,7 @@  uart0: serial@5000000 {
 		};
 
 		uart1: serial@5000400 {
-			compatible = "snps,dw-apb-uart";
+			compatible = "allwinner,sun50i-h616-uart", "snps,dw-apb-uart";
 			reg = <0x05000400 0x400>;
 			interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
 			reg-shift = <2>;
@@ -564,7 +564,7 @@  uart1: serial@5000400 {
 		};
 
 		uart2: serial@5000800 {
-			compatible = "snps,dw-apb-uart";
+			compatible = "allwinner,sun50i-h616-uart", "snps,dw-apb-uart";
 			reg = <0x05000800 0x400>;
 			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
 			reg-shift = <2>;
@@ -577,7 +577,7 @@  uart2: serial@5000800 {
 		};
 
 		uart3: serial@5000c00 {
-			compatible = "snps,dw-apb-uart";
+			compatible = "allwinner,sun50i-h616-uart", "snps,dw-apb-uart";
 			reg = <0x05000c00 0x400>;
 			interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
 			reg-shift = <2>;
@@ -590,7 +590,7 @@  uart3: serial@5000c00 {
 		};
 
 		uart4: serial@5001000 {
-			compatible = "snps,dw-apb-uart";
+			compatible = "allwinner,sun50i-h616-uart", "snps,dw-apb-uart";
 			reg = <0x05001000 0x400>;
 			interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
 			reg-shift = <2>;
@@ -603,7 +603,7 @@  uart4: serial@5001000 {
 		};
 
 		uart5: serial@5001400 {
-			compatible = "snps,dw-apb-uart";
+			compatible = "allwinner,sun50i-h616-uart", "snps,dw-apb-uart";
 			reg = <0x05001400 0x400>;
 			interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
 			reg-shift = <2>;