[PATCH v3 0/2] xen/ns16550: preserve UART operation across firmware handover

[PATCH v3 0/2] xen/ns16550: preserve UART operation across firmware handover

[Zhang Zheng]

This series identifies XScale-compatible UARTs from device tree and
keeps UUE set when updating IER. Pending output is drained before the
FIFOs are reset during ns16550 initialization.

Changes in v3:
- Identify XScale UARTs through intel,xscale-uart instead of probing IER.
- Preserve UUE on IER writes and enable RTOIE with receive interrupts.
- Bound the TX drain wait using the XHCI console's raw-counter approach.
- Explain why the drain checks TEMT instead of uart->lsr_mask.

Tested on SpacemiT K3 with Baptiste's preceding A/D bit patches applied.

v2: https://lore.kernel.org/xen-devel/20261010031406.2057735-1-zhangzheng@iscas.ac.cn/
v1: https://lore.kernel.org/xen-devel/20260928023857.1030884-1-zhangzheng@iscas.ac.cn/

Zhang Zheng (2):
  xen/ns16550: identify XScale UARTs from device tree
  xen/ns16550: drain TX before reinitializing the UART

 xen/drivers/char/ns16550.c  | 44 ++++++++++++++++++++++++++++++-------
 xen/include/xen/8250-uart.h |  2 ++
 2 files changed, 38 insertions(+), 8 deletions(-)


base-commit: 7e020a4dd6ed30eb6c91a9abe2767034e2bb270a
-- 
2.54.0

[PATCH v3 1/2] xen/ns16550: identify XScale UARTs from device tree

[Zhang Zheng]

Identify XScale UARTs using intel,xscale-uart, since IER bit 6 has other
meanings on some UART variants. Keep UUE set when writing IER so the
UART stays enabled when switching to polling, and enable RTOIE along
with receive interrupts.

Signed-off-by: Zhang Zheng <zhangzheng@iscas.ac.cn>
---
 xen/drivers/char/ns16550.c  | 29 +++++++++++++++++++++--------
 xen/include/xen/8250-uart.h |  2 ++
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index be388c8e65..1f54155d67 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -64,6 +64,7 @@ static struct ns16550 {
     bool intr_works;
     bool force_polling;
     bool dw_usr_bsy;
+    bool xscale;
 #ifdef NS16550_PCI
     /* PCI card parameters. */
     bool pb_bdf_enable;     /* if =1, pb-bdf effective, port behind bridge */
@@ -168,6 +169,14 @@ static void ns_write_reg(const struct ns16550 *uart, unsigned int reg, u8 c)
     }
 }
 
+static void ns_write_ier(const struct ns16550 *uart, u8 ier)
+{
+    if ( uart->xscale )
+        ier |= UART_IER_UUE;
+
+    ns_write_reg(uart, UART_IER, ier);
+}
+
 static int ns16550_ioport_invalid(struct ns16550 *uart)
 {
     return ns_read_reg(uart, UART_IER) == 0xff;
@@ -218,7 +227,7 @@ static void cf_check ns16550_interrupt(int irq, void *dev_id)
 
             /* Disable interrupt generation on the device and arm the timer. */
             uart->force_polling = true;
-            ns_write_reg(uart, UART_IER, 0);
+            ns_write_ier(uart, 0);
             set_timer(&uart->timer, now + MILLISECS(uart->timeout_ms));
             printk(XENLOG_WARNING
                    "uart interrupt taking more than %ums, switched to polling\n",
@@ -350,7 +359,7 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
     lcr = (uart->data_bits - 5) | ((uart->stop_bits - 1) << 2) | uart->parity;
 
     /* No interrupts. */
-    ns_write_reg(uart, UART_IER, 0);
+    ns_write_ier(uart, 0);
 
     /* Handle the DesignWare 8250 'busy-detect' quirk. */
     handle_dw_usr_busy_quirk(uart);
@@ -446,7 +455,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
                      UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
 
         /* Enable receive interrupts. */
-        ns_write_reg(uart, UART_IER, UART_IER_ERDAI);
+        ns_write_ier(uart, UART_IER_ERDAI |
+                     (uart->xscale ? UART_IER_RTOIE : 0));
     }
 
     if ( uart->irq >= 0 )
@@ -654,7 +664,7 @@ static void cf_check ns16550_start_tx(struct serial_port *port)
 
     /* Unmask transmit holding register empty interrupt if currently masked. */
     if ( !(ier & UART_IER_ETHREI) )
-        ns_write_reg(uart, UART_IER, ier | UART_IER_ETHREI);
+        ns_write_ier(uart, ier | UART_IER_ETHREI);
 }
 
 static void cf_check ns16550_stop_tx(struct serial_port *port)
@@ -664,7 +674,7 @@ static void cf_check ns16550_stop_tx(struct serial_port *port)
 
     /* Mask off transmit holding register empty interrupt if currently unmasked. */
     if ( ier & UART_IER_ETHREI )
-        ns_write_reg(uart, UART_IER, ier & ~UART_IER_ETHREI);
+        ns_write_ier(uart, ier & ~UART_IER_ETHREI);
 }
 
 #ifdef CONFIG_ARM
@@ -749,16 +759,16 @@ static int __init check_existence(struct ns16550 *uart)
      * there's no point trying anything else.
      */
     scratch = ns_read_reg(uart, UART_IER);
-    ns_write_reg(uart, UART_IER, 0);
+    ns_write_ier(uart, 0);
 
     /*
      * Mask out IER[7:4] bits for test as some UARTs (e.g. TL
      * 16C754B) allow only to modify them if an EFR bit is set.
      */
     scratch2 = ns_read_reg(uart, UART_IER) & 0x0f;
-    ns_write_reg(uart,UART_IER, 0x0F);
+    ns_write_ier(uart, 0x0f);
     scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
-    ns_write_reg(uart, UART_IER, scratch);
+    ns_write_ier(uart, scratch);
     if ( (scratch2 != 0) || (scratch3 != 0x0F) )
         return 0;
 
@@ -2034,6 +2044,8 @@ static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
 
     uart->dw_usr_bsy = dt_device_is_compatible(dev, "snps,dw-apb-uart");
 
+    uart->xscale = dt_device_is_compatible(dev, "intel,xscale-uart");
+
 #ifdef CONFIG_ARM
     uart->vuart.base_addr = uart->io_base;
     uart->vuart.size = uart->io_size;
@@ -2055,6 +2067,7 @@ static const struct dt_device_match ns16550_dt_match[] __initconst =
     DT_MATCH_COMPATIBLE("ns16550"),
     DT_MATCH_COMPATIBLE("ns16550a"),
     DT_MATCH_COMPATIBLE("snps,dw-apb-uart"),
+    DT_MATCH_COMPATIBLE("intel,xscale-uart"),
     DT_MATCH_COMPATIBLE("brcm,bcm2835-aux-uart"),
     { /* sentinel */ },
 };
diff --git a/xen/include/xen/8250-uart.h b/xen/include/xen/8250-uart.h
index d13352940c..5709a6c91b 100644
--- a/xen/include/xen/8250-uart.h
+++ b/xen/include/xen/8250-uart.h
@@ -42,6 +42,8 @@
 #define UART_IER_ETHREI   0x02    /* tx reg. empty        */
 #define UART_IER_ELSI     0x04    /* rx line status       */
 #define UART_IER_EMSI     0x08    /* MODEM status         */
+#define UART_IER_RTOIE    0x10    /* Receive timeout interrupt enable (XScale) */
+#define UART_IER_UUE      0x40    /* UART unit enable (XScale) */
 
 /* Interrupt Identification Register */
 #define UART_IIR_NOINT    0x01    /* no interrupt pending */
-- 
2.54.0

Re: [PATCH v3 1/2] xen/ns16550: identify XScale UARTs from device tree

[Oleksii Kurochko]

On 10/20/26 5:45 AM, Zhang Zheng wrote:
> Identify XScale UARTs using intel,xscale-uart, since IER bit 6 has other
> meanings on some UART variants. Keep UUE set when writing IER so the
> UART stays enabled when switching to polling, and enable RTOIE along
> with receive interrupts.
> 
> Signed-off-by: Zhang Zheng <zhangzheng@iscas.ac.cn>
> ---
>   xen/drivers/char/ns16550.c  | 29 +++++++++++++++++++++--------
>   xen/include/xen/8250-uart.h |  2 ++
>   2 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index be388c8e65..1f54155d67 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -64,6 +64,7 @@ static struct ns16550 {
>       bool intr_works;
>       bool force_polling;
>       bool dw_usr_bsy;
> +    bool xscale;
>   #ifdef NS16550_PCI
>       /* PCI card parameters. */
>       bool pb_bdf_enable;     /* if =1, pb-bdf effective, port behind bridge */
> @@ -168,6 +169,14 @@ static void ns_write_reg(const struct ns16550 *uart, unsigned int reg, u8 c)
>       }
>   }
>   
> +static void ns_write_ier(const struct ns16550 *uart, u8 ier)

Nit: please use uint8_t in new code.

> +{
> +    if ( uart->xscale )
> +        ier |= UART_IER_UUE;
> +
> +    ns_write_reg(uart, UART_IER, ier);
> +}
> +

[...]

>   
>   #ifdef CONFIG_ARM
> @@ -749,16 +759,16 @@ static int __init check_existence(struct ns16550 *uart)
>        * there's no point trying anything else.
>        */
>       scratch = ns_read_reg(uart, UART_IER);
> -    ns_write_reg(uart, UART_IER, 0);
> +    ns_write_ier(uart, 0);
>   
>       /*
>        * Mask out IER[7:4] bits for test as some UARTs (e.g. TL
>        * 16C754B) allow only to modify them if an EFR bit is set.
>        */
>       scratch2 = ns_read_reg(uart, UART_IER) & 0x0f;
> -    ns_write_reg(uart,UART_IER, 0x0F);
> +    ns_write_ier(uart, 0x0f);
>       scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
> -    ns_write_reg(uart, UART_IER, scratch);
> +    ns_write_ier(uart, scratch);
>       if ( (scratch2 != 0) || (scratch3 != 0x0F) )
>           return 0;

check_existence() is only reached from ns16550_parse_port_config()
(i.e. never for a DT-described UART), and for MMIO UARTs it returns
early anyway, so uart->xscale is always false here. I don't mind the
conversion for consistency, but then please mention it in the
description, together with the 0x0F -> 0x0f / missing blank style
changes.


>   
> @@ -2034,6 +2044,8 @@ static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
>   
>       uart->dw_usr_bsy = dt_device_is_compatible(dev, "snps,dw-apb-uart");
>   
> +    uart->xscale = dt_device_is_compatible(dev, "intel,xscale-uart");
> +
>   #ifdef CONFIG_ARM
>       uart->vuart.base_addr = uart->io_base;
>       uart->vuart.size = uart->io_size;
> @@ -2055,6 +2067,7 @@ static const struct dt_device_match ns16550_dt_match[] __initconst =
>       DT_MATCH_COMPATIBLE("ns16550"),
>       DT_MATCH_COMPATIBLE("ns16550a"),
>       DT_MATCH_COMPATIBLE("snps,dw-apb-uart"),
> +    DT_MATCH_COMPATIBLE("intel,xscale-uart"),
>       DT_MATCH_COMPATIBLE("brcm,bcm2835-aux-uart"),
>       { /* sentinel */ },
>   };
> diff --git a/xen/include/xen/8250-uart.h b/xen/include/xen/8250-uart.h
> index d13352940c..5709a6c91b 100644
> --- a/xen/include/xen/8250-uart.h
> +++ b/xen/include/xen/8250-uart.h
> @@ -42,6 +42,8 @@
>   #define UART_IER_ETHREI   0x02    /* tx reg. empty        */
>   #define UART_IER_ELSI     0x04    /* rx line status       */
>   #define UART_IER_EMSI     0x08    /* MODEM status         */
> +#define UART_IER_RTOIE    0x10    /* Receive timeout interrupt enable (XScale) */

This line is 81 characters long. Perhaps something like
"/* rx timeout (XScale)  */" to also match the style of the
surrounding comments?

> +#define UART_IER_UUE      0x40    /* UART unit enable (XScale) */

A side question, not for this patch: Linux uses a 32-byte
FIFO for PORT_XSCALE 
(https://elixir.bootlin.com/linux/v7.3/source/drivers/tty/serial/8250/8250_port.c#L147), 
while here fifo_size will end up being detected as 16. Is that intentional?

The code change itself looks fine to me.

~ Oleksii

Re: [PATCH v3 1/2] xen/ns16550: identify XScale UARTs from device tree

[Jan Beulich]

On 20.10.2026 05:45, Zhang Zheng wrote:
> Identify XScale UARTs using intel,xscale-uart, since IER bit 6 has other
> meanings on some UART variants. Keep UUE set when writing IER so the
> UART stays enabled when switching to polling, and enable RTOIE along
> with receive interrupts.

The RTOIE part needs more than just mentioning; it may even want splitting
off into a separate patch. It's not clear to me why setting this bit is
entirely benign to existing code, i.e. in particular why the present
interrupt handler doesn't need any adjustment to cope with the new reason
that it may be invoked for.

Jan

[PATCH v3 2/2] xen/ns16550: drain TX before reinitializing the UART

[Zhang Zheng]

Drain pending output before resetting FIFOs. Bound the wait to avoid
hanging boot if the UART never reports empty.

Wait for TEMT (which already implies THRE on a 16550) to ensure
transmission has finished before reconfiguration. uart->lsr_mask normally
checks only THRE, which indicates room for more output.

Signed-off-by: Zhang Zheng <zhangzheng@iscas.ac.cn>
---
 xen/drivers/char/ns16550.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 1f54155d67..29f784280e 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -410,6 +410,12 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
 static void __init cf_check ns16550_init_preirq(struct serial_port *port)
 {
     struct ns16550 *uart = port->uart;
+    cycles_t start;
+    /*
+     * Too early for proper time calibration, assume a 5GHz CPU and let it wait
+     * about 1s, with still reasonable timeout on a slower CPU.
+     */
+    cycles_t timeout_ticks = 5UL * 1000 * 1000 * 1000;
 
 #ifdef CONFIG_HAS_IOPORTS
     /* I/O ports are distinguished by their size (16 bits). */
@@ -427,6 +433,15 @@ static void __init cf_check ns16550_init_preirq(struct serial_port *port)
 #endif
     }
 
+    /*
+     * Drain firmware/early-console output before changing line settings or
+     * resetting FIFOs. Wait for TEMT so the shift register is empty too.
+     */
+    start = get_cycles();
+    while ( !(ns_read_reg(uart, UART_LSR) & UART_LSR_TEMT) &&
+            get_cycles() - start < timeout_ticks )
+        cpu_relax();
+
     ns16550_setup_preirq(uart);
 
     /* Check this really is a 16550+. Otherwise we have no FIFOs. */
-- 
2.54.0

Re: [PATCH v3 2/2] xen/ns16550: drain TX before reinitializing the UART

[Oleksii Kurochko]

On 10/20/26 5:45 AM, Zhang Zheng wrote:
> Drain pending output before resetting FIFOs. Bound the wait to avoid
> hanging boot if the UART never reports empty.
> 
> Wait for TEMT (which already implies THRE on a 16550) to ensure
> transmission has finished before reconfiguration. uart->lsr_mask normally
> checks only THRE, which indicates room for more output.
> 
> Signed-off-by: Zhang Zheng <zhangzheng@iscas.ac.cn>
> ---
>   xen/drivers/char/ns16550.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index 1f54155d67..29f784280e 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -410,6 +410,12 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>   static void __init cf_check ns16550_init_preirq(struct serial_port *port)
>   {
>       struct ns16550 *uart = port->uart;
> +    cycles_t start;
> +    /*
> +     * Too early for proper time calibration, assume a 5GHz CPU and let it wait
> +     * about 1s, with still reasonable timeout on a slower CPU.
> +     */
> +    cycles_t timeout_ticks = 5UL * 1000 * 1000 * 1000;

Shouldn't we use here ULL? 5UL * 1000 * 1000 * 1000 > MAX_unsingned_long 
on 32-bit platforms, at least, for arm33 and riscv32 unsigned long is 
4-byte type?

Also, I think that the comment should be updated as get_cycles() doesn't 
count CPU cycles:
  - on RISC-V it reads the time CSR, which ticks at the timebase
    frequency. E.g. on SpacemiT K1 that's 24MHz, so the "about 1s"
    becomes ~208s; with QEMU virt's 10MHz it's ~500s.
  - on Arm it reads CNTPCT, again typically in the tens of MHz range,
    with similar results.

So it won't be 1s for that arch-s.

Also for Arm and RISC-V preinit_xen_time() runs before 
console_init_preirq(), so time is already usable at this point if I am 
not mistaken something so "Too early for proper time calibration" sounds 
incorrect.

>   
>   #ifdef CONFIG_HAS_IOPORTS
>       /* I/O ports are distinguished by their size (16 bits). */
> @@ -427,6 +433,15 @@ static void __init cf_check ns16550_init_preirq(struct serial_port *port)
>   #endif
>       }
>   
> +    /*
> +     * Drain firmware/early-console output before changing line settings or
> +     * resetting FIFOs. Wait for TEMT so the shift register is empty too.
> +     */
> +    start = get_cycles();
> +    while ( !(ns_read_reg(uart, UART_LSR) & UART_LSR_TEMT) &&

This is done before pci_serial_early_init() (called from
ns16550_setup_preirq()), so for a PCI UART (specified via
<port-bdf> / <bridge-bdf> or found via "pci") whose decoding hasn't
been enabled by firmware, the LSR read may still return all ones.
With TEMT set in that value, the wait is then simply skipped. That's
harmless, but perhaps worth a word in the description or a code
comment.


> +            get_cycles() - start < timeout_ticks )
> +        cpu_relax();
> +
>       ns16550_setup_preirq(uart);
>   
>       /* Check this really is a 16550+. Otherwise we have no FIFOs. */

~ Oleksii