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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 10/12/26 2:50 PM, Jan Beulich wrote:
> On 12.10.2026 11:20, Oleksii Kurochko wrote:
> > 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.
> Are you suggesting that Linux'es XScale detection is flawed then, too?
I looked at it quickly and not flawed as such, but Linux uses that test 
only as a last-resort heuristic, which isn't how it would be used here.
In Linux the bit 6 test sits at the end of autoconfig_16550a(). That
only runs for ports whose type isn't known up front (legacy/PnP/PCI
autoprobing), and only with CONFIG_SERIAL_8250_16550A_VARIANTS or
UPF_FULL_PROBE. It also comes after the EFR probes, so ST16C65x /
TL16C75x style parts are classified and return before reaching it.
Even so, the comment there says "this is probably one of the Xscale
platform's internal UARTs", and a part whose EFR already has ECB set
would fall through the EFR check and be reported as XScale. So yes,
the same corner case exists there.

Where the type is known, Linux doesn't probe at all: 8250_of maps
"intel,xscale-uart" to PORT_XSCALE and sets UPF_FIXED_TYPE, so
autoconfig() (where it is happening the probing in a similar to 
suggested above way) never runs for DT ports. 8250_exar sets 
UPF_FIXED_TYPE as well.
> > > @@ -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()
> Yes, the more that as per Linux'es driver source, bit 4 may also need keeping
> permanently set (not only for XScale, but apparently also for Tegra).
Good point.

~ Oleksii