On 08.09.2025 23:11, dmuk...@xen.org wrote: > --- a/xen/include/xen/8250-uart.h > +++ b/xen/include/xen/8250-uart.h > @@ -32,6 +32,7 @@ > #define UART_MCR 0x04 /* Modem control */ > #define UART_LSR 0x05 /* line status */ > #define UART_MSR 0x06 /* Modem status */ > +#define UART_SCR 0x07 /* Scratch pad */ > #define UART_USR 0x1f /* Status register (DW) */ > #define UART_DLL 0x00 /* divisor latch (ls) (DLAB=1) */ > #define UART_DLM 0x01 /* divisor latch (ms) (DLAB=1) */ > @@ -42,6 +43,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_MASK \ > + (UART_IER_ERDAI | UART_IER_ETHREI | UART_IER_ELSI | UART_IER_EMSI)
Here, aiui, ..._MASK covers all known bits. No #define-s for reserved ones. > @@ -51,12 +54,19 @@ > #define UART_IIR_THR 0x02 /* - tx reg. empty */ > #define UART_IIR_MSI 0x00 /* - MODEM status */ > #define UART_IIR_BSY 0x07 /* - busy detect (DW) */ > +#define UART_IIR_FE 0xc0 /* FIFO enabled (2 bits) */ > > /* FIFO Control Register */ > #define UART_FCR_ENABLE 0x01 /* enable FIFO */ > #define UART_FCR_CLRX 0x02 /* clear Rx FIFO */ > #define UART_FCR_CLTX 0x04 /* clear Tx FIFO */ > -#define UART_FCR_DMA 0x10 /* enter DMA mode */ > +#define UART_FCR_DMA 0x08 /* enter DMA mode */ Question is whether we can actually use the source you indicate as reference. TL16C550C may already be too different from what a "standard" 16550 is (where admittedly it also looks unclear what "standard" would be, as I'm unaware of a "canonical" spec). The source I'm looking at says something entirely different. Maybe we're better off simply omitting this #define? > +#define UART_FCR_RSRVD0 0x10 /* reserved; always 0 */ > +#define UART_FCR_RSRVD1 0x20 /* reserved; always 0 */ > +#define UART_FCR_RTB0 0x40 /* receiver trigger bit #0 */ > +#define UART_FCR_RTB1 0x80 /* receiver trigger bit #1 */ > +#define UART_FCR_TRG_MASK (UART_FCR_RTB0 | UART_FCR_RTB1) Continuing from the top comment - here, with the TRG infix, the scope is clear, too. > @@ -98,9 +108,30 @@ > /* Modem Control Register */ > #define UART_MCR_DTR 0x01 /* Data Terminal Ready */ > #define UART_MCR_RTS 0x02 /* Request to Send */ > -#define UART_MCR_OUT2 0x08 /* OUT2: interrupt mask */ > +#define UART_MCR_OUT1 0x04 /* Output #1 */ > +#define UART_MCR_OUT2 0x08 /* Output #2 */ > #define UART_MCR_LOOP 0x10 /* Enable loopback test mode */ > +#define UART_MCR_RSRVD0 0x20 /* Reserved #0 */ > #define UART_MCR_TCRTLR 0x40 /* Access TCR/TLR (TI16C752, EFR[4]=1) */ > +#define UART_MCR_RSRVD1 0x80 /* Reserved #1 */ > +#define UART_MCR_MASK \ > + (UART_MCR_DTR | UART_MCR_RTS | \ > + UART_MCR_OUT1 | UART_MCR_OUT2 | \ > + UART_MCR_LOOP | UART_MCR_TCRTLR) Here it's again all non-reserved bits. Yet why do we need #define-s for the two reserved ones here? (Same question for FCR, even if there's no UART_FCR_MASK.) > +/* Modem Status Register */ > +#define UART_MSR_DCTS 0x01 /* Change in CTS */ > +#define UART_MSR_DDSR 0x02 /* Change in DSR */ > +#define UART_MSR_TERI 0x04 /* Change in RI */ > +#define UART_MSR_DDCD 0x08 /* Change in DCD */ > +#define UART_MSR_CTS 0x10 > +#define UART_MSR_DSR 0x20 > +#define UART_MSR_RI 0x40 > +#define UART_MSR_DCD 0x80 > +#define UART_MSR_CHANGE \ > + (UART_MSR_DCTS | UART_MSR_DDSR | UART_MSR_TERI | UART_MSR_DDCD) > +#define UART_MSR_STATUS \ > + (UART_MSR_CTS | UART_MSR_DSR | UART_MSR_RI | UART_MSR_DCD) Here it's properly two subsets. > @@ -111,6 +142,7 @@ > #define UART_LSR_THRE 0x20 /* Xmit hold reg empty */ > #define UART_LSR_TEMT 0x40 /* Xmitter empty */ > #define UART_LSR_ERR 0x80 /* Error */ > +#define UART_LSR_MASK (UART_LSR_OE | UART_LSR_BI) But what's the deal here? Why would only two of the bits be covered? Jan
