* [PATCH 0/3] serial: 8250_dw: describe the Allwinner H616 UART configuration
@ 2026-10-23 7:33 James Hilliard
2026-10-23 7:33 ` [PATCH 1/3] dt-bindings: serial: snps-dw-apb-uart: add Allwinner H616 James Hilliard
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: James Hilliard @ 2026-10-23 7:33 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andy Shevchenko, Ilpo Järvinen, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland
Cc: linux-kernel, linux-serial, devicetree, linux-arm-kernel,
linux-sunxi, 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>
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/3] dt-bindings: serial: snps-dw-apb-uart: add Allwinner H616 2026-10-23 7:33 [PATCH 0/3] serial: 8250_dw: describe the Allwinner H616 UART configuration James Hilliard @ 2026-10-23 7:33 ` James Hilliard 2026-10-23 7:33 ` [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART James Hilliard 2026-10-23 7:33 ` [PATCH 3/3] arm64: dts: allwinner: h616: identify the UART integration James Hilliard 2 siblings, 0 replies; 15+ messages in thread From: James Hilliard @ 2026-10-23 7:33 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andy Shevchenko, Ilpo Järvinen, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland Cc: linux-kernel, linux-serial, devicetree, linux-arm-kernel, linux-sunxi, 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART 2026-10-23 7:33 [PATCH 0/3] serial: 8250_dw: describe the Allwinner H616 UART configuration James Hilliard 2026-10-23 7:33 ` [PATCH 1/3] dt-bindings: serial: snps-dw-apb-uart: add Allwinner H616 James Hilliard @ 2026-10-23 7:33 ` James Hilliard 2026-10-23 8:23 ` sashiko-bot 2026-10-23 8:44 ` Andy Shevchenko 2026-10-23 7:33 ` [PATCH 3/3] arm64: dts: allwinner: h616: identify the UART integration James Hilliard 2 siblings, 2 replies; 15+ messages in thread From: James Hilliard @ 2026-10-23 7:33 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andy Shevchenko, Ilpo Järvinen, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland Cc: linux-kernel, linux-serial, devicetree, linux-arm-kernel, linux-sunxi, 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART 2026-10-23 7:33 ` [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART James Hilliard @ 2026-10-23 8:23 ` sashiko-bot 2026-10-23 8:44 ` Andy Shevchenko 1 sibling, 0 replies; 15+ messages in thread From: sashiko-bot @ 2026-10-23 8:23 UTC (permalink / raw) To: James Hilliard; +Cc: linux-serial, devicetree, linux-sunxi, robh, conor+dt 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART 2026-10-23 7:33 ` [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART James Hilliard 2026-10-23 8:23 ` sashiko-bot @ 2026-10-23 8:44 ` Andy Shevchenko 2026-10-23 15:12 ` James Hilliard 1 sibling, 1 reply; 15+ messages in thread From: Andy Shevchenko @ 2026-10-23 8:44 UTC (permalink / raw) To: James Hilliard Cc: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ilpo Järvinen, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, linux-kernel, linux-serial, devicetree, linux-arm-kernel, linux-sunxi 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART 2026-10-23 8:44 ` Andy Shevchenko @ 2026-10-23 15:12 ` James Hilliard 2026-10-24 13:10 ` Chen-Yu Tsai 0 siblings, 1 reply; 15+ messages in thread From: James Hilliard @ 2026-10-23 15:12 UTC (permalink / raw) To: Andy Shevchenko Cc: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ilpo Järvinen, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, linux-kernel, linux-serial, devicetree, linux-arm-kernel, linux-sunxi 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 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART 2026-10-23 15:12 ` James Hilliard @ 2026-10-24 13:10 ` Chen-Yu Tsai 2026-10-24 13:19 ` Andy Shevchenko 0 siblings, 1 reply; 15+ messages in thread From: Chen-Yu Tsai @ 2026-10-24 13:10 UTC (permalink / raw) To: James Hilliard Cc: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ilpo Järvinen, Jernej Skrabec, Samuel Holland, linux-kernel, linux-serial, devicetree, linux-arm-kernel, linux-sunxi 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 > > > > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART 2026-10-24 13:10 ` Chen-Yu Tsai @ 2026-10-24 13:19 ` Andy Shevchenko 2026-10-24 13:41 ` Chen-Yu Tsai 0 siblings, 1 reply; 15+ messages in thread From: Andy Shevchenko @ 2026-10-24 13:19 UTC (permalink / raw) To: Chen-Yu Tsai Cc: James Hilliard, Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ilpo Järvinen, Jernej Skrabec, Samuel Holland, linux-kernel, linux-serial, devicetree, linux-arm-kernel, linux-sunxi 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART 2026-10-24 13:19 ` Andy Shevchenko @ 2026-10-24 13:41 ` Chen-Yu Tsai 2026-10-24 13:51 ` Andy Shevchenko 0 siblings, 1 reply; 15+ messages in thread From: Chen-Yu Tsai @ 2026-10-24 13:41 UTC (permalink / raw) To: Andy Shevchenko, James Hilliard Cc: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ilpo Järvinen, Jernej Skrabec, Samuel Holland, linux-kernel, linux-serial, devicetree, linux-arm-kernel, linux-sunxi 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART 2026-10-24 13:41 ` Chen-Yu Tsai @ 2026-10-24 13:51 ` Andy Shevchenko 2026-10-24 14:10 ` Chen-Yu Tsai 0 siblings, 1 reply; 15+ messages in thread From: Andy Shevchenko @ 2026-10-24 13:51 UTC (permalink / raw) To: Chen-Yu Tsai Cc: James Hilliard, Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ilpo Järvinen, Jernej Skrabec, Samuel Holland, linux-kernel, linux-serial, devicetree, linux-arm-kernel, linux-sunxi 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART 2026-10-24 13:51 ` Andy Shevchenko @ 2026-10-24 14:10 ` Chen-Yu Tsai 2026-10-25 14:49 ` Andy Shevchenko 2026-10-25 17:38 ` James Hilliard 0 siblings, 2 replies; 15+ messages in thread From: Chen-Yu Tsai @ 2026-10-24 14:10 UTC (permalink / raw) To: Andy Shevchenko Cc: James Hilliard, Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ilpo Järvinen, Jernej Skrabec, Samuel Holland, linux-kernel, linux-serial, devicetree, linux-arm-kernel, linux-sunxi 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 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART 2026-10-24 14:10 ` Chen-Yu Tsai @ 2026-10-25 14:49 ` Andy Shevchenko 2026-10-25 17:38 ` James Hilliard 1 sibling, 0 replies; 15+ messages in thread From: Andy Shevchenko @ 2026-10-25 14:49 UTC (permalink / raw) To: Chen-Yu Tsai Cc: James Hilliard, Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ilpo Järvinen, Jernej Skrabec, Samuel Holland, linux-kernel, linux-serial, devicetree, linux-arm-kernel, linux-sunxi 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART 2026-10-24 14:10 ` Chen-Yu Tsai 2026-10-25 14:49 ` Andy Shevchenko @ 2026-10-25 17:38 ` James Hilliard 1 sibling, 0 replies; 15+ messages in thread From: James Hilliard @ 2026-10-25 17:38 UTC (permalink / raw) To: wens Cc: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ilpo Järvinen, Jernej Skrabec, Samuel Holland, linux-kernel, linux-serial, devicetree, linux-arm-kernel, linux-sunxi 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 > > > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/3] arm64: dts: allwinner: h616: identify the UART integration 2026-10-23 7:33 [PATCH 0/3] serial: 8250_dw: describe the Allwinner H616 UART configuration James Hilliard 2026-10-23 7:33 ` [PATCH 1/3] dt-bindings: serial: snps-dw-apb-uart: add Allwinner H616 James Hilliard 2026-10-23 7:33 ` [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART James Hilliard @ 2026-10-23 7:33 ` James Hilliard 2026-10-23 8:32 ` sashiko-bot 2 siblings, 1 reply; 15+ messages in thread From: James Hilliard @ 2026-10-23 7:33 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andy Shevchenko, Ilpo Järvinen, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland Cc: linux-kernel, linux-serial, devicetree, linux-arm-kernel, linux-sunxi, 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] arm64: dts: allwinner: h616: identify the UART integration 2026-10-23 7:33 ` [PATCH 3/3] arm64: dts: allwinner: h616: identify the UART integration James Hilliard @ 2026-10-23 8:32 ` sashiko-bot 0 siblings, 0 replies; 15+ messages in thread From: sashiko-bot @ 2026-10-23 8:32 UTC (permalink / raw) To: James Hilliard; +Cc: linux-serial, devicetree, linux-sunxi, robh, conor+dt 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-10-25 17:38 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-23 7:33 [PATCH 0/3] serial: 8250_dw: describe the Allwinner H616 UART configuration James Hilliard 2026-10-23 7:33 ` [PATCH 1/3] dt-bindings: serial: snps-dw-apb-uart: add Allwinner H616 James Hilliard 2026-10-23 7:33 ` [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART James Hilliard 2026-10-23 8:23 ` sashiko-bot 2026-10-23 8:44 ` Andy Shevchenko 2026-10-23 15:12 ` James Hilliard 2026-10-24 13:10 ` Chen-Yu Tsai 2026-10-24 13:19 ` Andy Shevchenko 2026-10-24 13:41 ` Chen-Yu Tsai 2026-10-24 13:51 ` Andy Shevchenko 2026-10-24 14:10 ` Chen-Yu Tsai 2026-10-25 14:49 ` Andy Shevchenko 2026-10-25 17:38 ` James Hilliard 2026-10-23 7:33 ` [PATCH 3/3] arm64: dts: allwinner: h616: identify the UART integration James Hilliard 2026-10-23 8:32 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox