thr3ads.net - Xen devel - [PATCH 0/4] Support 8250 compatible UART for OMAP5432 [Jul 2013]

If this information is useful, please help other people find it:
Share via:

Chen Baozi

2013-Jul-19 12:57 UTC

[PATCH 0/4] Support 8250 compatible UART for OMAP5432

This patchset enables UART support for OMAP5432 ES2.0 devboard.

Chen Baozi (4):
  xen: extract register definitions from ns16550 into a separated header
  xen/arm: add OMAP5432 UART support for early_printk
  xen: set the right flags when enabling interrupts for 8250
  xen/arm: enable ns16550 uart driver for OMAP5432

 docs/misc/arm/early-printk.txt    |   1 +
 xen/arch/arm/Rules.mk             |   5 +
 xen/arch/arm/arm32/debug-8250.inc |  48 ++++++
 xen/drivers/char/ns16550.c        | 312 ++++++++++++++++++++++----------------
 xen/include/asm-arm/io.h          |  49 ++++++
 xen/include/xen/8250.h            | 104 +++++++++++++
 xen/include/xen/serial.h          |   7 +-
 7 files changed, 393 insertions(+), 133 deletions(-)
 create mode 100644 xen/arch/arm/arm32/debug-8250.inc
 create mode 100644 xen/include/xen/8250.h

-- 
1.8.1.4

Chen Baozi

2013-Jul-19 12:57 UTC

head link

[PATCH 1/4] xen: extract register definitions from ns16550 into a separated header

Since both UART driver codes on Allwinner A31, OMAP5 and x86 would use
these definitions, we refactor the codes into a separated header to avoid
unnecessary duplication.

Signed-off-by: Chen Baozi <baozich@gmail.com>
---
 xen/drivers/char/ns16550.c | 155 +++++++++++++--------------------------------
 xen/include/xen/8250.h     | 104 ++++++++++++++++++++++++++++++
 2 files changed, 148 insertions(+), 111 deletions(-)
 create mode 100644 xen/include/xen/8250.h

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index e0c87bb..9248330 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -19,6 +19,7 @@
 #include <xen/iocap.h>
 #include <xen/pci.h>
 #include <xen/pci_regs.h>
+#include <xen/8250.h>
 #include <asm/io.h>
 #ifdef CONFIG_X86
 #include <asm/fixmap.h>
@@ -58,76 +59,6 @@ static struct ns16550 {
     u8 bar_idx;
 } ns16550_com[2] = { { 0 } };
 
-/* Register offsets */
-#define RBR             0x00    /* receive buffer       */
-#define THR             0x00    /* transmit holding     */
-#define IER             0x01    /* interrupt enable     */
-#define IIR             0x02    /* interrupt identity   */
-#define FCR             0x02    /* FIFO control         */
-#define LCR             0x03    /* line control         */
-#define MCR             0x04    /* Modem control        */
-#define LSR             0x05    /* line status          */
-#define MSR             0x06    /* Modem status         */
-#define DLL             0x00    /* divisor latch (ls) (DLAB=1) */
-#define DLM             0x01    /* divisor latch (ms) (DLAB=1) */
-
-/* Interrupt Enable Register */
-#define IER_ERDAI       0x01    /* rx data recv''d       */
-#define IER_ETHREI      0x02    /* tx reg. empty        */
-#define IER_ELSI        0x04    /* rx line status       */
-#define IER_EMSI        0x08    /* MODEM status         */
-
-/* Interrupt Identification Register */
-#define IIR_NOINT       0x01    /* no interrupt pending */
-#define IIR_IMASK       0x06    /* interrupt identity:  */
-#define IIR_LSI         0x06    /*  - rx line status    */
-#define IIR_RDAI        0x04    /*  - rx data recv''d    */
-#define IIR_THREI       0x02    /*  - tx reg. empty     */
-#define IIR_MSI         0x00    /*  - MODEM status      */
-
-/* FIFO Control Register */
-#define FCR_ENABLE      0x01    /* enable FIFO          */
-#define FCR_CLRX        0x02    /* clear Rx FIFO        */
-#define FCR_CLTX        0x04    /* clear Tx FIFO        */
-#define FCR_DMA         0x10    /* enter DMA mode       */
-#define FCR_TRG1        0x00    /* Rx FIFO trig lev 1   */
-#define FCR_TRG4        0x40    /* Rx FIFO trig lev 4   */
-#define FCR_TRG8        0x80    /* Rx FIFO trig lev 8   */
-#define FCR_TRG14       0xc0    /* Rx FIFO trig lev 14  */
-
-/* Line Control Register */
-#define LCR_DLAB        0x80    /* Divisor Latch Access */
-
-/* Modem Control Register */
-#define MCR_DTR         0x01    /* Data Terminal Ready  */
-#define MCR_RTS         0x02    /* Request to Send      */
-#define MCR_OUT2        0x08    /* OUT2: interrupt mask */
-#define MCR_LOOP        0x10    /* Enable loopback test mode */
-
-/* Line Status Register */
-#define LSR_DR          0x01    /* Data ready           */
-#define LSR_OE          0x02    /* Overrun              */
-#define LSR_PE          0x04    /* Parity error         */
-#define LSR_FE          0x08    /* Framing error        */
-#define LSR_BI          0x10    /* Break                */
-#define LSR_THRE        0x20    /* Xmit hold reg empty  */
-#define LSR_TEMT        0x40    /* Xmitter empty        */
-#define LSR_ERR         0x80    /* Error                */
-
-/* These parity settings can be ORed directly into the LCR. */
-#define PARITY_NONE     (0<<3)
-#define PARITY_ODD      (1<<3)
-#define PARITY_EVEN     (3<<3)
-#define PARITY_MARK     (5<<3)
-#define PARITY_SPACE    (7<<3)
-
-/* Frequency of external clock source. This definition assumes PC platform. */
-#define UART_CLOCK_HZ   1843200
-
-/* Resume retry settings */
-#define RESUME_DELAY    MILLISECS(10)
-#define RESUME_RETRIES  100
-
 static char ns_read_reg(struct ns16550 *uart, int reg)
 {
     if ( uart->remapped_io_base == NULL )
@@ -150,12 +81,12 @@ static void ns16550_interrupt(
 
     uart->intr_works = 1;
 
-    while ( !(ns_read_reg(uart, IIR) & IIR_NOINT) )
+    while ( !(ns_read_reg(uart, UART_IIR) & UART_IIR_NOINT) )
     {
-        char lsr = ns_read_reg(uart, LSR);
-        if ( lsr & LSR_THRE )
+        char lsr = ns_read_reg(uart, UART_LSR);
+        if ( lsr & UART_LSR_THRE )
             serial_tx_interrupt(port, regs);
-        if ( lsr & LSR_DR )
+        if ( lsr & UART_LSR_DR )
             serial_rx_interrupt(port, regs);
     }
 }
@@ -171,10 +102,10 @@ static void __ns16550_poll(struct cpu_user_regs *regs)
     if ( uart->intr_works )
         return; /* Interrupts work - no more polling */
 
-    while ( ns_read_reg(uart, LSR) & LSR_DR )
+    while ( ns_read_reg(uart, UART_LSR) & UART_LSR_DR )
         serial_rx_interrupt(port, regs);
 
-    if ( ns_read_reg(uart, LSR) & LSR_THRE )
+    if ( ns_read_reg(uart, UART_LSR) & UART_LSR_THRE )
         serial_tx_interrupt(port, regs);
 
     set_timer(&uart->timer, NOW() + MILLISECS(uart->timeout_ms));
@@ -194,23 +125,23 @@ static unsigned int ns16550_tx_ready(struct serial_port
*port)
 {
     struct ns16550 *uart = port->uart;
 
-    return ns_read_reg(uart, LSR) & LSR_THRE ? uart->fifo_size : 0;
+    return ns_read_reg(uart, UART_LSR) & UART_LSR_THRE ? uart->fifo_size
: 0;
 }
 
 static void ns16550_putc(struct serial_port *port, char c)
 {
     struct ns16550 *uart = port->uart;
-    ns_write_reg(uart, THR, c);
+    ns_write_reg(uart, UART_THR, c);
 }
 
 static int ns16550_getc(struct serial_port *port, char *pc)
 {
     struct ns16550 *uart = port->uart;
 
-    if ( !(ns_read_reg(uart, LSR) & LSR_DR) )
+    if ( !(ns_read_reg(uart, UART_LSR) & UART_LSR_DR) )
         return 0;
 
-    *pc = ns_read_reg(uart, RBR);
+    *pc = ns_read_reg(uart, UART_RBR);
     return 1;
 }
 
@@ -244,31 +175,32 @@ 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, IER, 0);
+    ns_write_reg(uart, UART_IER, 0);
 
     /* Line control and baud-rate generator. */
-    ns_write_reg(uart, LCR, lcr | LCR_DLAB);
+    ns_write_reg(uart, UART_LCR, lcr | UART_LCR_DLAB);
     if ( uart->baud != BAUD_AUTO )
     {
         /* Baud rate specified: program it into the divisor latch. */
         divisor = uart->clock_hz / (uart->baud << 4);
-        ns_write_reg(uart, DLL, (char)divisor);
-        ns_write_reg(uart, DLM, (char)(divisor >> 8));
+        ns_write_reg(uart, UART_DLL, (char)divisor);
+        ns_write_reg(uart, UART_DLM, (char)(divisor >> 8));
     }
     else
     {
         /* Baud rate already set: read it out from the divisor latch. */
-        divisor  = ns_read_reg(uart, DLL);
-        divisor |= ns_read_reg(uart, DLM) << 8;
+        divisor  = ns_read_reg(uart, UART_DLL);
+        divisor |= ns_read_reg(uart, UART_DLM) << 8;
         uart->baud = uart->clock_hz / (divisor << 4);
     }
-    ns_write_reg(uart, LCR, lcr);
+    ns_write_reg(uart, UART_LCR, lcr);
 
     /* No flow ctrl: DTR and RTS are both wedged high to keep remote happy. */
-    ns_write_reg(uart, MCR, MCR_DTR | MCR_RTS);
+    ns_write_reg(uart, UART_MCR, UART_MCR_DTR | UART_MCR_RTS);
 
     /* Enable and clear the FIFOs. Set a large trigger threshold. */
-    ns_write_reg(uart, FCR, FCR_ENABLE | FCR_CLRX | FCR_CLTX | FCR_TRG14);
+    ns_write_reg(uart, UART_FCR,
+                 UART_FCR_ENABLE | UART_FCR_CLRX | UART_FCR_CLTX |
UART_FCR_TRG14);
 }
 
 static void __init ns16550_init_preirq(struct serial_port *port)
@@ -292,8 +224,8 @@ static void __init ns16550_init_preirq(struct serial_port
*port)
     ns16550_setup_preirq(uart);
 
     /* Check this really is a 16550+. Otherwise we have no FIFOs. */
-    if ( ((ns_read_reg(uart, IIR) & 0xc0) == 0xc0) &&
-         ((ns_read_reg(uart, FCR) & FCR_TRG14) == FCR_TRG14) )
+    if ( ((ns_read_reg(uart, UART_IIR) & 0xc0) == 0xc0) &&
+         ((ns_read_reg(uart, UART_FCR) & UART_FCR_TRG14) == UART_FCR_TRG14)
)
         uart->fifo_size = 16;
 }
 
@@ -302,10 +234,11 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
     if ( uart->irq > 0 )
     {
         /* Master interrupt enable; also keep DTR/RTS asserted. */
-        ns_write_reg(uart, MCR, MCR_OUT2 | MCR_DTR | MCR_RTS);
+        ns_write_reg(uart,
+                     UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
 
         /* Enable receive and transmit interrupts. */
-        ns_write_reg(uart, IER, IER_ERDAI | IER_ETHREI);
+        ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ETHREI);
     }
 
     if ( uart->irq >= 0 )
@@ -374,11 +307,11 @@ static void _ns16550_resume(struct serial_port *port)
 
 static int ns16550_ioport_invalid(struct ns16550 *uart)
 {
-    return ((((unsigned char)ns_read_reg(uart, LSR)) == 0xff) &&
-            (((unsigned char)ns_read_reg(uart, MCR)) == 0xff) &&
-            (((unsigned char)ns_read_reg(uart, IER)) == 0xff) &&
-            (((unsigned char)ns_read_reg(uart, IIR)) == 0xff) &&
-            (((unsigned char)ns_read_reg(uart, LCR)) == 0xff));
+    return ((((unsigned char)ns_read_reg(uart, UART_LSR)) == 0xff) &&
+            (((unsigned char)ns_read_reg(uart, UART_MCR)) == 0xff) &&
+            (((unsigned char)ns_read_reg(uart, UART_IER)) == 0xff) &&
+            (((unsigned char)ns_read_reg(uart, UART_IIR)) == 0xff) &&
+            (((unsigned char)ns_read_reg(uart, UART_LCR)) == 0xff));
 }
 
 static int delayed_resume_tries;
@@ -457,15 +390,15 @@ static int __init parse_parity_char(int c)
     switch ( c )
     {
     case ''n'':
-        return PARITY_NONE;
+        return UART_PARITY_NONE;
     case ''o'': 
-        return PARITY_ODD;
+        return UART_PARITY_ODD;
     case ''e'': 
-        return PARITY_EVEN;
+        return UART_PARITY_EVEN;
     case ''m'': 
-        return PARITY_MARK;
+        return UART_PARITY_MARK;
     case ''s'': 
-        return PARITY_SPACE;
+        return UART_PARITY_SPACE;
     }
     return 0;
 }
@@ -500,17 +433,17 @@ static int __init check_existence(struct ns16550 *uart)
      * Do a simple existence test first; if we fail this,
      * there''s no point trying anything else.
      */
-    scratch = ns_read_reg(uart, IER);
-    ns_write_reg(uart, IER, 0);
+    scratch = ns_read_reg(uart, UART_IER);
+    ns_write_reg(uart, UART_IER, 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, IER) & 0x0f;
-    ns_write_reg(uart, IER, 0x0F);
-    scratch3 = ns_read_reg(uart, IER) & 0x0f;
-    ns_write_reg(uart, IER, scratch);
+    scratch2 = ns_read_reg(uart, UART_IER) & 0x0f;
+    ns_write_reg(uart,UART_IER, 0x0F);
+    scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
+    ns_write_reg(uart, UART_IER, scratch);
     if ( (scratch2 != 0) || (scratch3 != 0x0F) )
         return 0;
 
@@ -518,8 +451,8 @@ static int __init check_existence(struct ns16550 *uart)
      * Check to see if a UART is really there.
      * Use loopback test mode.
      */
-    ns_write_reg(uart, MCR, MCR_LOOP | 0x0A);
-    status = ns_read_reg(uart, MSR) & 0xF0;
+    ns_write_reg(uart, UART_MCR, UART_MCR_LOOP | 0x0A);
+    status = ns_read_reg(uart, UART_MSR) & 0xF0;
     return (status == 0x90);
 }
 
diff --git a/xen/include/xen/8250.h b/xen/include/xen/8250.h
new file mode 100644
index 0000000..84f35bc
--- /dev/null
+++ b/xen/include/xen/8250.h
@@ -0,0 +1,104 @@
+/*
+ * xen/include/xen/8250.h
+ *
+ * This header is extracted from driver/char/ns16550.c
+ *
+ * Common constant definition between early printk and the UART driver
+ * for the 16550-series UART
+ *
+ * Copyright (c) 2003-2005, K A Fraser
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __XEN_NS16550_H__
+#define __XEN_NS16550_H__
+
+/* Register offsets */
+#define UART_RBR          0x00    /* receive buffer       */
+#define UART_THR          0x00    /* transmit holding     */
+#define UART_IER          0x01    /* interrupt enable     */
+#define UART_IIR          0x02    /* interrupt identity   */
+#define UART_FCR          0x02    /* FIFO control         */
+#define UART_LCR          0x03    /* line control         */
+#define UART_MCR          0x04    /* Modem control        */
+#define UART_LSR          0x05    /* line status          */
+#define UART_MSR          0x06    /* Modem status         */
+#define UART_DLL          0x00    /* divisor latch (ls) (DLAB=1) */
+#define UART_DLM          0x01    /* divisor latch (ms) (DLAB=1) */
+
+/* Interrupt Enable Register */
+#define UART_IER_ERDAI    0x01    /* rx data recv''d       */
+#define UART_IER_ETHREI   0x02    /* tx reg. empty        */
+#define UART_IER_ELSI     0x04    /* rx line status       */
+#define UART_IER_EMSI     0x08    /* MODEM status         */
+
+/* Interrupt Identificatiegister */
+#define UART_IIR_NOINT    0x01    /* no interrupt pending */
+#define UART_IIR_IMA      0x06    /* interrupt identity:  */
+#define UART_IIR_LSI      0x06    /*  - rx line status    */
+#define UART_IIR_RDA      0x04    /*  - rx data recv''d    */
+#define UART_IIR_THR      0x02    /*  - tx reg. empty     */
+#define UART_IIR_MSI      0x00    /*  - MODEM status      */
+
+/* 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_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   */
+#define UART_FCR_TRG14    0xc0    /* Rx FIFO trig lev 14  */
+
+/* Line Control Register */
+#define UART_LCR_DLAB     0x80    /* Divisor Latch Access */
+
+/* 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_LOOP     0x10    /* Enable loopback test mode */
+
+/* Line Status Register */
+#define UART_LSR_DR       0x01    /* Data ready           */
+#define UART_LSR_OE       0x02    /* Overrun              */
+#define UART_LSR_PE       0x04    /* Parity error         */
+#define UART_LSR_FE       0x08    /* Framing error        */
+#define UART_LSR_BI       0x10    /* Break                */
+#define UART_LSR_THRE     0x20    /* Xmit hold reg empty  */
+#define UART_LSR_TEMT     0x40    /* Xmitter empty        */
+#define UART_LSR_ERR      0x80    /* Error                */
+
+/* These parity settings can be ORed directly into the LCR. */
+#define UART_PARITY_NONE  (0<<3)
+#define UART_PARITY_ODD   (1<<3)
+#define UART_PARITY_EVEN  (3<<3)
+#define UART_PARITY_MARK  (5<<3)
+#define UART_PARITY_SPACE (7<<3)
+
+/* Frequency of external clock source. This definition assumes PC platform. */
+#define UART_CLOCK_HZ     1843200
+
+/* Resume retry settings */
+#define RESUME_DELAY      MILLISECS(10)
+#define RESUME_RETRIES    100
+
+#endif /* __XEN_NS16550_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.8.1.4

Chen Baozi

2013-Jul-19 12:57 UTC

head link

[PATCH 2/4] xen/arm: add OMAP5432 UART support for early_printk

Signed-off-by: Chen Baozi <baozich@gmail.com>
---
 docs/misc/arm/early-printk.txt    |  1 +
 xen/arch/arm/Rules.mk             |  4 ++++
 xen/arch/arm/arm32/debug-8250.inc | 48 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+)
 create mode 100644 xen/arch/arm/arm32/debug-8250.inc

diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt
index fbc3208..874488f 100644
--- a/docs/misc/arm/early-printk.txt
+++ b/docs/misc/arm/early-printk.txt
@@ -13,6 +13,7 @@ where mach is the name of the machine:
   - exynos5250: printk with the second UART
   - midway: printk with the pl011 on Calxeda Midway processors
   - fastmodel: printk on ARM Fastmodel software emulators
+  - omap5432: printk with UART3 on TI OMAP5432 processors
 
 The base address and baud rate is hardcoded in xen/arch/arm/Rules.mk,
 see there when adding support for new machines.
diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index 422ed04..eaa03fb 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -64,6 +64,10 @@ EARLY_PRINTK_INC := pl011
 EARLY_PRINTK_BAUD := 115200
 EARLY_UART_BASE_ADDRESS := 0xfff36000
 endif
+ifeq ($(CONFIG_EARLY_PRINTK), omap5432)
+EARLY_PRINTK_INC := 8250
+EARLY_UART_BASE_ADDRESS := 0x48020000
+endif
 
 ifneq ($(EARLY_PRINTK_INC),)
 EARLY_PRINTK := y
diff --git a/xen/arch/arm/arm32/debug-8250.inc
b/xen/arch/arm/arm32/debug-8250.inc
new file mode 100644
index 0000000..cbb63ca
--- /dev/null
+++ b/xen/arch/arm/arm32/debug-8250.inc
@@ -0,0 +1,48 @@
+/*
+ * xen/arch/arm/arm32/debug-8250.inc
+ *
+ * 8250 specific debug code
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <xen/8250.h>
+
+/* OMAP UART initialization
+ * rb: register which contains the UART base address
+ * rc: scratch register 1
+ * rd: scratch register 2 */
+.macro early_uart_init rb rc rd
+.endm
+
+/* OMAP UART wait UART to be ready to transmit
+ * rb: register which contains the UART base address
+ * rc: scratch register */
+.macro early_uart_ready rb rc
+1:
+	ldr	\rc, [\rb, #(UART_LSR<<2)] /* Read line status register */
+	tst	\rc, #UART_LSR_THRE   /* Check Xmit holding register flag */
+	beq	1b		      /* Wait for the UART to be ready */
+.endm
+
+/* OMAP UART transmit character
+ * rb: register which contains the UART base address
+ * rt: register which contains the character to transmit */
+.macro early_uart_transmit rb rt
+        str   \rt, [\rb, #UART_THR]      /* Write Transmit buffer */
+.endm
+
+/*
+ * Local variables:
+ * mode: ASM
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.8.1.4

Chen Baozi

2013-Jul-19 12:57 UTC

head link

[PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

Previous bits setting would cause generating THRE interrupts, which cannot
be cleared and make system loop in gic_interrupt endlessly. Set
''received
data available'' and ''receiver line status'' bits of
IER when enabling
interrupts as what Linux 8250 driver does.

Signed-off-by: Chen Baozi <baozich@gmail.com>
---
 xen/drivers/char/ns16550.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 9248330..60512be 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -237,8 +237,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
         ns_write_reg(uart,
                      UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
 
-        /* Enable receive and transmit interrupts. */
-        ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ETHREI);
+        /* Enable receive and line status interrupts. */
+        ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ELSI);
     }
 
     if ( uart->irq >= 0 )
-- 
1.8.1.4

Chen Baozi

2013-Jul-19 12:57 UTC

head link

[PATCH 4/4] xen/arm: enable ns16550 uart driver for OMAP5432

Signed-off-by: Chen Baozi <baozich@gmail.com>
---
 xen/arch/arm/Rules.mk      |   1 +
 xen/drivers/char/ns16550.c | 155 +++++++++++++++++++++++++++++++++++++++------
 xen/include/asm-arm/io.h   |  49 ++++++++++++++
 xen/include/xen/serial.h   |   7 +-
 4 files changed, 191 insertions(+), 21 deletions(-)

diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index eaa03fb..8604e34 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -9,6 +9,7 @@
 HAS_DEVICE_TREE := y
 HAS_VIDEO := y
 HAS_ARM_HDLCD := y
+HAS_NS16550 := y
 
 CFLAGS += -fno-builtin -fno-common -Wredundant-decls
 CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 60512be..b8690eb 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -1,10 +1,10 @@
 /******************************************************************************
  * ns16550.c
- * 
+ *
  * Driver for 16550-series UARTs. This driver is to be kept within Xen as
  * it permits debugging of seriously-toasted machines (e.g., in situations
  * where a device driver within a guest OS would be inaccessible).
- * 
+ *
  * Copyright (c) 2003-2005, K A Fraser
  */
 
@@ -25,6 +25,13 @@
 #include <asm/fixmap.h>
 #endif
 
+#ifdef CONFIG_ARM
+#include <xen/device_tree.h>
+#include <xen/errno.h>
+#include <asm/early_printk.h>
+#include <asm/device.h>
+#endif
+
 /*
  * Configure serial port with a string:
  *  
<baud>[/<clock_hz>][,DPS[,<io-base>[,<irq>[,<port-bdf>[,<bridge-bdf>]]]]].
@@ -39,8 +46,11 @@ string_param("com1", opt_com1);
 string_param("com2", opt_com2);
 
 static struct ns16550 {
-    int baud, clock_hz, data_bits, parity, stop_bits, fifo_size, irq;
-    unsigned long io_base;   /* I/O port or memory-mapped I/O address. */
+    int baud, data_bits, parity, stop_bits, fifo_size;
+    u32 clock_hz;
+    struct dt_irq irq;
+    u64 io_base;   /* I/O port or memory-mapped I/O address. */
+    u64 io_size;
     char __iomem *remapped_io_base;  /* Remapped virtual address of MMIO. */
     /* UART with IRQ line: interrupt-driven I/O. */
     struct irqaction irqaction;
@@ -61,16 +71,24 @@ static struct ns16550 {
 
 static char ns_read_reg(struct ns16550 *uart, int reg)
 {
+#ifdef CONFIG_X86
     if ( uart->remapped_io_base == NULL )
         return inb(uart->io_base + reg);
     return readb(uart->remapped_io_base + reg);
+#else
+    return readb(uart->remapped_io_base + (reg << 2));
+#endif
 }
 
 static void ns_write_reg(struct ns16550 *uart, int reg, char c)
 {
+#ifdef CONFIG_X86
     if ( uart->remapped_io_base == NULL )
         return outb(c, uart->io_base + reg);
     writeb(c, uart->remapped_io_base + reg);
+#else
+    writeb(c, uart->remapped_io_base + (reg << 2));
+#endif
 }
 
 static void ns16550_interrupt(
@@ -145,11 +163,12 @@ static int ns16550_getc(struct serial_port *port, char
*pc)
     return 1;
 }
 
+#ifdef CONFIG_X86
 static void pci_serial_early_init(struct ns16550 *uart)
 {
     if ( !uart->ps_bdf_enable || uart->io_base >= 0x10000 )
         return;
-    
+
     if ( uart->pb_bdf_enable )
         pci_conf_write16(0, uart->pb_bdf[0], uart->pb_bdf[1],
uart->pb_bdf[2],
                          PCI_IO_BASE,
@@ -161,7 +180,13 @@ static void pci_serial_early_init(struct ns16550 *uart)
                      uart->io_base | PCI_BASE_ADDRESS_SPACE_IO);
     pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1],
uart->ps_bdf[2],
                      PCI_COMMAND, PCI_COMMAND_IO);
+
+}
+#else
+static void pci_serial_early_init(struct ns16550 *uart)
+{
 }
+#endif
 
 static void ns16550_setup_preirq(struct ns16550 *uart)
 {
@@ -217,7 +242,9 @@ static void __init ns16550_init_preirq(struct serial_port
*port)
         uart->remapped_io_base = (void __iomem *)fix_to_virt(idx);
         uart->remapped_io_base += uart->io_base & ~PAGE_MASK;
 #else
-        uart->remapped_io_base = (char *)ioremap(uart->io_base, 8);
+        uart->remapped_io_base = ioremap_attr(uart->io_base,
+                                              uart->io_size,
+                                              PAGE_HYPERVISOR_NOCACHE);
 #endif
     }
 
@@ -231,7 +258,7 @@ static void __init ns16550_init_preirq(struct serial_port
*port)
 
 static void ns16550_setup_postirq(struct ns16550 *uart)
 {
-    if ( uart->irq > 0 )
+    if ( uart->irq.irq > 0 )
     {
         /* Master interrupt enable; also keep DTR/RTS asserted. */
         ns_write_reg(uart,
@@ -241,7 +268,7 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
         ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ELSI);
     }
 
-    if ( uart->irq >= 0 )
+    if ( uart->irq.irq >= 0 )
         set_timer(&uart->timer, NOW() + MILLISECS(uart->timeout_ms));
 }
 
@@ -250,7 +277,7 @@ static void __init ns16550_init_postirq(struct serial_port
*port)
     struct ns16550 *uart = port->uart;
     int rc, bits;
 
-    if ( uart->irq < 0 )
+    if ( uart->irq.irq < 0 )
         return;
 
     serial_async_transmit(port);
@@ -262,20 +289,26 @@ static void __init ns16550_init_postirq(struct serial_port
*port)
     uart->timeout_ms = max_t(
         unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
 
-    if ( uart->irq > 0 )
+    if ( uart->irq.irq > 0 )
     {
         uart->irqaction.handler = ns16550_interrupt;
         uart->irqaction.name    = "ns16550";
         uart->irqaction.dev_id  = port;
-        if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
-            printk("ERROR: Failed to allocate ns16550 IRQ %d\n",
uart->irq);
+#ifdef CONFIG_X86
+        if ( (rc = setup_irq(uart->irq.irq, &uart->irqaction)) != 0 )
+#else
+        if ( (rc = setup_dt_irq(&uart->irq, &uart->irqaction)) !=
0 )
+#endif
+            printk("ERROR: Failed to allocate ns16550 IRQ %d\n",
uart->irq.irq);
     }
 
     ns16550_setup_postirq(uart);
 
+#ifdef CONFIG_X86
     if ( uart->bar || uart->ps_bdf_enable )
         pci_hide_device(uart->ps_bdf[0], PCI_DEVFN(uart->ps_bdf[1],
                                                    uart->ps_bdf[2]));
+#endif
 }
 
 static void ns16550_suspend(struct serial_port *port)
@@ -284,13 +317,16 @@ static void ns16550_suspend(struct serial_port *port)
 
     stop_timer(&uart->timer);
 
+#ifdef CONFIG_X86
     if ( uart->bar )
        uart->cr = pci_conf_read16(0, uart->ps_bdf[0], uart->ps_bdf[1],
                                   uart->ps_bdf[2], PCI_COMMAND);
+#endif
 }
 
 static void _ns16550_resume(struct serial_port *port)
 {
+#ifdef CONFIG_X86
     struct ns16550 *uart = port->uart;
 
     if ( uart->bar )
@@ -300,6 +336,7 @@ static void _ns16550_resume(struct serial_port *port)
        pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1],
uart->ps_bdf[2],
                         PCI_COMMAND, uart->cr);
     }
+#endif
 
     ns16550_setup_preirq(port->uart);
     ns16550_setup_postirq(port->uart);
@@ -370,9 +407,17 @@ static void __init ns16550_endboot(struct serial_port
*port)
 static int __init ns16550_irq(struct serial_port *port)
 {
     struct ns16550 *uart = port->uart;
-    return ((uart->irq > 0) ? uart->irq : -1);
+    return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
 }
 
+#ifdef CONFIG_ARM
+static const struct dt_irq __init *ns16550_dt_irq(struct serial_port *port)
+{
+    struct ns16550 *uart = port->uart;
+    return &uart->irq;
+}
+#endif
+
 static struct uart_driver __read_mostly ns16550_driver = {
     .init_preirq  = ns16550_init_preirq,
     .init_postirq = ns16550_init_postirq,
@@ -382,7 +427,10 @@ static struct uart_driver __read_mostly ns16550_driver = {
     .tx_ready     = ns16550_tx_ready,
     .putc         = ns16550_putc,
     .getc         = ns16550_getc,
-    .irq          = ns16550_irq
+    .irq          = ns16550_irq,
+#ifdef CONFIG_ARM
+    .dt_irq_get   = ns16550_dt_irq
+#endif
 };
 
 static int __init parse_parity_char(int c)
@@ -403,6 +451,7 @@ static int __init parse_parity_char(int c)
     return 0;
 }
 
+#ifdef CONFIG_X86
 static void __init parse_pci_bdf(const char **conf, unsigned int bdf[3])
 {
     bdf[0] = simple_strtoul(*conf, conf, 16);
@@ -415,6 +464,7 @@ static void __init parse_pci_bdf(const char **conf, unsigned
int bdf[3])
     (*conf)++;
     bdf[2] = simple_strtoul(*conf, conf, 16);
 }
+#endif
 
 static int __init check_existence(struct ns16550 *uart)
 {
@@ -428,7 +478,7 @@ static int __init check_existence(struct ns16550 *uart)
         return 1;
 
     pci_serial_early_init(uart);
-    
+
     /*
      * Do a simple existence test first; if we fail this,
      * there''s no point trying anything else.
@@ -445,7 +495,7 @@ static int __init check_existence(struct ns16550 *uart)
     scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
     ns_write_reg(uart, UART_IER, scratch);
     if ( (scratch2 != 0) || (scratch3 != 0x0F) )
-        return 0;
+            return 0;
 
     /*
      * Check to see if a UART is really there.
@@ -456,6 +506,7 @@ static int __init check_existence(struct ns16550 *uart)
     return (status == 0x90);
 }
 
+#ifdef CONFIG_X86
 static int
 pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
 {
@@ -507,7 +558,7 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int
bar_idx)
                 uart->bar = bar;
                 uart->bar_idx = bar_idx;
                 uart->io_base = bar & ~PCI_BASE_ADDRESS_SPACE_IO;
-                uart->irq = pci_conf_read8(0, b, d, f, PCI_INTERRUPT_PIN) ?
+                uart->irq.irq = pci_conf_read8(0, b, d, f,
PCI_INTERRUPT_PIN) ?
                     pci_conf_read8(0, b, d, f, PCI_INTERRUPT_LINE) : 0;
 
                 return 0;
@@ -519,11 +570,12 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int
bar_idx)
         return -1;
 
     uart->io_base = 0x3f8;
-    uart->irq = 0;
+    uart->irq.irq = 0;
     uart->clock_hz  = UART_CLOCK_HZ;
 
     return 0;
 }
+#endif
 
 #define PARSE_ERR(_f, _a...)                 \
     do {                                     \
@@ -534,6 +586,7 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int
bar_idx)
 static void __init ns16550_parse_port_config(
     struct ns16550 *uart, const char *conf)
 {
+#ifdef CONFIG_X86
     int baud;
 
     /* No user-specified configuration? */
@@ -589,7 +642,7 @@ static void __init ns16550_parse_port_config(
     }
 
     if ( *conf == '','' && *++conf !=
'','' )
-        uart->irq = simple_strtol(conf, &conf, 10);
+        uart->irq.irq = simple_strtol(conf, &conf, 10);
 
     if ( *conf == '','' && *++conf !=
'','' )
     {
@@ -604,6 +657,8 @@ static void __init ns16550_parse_port_config(
     }
 
  config_parsed:
+#endif
+
     /* Sanity checks. */
     if ( (uart->baud != BAUD_AUTO) &&
          ((uart->baud < 1200) || (uart->baud > 115200)) )
@@ -633,18 +688,80 @@ void __init ns16550_init(int index, struct
ns16550_defaults *defaults)
     uart->baud      = (defaults->baud ? :
                        console_has((index == 0) ? "com1" :
"com2")
                        ? BAUD_AUTO : 0);
+#ifdef CONFIG_ARM
+    uart->clock_hz = defaults->clock_hz;
+#else
     uart->clock_hz  = UART_CLOCK_HZ;
+#endif
     uart->data_bits = defaults->data_bits;
     uart->parity    = parse_parity_char(defaults->parity);
     uart->stop_bits = defaults->stop_bits;
     uart->irq       = defaults->irq;
     uart->io_base   = defaults->io_base;
+#ifdef CONFIG_ARM
+    uart->io_size   = defaults->io_size;
+#endif
+
     /* Default is no transmit FIFO. */
     uart->fifo_size = 1;
 
     ns16550_parse_port_config(uart, (index == 0) ? opt_com1 : opt_com2);
+
 }
 
+#ifdef CONFIG_ARM
+
+static int __init ns16550_dt_init(struct dt_device_node *dev,
+                                  const void *data)
+{
+    struct ns16550_defaults uart;
+    int res;
+    const __be32 *clkspec;
+
+    uart.baud      = 115200;
+    uart.data_bits = 8;
+    uart.parity    = ''n'';
+    uart.stop_bits = 1;
+
+    res = dt_device_get_address(dev, 0, &uart.io_base, &uart.io_size);
+    if (res) {
+        early_printk("ns16550: Unable to retrieve the base"
+                     " address of the UART\n");
+        return res;
+    }
+
+    res = dt_device_get_irq(dev, 0, &uart.irq);
+    if (res) {
+        early_printk("ns16550: Unable to retrieve the IRQ\n");
+        return res;
+    }
+
+    clkspec = dt_get_property(dev, "clock-frequency", NULL);
+    if (clkspec == NULL) {
+        early_printk("ns16550: Unable to retrieve the clock
frequency\n");
+        return -EINVAL;
+    }
+    uart.clock_hz = be32_to_cpup(clkspec);
+
+    ns16550_init(0, &uart);
+    dt_device_set_used_by(dev, DOMID_XEN);
+
+    return 0;
+}
+
+static const char const *ns16550_dt_compat[] __initdata +{
+    "ti,omap4-uart",
+    NULL
+};
+
+DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
+    .compatible = ns16550_dt_compat,
+    .init = ns16550_dt_init,
+DT_DEVICE_END
+
+#endif /* CONFIG_ARM */
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-arm/io.h b/xen/include/asm-arm/io.h
index aea5233..33674bc 100644
--- a/xen/include/asm-arm/io.h
+++ b/xen/include/asm-arm/io.h
@@ -1,6 +1,55 @@
 #ifndef _ASM_IO_H
 #define _ASM_IO_H
 
+static inline void __raw_writeb(u8 val, volatile void __iomem *addr)
+{
+        asm volatile("strb %1, %0"
+                     : "+Qo" (*(volatile u8 __force *)addr)
+                     : "r" (val));
+}
+
+static inline void __raw_writel(u32 val, volatile void __iomem *addr)
+{
+        asm volatile("str %1, %0"
+                     : "+Qo" (*(volatile u32 __force *)addr)
+                     : "r" (val));
+}
+
+static inline u8 __raw_readb(const volatile void __iomem *addr)
+{
+        u8 val;
+        asm volatile("ldrb %1, %0"
+                     : "+Qo" (*(volatile u8 __force *)addr),
+                       "=r" (val));
+        return val;
+}
+
+static inline u32 __raw_readl(const volatile void __iomem *addr)
+{
+        u32 val;
+        asm volatile("ldr %1, %0"
+                     : "+Qo" (*(volatile u32 __force *)addr),
+                       "=r" (val));
+        return val;
+}
+
+#define __iormb()               rmb()
+#define __iowmb()               wmb()
+
+#define readb_relaxed(c) ({ u8  __r = __raw_readb(c); __r; })
+#define readl_relaxed(c) ({ u32 __r = le32_to_cpu((__force __le32) \
+                                        __raw_readl(c)); __r; })
+
+#define writeb_relaxed(v,c)     __raw_writeb(v,c)
+#define writel_relaxed(v,c)     __raw_writel((__force u32) cpu_to_le32(v),c)
+
+#define readb(c)                ({ u8  __v = readb_relaxed(c); __iormb(); __v;
})
+#define readl(c)                ({ u32 __v = readl_relaxed(c); __iormb(); __v;
})
+
+#define writeb(v,c)             ({ __iowmb(); writeb_relaxed(v,c); })
+#define writel(v,c)             ({ __iowmb(); writel_relaxed(v,c); })
+
+
 #endif
 /*
  * Local variables:
diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
index 9caf776..09ca855 100644
--- a/xen/include/xen/serial.h
+++ b/xen/include/xen/serial.h
@@ -11,6 +11,7 @@
 
 #include <xen/init.h>
 #include <xen/spinlock.h>
+#include <xen/device_tree.h>
 
 struct cpu_user_regs;
 
@@ -151,8 +152,10 @@ struct ns16550_defaults {
     int data_bits; /* default data bits (5, 6, 7 or 8) */
     int parity;    /* default parity (n, o, e, m or s) */
     int stop_bits; /* default stop bits (1 or 2) */
-    int irq;       /* default irq */
-    unsigned long io_base; /* default io_base address */
+    struct dt_irq irq;     /* default irq */
+    u64 io_base;   /* default io_base address */
+    u64 io_size;
+    u32 clock_hz;  /* UART clock rate */
 };
 void ns16550_init(int index, struct ns16550_defaults *defaults);
 void ehci_dbgp_init(void);
-- 
1.8.1.4

Ian Campbell

2013-Jul-19 14:15 UTC

head link

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

On Fri, 2013-07-19 at 20:57 +0800, Chen Baozi wrote:> Previous bits setting would cause generating THRE interrupts, which cannot
> be cleared and make system loop in gic_interrupt endlessly. Set
''received
> data available'' and ''receiver line status'' bits
of IER when enabling
> interrupts as what Linux 8250 driver does.
We do actually want THRE interrupts though, don''t we? Otherwise how do
we know when we can send more stuff?

Are you using console_sync? It might mask any issue arising from not
getting these interrupts (or maybe even cause your initial issue?)

The LSI register is only of use if we care about RTS/CTS or something. I
don''t know if we do, but I''d be surprised if just asking for
the
interrupt was sufficient (i.e. we''d need to do something with the irq)

How did you configure this interrupt line, level or edge trigger? I''d
have expected it to need to be edge, and the issue sounds a bit like it
might be level.

Ian.
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> ---
>  xen/drivers/char/ns16550.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index 9248330..60512be 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -237,8 +237,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
>          ns_write_reg(uart,
>                       UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR |
UART_MCR_RTS);
>  
> -        /* Enable receive and transmit interrupts. */
> -        ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ETHREI);
> +        /* Enable receive and line status interrupts. */
> +        ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ELSI);
>      }
>  
>      if ( uart->irq >= 0 )

Chen Baozi

2013-Jul-19 15:18 UTC

head link

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

On Fri, Jul 19, 2013 at 10:15 PM, Ian Campbell <Ian.Campbell@citrix.com>
wrote:>
> On Fri, 2013-07-19 at 20:57 +0800, Chen Baozi wrote:
> > Previous bits setting would cause generating THRE interrupts, which
cannot
> > be cleared and make system loop in gic_interrupt endlessly. Set
''received
> > data available'' and ''receiver line status''
bits of IER when enabling
> > interrupts as what Linux 8250 driver does.
>
> We do actually want THRE interrupts though, don''t we? Otherwise
how do
> we know when we can send more stuff?
I''m not very sure about it. In Linux 8250 driver THRE is not set at
this stage.
And following it does work for OMAP5. I''ll try to dig it next to see
whether this
fix is reasonable.
>
> Are you using console_sync? It might mask any issue arising from not
> getting these interrupts (or maybe even cause your initial issue?)
Yes. I have tried to shut this down to test whether it is the cause of  my
issue. But it seems not.
>
> The LSI register is only of use if we care about RTS/CTS or something. I
> don''t know if we do, but I''d be surprised if just asking
for the
> interrupt was sufficient (i.e. we''d need to do something with the
irq)
>
> How did you configure this interrupt line, level or edge trigger?
I''d
> have expected it to need to be edge, and the issue sounds a bit like it
> might be level.
It is high-level read from the DTS. I''ve also changed to it to edge
trigger
manually and every other possibilities when debugging. And it seems
edge trigger doesn''t help.

Anyway, some research must be done for this patch and my fixes to the
issue next.

Thanks.

Baozi
>
> Ian.
>
> > Signed-off-by: Chen Baozi <baozich@gmail.com>
> > ---
> >  xen/drivers/char/ns16550.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> > index 9248330..60512be 100644
> > --- a/xen/drivers/char/ns16550.c
> > +++ b/xen/drivers/char/ns16550.c
> > @@ -237,8 +237,8 @@ static void ns16550_setup_postirq(struct ns16550
*uart)
> >          ns_write_reg(uart,
> >                       UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR |
UART_MCR_RTS);
> >
> > -        /* Enable receive and transmit interrupts. */
> > -        ns_write_reg(uart, UART_IER, UART_IER_ERDAI |
UART_IER_ETHREI);
> > +        /* Enable receive and line status interrupts. */
> > +        ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ELSI);
> >      }
> >
> >      if ( uart->irq >= 0 )
>
>

Julien Grall

2013-Jul-19 15:35 UTC

head link

Re: [PATCH 1/4] xen: extract register definitions from ns16550 into a separated header

On 07/19/2013 01:57 PM, Chen Baozi wrote:> Since both UART driver codes on Allwinner A31, OMAP5 and x86 would use
> these definitions, we refactor the codes into a separated header to avoid
> unnecessary duplication.
> 
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> ---
>  xen/drivers/char/ns16550.c | 155
+++++++++++++--------------------------------
>  xen/include/xen/8250.h     | 104 ++++++++++++++++++++++++++++++
>  2 files changed, 148 insertions(+), 111 deletions(-)
>  create mode 100644 xen/include/xen/8250.h
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index e0c87bb..9248330 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -19,6 +19,7 @@
>  #include <xen/iocap.h>
>  #include <xen/pci.h>
>  #include <xen/pci_regs.h>
> +#include <xen/8250.h>
>  #include <asm/io.h>
>  #ifdef CONFIG_X86
>  #include <asm/fixmap.h>
> @@ -58,76 +59,6 @@ static struct ns16550 {
>      u8 bar_idx;
>  } ns16550_com[2] = { { 0 } };
>  
> -/* Register offsets */
> -#define RBR             0x00    /* receive buffer       */
> -#define THR             0x00    /* transmit holding     */
> -#define IER             0x01    /* interrupt enable     */
> -#define IIR             0x02    /* interrupt identity   */
> -#define FCR             0x02    /* FIFO control         */
> -#define LCR             0x03    /* line control         */
> -#define MCR             0x04    /* Modem control        */
> -#define LSR             0x05    /* line status          */
> -#define MSR             0x06    /* Modem status         */
> -#define DLL             0x00    /* divisor latch (ls) (DLAB=1) */
> -#define DLM             0x01    /* divisor latch (ms) (DLAB=1) */
> -
> -/* Interrupt Enable Register */
> -#define IER_ERDAI       0x01    /* rx data recv''d       */
> -#define IER_ETHREI      0x02    /* tx reg. empty        */
> -#define IER_ELSI        0x04    /* rx line status       */
> -#define IER_EMSI        0x08    /* MODEM status         */
> -
> -/* Interrupt Identification Register */
> -#define IIR_NOINT       0x01    /* no interrupt pending */
> -#define IIR_IMASK       0x06    /* interrupt identity:  */
> -#define IIR_LSI         0x06    /*  - rx line status    */
> -#define IIR_RDAI        0x04    /*  - rx data recv''d    */
> -#define IIR_THREI       0x02    /*  - tx reg. empty     */
> -#define IIR_MSI         0x00    /*  - MODEM status      */
> -
> -/* FIFO Control Register */
> -#define FCR_ENABLE      0x01    /* enable FIFO          */
> -#define FCR_CLRX        0x02    /* clear Rx FIFO        */
> -#define FCR_CLTX        0x04    /* clear Tx FIFO        */
> -#define FCR_DMA         0x10    /* enter DMA mode       */
> -#define FCR_TRG1        0x00    /* Rx FIFO trig lev 1   */
> -#define FCR_TRG4        0x40    /* Rx FIFO trig lev 4   */
> -#define FCR_TRG8        0x80    /* Rx FIFO trig lev 8   */
> -#define FCR_TRG14       0xc0    /* Rx FIFO trig lev 14  */
> -
> -/* Line Control Register */
> -#define LCR_DLAB        0x80    /* Divisor Latch Access */
> -
> -/* Modem Control Register */
> -#define MCR_DTR         0x01    /* Data Terminal Ready  */
> -#define MCR_RTS         0x02    /* Request to Send      */
> -#define MCR_OUT2        0x08    /* OUT2: interrupt mask */
> -#define MCR_LOOP        0x10    /* Enable loopback test mode */
> -
> -/* Line Status Register */
> -#define LSR_DR          0x01    /* Data ready           */
> -#define LSR_OE          0x02    /* Overrun              */
> -#define LSR_PE          0x04    /* Parity error         */
> -#define LSR_FE          0x08    /* Framing error        */
> -#define LSR_BI          0x10    /* Break                */
> -#define LSR_THRE        0x20    /* Xmit hold reg empty  */
> -#define LSR_TEMT        0x40    /* Xmitter empty        */
> -#define LSR_ERR         0x80    /* Error                */
> -
> -/* These parity settings can be ORed directly into the LCR. */
> -#define PARITY_NONE     (0<<3)
> -#define PARITY_ODD      (1<<3)
> -#define PARITY_EVEN     (3<<3)
> -#define PARITY_MARK     (5<<3)
> -#define PARITY_SPACE    (7<<3)
> -
> -/* Frequency of external clock source. This definition assumes PC
platform. */
> -#define UART_CLOCK_HZ   1843200
> -
> -/* Resume retry settings */
> -#define RESUME_DELAY    MILLISECS(10)
> -#define RESUME_RETRIES  100
> -
>  static char ns_read_reg(struct ns16550 *uart, int reg)
>  {
>      if ( uart->remapped_io_base == NULL )
> @@ -150,12 +81,12 @@ static void ns16550_interrupt(
>  
>      uart->intr_works = 1;
>  
> -    while ( !(ns_read_reg(uart, IIR) & IIR_NOINT) )
> +    while ( !(ns_read_reg(uart, UART_IIR) & UART_IIR_NOINT) )
>      {
> -        char lsr = ns_read_reg(uart, LSR);
> -        if ( lsr & LSR_THRE )
> +        char lsr = ns_read_reg(uart, UART_LSR);
> +        if ( lsr & UART_LSR_THRE )
>              serial_tx_interrupt(port, regs);
> -        if ( lsr & LSR_DR )
> +        if ( lsr & UART_LSR_DR )
>              serial_rx_interrupt(port, regs);
>      }
>  }
> @@ -171,10 +102,10 @@ static void __ns16550_poll(struct cpu_user_regs
*regs)
>      if ( uart->intr_works )
>          return; /* Interrupts work - no more polling */
>  
> -    while ( ns_read_reg(uart, LSR) & LSR_DR )
> +    while ( ns_read_reg(uart, UART_LSR) & UART_LSR_DR )
>          serial_rx_interrupt(port, regs);
>  
> -    if ( ns_read_reg(uart, LSR) & LSR_THRE )
> +    if ( ns_read_reg(uart, UART_LSR) & UART_LSR_THRE )
>          serial_tx_interrupt(port, regs);
>  
>      set_timer(&uart->timer, NOW() +
MILLISECS(uart->timeout_ms));
> @@ -194,23 +125,23 @@ static unsigned int ns16550_tx_ready(struct
serial_port *port)
>  {
>      struct ns16550 *uart = port->uart;
>  
> -    return ns_read_reg(uart, LSR) & LSR_THRE ? uart->fifo_size : 0;
> +    return ns_read_reg(uart, UART_LSR) & UART_LSR_THRE ?
uart->fifo_size : 0;
>  }
>  
>  static void ns16550_putc(struct serial_port *port, char c)
>  {
>      struct ns16550 *uart = port->uart;
> -    ns_write_reg(uart, THR, c);
> +    ns_write_reg(uart, UART_THR, c);
>  }
>  
>  static int ns16550_getc(struct serial_port *port, char *pc)
>  {
>      struct ns16550 *uart = port->uart;
>  
> -    if ( !(ns_read_reg(uart, LSR) & LSR_DR) )
> +    if ( !(ns_read_reg(uart, UART_LSR) & UART_LSR_DR) )
>          return 0;
>  
> -    *pc = ns_read_reg(uart, RBR);
> +    *pc = ns_read_reg(uart, UART_RBR);
>      return 1;
>  }
>  
> @@ -244,31 +175,32 @@ 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, IER, 0);
> +    ns_write_reg(uart, UART_IER, 0);
>  
>      /* Line control and baud-rate generator. */
> -    ns_write_reg(uart, LCR, lcr | LCR_DLAB);
> +    ns_write_reg(uart, UART_LCR, lcr | UART_LCR_DLAB);
>      if ( uart->baud != BAUD_AUTO )
>      {
>          /* Baud rate specified: program it into the divisor latch. */
>          divisor = uart->clock_hz / (uart->baud << 4);
> -        ns_write_reg(uart, DLL, (char)divisor);
> -        ns_write_reg(uart, DLM, (char)(divisor >> 8));
> +        ns_write_reg(uart, UART_DLL, (char)divisor);
> +        ns_write_reg(uart, UART_DLM, (char)(divisor >> 8));
>      }
>      else
>      {
>          /* Baud rate already set: read it out from the divisor latch. */
> -        divisor  = ns_read_reg(uart, DLL);
> -        divisor |= ns_read_reg(uart, DLM) << 8;
> +        divisor  = ns_read_reg(uart, UART_DLL);
> +        divisor |= ns_read_reg(uart, UART_DLM) << 8;
>          uart->baud = uart->clock_hz / (divisor << 4);
>      }
> -    ns_write_reg(uart, LCR, lcr);
> +    ns_write_reg(uart, UART_LCR, lcr);
>  
>      /* No flow ctrl: DTR and RTS are both wedged high to keep remote
happy. */
> -    ns_write_reg(uart, MCR, MCR_DTR | MCR_RTS);
> +    ns_write_reg(uart, UART_MCR, UART_MCR_DTR | UART_MCR_RTS);
>  
>      /* Enable and clear the FIFOs. Set a large trigger threshold. */
> -    ns_write_reg(uart, FCR, FCR_ENABLE | FCR_CLRX | FCR_CLTX | FCR_TRG14);
> +    ns_write_reg(uart, UART_FCR,
> +                 UART_FCR_ENABLE | UART_FCR_CLRX | UART_FCR_CLTX |
UART_FCR_TRG14);
>  }
>  
>  static void __init ns16550_init_preirq(struct serial_port *port)
> @@ -292,8 +224,8 @@ static void __init ns16550_init_preirq(struct
serial_port *port)
>      ns16550_setup_preirq(uart);
>  
>      /* Check this really is a 16550+. Otherwise we have no FIFOs. */
> -    if ( ((ns_read_reg(uart, IIR) & 0xc0) == 0xc0) &&
> -         ((ns_read_reg(uart, FCR) & FCR_TRG14) == FCR_TRG14) )
> +    if ( ((ns_read_reg(uart, UART_IIR) & 0xc0) == 0xc0) &&
> +         ((ns_read_reg(uart, UART_FCR) & UART_FCR_TRG14) ==
UART_FCR_TRG14) )
>          uart->fifo_size = 16;
>  }
>  
> @@ -302,10 +234,11 @@ static void ns16550_setup_postirq(struct ns16550
*uart)
>      if ( uart->irq > 0 )
>      {
>          /* Master interrupt enable; also keep DTR/RTS asserted. */
> -        ns_write_reg(uart, MCR, MCR_OUT2 | MCR_DTR | MCR_RTS);
> +        ns_write_reg(uart,
> +                     UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR |
UART_MCR_RTS);
>  
>          /* Enable receive and transmit interrupts. */
> -        ns_write_reg(uart, IER, IER_ERDAI | IER_ETHREI);
> +        ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ETHREI);
>      }
>  
>      if ( uart->irq >= 0 )
> @@ -374,11 +307,11 @@ static void _ns16550_resume(struct serial_port *port)
>  
>  static int ns16550_ioport_invalid(struct ns16550 *uart)
>  {
> -    return ((((unsigned char)ns_read_reg(uart, LSR)) == 0xff) &&
> -            (((unsigned char)ns_read_reg(uart, MCR)) == 0xff) &&
> -            (((unsigned char)ns_read_reg(uart, IER)) == 0xff) &&
> -            (((unsigned char)ns_read_reg(uart, IIR)) == 0xff) &&
> -            (((unsigned char)ns_read_reg(uart, LCR)) == 0xff));
> +    return ((((unsigned char)ns_read_reg(uart, UART_LSR)) == 0xff)
&&
> +            (((unsigned char)ns_read_reg(uart, UART_MCR)) == 0xff)
&&
> +            (((unsigned char)ns_read_reg(uart, UART_IER)) == 0xff)
&&
> +            (((unsigned char)ns_read_reg(uart, UART_IIR)) == 0xff)
&&
> +            (((unsigned char)ns_read_reg(uart, UART_LCR)) == 0xff));
>  }
>  
>  static int delayed_resume_tries;
> @@ -457,15 +390,15 @@ static int __init parse_parity_char(int c)
>      switch ( c )
>      {
>      case ''n'':
> -        return PARITY_NONE;
> +        return UART_PARITY_NONE;
>      case ''o'': 
> -        return PARITY_ODD;
> +        return UART_PARITY_ODD;
>      case ''e'': 
> -        return PARITY_EVEN;
> +        return UART_PARITY_EVEN;
>      case ''m'': 
> -        return PARITY_MARK;
> +        return UART_PARITY_MARK;
>      case ''s'': 
> -        return PARITY_SPACE;
> +        return UART_PARITY_SPACE;
>      }
>      return 0;
>  }
> @@ -500,17 +433,17 @@ static int __init check_existence(struct ns16550
*uart)
>       * Do a simple existence test first; if we fail this,
>       * there''s no point trying anything else.
>       */
> -    scratch = ns_read_reg(uart, IER);
> -    ns_write_reg(uart, IER, 0);
> +    scratch = ns_read_reg(uart, UART_IER);
> +    ns_write_reg(uart, UART_IER, 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, IER) & 0x0f;
> -    ns_write_reg(uart, IER, 0x0F);
> -    scratch3 = ns_read_reg(uart, IER) & 0x0f;
> -    ns_write_reg(uart, IER, scratch);
> +    scratch2 = ns_read_reg(uart, UART_IER) & 0x0f;
> +    ns_write_reg(uart,UART_IER, 0x0F);
> +    scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
> +    ns_write_reg(uart, UART_IER, scratch);
>      if ( (scratch2 != 0) || (scratch3 != 0x0F) )
>          return 0;
>  
> @@ -518,8 +451,8 @@ static int __init check_existence(struct ns16550 *uart)
>       * Check to see if a UART is really there.
>       * Use loopback test mode.
>       */
> -    ns_write_reg(uart, MCR, MCR_LOOP | 0x0A);
> -    status = ns_read_reg(uart, MSR) & 0xF0;
> +    ns_write_reg(uart, UART_MCR, UART_MCR_LOOP | 0x0A);
> +    status = ns_read_reg(uart, UART_MSR) & 0xF0;
>      return (status == 0x90);
>  }
>  
> diff --git a/xen/include/xen/8250.h b/xen/include/xen/8250.h
> new file mode 100644
> index 0000000..84f35bc
> --- /dev/null
> +++ b/xen/include/xen/8250.h
When you look to the name, it''s not clear what this file is. Could you
rename it to 8250-uart.h?
> @@ -0,0 +1,104 @@
> +/*
> + * xen/include/xen/8250.h
> + *
> + * This header is extracted from driver/char/ns16550.c
> + *
> + * Common constant definition between early printk and the UART driver
> + * for the 16550-series UART
> + *
> + * Copyright (c) 2003-2005, K A Fraser
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __XEN_NS16550_H__
> +#define __XEN_NS16550_H__
> +
> +/* Register offsets */
> +#define UART_RBR          0x00    /* receive buffer       */
> +#define UART_THR          0x00    /* transmit holding     */
> +#define UART_IER          0x01    /* interrupt enable     */
> +#define UART_IIR          0x02    /* interrupt identity   */
> +#define UART_FCR          0x02    /* FIFO control         */
> +#define UART_LCR          0x03    /* line control         */
> +#define UART_MCR          0x04    /* Modem control        */
> +#define UART_LSR          0x05    /* line status          */
> +#define UART_MSR          0x06    /* Modem status         */
> +#define UART_DLL          0x00    /* divisor latch (ls) (DLAB=1) */
> +#define UART_DLM          0x01    /* divisor latch (ms) (DLAB=1) */
> +
> +/* Interrupt Enable Register */
> +#define UART_IER_ERDAI    0x01    /* rx data recv''d       */
> +#define UART_IER_ETHREI   0x02    /* tx reg. empty        */
> +#define UART_IER_ELSI     0x04    /* rx line status       */
> +#define UART_IER_EMSI     0x08    /* MODEM status         */
> +
> +/* Interrupt Identificatiegister */
> +#define UART_IIR_NOINT    0x01    /* no interrupt pending */
> +#define UART_IIR_IMA      0x06    /* interrupt identity:  */
> +#define UART_IIR_LSI      0x06    /*  - rx line status    */
> +#define UART_IIR_RDA      0x04    /*  - rx data recv''d    */
> +#define UART_IIR_THR      0x02    /*  - tx reg. empty     */
> +#define UART_IIR_MSI      0x00    /*  - MODEM status      */
> +
> +/* 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_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   */
> +#define UART_FCR_TRG14    0xc0    /* Rx FIFO trig lev 14  */
> +
> +/* Line Control Register */
> +#define UART_LCR_DLAB     0x80    /* Divisor Latch Access */
> +
> +/* 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_LOOP     0x10    /* Enable loopback test mode */
> +
> +/* Line Status Register */
> +#define UART_LSR_DR       0x01    /* Data ready           */
> +#define UART_LSR_OE       0x02    /* Overrun              */
> +#define UART_LSR_PE       0x04    /* Parity error         */
> +#define UART_LSR_FE       0x08    /* Framing error        */
> +#define UART_LSR_BI       0x10    /* Break                */
> +#define UART_LSR_THRE     0x20    /* Xmit hold reg empty  */
> +#define UART_LSR_TEMT     0x40    /* Xmitter empty        */
> +#define UART_LSR_ERR      0x80    /* Error                */
> +
> +/* These parity settings can be ORed directly into the LCR. */
> +#define UART_PARITY_NONE  (0<<3)
> +#define UART_PARITY_ODD   (1<<3)
> +#define UART_PARITY_EVEN  (3<<3)
> +#define UART_PARITY_MARK  (5<<3)
> +#define UART_PARITY_SPACE (7<<3)
> +
> +/* Frequency of external clock source. This definition assumes PC
platform. */
> +#define UART_CLOCK_HZ     1843200
> +
> +/* Resume retry settings */
> +#define RESUME_DELAY      MILLISECS(10)
> +#define RESUME_RETRIES    100
> +
> +#endif /* __XEN_NS16550_H__ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> 
-- 
Julien

Chen Baozi

2013-Jul-19 15:37 UTC

head link

Re: [PATCH 1/4] xen: extract register definitions from ns16550 into a separated header

On Fri, Jul 19, 2013 at 11:35 PM, Julien Grall <julien.grall@linaro.org>
wrote:> On 07/19/2013 01:57 PM, Chen Baozi wrote:
>> Since both UART driver codes on Allwinner A31, OMAP5 and x86 would use
>> these definitions, we refactor the codes into a separated header to
avoid
>> unnecessary duplication.
>>
>> Signed-off-by: Chen Baozi <baozich@gmail.com>
>> ---
>>  xen/drivers/char/ns16550.c | 155
+++++++++++++--------------------------------
>>  xen/include/xen/8250.h     | 104 ++++++++++++++++++++++++++++++
>>  2 files changed, 148 insertions(+), 111 deletions(-)
>>  create mode 100644 xen/include/xen/8250.h
>>
>> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
>> index e0c87bb..9248330 100644
>> --- a/xen/drivers/char/ns16550.c
>> +++ b/xen/drivers/char/ns16550.c
>> @@ -19,6 +19,7 @@
>>  #include <xen/iocap.h>
>>  #include <xen/pci.h>
>>  #include <xen/pci_regs.h>
>> +#include <xen/8250.h>
>>  #include <asm/io.h>
>>  #ifdef CONFIG_X86
>>  #include <asm/fixmap.h>
>> @@ -58,76 +59,6 @@ static struct ns16550 {
>>      u8 bar_idx;
>>  } ns16550_com[2] = { { 0 } };
>>
>> -/* Register offsets */
>> -#define RBR             0x00    /* receive buffer       */
>> -#define THR             0x00    /* transmit holding     */
>> -#define IER             0x01    /* interrupt enable     */
>> -#define IIR             0x02    /* interrupt identity   */
>> -#define FCR             0x02    /* FIFO control         */
>> -#define LCR             0x03    /* line control         */
>> -#define MCR             0x04    /* Modem control        */
>> -#define LSR             0x05    /* line status          */
>> -#define MSR             0x06    /* Modem status         */
>> -#define DLL             0x00    /* divisor latch (ls) (DLAB=1) */
>> -#define DLM             0x01    /* divisor latch (ms) (DLAB=1) */
>> -
>> -/* Interrupt Enable Register */
>> -#define IER_ERDAI       0x01    /* rx data recv''d       */
>> -#define IER_ETHREI      0x02    /* tx reg. empty        */
>> -#define IER_ELSI        0x04    /* rx line status       */
>> -#define IER_EMSI        0x08    /* MODEM status         */
>> -
>> -/* Interrupt Identification Register */
>> -#define IIR_NOINT       0x01    /* no interrupt pending */
>> -#define IIR_IMASK       0x06    /* interrupt identity:  */
>> -#define IIR_LSI         0x06    /*  - rx line status    */
>> -#define IIR_RDAI        0x04    /*  - rx data recv''d    */
>> -#define IIR_THREI       0x02    /*  - tx reg. empty     */
>> -#define IIR_MSI         0x00    /*  - MODEM status      */
>> -
>> -/* FIFO Control Register */
>> -#define FCR_ENABLE      0x01    /* enable FIFO          */
>> -#define FCR_CLRX        0x02    /* clear Rx FIFO        */
>> -#define FCR_CLTX        0x04    /* clear Tx FIFO        */
>> -#define FCR_DMA         0x10    /* enter DMA mode       */
>> -#define FCR_TRG1        0x00    /* Rx FIFO trig lev 1   */
>> -#define FCR_TRG4        0x40    /* Rx FIFO trig lev 4   */
>> -#define FCR_TRG8        0x80    /* Rx FIFO trig lev 8   */
>> -#define FCR_TRG14       0xc0    /* Rx FIFO trig lev 14  */
>> -
>> -/* Line Control Register */
>> -#define LCR_DLAB        0x80    /* Divisor Latch Access */
>> -
>> -/* Modem Control Register */
>> -#define MCR_DTR         0x01    /* Data Terminal Ready  */
>> -#define MCR_RTS         0x02    /* Request to Send      */
>> -#define MCR_OUT2        0x08    /* OUT2: interrupt mask */
>> -#define MCR_LOOP        0x10    /* Enable loopback test mode */
>> -
>> -/* Line Status Register */
>> -#define LSR_DR          0x01    /* Data ready           */
>> -#define LSR_OE          0x02    /* Overrun              */
>> -#define LSR_PE          0x04    /* Parity error         */
>> -#define LSR_FE          0x08    /* Framing error        */
>> -#define LSR_BI          0x10    /* Break                */
>> -#define LSR_THRE        0x20    /* Xmit hold reg empty  */
>> -#define LSR_TEMT        0x40    /* Xmitter empty        */
>> -#define LSR_ERR         0x80    /* Error                */
>> -
>> -/* These parity settings can be ORed directly into the LCR. */
>> -#define PARITY_NONE     (0<<3)
>> -#define PARITY_ODD      (1<<3)
>> -#define PARITY_EVEN     (3<<3)
>> -#define PARITY_MARK     (5<<3)
>> -#define PARITY_SPACE    (7<<3)
>> -
>> -/* Frequency of external clock source. This definition assumes PC
platform. */
>> -#define UART_CLOCK_HZ   1843200
>> -
>> -/* Resume retry settings */
>> -#define RESUME_DELAY    MILLISECS(10)
>> -#define RESUME_RETRIES  100
>> -
>>  static char ns_read_reg(struct ns16550 *uart, int reg)
>>  {
>>      if ( uart->remapped_io_base == NULL )
>> @@ -150,12 +81,12 @@ static void ns16550_interrupt(
>>
>>      uart->intr_works = 1;
>>
>> -    while ( !(ns_read_reg(uart, IIR) & IIR_NOINT) )
>> +    while ( !(ns_read_reg(uart, UART_IIR) & UART_IIR_NOINT) )
>>      {
>> -        char lsr = ns_read_reg(uart, LSR);
>> -        if ( lsr & LSR_THRE )
>> +        char lsr = ns_read_reg(uart, UART_LSR);
>> +        if ( lsr & UART_LSR_THRE )
>>              serial_tx_interrupt(port, regs);
>> -        if ( lsr & LSR_DR )
>> +        if ( lsr & UART_LSR_DR )
>>              serial_rx_interrupt(port, regs);
>>      }
>>  }
>> @@ -171,10 +102,10 @@ static void __ns16550_poll(struct cpu_user_regs
*regs)
>>      if ( uart->intr_works )
>>          return; /* Interrupts work - no more polling */
>>
>> -    while ( ns_read_reg(uart, LSR) & LSR_DR )
>> +    while ( ns_read_reg(uart, UART_LSR) & UART_LSR_DR )
>>          serial_rx_interrupt(port, regs);
>>
>> -    if ( ns_read_reg(uart, LSR) & LSR_THRE )
>> +    if ( ns_read_reg(uart, UART_LSR) & UART_LSR_THRE )
>>          serial_tx_interrupt(port, regs);
>>
>>      set_timer(&uart->timer, NOW() +
MILLISECS(uart->timeout_ms));
>> @@ -194,23 +125,23 @@ static unsigned int ns16550_tx_ready(struct
serial_port *port)
>>  {
>>      struct ns16550 *uart = port->uart;
>>
>> -    return ns_read_reg(uart, LSR) & LSR_THRE ? uart->fifo_size
: 0;
>> +    return ns_read_reg(uart, UART_LSR) & UART_LSR_THRE ?
uart->fifo_size : 0;
>>  }
>>
>>  static void ns16550_putc(struct serial_port *port, char c)
>>  {
>>      struct ns16550 *uart = port->uart;
>> -    ns_write_reg(uart, THR, c);
>> +    ns_write_reg(uart, UART_THR, c);
>>  }
>>
>>  static int ns16550_getc(struct serial_port *port, char *pc)
>>  {
>>      struct ns16550 *uart = port->uart;
>>
>> -    if ( !(ns_read_reg(uart, LSR) & LSR_DR) )
>> +    if ( !(ns_read_reg(uart, UART_LSR) & UART_LSR_DR) )
>>          return 0;
>>
>> -    *pc = ns_read_reg(uart, RBR);
>> +    *pc = ns_read_reg(uart, UART_RBR);
>>      return 1;
>>  }
>>
>> @@ -244,31 +175,32 @@ 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, IER, 0);
>> +    ns_write_reg(uart, UART_IER, 0);
>>
>>      /* Line control and baud-rate generator. */
>> -    ns_write_reg(uart, LCR, lcr | LCR_DLAB);
>> +    ns_write_reg(uart, UART_LCR, lcr | UART_LCR_DLAB);
>>      if ( uart->baud != BAUD_AUTO )
>>      {
>>          /* Baud rate specified: program it into the divisor latch. */
>>          divisor = uart->clock_hz / (uart->baud << 4);
>> -        ns_write_reg(uart, DLL, (char)divisor);
>> -        ns_write_reg(uart, DLM, (char)(divisor >> 8));
>> +        ns_write_reg(uart, UART_DLL, (char)divisor);
>> +        ns_write_reg(uart, UART_DLM, (char)(divisor >> 8));
>>      }
>>      else
>>      {
>>          /* Baud rate already set: read it out from the divisor latch.
*/
>> -        divisor  = ns_read_reg(uart, DLL);
>> -        divisor |= ns_read_reg(uart, DLM) << 8;
>> +        divisor  = ns_read_reg(uart, UART_DLL);
>> +        divisor |= ns_read_reg(uart, UART_DLM) << 8;
>>          uart->baud = uart->clock_hz / (divisor << 4);
>>      }
>> -    ns_write_reg(uart, LCR, lcr);
>> +    ns_write_reg(uart, UART_LCR, lcr);
>>
>>      /* No flow ctrl: DTR and RTS are both wedged high to keep remote
happy. */
>> -    ns_write_reg(uart, MCR, MCR_DTR | MCR_RTS);
>> +    ns_write_reg(uart, UART_MCR, UART_MCR_DTR | UART_MCR_RTS);
>>
>>      /* Enable and clear the FIFOs. Set a large trigger threshold. */
>> -    ns_write_reg(uart, FCR, FCR_ENABLE | FCR_CLRX | FCR_CLTX |
FCR_TRG14);
>> +    ns_write_reg(uart, UART_FCR,
>> +                 UART_FCR_ENABLE | UART_FCR_CLRX | UART_FCR_CLTX |
UART_FCR_TRG14);
>>  }
>>
>>  static void __init ns16550_init_preirq(struct serial_port *port)
>> @@ -292,8 +224,8 @@ static void __init ns16550_init_preirq(struct
serial_port *port)
>>      ns16550_setup_preirq(uart);
>>
>>      /* Check this really is a 16550+. Otherwise we have no FIFOs. */
>> -    if ( ((ns_read_reg(uart, IIR) & 0xc0) == 0xc0) &&
>> -         ((ns_read_reg(uart, FCR) & FCR_TRG14) == FCR_TRG14) )
>> +    if ( ((ns_read_reg(uart, UART_IIR) & 0xc0) == 0xc0) &&
>> +         ((ns_read_reg(uart, UART_FCR) & UART_FCR_TRG14) ==
UART_FCR_TRG14) )
>>          uart->fifo_size = 16;
>>  }
>>
>> @@ -302,10 +234,11 @@ static void ns16550_setup_postirq(struct ns16550
*uart)
>>      if ( uart->irq > 0 )
>>      {
>>          /* Master interrupt enable; also keep DTR/RTS asserted. */
>> -        ns_write_reg(uart, MCR, MCR_OUT2 | MCR_DTR | MCR_RTS);
>> +        ns_write_reg(uart,
>> +                     UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR |
UART_MCR_RTS);
>>
>>          /* Enable receive and transmit interrupts. */
>> -        ns_write_reg(uart, IER, IER_ERDAI | IER_ETHREI);
>> +        ns_write_reg(uart, UART_IER, UART_IER_ERDAI |
UART_IER_ETHREI);
>>      }
>>
>>      if ( uart->irq >= 0 )
>> @@ -374,11 +307,11 @@ static void _ns16550_resume(struct serial_port
*port)
>>
>>  static int ns16550_ioport_invalid(struct ns16550 *uart)
>>  {
>> -    return ((((unsigned char)ns_read_reg(uart, LSR)) == 0xff)
&&
>> -            (((unsigned char)ns_read_reg(uart, MCR)) == 0xff)
&&
>> -            (((unsigned char)ns_read_reg(uart, IER)) == 0xff)
&&
>> -            (((unsigned char)ns_read_reg(uart, IIR)) == 0xff)
&&
>> -            (((unsigned char)ns_read_reg(uart, LCR)) == 0xff));
>> +    return ((((unsigned char)ns_read_reg(uart, UART_LSR)) == 0xff)
&&
>> +            (((unsigned char)ns_read_reg(uart, UART_MCR)) == 0xff)
&&
>> +            (((unsigned char)ns_read_reg(uart, UART_IER)) == 0xff)
&&
>> +            (((unsigned char)ns_read_reg(uart, UART_IIR)) == 0xff)
&&
>> +            (((unsigned char)ns_read_reg(uart, UART_LCR)) == 0xff));
>>  }
>>
>>  static int delayed_resume_tries;
>> @@ -457,15 +390,15 @@ static int __init parse_parity_char(int c)
>>      switch ( c )
>>      {
>>      case ''n'':
>> -        return PARITY_NONE;
>> +        return UART_PARITY_NONE;
>>      case ''o'':
>> -        return PARITY_ODD;
>> +        return UART_PARITY_ODD;
>>      case ''e'':
>> -        return PARITY_EVEN;
>> +        return UART_PARITY_EVEN;
>>      case ''m'':
>> -        return PARITY_MARK;
>> +        return UART_PARITY_MARK;
>>      case ''s'':
>> -        return PARITY_SPACE;
>> +        return UART_PARITY_SPACE;
>>      }
>>      return 0;
>>  }
>> @@ -500,17 +433,17 @@ static int __init check_existence(struct ns16550
*uart)
>>       * Do a simple existence test first; if we fail this,
>>       * there''s no point trying anything else.
>>       */
>> -    scratch = ns_read_reg(uart, IER);
>> -    ns_write_reg(uart, IER, 0);
>> +    scratch = ns_read_reg(uart, UART_IER);
>> +    ns_write_reg(uart, UART_IER, 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, IER) & 0x0f;
>> -    ns_write_reg(uart, IER, 0x0F);
>> -    scratch3 = ns_read_reg(uart, IER) & 0x0f;
>> -    ns_write_reg(uart, IER, scratch);
>> +    scratch2 = ns_read_reg(uart, UART_IER) & 0x0f;
>> +    ns_write_reg(uart,UART_IER, 0x0F);
>> +    scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
>> +    ns_write_reg(uart, UART_IER, scratch);
>>      if ( (scratch2 != 0) || (scratch3 != 0x0F) )
>>          return 0;
>>
>> @@ -518,8 +451,8 @@ static int __init check_existence(struct ns16550
*uart)
>>       * Check to see if a UART is really there.
>>       * Use loopback test mode.
>>       */
>> -    ns_write_reg(uart, MCR, MCR_LOOP | 0x0A);
>> -    status = ns_read_reg(uart, MSR) & 0xF0;
>> +    ns_write_reg(uart, UART_MCR, UART_MCR_LOOP | 0x0A);
>> +    status = ns_read_reg(uart, UART_MSR) & 0xF0;
>>      return (status == 0x90);
>>  }
>>
>> diff --git a/xen/include/xen/8250.h b/xen/include/xen/8250.h
>> new file mode 100644
>> index 0000000..84f35bc
>> --- /dev/null
>> +++ b/xen/include/xen/8250.h
>
> When you look to the name, it''s not clear what this file is. Could
you
> rename it to 8250-uart.h?
No problem. Thanks.
>
>> @@ -0,0 +1,104 @@
>> +/*
>> + * xen/include/xen/8250.h
>> + *
>> + * This header is extracted from driver/char/ns16550.c
>> + *
>> + * Common constant definition between early printk and the UART driver
>> + * for the 16550-series UART
>> + *
>> + * Copyright (c) 2003-2005, K A Fraser
>> + *
>> + * This program is free software; you can redistribute it and/or
modify
>> + * it under the terms of the GNU General Public License as published
by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#ifndef __XEN_NS16550_H__
>> +#define __XEN_NS16550_H__
>> +
>> +/* Register offsets */
>> +#define UART_RBR          0x00    /* receive buffer       */
>> +#define UART_THR          0x00    /* transmit holding     */
>> +#define UART_IER          0x01    /* interrupt enable     */
>> +#define UART_IIR          0x02    /* interrupt identity   */
>> +#define UART_FCR          0x02    /* FIFO control         */
>> +#define UART_LCR          0x03    /* line control         */
>> +#define UART_MCR          0x04    /* Modem control        */
>> +#define UART_LSR          0x05    /* line status          */
>> +#define UART_MSR          0x06    /* Modem status         */
>> +#define UART_DLL          0x00    /* divisor latch (ls) (DLAB=1) */
>> +#define UART_DLM          0x01    /* divisor latch (ms) (DLAB=1) */
>> +
>> +/* Interrupt Enable Register */
>> +#define UART_IER_ERDAI    0x01    /* rx data recv''d       */
>> +#define UART_IER_ETHREI   0x02    /* tx reg. empty        */
>> +#define UART_IER_ELSI     0x04    /* rx line status       */
>> +#define UART_IER_EMSI     0x08    /* MODEM status         */
>> +
>> +/* Interrupt Identificatiegister */
>> +#define UART_IIR_NOINT    0x01    /* no interrupt pending */
>> +#define UART_IIR_IMA      0x06    /* interrupt identity:  */
>> +#define UART_IIR_LSI      0x06    /*  - rx line status    */
>> +#define UART_IIR_RDA      0x04    /*  - rx data recv''d    */
>> +#define UART_IIR_THR      0x02    /*  - tx reg. empty     */
>> +#define UART_IIR_MSI      0x00    /*  - MODEM status      */
>> +
>> +/* 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_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   */
>> +#define UART_FCR_TRG14    0xc0    /* Rx FIFO trig lev 14  */
>> +
>> +/* Line Control Register */
>> +#define UART_LCR_DLAB     0x80    /* Divisor Latch Access */
>> +
>> +/* 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_LOOP     0x10    /* Enable loopback test mode */
>> +
>> +/* Line Status Register */
>> +#define UART_LSR_DR       0x01    /* Data ready           */
>> +#define UART_LSR_OE       0x02    /* Overrun              */
>> +#define UART_LSR_PE       0x04    /* Parity error         */
>> +#define UART_LSR_FE       0x08    /* Framing error        */
>> +#define UART_LSR_BI       0x10    /* Break                */
>> +#define UART_LSR_THRE     0x20    /* Xmit hold reg empty  */
>> +#define UART_LSR_TEMT     0x40    /* Xmitter empty        */
>> +#define UART_LSR_ERR      0x80    /* Error                */
>> +
>> +/* These parity settings can be ORed directly into the LCR. */
>> +#define UART_PARITY_NONE  (0<<3)
>> +#define UART_PARITY_ODD   (1<<3)
>> +#define UART_PARITY_EVEN  (3<<3)
>> +#define UART_PARITY_MARK  (5<<3)
>> +#define UART_PARITY_SPACE (7<<3)
>> +
>> +/* Frequency of external clock source. This definition assumes PC
platform. */
>> +#define UART_CLOCK_HZ     1843200
>> +
>> +/* Resume retry settings */
>> +#define RESUME_DELAY      MILLISECS(10)
>> +#define RESUME_RETRIES    100
>> +
>> +#endif /* __XEN_NS16550_H__ */
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>>
>
> --
> Julien

Julien Grall

2013-Jul-19 15:43 UTC

head link

Re: [PATCH 1/4] xen: extract register definitions from ns16550 into a separated header

On 07/19/2013 04:37 PM, Chen Baozi wrote:> On Fri, Jul 19, 2013 at 11:35 PM, Julien Grall
<julien.grall@linaro.org> wrote:
>> On 07/19/2013 01:57 PM, Chen Baozi wrote:
>>> Since both UART driver codes on Allwinner A31, OMAP5 and x86 would
use
>>> these definitions, we refactor the codes into a separated header to
avoid
>>> unnecessary duplication.
>>>
>>> Signed-off-by: Chen Baozi <baozich@gmail.com>
>>> ---
>>>  xen/drivers/char/ns16550.c | 155
+++++++++++++--------------------------------
>>>  xen/include/xen/8250.h     | 104 ++++++++++++++++++++++++++++++
>>>  2 files changed, 148 insertions(+), 111 deletions(-)
>>>  create mode 100644 xen/include/xen/8250.h
>>>
>>> diff --git a/xen/drivers/char/ns16550.c
b/xen/drivers/char/ns16550.c
>>> index e0c87bb..9248330 100644
>>> --- a/xen/drivers/char/ns16550.c
>>> +++ b/xen/drivers/char/ns16550.c
>>> @@ -19,6 +19,7 @@
>>>  #include <xen/iocap.h>
>>>  #include <xen/pci.h>
>>>  #include <xen/pci_regs.h>
>>> +#include <xen/8250.h>
>>>  #include <asm/io.h>
>>>  #ifdef CONFIG_X86
>>>  #include <asm/fixmap.h>
>>> @@ -58,76 +59,6 @@ static struct ns16550 {
>>>      u8 bar_idx;
>>>  } ns16550_com[2] = { { 0 } };
>>>
>>> -/* Register offsets */
>>> -#define RBR             0x00    /* receive buffer       */
>>> -#define THR             0x00    /* transmit holding     */
>>> -#define IER             0x01    /* interrupt enable     */
>>> -#define IIR             0x02    /* interrupt identity   */
>>> -#define FCR             0x02    /* FIFO control         */
>>> -#define LCR             0x03    /* line control         */
>>> -#define MCR             0x04    /* Modem control        */
>>> -#define LSR             0x05    /* line status          */
>>> -#define MSR             0x06    /* Modem status         */
>>> -#define DLL             0x00    /* divisor latch (ls) (DLAB=1) */
>>> -#define DLM             0x01    /* divisor latch (ms) (DLAB=1) */
>>> -
>>> -/* Interrupt Enable Register */
>>> -#define IER_ERDAI       0x01    /* rx data recv''d      
*/
>>> -#define IER_ETHREI      0x02    /* tx reg. empty        */
>>> -#define IER_ELSI        0x04    /* rx line status       */
>>> -#define IER_EMSI        0x08    /* MODEM status         */
>>> -
>>> -/* Interrupt Identification Register */
>>> -#define IIR_NOINT       0x01    /* no interrupt pending */
>>> -#define IIR_IMASK       0x06    /* interrupt identity:  */
>>> -#define IIR_LSI         0x06    /*  - rx line status    */
>>> -#define IIR_RDAI        0x04    /*  - rx data recv''d   
*/
>>> -#define IIR_THREI       0x02    /*  - tx reg. empty     */
>>> -#define IIR_MSI         0x00    /*  - MODEM status      */
>>> -
>>> -/* FIFO Control Register */
>>> -#define FCR_ENABLE      0x01    /* enable FIFO          */
>>> -#define FCR_CLRX        0x02    /* clear Rx FIFO        */
>>> -#define FCR_CLTX        0x04    /* clear Tx FIFO        */
>>> -#define FCR_DMA         0x10    /* enter DMA mode       */
>>> -#define FCR_TRG1        0x00    /* Rx FIFO trig lev 1   */
>>> -#define FCR_TRG4        0x40    /* Rx FIFO trig lev 4   */
>>> -#define FCR_TRG8        0x80    /* Rx FIFO trig lev 8   */
>>> -#define FCR_TRG14       0xc0    /* Rx FIFO trig lev 14  */
>>> -
>>> -/* Line Control Register */
>>> -#define LCR_DLAB        0x80    /* Divisor Latch Access */
>>> -
>>> -/* Modem Control Register */
>>> -#define MCR_DTR         0x01    /* Data Terminal Ready  */
>>> -#define MCR_RTS         0x02    /* Request to Send      */
>>> -#define MCR_OUT2        0x08    /* OUT2: interrupt mask */
>>> -#define MCR_LOOP        0x10    /* Enable loopback test mode */
>>> -
>>> -/* Line Status Register */
>>> -#define LSR_DR          0x01    /* Data ready           */
>>> -#define LSR_OE          0x02    /* Overrun              */
>>> -#define LSR_PE          0x04    /* Parity error         */
>>> -#define LSR_FE          0x08    /* Framing error        */
>>> -#define LSR_BI          0x10    /* Break                */
>>> -#define LSR_THRE        0x20    /* Xmit hold reg empty  */
>>> -#define LSR_TEMT        0x40    /* Xmitter empty        */
>>> -#define LSR_ERR         0x80    /* Error                */
>>> -
>>> -/* These parity settings can be ORed directly into the LCR. */
>>> -#define PARITY_NONE     (0<<3)
>>> -#define PARITY_ODD      (1<<3)
>>> -#define PARITY_EVEN     (3<<3)
>>> -#define PARITY_MARK     (5<<3)
>>> -#define PARITY_SPACE    (7<<3)
>>> -
>>> -/* Frequency of external clock source. This definition assumes PC
platform. */
>>> -#define UART_CLOCK_HZ   1843200
>>> -
>>> -/* Resume retry settings */
>>> -#define RESUME_DELAY    MILLISECS(10)
>>> -#define RESUME_RETRIES  100
>>> -
>>>  static char ns_read_reg(struct ns16550 *uart, int reg)
>>>  {
>>>      if ( uart->remapped_io_base == NULL )
>>> @@ -150,12 +81,12 @@ static void ns16550_interrupt(
>>>
>>>      uart->intr_works = 1;
>>>
>>> -    while ( !(ns_read_reg(uart, IIR) & IIR_NOINT) )
>>> +    while ( !(ns_read_reg(uart, UART_IIR) & UART_IIR_NOINT) )
>>>      {
>>> -        char lsr = ns_read_reg(uart, LSR);
>>> -        if ( lsr & LSR_THRE )
>>> +        char lsr = ns_read_reg(uart, UART_LSR);
>>> +        if ( lsr & UART_LSR_THRE )
>>>              serial_tx_interrupt(port, regs);
>>> -        if ( lsr & LSR_DR )
>>> +        if ( lsr & UART_LSR_DR )
>>>              serial_rx_interrupt(port, regs);
>>>      }
>>>  }
>>> @@ -171,10 +102,10 @@ static void __ns16550_poll(struct
cpu_user_regs *regs)
>>>      if ( uart->intr_works )
>>>          return; /* Interrupts work - no more polling */
>>>
>>> -    while ( ns_read_reg(uart, LSR) & LSR_DR )
>>> +    while ( ns_read_reg(uart, UART_LSR) & UART_LSR_DR )
>>>          serial_rx_interrupt(port, regs);
>>>
>>> -    if ( ns_read_reg(uart, LSR) & LSR_THRE )
>>> +    if ( ns_read_reg(uart, UART_LSR) & UART_LSR_THRE )
>>>          serial_tx_interrupt(port, regs);
>>>
>>>      set_timer(&uart->timer, NOW() +
MILLISECS(uart->timeout_ms));
>>> @@ -194,23 +125,23 @@ static unsigned int ns16550_tx_ready(struct
serial_port *port)
>>>  {
>>>      struct ns16550 *uart = port->uart;
>>>
>>> -    return ns_read_reg(uart, LSR) & LSR_THRE ?
uart->fifo_size : 0;
>>> +    return ns_read_reg(uart, UART_LSR) & UART_LSR_THRE ?
uart->fifo_size : 0;
>>>  }
>>>
>>>  static void ns16550_putc(struct serial_port *port, char c)
>>>  {
>>>      struct ns16550 *uart = port->uart;
>>> -    ns_write_reg(uart, THR, c);
>>> +    ns_write_reg(uart, UART_THR, c);
>>>  }
>>>
>>>  static int ns16550_getc(struct serial_port *port, char *pc)
>>>  {
>>>      struct ns16550 *uart = port->uart;
>>>
>>> -    if ( !(ns_read_reg(uart, LSR) & LSR_DR) )
>>> +    if ( !(ns_read_reg(uart, UART_LSR) & UART_LSR_DR) )
>>>          return 0;
>>>
>>> -    *pc = ns_read_reg(uart, RBR);
>>> +    *pc = ns_read_reg(uart, UART_RBR);
>>>      return 1;
>>>  }
>>>
>>> @@ -244,31 +175,32 @@ 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, IER, 0);
>>> +    ns_write_reg(uart, UART_IER, 0);
>>>
>>>      /* Line control and baud-rate generator. */
>>> -    ns_write_reg(uart, LCR, lcr | LCR_DLAB);
>>> +    ns_write_reg(uart, UART_LCR, lcr | UART_LCR_DLAB);
>>>      if ( uart->baud != BAUD_AUTO )
>>>      {
>>>          /* Baud rate specified: program it into the divisor latch.
*/
>>>          divisor = uart->clock_hz / (uart->baud << 4);
>>> -        ns_write_reg(uart, DLL, (char)divisor);
>>> -        ns_write_reg(uart, DLM, (char)(divisor >> 8));
>>> +        ns_write_reg(uart, UART_DLL, (char)divisor);
>>> +        ns_write_reg(uart, UART_DLM, (char)(divisor >> 8));
>>>      }
>>>      else
>>>      {
>>>          /* Baud rate already set: read it out from the divisor
latch. */
>>> -        divisor  = ns_read_reg(uart, DLL);
>>> -        divisor |= ns_read_reg(uart, DLM) << 8;
>>> +        divisor  = ns_read_reg(uart, UART_DLL);
>>> +        divisor |= ns_read_reg(uart, UART_DLM) << 8;
>>>          uart->baud = uart->clock_hz / (divisor << 4);
>>>      }
>>> -    ns_write_reg(uart, LCR, lcr);
>>> +    ns_write_reg(uart, UART_LCR, lcr);
>>>
>>>      /* No flow ctrl: DTR and RTS are both wedged high to keep
remote happy. */
>>> -    ns_write_reg(uart, MCR, MCR_DTR | MCR_RTS);
>>> +    ns_write_reg(uart, UART_MCR, UART_MCR_DTR | UART_MCR_RTS);
>>>
>>>      /* Enable and clear the FIFOs. Set a large trigger threshold.
*/
>>> -    ns_write_reg(uart, FCR, FCR_ENABLE | FCR_CLRX | FCR_CLTX |
FCR_TRG14);
>>> +    ns_write_reg(uart, UART_FCR,
>>> +                 UART_FCR_ENABLE | UART_FCR_CLRX | UART_FCR_CLTX |
UART_FCR_TRG14);
>>>  }
>>>
>>>  static void __init ns16550_init_preirq(struct serial_port *port)
>>> @@ -292,8 +224,8 @@ static void __init ns16550_init_preirq(struct
serial_port *port)
>>>      ns16550_setup_preirq(uart);
>>>
>>>      /* Check this really is a 16550+. Otherwise we have no FIFOs.
*/
>>> -    if ( ((ns_read_reg(uart, IIR) & 0xc0) == 0xc0) &&
>>> -         ((ns_read_reg(uart, FCR) & FCR_TRG14) == FCR_TRG14) )
>>> +    if ( ((ns_read_reg(uart, UART_IIR) & 0xc0) == 0xc0)
&&
>>> +         ((ns_read_reg(uart, UART_FCR) & UART_FCR_TRG14) ==
UART_FCR_TRG14) )
>>>          uart->fifo_size = 16;
>>>  }
>>>
>>> @@ -302,10 +234,11 @@ static void ns16550_setup_postirq(struct
ns16550 *uart)
>>>      if ( uart->irq > 0 )
>>>      {
>>>          /* Master interrupt enable; also keep DTR/RTS asserted. */
>>> -        ns_write_reg(uart, MCR, MCR_OUT2 | MCR_DTR | MCR_RTS);
>>> +        ns_write_reg(uart,
>>> +                     UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR |
UART_MCR_RTS);
>>>
>>>          /* Enable receive and transmit interrupts. */
>>> -        ns_write_reg(uart, IER, IER_ERDAI | IER_ETHREI);
>>> +        ns_write_reg(uart, UART_IER, UART_IER_ERDAI |
UART_IER_ETHREI);
>>>      }
>>>
>>>      if ( uart->irq >= 0 )
>>> @@ -374,11 +307,11 @@ static void _ns16550_resume(struct
serial_port *port)
>>>
>>>  static int ns16550_ioport_invalid(struct ns16550 *uart)
>>>  {
>>> -    return ((((unsigned char)ns_read_reg(uart, LSR)) == 0xff)
&&
>>> -            (((unsigned char)ns_read_reg(uart, MCR)) == 0xff)
&&
>>> -            (((unsigned char)ns_read_reg(uart, IER)) == 0xff)
&&
>>> -            (((unsigned char)ns_read_reg(uart, IIR)) == 0xff)
&&
>>> -            (((unsigned char)ns_read_reg(uart, LCR)) == 0xff));
>>> +    return ((((unsigned char)ns_read_reg(uart, UART_LSR)) == 0xff)
&&
>>> +            (((unsigned char)ns_read_reg(uart, UART_MCR)) == 0xff)
&&
>>> +            (((unsigned char)ns_read_reg(uart, UART_IER)) == 0xff)
&&
>>> +            (((unsigned char)ns_read_reg(uart, UART_IIR)) == 0xff)
&&
>>> +            (((unsigned char)ns_read_reg(uart, UART_LCR)) ==
0xff));
>>>  }
>>>
>>>  static int delayed_resume_tries;
>>> @@ -457,15 +390,15 @@ static int __init parse_parity_char(int c)
>>>      switch ( c )
>>>      {
>>>      case ''n'':
>>> -        return PARITY_NONE;
>>> +        return UART_PARITY_NONE;
>>>      case ''o'':
>>> -        return PARITY_ODD;
>>> +        return UART_PARITY_ODD;
>>>      case ''e'':
>>> -        return PARITY_EVEN;
>>> +        return UART_PARITY_EVEN;
>>>      case ''m'':
>>> -        return PARITY_MARK;
>>> +        return UART_PARITY_MARK;
>>>      case ''s'':
>>> -        return PARITY_SPACE;
>>> +        return UART_PARITY_SPACE;
>>>      }
>>>      return 0;
>>>  }
>>> @@ -500,17 +433,17 @@ static int __init check_existence(struct
ns16550 *uart)
>>>       * Do a simple existence test first; if we fail this,
>>>       * there''s no point trying anything else.
>>>       */
>>> -    scratch = ns_read_reg(uart, IER);
>>> -    ns_write_reg(uart, IER, 0);
>>> +    scratch = ns_read_reg(uart, UART_IER);
>>> +    ns_write_reg(uart, UART_IER, 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, IER) & 0x0f;
>>> -    ns_write_reg(uart, IER, 0x0F);
>>> -    scratch3 = ns_read_reg(uart, IER) & 0x0f;
>>> -    ns_write_reg(uart, IER, scratch);
>>> +    scratch2 = ns_read_reg(uart, UART_IER) & 0x0f;
>>> +    ns_write_reg(uart,UART_IER, 0x0F);
>>> +    scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
>>> +    ns_write_reg(uart, UART_IER, scratch);
>>>      if ( (scratch2 != 0) || (scratch3 != 0x0F) )
>>>          return 0;
>>>
>>> @@ -518,8 +451,8 @@ static int __init check_existence(struct
ns16550 *uart)
>>>       * Check to see if a UART is really there.
>>>       * Use loopback test mode.
>>>       */
>>> -    ns_write_reg(uart, MCR, MCR_LOOP | 0x0A);
>>> -    status = ns_read_reg(uart, MSR) & 0xF0;
>>> +    ns_write_reg(uart, UART_MCR, UART_MCR_LOOP | 0x0A);
>>> +    status = ns_read_reg(uart, UART_MSR) & 0xF0;
>>>      return (status == 0x90);
>>>  }
>>>
>>> diff --git a/xen/include/xen/8250.h b/xen/include/xen/8250.h
>>> new file mode 100644
>>> index 0000000..84f35bc
>>> --- /dev/null
>>> +++ b/xen/include/xen/8250.h
>>
>> When you look to the name, it''s not clear what this file is.
Could you
>> rename it to 8250-uart.h?
> 
> No problem. Thanks.
BTW, this patch was already committed on 16 july: 3726b10.
The file was named ns16550-uart.h
>>
>>> @@ -0,0 +1,104 @@
>>> +/*
>>> + * xen/include/xen/8250.h
>>> + *
>>> + * This header is extracted from driver/char/ns16550.c
>>> + *
>>> + * Common constant definition between early printk and the UART
driver
>>> + * for the 16550-series UART
>>> + *
>>> + * Copyright (c) 2003-2005, K A Fraser
>>> + *
>>> + * This program is free software; you can redistribute it and/or
modify
>>> + * it under the terms of the GNU General Public License as
published by
>>> + * the Free Software Foundation; either version 2 of the License,
or
>>> + * (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>>> + */
>>> +
>>> +#ifndef __XEN_NS16550_H__
>>> +#define __XEN_NS16550_H__
>>> +
>>> +/* Register offsets */
>>> +#define UART_RBR          0x00    /* receive buffer       */
>>> +#define UART_THR          0x00    /* transmit holding     */
>>> +#define UART_IER          0x01    /* interrupt enable     */
>>> +#define UART_IIR          0x02    /* interrupt identity   */
>>> +#define UART_FCR          0x02    /* FIFO control         */
>>> +#define UART_LCR          0x03    /* line control         */
>>> +#define UART_MCR          0x04    /* Modem control        */
>>> +#define UART_LSR          0x05    /* line status          */
>>> +#define UART_MSR          0x06    /* Modem status         */
>>> +#define UART_DLL          0x00    /* divisor latch (ls) (DLAB=1)
*/
>>> +#define UART_DLM          0x01    /* divisor latch (ms) (DLAB=1)
*/
>>> +
>>> +/* Interrupt Enable Register */
>>> +#define UART_IER_ERDAI    0x01    /* rx data recv''d      
*/
>>> +#define UART_IER_ETHREI   0x02    /* tx reg. empty        */
>>> +#define UART_IER_ELSI     0x04    /* rx line status       */
>>> +#define UART_IER_EMSI     0x08    /* MODEM status         */
>>> +
>>> +/* Interrupt Identificatiegister */
>>> +#define UART_IIR_NOINT    0x01    /* no interrupt pending */
>>> +#define UART_IIR_IMA      0x06    /* interrupt identity:  */
>>> +#define UART_IIR_LSI      0x06    /*  - rx line status    */
>>> +#define UART_IIR_RDA      0x04    /*  - rx data recv''d   
*/
>>> +#define UART_IIR_THR      0x02    /*  - tx reg. empty     */
>>> +#define UART_IIR_MSI      0x00    /*  - MODEM status      */
>>> +
>>> +/* 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_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   */
>>> +#define UART_FCR_TRG14    0xc0    /* Rx FIFO trig lev 14  */
>>> +
>>> +/* Line Control Register */
>>> +#define UART_LCR_DLAB     0x80    /* Divisor Latch Access */
>>> +
>>> +/* 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_LOOP     0x10    /* Enable loopback test mode */
>>> +
>>> +/* Line Status Register */
>>> +#define UART_LSR_DR       0x01    /* Data ready           */
>>> +#define UART_LSR_OE       0x02    /* Overrun              */
>>> +#define UART_LSR_PE       0x04    /* Parity error         */
>>> +#define UART_LSR_FE       0x08    /* Framing error        */
>>> +#define UART_LSR_BI       0x10    /* Break                */
>>> +#define UART_LSR_THRE     0x20    /* Xmit hold reg empty  */
>>> +#define UART_LSR_TEMT     0x40    /* Xmitter empty        */
>>> +#define UART_LSR_ERR      0x80    /* Error                */
>>> +
>>> +/* These parity settings can be ORed directly into the LCR. */
>>> +#define UART_PARITY_NONE  (0<<3)
>>> +#define UART_PARITY_ODD   (1<<3)
>>> +#define UART_PARITY_EVEN  (3<<3)
>>> +#define UART_PARITY_MARK  (5<<3)
>>> +#define UART_PARITY_SPACE (7<<3)
>>> +
>>> +/* Frequency of external clock source. This definition assumes PC
platform. */
>>> +#define UART_CLOCK_HZ     1843200
>>> +
>>> +/* Resume retry settings */
>>> +#define RESUME_DELAY      MILLISECS(10)
>>> +#define RESUME_RETRIES    100
>>> +
>>> +#endif /* __XEN_NS16550_H__ */
>>> +
>>> +/*
>>> + * Local variables:
>>> + * mode: C
>>> + * c-file-style: "BSD"
>>> + * c-basic-offset: 4
>>> + * indent-tabs-mode: nil
>>> + * End:
>>> + */
>>>
>>
>> --
>> Julien

Julien Grall

2013-Jul-19 15:49 UTC

head link

Re: [PATCH 2/4] xen/arm: add OMAP5432 UART support for early_printk

On 07/19/2013 01:57 PM, Chen Baozi wrote:> 
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> ---
>  docs/misc/arm/early-printk.txt    |  1 +
>  xen/arch/arm/Rules.mk             |  4 ++++
>  xen/arch/arm/arm32/debug-8250.inc | 48
+++++++++++++++++++++++++++++++++++++++
>  3 files changed, 53 insertions(+)
>  create mode 100644 xen/arch/arm/arm32/debug-8250.inc
> 
> diff --git a/docs/misc/arm/early-printk.txt
b/docs/misc/arm/early-printk.txt
> index fbc3208..874488f 100644
> --- a/docs/misc/arm/early-printk.txt
> +++ b/docs/misc/arm/early-printk.txt
> @@ -13,6 +13,7 @@ where mach is the name of the machine:
>    - exynos5250: printk with the second UART
>    - midway: printk with the pl011 on Calxeda Midway processors
>    - fastmodel: printk on ARM Fastmodel software emulators
> +  - omap5432: printk with UART3 on TI OMAP5432 processors
>  
>  The base address and baud rate is hardcoded in xen/arch/arm/Rules.mk,
>  see there when adding support for new machines.
> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index 422ed04..eaa03fb 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -64,6 +64,10 @@ EARLY_PRINTK_INC := pl011
>  EARLY_PRINTK_BAUD := 115200
>  EARLY_UART_BASE_ADDRESS := 0xfff36000
>  endif
> +ifeq ($(CONFIG_EARLY_PRINTK), omap5432)
> +EARLY_PRINTK_INC := 8250
> +EARLY_UART_BASE_ADDRESS := 0x48020000
> +endif
>  
>  ifneq ($(EARLY_PRINTK_INC),)
>  EARLY_PRINTK := y
> diff --git a/xen/arch/arm/arm32/debug-8250.inc
b/xen/arch/arm/arm32/debug-8250.inc
> new file mode 100644
> index 0000000..cbb63ca
> --- /dev/null
> +++ b/xen/arch/arm/arm32/debug-8250.inc
> @@ -0,0 +1,48 @@
> +/*
> + * xen/arch/arm/arm32/debug-8250.inc
> + *
> + * 8250 specific debug code
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <xen/8250.h>
> +
> +/* OMAP UART initialization
> + * rb: register which contains the UART base address
> + * rc: scratch register 1
> + * rd: scratch register 2 */
> +.macro early_uart_init rb rc rd
> +.endm
You don''t need to define early_uart_init. The debug code will not use
it
if EARLY_PRINTK_INIT_UART is not enabled.
> +
> +/* OMAP UART wait UART to be ready to transmit
> + * rb: register which contains the UART base address
> + * rc: scratch register */
> +.macro early_uart_ready rb rc
> +1:
> +	ldr	\rc, [\rb, #(UART_LSR<<2)] /* Read line status register */
Where does "<<2" come from?
> +	tst	\rc, #UART_LSR_THRE   /* Check Xmit holding register flag */
> +	beq	1b		      /* Wait for the UART to be ready */
> +.endm
> +
> +/* OMAP UART transmit character
> + * rb: register which contains the UART base address
> + * rt: register which contains the character to transmit */
> +.macro early_uart_transmit rb rt
> +        str   \rt, [\rb, #UART_THR]      /* Write Transmit buffer */
> +.endm
> +
> +/*
> + * Local variables:
> + * mode: ASM
> + * indent-tabs-mode: nil
> + * End:
> + */
>

Julien Grall

2013-Jul-19 17:06 UTC

head link

Re: [PATCH 4/4] xen/arm: enable ns16550 uart driver for OMAP5432

On 07/19/2013 01:57 PM, Chen Baozi wrote:> 
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> ---
>  xen/arch/arm/Rules.mk      |   1 +
>  xen/drivers/char/ns16550.c | 155
+++++++++++++++++++++++++++++++++++++++------
>  xen/include/asm-arm/io.h   |  49 ++++++++++++++
>  xen/include/xen/serial.h   |   7 +-
>  4 files changed, 191 insertions(+), 21 deletions(-)
> 
> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index eaa03fb..8604e34 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -9,6 +9,7 @@
>  HAS_DEVICE_TREE := y
>  HAS_VIDEO := y
>  HAS_ARM_HDLCD := y
> +HAS_NS16550 := y
>  
>  CFLAGS += -fno-builtin -fno-common -Wredundant-decls
>  CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index 60512be..b8690eb 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -1,10 +1,10 @@
> 
/******************************************************************************
>   * ns16550.c
> - * 
> + *
Spurious change.>   * Driver for 16550-series UARTs. This driver is to be kept within Xen as
>   * it permits debugging of seriously-toasted machines (e.g., in situations
>   * where a device driver within a guest OS would be inaccessible).
> - * 
> + *Spurious change.
>   * Copyright (c) 2003-2005, K A Fraser
>   */
>  
> @@ -25,6 +25,13 @@
>  #include <asm/fixmap.h>
>  #endif
>  
> +#ifdef CONFIG_ARM
> +#include <xen/device_tree.h>
> +#include <xen/errno.h>
The above 2 includes can be moved outside the ifdef.
> +#include <asm/early_printk.h>
> +#include <asm/device.h>
> +#endif
> +
>  /*
>   * Configure serial port with a string:
>   *  
<baud>[/<clock_hz>][,DPS[,<io-base>[,<irq>[,<port-bdf>[,<bridge-bdf>]]]]].
> @@ -39,8 +46,11 @@ string_param("com1", opt_com1);
>  string_param("com2", opt_com2);
>  
>  static struct ns16550 {
> -    int baud, clock_hz, data_bits, parity, stop_bits, fifo_size, irq;
> -    unsigned long io_base;   /* I/O port or memory-mapped I/O address. */
> +    int baud, data_bits, parity, stop_bits, fifo_size;
> +    u32 clock_hz;
> +    struct dt_irq irq;
> +    u64 io_base;   /* I/O port or memory-mapped I/O address. */
> +    u64 io_size;
>      char __iomem *remapped_io_base;  /* Remapped virtual address of MMIO.
*/
>      /* UART with IRQ line: interrupt-driven I/O. */
>      struct irqaction irqaction;
> @@ -61,16 +71,24 @@ static struct ns16550 {
>  
>  static char ns_read_reg(struct ns16550 *uart, int reg)
>  {
> +#ifdef CONFIG_X86
>      if ( uart->remapped_io_base == NULL )
>          return inb(uart->io_base + reg);
>      return readb(uart->remapped_io_base + reg);
> +#else
> +    return readb(uart->remapped_io_base + (reg << 2));
> +#endif
>  }
>  
Is it possible to add a new private field "shift"? So the both readb
can
be merged.
>  static void ns_write_reg(struct ns16550 *uart, int reg, char c)
>  {
> +#ifdef CONFIG_X86
>      if ( uart->remapped_io_base == NULL )
>          return outb(c, uart->io_base + reg);
>      writeb(c, uart->remapped_io_base + reg);
> +#else
> +    writeb(c, uart->remapped_io_base + (reg << 2));
> +#endif
Same here for readb.
>  }
>  
>  static void ns16550_interrupt(
> @@ -145,11 +163,12 @@ static int ns16550_getc(struct serial_port *port,
char *pc)
>      return 1;
>  }
>  
> +#ifdef CONFIG_X86
>  static void pci_serial_early_init(struct ns16550 *uart)
>  {
>      if ( !uart->ps_bdf_enable || uart->io_base >= 0x10000 )
>          return;
> -    
> +Spurious change.
>      if ( uart->pb_bdf_enable )
>          pci_conf_write16(0, uart->pb_bdf[0], uart->pb_bdf[1],
uart->pb_bdf[2],
>                           PCI_IO_BASE,
> @@ -161,7 +180,13 @@ static void pci_serial_early_init(struct ns16550
*uart)
>                       uart->io_base | PCI_BASE_ADDRESS_SPACE_IO);
>      pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1],
uart->ps_bdf[2],
>                       PCI_COMMAND, PCI_COMMAND_IO);
> +
> +}
> +#else
> +static void pci_serial_early_init(struct ns16550 *uart)
> +{
>  }
> +#endif
>  
>  static void ns16550_setup_preirq(struct ns16550 *uart)
>  {
> @@ -217,7 +242,9 @@ static void __init ns16550_init_preirq(struct
serial_port *port)
>          uart->remapped_io_base = (void __iomem *)fix_to_virt(idx);
>          uart->remapped_io_base += uart->io_base & ~PAGE_MASK;
>  #else
> -        uart->remapped_io_base = (char *)ioremap(uart->io_base, 8);
> +        uart->remapped_io_base = ioremap_attr(uart->io_base,
> +                                              uart->io_size,
> +                                              PAGE_HYPERVISOR_NOCACHE);
>  #endif
>      }
>  
> @@ -231,7 +258,7 @@ static void __init ns16550_init_preirq(struct
serial_port *port)
>  
>  static void ns16550_setup_postirq(struct ns16550 *uart)
>  {
> -    if ( uart->irq > 0 )
> +    if ( uart->irq.irq > 0 )
>      {
>          /* Master interrupt enable; also keep DTR/RTS asserted. */
>          ns_write_reg(uart,
> @@ -241,7 +268,7 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
>          ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ELSI);
>      }
>  
> -    if ( uart->irq >= 0 )
> +    if ( uart->irq.irq >= 0 )
>          set_timer(&uart->timer, NOW() +
MILLISECS(uart->timeout_ms));
>  }
>  
> @@ -250,7 +277,7 @@ static void __init ns16550_init_postirq(struct
serial_port *port)
>      struct ns16550 *uart = port->uart;
>      int rc, bits;
>  
> -    if ( uart->irq < 0 )
> +    if ( uart->irq.irq < 0 )
>          return;
>  
>      serial_async_transmit(port);
> @@ -262,20 +289,26 @@ static void __init ns16550_init_postirq(struct
serial_port *port)
>      uart->timeout_ms = max_t(
>          unsigned int, 1, (bits * uart->fifo_size * 1000) /
uart->baud);
>  
> -    if ( uart->irq > 0 )
> +    if ( uart->irq.irq > 0 )
>      {
>          uart->irqaction.handler = ns16550_interrupt;
>          uart->irqaction.name    = "ns16550";
>          uart->irqaction.dev_id  = port;
> -        if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0
)
> -            printk("ERROR: Failed to allocate ns16550 IRQ %d\n",
uart->irq);
> +#ifdef CONFIG_X86
> +        if ( (rc = setup_irq(uart->irq.irq, &uart->irqaction))
!= 0 )
> +#else
> +        if ( (rc = setup_dt_irq(&uart->irq,
&uart->irqaction)) != 0 )
> +#endif
> +            printk("ERROR: Failed to allocate ns16550 IRQ %d\n",
uart->irq.irq);
>      }
>  
>      ns16550_setup_postirq(uart);
>  
> +#ifdef CONFIG_X86
>      if ( uart->bar || uart->ps_bdf_enable )
>          pci_hide_device(uart->ps_bdf[0], PCI_DEVFN(uart->ps_bdf[1],
>                                                     uart->ps_bdf[2]));
> +#endif
>  }
>  
>  static void ns16550_suspend(struct serial_port *port)
> @@ -284,13 +317,16 @@ static void ns16550_suspend(struct serial_port *port)
>  
>      stop_timer(&uart->timer);
>  
> +#ifdef CONFIG_X86
>      if ( uart->bar )
>         uart->cr = pci_conf_read16(0, uart->ps_bdf[0],
uart->ps_bdf[1],
>                                    uart->ps_bdf[2], PCI_COMMAND);
> +#endif
>  }
>  
>  static void _ns16550_resume(struct serial_port *port)
>  {
> +#ifdef CONFIG_X86
>      struct ns16550 *uart = port->uart;
>  
>      if ( uart->bar )
> @@ -300,6 +336,7 @@ static void _ns16550_resume(struct serial_port *port)
>         pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1],
uart->ps_bdf[2],
>                          PCI_COMMAND, uart->cr);
>      }
> +#endif
>  
>      ns16550_setup_preirq(port->uart);
>      ns16550_setup_postirq(port->uart);
> @@ -370,9 +407,17 @@ static void __init ns16550_endboot(struct serial_port
*port)
>  static int __init ns16550_irq(struct serial_port *port)
>  {
>      struct ns16550 *uart = port->uart;
> -    return ((uart->irq > 0) ? uart->irq : -1);
> +    return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
>  }
>  
> +#ifdef CONFIG_ARM
> +static const struct dt_irq __init *ns16550_dt_irq(struct serial_port
*port)
> +{
> +    struct ns16550 *uart = port->uart;
> +    return &uart->irq;
> +}
> +#endif
You don''t need to surround this function by ifdef.
>  static struct uart_driver __read_mostly ns16550_driver = {
>      .init_preirq  = ns16550_init_preirq,
>      .init_postirq = ns16550_init_postirq,
> @@ -382,7 +427,10 @@ static struct uart_driver __read_mostly ns16550_driver
= {
>      .tx_ready     = ns16550_tx_ready,
>      .putc         = ns16550_putc,
>      .getc         = ns16550_getc,
> -    .irq          = ns16550_irq
> +    .irq          = ns16550_irq,
> +#ifdef CONFIG_ARM
> +    .dt_irq_get   = ns16550_dt_irq
> +#endif
>  };
same here.
>  static int __init parse_parity_char(int c)
> @@ -403,6 +451,7 @@ static int __init parse_parity_char(int c)
>      return 0;
>  }
>  
> +#ifdef CONFIG_X86
>  static void __init parse_pci_bdf(const char **conf, unsigned int bdf[3])
>  {
>      bdf[0] = simple_strtoul(*conf, conf, 16);
> @@ -415,6 +464,7 @@ static void __init parse_pci_bdf(const char **conf,
unsigned int bdf[3])
>      (*conf)++;
>      bdf[2] = simple_strtoul(*conf, conf, 16);
>  }
> +#endif
>  
>  static int __init check_existence(struct ns16550 *uart)
>  {
> @@ -428,7 +478,7 @@ static int __init check_existence(struct ns16550 *uart)
>          return 1;
>  
>      pci_serial_early_init(uart);
> -    
> +
spurious change.
>      /*
>       * Do a simple existence test first; if we fail this,
>       * there''s no point trying anything else.
> @@ -445,7 +495,7 @@ static int __init check_existence(struct ns16550 *uart)
>      scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
>      ns_write_reg(uart, UART_IER, scratch);
>      if ( (scratch2 != 0) || (scratch3 != 0x0F) )
> -        return 0;
> +            return 0;
spurious change.
>  
>      /*
>       * Check to see if a UART is really there.
> @@ -456,6 +506,7 @@ static int __init check_existence(struct ns16550 *uart)
>      return (status == 0x90);
>  }
>  
> +#ifdef CONFIG_X86
>  static int
>  pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
>  {
> @@ -507,7 +558,7 @@ pci_uart_config (struct ns16550 *uart, int skip_amt,
int bar_idx)
>                  uart->bar = bar;
>                  uart->bar_idx = bar_idx;
>                  uart->io_base = bar & ~PCI_BASE_ADDRESS_SPACE_IO;
> -                uart->irq = pci_conf_read8(0, b, d, f,
PCI_INTERRUPT_PIN) ?
> +                uart->irq.irq = pci_conf_read8(0, b, d, f,
PCI_INTERRUPT_PIN) ?
>                      pci_conf_read8(0, b, d, f, PCI_INTERRUPT_LINE) : 0;
>  
>                  return 0;
> @@ -519,11 +570,12 @@ pci_uart_config (struct ns16550 *uart, int skip_amt,
int bar_idx)
>          return -1;
>  
>      uart->io_base = 0x3f8;
> -    uart->irq = 0;
> +    uart->irq.irq = 0;
>      uart->clock_hz  = UART_CLOCK_HZ;
>  
>      return 0;
>  }
> +#endif
>  
>  #define PARSE_ERR(_f, _a...)                 \
>      do {                                     \
> @@ -534,6 +586,7 @@ pci_uart_config (struct ns16550 *uart, int skip_amt,
int bar_idx)
>  static void __init ns16550_parse_port_config(
>      struct ns16550 *uart, const char *conf)
>  {
> +#ifdef CONFIG_X86
>      int baud;
>  
>      /* No user-specified configuration? */
> @@ -589,7 +642,7 @@ static void __init ns16550_parse_port_config(
>      }
>  
>      if ( *conf == '','' && *++conf !=
'','' )
> -        uart->irq = simple_strtol(conf, &conf, 10);
> +        uart->irq.irq = simple_strtol(conf, &conf, 10);
>  
>      if ( *conf == '','' && *++conf !=
'','' )
>      {
> @@ -604,6 +657,8 @@ static void __init ns16550_parse_port_config(
>      }
>  
>   config_parsed:
> +#endif
> +
You have disabled nearly 80% of the code for ARM. Perhaps you can split
this function in 2:
	- Parsing pci/user configuration
        - Register the UART
>      /* Sanity checks. */
>      if ( (uart->baud != BAUD_AUTO) &&
>           ((uart->baud < 1200) || (uart->baud > 115200)) )
> @@ -633,18 +688,80 @@ void __init ns16550_init(int index, struct
ns16550_defaults *defaults)
>      uart->baud      = (defaults->baud ? :
>                         console_has((index == 0) ? "com1" :
"com2")
>                         ? BAUD_AUTO : 0);
> +#ifdef CONFIG_ARM
> +    uart->clock_hz = defaults->clock_hz;
> +#else
>      uart->clock_hz  = UART_CLOCK_HZ;
> +#endif
>      uart->data_bits = defaults->data_bits;
>      uart->parity    = parse_parity_char(defaults->parity);
>      uart->stop_bits = defaults->stop_bits;
>      uart->irq       = defaults->irq;
>      uart->io_base   = defaults->io_base;
> +#ifdef CONFIG_ARM
> +    uart->io_size   = defaults->io_size;
> +#endif
> +
>      /* Default is no transmit FIFO. */
>      uart->fifo_size = 1;
>  
>      ns16550_parse_port_config(uart, (index == 0) ? opt_com1 : opt_com2);
> +
>  }
>  
> +#ifdef CONFIG_ARM
> +
> +static int __init ns16550_dt_init(struct dt_device_node *dev,
> +                                  const void *data)
> +{
> +    struct ns16550_defaults uart;
> +    int res;
> +    const __be32 *clkspec;
> +
> +    uart.baud      = 115200;
> +    uart.data_bits = 8;
> +    uart.parity    = ''n'';
> +    uart.stop_bits = 1;
> +
> +    res = dt_device_get_address(dev, 0, &uart.io_base,
&uart.io_size);
> +    if (res) {
> +        early_printk("ns16550: Unable to retrieve the base"
> +                     " address of the UART\n");
> +        return res;
> +    }
> +
> +    res = dt_device_get_irq(dev, 0, &uart.irq);
> +    if (res) {
> +        early_printk("ns16550: Unable to retrieve the IRQ\n");
> +        return res;
> +    }
> +
> +    clkspec = dt_get_property(dev, "clock-frequency", NULL);
> +    if (clkspec == NULL) {
> +        early_printk("ns16550: Unable to retrieve the clock
frequency\n");
> +        return -EINVAL;
> +    }
> +    uart.clock_hz = be32_to_cpup(clkspec);
> +
> +    ns16550_init(0, &uart);
> +    dt_device_set_used_by(dev, DOMID_XEN);
> +
> +    return 0;
> +}
> +
> +static const char const *ns16550_dt_compat[] __initdata > +{
> +    "ti,omap4-uart",
> +    NULL
> +};
> +
> +DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
> +    .compatible = ns16550_dt_compat,
> +    .init = ns16550_dt_init,
> +DT_DEVICE_END
> +
> +#endif /* CONFIG_ARM */
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/asm-arm/io.h b/xen/include/asm-arm/io.h
> index aea5233..33674bc 100644
> --- a/xen/include/asm-arm/io.h
> +++ b/xen/include/asm-arm/io.h
> @@ -1,6 +1,55 @@
>  #ifndef _ASM_IO_H
>  #define _ASM_IO_H
>  
> +static inline void __raw_writeb(u8 val, volatile void __iomem *addr)
> +{
> +        asm volatile("strb %1, %0"
> +                     : "+Qo" (*(volatile u8 __force *)addr)
> +                     : "r" (val));
> +}
> +
> +static inline void __raw_writel(u32 val, volatile void __iomem *addr)
> +{
> +        asm volatile("str %1, %0"
> +                     : "+Qo" (*(volatile u32 __force *)addr)
> +                     : "r" (val));
> +}
> +
> +static inline u8 __raw_readb(const volatile void __iomem *addr)
> +{
> +        u8 val;
> +        asm volatile("ldrb %1, %0"
> +                     : "+Qo" (*(volatile u8 __force *)addr),
> +                       "=r" (val));
> +        return val;
> +}
> +
> +static inline u32 __raw_readl(const volatile void __iomem *addr)
> +{
> +        u32 val;
> +        asm volatile("ldr %1, %0"
> +                     : "+Qo" (*(volatile u32 __force *)addr),
> +                       "=r" (val));
> +        return val;
> +}
> +
> +#define __iormb()               rmb()
> +#define __iowmb()               wmb()
> +
> +#define readb_relaxed(c) ({ u8  __r = __raw_readb(c); __r; })
> +#define readl_relaxed(c) ({ u32 __r = le32_to_cpu((__force __le32) \
> +                                        __raw_readl(c)); __r; })
> +
> +#define writeb_relaxed(v,c)     __raw_writeb(v,c)
> +#define writel_relaxed(v,c)     __raw_writel((__force u32)
cpu_to_le32(v),c)
> +
> +#define readb(c)                ({ u8  __v = readb_relaxed(c); __iormb();
__v; })
> +#define readl(c)                ({ u32 __v = readl_relaxed(c); __iormb();
__v; })
> +
> +#define writeb(v,c)             ({ __iowmb(); writeb_relaxed(v,c); })
> +#define writel(v,c)             ({ __iowmb(); writel_relaxed(v,c); })
> +
> +
>  #endif
>  /*
>   * Local variables:
Theses changes are not in the right place. The asm-arm/io.h must only
contain common code between arm32 and arm64. Your code is arm32
specific, so it should be moved in asm-arm/arm32/io.h.

There is already an implementation for (io)readl and (io)writel with
another name. I will be happy if you rename/alias these 2 functions, it
will avoid ifdef... in ns16550 code.
> diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
> index 9caf776..09ca855 100644
> --- a/xen/include/xen/serial.h
> +++ b/xen/include/xen/serial.h
> @@ -11,6 +11,7 @@
>  
>  #include <xen/init.h>
>  #include <xen/spinlock.h>
> +#include <xen/device_tree.h>
>  
>  struct cpu_user_regs;
>  
> @@ -151,8 +152,10 @@ struct ns16550_defaults {
>      int data_bits; /* default data bits (5, 6, 7 or 8) */
>      int parity;    /* default parity (n, o, e, m or s) */
>      int stop_bits; /* default stop bits (1 or 2) */
> -    int irq;       /* default irq */
> -    unsigned long io_base; /* default io_base address */
> +    struct dt_irq irq;     /* default irq */
This change will break build on x86. This structure is filled by
arch/x86/setup.c
> +    u64 io_base;   /* default io_base address */
> +    u64 io_size;
> +    u32 clock_hz;  /* UART clock rate */
>  };
As I understand, you use these modifications only in ns16550.c to fill
it with device tree value. Considering the number of ifdef you have
added in ns16550_init, I think it''s better to clone the function and
directly fill ns16550, instead of ns16550_defaults. Anyone got any other
thoughts?
>  void ns16550_init(int index, struct ns16550_defaults *defaults);
>  void ehci_dbgp_init(void);
> 
Cheers,

-- 
Julien

Chen Baozi

2013-Jul-22 12:59 UTC

head link

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

On Jul 19, 2013, at 11:18 PM, Chen Baozi <baozich@gmail.com> wrote:
> On Fri, Jul 19, 2013 at 10:15 PM, Ian Campbell
<Ian.Campbell@citrix.com> wrote:
>> 
>> On Fri, 2013-07-19 at 20:57 +0800, Chen Baozi wrote:
>>> Previous bits setting would cause generating THRE interrupts, which
cannot
>>> be cleared and make system loop in gic_interrupt endlessly. Set
''received
>>> data available'' and ''receiver line
status'' bits of IER when enabling
>>> interrupts as what Linux 8250 driver does.
>> 
>> We do actually want THRE interrupts though, don''t we?
Otherwise how do
>> we know when we can send more stuff?
> 
> I''m not very sure about it. In Linux 8250 driver THRE is not set
at this stage.
> And following it does work for OMAP5. I''ll try to dig it next to
see
> whether this fix is reasonable.
I rechecked the Linux codes today. In Linux, THRE is only enabled during tx
(enable at start_tx and disable at stop_tx) when enabling Modem Status
interrupt.
Every time when handling irq, the Linux driver will check the MSR. If Modem
Status interrupt is enabled and delta-CTS flag is set, it will enter the codes
switching THRE status (by start_tx or stop_tx). So at least, I think, THRE
should
not be enabled all the time once 8250 driver is initialized. Since Xen UART
driver
doesn''t seem to be complex as Linux does, I haven''t found a
proper pointer to insert
"start_tx/stop_tx" logic in Xen UART driver yet. And I think THRE
might not be set
at ns16550_setup_postirq actually.

For "how do we know when we can send more stuff?", I think the while
loop in
ns16550_interrupt checking UART_LSR_THRE flag can guarantee it. UART_LSR_THRE
is set when the THR (or transmitter FIFO) and the TSR are both empty regardless
of status of THRE flag in IER register.

BTW, I''ve also re-checked about console_sync and irq type. Neither of
them is
the cause of my issue.

Any ideas?

Cheers,

Baozi
> 
>> 
>> Are you using console_sync? It might mask any issue arising from not
>> getting these interrupts (or maybe even cause your initial issue?)
> 
> Yes. I have tried to shut this down to test whether it is the cause of  my
> issue. But it seems not.
> 
>> 
>> The LSI register is only of use if we care about RTS/CTS or something.
I
>> don''t know if we do, but I''d be surprised if just
asking for the
>> interrupt was sufficient (i.e. we''d need to do something with
the irq)
>> 
>> How did you configure this interrupt line, level or edge trigger?
I''d
>> have expected it to need to be edge, and the issue sounds a bit like it
>> might be level.
> 
> It is high-level read from the DTS. I''ve also changed to it to
edge trigger
> manually and every other possibilities when debugging. And it seems
> edge trigger doesn''t help.
> 
> Anyway, some research must be done for this patch and my fixes to the
> issue next.
> 
> Thanks.
> 
> Baozi
> 
>> 
>> Ian.
>> 
>>> Signed-off-by: Chen Baozi <baozich@gmail.com>
>>> ---
>>> xen/drivers/char/ns16550.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/xen/drivers/char/ns16550.c
b/xen/drivers/char/ns16550.c
>>> index 9248330..60512be 100644
>>> --- a/xen/drivers/char/ns16550.c
>>> +++ b/xen/drivers/char/ns16550.c
>>> @@ -237,8 +237,8 @@ static void ns16550_setup_postirq(struct
ns16550 *uart)
>>>         ns_write_reg(uart,
>>>                      UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR |
UART_MCR_RTS);
>>> 
>>> -        /* Enable receive and transmit interrupts. */
>>> -        ns_write_reg(uart, UART_IER, UART_IER_ERDAI |
UART_IER_ETHREI);
>>> +        /* Enable receive and line status interrupts. */
>>> +        ns_write_reg(uart, UART_IER, UART_IER_ERDAI |
UART_IER_ELSI);
>>>     }
>>> 
>>>     if ( uart->irq >= 0 )
>> 
>>

Chen Baozi

2013-Jul-22 13:06 UTC

head link

Re: [PATCH 4/4] xen/arm: enable ns16550 uart driver for OMAP5432

On Jul 20, 2013, at 1:06 AM, Julien Grall <julien.grall@citrix.com> wrote:
> On 07/19/2013 01:57 PM, Chen Baozi wrote:
>> 
>> Signed-off-by: Chen Baozi <baozich@gmail.com>
>> ---
>> xen/arch/arm/Rules.mk      |   1 +
>> xen/drivers/char/ns16550.c | 155
+++++++++++++++++++++++++++++++++++++++------
>> xen/include/asm-arm/io.h   |  49 ++++++++++++++
>> xen/include/xen/serial.h   |   7 +-
>> 4 files changed, 191 insertions(+), 21 deletions(-)
>> 
>> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
>> index eaa03fb..8604e34 100644
>> --- a/xen/arch/arm/Rules.mk
>> +++ b/xen/arch/arm/Rules.mk
>> @@ -9,6 +9,7 @@
>> HAS_DEVICE_TREE := y
>> HAS_VIDEO := y
>> HAS_ARM_HDLCD := y
>> +HAS_NS16550 := y
>> 
>> CFLAGS += -fno-builtin -fno-common -Wredundant-decls
>> CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe
>> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
>> index 60512be..b8690eb 100644
>> --- a/xen/drivers/char/ns16550.c
>> +++ b/xen/drivers/char/ns16550.c
>> @@ -1,10 +1,10 @@
>>
/******************************************************************************
>>  * ns16550.c
>> - * 
>> + *
> 
> Spurious change.
>>  * Driver for 16550-series UARTs. This driver is to be kept within Xen
as
>>  * it permits debugging of seriously-toasted machines (e.g., in
situations
>>  * where a device driver within a guest OS would be inaccessible).
>> - * 
>> + *
> Spurious change.
> 
>>  * Copyright (c) 2003-2005, K A Fraser
>>  */
>> 
>> @@ -25,6 +25,13 @@
>> #include <asm/fixmap.h>
>> #endif
>> 
>> +#ifdef CONFIG_ARM
>> +#include <xen/device_tree.h>
>> +#include <xen/errno.h>
> 
> The above 2 includes can be moved outside the ifdef.
> 
>> +#include <asm/early_printk.h>
>> +#include <asm/device.h>
>> +#endif
>> +
>> /*
>>  * Configure serial port with a string:
>>  *  
<baud>[/<clock_hz>][,DPS[,<io-base>[,<irq>[,<port-bdf>[,<bridge-bdf>]]]]].
>> @@ -39,8 +46,11 @@ string_param("com1", opt_com1);
>> string_param("com2", opt_com2);
>> 
>> static struct ns16550 {
>> -    int baud, clock_hz, data_bits, parity, stop_bits, fifo_size, irq;
>> -    unsigned long io_base;   /* I/O port or memory-mapped I/O address.
*/
>> +    int baud, data_bits, parity, stop_bits, fifo_size;
>> +    u32 clock_hz;
>> +    struct dt_irq irq;
>> +    u64 io_base;   /* I/O port or memory-mapped I/O address. */
>> +    u64 io_size;
>>     char __iomem *remapped_io_base;  /* Remapped virtual address of
MMIO. */
>>     /* UART with IRQ line: interrupt-driven I/O. */
>>     struct irqaction irqaction;
>> @@ -61,16 +71,24 @@ static struct ns16550 {
>> 
>> static char ns_read_reg(struct ns16550 *uart, int reg)
>> {
>> +#ifdef CONFIG_X86
>>     if ( uart->remapped_io_base == NULL )
>>         return inb(uart->io_base + reg);
>>     return readb(uart->remapped_io_base + reg);
>> +#else
>> +    return readb(uart->remapped_io_base + (reg << 2));
>> +#endif
>> }
>> 
> 
> Is it possible to add a new private field "shift"? So the both
readb can
> be merged.
> 
>> static void ns_write_reg(struct ns16550 *uart, int reg, char c)
>> {
>> +#ifdef CONFIG_X86
>>     if ( uart->remapped_io_base == NULL )
>>         return outb(c, uart->io_base + reg);
>>     writeb(c, uart->remapped_io_base + reg);
>> +#else
>> +    writeb(c, uart->remapped_io_base + (reg << 2));
>> +#endif
> 
> Same here for readb.
> 
>> }
>> 
>> static void ns16550_interrupt(
>> @@ -145,11 +163,12 @@ static int ns16550_getc(struct serial_port *port,
char *pc)
>>     return 1;
>> }
>> 
>> +#ifdef CONFIG_X86
>> static void pci_serial_early_init(struct ns16550 *uart)
>> {
>>     if ( !uart->ps_bdf_enable || uart->io_base >= 0x10000 )
>>         return;
>> -    
>> +
> Spurious change.
> 
>>     if ( uart->pb_bdf_enable )
>>         pci_conf_write16(0, uart->pb_bdf[0], uart->pb_bdf[1],
uart->pb_bdf[2],
>>                          PCI_IO_BASE,
>> @@ -161,7 +180,13 @@ static void pci_serial_early_init(struct ns16550
*uart)
>>                      uart->io_base | PCI_BASE_ADDRESS_SPACE_IO);
>>     pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1],
uart->ps_bdf[2],
>>                      PCI_COMMAND, PCI_COMMAND_IO);
>> +
>> +}
>> +#else
>> +static void pci_serial_early_init(struct ns16550 *uart)
>> +{
>> }
>> +#endif
>> 
>> static void ns16550_setup_preirq(struct ns16550 *uart)
>> {
>> @@ -217,7 +242,9 @@ static void __init ns16550_init_preirq(struct
serial_port *port)
>>         uart->remapped_io_base = (void __iomem *)fix_to_virt(idx);
>>         uart->remapped_io_base += uart->io_base & ~PAGE_MASK;
>> #else
>> -        uart->remapped_io_base = (char *)ioremap(uart->io_base,
8);
>> +        uart->remapped_io_base = ioremap_attr(uart->io_base,
>> +                                              uart->io_size,
>> +                                             
PAGE_HYPERVISOR_NOCACHE);
>> #endif
>>     }
>> 
>> @@ -231,7 +258,7 @@ static void __init ns16550_init_preirq(struct
serial_port *port)
>> 
>> static void ns16550_setup_postirq(struct ns16550 *uart)
>> {
>> -    if ( uart->irq > 0 )
>> +    if ( uart->irq.irq > 0 )
>>     {
>>         /* Master interrupt enable; also keep DTR/RTS asserted. */
>>         ns_write_reg(uart,
>> @@ -241,7 +268,7 @@ static void ns16550_setup_postirq(struct ns16550
*uart)
>>         ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ELSI);
>>     }
>> 
>> -    if ( uart->irq >= 0 )
>> +    if ( uart->irq.irq >= 0 )
>>         set_timer(&uart->timer, NOW() +
MILLISECS(uart->timeout_ms));
>> }
>> 
>> @@ -250,7 +277,7 @@ static void __init ns16550_init_postirq(struct
serial_port *port)
>>     struct ns16550 *uart = port->uart;
>>     int rc, bits;
>> 
>> -    if ( uart->irq < 0 )
>> +    if ( uart->irq.irq < 0 )
>>         return;
>> 
>>     serial_async_transmit(port);
>> @@ -262,20 +289,26 @@ static void __init ns16550_init_postirq(struct
serial_port *port)
>>     uart->timeout_ms = max_t(
>>         unsigned int, 1, (bits * uart->fifo_size * 1000) /
uart->baud);
>> 
>> -    if ( uart->irq > 0 )
>> +    if ( uart->irq.irq > 0 )
>>     {
>>         uart->irqaction.handler = ns16550_interrupt;
>>         uart->irqaction.name    = "ns16550";
>>         uart->irqaction.dev_id  = port;
>> -        if ( (rc = setup_irq(uart->irq, &uart->irqaction))
!= 0 )
>> -            printk("ERROR: Failed to allocate ns16550 IRQ
%d\n", uart->irq);
>> +#ifdef CONFIG_X86
>> +        if ( (rc = setup_irq(uart->irq.irq,
&uart->irqaction)) != 0 )
>> +#else
>> +        if ( (rc = setup_dt_irq(&uart->irq,
&uart->irqaction)) != 0 )
>> +#endif
>> +            printk("ERROR: Failed to allocate ns16550 IRQ
%d\n", uart->irq.irq);
>>     }
>> 
>>     ns16550_setup_postirq(uart);
>> 
>> +#ifdef CONFIG_X86
>>     if ( uart->bar || uart->ps_bdf_enable )
>>         pci_hide_device(uart->ps_bdf[0],
PCI_DEVFN(uart->ps_bdf[1],
>>                                                   
uart->ps_bdf[2]));
>> +#endif
>> }
>> 
>> static void ns16550_suspend(struct serial_port *port)
>> @@ -284,13 +317,16 @@ static void ns16550_suspend(struct serial_port
*port)
>> 
>>     stop_timer(&uart->timer);
>> 
>> +#ifdef CONFIG_X86
>>     if ( uart->bar )
>>        uart->cr = pci_conf_read16(0, uart->ps_bdf[0],
uart->ps_bdf[1],
>>                                   uart->ps_bdf[2], PCI_COMMAND);
>> +#endif
>> }
>> 
>> static void _ns16550_resume(struct serial_port *port)
>> {
>> +#ifdef CONFIG_X86
>>     struct ns16550 *uart = port->uart;
>> 
>>     if ( uart->bar )
>> @@ -300,6 +336,7 @@ static void _ns16550_resume(struct serial_port
*port)
>>        pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1],
uart->ps_bdf[2],
>>                         PCI_COMMAND, uart->cr);
>>     }
>> +#endif
>> 
>>     ns16550_setup_preirq(port->uart);
>>     ns16550_setup_postirq(port->uart);
>> @@ -370,9 +407,17 @@ static void __init ns16550_endboot(struct
serial_port *port)
>> static int __init ns16550_irq(struct serial_port *port)
>> {
>>     struct ns16550 *uart = port->uart;
>> -    return ((uart->irq > 0) ? uart->irq : -1);
>> +    return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
>> }
>> 
>> +#ifdef CONFIG_ARM
>> +static const struct dt_irq __init *ns16550_dt_irq(struct serial_port
*port)
>> +{
>> +    struct ns16550 *uart = port->uart;
>> +    return &uart->irq;
>> +}
>> +#endif
> 
> You don''t need to surround this function by ifdef.
> 
>> static struct uart_driver __read_mostly ns16550_driver = {
>>     .init_preirq  = ns16550_init_preirq,
>>     .init_postirq = ns16550_init_postirq,
>> @@ -382,7 +427,10 @@ static struct uart_driver __read_mostly
ns16550_driver = {
>>     .tx_ready     = ns16550_tx_ready,
>>     .putc         = ns16550_putc,
>>     .getc         = ns16550_getc,
>> -    .irq          = ns16550_irq
>> +    .irq          = ns16550_irq,
>> +#ifdef CONFIG_ARM
>> +    .dt_irq_get   = ns16550_dt_irq
>> +#endif
>> };
> 
> same here.
> 
>> static int __init parse_parity_char(int c)
>> @@ -403,6 +451,7 @@ static int __init parse_parity_char(int c)
>>     return 0;
>> }
>> 
>> +#ifdef CONFIG_X86
>> static void __init parse_pci_bdf(const char **conf, unsigned int
bdf[3])
>> {
>>     bdf[0] = simple_strtoul(*conf, conf, 16);
>> @@ -415,6 +464,7 @@ static void __init parse_pci_bdf(const char **conf,
unsigned int bdf[3])
>>     (*conf)++;
>>     bdf[2] = simple_strtoul(*conf, conf, 16);
>> }
>> +#endif
>> 
>> static int __init check_existence(struct ns16550 *uart)
>> {
>> @@ -428,7 +478,7 @@ static int __init check_existence(struct ns16550
*uart)
>>         return 1;
>> 
>>     pci_serial_early_init(uart);
>> -    
>> +
> 
> spurious change.
> 
>>     /*
>>      * Do a simple existence test first; if we fail this,
>>      * there''s no point trying anything else.
>> @@ -445,7 +495,7 @@ static int __init check_existence(struct ns16550
*uart)
>>     scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
>>     ns_write_reg(uart, UART_IER, scratch);
>>     if ( (scratch2 != 0) || (scratch3 != 0x0F) )
>> -        return 0;
>> +            return 0;
> 
> spurious change.
> 
>> 
>>     /*
>>      * Check to see if a UART is really there.
>> @@ -456,6 +506,7 @@ static int __init check_existence(struct ns16550
*uart)
>>     return (status == 0x90);
>> }
>> 
>> +#ifdef CONFIG_X86
>> static int
>> pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
>> {
>> @@ -507,7 +558,7 @@ pci_uart_config (struct ns16550 *uart, int
skip_amt, int bar_idx)
>>                 uart->bar = bar;
>>                 uart->bar_idx = bar_idx;
>>                 uart->io_base = bar &
~PCI_BASE_ADDRESS_SPACE_IO;
>> -                uart->irq = pci_conf_read8(0, b, d, f,
PCI_INTERRUPT_PIN) ?
>> +                uart->irq.irq = pci_conf_read8(0, b, d, f,
PCI_INTERRUPT_PIN) ?
>>                     pci_conf_read8(0, b, d, f, PCI_INTERRUPT_LINE) : 0;
>> 
>>                 return 0;
>> @@ -519,11 +570,12 @@ pci_uart_config (struct ns16550 *uart, int
skip_amt, int bar_idx)
>>         return -1;
>> 
>>     uart->io_base = 0x3f8;
>> -    uart->irq = 0;
>> +    uart->irq.irq = 0;
>>     uart->clock_hz  = UART_CLOCK_HZ;
>> 
>>     return 0;
>> }
>> +#endif
>> 
>> #define PARSE_ERR(_f, _a...)                 \
>>     do {                                     \
>> @@ -534,6 +586,7 @@ pci_uart_config (struct ns16550 *uart, int
skip_amt, int bar_idx)
>> static void __init ns16550_parse_port_config(
>>     struct ns16550 *uart, const char *conf)
>> {
>> +#ifdef CONFIG_X86
>>     int baud;
>> 
>>     /* No user-specified configuration? */
>> @@ -589,7 +642,7 @@ static void __init ns16550_parse_port_config(
>>     }
>> 
>>     if ( *conf == '','' && *++conf !=
'','' )
>> -        uart->irq = simple_strtol(conf, &conf, 10);
>> +        uart->irq.irq = simple_strtol(conf, &conf, 10);
>> 
>>     if ( *conf == '','' && *++conf !=
'','' )
>>     {
>> @@ -604,6 +657,8 @@ static void __init ns16550_parse_port_config(
>>     }
>> 
>>  config_parsed:
>> +#endif
>> +
> 
> You have disabled nearly 80% of the code for ARM. Perhaps you can split
> this function in 2:
> 	- Parsing pci/user configuration
>        - Register the UART
> 
>>     /* Sanity checks. */
>>     if ( (uart->baud != BAUD_AUTO) &&
>>          ((uart->baud < 1200) || (uart->baud > 115200)) )
>> @@ -633,18 +688,80 @@ void __init ns16550_init(int index, struct
ns16550_defaults *defaults)
>>     uart->baud      = (defaults->baud ? :
>>                        console_has((index == 0) ? "com1" :
"com2")
>>                        ? BAUD_AUTO : 0);
>> +#ifdef CONFIG_ARM
>> +    uart->clock_hz = defaults->clock_hz;
>> +#else
>>     uart->clock_hz  = UART_CLOCK_HZ;
>> +#endif
>>     uart->data_bits = defaults->data_bits;
>>     uart->parity    = parse_parity_char(defaults->parity);
>>     uart->stop_bits = defaults->stop_bits;
>>     uart->irq       = defaults->irq;
>>     uart->io_base   = defaults->io_base;
>> +#ifdef CONFIG_ARM
>> +    uart->io_size   = defaults->io_size;
>> +#endif
>> +
>>     /* Default is no transmit FIFO. */
>>     uart->fifo_size = 1;
>> 
>>     ns16550_parse_port_config(uart, (index == 0) ? opt_com1 :
opt_com2);
>> +
>> }
>> 
>> +#ifdef CONFIG_ARM
>> +
>> +static int __init ns16550_dt_init(struct dt_device_node *dev,
>> +                                  const void *data)
>> +{
>> +    struct ns16550_defaults uart;
>> +    int res;
>> +    const __be32 *clkspec;
>> +
>> +    uart.baud      = 115200;
>> +    uart.data_bits = 8;
>> +    uart.parity    = ''n'';
>> +    uart.stop_bits = 1;
>> +
>> +    res = dt_device_get_address(dev, 0, &uart.io_base,
&uart.io_size);
>> +    if (res) {
>> +        early_printk("ns16550: Unable to retrieve the base"
>> +                     " address of the UART\n");
>> +        return res;
>> +    }
>> +
>> +    res = dt_device_get_irq(dev, 0, &uart.irq);
>> +    if (res) {
>> +        early_printk("ns16550: Unable to retrieve the
IRQ\n");
>> +        return res;
>> +    }
>> +
>> +    clkspec = dt_get_property(dev, "clock-frequency", NULL);
>> +    if (clkspec == NULL) {
>> +        early_printk("ns16550: Unable to retrieve the clock
frequency\n");
>> +        return -EINVAL;
>> +    }
>> +    uart.clock_hz = be32_to_cpup(clkspec);
>> +
>> +    ns16550_init(0, &uart);
>> +    dt_device_set_used_by(dev, DOMID_XEN);
>> +
>> +    return 0;
>> +}
>> +
>> +static const char const *ns16550_dt_compat[] __initdata >> +{
>> +    "ti,omap4-uart",
>> +    NULL
>> +};
>> +
>> +DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
>> +    .compatible = ns16550_dt_compat,
>> +    .init = ns16550_dt_init,
>> +DT_DEVICE_END
>> +
>> +#endif /* CONFIG_ARM */
>> +
>> /*
>>  * Local variables:
>>  * mode: C
>> diff --git a/xen/include/asm-arm/io.h b/xen/include/asm-arm/io.h
>> index aea5233..33674bc 100644
>> --- a/xen/include/asm-arm/io.h
>> +++ b/xen/include/asm-arm/io.h
>> @@ -1,6 +1,55 @@
>> #ifndef _ASM_IO_H
>> #define _ASM_IO_H
>> 
>> +static inline void __raw_writeb(u8 val, volatile void __iomem *addr)
>> +{
>> +        asm volatile("strb %1, %0"
>> +                     : "+Qo" (*(volatile u8 __force *)addr)
>> +                     : "r" (val));
>> +}
>> +
>> +static inline void __raw_writel(u32 val, volatile void __iomem *addr)
>> +{
>> +        asm volatile("str %1, %0"
>> +                     : "+Qo" (*(volatile u32 __force *)addr)
>> +                     : "r" (val));
>> +}
>> +
>> +static inline u8 __raw_readb(const volatile void __iomem *addr)
>> +{
>> +        u8 val;
>> +        asm volatile("ldrb %1, %0"
>> +                     : "+Qo" (*(volatile u8 __force *)addr),
>> +                       "=r" (val));
>> +        return val;
>> +}
>> +
>> +static inline u32 __raw_readl(const volatile void __iomem *addr)
>> +{
>> +        u32 val;
>> +        asm volatile("ldr %1, %0"
>> +                     : "+Qo" (*(volatile u32 __force
*)addr),
>> +                       "=r" (val));
>> +        return val;
>> +}
>> +
>> +#define __iormb()               rmb()
>> +#define __iowmb()               wmb()
>> +
>> +#define readb_relaxed(c) ({ u8  __r = __raw_readb(c); __r; })
>> +#define readl_relaxed(c) ({ u32 __r = le32_to_cpu((__force __le32) \
>> +                                        __raw_readl(c)); __r; })
>> +
>> +#define writeb_relaxed(v,c)     __raw_writeb(v,c)
>> +#define writel_relaxed(v,c)     __raw_writel((__force u32)
cpu_to_le32(v),c)
>> +
>> +#define readb(c)                ({ u8  __v = readb_relaxed(c);
__iormb(); __v; })
>> +#define readl(c)                ({ u32 __v = readl_relaxed(c);
__iormb(); __v; })
>> +
>> +#define writeb(v,c)             ({ __iowmb(); writeb_relaxed(v,c); })
>> +#define writel(v,c)             ({ __iowmb(); writel_relaxed(v,c); })
>> +
>> +
>> #endif
>> /*
>>  * Local variables:
> 
> Theses changes are not in the right place. The asm-arm/io.h must only
> contain common code between arm32 and arm64. Your code is arm32
> specific, so it should be moved in asm-arm/arm32/io.h.
> 
> There is already an implementation for (io)readl and (io)writel with
> another name. I will be happy if you rename/alias these 2 functions, it
> will avoid ifdef... in ns16550 code.
> 
>> diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
>> index 9caf776..09ca855 100644
>> --- a/xen/include/xen/serial.h
>> +++ b/xen/include/xen/serial.h
>> @@ -11,6 +11,7 @@
>> 
>> #include <xen/init.h>
>> #include <xen/spinlock.h>
>> +#include <xen/device_tree.h>
> 
>> 
>> struct cpu_user_regs;
>> 
>> @@ -151,8 +152,10 @@ struct ns16550_defaults {
>>     int data_bits; /* default data bits (5, 6, 7 or 8) */
>>     int parity;    /* default parity (n, o, e, m or s) */
>>     int stop_bits; /* default stop bits (1 or 2) */
>> -    int irq;       /* default irq */
>> -    unsigned long io_base; /* default io_base address */
>> +    struct dt_irq irq;     /* default irq */
> 
> This change will break build on x86. This structure is filled by
> arch/x86/setup.c
> 
>> +    u64 io_base;   /* default io_base address */
>> +    u64 io_size;
>> +    u32 clock_hz;  /* UART clock rate */
>> };
> 
> As I understand, you use these modifications only in ns16550.c to fill
> it with device tree value. Considering the number of ifdef you have
> added in ns16550_init, I think it''s better to clone the function
and
> directly fill ns16550, instead of ns16550_defaults. Anyone got any other
> thoughts?
> 
>> void ns16550_init(int index, struct ns16550_defaults *defaults);
>> void ehci_dbgp_init(void);
>> 
> 
> Cheers,
> 
Hi Julien

Thanks for reviewing. I''ll rework my patchset according to your
suggestions
and resend it later.

Cheers,

Baozi

Chen Baozi

2013-Jul-23 11:47 UTC

head link

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

On Jul 22, 2013, at 8:59 PM, Chen Baozi <baozich@gmail.com> wrote:
> 
> On Jul 19, 2013, at 11:18 PM, Chen Baozi <baozich@gmail.com> wrote:
> 
>> On Fri, Jul 19, 2013 at 10:15 PM, Ian Campbell
<Ian.Campbell@citrix.com> wrote:
>>> 
>>> On Fri, 2013-07-19 at 20:57 +0800, Chen Baozi wrote:
>>>> Previous bits setting would cause generating THRE interrupts,
which cannot
>>>> be cleared and make system loop in gic_interrupt endlessly. Set
'received
>>>> data available' and 'receiver line status' bits of
IER when enabling
>>>> interrupts as what Linux 8250 driver does.
>>> 
>>> We do actually want THRE interrupts though, don't we? Otherwise
how do
>>> we know when we can send more stuff?
>> 
>> I'm not very sure about it. In Linux 8250 driver THRE is not set at
this stage.
>> And following it does work for OMAP5. I'll try to dig it next to
see
>> whether this fix is reasonable.
> 
> I rechecked the Linux codes today. In Linux, THRE is only enabled during tx
> (enable at start_tx and disable at stop_tx) when enabling Modem Status
interrupt.
> Every time when handling irq, the Linux driver will check the MSR. If Modem
> Status interrupt is enabled and delta-CTS flag is set, it will enter the
codes
> switching THRE status (by start_tx or stop_tx). So at least, I think, THRE
should
> not be enabled all the time once 8250 driver is initialized. Since Xen UART
driver
> doesn't seem to be complex as Linux does, I haven't found a proper
pointer to insert
> "start_tx/stop_tx" logic in Xen UART driver yet. And I think THRE
might not be set
> at ns16550_setup_postirq actually.
> 
> For "how do we know when we can send more stuff?", I think the
while loop in
> ns16550_interrupt checking UART_LSR_THRE flag can guarantee it.
UART_LSR_THRE
> is set when the THR (or transmitter FIFO) and the TSR are both empty
regardless
> of status of THRE flag in IER register.
> 
> BTW, I've also re-checked about console_sync and irq type. Neither of
them is
> the cause of my issue.
> 
> Any ideas?
According to my test today, when disable sync_console，THRE interrupt is needed
to transmit info. However, enabling it all the time since postirq is not
appropriate because it would keep on interrupting system endless after the tx
is completed.

Baozi


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Chen Baozi

2013-Jul-24 10:04 UTC

head link

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

On Jul 23, 2013, at 7:47 PM, Chen Baozi <baozich@gmail.com> wrote:
> 
> On Jul 22, 2013, at 8:59 PM, Chen Baozi <baozich@gmail.com> wrote:
> 
>> 
>> On Jul 19, 2013, at 11:18 PM, Chen Baozi <baozich@gmail.com>
wrote:
>> 
>>> On Fri, Jul 19, 2013 at 10:15 PM, Ian Campbell
<Ian.Campbell@citrix.com> wrote:
>>>> 
>>>> On Fri, 2013-07-19 at 20:57 +0800, Chen Baozi wrote:
>>>>> Previous bits setting would cause generating THRE
interrupts, which cannot
>>>>> be cleared and make system loop in gic_interrupt endlessly.
Set 'received
>>>>> data available' and 'receiver line status' bits
of IER when enabling
>>>>> interrupts as what Linux 8250 driver does.
>>>> 
>>>> We do actually want THRE interrupts though, don't we?
Otherwise how do
>>>> we know when we can send more stuff?
>>> 
>>> I'm not very sure about it. In Linux 8250 driver THRE is not
set at this stage.
>>> And following it does work for OMAP5. I'll try to dig it next
to see
>>> whether this fix is reasonable.
>> 
>> I rechecked the Linux codes today. In Linux, THRE is only enabled
during tx
>> (enable at start_tx and disable at stop_tx) when enabling Modem Status
interrupt.
>> Every time when handling irq, the Linux driver will check the MSR. If
Modem
>> Status interrupt is enabled and delta-CTS flag is set, it will enter
the codes
>> switching THRE status (by start_tx or stop_tx). So at least, I think,
THRE should
>> not be enabled all the time once 8250 driver is initialized. Since Xen
UART driver
>> doesn't seem to be complex as Linux does, I haven't found a
proper pointer to insert
>> "start_tx/stop_tx" logic in Xen UART driver yet. And I think
THRE might not be set
>> at ns16550_setup_postirq actually.
>> 
>> For "how do we know when we can send more stuff?", I think
the while loop in
>> ns16550_interrupt checking UART_LSR_THRE flag can guarantee it.
UART_LSR_THRE
>> is set when the THR (or transmitter FIFO) and the TSR are both empty
regardless
>> of status of THRE flag in IER register.
>> 
>> BTW, I've also re-checked about console_sync and irq type. Neither
of them is
>> the cause of my issue.
>> 
>> Any ideas?
> 
> According to my test today, when disable sync_console，THRE interrupt is
needed
> to transmit info. However, enabling it all the time since postirq is not
> appropriate because it would keep on interrupting system endless after the
tx
> is completed.
There is a very weird phenomenon of my OMAP5 UART.

According to its manual, when the transmitter FIFO and THRE interrupt are
enabled,
transmit interrupts occur as follows:

1. The transmitter holding register interrupt (IIR & 0xf == 0x2) occurs when
the
transmit FIFO is empty. The transmit FIFO is cleared (IIR & 0xf == 0x1) when
the
THR is written to or the IIR is read.

2. ...

What is weird is that I found the transmit FIFO is not cleared (IIR & 0xf ==
0x2)
no matter I write THR or read IIR.

I add the following piece of codes to test in ns16550_interrupt:

+	while (ns_read_reg(hart, UART_IIR) & 0x2) {
+		ns_write_reg(uart, UART_THR, 'a');
+	}

The result shows that it will never end the loop even if I've both written
THR
and read IIR.

PS, I re-confirmed about the interrupt type in the manual and the latest dts 
description in Linux kernel. It is a level triggered interrupt rather than the
edge.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Chen Baozi

2013-Jul-25 09:14 UTC

head link

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

On Jul 24, 2013, at 6:04 PM, Chen Baozi <baozich@gmail.com> wrote:
> 
> On Jul 23, 2013, at 7:47 PM, Chen Baozi <baozich@gmail.com> wrote:
> 
>> 
>> On Jul 22, 2013, at 8:59 PM, Chen Baozi <baozich@gmail.com>
wrote:
>> 
>>> 
>>> On Jul 19, 2013, at 11:18 PM, Chen Baozi <baozich@gmail.com>
wrote:
>>> 
>>>> On Fri, Jul 19, 2013 at 10:15 PM, Ian Campbell
<Ian.Campbell@citrix.com> wrote:
>>>>> 
>>>>> On Fri, 2013-07-19 at 20:57 +0800, Chen Baozi wrote:
>>>>>> Previous bits setting would cause generating THRE
interrupts, which cannot
>>>>>> be cleared and make system loop in gic_interrupt
endlessly. Set 'received
>>>>>> data available' and 'receiver line status'
bits of IER when enabling
>>>>>> interrupts as what Linux 8250 driver does.
>>>>> 
>>>>> We do actually want THRE interrupts though, don't we?
Otherwise how do
>>>>> we know when we can send more stuff?
>>>> 
>>>> I'm not very sure about it. In Linux 8250 driver THRE is
not set at this stage.
>>>> And following it does work for OMAP5. I'll try to dig it
next to see
>>>> whether this fix is reasonable.
>>> 
>>> I rechecked the Linux codes today. In Linux, THRE is only enabled
during tx
>>> (enable at start_tx and disable at stop_tx) when enabling Modem
Status interrupt.
>>> Every time when handling irq, the Linux driver will check the MSR.
If Modem
>>> Status interrupt is enabled and delta-CTS flag is set, it will
enter the codes
>>> switching THRE status (by start_tx or stop_tx). So at least, I
think, THRE should
>>> not be enabled all the time once 8250 driver is initialized. Since
Xen UART driver
>>> doesn't seem to be complex as Linux does, I haven't found a
proper pointer to insert
>>> "start_tx/stop_tx" logic in Xen UART driver yet. And I
think THRE might not be set
>>> at ns16550_setup_postirq actually.
>>> 
>>> For "how do we know when we can send more stuff?", I
think the while loop in
>>> ns16550_interrupt checking UART_LSR_THRE flag can guarantee it.
UART_LSR_THRE
>>> is set when the THR (or transmitter FIFO) and the TSR are both
empty regardless
>>> of status of THRE flag in IER register.
>>> 
>>> BTW, I've also re-checked about console_sync and irq type.
Neither of them is
>>> the cause of my issue.
>>> 
>>> Any ideas?
>> 
>> According to my test today, when disable sync_console，THRE interrupt is
needed
>> to transmit info. However, enabling it all the time since postirq is
not
>> appropriate because it would keep on interrupting system endless after
the tx
>> is completed.
> 
> There is a very weird phenomenon of my OMAP5 UART.
> 
> According to its manual, when the transmitter FIFO and THRE interrupt are
enabled,
> transmit interrupts occur as follows:
> 
> 1. The transmitter holding register interrupt (IIR & 0xf == 0x2) occurs
when the
> transmit FIFO is empty. The transmit FIFO is cleared (IIR & 0xf == 0x1)
when the
> THR is written to or the IIR is read.
> 
> 2. ...
> 
> What is weird is that I found the transmit FIFO is not cleared (IIR &
0xf == 0x2)
> no matter I write THR or read IIR.
> 
> I add the following piece of codes to test in ns16550_interrupt:
> 
> +	while (ns_read_reg(hart, UART_IIR) & 0x2) {
> +		ns_write_reg(uart, UART_THR, 'a');
> +	}
> 
> The result shows that it will never end the loop even if I've both
written THR
> and read IIR.

According to my test today, this dead loop above only happens in case that I
have
loaded both xen-uImage & zImage(dom0). Once I loaded both two images, reset
the
board and boot the system with xen-uImage only at the same address, the
phenomenon
keeps the same (dead loop). However, if I powered down the board and then start
the system with xen-uImage only, or if I reset the board and load xen-uImage at
the address where zImage should be, this would not happen. Instead, there would
be a bunch of 'a' printed after interrupt enabled. And the following
output info
after interrupt enabled would be print as expected. In other word, the while
loop does clear the transmit FIFO. 

Interestingly, the phenomenon when IER_ETHREI is not enabled is similar. If both
xen-uImage and zImage are loaded at the right address (zImage is loaded at
the address specified in DTS module@0), some external events need to be sent to
trigger the driver to continue outputting infos.

All the test described above is done without 'sync_console' bootargs.

Besides this test, I noticed that we set a timer to poll UART after enabling
the interrupt. And the timer initialization codes are platform specific, which
should be implemented in xen/arch/arm/platforms/omap5.c for example. However,
I haven't done this yet. (I used to plan to turn to this work after UART
porting
has been done.) Is it a possible factor that may cause my issues?

Any ideas?

Cheers,

Baozi
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Julien Grall

2013-Jul-25 11:17 UTC

head link

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

On 07/25/2013 10:14 AM, Chen Baozi wrote:
> Besides this test, I noticed that we set a timer to poll UART after
enabling
> the interrupt. And the timer initialization codes are platform specific,
which
> should be implemented in xen/arch/arm/platforms/omap5.c for example.
However,
> I haven''t done this yet. (I used to plan to turn to this work
after UART porting
> has been done.) Is it a possible factor that may cause my issues?
Xen uses the arch timers to handle the time. On some platform (for
instance the Arndale), it''s not enabled by default.

If the omap5 boot with the arch timers enabled, you don''t need to
implement the init_time callback.

Do you know if Xen receives timer interrupts?

-- 
Julien

Chen Baozi

2013-Jul-25 11:31 UTC

head link

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

On Jul 25, 2013, at 7:17 PM, Julien Grall <julien.grall@linaro.org> wrote:
> On 07/25/2013 10:14 AM, Chen Baozi wrote:
> 
>> Besides this test, I noticed that we set a timer to poll UART after
enabling
>> the interrupt. And the timer initialization codes are platform
specific, which
>> should be implemented in xen/arch/arm/platforms/omap5.c for example.
However,
>> I haven''t done this yet. (I used to plan to turn to this work
after UART porting
>> has been done.) Is it a possible factor that may cause my issues?
> 
> Xen uses the arch timers to handle the time. On some platform (for
> instance the Arndale), it''s not enabled by default.
> 
> If the omap5 boot with the arch timers enabled, you don''t need to
> implement the init_time callback.
> 
> Do you know if Xen receives timer interrupts?
Any way that I could confirm this?

There is a line of boot message:

"Using generic timer at 0 KHz."

Julien Grall

2013-Jul-25 12:33 UTC

head link

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

On 07/25/2013 12:31 PM, Chen Baozi wrote:> 
> On Jul 25, 2013, at 7:17 PM, Julien Grall <julien.grall@linaro.org>
wrote:
> 
>> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>>
>>> Besides this test, I noticed that we set a timer to poll UART after
enabling
>>> the interrupt. And the timer initialization codes are platform
specific, which
>>> should be implemented in xen/arch/arm/platforms/omap5.c for
example. However,
>>> I haven''t done this yet. (I used to plan to turn to this
work after UART porting
>>> has been done.) Is it a possible factor that may cause my issues?
>>
>> Xen uses the arch timers to handle the time. On some platform (for
>> instance the Arndale), it''s not enabled by default.
>>
>> If the omap5 boot with the arch timers enabled, you don''t need
to
>> implement the init_time callback.
>>
>> Do you know if Xen receives timer interrupts?
> 
> Any way that I could confirm this?
> 
> There is a line of boot message:
> 
> "Using generic timer at 0 KHz."
> 
It seems the arch timer is not configured/enabled. Do you know if U-boot
enables it?

It''s not clear to me, how you need to implement init_time. Do you have
a
datasheet with the board? If yes, is there a section for the section timer?

Perhaps the good start is linux/arch/arm/mach-omap2/timer.c.

-- 
Julien

Chen Baozi

2013-Jul-26 01:14 UTC

head link

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

On Jul 25, 2013, at 8:33 PM, Julien Grall <julien.grall@linaro.org> wrote:
> On 07/25/2013 12:31 PM, Chen Baozi wrote:
>> 
>> On Jul 25, 2013, at 7:17 PM, Julien Grall
<julien.grall@linaro.org> wrote:
>> 
>>> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>>> 
>>>> Besides this test, I noticed that we set a timer to poll UART
after enabling
>>>> the interrupt. And the timer initialization codes are platform
specific, which
>>>> should be implemented in xen/arch/arm/platforms/omap5.c for
example. However,
>>>> I haven''t done this yet. (I used to plan to turn to
this work after UART porting
>>>> has been done.) Is it a possible factor that may cause my
issues?
>>> 
>>> Xen uses the arch timers to handle the time. On some platform (for
>>> instance the Arndale), it''s not enabled by default.
>>> 
>>> If the omap5 boot with the arch timers enabled, you don''t
need to
>>> implement the init_time callback.
>>> 
>>> Do you know if Xen receives timer interrupts?
>> 
>> Any way that I could confirm this?
>> 
>> There is a line of boot message:
>> 
>> "Using generic timer at 0 KHz."
>> 
> 
> It seems the arch timer is not configured/enabled. Do you know if U-boot
> enables it?
I could see clocks.c/clocks.o in its u-boot sources, but I don''t think
U-boot has already enabled it, for I could read such lines from Linux kernel
boot messages:

"""
OMAP clockevent source: GPTIMER1 at 32768 Hz
sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999s
OMAP clocksource: 32k_counter at 32768 Hz
arch_timer: No interrupt available, giving up
omap5_realtime_timer_init: arch_timer_register failed -22
> 
> It''s not clear to me, how you need to implement init_time. Do you
have a
> datasheet with the board? If yes, is there a section for the section timer?
Yes, TI has published an 49.6MB Technical Reference Manual online with 88 pages
section about timer.
> 
> Perhaps the good start is linux/arch/arm/mach-omap2/timer.c.
Thanks a lot, it should be helpful!

Baozi

Chen Baozi

2013-Jul-26 03:31 UTC

head link

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

On Jul 26, 2013, at 9:14 AM, Chen Baozi <baozich@gmail.com> wrote:
> 
> On Jul 25, 2013, at 8:33 PM, Julien Grall <julien.grall@linaro.org>
wrote:
> 
>> On 07/25/2013 12:31 PM, Chen Baozi wrote:
>>> 
>>> On Jul 25, 2013, at 7:17 PM, Julien Grall
<julien.grall@linaro.org> wrote:
>>> 
>>>> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>>>> 
>>>>> Besides this test, I noticed that we set a timer to poll
UART after enabling
>>>>> the interrupt. And the timer initialization codes are
platform specific, which
>>>>> should be implemented in xen/arch/arm/platforms/omap5.c for
example. However,
>>>>> I haven''t done this yet. (I used to plan to turn
to this work after UART porting
>>>>> has been done.) Is it a possible factor that may cause my
issues?
>>>> 
>>>> Xen uses the arch timers to handle the time. On some platform
(for
>>>> instance the Arndale), it''s not enabled by default.
>>>> 
>>>> If the omap5 boot with the arch timers enabled, you
don''t need to
>>>> implement the init_time callback.
>>>> 
>>>> Do you know if Xen receives timer interrupts?
>>> 
>>> Any way that I could confirm this?
>>> 
>>> There is a line of boot message:
>>> 
>>> "Using generic timer at 0 KHz."
>>> 
>> 
>> It seems the arch timer is not configured/enabled. Do you know if
U-boot
>> enables it?
> 
> I could see clocks.c/clocks.o in its u-boot sources, but I don''t
think U-boot has already enabled it, for I could read such lines from Linux
kernel boot messages:
> 
> """
> OMAP clockevent source: GPTIMER1 at 32768 Hz
> sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999s
> OMAP clocksource: 32k_counter at 32768 Hz
> arch_timer: No interrupt available, giving up
> omap5_realtime_timer_init: arch_timer_register failed -22
> 
>> 
>> It''s not clear to me, how you need to implement init_time. Do
you have a
>> datasheet with the board? If yes, is there a section for the section
timer?
> 
> Yes, TI has published an 49.6MB Technical Reference Manual online with 88
pages section about timer.
Wait. I think I might make a mistake. The "arch timer" is different
from "timer" as devices, right? The timer section in Technical
Reference Manual seems to be the latter one, for it is  connect to the Level-4
interconnect of OMAP5432 and described as "ti,omap5430-timer" in DTS.
I think the "arch timer" you mentioned should refer to
"arm,armv7-timer", right?
> 
>> 
>> Perhaps the good start is linux/arch/arm/mach-omap2/timer.c.
> 
> Thanks a lot, it should be helpful!
> 
> Baozi
>

Chen Baozi

2013-Jul-26 09:30 UTC

head link

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

On Jul 26, 2013, at 11:31 AM, Chen Baozi <baozich@gmail.com> wrote:
> 
> On Jul 26, 2013, at 9:14 AM, Chen Baozi <baozich@gmail.com> wrote:
> 
>> 
>> On Jul 25, 2013, at 8:33 PM, Julien Grall
<julien.grall@linaro.org> wrote:
>> 
>>> On 07/25/2013 12:31 PM, Chen Baozi wrote:
>>>> 
>>>> On Jul 25, 2013, at 7:17 PM, Julien Grall
<julien.grall@linaro.org> wrote:
>>>> 
>>>>> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>>>>> 
>>>>>> Besides this test, I noticed that we set a timer to
poll UART after enabling
>>>>>> the interrupt. And the timer initialization codes are
platform specific, which
>>>>>> should be implemented in xen/arch/arm/platforms/omap5.c
for example. However,
>>>>>> I haven''t done this yet. (I used to plan to
turn to this work after UART porting
>>>>>> has been done.) Is it a possible factor that may cause
my issues?
>>>>> 
>>>>> Xen uses the arch timers to handle the time. On some
platform (for
>>>>> instance the Arndale), it''s not enabled by
default.
>>>>> 
>>>>> If the omap5 boot with the arch timers enabled, you
don''t need to
>>>>> implement the init_time callback.
>>>>> 
>>>>> Do you know if Xen receives timer interrupts?
>>>> 
>>>> Any way that I could confirm this?
>>>> 
>>>> There is a line of boot message:
>>>> 
>>>> "Using generic timer at 0 KHz."
>>>> 
>>> 
>>> It seems the arch timer is not configured/enabled. Do you know if
U-boot
>>> enables it?
>> 
>> I could see clocks.c/clocks.o in its u-boot sources, but I
don''t think U-boot has already enabled it, for I could read such lines
from Linux kernel boot messages:
>> 
>> """
>> OMAP clockevent source: GPTIMER1 at 32768 Hz
>> sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every
131071999s
>> OMAP clocksource: 32k_counter at 32768 Hz
>> arch_timer: No interrupt available, giving up
>> omap5_realtime_timer_init: arch_timer_register failed -22
>> 
>>> 
>>> It''s not clear to me, how you need to implement init_time.
Do you have a
>>> datasheet with the board? If yes, is there a section for the
section timer?
>> 
>> Yes, TI has published an 49.6MB Technical Reference Manual online with
88 pages section about timer.
> 
> Wait. I think I might make a mistake. The "arch timer" is
different from "timer" as devices, right? The timer section in
Technical Reference Manual seems to be the latter one, for it is  connect to the
Level-4 interconnect of OMAP5432 and described as "ti,omap5430-timer"
in DTS. I think the "arch timer" you mentioned should refer to
"arm,armv7-timer", right?
According to my understanding, the "arch timer" refers to the Generic
Timer inside the Cortex-A15 MPCore. And it is configured through the
memory-mapped interface. Right?
> 
>> 
>>> 
>>> Perhaps the good start is linux/arch/arm/mach-omap2/timer.c.
>> 
>> Thanks a lot, it should be helpful!
>> 
>> Baozi
>> 
>

Julien Grall

2013-Jul-26 10:42 UTC

head link

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

On 07/26/2013 04:31 AM, Chen Baozi wrote:> 
> On Jul 26, 2013, at 9:14 AM, Chen Baozi <baozich@gmail.com> wrote:
> 
>>
>> On Jul 25, 2013, at 8:33 PM, Julien Grall
<julien.grall@linaro.org> wrote:
>>
>>> On 07/25/2013 12:31 PM, Chen Baozi wrote:
>>>>
>>>> On Jul 25, 2013, at 7:17 PM, Julien Grall
<julien.grall@linaro.org> wrote:
>>>>
>>>>> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>>>>>
>>>>>> Besides this test, I noticed that we set a timer to
poll UART after enabling
>>>>>> the interrupt. And the timer initialization codes are
platform specific, which
>>>>>> should be implemented in xen/arch/arm/platforms/omap5.c
for example. However,
>>>>>> I haven''t done this yet. (I used to plan to
turn to this work after UART porting
>>>>>> has been done.) Is it a possible factor that may cause
my issues?
>>>>>
>>>>> Xen uses the arch timers to handle the time. On some
platform (for
>>>>> instance the Arndale), it''s not enabled by
default.
>>>>>
>>>>> If the omap5 boot with the arch timers enabled, you
don''t need to
>>>>> implement the init_time callback.
>>>>>
>>>>> Do you know if Xen receives timer interrupts?
>>>>
>>>> Any way that I could confirm this?
>>>>
>>>> There is a line of boot message:
>>>>
>>>> "Using generic timer at 0 KHz."
>>>>
>>>
>>> It seems the arch timer is not configured/enabled. Do you know if
U-boot
>>> enables it?
>>
>> I could see clocks.c/clocks.o in its u-boot sources, but I
don''t think U-boot has already enabled it, for I could read such lines
from Linux kernel boot messages:
Are you trying to boot Linux on bare metal or as dom0?
>> """
>> OMAP clockevent source: GPTIMER1 at 32768 Hz
>> sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every
131071999s
>> OMAP clocksource: 32k_counter at 32768 Hz
>> arch_timer: No interrupt available, giving up
Do you have a "arm,armv7-timer" compatible node in your DTS with
interrupts? Here Linux can''t find an appropriate interrupt.
>> omap5_realtime_timer_init: arch_timer_register failed -22
>>
>>>
>>> It''s not clear to me, how you need to implement init_time.
Do you have a
>>> datasheet with the board? If yes, is there a section for the
section timer?
>>
>> Yes, TI has published an 49.6MB Technical Reference Manual online with
88 pages section about timer.
> 
> Wait. I think I might make a mistake. The "arch timer" is
different from "timer" as devices, right? The timer section in
Technical Reference Manual seems to be the latter one, for it is  connect to the
Level-4 interconnect of OMAP5432 and described as "ti,omap5430-timer"
in DTS. I think the "arch timer" you mentioned should refer to
"arm,armv7-timer", right?
By "arch timer" I mean "arm,armv7-timer".

-- 
Julien

Chen Baozi

2013-Jul-26 11:01 UTC

head link

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

On Jul 26, 2013, at 6:42 PM, Julien Grall <julien.grall@linaro.org> wrote:
> On 07/26/2013 04:31 AM, Chen Baozi wrote:
>> 
>> On Jul 26, 2013, at 9:14 AM, Chen Baozi <baozich@gmail.com>
wrote:
>> 
>>> 
>>> On Jul 25, 2013, at 8:33 PM, Julien Grall
<julien.grall@linaro.org> wrote:
>>> 
>>>> On 07/25/2013 12:31 PM, Chen Baozi wrote:
>>>>> 
>>>>> On Jul 25, 2013, at 7:17 PM, Julien Grall
<julien.grall@linaro.org> wrote:
>>>>> 
>>>>>> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>>>>>> 
>>>>>>> Besides this test, I noticed that we set a timer to
poll UART after enabling
>>>>>>> the interrupt. And the timer initialization codes
are platform specific, which
>>>>>>> should be implemented in
xen/arch/arm/platforms/omap5.c for example. However,
>>>>>>> I haven''t done this yet. (I used to plan
to turn to this work after UART porting
>>>>>>> has been done.) Is it a possible factor that may
cause my issues?
>>>>>> 
>>>>>> Xen uses the arch timers to handle the time. On some
platform (for
>>>>>> instance the Arndale), it''s not enabled by
default.
>>>>>> 
>>>>>> If the omap5 boot with the arch timers enabled, you
don''t need to
>>>>>> implement the init_time callback.
>>>>>> 
>>>>>> Do you know if Xen receives timer interrupts?
>>>>> 
>>>>> Any way that I could confirm this?
>>>>> 
>>>>> There is a line of boot message:
>>>>> 
>>>>> "Using generic timer at 0 KHz."
>>>>> 
>>>> 
>>>> It seems the arch timer is not configured/enabled. Do you know
if U-boot
>>>> enables it?
>>> 
>>> I could see clocks.c/clocks.o in its u-boot sources, but I
don''t think U-boot has already enabled it, for I could read such lines
from Linux kernel boot messages:
> 
> Are you trying to boot Linux on bare metal or as dom0?
bare metal.
> 
>>> """
>>> OMAP clockevent source: GPTIMER1 at 32768 Hz
>>> sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every
131071999s
>>> OMAP clocksource: 32k_counter at 32768 Hz
>>> arch_timer: No interrupt available, giving up
> 
> Do you have a "arm,armv7-timer" compatible node in your DTS with
> interrupts? Here Linux can''t find an appropriate interrupt.
You reminds me that the DTS shipped along with the board has issues on
"arm,armv7-timer". I have corrected it for xen but leave the bare
metal booting dtb the original status.
> 
>>> omap5_realtime_timer_init: arch_timer_register failed -22
>>> 
>>>> 
>>>> It''s not clear to me, how you need to implement
init_time. Do you have a
>>>> datasheet with the board? If yes, is there a section for the
section timer?
>>> 
>>> Yes, TI has published an 49.6MB Technical Reference Manual online
with 88 pages section about timer.
>> 
>> Wait. I think I might make a mistake. The "arch timer" is
different from "timer" as devices, right? The timer section in
Technical Reference Manual seems to be the latter one, for it is  connect to the
Level-4 interconnect of OMAP5432 and described as "ti,omap5430-timer"
in DTS. I think the "arch timer" you mentioned should refer to
"arm,armv7-timer", right?
> 
> By "arch timer" I mean "arm,armv7-timer".
Thanks. So I think the codes to start with it is
linux/drivers/clocksource/arm_arch_timer.c: arch_timer_init(). The
linux/arch/arm/mach-omap2/timer.c is for the timer device
"ti,omap543-timer".

Again, Thanks a lot.

Baozi

Julien Grall

2013-Jul-26 12:01 UTC

head link

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

On 07/26/2013 12:01 PM, Chen Baozi wrote:> 
> On Jul 26, 2013, at 6:42 PM, Julien Grall <julien.grall@linaro.org>
wrote:
> 
>> On 07/26/2013 04:31 AM, Chen Baozi wrote:
>>>
>>> On Jul 26, 2013, at 9:14 AM, Chen Baozi <baozich@gmail.com>
wrote:
>>>
>>>>
>>>> On Jul 25, 2013, at 8:33 PM, Julien Grall
<julien.grall@linaro.org> wrote:
>>>>
>>>>> On 07/25/2013 12:31 PM, Chen Baozi wrote:
>>>>>>
>>>>>> On Jul 25, 2013, at 7:17 PM, Julien Grall
<julien.grall@linaro.org> wrote:
>>>>>>
>>>>>>> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>>>>>>>
>>>>>>>> Besides this test, I noticed that we set a
timer to poll UART after enabling
>>>>>>>> the interrupt. And the timer initialization
codes are platform specific, which
>>>>>>>> should be implemented in
xen/arch/arm/platforms/omap5.c for example. However,
>>>>>>>> I haven''t done this yet. (I used to
plan to turn to this work after UART porting
>>>>>>>> has been done.) Is it a possible factor that
may cause my issues?
>>>>>>>
>>>>>>> Xen uses the arch timers to handle the time. On
some platform (for
>>>>>>> instance the Arndale), it''s not enabled by
default.
>>>>>>>
>>>>>>> If the omap5 boot with the arch timers enabled, you
don''t need to
>>>>>>> implement the init_time callback.
>>>>>>>
>>>>>>> Do you know if Xen receives timer interrupts?
>>>>>>
>>>>>> Any way that I could confirm this?
>>>>>>
>>>>>> There is a line of boot message:
>>>>>>
>>>>>> "Using generic timer at 0 KHz."
>>>>>>
>>>>>
>>>>> It seems the arch timer is not configured/enabled. Do you
know if U-boot
>>>>> enables it?
>>>>
>>>> I could see clocks.c/clocks.o in its u-boot sources, but I
don''t think U-boot has already enabled it, for I could read such lines
from Linux kernel boot messages:
>>
>> Are you trying to boot Linux on bare metal or as dom0?
> 
> bare metal.
> 
>>
>>>> """
>>>> OMAP clockevent source: GPTIMER1 at 32768 Hz
>>>> sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every
131071999s
>>>> OMAP clocksource: 32k_counter at 32768 Hz
>>>> arch_timer: No interrupt available, giving up
>>
>> Do you have a "arm,armv7-timer" compatible node in your DTS
with
>> interrupts? Here Linux can''t find an appropriate interrupt.
> 
> You reminds me that the DTS shipped along with the board has issues on
"arm,armv7-timer". I have corrected it for xen but leave the bare
metal booting dtb the original status.
> 
>>
>>>> omap5_realtime_timer_init: arch_timer_register failed -22
>>>>
>>>>>
>>>>> It''s not clear to me, how you need to implement
init_time. Do you have a
>>>>> datasheet with the board? If yes, is there a section for
the section timer?
>>>>
>>>> Yes, TI has published an 49.6MB Technical Reference Manual
online with 88 pages section about timer.
>>>
>>> Wait. I think I might make a mistake. The "arch timer" is
different from "timer" as devices, right? The timer section in
Technical Reference Manual seems to be the latter one, for it is  connect to the
Level-4 interconnect of OMAP5432 and described as "ti,omap5430-timer"
in DTS. I think the "arch timer" you mentioned should refer to
"arm,armv7-timer", right?
>>
>> By "arch timer" I mean "arm,armv7-timer".
> 
> Thanks. So I think the codes to start with it is
linux/drivers/clocksource/arm_arch_timer.c: arch_timer_init(). The
linux/arch/arm/mach-omap2/timer.c is for the timer device
"ti,omap543-timer".
arch_timer_init is already implemented in init_xen_time
(xen/arch/arm/time.c).
On some board, the arch timer is not enabled by default and need a
"workaround" for this purpose.

For instance on the Arndale, we need to enable the MCTimer. I think you
need to do something like that for the OMAP.

Do you know if the boot CPU already start in HYP mode? If no, did you
set up correctly the timer in mode_switch.S?

-- 
Julien

Chen Baozi

2013-Jul-26 12:32 UTC

head link

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

在 2013-7-26，20:01，Julien Grall <julien.grall@linaro.org> 写道：
> On 07/26/2013 12:01 PM, Chen Baozi wrote:
>> 
>> On Jul 26, 2013, at 6:42 PM, Julien Grall
<julien.grall@linaro.org> wrote:
>> 
>>> On 07/26/2013 04:31 AM, Chen Baozi wrote:
>>>> 
>>>> On Jul 26, 2013, at 9:14 AM, Chen Baozi
<baozich@gmail.com> wrote:
>>>> 
>>>>> 
>>>>> On Jul 25, 2013, at 8:33 PM, Julien Grall
<julien.grall@linaro.org> wrote:
>>>>> 
>>>>>> On 07/25/2013 12:31 PM, Chen Baozi wrote:
>>>>>>> 
>>>>>>> On Jul 25, 2013, at 7:17 PM, Julien Grall
<julien.grall@linaro.org> wrote:
>>>>>>> 
>>>>>>>> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>>>>>>>> 
>>>>>>>>> Besides this test, I noticed that we set a
timer to poll UART after enabling
>>>>>>>>> the interrupt. And the timer initialization
codes are platform specific, which
>>>>>>>>> should be implemented in
xen/arch/arm/platforms/omap5.c for example. However,
>>>>>>>>> I haven't done this yet. (I used to
plan to turn to this work after UART porting
>>>>>>>>> has been done.) Is it a possible factor
that may cause my issues?
>>>>>>>> 
>>>>>>>> Xen uses the arch timers to handle the time. On
some platform (for
>>>>>>>> instance the Arndale), it's not enabled by
default.
>>>>>>>> 
>>>>>>>> If the omap5 boot with the arch timers enabled,
you don't need to
>>>>>>>> implement the init_time callback.
>>>>>>>> 
>>>>>>>> Do you know if Xen receives timer interrupts?
>>>>>>> 
>>>>>>> Any way that I could confirm this?
>>>>>>> 
>>>>>>> There is a line of boot message:
>>>>>>> 
>>>>>>> "Using generic timer at 0 KHz."
>>>>>> 
>>>>>> It seems the arch timer is not configured/enabled. Do
you know if U-boot
>>>>>> enables it?
>>>>> 
>>>>> I could see clocks.c/clocks.o in its u-boot sources, but I
don't think U-boot has already enabled it, for I could read such lines from
Linux kernel boot messages:
>>> 
>>> Are you trying to boot Linux on bare metal or as dom0?
>> 
>> bare metal.
>> 
>>> 
>>>>> """
>>>>> OMAP clockevent source: GPTIMER1 at 32768 Hz
>>>>> sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps
every 131071999s
>>>>> OMAP clocksource: 32k_counter at 32768 Hz
>>>>> arch_timer: No interrupt available, giving up
>>> 
>>> Do you have a "arm,armv7-timer" compatible node in your
DTS with
>>> interrupts? Here Linux can't find an appropriate interrupt.
>> 
>> You reminds me that the DTS shipped along with the board has issues on
"arm,armv7-timer". I have corrected it for xen but leave the bare
metal booting dtb the original status.
>> 
>>> 
>>>>> omap5_realtime_timer_init: arch_timer_register failed -22
>>>>> 
>>>>>> 
>>>>>> It's not clear to me, how you need to implement
init_time. Do you have a
>>>>>> datasheet with the board? If yes, is there a section
for the section timer?
>>>>> 
>>>>> Yes, TI has published an 49.6MB Technical Reference Manual
online with 88 pages section about timer.
>>>> 
>>>> Wait. I think I might make a mistake. The "arch
timer" is different from "timer" as devices, right? The timer
section in Technical Reference Manual seems to be the latter one, for it is 
connect to the Level-4 interconnect of OMAP5432 and described as
"ti,omap5430-timer" in DTS. I think the "arch timer" you
mentioned should refer to "arm,armv7-timer", right?
>>> 
>>> By "arch timer" I mean "arm,armv7-timer".
>> 
>> Thanks. So I think the codes to start with it is
linux/drivers/clocksource/arm_arch_timer.c: arch_timer_init(). The
linux/arch/arm/mach-omap2/timer.c is for the timer device
"ti,omap543-timer".
> 
> arch_timer_init is already implemented in init_xen_time
> (xen/arch/arm/time.c).
> On some board, the arch timer is not enabled by default and need a
> "workaround" for this purpose.
> 
> For instance on the Arndale, we need to enable the MCTimer. I think you
> need to do something like that for the OMAP.
> 
> Do you know if the boot CPU already start in HYP mode? If no, did you
> set up correctly the timer in mode_switch.S?
I have patched the u-boot. The boot CPU now starts in HYP mode.

Cheers,

Baozi
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Xen devel - Jul 2013 - [PATCH 0/4] Support 8250 compatible UART for OMAP5432

[PATCH 0/4] Support 8250 compatible UART for OMAP5432

[PATCH 1/4] xen: extract register definitions from ns16550 into a separated header

[PATCH 2/4] xen/arm: add OMAP5432 UART support for early_printk

[PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

[PATCH 4/4] xen/arm: enable ns16550 uart driver for OMAP5432

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

Re: [PATCH 1/4] xen: extract register definitions from ns16550 into a separated header

Re: [PATCH 1/4] xen: extract register definitions from ns16550 into a separated header

Re: [PATCH 1/4] xen: extract register definitions from ns16550 into a separated header

Re: [PATCH 2/4] xen/arm: add OMAP5432 UART support for early_printk

Re: [PATCH 4/4] xen/arm: enable ns16550 uart driver for OMAP5432

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

Re: [PATCH 4/4] xen/arm: enable ns16550 uart driver for OMAP5432

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250

Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250