All of lore.kernel.org
 help / color / mirror / Atom feed
* Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
@ 2013-07-11 10:53 Chen Baozi
  2013-07-11 11:21 ` Ian Campbell
  0 siblings, 1 reply; 17+ messages in thread
From: Chen Baozi @ 2013-07-11 10:53 UTC (permalink / raw)
  To: Ian.Campbell; +Cc: xen-devel

Hi Ian,

I was struggling with ns16550 UART driver post-irq initialization. It seems
that the system stop after enabling rcv/xmit interrupts of ns16550. And the
output I could read from console is like:

...
(XEN) Xen version 4.3-unstable (cbz@) (arm-linux-gnueabihf-gcc (crosstool-NG
linaro-1.13.3
(XEN) Latest ChangeSet: Wed Jul 3 11:37:02 2013 +0800 git:a4656de-dirty
(XEN) Console output is synchronous.
(XEN) Processor: "ARM Limited", variant: 0x2, part 0xc0f, rev 0x2
(XEN) 32-bit Execution:
(XEN)   Processor Features: 00001131:00011011
(XEN)     Instruction Sets: AArch32 Thumb Thumb-2 ThumbEE Jazelle
(XEN)     Extensions: GenericTimer Security
(XEN)   Debug Features: 02010555
(XEN)   Auxiliary Features: 00000000
(XEN)   Memory Model Features: 10201105 20000000 01240000 02102211
(XEN)  ISA Features: 02101110 13112111 21232041 11112131 10011142 00000000
(XEN) Platform: TI OMAP5
(XEN) Generic Timer IRQ: phys=30 hyp=26 virt=27
(XEN) Using generic timer at 0 KHz
(XEN) GIC initialization:
(XEN)         gic_dist_addr=0000000048211000
(XEN)         gic_cpu_addr=0000000048212000
(XEN)         gic_hyp_addr=0000000048214000
(XEN)         gic_vcpu_addr=0000000048216000
(XEN)         gic_maintenance_irq=25
(XEN) GIC: 192 lines, 2 cpus, secure (IID 0000043b).
(XEN) Waiting for 0 other CPUs to be ready
(XEN) Using scheduler: SMP Credit Scheduler (credit)

I pressed 'd' to dump the debug info. And here it is:

(XEN) 'd' pressed -> dumping registers
(XEN)
(XEN) *** Dumping CPU0 host state: ***
(XEN) ----[ Xen-4.3-unstable  arm32  debug=y  Tainted:    C ]----
(XEN) CPU:    0
(XEN) PC:     0022ad48 set_timer+0x248/0x26c
(XEN) CPSR:   2000015a MODE:Hypervisor
(XEN)      R0: 002d6380 R1: 00000000 R2: 00000000 R3: 00000000
(XEN)      R4: 002a3440 R5: 00000000 R6: 002d6380 R7: 002d6380
(XEN)      R8: 002d4000 R9: 002a3458 R10:2000015a R11:0028fe8c R12:00000000
(XEN) HYP: SP: 0028fe5c LR: 0022ad44
(XEN)
(XEN) HTTBR feed2000
(XEN) HDFAR 0
(XEN) HIFAR 0
(XEN) HPFAR 0
(XEN) HCR 00082835
(XEN) HSR   0
(XEN) VTTBR 0000000000
(XEN)
(XEN) DFSR 0 DFAR 0
(XEN) IFSR 0 IFAR 0
(XEN)
(XEN) Xen stack trace from sp=0028fe5c:
(XEN)    0022ad44 000f4240 00000000 0026075c 002a3408 0026075c 0026075c 00000002
(XEN)    0026075c 00040000 00000000 0028fe9c 002405e0 0025cf08 002a3408 0028feac
(XEN)    00267a68 0026075c 00000000 0028fecc 00268738 00000fff 00260d9c 0025e9b0
(XEN)    fe000000 00000000 00000001 0028fee4 00267750 002d1fd8 00260d9c 0025e9b0
(XEN)    fe000000 0028ff54 0026b02c 11112131 10011142 00000000 00000000 00000000
(XEN)    00000000 00000000 fdffa000 00004eb4 802d7700 80000000 00000001 80000000
(XEN)    00000000 ff000000 00000000 ff000000 00000000 00000000 00000018 00000000
(XEN)    00000000 00000000 802d7700 80200000 80000000 00400000 0020040c 00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
(XEN)    00000000
(XEN) Xen call trace:
(XEN)    [<0022ad48>] set_timer+0x248/0x26c (PC)
(XEN)    [<0022ad44>] set_timer+0x244/0x26c (LR)
(XEN)    [<002405e0>] ns16550_setup_postirq+0x58/0x5c
(XEN)    [<00267a68>] ns16550_init_postirq+0xbc/0xc0
(XEN)    [<00268738>] serial_init_postirq+0x48/0x70
(XEN)    [<00267750>] console_init_postirq+0x18/0x1e8
(XEN)    [<0026b02c>] start_xen+0x73c/0xae8
(XEN)    [<0020040c>] paging+0xc/0x94
(XEN)

I disassembled xen-syms to see which instructions are around the top address
of call trace:

22ad14:       0a000002        beq     22ad24 <set_timer+0x224>
22ad18:       e1d401b8        ldrh    r0, [r4, #24]
22ad1c:       e3a01000        mov     r1, #0
22ad20:       ebffeec8        bl      226848 <cpu_raise_softirq>
22ad24:       e3060380        movw    r0, #25472      ; 0x6380
22ad28:       e340002d        movt    r0, #45 ; 0x2d
22ad2c:       e3043000        movw    r3, #16384      ; 0x4000
22ad30:       e340302d        movt    r3, #45 ; 0x2d
22ad34:       e1d421b8        ldrh    r2, [r4, #24]
22ad38:       e7933102        ldr     r3, [r3, r2, lsl #2]
22ad3c:       e0800003        add     r0, r0, r3
22ad40:       ebfff0ed        bl      2270fc <_spin_unlock>
22ad44:       e121f00a        msr     CPSR_c, sl
22ad48:       e24bd020        sub     sp, fp, #32
^^^^^^
22ad4c:       e8bd8ff0        pop     {r4, r5, r6, r7, r8, r9, sl, fp, pc}
22ad50:       002d6298        .word   0x002d6298
22ad54:       0025a3a8        .word   0x0025a3a8
22ad58:       00256b54        .word   0x00256b54

If I comment the "ns_write_reg(uart, IER, IER_ERDAI | IER_ETHREI);" line,
which is used to enable rcv/xmit interrupts in the ns16550_setup_postirq(),
then I could get a little more output such as:

(XEN) Allocated console ring of 16 KiB.
(XEN) Brought up 1 CPUs
(XEN) *** LOADING DOMAIN 0 ***
(XEN) Populate P2M 0x80000000->0x90000000
(XEN) Loading kernel from boot module 1
(XEN) Loading zImage from 00000000a0000000 to 0000000080008000-00000000803cccc8
(XEN) Std. Loglevel: All
(XEN) Guest Loglevel: All
(XEN) **********************************************
(XEN) ******* WARNING: CONSOLE OUTPUT IS SYNCHRONOUS
(XEN) ******* This option is intended to aid debugging of Xen by ensuring
(XEN) ******* that all output is synchronously delivered on the serial line.
(XEN) ******* However it can introduce SIGNIFICANT latencies and affect
(XEN) ******* timekeeping. It is NOT recommended for production use!
(XEN) **********************************************
(XEN) 3...

I've also sprinkled some early_printk around "ns_write_reg(uart, IER,
IER_ERDAI | IER_ETHREI);' to see what would be looked like. It is also
stuck. And the stuck point seems to be at 'early_uart_ready' which checks
Xmit holding register flag.

Any ideas?

Cheers,

Baozi

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
  2013-07-11 10:53 Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5 Chen Baozi
@ 2013-07-11 11:21 ` Ian Campbell
  2013-07-11 11:37   ` Chen Baozi
                     ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Ian Campbell @ 2013-07-11 11:21 UTC (permalink / raw)
  To: Chen Baozi; +Cc: xen-devel

On Thu, 2013-07-11 at 18:53 +0800, Chen Baozi wrote:

> Any ideas?

Nothing immediately springs to mind. Did you DT-enable the ns16550
driver?

I did a quick hack and slash job to get it going on the cubieboard2,
which I've pasted below (also needed io.h to actually implement the
accessors). (Be kind, it obviously needs proper cleanup ;-))

Do you know if you are getting any interrupts at all? I'm wondering if
maybe there is a storm?

Ian.

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index e0c87bb..ed18b91 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -19,7 +19,10 @@
 #include <xen/iocap.h>
 #include <xen/pci.h>
 #include <xen/pci_regs.h>
+#include <xen/vmap.h>
 #include <asm/io.h>
+#include <asm/early_printk.h>
+#include <asm/device.h>
 #ifdef CONFIG_X86
 #include <asm/fixmap.h>
 #endif
@@ -39,7 +42,8 @@ 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. */
+    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;
@@ -56,6 +60,7 @@ static struct ns16550 {
     u32 bar;
     u16 cr;
     u8 bar_idx;
+    struct dt_irq dt_irq;
 } ns16550_com[2] = { { 0 } };
 
 /* Register offsets */
@@ -128,18 +133,20 @@ static struct ns16550 {
 #define RESUME_DELAY    MILLISECS(10)
 #define RESUME_RETRIES  100
 
+#define REG_SHIFT 2
+
 static char ns_read_reg(struct ns16550 *uart, int reg)
 {
-    if ( uart->remapped_io_base == NULL )
-        return inb(uart->io_base + reg);
-    return readb(uart->remapped_io_base + reg);
+//    if ( uart->remapped_io_base == NULL )
+//        return inb(uart->io_base + reg);
+    return readl(uart->remapped_io_base + (reg<<REG_SHIFT));
 }
 
 static void ns_write_reg(struct ns16550 *uart, int reg, char c)
 {
-    if ( uart->remapped_io_base == NULL )
-        return outb(c, uart->io_base + reg);
-    writeb(c, uart->remapped_io_base + reg);
+//    if ( uart->remapped_io_base == NULL )
+//        return outb(c, uart->io_base + reg);
+    writel(c, uart->remapped_io_base + (reg<<REG_SHIFT));
 }
 
 static void ns16550_interrupt(
@@ -214,6 +221,7 @@ static int ns16550_getc(struct serial_port *port, char *pc)
     return 1;
 }
 
+#if 0
 static void pci_serial_early_init(struct ns16550 *uart)
 {
     if ( !uart->ps_bdf_enable || uart->io_base >= 0x10000 )
@@ -231,6 +239,7 @@ static void pci_serial_early_init(struct ns16550 *uart)
     pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1], uart->ps_bdf[2],
                      PCI_COMMAND, PCI_COMMAND_IO);
 }
+#endif
 
 static void ns16550_setup_preirq(struct ns16550 *uart)
 {
@@ -239,7 +248,9 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
 
     uart->intr_works = 0;
 
+#if 0
     pci_serial_early_init(uart);
+#endif
 
     lcr = (uart->data_bits - 5) | ((uart->stop_bits - 1) << 2) | uart->parity;
 
@@ -260,7 +271,8 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
         /* Baud rate already set: read it out from the divisor latch. */
         divisor  = ns_read_reg(uart, DLL);
         divisor |= ns_read_reg(uart, DLM) << 8;
-        uart->baud = uart->clock_hz / (divisor << 4);
+        early_printk("divisor %d\n", divisor);
+        //uart->baud = uart->clock_hz / (divisor << 4);
     }
     ns_write_reg(uart, LCR, lcr);
 
@@ -285,8 +297,10 @@ 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);
+//        uart->remapped_io_base = (char *)ioremap(uart->io_base, 8);
 #endif
+        early_printk("UART mapped at %p\n", uart->remapped_io_base);
     }
 
     ns16550_setup_preirq(uart);
@@ -326,23 +340,27 @@ static void __init ns16550_init_postirq(struct serial_port *port)
 
     /* Calculate time to fill RX FIFO and/or empty TX FIFO for polling. */
     bits = uart->data_bits + uart->stop_bits + !!uart->parity;
-    uart->timeout_ms = max_t(
-        unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
-
+//    uart->timeout_ms = max_t(
+//        unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
+    uart->timeout_ms = 1;
     if ( uart->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);
+        //if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
+        //    printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq);
+        if ( (rc = setup_dt_irq(&uart->dt_irq, &uart->irqaction)) != 0 )
+            printk("ERROR: Failed to allocate ns16550 DT IRQ.\n");
     }
 
     ns16550_setup_postirq(uart);
 
+#if 0
     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)
@@ -351,13 +369,16 @@ static void ns16550_suspend(struct serial_port *port)
 
     stop_timer(&uart->timer);
 
+#if 0
     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)
 {
+#if 0
     struct ns16550 *uart = port->uart;
 
     if ( uart->bar )
@@ -367,6 +388,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);
@@ -440,6 +462,12 @@ static int __init ns16550_irq(struct serial_port *port)
     return ((uart->irq > 0) ? uart->irq : -1);
 }
 
+static const struct dt_irq __init *ns16550_dt_irq(struct serial_port *port)
+{
+    struct ns16550 *uart = port->uart;
+    return &uart->dt_irq;
+}
+
 static struct uart_driver __read_mostly ns16550_driver = {
     .init_preirq  = ns16550_init_preirq,
     .init_postirq = ns16550_init_postirq,
@@ -449,9 +477,13 @@ 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,
+    .dt_irq_get   = ns16550_dt_irq,
+
 };
 
+#if 0
+
 static int __init parse_parity_char(int c)
 {
     switch ( c )
@@ -709,8 +741,78 @@ void __init ns16550_init(int index, struct ns16550_defaults *defaults)
     /* Default is no transmit FIFO. */
     uart->fifo_size = 1;
 
-    ns16550_parse_port_config(uart, (index == 0) ? opt_com1 : opt_com2);
+    /* Register with generic serial driver. */
+    serial_register_uart(uart - ns16550_com, &ns16550_driver, uart);
 }
+#endif
+
+#if 1
+static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
+                                       const void *data)
+{
+    struct ns16550 *uart;
+    int res;
+
+    early_printk("%s\n", __func__);
+
+    uart = &ns16550_com[0];
+
+    uart->baud      = BAUD_AUTO;//115200;
+    uart->clock_hz  = UART_CLOCK_HZ;
+    uart->data_bits = 8;
+    uart->parity    = PARITY_NONE;
+    uart->stop_bits = 1;
+    //uart->irq       = defaults->irq;
+    //uart->io_base   = defaults->io_base;
+    /* Default is no transmit FIFO. */
+    uart->fifo_size = 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;
+    }
+
+    early_printk("ns16550 at %"PRIx64"-%"PRIx64"\n", uart->io_base, uart->io_base + uart->io_size);
+
+//    uart->io_base = addr; //ioremap_attr(addr, size, PAGE_HYPERVISOR_NOCACHE);
+//    if ( !uart->io_base )
+//    {
+//        early_printk("ns16550: Unable to map the UART memory\n");
+//
+//        return -ENOMEM;
+//    }
+
+    res = dt_device_get_irq(dev, 0, &uart->dt_irq);
+    if ( res )
+    {
+        early_printk("ns16550: Unable to retrieve the IRQ\n");
+        return res;
+    }
+
+    /* Register with generic serial driver. */
+    serial_register_uart(uart - ns16550_com, &ns16550_driver, uart);
+
+    dt_device_set_used_by(dev, DOMID_XEN);
+
+    early_printk("console done?\n");
+    printk("normal printk\n");
+    return 0;
+}
+
+static const char const *ns16550_dt_compat[] __initdata =
+{
+    "snps,dw-apb-uart",
+    NULL
+};
+
+DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
+        .compatible = ns16550_dt_compat,
+        .init = ns16550_uart_dt_init,
+DT_DEVICE_END
+#endif
 
 /*
  * Local variables:
diff --git a/xen/include/asm-arm/io.h b/xen/include/asm-arm/io.h
index aea5233..170263f 100644
--- a/xen/include/asm-arm/io.h
+++ b/xen/include/asm-arm/io.h
@@ -1,6 +1,54 @@
 #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:

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
  2013-07-11 11:21 ` Ian Campbell
@ 2013-07-11 11:37   ` Chen Baozi
  2013-07-11 12:44   ` Chen Baozi
  2013-07-12  7:41   ` Chen Baozi
  2 siblings, 0 replies; 17+ messages in thread
From: Chen Baozi @ 2013-07-11 11:37 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel


On Jul 11, 2013, at 7:21 PM, Ian Campbell <ian.campbell@citrix.com> wrote:

> On Thu, 2013-07-11 at 18:53 +0800, Chen Baozi wrote:
> 
>> Any ideas?
> 
> Nothing immediately springs to mind. Did you DT-enable the ns16550
> driver?
Yes, I think. The printk works well before init_postirq.

> 
> I did a quick hack and slash job to get it going on the cubieboard2,
> which I've pasted below (also needed io.h to actually implement the
> accessors). (Be kind, it obviously needs proper cleanup ;-))
> 
> Do you know if you are getting any interrupts at all? I'm wondering if
> maybe there is a storm?
I think "press 'd'" triggers to dump the debug info states that the console's interrupt works at a certain point?

> 
> Ian.
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index e0c87bb..ed18b91 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -19,7 +19,10 @@
> #include <xen/iocap.h>
> #include <xen/pci.h>
> #include <xen/pci_regs.h>
> +#include <xen/vmap.h>
> #include <asm/io.h>
> +#include <asm/early_printk.h>
> +#include <asm/device.h>
> #ifdef CONFIG_X86
> #include <asm/fixmap.h>
> #endif
> @@ -39,7 +42,8 @@ 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. */
> +    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;
> @@ -56,6 +60,7 @@ static struct ns16550 {
>     u32 bar;
>     u16 cr;
>     u8 bar_idx;
> +    struct dt_irq dt_irq;
> } ns16550_com[2] = { { 0 } };
> 
> /* Register offsets */
> @@ -128,18 +133,20 @@ static struct ns16550 {
> #define RESUME_DELAY    MILLISECS(10)
> #define RESUME_RETRIES  100
> 
> +#define REG_SHIFT 2
> +
> static char ns_read_reg(struct ns16550 *uart, int reg)
> {
> -    if ( uart->remapped_io_base == NULL )
> -        return inb(uart->io_base + reg);
> -    return readb(uart->remapped_io_base + reg);
> +//    if ( uart->remapped_io_base == NULL )
> +//        return inb(uart->io_base + reg);
> +    return readl(uart->remapped_io_base + (reg<<REG_SHIFT));
> }
> 
> static void ns_write_reg(struct ns16550 *uart, int reg, char c)
> {
> -    if ( uart->remapped_io_base == NULL )
> -        return outb(c, uart->io_base + reg);
> -    writeb(c, uart->remapped_io_base + reg);
> +//    if ( uart->remapped_io_base == NULL )
> +//        return outb(c, uart->io_base + reg);
> +    writel(c, uart->remapped_io_base + (reg<<REG_SHIFT));
> }
> 
> static void ns16550_interrupt(
> @@ -214,6 +221,7 @@ static int ns16550_getc(struct serial_port *port, char *pc)
>     return 1;
> }
> 
> +#if 0
> static void pci_serial_early_init(struct ns16550 *uart)
> {
>     if ( !uart->ps_bdf_enable || uart->io_base >= 0x10000 )
> @@ -231,6 +239,7 @@ static void pci_serial_early_init(struct ns16550 *uart)
>     pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1], uart->ps_bdf[2],
>                      PCI_COMMAND, PCI_COMMAND_IO);
> }
> +#endif
> 
> static void ns16550_setup_preirq(struct ns16550 *uart)
> {
> @@ -239,7 +248,9 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
> 
>     uart->intr_works = 0;
> 
> +#if 0
>     pci_serial_early_init(uart);
> +#endif
> 
>     lcr = (uart->data_bits - 5) | ((uart->stop_bits - 1) << 2) | uart->parity;
> 
> @@ -260,7 +271,8 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>         /* Baud rate already set: read it out from the divisor latch. */
>         divisor  = ns_read_reg(uart, DLL);
>         divisor |= ns_read_reg(uart, DLM) << 8;
> -        uart->baud = uart->clock_hz / (divisor << 4);
> +        early_printk("divisor %d\n", divisor);
> +        //uart->baud = uart->clock_hz / (divisor << 4);
>     }
>     ns_write_reg(uart, LCR, lcr);
> 
> @@ -285,8 +297,10 @@ 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);
> +//        uart->remapped_io_base = (char *)ioremap(uart->io_base, 8);
> #endif
> +        early_printk("UART mapped at %p\n", uart->remapped_io_base);
>     }
> 
>     ns16550_setup_preirq(uart);
> @@ -326,23 +340,27 @@ static void __init ns16550_init_postirq(struct serial_port *port)
> 
>     /* Calculate time to fill RX FIFO and/or empty TX FIFO for polling. */
>     bits = uart->data_bits + uart->stop_bits + !!uart->parity;
> -    uart->timeout_ms = max_t(
> -        unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
> -
> +//    uart->timeout_ms = max_t(
> +//        unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
> +    uart->timeout_ms = 1;
>     if ( uart->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);
> +        //if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
> +        //    printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq);
> +        if ( (rc = setup_dt_irq(&uart->dt_irq, &uart->irqaction)) != 0 )
> +            printk("ERROR: Failed to allocate ns16550 DT IRQ.\n");
>     }
> 
>     ns16550_setup_postirq(uart);
> 
> +#if 0
>     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)
> @@ -351,13 +369,16 @@ static void ns16550_suspend(struct serial_port *port)
> 
>     stop_timer(&uart->timer);
> 
> +#if 0
>     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)
> {
> +#if 0
>     struct ns16550 *uart = port->uart;
> 
>     if ( uart->bar )
> @@ -367,6 +388,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);
> @@ -440,6 +462,12 @@ static int __init ns16550_irq(struct serial_port *port)
>     return ((uart->irq > 0) ? uart->irq : -1);
> }
> 
> +static const struct dt_irq __init *ns16550_dt_irq(struct serial_port *port)
> +{
> +    struct ns16550 *uart = port->uart;
> +    return &uart->dt_irq;
> +}
> +
> static struct uart_driver __read_mostly ns16550_driver = {
>     .init_preirq  = ns16550_init_preirq,
>     .init_postirq = ns16550_init_postirq,
> @@ -449,9 +477,13 @@ 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,
> +    .dt_irq_get   = ns16550_dt_irq,
> +
> };
> 
> +#if 0
> +
> static int __init parse_parity_char(int c)
> {
>     switch ( c )
> @@ -709,8 +741,78 @@ void __init ns16550_init(int index, struct ns16550_defaults *defaults)
>     /* Default is no transmit FIFO. */
>     uart->fifo_size = 1;
> 
> -    ns16550_parse_port_config(uart, (index == 0) ? opt_com1 : opt_com2);
> +    /* Register with generic serial driver. */
> +    serial_register_uart(uart - ns16550_com, &ns16550_driver, uart);
> }
> +#endif
> +
> +#if 1
> +static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
> +                                       const void *data)
> +{
> +    struct ns16550 *uart;
> +    int res;
> +
> +    early_printk("%s\n", __func__);
> +
> +    uart = &ns16550_com[0];
> +
> +    uart->baud      = BAUD_AUTO;//115200;
> +    uart->clock_hz  = UART_CLOCK_HZ;
> +    uart->data_bits = 8;
> +    uart->parity    = PARITY_NONE;
> +    uart->stop_bits = 1;
> +    //uart->irq       = defaults->irq;
> +    //uart->io_base   = defaults->io_base;
> +    /* Default is no transmit FIFO. */
> +    uart->fifo_size = 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;
> +    }
> +
> +    early_printk("ns16550 at %"PRIx64"-%"PRIx64"\n", uart->io_base, uart->io_base + uart->io_size);
> +
> +//    uart->io_base = addr; //ioremap_attr(addr, size, PAGE_HYPERVISOR_NOCACHE);
> +//    if ( !uart->io_base )
> +//    {
> +//        early_printk("ns16550: Unable to map the UART memory\n");
> +//
> +//        return -ENOMEM;
> +//    }
> +
> +    res = dt_device_get_irq(dev, 0, &uart->dt_irq);
> +    if ( res )
> +    {
> +        early_printk("ns16550: Unable to retrieve the IRQ\n");
> +        return res;
> +    }
> +
> +    /* Register with generic serial driver. */
> +    serial_register_uart(uart - ns16550_com, &ns16550_driver, uart);
> +
> +    dt_device_set_used_by(dev, DOMID_XEN);
> +
> +    early_printk("console done?\n");
> +    printk("normal printk\n");
> +    return 0;
> +}
> +
> +static const char const *ns16550_dt_compat[] __initdata =
> +{
> +    "snps,dw-apb-uart",
> +    NULL
> +};
> +
> +DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
> +        .compatible = ns16550_dt_compat,
> +        .init = ns16550_uart_dt_init,
> +DT_DEVICE_END
> +#endif
> 
> /*
>  * Local variables:
> diff --git a/xen/include/asm-arm/io.h b/xen/include/asm-arm/io.h
> index aea5233..170263f 100644
> --- a/xen/include/asm-arm/io.h
> +++ b/xen/include/asm-arm/io.h
> @@ -1,6 +1,54 @@
> #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:
> 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
  2013-07-11 11:21 ` Ian Campbell
  2013-07-11 11:37   ` Chen Baozi
@ 2013-07-11 12:44   ` Chen Baozi
  2013-07-11 12:46     ` Ian Campbell
  2013-07-12  7:41   ` Chen Baozi
  2 siblings, 1 reply; 17+ messages in thread
From: Chen Baozi @ 2013-07-11 12:44 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel


On Jul 11, 2013, at 7:21 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:

> On Thu, 2013-07-11 at 18:53 +0800, Chen Baozi wrote:
> 
>> Any ideas?

Wait, there might be some bugs in my ns16550 driver...

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
  2013-07-11 12:44   ` Chen Baozi
@ 2013-07-11 12:46     ` Ian Campbell
  2013-07-11 12:54       ` Chen Baozi
  0 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2013-07-11 12:46 UTC (permalink / raw)
  To: Chen Baozi; +Cc: xen-devel

On Thu, 2013-07-11 at 20:44 +0800, Chen Baozi wrote:
> On Jul 11, 2013, at 7:21 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> 
> > On Thu, 2013-07-11 at 18:53 +0800, Chen Baozi wrote:
> > 
> >> Any ideas?
> 
> Wait, there might be some bugs in my ns16550 driver...

Did you use the existing one or did you write a new one from scratch?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
  2013-07-11 12:46     ` Ian Campbell
@ 2013-07-11 12:54       ` Chen Baozi
  0 siblings, 0 replies; 17+ messages in thread
From: Chen Baozi @ 2013-07-11 12:54 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel


On Jul 11, 2013, at 8:46 PM, Ian Campbell <ian.campbell@citrix.com> wrote:

> On Thu, 2013-07-11 at 20:44 +0800, Chen Baozi wrote:
>> On Jul 11, 2013, at 7:21 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> 
>>> On Thu, 2013-07-11 at 18:53 +0800, Chen Baozi wrote:
>>> 
>>>> Any ideas?
>> 
>> Wait, there might be some bugs in my ns16550 driver...
> 
> Did you use the existing one or did you write a new one from scratch?
> 
I did some similar hacks as you did, just a few differences on some lines.

There must be some bugs in my hacks, for your patch seems to work...

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
  2013-07-11 11:21 ` Ian Campbell
  2013-07-11 11:37   ` Chen Baozi
  2013-07-11 12:44   ` Chen Baozi
@ 2013-07-12  7:41   ` Chen Baozi
  2013-07-17  8:02     ` Chen Baozi
  2 siblings, 1 reply; 17+ messages in thread
From: Chen Baozi @ 2013-07-12  7:41 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

Hi Ian,

I think I've caught the point why your patch works while mine don't.

On Jul 11, 2013, at 7:21 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:

> ...
>     char __iomem *remapped_io_base;  /* Remapped virtual address of MMIO. */
>     /* UART with IRQ line: interrupt-driven I/O. */
>     struct irqaction irqaction;
> @@ -56,6 +60,7 @@ static struct ns16550 {
>     u32 bar;
>     u16 cr;
>     u8 bar_idx;
> +    struct dt_irq dt_irq;

Here you added a new dt_irq struct component.

> } ns16550_com[2] = { { 0 } };
> ...
> +
> +    uart->baud      = BAUD_AUTO;//115200;
> +    uart->clock_hz  = UART_CLOCK_HZ;
> +    uart->data_bits = 8;
> +    uart->parity    = PARITY_NONE;
> +    uart->stop_bits = 1;
> +    //uart->irq       = defaults->irq;

And uart->irq is no longer initialized.

> ...
> +    res = dt_device_get_irq(dev, 0, &uart->dt_irq);
> +    if ( res )
> +    {
> +        early_printk("ns16550: Unable to retrieve the IRQ\n");
> +        return res;
> +    }
> +
> +    /* Register with generic serial driver. */
> +    serial_register_uart(uart - ns16550_com, &ns16550_driver, uart);
> ...

However, in ns16550_setup_postirq() and ns16550_init_postirq(), there are blocks like this:

    if ( uart->irq > 0 )
    {
        /* Master interrupt enable; also keep DTR/RTS asserted. */
        ns_write_reg(uart, MCR, MCR_OUT2 | MCR_OUT2 | MCR_DTR | MCR_RTS);

        /* Enabled receive and transmit interrupts
        ns_write_reg(hart, IER, IER_ERDAI | IER_ETHREI);
    }

and this:

    if ( uart->irq > 0 )
    {
        uart->irqaction.handler = ns16550_interrupt;
        uart->irqaction.name    = "ns16550";
        uart->irqaction.dev_id  = port;
        if ( (rc = setup_dt_irq(&uart->dt_irq, &uart->irqaction)) != 0)
            printk("ERROR: Failed to allocate ns16550 DT IRQ %d\n", hart->irq);
    }

This means you have neither registered the irq nor enabled the interrupts. If I turned both of these two blocks into:

    if ( uart->dt_irq.irq > )
    {
        ...
    }

There is the same phenomenon that I described in previous email. And If I comment either of these two blocks in my patch, my implementation works.

Any ideas?

Cheers,

Baozi

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
  2013-07-12  7:41   ` Chen Baozi
@ 2013-07-17  8:02     ` Chen Baozi
  2013-07-17  8:21       ` Ian Campbell
  0 siblings, 1 reply; 17+ messages in thread
From: Chen Baozi @ 2013-07-17  8:02 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel


On Jul 12, 2013, at 3:41 PM, Chen Baozi <baozich@gmail.com> wrote:

> This means you have neither registered the irq nor enabled the interrupts. If I turned both of these two blocks into:
> 
>    if ( uart->dt_irq.irq > )
>    {
>        ...
>    }
> 
> There is the same phenomenon that I described in previous email. And If I comment either of these two blocks in my patch, my implementation works.
> 
> Any ideas?

Hi Ian,

I inserted some codes in ns16550_setup_postirq to see what would cause the stuck. It seems that it will stuck while restoring CPSR or "cpsie i". But I'm not sure that these two cases are the only factors that will lead the phenomenons.

Cheers,

Baozi

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
  2013-07-17  8:02     ` Chen Baozi
@ 2013-07-17  8:21       ` Ian Campbell
  2013-07-17  9:01         ` Chen Baozi
  0 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2013-07-17  8:21 UTC (permalink / raw)
  To: Chen Baozi; +Cc: xen-devel

On Wed, 2013-07-17 at 16:02 +0800, Chen Baozi wrote:
> On Jul 12, 2013, at 3:41 PM, Chen Baozi <baozich@gmail.com> wrote:
> 
> > This means you have neither registered the irq nor enabled the interrupts. If I turned both of these two blocks into:
> > 
> >    if ( uart->dt_irq.irq > )
> >    {
> >        ...
> >    }
> > 
> > There is the same phenomenon that I described in previous email. And If I comment either of these two blocks in my patch, my implementation works.
> > 
> > Any ideas?

Sorry for not replying to this, the answer was going to be "no, but I'll
see if I can reproduce" -- but I never got a chance.

> I inserted some codes in ns16550_setup_postirq to see what would cause
> the stuck. It seems that it will stuck while restoring CPSR or "cpsie
> i". But I'm not sure that these two cases are the only factors that
> will lead the phenomenons.

Does it get stuck or does it just immediately take another interrupt?
(Could indicate a level vs. edge misconfiguration perhaps?)

Is "restoring CPSR" this from arm32/entry.S:
        ENTRY(return_to_hypervisor)
                cpsid i
                ldr lr, [sp, #UREGS_lr]
                ldr r11, [sp, #UREGS_pc]
                msr ELR_hyp, r11
                ldr r11, [sp, #UREGS_cpsr]
                msr SPSR_hyp, r11
                pop {r0-r12}
                add sp, #(UREGS_SP_usr - UREGS_sp); /* SP, LR, SPSR, PC */
                eret
or elsewhere? If it is this one does changing SPSR_hyp into SPSR_cxsf change anything?

The "cpsie i" is from the call to local_irq_enable in gic_interrupt() or
somewhere else?

Ian.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
  2013-07-17  8:21       ` Ian Campbell
@ 2013-07-17  9:01         ` Chen Baozi
  2013-07-17  9:21           ` Ian Campbell
  0 siblings, 1 reply; 17+ messages in thread
From: Chen Baozi @ 2013-07-17  9:01 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel


On Jul 17, 2013, at 4:21 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:

> On Wed, 2013-07-17 at 16:02 +0800, Chen Baozi wrote:
>> On Jul 12, 2013, at 3:41 PM, Chen Baozi <baozich@gmail.com> wrote:
>> 
>>> This means you have neither registered the irq nor enabled the interrupts. If I turned both of these two blocks into:
>>> 
>>>   if ( uart->dt_irq.irq > )
>>>   {
>>>       ...
>>>   }
>>> 
>>> There is the same phenomenon that I described in previous email. And If I comment either of these two blocks in my patch, my implementation works.
>>> 
>>> Any ideas?
> 
> Sorry for not replying to this, the answer was going to be "no, but I'll
> see if I can reproduce" -- but I never got a chance.
> 
>> I inserted some codes in ns16550_setup_postirq to see what would cause
>> the stuck. It seems that it will stuck while restoring CPSR or "cpsie
>> i". But I'm not sure that these two cases are the only factors that
>> will lead the phenomenons.
> 
> Does it get stuck or does it just immediately take another interrupt?

No idea. It seems the UART's receive interrupt works well, because pressing "d" in the serial console could trigger dumping registers as normal. I'm going into these possibilities and trying to make it sure.

> (Could indicate a level vs. edge misconfiguration perhaps?)
> 
> Is "restoring CPSR" this from arm32/entry.S:
>        ENTRY(return_to_hypervisor)
>                cpsid i
>                ldr lr, [sp, #UREGS_lr]
>                ldr r11, [sp, #UREGS_pc]
>                msr ELR_hyp, r11
>                ldr r11, [sp, #UREGS_cpsr]
>                msr SPSR_hyp, r11
>                pop {r0-r12}
>                add sp, #(UREGS_SP_usr - UREGS_sp); /* SP, LR, SPSR, PC */
>                eret
> or elsewhere? If it is this one does changing SPSR_hyp into SPSR_cxsf change anything?
> 
> The "cpsie i" is from the call to local_irq_enable in gic_interrupt() or

"restoring CPSR" refers to the instruction "msr CPSR_c, <reg>" which is from "local_irq_restore". And "cpsie i" is from the call to local_irq_enable".

> somewhere else?
> 
> Ian.
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
  2013-07-17  9:01         ` Chen Baozi
@ 2013-07-17  9:21           ` Ian Campbell
  2013-07-17 13:53             ` Chen Baozi
  0 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2013-07-17  9:21 UTC (permalink / raw)
  To: Chen Baozi; +Cc: xen-devel

On Wed, 2013-07-17 at 17:01 +0800, Chen Baozi wrote:
> On Jul 17, 2013, at 4:21 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> 
> > On Wed, 2013-07-17 at 16:02 +0800, Chen Baozi wrote:
> >> On Jul 12, 2013, at 3:41 PM, Chen Baozi <baozich@gmail.com> wrote:
> >> 
> >>> This means you have neither registered the irq nor enabled the interrupts. If I turned both of these two blocks into:
> >>> 
> >>>   if ( uart->dt_irq.irq > )
> >>>   {
> >>>       ...
> >>>   }
> >>> 
> >>> There is the same phenomenon that I described in previous email. And If I comment either of these two blocks in my patch, my implementation works.
> >>> 
> >>> Any ideas?
> > 
> > Sorry for not replying to this, the answer was going to be "no, but I'll
> > see if I can reproduce" -- but I never got a chance.
> > 
> >> I inserted some codes in ns16550_setup_postirq to see what would cause
> >> the stuck. It seems that it will stuck while restoring CPSR or "cpsie
> >> i". But I'm not sure that these two cases are the only factors that
> >> will lead the phenomenons.
> > 
> > Does it get stuck or does it just immediately take another interrupt?
> 
> No idea. It seems the UART's receive interrupt works well, because pressing "d" in the serial console could trigger dumping registers as normal. I'm going into these possibilities and trying to make it sure.
> 
> > (Could indicate a level vs. edge misconfiguration perhaps?)
> > 
> > Is "restoring CPSR" this from arm32/entry.S:
> >        ENTRY(return_to_hypervisor)
> >                cpsid i
> >                ldr lr, [sp, #UREGS_lr]
> >                ldr r11, [sp, #UREGS_pc]
> >                msr ELR_hyp, r11
> >                ldr r11, [sp, #UREGS_cpsr]
> >                msr SPSR_hyp, r11
> >                pop {r0-r12}
> >                add sp, #(UREGS_SP_usr - UREGS_sp); /* SP, LR, SPSR, PC */
> >                eret
> > or elsewhere? If it is this one does changing SPSR_hyp into SPSR_cxsf change anything?
> > 
> > The "cpsie i" is from the call to local_irq_enable in gic_interrupt() or
> 
> "restoring CPSR" refers to the instruction "msr CPSR_c, <reg>" which
> is from "local_irq_restore". And "cpsie i" is from the call to
> local_irq_enable".

Ah right. So in both cases you will immediately take any pending
interrupt. I think I would continue instrumenting starting from
gic_interrupt() and hopefully eventually into the ns16550 interrupt
handler.

> 
> > somewhere else?
> > 
> > Ian.
> > 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
  2013-07-17  9:21           ` Ian Campbell
@ 2013-07-17 13:53             ` Chen Baozi
  2013-07-17 15:26               ` Ian Campbell
  0 siblings, 1 reply; 17+ messages in thread
From: Chen Baozi @ 2013-07-17 13:53 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

On Wed, Jul 17, 2013 at 10:21:52AM +0100, Ian Campbell wrote:
> On Wed, 2013-07-17 at 17:01 +0800, Chen Baozi wrote:
> > On Jul 17, 2013, at 4:21 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > 
> > > On Wed, 2013-07-17 at 16:02 +0800, Chen Baozi wrote:
> > >> On Jul 12, 2013, at 3:41 PM, Chen Baozi <baozich@gmail.com> wrote:
> > >> 
> > >>> This means you have neither registered the irq nor enabled the interrupts. If I turned both of these two blocks into:
> > >>> 
> > >>>   if ( uart->dt_irq.irq > )
> > >>>   {
> > >>>       ...
> > >>>   }
> > >>> 
> > >>> There is the same phenomenon that I described in previous email. And If I comment either of these two blocks in my patch, my implementation works.
> > >>> 
> > >>> Any ideas?
> > > 
> > > Sorry for not replying to this, the answer was going to be "no, but I'll
> > > see if I can reproduce" -- but I never got a chance.
> > > 
> > >> I inserted some codes in ns16550_setup_postirq to see what would cause
> > >> the stuck. It seems that it will stuck while restoring CPSR or "cpsie
> > >> i". But I'm not sure that these two cases are the only factors that
> > >> will lead the phenomenons.
> > > 
> > > Does it get stuck or does it just immediately take another interrupt?
> > 
> > No idea. It seems the UART's receive interrupt works well, because pressing "d" in the serial console could trigger dumping registers as normal. I'm going into these possibilities and trying to make it sure.
> > 
> > > (Could indicate a level vs. edge misconfiguration perhaps?)
> > > 
> > > Is "restoring CPSR" this from arm32/entry.S:
> > >        ENTRY(return_to_hypervisor)
> > >                cpsid i
> > >                ldr lr, [sp, #UREGS_lr]
> > >                ldr r11, [sp, #UREGS_pc]
> > >                msr ELR_hyp, r11
> > >                ldr r11, [sp, #UREGS_cpsr]
> > >                msr SPSR_hyp, r11
> > >                pop {r0-r12}
> > >                add sp, #(UREGS_SP_usr - UREGS_sp); /* SP, LR, SPSR, PC */
> > >                eret
> > > or elsewhere? If it is this one does changing SPSR_hyp into SPSR_cxsf change anything?
> > > 
> > > The "cpsie i" is from the call to local_irq_enable in gic_interrupt() or
> > 
> > "restoring CPSR" refers to the instruction "msr CPSR_c, <reg>" which
> > is from "local_irq_restore". And "cpsie i" is from the call to
> > local_irq_enable".
> 
> Ah right. So in both cases you will immediately take any pending
> interrupt. I think I would continue instrumenting starting from
> gic_interrupt() and hopefully eventually into the ns16550 interrupt
> handler.
> 

I went through gic_interrupt() and thought got the points cause the stuck.

If I change the while(...) in ns16550_interrupt() into if(...) and comment
either "GICC[GICC_EOIR] = irq;" or "GICC[GICC_DIR] = irq;" in
git_host_irq_end(), it won't get stuck after enabling receive and transmit
interrupts in ns16550_setup_postirq().

Baozi

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
  2013-07-17 13:53             ` Chen Baozi
@ 2013-07-17 15:26               ` Ian Campbell
  2013-07-17 16:05                 ` Chen Baozi
  0 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2013-07-17 15:26 UTC (permalink / raw)
  To: Chen Baozi; +Cc: xen-devel

> > > "restoring CPSR" refers to the instruction "msr CPSR_c, <reg>" which
> > > is from "local_irq_restore". And "cpsie i" is from the call to
> > > local_irq_enable".
> > 
> > Ah right. So in both cases you will immediately take any pending
> > interrupt. I think I would continue instrumenting starting from
> > gic_interrupt() and hopefully eventually into the ns16550 interrupt
> > handler.
> > 
> 
> I went through gic_interrupt() and thought got the points cause the stuck.

Please can you clarify exactly what you mean by "stuck". Previously you
thought it was stuck in ns16550_setup_postirq when in actual fact it was
taking an interrupt. Are you sure that you are taking multiple,
potentially nested interrupts and eventually blowing the hypervisor
stack? This seems like the most likely scenario to me.

> If I change the while(...) in ns16550_interrupt() into if(...) and comment
> either "GICC[GICC_EOIR] = irq;" or "GICC[GICC_DIR] = irq;" in
> git_host_irq_end(), it won't get stuck after enabling receive and transmit
> interrupts in ns16550_setup_postirq().

By removing the writes to either EOIR or DIR you are in effect never
unmasking the interrupt, so you avoid the nest interrupt problem.

If this is the case then real issue is perhaps that for whatever reason
ns16550_interrupt is not causing the hardware to deassert its interrupt
line.

The UART on the sunxi is compatible (in DTS terms) with
"snps,dw-apb-uart", which seems to be an 8250 variant, but one which
differs enough to warrant its own compatibility string -- perhaps Xen's
ns16550 driver isn't dealing with some quirk of this device?

It seems like the driver in Linux is drivers/tty/serial/8250/8250_dw.c.
dw8250_handle_irq looks interesting...

        struct dw8250_data *d = p->private_data;
        unsigned int iir = p->serial_in(p, UART_IIR);

        if (serial8250_handle_irq(p, iir)) {
                return 1;
        } else if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
                /* Clear the USR and write the LCR again. */
                (void)p->serial_in(p, DW_UART_USR);
                p->serial_out(p, UART_LCR, d->last_lcr);

                return 1;
        }

        return 0;

In particular the fallback code there when the common 8250 handler
didn't deal with the issue...

Ian.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
  2013-07-17 15:26               ` Ian Campbell
@ 2013-07-17 16:05                 ` Chen Baozi
  2013-07-18 11:53                   ` Ian Campbell
  0 siblings, 1 reply; 17+ messages in thread
From: Chen Baozi @ 2013-07-17 16:05 UTC (permalink / raw)
  To: Ian Campbell; +Cc: <xen-devel@lists.xen.org>

在 2013-7-17,23:26,Ian Campbell <Ian.Campbell@citrix.com> 写道:

>>>> "restoring CPSR" refers to the instruction "msr CPSR_c, <reg>" which
>>>> is from "local_irq_restore". And "cpsie i" is from the call to
>>>> local_irq_enable".
>>> 
>>> Ah right. So in both cases you will immediately take any pending
>>> interrupt. I think I would continue instrumenting starting from
>>> gic_interrupt() and hopefully eventually into the ns16550 interrupt
>>> handler.
>> 
>> I went through gic_interrupt() and thought got the points cause the stuck.
> 
> Please can you clarify exactly what you mean by "stuck". Previously you
> thought it was stuck in ns16550_setup_postirq when in actual fact it was
> taking an interrupt.

I thought it was "stuck" because since every time I pressed 'd' to dump the registers the PC always stayed at the same position during executing ns16550_setup_postirq. So it really looks like that that the system get stuck at that point. Sorry if I made a wrong description.

> Are you sure that you are taking multiple,
> potentially nested interrupts and eventually blowing the hypervisor
> stack? This seems like the most likely scenario to me.

Seems reasonable. Is there any way to prove that we are under this situation? I didn't expect this possibility before. Thanks.

> 
>> If I change the while(...) in ns16550_interrupt() into if(...) and comment
>> either "GICC[GICC_EOIR] = irq;" or "GICC[GICC_DIR] = irq;" in
>> git_host_irq_end(), it won't get stuck after enabling receive and transmit
>> interrupts in ns16550_setup_postirq().
> 
> By removing the writes to either EOIR or DIR you are in effect never
> unmasking the interrupt, so you avoid the nest interrupt problem.
> 
> If this is the case then real issue is perhaps that for whatever reason
> ns16550_interrupt is not causing the hardware to deassert its interrupt
> line.
> 
> The UART on the sunxi is compatible (in DTS terms) with
> "snps,dw-apb-uart", which seems to be an 8250 variant, but one which
> differs enough to warrant its own compatibility string -- perhaps Xen's
> ns16550 driver isn't dealing with some quirk of this device?

I checked my OMAP5's data sheet. Generally, they looks very similar. But I will read the manual more carefully again tomorrow to make sure this point.

> 
> It seems like the driver in Linux is drivers/tty/serial/8250/8250_dw.c.
> dw8250_handle_irq looks interesting...
> 
>        struct dw8250_data *d = p->private_data;
>        unsigned int iir = p->serial_in(p, UART_IIR);
> 
>        if (serial8250_handle_irq(p, iir)) {
>                return 1;
>        } else if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
>                /* Clear the USR and write the LCR again. */
>                (void)p->serial_in(p, DW_UART_USR);
>                p->serial_out(p, UART_LCR, d->last_lcr);
> 
>                return 1;
>        }
> 
>        return 0;
> 
> In particular the fallback code there when the common 8250 handler
> didn't deal with the issue...

I'll get down to the Linux driver tomorrow to see whether I could catch the point.

Thanks a lot.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
  2013-07-17 16:05                 ` Chen Baozi
@ 2013-07-18 11:53                   ` Ian Campbell
  2013-07-18 12:44                     ` Chen Baozi
  0 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2013-07-18 11:53 UTC (permalink / raw)
  To: Chen Baozi; +Cc: <xen-devel@lists.xen.org>

On Thu, 2013-07-18 at 00:05 +0800, Chen Baozi wrote:
> 在 2013-7-17,23:26,Ian Campbell <Ian.Campbell@citrix.com> 写道:
> 
> >>>> "restoring CPSR" refers to the instruction "msr CPSR_c, <reg>" which
> >>>> is from "local_irq_restore". And "cpsie i" is from the call to
> >>>> local_irq_enable".
> >>> 
> >>> Ah right. So in both cases you will immediately take any pending
> >>> interrupt. I think I would continue instrumenting starting from
> >>> gic_interrupt() and hopefully eventually into the ns16550 interrupt
> >>> handler.
> >> 
> >> I went through gic_interrupt() and thought got the points cause the stuck.
> > 
> > Please can you clarify exactly what you mean by "stuck". Previously you
> > thought it was stuck in ns16550_setup_postirq when in actual fact it was
> > taking an interrupt.
> 
> I thought it was "stuck" because since every time I pressed 'd' to
> dump the registers the PC always stayed at the same position during
> executing ns16550_setup_postirq. So it really looks like that that the
> system get stuck at that point. Sorry if I made a wrong description.

No problem. In fact if 'd' works perhaps you are not blowing the stack
at all with multiple interrupts.

Ah, you are probably never escaping the loop in gic_interrupt because
the read of IAR always returns the UART interrupt.

> 
> > Are you sure that you are taking multiple,
> > potentially nested interrupts and eventually blowing the hypervisor
> > stack? This seems like the most likely scenario to me.
> 
> Seems reasonable. Is there any way to prove that we are under this
> situation? I didn't expect this possibility before. Thanks.

I was about to say that a printk in gic_interrupt ought to confirm, but
since the UART IRQ is the problem perhaps that isn't so obvious, unless
sync_console helps in some way. Worth a try.

If not then since 'd' works then perhaps you could keep a count of the
number serial IRQs in a global var and dump it?

> 
> > 
> >> If I change the while(...) in ns16550_interrupt() into if(...) and comment
> >> either "GICC[GICC_EOIR] = irq;" or "GICC[GICC_DIR] = irq;" in
> >> git_host_irq_end(), it won't get stuck after enabling receive and transmit
> >> interrupts in ns16550_setup_postirq().
> > 
> > By removing the writes to either EOIR or DIR you are in effect never
> > unmasking the interrupt, so you avoid the nest interrupt problem.
> > 
> > If this is the case then real issue is perhaps that for whatever reason
> > ns16550_interrupt is not causing the hardware to deassert its interrupt
> > line.
> > 
> > The UART on the sunxi is compatible (in DTS terms) with
> > "snps,dw-apb-uart", which seems to be an 8250 variant, but one which
> > differs enough to warrant its own compatibility string -- perhaps Xen's
> > ns16550 driver isn't dealing with some quirk of this device?
> 
> I checked my OMAP5's data sheet. Generally, they looks very similar.
> But I will read the manual more carefully again tomorrow to make sure
> this point.

Good idea.

> 
> > 
> > It seems like the driver in Linux is drivers/tty/serial/8250/8250_dw.c.
> > dw8250_handle_irq looks interesting...
> > 
> >        struct dw8250_data *d = p->private_data;
> >        unsigned int iir = p->serial_in(p, UART_IIR);
> > 
> >        if (serial8250_handle_irq(p, iir)) {
> >                return 1;
> >        } else if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
> >                /* Clear the USR and write the LCR again. */
> >                (void)p->serial_in(p, DW_UART_USR);
> >                p->serial_out(p, UART_LCR, d->last_lcr);
> > 
> >                return 1;
> >        }
> > 
> >        return 0;
> > 
> > In particular the fallback code there when the common 8250 handler
> > didn't deal with the issue...
> 
> I'll get down to the Linux driver tomorrow to see whether I could catch the point.

Actually, the comment at the top is interesting:
 12  * The Synopsys DesignWare 8250 has an extra feature whereby it detects if the
 13  * LCR is written whilst busy.  If it is, then a busy detect interrupt is
 14  * raised, the LCR needs to be rewritten and the uart status register read.

I'm not sure that "extra feature" doesn't mean "weird quirk" but there we go ;-)

The changelog of the patch which added it is interesting too:
http://permalink.gmane.org/gmane.linux.serial/5855

Ian.


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
  2013-07-18 11:53                   ` Ian Campbell
@ 2013-07-18 12:44                     ` Chen Baozi
  2013-07-19  8:25                       ` Chen Baozi
  0 siblings, 1 reply; 17+ messages in thread
From: Chen Baozi @ 2013-07-18 12:44 UTC (permalink / raw)
  To: Ian Campbell; +Cc: <xen-devel@lists.xen.org>


On Jul 18, 2013, at 7:53 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:

> On Thu, 2013-07-18 at 00:05 +0800, Chen Baozi wrote:
>> 在 2013-7-17,23:26,Ian Campbell <Ian.Campbell@citrix.com> 写道:
>> 
>>>>>> "restoring CPSR" refers to the instruction "msr CPSR_c, <reg>" which
>>>>>> is from "local_irq_restore". And "cpsie i" is from the call to
>>>>>> local_irq_enable".
>>>>> 
>>>>> Ah right. So in both cases you will immediately take any pending
>>>>> interrupt. I think I would continue instrumenting starting from
>>>>> gic_interrupt() and hopefully eventually into the ns16550 interrupt
>>>>> handler.
>>>> 
>>>> I went through gic_interrupt() and thought got the points cause the stuck.
>>> 
>>> Please can you clarify exactly what you mean by "stuck". Previously you
>>> thought it was stuck in ns16550_setup_postirq when in actual fact it was
>>> taking an interrupt.
>> 
>> I thought it was "stuck" because since every time I pressed 'd' to
>> dump the registers the PC always stayed at the same position during
>> executing ns16550_setup_postirq. So it really looks like that that the
>> system get stuck at that point. Sorry if I made a wrong description.
> 
> No problem. In fact if 'd' works perhaps you are not blowing the stack
> at all with multiple interrupts.
> 
> Ah, you are probably never escaping the loop in gic_interrupt because
> the read of IAR always returns the UART interrupt.
> 
>> 
>>> Are you sure that you are taking multiple,
>>> potentially nested interrupts and eventually blowing the hypervisor
>>> stack? This seems like the most likely scenario to me.
>> 
>> Seems reasonable. Is there any way to prove that we are under this
>> situation? I didn't expect this possibility before. Thanks.
> 
> I was about to say that a printk in gic_interrupt ought to confirm, but
> since the UART IRQ is the problem perhaps that isn't so obvious, unless
> sync_console helps in some way. Worth a try.
> 
> If not then since 'd' works then perhaps you could keep a count of the
> number serial IRQs in a global var and dump it?

Thanks. I'll have a try.

> 
>> 
>>> 
>>>> If I change the while(...) in ns16550_interrupt() into if(...) and comment
>>>> either "GICC[GICC_EOIR] = irq;" or "GICC[GICC_DIR] = irq;" in
>>>> git_host_irq_end(), it won't get stuck after enabling receive and transmit
>>>> interrupts in ns16550_setup_postirq().
>>> 
>>> By removing the writes to either EOIR or DIR you are in effect never
>>> unmasking the interrupt, so you avoid the nest interrupt problem.
>>> 
>>> If this is the case then real issue is perhaps that for whatever reason
>>> ns16550_interrupt is not causing the hardware to deassert its interrupt
>>> line.
>>> 
>>> The UART on the sunxi is compatible (in DTS terms) with
>>> "snps,dw-apb-uart", which seems to be an 8250 variant, but one which
>>> differs enough to warrant its own compatibility string -- perhaps Xen's
>>> ns16550 driver isn't dealing with some quirk of this device?
>> 
>> I checked my OMAP5's data sheet. Generally, they looks very similar.
>> But I will read the manual more carefully again tomorrow to make sure
>> this point.
> 
> Good idea.
> 
>> 
>>> 
>>> It seems like the driver in Linux is drivers/tty/serial/8250/8250_dw.c.
>>> dw8250_handle_irq looks interesting...
>>> 
>>>       struct dw8250_data *d = p->private_data;
>>>       unsigned int iir = p->serial_in(p, UART_IIR);
>>> 
>>>       if (serial8250_handle_irq(p, iir)) {
>>>               return 1;
>>>       } else if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
>>>               /* Clear the USR and write the LCR again. */
>>>               (void)p->serial_in(p, DW_UART_USR);
>>>               p->serial_out(p, UART_LCR, d->last_lcr);
>>> 
>>>               return 1;
>>>       }
>>> 
>>>       return 0;
>>> 
>>> In particular the fallback code there when the common 8250 handler
>>> didn't deal with the issue...
>> 
>> I'll get down to the Linux driver tomorrow to see whether I could catch the point.
> 
> Actually, the comment at the top is interesting:
> 12  * The Synopsys DesignWare 8250 has an extra feature whereby it detects if the
> 13  * LCR is written whilst busy.  If it is, then a busy detect interrupt is
> 14  * raised, the LCR needs to be rewritten and the uart status register read.
> 
> I'm not sure that "extra feature" doesn't mean "weird quirk" but there we go ;-)
> 
> The changelog of the patch which added it is interesting too:
> http://permalink.gmane.org/gmane.linux.serial/5855

I checked the Linux driver today. Since the UART of my OMAP5432 board is compatible with "ti,omap4-uart", the driver in Linux should be drivers/tty/serial/omap-serial.c rather than drivers/tty/serial/8250/8250_dw.c.

In serial_omap_irq of omap-serial.c, there are no such fallback codes as DesignWare's. However, it does check the modem status register. I used to think this would be the point, because "Modem Status" interrupt must be cleared by reading the modem status register. However, it seems reading this register doesn't work :-(

Baozi



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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5
  2013-07-18 12:44                     ` Chen Baozi
@ 2013-07-19  8:25                       ` Chen Baozi
  0 siblings, 0 replies; 17+ messages in thread
From: Chen Baozi @ 2013-07-19  8:25 UTC (permalink / raw)
  To: Ian Campbell; +Cc: <xen-devel@lists.xen.org>


On Jul 18, 2013, at 8:44 PM, Chen Baozi <baozich@gmail.com> wrote:

> 
> On Jul 18, 2013, at 7:53 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> 
>> On Thu, 2013-07-18 at 00:05 +0800, Chen Baozi wrote:
>>> 在 2013-7-17,23:26,Ian Campbell <Ian.Campbell@citrix.com> 写道:
>>> 
>>>>>>> "restoring CPSR" refers to the instruction "msr CPSR_c, <reg>" which
>>>>>>> is from "local_irq_restore". And "cpsie i" is from the call to
>>>>>>> local_irq_enable".
>>>>>> 
>>>>>> Ah right. So in both cases you will immediately take any pending
>>>>>> interrupt. I think I would continue instrumenting starting from
>>>>>> gic_interrupt() and hopefully eventually into the ns16550 interrupt
>>>>>> handler.
>>>>> 
>>>>> I went through gic_interrupt() and thought got the points cause the stuck.
>>>> 
>>>> Please can you clarify exactly what you mean by "stuck". Previously you
>>>> thought it was stuck in ns16550_setup_postirq when in actual fact it was
>>>> taking an interrupt.
>>> 
>>> I thought it was "stuck" because since every time I pressed 'd' to
>>> dump the registers the PC always stayed at the same position during
>>> executing ns16550_setup_postirq. So it really looks like that that the
>>> system get stuck at that point. Sorry if I made a wrong description.
>> 
>> No problem. In fact if 'd' works perhaps you are not blowing the stack
>> at all with multiple interrupts.
>> 
>> Ah, you are probably never escaping the loop in gic_interrupt because
>> the read of IAR always returns the UART interrupt.
>> 
>>> 
>>>> Are you sure that you are taking multiple,
>>>> potentially nested interrupts and eventually blowing the hypervisor
>>>> stack? This seems like the most likely scenario to me.
>>> 
>>> Seems reasonable. Is there any way to prove that we are under this
>>> situation? I didn't expect this possibility before. Thanks.
>> 
>> I was about to say that a printk in gic_interrupt ought to confirm, but
>> since the UART IRQ is the problem perhaps that isn't so obvious, unless
>> sync_console helps in some way. Worth a try.
>> 
>> If not then since 'd' works then perhaps you could keep a count of the
>> number serial IRQs in a global var and dump it?
> 
> Thanks. I'll have a try.
> 
>> 
>>> 
>>>> 
>>>>> If I change the while(...) in ns16550_interrupt() into if(...) and comment
>>>>> either "GICC[GICC_EOIR] = irq;" or "GICC[GICC_DIR] = irq;" in
>>>>> git_host_irq_end(), it won't get stuck after enabling receive and transmit
>>>>> interrupts in ns16550_setup_postirq().
>>>> 
>>>> By removing the writes to either EOIR or DIR you are in effect never
>>>> unmasking the interrupt, so you avoid the nest interrupt problem.
>>>> 
>>>> If this is the case then real issue is perhaps that for whatever reason
>>>> ns16550_interrupt is not causing the hardware to deassert its interrupt
>>>> line.
>>>> 
>>>> The UART on the sunxi is compatible (in DTS terms) with
>>>> "snps,dw-apb-uart", which seems to be an 8250 variant, but one which
>>>> differs enough to warrant its own compatibility string -- perhaps Xen's
>>>> ns16550 driver isn't dealing with some quirk of this device?
>>> 
>>> I checked my OMAP5's data sheet. Generally, they looks very similar.
>>> But I will read the manual more carefully again tomorrow to make sure
>>> this point.
>> 
>> Good idea.
>> 
>>> 
>>>> 
>>>> It seems like the driver in Linux is drivers/tty/serial/8250/8250_dw.c.
>>>> dw8250_handle_irq looks interesting...
>>>> 
>>>>      struct dw8250_data *d = p->private_data;
>>>>      unsigned int iir = p->serial_in(p, UART_IIR);
>>>> 
>>>>      if (serial8250_handle_irq(p, iir)) {
>>>>              return 1;
>>>>      } else if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
>>>>              /* Clear the USR and write the LCR again. */
>>>>              (void)p->serial_in(p, DW_UART_USR);
>>>>              p->serial_out(p, UART_LCR, d->last_lcr);
>>>> 
>>>>              return 1;
>>>>      }
>>>> 
>>>>      return 0;
>>>> 
>>>> In particular the fallback code there when the common 8250 handler
>>>> didn't deal with the issue...
>>> 
>>> I'll get down to the Linux driver tomorrow to see whether I could catch the point.
>> 
>> Actually, the comment at the top is interesting:
>> 12  * The Synopsys DesignWare 8250 has an extra feature whereby it detects if the
>> 13  * LCR is written whilst busy.  If it is, then a busy detect interrupt is
>> 14  * raised, the LCR needs to be rewritten and the uart status register read.
>> 
>> I'm not sure that "extra feature" doesn't mean "weird quirk" but there we go ;-)
>> 
>> The changelog of the patch which added it is interesting too:
>> http://permalink.gmane.org/gmane.linux.serial/5855
> 
> I checked the Linux driver today. Since the UART of my OMAP5432 board is compatible with "ti,omap4-uart", the driver in Linux should be drivers/tty/serial/omap-serial.c rather than drivers/tty/serial/8250/8250_dw.c.
> 
> In serial_omap_irq of omap-serial.c, there are no such fallback codes as DesignWare's. However, it does check the modem status register. I used to think this would be the point, because "Modem Status" interrupt must be cleared by reading the modem status register. However, it seems reading this register doesn't work :-(

Hurrah! I've finally made it.

It is about the bit flags to be set when enabling interrupts in ns16550_setup_postirq. I checked the startup function of 8250 compatible drivers in Linux, in which "receiver data interrupt" (bit 0) and "receiver line status interrupt" (bit 2) are set. However, in ns16550_setup_postirq, they are "receiver data interrupt" (bit 0) and "transmitter holding register empty interrupt" (bit 1). So after applying the following changes in ns16550_setup_postirq:

-		ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ETHREI);
+		ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ELSI);

UART console works!

And a printk in ns16550_interrupt works. I used it to check the register flags and was finally led to the point.

I'm wondering if this would be a common fix to ns16550 driver considering Linux driver doesn't set UART_IER_THRI (UART_IER_ETHREI in ns16550 driver of Xen) while enabling interrupts for 8250.

Cheers,

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2013-07-19  8:25 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-11 10:53 Problems after enabling rcv/xmit interrupts of ns16550 on OMAP5 Chen Baozi
2013-07-11 11:21 ` Ian Campbell
2013-07-11 11:37   ` Chen Baozi
2013-07-11 12:44   ` Chen Baozi
2013-07-11 12:46     ` Ian Campbell
2013-07-11 12:54       ` Chen Baozi
2013-07-12  7:41   ` Chen Baozi
2013-07-17  8:02     ` Chen Baozi
2013-07-17  8:21       ` Ian Campbell
2013-07-17  9:01         ` Chen Baozi
2013-07-17  9:21           ` Ian Campbell
2013-07-17 13:53             ` Chen Baozi
2013-07-17 15:26               ` Ian Campbell
2013-07-17 16:05                 ` Chen Baozi
2013-07-18 11:53                   ` Ian Campbell
2013-07-18 12:44                     ` Chen Baozi
2013-07-19  8:25                       ` Chen Baozi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.