[PATCH 0/3] serial: 8250_dw: describe the Allwinner H616 UART configuration

[PATCH 0/3] serial: 8250_dw: describe the Allwinner H616 UART configuration

[James Hilliard]

H616 has 256-byte UART FIFOs, but lacks the DesignWare component parameter
register used to discover their capacity. Its debug-divisor and RS485
registers also overlap offsets used to probe optional DesignWare features.

Add an H616 compatible and select a fixed port configuration with the
documented FIFO capacity and receive trigger levels. Preserve the
BUSY-safe integer divisor path and keep the UART gate enabled during
termios changes. Attach the existing 8250 DMA path using single-byte
peripheral bursts, with channel acquisition and PIO fallback handled
by the normal startup path.

The patches add the binding, driver configuration and SoC DTS compatible
in that order. Based on tty-next at 1184f87e2790.

Assisted-by: Codex:gpt-6-astra
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
James Hilliard (3):
      dt-bindings: serial: snps-dw-apb-uart: add Allwinner H616
      serial: 8250_dw: configure the Allwinner H616 UART
      arm64: dts: allwinner: h616: identify the UART integration

 .../bindings/serial/snps-dw-apb-uart.yaml          |  1 +
 arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi     | 12 +++---
 drivers/tty/serial/8250/8250.h                     |  1 +
 drivers/tty/serial/8250/8250_dw.c                  | 45 ++++++++++++++++++----
 drivers/tty/serial/8250/8250_port.c                |  8 ++++
 5 files changed, 54 insertions(+), 13 deletions(-)
---
base-commit: 1184f87e2790e8af9be3116f608d2e86b4792d7d
change-id: 20261022-submit-h616-uart-fifo-v1-c5afcdfe5cb0

Best regards,
--  
James Hilliard <james.hilliard1@gmail.com>

[PATCH 1/3] dt-bindings: serial: snps-dw-apb-uart: add Allwinner H616

[James Hilliard]

The Allwinner H616 UART has 256-byte FIFOs but does not implement the
DesignWare component parameter register used to discover their size.

Add a compatible to identify this integration and its fixed FIFO
configuration. Retain snps,dw-apb-uart as a fallback.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
index e2ca126d58ba..fc1289073241 100644
--- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
@@ -60,6 +60,7 @@ properties:
           - const: renesas,rzn1-uart
       - items:
           - enum:
+              - allwinner,sun50i-h616-uart
               - anlogic,dr1v90-uart
               - brcm,bcm11351-dw-apb-uart
               - brcm,bcm21664-dw-apb-uart

-- 
2.53.0

[PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART

[James Hilliard]

H616 has 256-byte transmit and receive FIFOs, but its optional DesignWare
component parameter register reads as zero. Generic 16550 probing leaves
the port using the 16-byte FIFO configuration and its trigger-level table.
DMA attachment also depends on a FIFO size being available during probe,
so these ports do not reach the existing 8250 DMA path.

Use a fixed port configuration for the H616 compatible, with a 256-byte
transmit load and the documented receive thresholds of 1, 64, 128 and 254
bytes. Select the quarter-full receive trigger by default.

Skip optional-register probing for this fixed configuration. H616 has
debug-divisor and RS485 control registers at the offsets used to detect
DesignWare hardware RS485 and fractional-divisor support. Probing these
offsets writes unrelated registers and can select incompatible callbacks.
Keep the BUSY-safe integer divisor hook and software RS485 handling.

Use the existing SKIP_SET_RATE quirk to keep the UART clock enabled during
termios changes. Its clock is a gate on the shared APB2 source, rather than
an independent baud-rate generator.

Attach DMA from the fixed configuration before registering the port. The
8250 core installs its FIFO defaults during registration, after the probe's
FIFO-size test. Configure single-byte RX and TX bursts because the
peripheral request can correspond to just one character. The existing
startup path requests the firmware-described channels and falls back to
PIO when they are unavailable or the port is a kernel console.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/tty/serial/8250/8250.h      |  1 +
 drivers/tty/serial/8250/8250_dw.c   | 45 +++++++++++++++++++++++++++++++------
 drivers/tty/serial/8250/8250_port.c |  8 +++++++
 3 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 21ed9c41b751..0f8c1b0b4a71 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -77,6 +77,7 @@ enum uart_port_type {
 	/* Internal 8250 only */
 	UART_PORT_AIROHA		= 124,
 	UART_PORT_AIROHA_HS		= 125,
+	UART_PORT_DW_APB_256	= 126,
 };
 
 #define UART_CAP_FIFO	BIT(8)	/* UART has FIFO */
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index eca02e8bd6d1..f03051740be0 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -63,6 +63,9 @@
 struct dw8250_platform_data {
 	u8 usr_reg;
 	u32 cpr_value;
+	/* Use a fixed configuration instead of probing optional registers. */
+	unsigned int type;
+	unsigned int dma_maxburst;
 	unsigned int quirks;
 };
 
@@ -630,6 +633,18 @@ static void dw8250_shutdown(struct uart_port *port)
 	d->no_int_count = 0;
 }
 
+static void dw8250_setup_port_fixed(struct uart_8250_port *up, unsigned int type)
+{
+	struct uart_port *p = &up->port;
+
+	p->type = type;
+	p->flags |= UPF_FIXED_TYPE;
+	p->rs485_config = serial8250_em485_config;
+	p->rs485_supported = serial8250_em485_supported;
+	up->rs485_start_tx = serial8250_em485_start_tx;
+	up->rs485_stop_tx = serial8250_em485_stop_tx;
+}
+
 static int dw8250_probe(struct platform_device *pdev)
 {
 	struct uart_8250_port uart = {}, *up = &uart;
@@ -765,14 +780,21 @@ static int dw8250_probe(struct platform_device *pdev)
 
 	dw8250_setup_dma_filter(p, data);
 
-	if (!data->skip_autocfg)
-		dw8250_setup_port(p);
-
-	/* If we have a valid fifosize, try hooking up DMA */
-	if (p->fifosize) {
-		data->data.dma.rxconf.src_maxburst = p->fifosize / 4;
-		data->data.dma.txconf.dst_maxburst = p->fifosize / 4;
+	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;
+	} else {
+		if (!data->skip_autocfg)
+			dw8250_setup_port(p);
+
+		/* If we have a valid fifosize, try hooking up DMA */
+		if (p->fifosize) {
+			data->data.dma.rxconf.src_maxburst = p->fifosize / 4;
+			data->data.dma.txconf.dst_maxburst = p->fifosize / 4;
+			up->dma = &data->data.dma;
+		}
 	}
 
 	data->data.uport = serial8250_register_8250_port(up);
@@ -854,6 +876,14 @@ static const struct dw8250_platform_data dw8250_dw_apb = {
 	.usr_reg = DW_UART_USR,
 };
 
+static const struct dw8250_platform_data dw8250_h616_data = {
+	.usr_reg = DW_UART_USR,
+	.type = UART_PORT_DW_APB_256,
+	/* A peripheral request can correspond to just one character. */
+	.dma_maxburst = 1,
+	.quirks = DW_UART_QUIRK_SKIP_SET_RATE,
+};
+
 static const struct dw8250_platform_data dw8250_octeon_3860_data = {
 	.usr_reg = OCTEON_UART_USR,
 	.quirks = DW_UART_QUIRK_OCTEON,
@@ -904,6 +934,7 @@ static const struct dw8250_platform_data dw8250_tda54 = {
 
 static const struct of_device_id dw8250_of_match[] = {
 	{ .compatible = "snps,dw-apb-uart", .data = &dw8250_dw_apb },
+	{ .compatible = "allwinner,sun50i-h616-uart", .data = &dw8250_h616_data },
 	{ .compatible = "cavium,octeon-3860-uart", .data = &dw8250_octeon_3860_data },
 	{ .compatible = "marvell,armada-38x-uart", .data = &dw8250_armada_38x_data },
 	{ .compatible = "renesas,rzn1-uart", .data = &dw8250_renesas_rzn1_data },
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index db066a42f61d..e99d35880a68 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -335,6 +335,14 @@ static const struct serial8250_config uart_config[] = {
 		.rxtrig_bytes	= {1, 4},
 		.flags		= UART_CAP_FIFO,
 	},
+	[UART_PORT_DW_APB_256] = {
+		.name		= "DW APB UART 256",
+		.fifo_size	= 256,
+		.tx_loadsz	= 256,
+		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_01,
+		.rxtrig_bytes	= {1, 64, 128, 254},
+		.flags		= UART_CAP_FIFO | UART_CAP_NOTEMT,
+	},
 };
 
 /* Uart divisor latch read */

-- 
2.53.0

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

[James Hilliard]

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(-)

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>;

-- 
2.53.0

Re: [PATCH 1/3] dt-bindings: serial: snps-dw-apb-uart: add Allwinner H616

[sashiko-bot]

> The Allwinner H616 UART has 256-byte FIFOs but does not implement the
> DesignWare component parameter register used to discover their size.
> 
> Add a compatible to identify this integration and its fixed FIFO
> configuration. Retain snps,dw-apb-uart as a fallback.
> 
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023-submit-h616-uart-fifo-v1-v1-0-e4a3bae2eba8@gmail.com?part=1

Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] serial: 8250_dw: broken H616 RX DMA due to deferred sun6i_dma_issue_pending()
- [High] serial: 8250_dw: missing UART_CAP_AFE in UART_PORT_DW_APB_256 breaks CRTSCTS
--

commit 7b1955806965488d36d1ed6fbef81df345079c9d
Author: James Hilliard <james.hilliard1@gmail.com>

serial: 8250_dw: configure the Allwinner H616 UART

This patch adds a fixed 256-byte FIFO port configuration for the Allwinner
H616 UART, skips optional register probing, sets the SKIP_SET_RATE quirk, and
attaches 8250 DMA with single-byte burst sizes during probe.

> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index eca02e8bd6d13..f03051740be0a 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c

[ ... ]

> @@ -765,14 +780,21 @@ static int dw8250_probe(struct platform_device *pdev)
>  
>  	dw8250_setup_dma_filter(p, data);
>  
> -	if (!data->skip_autocfg)
> -		dw8250_setup_port(p);
> -
> -	/* If we have a valid fifosize, try hooking up DMA */
> -	if (p->fifosize) {
> -		data->data.dma.rxconf.src_maxburst = p->fifosize / 4;
> -		data->data.dma.txconf.dst_maxburst = p->fifosize / 4;
> +	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;

[Severity: High]
Will attaching the standard 8250 RX DMA path here work with the Allwinner
sun6i-dma controller, or does it cause RX DMA transfers to be immediately
aborted and leave RX DMA wedged?

When UART_IIR_RDI fires, serial8250_interrupt() loops in hardirq context:

drivers/tty/serial/8250/8250_core.c:serial8250_interrupt() {
	l = i->head;
	do {
		struct uart_8250_port *up = list_entry(l, struct uart_8250_port, list);
		struct uart_port *port = &up->port;

		if (port->handle_irq(port)) {
			handled = 1;
			end = NULL;
		} else if (end == NULL)
			end = l;
		...
	} while (l != end);
}

On the first pass, dw8250_handle_irq() -> serial8250_handle_irq_locked() ->
handle_rx_dma() sees !up->dma->rx_running and breaks out to call
serial8250_rx_dma(), which sets dma->rx_running = 1 and calls
dma_async_issue_pending(dma->rxchan) without masking UART_IER_RDI or draining
the RX FIFO:

drivers/tty/serial/8250/8250_port.c:handle_rx_dma() {
	switch (iir & 0x3f) {
	case UART_IIR_RDI:
		if (!up->dma->rx_running)
			break;
		fallthrough;
	case UART_IIR_RLSI:
	case UART_IIR_RX_TIMEOUT:
		serial8250_rx_dma_flush(up);
		return true;
	}
	return up->dma->rx_dma(up);
}

Unlike DMA controllers that start transfers synchronously in
device_issue_pending, sun6i_dma_issue_pending() defers physical channel
allocation and hardware transfer start to a softirq tasklet:

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);
			...
		}

		spin_unlock(&sdev->lock);
	}
}

Because serial8250_interrupt() is running in hardirq context, the tasklet
cannot run before serial8250_interrupt() immediately executes a second pass
on the port. Since no bytes have been read from the RX FIFO and UART_IER_RDI
is still enabled, UART_IIR still reports UART_IIR_RDI. On this second pass,
handle_rx_dma() sees up->dma->rx_running == 1 and calls
serial8250_rx_dma_flush(up), which pauses and terminates the DMA transfer that
was just queued.

Furthermore, inside serial8250_rx_dma_flush() -> __dma_rx_complete(),
sun6i_dma_tx_status() returns DMA_IN_PROGRESS even after dmaengine_pause(),
causing __dma_rx_complete() to return early without clearing dma->rx_running
or copying any received bytes before dmaengine_terminate_async() discards the
descriptor:

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;
}

> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index db066a42f61d2..e99d35880a689 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -335,6 +335,14 @@ static const struct serial8250_config uart_config[] = {
>  		.rxtrig_bytes	= {1, 4},
>  		.flags		= UART_CAP_FIFO,
>  	},
> +	[UART_PORT_DW_APB_256] = {
> +		.name		= "DW APB UART 256",
> +		.fifo_size	= 256,
> +		.tx_loadsz	= 256,
> +		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_01,
> +		.rxtrig_bytes	= {1, 64, 128, 254},
> +		.flags		= UART_CAP_FIFO | UART_CAP_NOTEMT,
> +	},

[Severity: High]
Should uart_config[UART_PORT_DW_APB_256].flags also include UART_CAP_AFE?

In dw8250_probe(), dw8250_quirks() sets p->set_termios = dw8250_do_set_termios.
When CRTSCTS is enabled, dw8250_do_set_termios() unconditionally sets
UPSTAT_AUTOCTS, which tells serial_core to disable software CTS flow control:

drivers/tty/serial/8250/8250_dwlib.c:dw8250_do_set_termios() {
	p->status &= ~UPSTAT_AUTOCTS;
	if (termios->c_cflag & CRTSCTS)
		p->status |= UPSTAT_AUTOCTS;

	serial8250_do_set_termios(p, termios, old);
}

However, serial8250_do_set_termios() -> serial8250_set_afe() only enables
hardware auto flow control (UART_MCR_AFE) if UART_CAP_AFE is set in
up->capabilities:

drivers/tty/serial/8250/8250_port.c:serial8250_set_afe() {
	if (!(up->capabilities & UART_CAP_AFE))
		return;

	up->mcr &= ~UART_MCR_AFE;
	if (termios->c_cflag & CRTSCTS)
		up->mcr |= UART_MCR_AFE;
}

Because dw8250_setup_port_fixed() bypasses dw8250_setup_port(),
up->capabilities is populated solely from
uart_config[UART_PORT_DW_APB_256].flags. Without UART_CAP_AFE, enabling
CRTSCTS on H616 UARTs (such as Bluetooth HCI UARTs using uart-has-rtscts)
disables software CTS flow control without enabling hardware auto flow control
in UART_MCR.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023-submit-h616-uart-fifo-v1-v1-0-e4a3bae2eba8@gmail.com?part=2

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

[sashiko-bot]

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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023-submit-h616-uart-fifo-v1-v1-0-e4a3bae2eba8@gmail.com?part=3

Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART

[Andy Shevchenko]

On Fri, Oct 23, 2026 at 01:33:50AM -0600, James Hilliard wrote:
> H616 has 256-byte transmit and receive FIFOs, but its optional DesignWare
> component parameter register reads as zero. Generic 16550 probing leaves
> the port using the 16-byte FIFO configuration and its trigger-level table.
> DMA attachment also depends on a FIFO size being available during probe,
> so these ports do not reach the existing 8250 DMA path.
> 
> Use a fixed port configuration for the H616 compatible, with a 256-byte
> transmit load and the documented receive thresholds of 1, 64, 128 and 254
> bytes. Select the quarter-full receive trigger by default.
> 
> Skip optional-register probing for this fixed configuration. H616 has
> debug-divisor and RS485 control registers at the offsets used to detect
> DesignWare hardware RS485 and fractional-divisor support. Probing these
> offsets writes unrelated registers and can select incompatible callbacks.
> Keep the BUSY-safe integer divisor hook and software RS485 handling.
> 
> Use the existing SKIP_SET_RATE quirk to keep the UART clock enabled during
> termios changes. Its clock is a gate on the shared APB2 source, rather than
> an independent baud-rate generator.
> 
> Attach DMA from the fixed configuration before registering the port. The
> 8250 core installs its FIFO defaults during registration, after the probe's
> FIFO-size test. Configure single-byte RX and TX bursts because the
> peripheral request can correspond to just one character. The existing
> startup path requests the firmware-described channels and falls back to
> PIO when they are unavailable or the port is a kernel console.

...

>  	/* Internal 8250 only */
>  	UART_PORT_AIROHA		= 124,
>  	UART_PORT_AIROHA_HS		= 125,
> +	UART_PORT_DW_APB_256	= 126,

Why? Besides the poorly chosen name, what prevents us from modifying the
corresponding parameters at probe time by leaving the one of the existing
types?

...

>  struct dw8250_platform_data {
>  	u8 usr_reg;
>  	u32 cpr_value;
> +	/* Use a fixed configuration instead of probing optional registers. */
> +	unsigned int type;
> +	unsigned int dma_maxburst;

This sounds like a hack.  Why do we need type here? The address of platform
data is enough, no?

>  	unsigned int quirks;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART

[James Hilliard]

On Fri, Oct 23, 2026 at 2:44 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Oct 23, 2026 at 01:33:50AM -0600, James Hilliard wrote:
> > H616 has 256-byte transmit and receive FIFOs, but its optional DesignWare
> > component parameter register reads as zero. Generic 16550 probing leaves
> > the port using the 16-byte FIFO configuration and its trigger-level table.
> > DMA attachment also depends on a FIFO size being available during probe,
> > so these ports do not reach the existing 8250 DMA path.
> >
> > Use a fixed port configuration for the H616 compatible, with a 256-byte
> > transmit load and the documented receive thresholds of 1, 64, 128 and 254
> > bytes. Select the quarter-full receive trigger by default.
> >
> > Skip optional-register probing for this fixed configuration. H616 has
> > debug-divisor and RS485 control registers at the offsets used to detect
> > DesignWare hardware RS485 and fractional-divisor support. Probing these
> > offsets writes unrelated registers and can select incompatible callbacks.
> > Keep the BUSY-safe integer divisor hook and software RS485 handling.
> >
> > Use the existing SKIP_SET_RATE quirk to keep the UART clock enabled during
> > termios changes. Its clock is a gate on the shared APB2 source, rather than
> > an independent baud-rate generator.
> >
> > Attach DMA from the fixed configuration before registering the port. The
> > 8250 core installs its FIFO defaults during registration, after the probe's
> > FIFO-size test. Configure single-byte RX and TX bursts because the
> > peripheral request can correspond to just one character. The existing
> > startup path requests the firmware-described channels and falls back to
> > PIO when they are unavailable or the port is a kernel console.
>
> ...
>
> >       /* Internal 8250 only */
> >       UART_PORT_AIROHA                = 124,
> >       UART_PORT_AIROHA_HS             = 125,
> > +     UART_PORT_DW_APB_256    = 126,
>
> Why? Besides the poorly chosen name, what prevents us from modifying the
> corresponding parameters at probe time by leaving the one of the existing
> types?

The new type was primarily a way to reuse the core's FIFO configuration
table, including the RX trigger mapping. H616 uses thresholds of 1, 64,
128 and 254 bytes, whereas PORT_16550A maps the same FCR bits to 1, 4, 8
and 14 bytes. Overriding only fifosize and tx_loadsz would leave
rx_trig_bytes reporting and selecting the wrong thresholds.

The core already has get_rxtrig/set_rxtrig callbacks, though. I'll rework
this to retain PORT_16550A, supply the H616 FIFO parameters during setup,
and use those callbacks for the correct trigger mapping.

>
> ...
>
> >  struct dw8250_platform_data {
> >       u8 usr_reg;
> >       u32 cpr_value;
> > +     /* Use a fixed configuration instead of probing optional registers. */
> > +     unsigned int type;
> > +     unsigned int dma_maxburst;
>
> This sounds like a hack.  Why do we need type here? The address of platform
> data is enough, no?

The type member selected the new table entry and also identified the
fixed-configuration path. The matched platform-data pointer can select
the H616 setup directly, including bypassing the incompatible optional
register probes. I'll remove the type member and use that instead.

>
> >       unsigned int quirks;
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART

[Chen-Yu Tsai]

On Sat, Oct 24, 2026 at 12:12 AM James Hilliard
<james.hilliard1@gmail.com> wrote:
>
> On Fri, Oct 23, 2026 at 2:44 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Fri, Oct 23, 2026 at 01:33:50AM -0600, James Hilliard wrote:
> > > H616 has 256-byte transmit and receive FIFOs, but its optional DesignWare
> > > component parameter register reads as zero. Generic 16550 probing leaves
> > > the port using the 16-byte FIFO configuration and its trigger-level table.
> > > DMA attachment also depends on a FIFO size being available during probe,
> > > so these ports do not reach the existing 8250 DMA path.
> > >
> > > Use a fixed port configuration for the H616 compatible, with a 256-byte
> > > transmit load and the documented receive thresholds of 1, 64, 128 and 254
> > > bytes. Select the quarter-full receive trigger by default.
> > >
> > > Skip optional-register probing for this fixed configuration. H616 has
> > > debug-divisor and RS485 control registers at the offsets used to detect
> > > DesignWare hardware RS485 and fractional-divisor support. Probing these
> > > offsets writes unrelated registers and can select incompatible callbacks.
> > > Keep the BUSY-safe integer divisor hook and software RS485 handling.
> > >
> > > Use the existing SKIP_SET_RATE quirk to keep the UART clock enabled during
> > > termios changes. Its clock is a gate on the shared APB2 source, rather than
> > > an independent baud-rate generator.
> > >
> > > Attach DMA from the fixed configuration before registering the port. The
> > > 8250 core installs its FIFO defaults during registration, after the probe's
> > > FIFO-size test. Configure single-byte RX and TX bursts because the
> > > peripheral request can correspond to just one character. The existing
> > > startup path requests the firmware-described channels and falls back to
> > > PIO when they are unavailable or the port is a kernel console.
> >
> > ...
> >
> > >       /* Internal 8250 only */
> > >       UART_PORT_AIROHA                = 124,
> > >       UART_PORT_AIROHA_HS             = 125,
> > > +     UART_PORT_DW_APB_256    = 126,
> >
> > Why? Besides the poorly chosen name, what prevents us from modifying the
> > corresponding parameters at probe time by leaving the one of the existing
> > types?
>
> The new type was primarily a way to reuse the core's FIFO configuration
> table, including the RX trigger mapping. H616 uses thresholds of 1, 64,
> 128 and 254 bytes, whereas PORT_16550A maps the same FCR bits to 1, 4, 8
> and 14 bytes. Overriding only fifosize and tx_loadsz would leave
> rx_trig_bytes reporting and selecting the wrong thresholds.
>
> The core already has get_rxtrig/set_rxtrig callbacks, though. I'll rework
> this to retain PORT_16550A, supply the H616 FIFO parameters during setup,
> and use those callbacks for the correct trigger mapping.

Please consider just taking the FIFO size from the DT. On some chips
not every UART has the same size FIFO, so we already need to deal with
it.

Also, FIFO size is a standard UART DT property, though somehow not included
for dw-uart.


ChenYu

> >
> > ...
> >
> > >  struct dw8250_platform_data {
> > >       u8 usr_reg;
> > >       u32 cpr_value;
> > > +     /* Use a fixed configuration instead of probing optional registers. */
> > > +     unsigned int type;
> > > +     unsigned int dma_maxburst;
> >
> > This sounds like a hack.  Why do we need type here? The address of platform
> > data is enough, no?
>
> The type member selected the new table entry and also identified the
> fixed-configuration path. The matched platform-data pointer can select
> the H616 setup directly, including bypassing the incompatible optional
> register probes. I'll remove the type member and use that instead.
>
> >
> > >       unsigned int quirks;
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
> >
>

Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART

[Andy Shevchenko]

On Sat, Oct 24, 2026 at 10:10:33PM +0900, Chen-Yu Tsai wrote:
> On Sat, Oct 24, 2026 at 12:12 AM James Hilliard
> <james.hilliard1@gmail.com> wrote:
> > On Fri, Oct 23, 2026 at 2:44 AM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Fri, Oct 23, 2026 at 01:33:50AM -0600, James Hilliard wrote:

...

> > > >       /* Internal 8250 only */
> > > >       UART_PORT_AIROHA                = 124,
> > > >       UART_PORT_AIROHA_HS             = 125,
> > > > +     UART_PORT_DW_APB_256    = 126,
> > >
> > > Why? Besides the poorly chosen name, what prevents us from modifying the
> > > corresponding parameters at probe time by leaving the one of the existing
> > > types?
> >
> > The new type was primarily a way to reuse the core's FIFO configuration
> > table, including the RX trigger mapping. H616 uses thresholds of 1, 64,
> > 128 and 254 bytes, whereas PORT_16550A maps the same FCR bits to 1, 4, 8
> > and 14 bytes. Overriding only fifosize and tx_loadsz would leave
> > rx_trig_bytes reporting and selecting the wrong thresholds.
> >
> > The core already has get_rxtrig/set_rxtrig callbacks, though. I'll rework
> > this to retain PORT_16550A, supply the H616 FIFO parameters during setup,
> > and use those callbacks for the correct trigger mapping.
> 
> Please consider just taking the FIFO size from the DT. On some chips
> not every UART has the same size FIFO, so we already need to deal with
> it.
> 
> Also, FIFO size is a standard UART DT property, though somehow not included
> for dw-uart.

FWIW, it supports all standard properties for UART.
Check what uart_read_port_properties() does.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART

[Chen-Yu Tsai]

On Sat, Oct 24, 2026 at 10:19 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Sat, Oct 24, 2026 at 10:10:33PM +0900, Chen-Yu Tsai wrote:
> > On Sat, Oct 24, 2026 at 12:12 AM James Hilliard
> > <james.hilliard1@gmail.com> wrote:
> > > On Fri, Oct 23, 2026 at 2:44 AM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Fri, Oct 23, 2026 at 01:33:50AM -0600, James Hilliard wrote:
>
> ...
>
> > > > >       /* Internal 8250 only */
> > > > >       UART_PORT_AIROHA                = 124,
> > > > >       UART_PORT_AIROHA_HS             = 125,
> > > > > +     UART_PORT_DW_APB_256    = 126,
> > > >
> > > > Why? Besides the poorly chosen name, what prevents us from modifying the
> > > > corresponding parameters at probe time by leaving the one of the existing
> > > > types?
> > >
> > > The new type was primarily a way to reuse the core's FIFO configuration
> > > table, including the RX trigger mapping. H616 uses thresholds of 1, 64,
> > > 128 and 254 bytes, whereas PORT_16550A maps the same FCR bits to 1, 4, 8
> > > and 14 bytes. Overriding only fifosize and tx_loadsz would leave
> > > rx_trig_bytes reporting and selecting the wrong thresholds.
> > >
> > > The core already has get_rxtrig/set_rxtrig callbacks, though. I'll rework
> > > this to retain PORT_16550A, supply the H616 FIFO parameters during setup,
> > > and use those callbacks for the correct trigger mapping.
> >
> > Please consider just taking the FIFO size from the DT. On some chips
> > not every UART has the same size FIFO, so we already need to deal with
> > it.
> >
> > Also, FIFO size is a standard UART DT property, though somehow not included
> > for dw-uart.
>
> FWIW, it supports all standard properties for UART.
> Check what uart_read_port_properties() does.

Ah, that's nice. I hacked up something [1] in the past to get it to
work, but that was probably before you added the helper. I never got
around to splitting up the commits and sending it out.


ChenYu

[1] https://git.kernel.org/pub/scm/linux/kernel/git/wens/linux.git/commit/?id=e7e0741fa04f349ad9f94a4059e32467a30ef5d9

Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART

[Andy Shevchenko]

On Sat, Oct 24, 2026 at 10:41:55PM +0900, Chen-Yu Tsai wrote:
> On Sat, Oct 24, 2026 at 10:19 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Sat, Oct 24, 2026 at 10:10:33PM +0900, Chen-Yu Tsai wrote:
> > > On Sat, Oct 24, 2026 at 12:12 AM James Hilliard
> > > <james.hilliard1@gmail.com> wrote:
> > > > On Fri, Oct 23, 2026 at 2:44 AM Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > On Fri, Oct 23, 2026 at 01:33:50AM -0600, James Hilliard wrote:

...

> > > > > >       /* Internal 8250 only */
> > > > > >       UART_PORT_AIROHA                = 124,
> > > > > >       UART_PORT_AIROHA_HS             = 125,
> > > > > > +     UART_PORT_DW_APB_256    = 126,
> > > > >
> > > > > Why? Besides the poorly chosen name, what prevents us from modifying the
> > > > > corresponding parameters at probe time by leaving the one of the existing
> > > > > types?
> > > >
> > > > The new type was primarily a way to reuse the core's FIFO configuration
> > > > table, including the RX trigger mapping. H616 uses thresholds of 1, 64,
> > > > 128 and 254 bytes, whereas PORT_16550A maps the same FCR bits to 1, 4, 8
> > > > and 14 bytes. Overriding only fifosize and tx_loadsz would leave
> > > > rx_trig_bytes reporting and selecting the wrong thresholds.
> > > >
> > > > The core already has get_rxtrig/set_rxtrig callbacks, though. I'll rework
> > > > this to retain PORT_16550A, supply the H616 FIFO parameters during setup,
> > > > and use those callbacks for the correct trigger mapping.
> > >
> > > Please consider just taking the FIFO size from the DT. On some chips
> > > not every UART has the same size FIFO, so we already need to deal with
> > > it.
> > >
> > > Also, FIFO size is a standard UART DT property, though somehow not included
> > > for dw-uart.
> >
> > FWIW, it supports all standard properties for UART.
> > Check what uart_read_port_properties() does.
> 
> Ah, that's nice. I hacked up something [1] in the past to get it to
> work, but that was probably before you added the helper. I never got
> around to splitting up the commits and sending it out.

Hmm... The helper is in the kernel for the past 2.5 years.
Maybe you rebased it and it lost original dates?

But if it's not working, we need to address that, I believe it should work out
of the box, because the same function is used across dozen of serial drivers.

It might be an issue in 8250_dw.c, but I can't tell for sure.

> [1] https://git.kernel.org/pub/scm/linux/kernel/git/wens/linux.git/commit/?id=e7e0741fa04f349ad9f94a4059e32467a30ef5d9

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART

[Chen-Yu Tsai]

On Sat, Oct 24, 2026 at 10:51 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Sat, Oct 24, 2026 at 10:41:55PM +0900, Chen-Yu Tsai wrote:
> > On Sat, Oct 24, 2026 at 10:19 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Sat, Oct 24, 2026 at 10:10:33PM +0900, Chen-Yu Tsai wrote:
> > > > On Sat, Oct 24, 2026 at 12:12 AM James Hilliard
> > > > <james.hilliard1@gmail.com> wrote:
> > > > > On Fri, Oct 23, 2026 at 2:44 AM Andy Shevchenko
> > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > On Fri, Oct 23, 2026 at 01:33:50AM -0600, James Hilliard wrote:
>
> ...
>
> > > > > > >       /* Internal 8250 only */
> > > > > > >       UART_PORT_AIROHA                = 124,
> > > > > > >       UART_PORT_AIROHA_HS             = 125,
> > > > > > > +     UART_PORT_DW_APB_256    = 126,
> > > > > >
> > > > > > Why? Besides the poorly chosen name, what prevents us from modifying the
> > > > > > corresponding parameters at probe time by leaving the one of the existing
> > > > > > types?
> > > > >
> > > > > The new type was primarily a way to reuse the core's FIFO configuration
> > > > > table, including the RX trigger mapping. H616 uses thresholds of 1, 64,
> > > > > 128 and 254 bytes, whereas PORT_16550A maps the same FCR bits to 1, 4, 8
> > > > > and 14 bytes. Overriding only fifosize and tx_loadsz would leave
> > > > > rx_trig_bytes reporting and selecting the wrong thresholds.
> > > > >
> > > > > The core already has get_rxtrig/set_rxtrig callbacks, though. I'll rework
> > > > > this to retain PORT_16550A, supply the H616 FIFO parameters during setup,
> > > > > and use those callbacks for the correct trigger mapping.
> > > >
> > > > Please consider just taking the FIFO size from the DT. On some chips
> > > > not every UART has the same size FIFO, so we already need to deal with
> > > > it.
> > > >
> > > > Also, FIFO size is a standard UART DT property, though somehow not included
> > > > for dw-uart.
> > >
> > > FWIW, it supports all standard properties for UART.
> > > Check what uart_read_port_properties() does.
> >
> > Ah, that's nice. I hacked up something [1] in the past to get it to
> > work, but that was probably before you added the helper. I never got
> > around to splitting up the commits and sending it out.
>
> Hmm... The helper is in the kernel for the past 2.5 years.
> Maybe you rebased it and it lost original dates?

I had been carrying the changes locally without committing them. I only
committed them recently to back them up remotely. So yeah, the dates are
not accurate.

> But if it's not working, we need to address that, I believe it should work out
> of the box, because the same function is used across dozen of serial drivers.

The new helper probably works fine. I can rework my code to use the values
given by the helpers and give it a try sometime next week.

> It might be an issue in 8250_dw.c, but I can't tell for sure.
>
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/wens/linux.git/commit/?id=e7e0741fa04f349ad9f94a4059e32467a30ef5d9
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART

[Andy Shevchenko]

On Sat, Oct 24, 2026 at 11:10:40PM +0900, Chen-Yu Tsai wrote:
> On Sat, Oct 24, 2026 at 10:51 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Sat, Oct 24, 2026 at 10:41:55PM +0900, Chen-Yu Tsai wrote:
> > > On Sat, Oct 24, 2026 at 10:19 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Sat, Oct 24, 2026 at 10:10:33PM +0900, Chen-Yu Tsai wrote:

...

> > > Ah, that's nice. I hacked up something [1] in the past to get it to
> > > work, but that was probably before you added the helper. I never got
> > > around to splitting up the commits and sending it out.
> >
> > Hmm... The helper is in the kernel for the past 2.5 years.
> > Maybe you rebased it and it lost original dates?
> 
> I had been carrying the changes locally without committing them. I only
> committed them recently to back them up remotely. So yeah, the dates are
> not accurate.
> 
> > But if it's not working, we need to address that, I believe it should work out
> > of the box, because the same function is used across dozen of serial drivers.
> 
> The new helper probably works fine. I can rework my code to use the values
> given by the helpers and give it a try sometime next week.

Would be nice, but I assume there is no hurry.

> > It might be an issue in 8250_dw.c, but I can't tell for sure.
> >
> > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/wens/linux.git/commit/?id=e7e0741fa04f349ad9f94a4059e32467a30ef5d9

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART

[James Hilliard]

On Sat, Oct 24, 2026 at 8:10 AM Chen-Yu Tsai <wens@kernel.org> wrote:
>
> On Sat, Oct 24, 2026 at 10:51 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Sat, Oct 24, 2026 at 10:41:55PM +0900, Chen-Yu Tsai wrote:
> > > On Sat, Oct 24, 2026 at 10:19 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Sat, Oct 24, 2026 at 10:10:33PM +0900, Chen-Yu Tsai wrote:
> > > > > On Sat, Oct 24, 2026 at 12:12 AM James Hilliard
> > > > > <james.hilliard1@gmail.com> wrote:
> > > > > > On Fri, Oct 23, 2026 at 2:44 AM Andy Shevchenko
> > > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > > On Fri, Oct 23, 2026 at 01:33:50AM -0600, James Hilliard wrote:
> >
> > ...
> >
> > > > > > > >       /* Internal 8250 only */
> > > > > > > >       UART_PORT_AIROHA                = 124,
> > > > > > > >       UART_PORT_AIROHA_HS             = 125,
> > > > > > > > +     UART_PORT_DW_APB_256    = 126,
> > > > > > >
> > > > > > > Why? Besides the poorly chosen name, what prevents us from modifying the
> > > > > > > corresponding parameters at probe time by leaving the one of the existing
> > > > > > > types?
> > > > > >
> > > > > > The new type was primarily a way to reuse the core's FIFO configuration
> > > > > > table, including the RX trigger mapping. H616 uses thresholds of 1, 64,
> > > > > > 128 and 254 bytes, whereas PORT_16550A maps the same FCR bits to 1, 4, 8
> > > > > > and 14 bytes. Overriding only fifosize and tx_loadsz would leave
> > > > > > rx_trig_bytes reporting and selecting the wrong thresholds.
> > > > > >
> > > > > > The core already has get_rxtrig/set_rxtrig callbacks, though. I'll rework
> > > > > > this to retain PORT_16550A, supply the H616 FIFO parameters during setup,
> > > > > > and use those callbacks for the correct trigger mapping.
> > > > >
> > > > > Please consider just taking the FIFO size from the DT. On some chips
> > > > > not every UART has the same size FIFO, so we already need to deal with
> > > > > it.
> > > > >
> > > > > Also, FIFO size is a standard UART DT property, though somehow not included
> > > > > for dw-uart.
> > > >
> > > > FWIW, it supports all standard properties for UART.
> > > > Check what uart_read_port_properties() does.
> > >
> > > Ah, that's nice. I hacked up something [1] in the past to get it to
> > > work, but that was probably before you added the helper. I never got
> > > around to splitting up the commits and sending it out.
> >
> > Hmm... The helper is in the kernel for the past 2.5 years.
> > Maybe you rebased it and it lost original dates?
>
> I had been carrying the changes locally without committing them. I only
> committed them recently to back them up remotely. So yeah, the dates are
> not accurate.
>
> > But if it's not working, we need to address that, I believe it should work out
> > of the box, because the same function is used across dozen of serial drivers.
>
> The new helper probably works fine. I can rework my code to use the values
> given by the helpers and give it a try sometime next week.

I've incorporated that approach into v3[0]. The H616 DTS supplies
fifo-size = <256>, and the driver uses the value populated by
uart_read_port_properties() for the transmit load and receive-trigger
mapping.

[0] https://lore.kernel.org/all/20261024-submit-h616-uart-fifo-v1-v3-4-286fc1b8c3c1@gmail.com/T/

>
> > It might be an issue in 8250_dw.c, but I can't tell for sure.
> >
> > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/wens/linux.git/commit/?id=e7e0741fa04f349ad9f94a4059e32467a30ef5d9
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
> >