[2/3] serial: 8250_dw: configure the Allwinner H616 UART
Commit Message
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(-)
Comments
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.
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;
@@ -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 */
@@ -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 },
@@ -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 */