From: Denis Mukhin <dmukhin@ford.com>
Added missing definitions needed for NS16550 UART emulator.
Newly introduced MSR definitions re-used in the existing ns16550 driver.
Also, corrected FCR DMA definition bit#3 (0x08) as per:
https://www.ti.com/lit/ds/symlink/tl16c550c.pdf
See "7.7.2 FIFO Control Register (FCR)".
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v6:
- used raw bitmasks instead of BIT() for consistency
---
xen/drivers/char/ns16550.c | 16 ++++++++--------
xen/include/xen/8250-uart.h | 36 ++++++++++++++++++++++++++++++++++--
2 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index df7fff7f81df..0e80fadbb894 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -388,7 +388,7 @@ static void __init cf_check ns16550_init_preirq(struct serial_port *port)
/* Check this really is a 16550+. Otherwise we have no FIFOs. */
if ( uart->fifo_size <= 1 &&
- ((ns_read_reg(uart, UART_IIR) & 0xc0) == 0xc0) &&
+ ((ns_read_reg(uart, UART_IIR) & UART_IIR_FE) == UART_IIR_FE) &&
((ns_read_reg(uart, UART_FCR) & UART_FCR_TRG14) == UART_FCR_TRG14) )
uart->fifo_size = 16;
}
@@ -728,20 +728,20 @@ static int __init check_existence(struct ns16550 *uart)
* 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);
- scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
+ scratch2 = ns_read_reg(uart, UART_IER) & UART_IER_MASK;
+ ns_write_reg(uart, UART_IER, UART_IER_MASK);
+ scratch3 = ns_read_reg(uart, UART_IER) & UART_IER_MASK;
ns_write_reg(uart, UART_IER, scratch);
- if ( (scratch2 != 0) || (scratch3 != 0x0F) )
+ if ( (scratch2 != 0) || (scratch3 != UART_IER_MASK) )
return 0;
/*
* Check to see if a UART is really there.
* Use loopback test mode.
*/
- ns_write_reg(uart, UART_MCR, UART_MCR_LOOP | 0x0A);
- status = ns_read_reg(uart, UART_MSR) & 0xF0;
- return (status == 0x90);
+ ns_write_reg(uart, UART_MCR, UART_MCR_LOOP | UART_MCR_RTS | UART_MCR_OUT2);
+ status = ns_read_reg(uart, UART_MSR) & UART_MSR_STATUS;
+ return (status == (UART_MSR_CTS | UART_MSR_DCD));
}
#ifdef CONFIG_HAS_PCI
diff --git a/xen/include/xen/8250-uart.h b/xen/include/xen/8250-uart.h
index d13352940c13..a8a26b64689e 100644
--- 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)
/* Interrupt Identification Register */
#define UART_IIR_NOINT 0x01 /* no interrupt pending */
@@ -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 */
+#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)
+
#define UART_FCR_TRG1 0x00 /* Rx FIFO trig lev 1 */
#define UART_FCR_TRG4 0x40 /* Rx FIFO trig lev 4 */
#define UART_FCR_TRG8 0x80 /* Rx FIFO trig lev 8 */
@@ -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)
+
+/* 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)
/* Line Status Register */
#define UART_LSR_DR 0x01 /* Data ready */
@@ -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)
/* These parity settings can be ORed directly into the LCR. */
#define UART_PARITY_NONE (0<<3)
--
2.51.0On 08.09.2025 23:11, dmukhin@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
On Tue, Sep 09, 2025 at 12:05:41PM +0200, Jan Beulich wrote: > On 08.09.2025 23:11, dmukhin@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). Yeah, I am not sure there's a "standard" spec for NS16550. > > The source I'm looking at says something entirely different. Maybe we're > better off simply omitting this #define? All TL16Cx50 I have mentioned, including Synopsys uart databook, say FCR's "DMA mode select" is Bit 3. And Linux'es driver defines it as 0x08 (include/uapi/linux/serial_reg.h) > > > +#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
© 2016 - 2025 Red Hat, Inc.