[PATCH 0/2] serial: 8250_dw: Add ti,tda54-uart support

[PATCH 0/2] serial: 8250_dw: Add ti,tda54-uart support

[Moteen Shah]

Add the ti,tda54-uart compatible string to the DW APB UART binding,
and add a quirk to skip the RX_TIMEOUT dummy read on TDA54.

Reading an empty FIFO on the IP version used in TDA54 triggers a
data abort. The interrupt storm this dummy read was meant to avoid
no longer occurs on this version of the IP, so skip the read via a
new quirk.

Moteen Shah (2):
  dt-bindings: serial: snps-dw-apb-uart: Add ti,tda54-uart
  serial: 8250_dw: Add ti,tda54-uart quirk to skip empty FIFO read

 .../devicetree/bindings/serial/snps-dw-apb-uart.yaml     | 1 +
 drivers/tty/serial/8250/8250_dw.c                        | 9 ++++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH 1/2] dt-bindings: serial: snps-dw-apb-uart: Add ti,tda54-uart

[Moteen Shah]

The TDA54 UART is compatible with the existing DesignWare APB UART
binding. Add the ti,tda54-uart compatible string.

Signed-off-by: Udit Kumar <u-kumar1@ti.com>
Signed-off-by: Moteen Shah <m-shah@ti.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 49f51b002879..518c899c52d1 100644
--- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
@@ -78,6 +78,7 @@ properties:
               - starfive,jh7100-hsuart
               - starfive,jh7100-uart
               - starfive,jh7110-uart
+              - ti,tda54-uart
               - ultrarisc,dp1000-uart
           - const: snps,dw-apb-uart
       - const: snps,dw-apb-uart
-- 
2.34.1

[PATCH 2/2] serial: 8250_dw: Add ti,tda54-uart quirk to skip empty FIFO read

[Moteen Shah]

dw8250_handle_irq() does a bogus RX read on RX_TIMEOUT with no data
present, to avoid an interrupt storm. On the IP version used in
TDA54, that interrupt storm no longer occurs, but reading an empty
FIFO instead triggers a data abort.

Add a DW_UART_QUIRK_SKIP_RX_TIMEOUT_READ quirk for ti,tda54-uart to
skip that read and avoid the data abort.

Signed-off-by: Udit Kumar <u-kumar1@ti.com>
Signed-off-by: Moteen Shah <m-shah@ti.com>
---
 drivers/tty/serial/8250/8250_dw.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 5fba913f3301..a1ce00156ae2 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -51,6 +51,7 @@
 #define DW_UART_QUIRK_APMC0D08		BIT(4)
 #define DW_UART_QUIRK_CPR_VALUE		BIT(5)
 #define DW_UART_QUIRK_IER_KICK		BIT(6)
+#define DW_UART_QUIRK_SKIP_RX_TIMEOUT_READ	BIT(7)
 
 /*
  * Number of consecutive IIR_NO_INT interrupts required to trigger interrupt
@@ -436,7 +437,7 @@ static int dw8250_handle_irq(struct uart_port *p)
 	 * This problem has only been observed so far when not in DMA mode
 	 * so we limit the workaround only to non-DMA mode.
 	 */
-	if (!up->dma && rx_timeout) {
+	if (!(quirks & DW_UART_QUIRK_SKIP_RX_TIMEOUT_READ) && !up->dma && rx_timeout) {
 		status = serial_lsr_in(up);
 
 		if (!(status & (UART_LSR_DR | UART_LSR_BI)))
@@ -888,6 +889,11 @@ static const struct dw8250_platform_data dw8250_ultrarisc_dp1000_data = {
 	.quirks = DW_UART_QUIRK_CPR_VALUE,
 };
 
+static const struct dw8250_platform_data dw8250_tda54 = {
+	.usr_reg = DW_UART_USR,
+	.quirks = DW_UART_QUIRK_SKIP_RX_TIMEOUT_READ,
+};
+
 static const struct of_device_id dw8250_of_match[] = {
 	{ .compatible = "snps,dw-apb-uart", .data = &dw8250_dw_apb },
 	{ .compatible = "cavium,octeon-3860-uart", .data = &dw8250_octeon_3860_data },
@@ -895,6 +901,7 @@ static const struct of_device_id dw8250_of_match[] = {
 	{ .compatible = "renesas,rzn1-uart", .data = &dw8250_renesas_rzn1_data },
 	{ .compatible = "sophgo,sg2044-uart", .data = &dw8250_skip_set_rate_data },
 	{ .compatible = "starfive,jh7100-uart", .data = &dw8250_skip_set_rate_data },
+	{ .compatible = "ti,tda54-uart", .data = &dw8250_tda54 },
 	{ .compatible = "ultrarisc,dp1000-uart", .data = &dw8250_ultrarisc_dp1000_data },
 	{ /* Sentinel */ }
 };
-- 
2.34.1

Re: [PATCH 1/2] dt-bindings: serial: snps-dw-apb-uart: Add ti,tda54-uart

[sashiko-bot]

> The TDA54 UART is compatible with the existing DesignWare APB UART
> binding. Add the ti,tda54-uart compatible string.
> 
> Signed-off-by: Udit Kumar <u-kumar1@ti.com>
> Signed-off-by: Moteen Shah <m-shah@ti.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911092529.306082-1-m-shah@ti.com?part=1

Re: [PATCH 2/2] serial: 8250_dw: Add ti,tda54-uart quirk to skip empty FIFO read

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The quirk avoids reading an empty FIFO in the IRQ handler, but generic 8250 core code performs unconditional empty FIFO reads during port startup and shutdown, which will still trigger a data abort on this hardware.

Pre-existing issues:
- [High] pm_runtime_enable() is called after serial port registration, violating callback timing rules.
--

commit 31fb31567ebe4c6e4b9eb7b2826e35722f3e46f5
Author: Moteen Shah <m-shah@ti.com>
serial: 8250_dw: Add ti,tda54-uart quirk to skip empty FIFO read

This commit adds a quirk to the 8250_dw driver for the ti,tda54-uart. The
quirk skips an empty FIFO read during an RX_TIMEOUT interrupt condition. This
is intended to avoid triggering a data abort on the TDA54 IP, which crashes
when reading an empty FIFO.

> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index 51d026f20825a..f97cc3dfbab4a 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c

[ ... ]

> @@ -436,7 +437,7 @@ static int dw8250_handle_irq(struct uart_port *p)
>  	 * This problem has only been observed so far when not in DMA mode
>  	 * so we limit the workaround only to non-DMA mode.
>  	 */
> -	if (!up->dma && rx_timeout) {
> +	if (!(quirks & DW_UART_QUIRK_SKIP_RX_TIMEOUT_READ) && !up->dma && rx_timeout) {

[Severity: High]
The quirk avoids reading an empty FIFO in the IRQ handler, but will this
fully protect the hardware from data aborts?

The generic 8250 core code performs unconditional empty FIFO reads during
port startup and shutdown. For example, serial8250_clear_interrupts()
unconditionally reads UART_RX, and serial8250_do_shutdown() does the same
immediately after clearing FIFOs.

Won't opening or closing the TTY device still trigger a data abort on the
TDA54 IP due to these core empty FIFO reads?

>  		status = serial_lsr_in(up);
>  
>  		if (!(status & (UART_LSR_DR | UART_LSR_BI)))

[ ... ]

> @@ -888,6 +889,11 @@ static const struct dw8250_platform_data dw8250_ultrarisc_dp1000_data = {
>  	.quirks = DW_UART_QUIRK_CPR_VALUE,
>  };
>  
> +static const struct dw8250_platform_data dw8250_tda54 = {
> +	.usr_reg = DW_UART_USR,
> +	.quirks = DW_UART_QUIRK_SKIP_RX_TIMEOUT_READ,
> +};
> +
>  static const struct of_device_id dw8250_of_match[] = {
>  	{ .compatible = "snps,dw-apb-uart", .data = &dw8250_dw_apb },
>  	{ .compatible = "cavium,octeon-3860-uart", .data = &dw8250_octeon_3860_data },
> @@ -895,6 +901,7 @@ static const struct of_device_id dw8250_of_match[] = {
>  	{ .compatible = "renesas,rzn1-uart", .data = &dw8250_renesas_rzn1_data },
>  	{ .compatible = "sophgo,sg2044-uart", .data = &dw8250_skip_set_rate_data },
>  	{ .compatible = "starfive,jh7100-uart", .data = &dw8250_skip_set_rate_data },
> +	{ .compatible = "ti,tda54-uart", .data = &dw8250_tda54 },

[Severity: High]
This is a pre-existing issue, but does dw8250_probe() violate the runtime PM
callback timing rules, which could cause a boot deadlock when this new
compatible device is probed?

dw8250_probe() calls serial8250_register_8250_port(), which synchronously
invokes the dw8250_do_pm() callback. This happens before pm_runtime_enable()
is called at the end of the probe function:

drivers/tty/serial/8250/8250_dw.c:dw8250_probe() {
	...
	data->data.line = serial8250_register_8250_port(up);
	if (data->data.line < 0)
		return data->data.line;

	platform_set_drvdata(pdev, data);

	pm_runtime_enable(dev);
	...
}

Since dw8250_do_pm() uses pm_runtime_get_sync() before runtime PM is enabled,
could this cause a circular dependency and hung worker threads during device
registration?

>  	{ .compatible = "ultrarisc,dp1000-uart", .data = &dw8250_ultrarisc_dp1000_data },
>  	{ /* Sentinel */ }
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911092529.306082-1-m-shah@ti.com?part=2

Re: [PATCH 1/2] dt-bindings: serial: snps-dw-apb-uart: Add ti,tda54-uart

[Krzysztof Kozlowski]

On Fri, Sep 11, 2026 at 02:55:28PM +0530, Moteen Shah wrote:
> The TDA54 UART is compatible with the existing DesignWare APB UART
> binding. Add the ti,tda54-uart compatible string.
> 
> Signed-off-by: Udit Kumar <u-kumar1@ti.com>
> Signed-off-by: Moteen Shah <m-shah@ti.com>
> ---
>  Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof