Re: [PATCH v2 1/2] xen/ns16550: detect and preserve UART_IER_UUE

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 10/10/26 5:14 AM, Zhang Zheng wrote:
> XScale-compatible UARTs require UART_IER_UUE to remain set.  Probe the
> bit during setup and preserve it when enabling receive interrupts, so
> the xscale/PXA UART remains operational after ns16550 takes over.
>
> Signed-off-by: Zhang Zheng <zhangzheng@xxxxxxxxxxx>
> ---
>   xen/drivers/char/ns16550.c  | 12 +++++++++++-
>   xen/include/xen/8250-uart.h |  1 +
>   2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index cdda28879e..c174465059 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 */
> @@ -352,6 +353,14 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>       /* No interrupts. */
>       ns_write_reg(uart, UART_IER, 0);
>   
> +    uart->xscale = false;
> +    if ( !(ns_read_reg(uart, UART_IER) & UART_IER_UUE) )
> +    {
> +        ns_write_reg(uart, UART_IER, UART_IER_UUE);
> +        if ( ns_read_reg(uart, UART_IER) & UART_IER_UUE )
> +            uart->xscale = true;
> +    }
> +
This probe only checks whether IER bit 6 is writable, which isn't
specific to XScale. On UARTs with an EFR (e.g. ST16C650, TI TL16C752,
Exar XR17V35x) the upper IER bits become writable once EFR[4] (ECB) is
set, and bit 6 is then the RTS interrupt enable. If ECB is already set
when the probe runs, it would report XScale, and the driver would then
keep the RTS interrupt enabled.

Could the flag be derived from the compatible in ns16550_uart_dt_init()
instead, the same way dw_usr_bsy is?

    uart->xscale = dt_device_is_compatible(dev, "intel,xscale-uart");

That compatible would also need adding to ns16550_dt_match[]. The
SpacemiT K1 nodes, for example, have no "ns16550a" fallback.

>       /* 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_reg(uart, UART_IER,
> +                     UART_IER_ERDAI | (uart->xscale ? UART_IER_UUE : 0));
It seems like not all cases are covered. For example, 
ns16550_interrupt() writes 0 to IER when it switches to polling, which 
would turn off the UART on XScale. Am I missing something? If not then 
something like:
static void ns_write_ier(const struct ns16550 *uart, uint8_t ier)
{
    if ( uart->xscale )
        ier |= UART_IER_UUE;

    ns_write_reg(uart, UART_IER, ier);
}

would be needed, used in ns16550_setup_preirq(), ns16550_setup_postirq()
and ns16550_interrupt().

ns16550_start_tx()/ns16550_stop_tx() do a read-modify-write of IER, so
they read UUE back from the register and write it out again unchanged.
They therefore work correctly without the wrapper. They could be
switched to ns_write_ier() as well for consistency, but that isn't
strictly necessary and likely could be left in way it looks like now.

~ Oleksii

>       }
>   
>       if ( uart->irq >= 0 )
> diff --git a/xen/include/xen/8250-uart.h b/xen/include/xen/8250-uart.h
> index d13352940c..358d264698 100644
> --- a/xen/include/xen/8250-uart.h
> +++ b/xen/include/xen/8250-uart.h
> @@ -42,6 +42,7 @@
>   #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_UUE      0x40    /* UART unit enable (XScale/PXA) */
>   
>   /* Interrupt Identification Register */
>   #define UART_IIR_NOINT    0x01    /* no interrupt pending */