All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support.
@ 2013-09-10 14:18 Ian Campbell
  2013-09-10 14:18 ` [PATCH RFC 1/8] xen/arm: Implement ioremap Ian Campbell
                   ` (8 more replies)
  0 siblings, 9 replies; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 14:18 UTC (permalink / raw)
  To: xen-devel
  Cc: keir, Ian Campbell, stefano.stabellini, julien.grall, tim,
	jbuelich, Josh Zhao

The following patches are a very early set of cubieboard patches, based
on latest staging, eb786f709249666e1f7364706f94be1a6c4e04da

Several are not to be applied because they can be done better using
infrastructure from Julien's "Allow Xen to boot with a raw Device Tree"
patch. They are included for completeness.

With this I can boot up until dom0 panics due to lack of a root
filesystem. This is because upstream Linux currently lacks any useful
storage drivers (SATA, USB host, MMC). Hopefully this will resolve
itself soon!

The bulk of this is the hacking around of the ns16550 driver to support
ARM platforms and DTB.

It's currently a bit of a faff to get going. Hopefully the following is
helpful:

u-boot
======

https://github.com/linux-sunxi/u-boot-sunxi.git linux-sunxi/sunxi
commit e94cff93c273b0825bea135e0f559f5580156fa6

Plus git://git.linaro.org/people/aprzywara/u-boot.git hypmode_v4
commit 5068b337518617586f2e51b6d616c54dbec4fa62 

Plus patches for hypmode on sunxi and to initialise CNTFRQ.

All of which can be found in
     git://xenbits.xen.org/people/ianc/u-boot.git devel/cubieboard2

Built with:
     make Cubieboard2_FEL CROSS_COMPILE=arm-linux-gnueabihf-

Linux
=====

Linux tree:
https://github.com/mripard/linux.git sunxi/dt-for-3.12
commit 82abe5294aeadc42508c7944f3a9aec0eece214c

Using in tree DTS arch/arm/boot/dts/sun7i-a20-cubieboard2.dts with the
following patch.

Build Image and dtbs in the usual way

Xen
===

Build in the usual way.

Xen can use CONFIG_EARLY_PRINTK=sun7i.

Booting
=======

I am using FEL boot per http://linux-sunxi.org/FEL/USBBoot and
http://linux-sunxi.org/FEL using the fel-sdboot.sunxi on MMC
mechanism.

linux-sunxi tools from git://github.com/linux-sunxi/sunxi-tools.git
commit b398456630b310dbf31c7515e8d6af37903c4975 and:

./usb-boot ~/devel/u-boot.git/spl/u-boot-spl.bin ~/devel/u-boot.git/u-boot.bin \
           bootxen.scr ~/devel/xen.git/xen/xen \
	   ~/devel/linux.git/arch/arm/boot/dts/sun7i-a20-cubieboard2.dtb
	   ~/devel/linux.git/arch/arm/boot/zImage

bootxen.scr is:
$ cat bootxen
setenv bootargs dtuart=serial0 earlyprint loglvl=all conswitch=x
bootz 0x44000000 - 0x43000000
$ mkimage -T script -A arm -d bootxen bootxen.scr 

DTS patch
=========


diff --git a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
 31b76f0..6c0c387 100644
--- a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
+++ b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
@@ -18,6 +18,21 @@
 	model = "Cubietech Cubieboard2";
 	compatible = "cubietech,cubieboard2", "allwinner,sun7i-a20";
 
+	chosen {
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		module@0 {
+			compatible = "xen,linux-zimage", "xen,multiboot-module";
+			bootargs = "console=hvc0 earlyprintk ";
+			reg = <0x50000000 0x800000>;
+		};
+	};
+
+	aliases {
+		serial0 = &uart0;
+	};
+
 	soc@01c00000 {
 		pinctrl@01c20800 {
 			led_pins_cubieboard2: led_pins@0 {
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index f4e4524..bfc5dc2 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -26,11 +26,6 @@
 			reg = <0>;
 		};
 
-		cpu@1 {
-			compatible = "arm,cortex-a7";
-			device_type = "cpu";
-			reg = <1>;
-		};
 	};
 
 	memory {
@@ -55,6 +55,14 @@
 		};
 	};
 
+        timer {
+                compatible = "arm,armv7-timer";
+                interrupts = <1 13 0xf08>,
+                             <1 14 0xf08>,
+                             <1 11 0xf08>,
+                             <1 10 0xf08>;
+        };
+
 	soc@01c00000 {
 		compatible = "simple-bus";
 		#address-cells = <1>;
@@ -94,17 +102,6 @@
 			};
 		};
 
-		timer@01c20c00 {
-			compatible = "allwinner,sun4i-timer";
-			reg = <0x01c20c00 0x90>;
-			interrupts = <0 22 1>,
-				     <0 23 1>,
-				     <0 24 1>,
-				     <0 25 1>,
-				     <0 67 1>,
-				     <0 68 1>;
-			clocks = <&osc24M>;
-		};
 
 		wdt: watchdog@01c20c90 {
 			compatible = "allwinner,sun4i-wdt";

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

* [PATCH RFC 1/8] xen/arm: Implement ioremap.
  2013-09-10 14:18 [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support Ian Campbell
@ 2013-09-10 14:18 ` Ian Campbell
  2013-09-10 15:03   ` Julien Grall
  2013-09-10 14:18 ` [PATCH RFC 2/8] xen/arm: implement read[bl] and write[bl] Ian Campbell
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 14:18 UTC (permalink / raw)
  To: xen-devel; +Cc: Josh Zhao, julien.grall, tim, Ian Campbell, stefano.stabellini

Common code uses this, it expects an uncached mapping.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/mm.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 69c157a..4521c8d 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -694,6 +694,11 @@ void *ioremap_attr(paddr_t pa, size_t len, unsigned int attributes)
     return (__vmap(&pfn, nr, 1, 1, attributes) + offs);
 }
 
+void *ioremap(paddr_t pa, size_t len)
+{
+    return ioremap_attr(pa, len, PAGE_HYPERVISOR_NOCACHE);
+}
+
 static int create_xen_table(lpae_t *entry)
 {
     void *p;
-- 
1.7.10.4

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

* [PATCH RFC 2/8] xen/arm: implement read[bl] and write[bl]
  2013-09-10 14:18 [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support Ian Campbell
  2013-09-10 14:18 ` [PATCH RFC 1/8] xen/arm: Implement ioremap Ian Campbell
@ 2013-09-10 14:18 ` Ian Campbell
  2013-09-10 15:00   ` Julien Grall
  2013-09-10 14:18 ` [PATCH RFC 3/8] ns16550: make usable on ARM Ian Campbell
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 14:18 UTC (permalink / raw)
  To: xen-devel; +Cc: Josh Zhao, julien.grall, tim, Ian Campbell, stefano.stabellini

These are used in common driver code (specifically ns16550)

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/include/asm-arm/io.h |   48 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

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:
-- 
1.7.10.4

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

* [PATCH RFC 3/8] ns16550: make usable on ARM
  2013-09-10 14:18 [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support Ian Campbell
  2013-09-10 14:18 ` [PATCH RFC 1/8] xen/arm: Implement ioremap Ian Campbell
  2013-09-10 14:18 ` [PATCH RFC 2/8] xen/arm: implement read[bl] and write[bl] Ian Campbell
@ 2013-09-10 14:18 ` Ian Campbell
  2013-09-10 14:36   ` Keir Fraser
  2013-09-10 15:00   ` Jan Beulich
  2013-09-10 14:18 ` [PATCH RFC 4/8] ns16550: support DesignWare 8250 Ian Campbell
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 14:18 UTC (permalink / raw)
  To: xen-devel
  Cc: keir, Ian Campbell, stefano.stabellini, julien.grall, tim,
	jbeulich, Josh Zhao

There are several aspects to this:
- Correctly conditionalise use of PCI
- Correctly conditionalise use of IO ports
- Add discovery via device tree
- Support different registers shift/stride and widths
- Add vuart hooks.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: keir@xen.org
Cc: jbeulich@suse.com
---
 config/arm32.mk            |    1 +
 xen/Rules.mk               |    3 +
 xen/arch/x86/Rules.mk      |    1 +
 xen/drivers/char/ns16550.c |  183 +++++++++++++++++++++++++++++++++++++++++---
 4 files changed, 176 insertions(+), 12 deletions(-)

diff --git a/config/arm32.mk b/config/arm32.mk
index 76e229d..aa79d22 100644
--- a/config/arm32.mk
+++ b/config/arm32.mk
@@ -12,6 +12,7 @@ CFLAGS += -marm
 HAS_PL011 := y
 HAS_EXYNOS4210 := y
 HAS_OMAP := y
+HAS_NS16550 := y
 
 # Use only if calling $(LD) directly.
 LDFLAGS_DIRECT += -EL
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 736882a..df1428f 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -60,6 +60,9 @@ CFLAGS-$(lock_profile)  += -DLOCK_PROFILE
 CFLAGS-$(HAS_ACPI)      += -DHAS_ACPI
 CFLAGS-$(HAS_GDBSX)     += -DHAS_GDBSX
 CFLAGS-$(HAS_PASSTHROUGH) += -DHAS_PASSTHROUGH
+CFLAGS-$(HAS_DEVICE_TREE) += -DHAS_DEVICE_TREE
+CFLAGS-$(HAS_PCI)       += -DHAS_PCI
+CFLAGS-$(HAS_IOPORTS)   += -DHAS_IOPORTS
 CFLAGS-$(frame_pointer) += -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER
 
 ifneq ($(max_phys_cpus),)
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index eb11b5b..c93d2af 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -1,6 +1,7 @@
 ########################################
 # x86-specific definitions
 
+HAS_IOPORTS := y
 HAS_ACPI := y
 HAS_VGA  := y
 HAS_VIDEO  := y
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index e0f80f6..854a572 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -13,14 +13,19 @@
 #include <xen/init.h>
 #include <xen/irq.h>
 #include <xen/sched.h>
-#include <xen/pci.h>
 #include <xen/timer.h>
 #include <xen/serial.h>
 #include <xen/iocap.h>
+#ifdef HAS_PCI
 #include <xen/pci.h>
 #include <xen/pci_regs.h>
+#endif
 #include <xen/8250-uart.h>
+#include <xen/vmap.h>
 #include <asm/io.h>
+#ifdef HAS_DEVICE_TREE
+#include <asm/device.h>
+#endif
 #ifdef CONFIG_X86
 #include <asm/fixmap.h>
 #endif
@@ -40,15 +45,22 @@ 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;
+    int reg_shift; /* Bits to shift register offset by */
+    int reg_width; /* Number of bytes in each register */
     char __iomem *remapped_io_base;  /* Remapped virtual address of MMIO. */
     /* UART with IRQ line: interrupt-driven I/O. */
     struct irqaction irqaction;
+#ifdef CONFIG_ARM
+    struct vuart_info vuart;
+#endif
     /* UART with no IRQ line: periodically-polled I/O. */
     struct timer timer;
     struct timer resume_timer;
     unsigned int timeout_ms;
     bool_t intr_works;
+#ifdef HAS_PCI
     /* PCI card parameters. */
     unsigned int pb_bdf[3]; /* pci bridge BDF */
     unsigned int ps_bdf[3]; /* pci serial port BDF */
@@ -57,22 +69,46 @@ static struct ns16550 {
     u32 bar;
     u16 cr;
     u8 bar_idx;
+#endif
+#ifdef HAS_DEVICE_TREE
+    struct dt_irq dt_irq;
+#endif
 } ns16550_com[2] = { { 0 } };
 
 static void ns16550_delayed_resume(void *data);
 
 static char ns_read_reg(struct ns16550 *uart, int reg)
 {
+    void __iomem *addr = uart->remapped_io_base + (reg<<uart->reg_shift);
+#ifdef HAS_IOPORTS
     if ( uart->remapped_io_base == NULL )
         return inb(uart->io_base + reg);
-    return readb(uart->remapped_io_base + reg);
+#endif
+    switch (uart->reg_width) {
+    default: /* assume single byte */
+    case 1:
+        return readb(addr);
+    case 4:
+        return readl(addr);
+    }
 }
 
 static void ns_write_reg(struct ns16550 *uart, int reg, char c)
 {
+    void __iomem *addr = uart->remapped_io_base + (reg<<uart->reg_shift);
+#ifdef HAS_IOPORTS
     if ( uart->remapped_io_base == NULL )
         return outb(c, uart->io_base + reg);
-    writeb(c, uart->remapped_io_base + reg);
+#endif
+    switch (uart->reg_width) {
+    default: /* assume single byte */
+    case 1:
+        writeb(c, addr);
+        break;
+    case 4:
+        writel(c, addr);
+        break;
+    }
 }
 
 static int ns16550_ioport_invalid(struct ns16550 *uart)
@@ -161,6 +197,7 @@ static int ns16550_getc(struct serial_port *port, char *pc)
     return 1;
 }
 
+#ifdef HAS_PCI
 static void pci_serial_early_init(struct ns16550 *uart)
 {
     if ( !uart->ps_bdf_enable || uart->io_base >= 0x10000 )
@@ -178,6 +215,9 @@ 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);
 }
+#else
+static void pci_serial_early_init(struct ns16550 *uart) {}
+#endif
 
 static void ns16550_setup_preirq(struct ns16550 *uart)
 {
@@ -223,8 +263,10 @@ static void __init ns16550_init_preirq(struct serial_port *port)
 {
     struct ns16550 *uart = port->uart;
 
+#ifdef HAS_IOPORTS
     /* I/O ports are distinguished by their size (16 bits). */
     if ( uart->io_base >= 0x10000 )
+#endif
     {
 #ifdef CONFIG_X86
         enum fixed_addresses idx = FIX_COM_BEGIN + (uart - ns16550_com);
@@ -233,7 +275,7 @@ 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 = (char *)ioremap(uart->io_base, uart->io_size);
 #endif
     }
 
@@ -278,21 +320,27 @@ static void __init ns16550_init_postirq(struct serial_port *port)
     bits = uart->data_bits + uart->stop_bits + !!uart->parity;
     uart->timeout_ms = max_t(
         unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
-
     if ( uart->irq > 0 )
     {
         uart->irqaction.handler = ns16550_interrupt;
         uart->irqaction.name    = "ns16550";
         uart->irqaction.dev_id  = port;
+#ifdef HAS_DEVICE_TREE
+        if ( (rc = setup_dt_irq(&uart->dt_irq, &uart->irqaction)) != 0 )
+            printk("ERROR: Failed to allocate ns16550 DT IRQ.\n");
+#else
         if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
             printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq);
+#endif
     }
 
     ns16550_setup_postirq(uart);
 
+#ifdef HAS_PCI
     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)
@@ -301,13 +349,16 @@ static void ns16550_suspend(struct serial_port *port)
 
     stop_timer(&uart->timer);
 
+#ifdef HAS_PCI
     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 HAS_PCI
     struct ns16550 *uart = port->uart;
 
     if ( uart->bar )
@@ -317,6 +368,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);
@@ -360,19 +412,17 @@ static void ns16550_resume(struct serial_port *port)
         _ns16550_resume(port);
 }
 
-#ifdef CONFIG_X86
 static void __init ns16550_endboot(struct serial_port *port)
 {
+#ifdef HAS_IOPORTS
     struct ns16550 *uart = port->uart;
 
     if ( uart->remapped_io_base )
         return;
     if ( ioports_deny_access(dom0, uart->io_base, uart->io_base + 7) != 0 )
         BUG();
-}
-#else
-#define ns16550_endboot NULL
 #endif
+}
 
 static int __init ns16550_irq(struct serial_port *port)
 {
@@ -380,6 +430,23 @@ static int __init ns16550_irq(struct serial_port *port)
     return ((uart->irq > 0) ? uart->irq : -1);
 }
 
+#ifdef HAS_DEVICE_TREE
+static const struct dt_irq __init *ns16550_dt_irq(struct serial_port *port)
+{
+    struct ns16550 *uart = port->uart;
+    return &uart->dt_irq;
+}
+#endif
+
+#ifdef CONFIG_ARM
+static const struct vuart_info *ns16550_vuart_info(struct serial_port *port)
+{
+    struct ns16550 *uart = port->uart;
+
+    return &uart->vuart;
+}
+#endif
+
 static struct uart_driver __read_mostly ns16550_driver = {
     .init_preirq  = ns16550_init_preirq,
     .init_postirq = ns16550_init_postirq,
@@ -389,7 +456,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,
+#ifdef HAS_DEVICE_TREE
+    .dt_irq_get   = ns16550_dt_irq,
+#endif
+#ifdef CONFIG_ARM
+    .vuart_info   = ns16550_vuart_info,
+#endif
 };
 
 static int __init parse_parity_char(int c)
@@ -414,15 +487,21 @@ static int __init check_existence(struct ns16550 *uart)
 {
     unsigned char status, scratch, scratch2, scratch3;
 
+#ifdef HAS_IO_PORTS
     /*
      * We can't poke MMIO UARTs until they get I/O remapped later. Assume that
      * if we're getting MMIO UARTs, the arch code knows what it's doing.
      */
     if ( uart->io_base >= 0x10000 )
         return 1;
+#else
+    return 1; /* Everything is MMIO */
+#endif
 
+#ifdef HAS_PCI
     pci_serial_early_init(uart);
-    
+#endif
+
     /*
      * Do a simple existence test first; if we fail this,
      * there's no point trying anything else.
@@ -450,6 +529,7 @@ static int __init check_existence(struct ns16550 *uart)
     return (status == 0x90);
 }
 
+#ifdef HAS_PCI
 static int
 pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
 {
@@ -518,6 +598,7 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
 
     return 0;
 }
+#endif
 
 #define PARSE_ERR(_f, _a...)                 \
     do {                                     \
@@ -564,6 +645,7 @@ static void __init ns16550_parse_port_config(
 
     if ( *conf == ',' && *++conf != ',' )
     {
+#ifdef HAS_PCI
         if ( strncmp(conf, "pci", 3) == 0 )
         {
             if ( pci_uart_config(uart, 1/* skip AMT */, uart - ns16550_com) )
@@ -577,6 +659,7 @@ static void __init ns16550_parse_port_config(
             conf += 3;
         }
         else
+#endif
         {
             uart->io_base = simple_strtoul(conf, &conf, 0);
         }
@@ -585,6 +668,7 @@ static void __init ns16550_parse_port_config(
     if ( *conf == ',' && *++conf != ',' )
         uart->irq = simple_strtol(conf, &conf, 10);
 
+#ifdef HAS_PCI
     if ( *conf == ',' && *++conf != ',' )
     {
         conf = parse_pci(conf, NULL, &uart->ps_bdf[0],
@@ -601,6 +685,7 @@ static void __init ns16550_parse_port_config(
             PARSE_ERR("Bad bridge PCI coordinates");
         uart->pb_bdf_enable = 1;
     }
+#endif
 
  config_parsed:
     /* Sanity checks. */
@@ -638,12 +723,86 @@ void __init ns16550_init(int index, struct ns16550_defaults *defaults)
     uart->stop_bits = defaults->stop_bits;
     uart->irq       = defaults->irq;
     uart->io_base   = defaults->io_base;
+    uart->io_size   = 8;
+    uart->reg_width = 1;
+    uart->reg_shift = 0;
+
     /* Default is no transmit FIFO. */
     uart->fifo_size = 1;
 
     ns16550_parse_port_config(uart, (index == 0) ? opt_com1 : opt_com2);
 }
 
+#ifdef HAS_DEVICE_TREE
+static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
+                                       const void *data)
+{
+    struct ns16550 *uart;
+    int res;
+    u32 reg_shift, reg_width;
+
+    uart = &ns16550_com[0];
+
+    uart->baud      = BAUD_AUTO;
+    uart->clock_hz  = UART_CLOCK_HZ;
+    uart->data_bits = 8;
+    uart->parity    = UART_PARITY_NONE;
+    uart->stop_bits = 1;
+    /* Default is no transmit FIFO. */
+    uart->fifo_size = 1;
+
+    res = dt_device_get_address(dev, 0, &uart->io_base, &uart->io_size);
+    if ( res )
+        return res;
+
+    res = dt_property_read_u32(dev, "reg-shift", &reg_shift);
+    if ( !res )
+        uart->reg_shift = 0;
+    else
+        uart->reg_shift = reg_shift;
+
+    res = dt_property_read_u32(dev, "reg-io-width", &reg_width);
+    if ( !res )
+        uart->reg_width = 1;
+    else
+        uart->reg_width = reg_width;
+
+    if ( uart->reg_width != 1 && uart->reg_width != 4 )
+        return -EINVAL;
+
+    res = dt_device_get_irq(dev, 0, &uart->dt_irq);
+    if ( res )
+        return res;
+
+    /* The common bit of the driver mostly deals with irq not dt_irq. */
+    uart->irq = uart->dt_irq.irq;
+
+    uart->vuart.base_addr = uart->io_base;
+    uart->vuart.size = uart->io_size;
+    uart->vuart.data_off = UART_THR <<uart->reg_shift;
+    uart->vuart.status_off = UART_LSR<<uart->reg_shift;
+    uart->vuart.status = UART_LSR_THRE|UART_LSR_TEMT;
+
+    /* Register with generic serial driver. */
+    serial_register_uart(uart - ns16550_com, &ns16550_driver, uart);
+
+    dt_device_set_used_by(dev, DOMID_XEN);
+
+    return 0;
+}
+
+static const char const *ns16550_dt_compat[] __initdata =
+{
+    "ns16550",
+    NULL
+};
+
+DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
+        .compatible = ns16550_dt_compat,
+        .init = ns16550_uart_dt_init,
+DT_DEVICE_END
+
+#endif /* HAS_DEVICE_TREE */
 /*
  * Local variables:
  * mode: C
-- 
1.7.10.4

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

* [PATCH RFC 4/8] ns16550: support DesignWare 8250
  2013-09-10 14:18 [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support Ian Campbell
                   ` (2 preceding siblings ...)
  2013-09-10 14:18 ` [PATCH RFC 3/8] ns16550: make usable on ARM Ian Campbell
@ 2013-09-10 14:18 ` Ian Campbell
  2013-09-10 14:36   ` Keir Fraser
  2013-09-10 15:02   ` Jan Beulich
  2013-09-10 14:18 ` [PATCH RFC 5/8] xen/arm: Support Cortex-A7 GIC Ian Campbell
                   ` (4 subsequent siblings)
  8 siblings, 2 replies; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 14:18 UTC (permalink / raw)
  To: xen-devel
  Cc: keir, Ian Campbell, stefano.stabellini, julien.grall, tim,
	jbeulich, Josh Zhao

This hardware has an additional feature which signals an error if you try to
write LCR while the UART is busy. We need to clear this error during setup,
otherwise LCR.DLAB doesn't get set and we cannot read/write the divisor.

This has been tested on the cubieboard2

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: keir@xen.org
Cc: jbeulich@suse.com
---
 xen/drivers/char/ns16550.c  |   14 ++++++++++++++
 xen/include/xen/8250-uart.h |    2 ++
 2 files changed, 16 insertions(+)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 854a572..c580bda 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -60,6 +60,7 @@ static struct ns16550 {
     struct timer resume_timer;
     unsigned int timeout_ms;
     bool_t intr_works;
+    bool_t dw_usr_bsy;
 #ifdef HAS_PCI
     /* PCI card parameters. */
     unsigned int pb_bdf[3]; /* pci bridge BDF */
@@ -233,6 +234,16 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
     /* No interrupts. */
     ns_write_reg(uart, UART_IER, 0);
 
+    if ( uart->dw_usr_bsy &&
+         (ns_read_reg(uart, UART_IIR) & UART_IIR_BSY) == UART_IIR_BSY )
+    {
+        /* DesignWare 8250 detects if LCR is written while the UART is
+         * busy and raises a "busy detect" interrupt. Read the UART
+         * Status Register to clear this state.
+         */
+        (void)ns_read_reg(uart, UART_USR);
+    }
+
     /* Line control and baud-rate generator. */
     ns_write_reg(uart, UART_LCR, lcr | UART_LCR_DLAB);
     if ( uart->baud != BAUD_AUTO )
@@ -777,6 +788,8 @@ static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
     /* The common bit of the driver mostly deals with irq not dt_irq. */
     uart->irq = uart->dt_irq.irq;
 
+    uart->dw_usr_bsy = dt_device_is_compatible(dev, "snps,dw-apb-uart");
+
     uart->vuart.base_addr = uart->io_base;
     uart->vuart.size = uart->io_size;
     uart->vuart.data_off = UART_THR <<uart->reg_shift;
@@ -794,6 +807,7 @@ static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
 static const char const *ns16550_dt_compat[] __initdata =
 {
     "ns16550",
+    "snps,dw-apb-uart",
     NULL
 };
 
diff --git a/xen/include/xen/8250-uart.h b/xen/include/xen/8250-uart.h
index 8693d15..a682bae 100644
--- a/xen/include/xen/8250-uart.h
+++ b/xen/include/xen/8250-uart.h
@@ -32,6 +32,7 @@
 #define UART_MCR          0x04    /* Modem control        */
 #define UART_LSR          0x05    /* line status          */
 #define UART_MSR          0x06    /* Modem status         */
+#define UART_USR          0x1f    /* Status register (DW) */
 #define UART_DLL          0x00    /* divisor latch (ls) (DLAB=1) */
 #define UART_DLM          0x01    /* divisor latch (ms) (DLAB=1) */
 
@@ -48,6 +49,7 @@
 #define UART_IIR_RDA      0x04    /*  - rx data recv'd    */
 #define UART_IIR_THR      0x02    /*  - tx reg. empty     */
 #define UART_IIR_MSI      0x00    /*  - MODEM status      */
+#define UART_IIR_BSY      0x07    /*  - busy detect (DW) */
 
 /* FIFO Control Register */
 #define UART_FCR_ENABLE   0x01    /* enable FIFO          */
-- 
1.7.10.4

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

* [PATCH RFC 5/8] xen/arm: Support Cortex-A7 GIC
  2013-09-10 14:18 [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support Ian Campbell
                   ` (3 preceding siblings ...)
  2013-09-10 14:18 ` [PATCH RFC 4/8] ns16550: support DesignWare 8250 Ian Campbell
@ 2013-09-10 14:18 ` Ian Campbell
  2013-09-10 14:18 ` [PATCH RFC 6/8] xen/arm: Basic support for sunxi/sun7i platform Ian Campbell
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 14:18 UTC (permalink / raw)
  To: xen-devel; +Cc: Josh Zhao, julien.grall, tim, Ian Campbell, stefano.stabellini

NB: NOT TO BE APPLIED

Julien has a patch to make this use a match list. This should be rebased onto
that.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/gic.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 7c24811..ab0f00d 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -360,6 +360,8 @@ void __init gic_init(void)
 
     node = dt_find_interrupt_controller("arm,cortex-a15-gic");
     if ( !node )
+        node = dt_find_interrupt_controller("arm,cortex-a7-gic");
+    if ( !node )
         panic("Unable to find compatible GIC in the device tree\n");
 
     dt_device_set_used_by(node, DOMID_XEN);
-- 
1.7.10.4

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

* [PATCH RFC 6/8] xen/arm: Basic support for sunxi/sun7i platform.
  2013-09-10 14:18 [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support Ian Campbell
                   ` (4 preceding siblings ...)
  2013-09-10 14:18 ` [PATCH RFC 5/8] xen/arm: Support Cortex-A7 GIC Ian Campbell
@ 2013-09-10 14:18 ` Ian Campbell
  2013-09-10 14:18 ` [PATCH RFC 7/8] xen/arm: Blacklist some sun7i UARTs Ian Campbell
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 14:18 UTC (permalink / raw)
  To: xen-devel; +Cc: Josh Zhao, julien.grall, tim, Ian Campbell, stefano.stabellini

Specifically cubieboard2

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/platforms/Makefile |    1 +
 xen/arch/arm/platforms/sunxi.c  |   42 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)
 create mode 100644 xen/arch/arm/platforms/sunxi.c

diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
index 4aa82e8..8c77ace 100644
--- a/xen/arch/arm/platforms/Makefile
+++ b/xen/arch/arm/platforms/Makefile
@@ -2,3 +2,4 @@ obj-y += vexpress.o
 obj-y += exynos5.o
 obj-y += midway.o
 obj-y += omap5.o
+obj-y += sunxi.o
diff --git a/xen/arch/arm/platforms/sunxi.c b/xen/arch/arm/platforms/sunxi.c
new file mode 100644
index 0000000..ee0f39b
--- /dev/null
+++ b/xen/arch/arm/platforms/sunxi.c
@@ -0,0 +1,42 @@
+/*
+ * xen/arch/arm/platforms/sunxi.c
+ *
+ * SUNXI (AllWinner A20/A31) specific settings
+ *
+ * Copyright (c) 2013 Citrix Systems.
+ *
+ * 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 <asm/p2m.h>
+#include <xen/config.h>
+#include <asm/platform.h>
+#include <xen/mm.h>
+#include <xen/device_tree.h>
+
+static const char const *sunxi_dt_compat[] __initdata =
+{
+    "allwinner,sun7i-a20",
+    NULL
+};
+
+PLATFORM_START(sunxi, "Allwinner A20")
+    .compatible = sunxi_dt_compat,
+PLATFORM_END
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.7.10.4

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

* [PATCH RFC 7/8] xen/arm: Blacklist some sun7i UARTs
  2013-09-10 14:18 [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support Ian Campbell
                   ` (5 preceding siblings ...)
  2013-09-10 14:18 ` [PATCH RFC 6/8] xen/arm: Basic support for sunxi/sun7i platform Ian Campbell
@ 2013-09-10 14:18 ` Ian Campbell
  2013-09-10 14:18 ` [PATCH RFC 8/8] xen/arm: Some early trap logging Ian Campbell
  2013-09-11  2:05 ` [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support Josh Zhao
  8 siblings, 0 replies; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 14:18 UTC (permalink / raw)
  To: xen-devel; +Cc: Josh Zhao, julien.grall, tim, Ian Campbell, stefano.stabellini

These are in the same page as the UART which is used as the Xen console. We are
not currently smart enough to avoid passing them through to the guest,
accidentally giving the guest access to the Xen console UART.

NOT TO BE APPLIED: Short term this should use Juliens forthcoming platform
blacklist patch. Long term we need to be much cleverer about devices which
share the same MMIO page...

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/platforms/sunxi.c |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/xen/arch/arm/platforms/sunxi.c b/xen/arch/arm/platforms/sunxi.c
index ee0f39b..4252580 100644
--- a/xen/arch/arm/platforms/sunxi.c
+++ b/xen/arch/arm/platforms/sunxi.c
@@ -22,6 +22,36 @@
 #include <xen/mm.h>
 #include <xen/device_tree.h>
 
+static int hide_by_name(const char *name)
+{
+    struct dt_device_node *n;
+
+    n = dt_find_node_by_path(name);
+    if ( !n )
+    {
+        printk("Unable to find %s to hide\n", name);
+        return -ENODEV;
+    }
+    dt_device_set_used_by(n, DOMID_XEN);
+    return 0;
+}
+
+static int sunxi_init(void)
+{
+    int rc;
+
+    if ( (rc = hide_by_name("/soc@01c00000/serial@01c28000")) < 0 )
+        return rc;
+    if ( (rc = hide_by_name("/soc@01c00000/serial@01c28400")) < 0 )
+        return rc;
+    if ( (rc = hide_by_name("/soc@01c00000/serial@01c28800")) < 0 )
+        return rc;
+    if ( (rc = hide_by_name("/soc@01c00000/serial@01c28c00")) < 0 )
+        return rc;
+
+    return 0;
+}
+
 static const char const *sunxi_dt_compat[] __initdata =
 {
     "allwinner,sun7i-a20",
@@ -30,6 +60,7 @@ static const char const *sunxi_dt_compat[] __initdata =
 
 PLATFORM_START(sunxi, "Allwinner A20")
     .compatible = sunxi_dt_compat,
+    .init = sunxi_init,
 PLATFORM_END
 
 /*
-- 
1.7.10.4

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

* [PATCH RFC 8/8] xen/arm: Some early trap logging
  2013-09-10 14:18 [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support Ian Campbell
                   ` (6 preceding siblings ...)
  2013-09-10 14:18 ` [PATCH RFC 7/8] xen/arm: Blacklist some sun7i UARTs Ian Campbell
@ 2013-09-10 14:18 ` Ian Campbell
  2013-09-13 13:26   ` Julien Grall
  2013-09-11  2:05 ` [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support Josh Zhao
  8 siblings, 1 reply; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 14:18 UTC (permalink / raw)
  To: xen-devel; +Cc: Josh Zhao, julien.grall, tim, Ian Campbell, stefano.stabellini

Not for really for application, might be useful to someone?

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/arm32/head.S |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 79e95b6..06d53ad 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -366,6 +366,9 @@ paging:
         bne   1b
 
 launch:
+        adr   r0, early_trap_vector
+        mcr   CP32(r0, VBAR_EL2)
+        
         ldr   r0, =init_stack        /* Find the boot-time stack */
         ldr   sp, [r0]
         add   sp, #STACK_SIZE        /* (which grows down from the top). */
@@ -376,12 +379,24 @@ launch:
         beq   start_xen              /* and disappear into the land of C */
         b     start_secondary        /* (to the appropriate entry point) */
 
+trap_early:
+        PRINT("\r\nEARLY TRAP\r\n")
 /* Fail-stop
  * r0: string explaining why */
 fail:   PRINT("- Boot failed -\r\n")
 1:      wfe
         b     1b
 
+        .align 5
+early_trap_vector:
+        .word 0                         /* 0x00 - Reset */
+        b trap_early                    /* 0x04 - Undefined Instruction */
+        b trap_early                    /* 0x08 - Supervisor Call */
+        b trap_early                    /* 0x0c - Prefetch Abort */
+        b trap_early                    /* 0x10 - Data Abort */
+        b trap_early                    /* 0x14 - Hypervisor */
+        b trap_early                    /* 0x18 - IRQ */
+        b trap_early                    /* 0x1c - FIQ */
 
 #ifdef EARLY_PRINTK
 /* Bring up the UART.
-- 
1.7.10.4

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

* Re: [PATCH RFC 3/8] ns16550: make usable on ARM
  2013-09-10 14:18 ` [PATCH RFC 3/8] ns16550: make usable on ARM Ian Campbell
@ 2013-09-10 14:36   ` Keir Fraser
  2013-09-10 14:45     ` Ian Campbell
  2013-09-10 15:00   ` Jan Beulich
  1 sibling, 1 reply; 34+ messages in thread
From: Keir Fraser @ 2013-09-10 14:36 UTC (permalink / raw)
  To: Ian Campbell, xen-devel
  Cc: Josh Zhao, julien.grall, tim, jbeulich, stefano.stabellini

On 10/09/2013 07:18, "Ian Campbell" <ian.campbell@citrix.com> wrote:

> There are several aspects to this:
> - Correctly conditionalise use of PCI
> - Correctly conditionalise use of IO ports
> - Add discovery via device tree
> - Support different registers shift/stride and widths
> - Add vuart hooks.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: keir@xen.org
> Cc: jbeulich@suse.com

Bit of an ugly ifdef mess but it is only a serial driver.

Acked-by: Keir Fraser <keir@xen.org>

> ---
>  config/arm32.mk            |    1 +
>  xen/Rules.mk               |    3 +
>  xen/arch/x86/Rules.mk      |    1 +
>  xen/drivers/char/ns16550.c |  183
> +++++++++++++++++++++++++++++++++++++++++---
>  4 files changed, 176 insertions(+), 12 deletions(-)
> 
> diff --git a/config/arm32.mk b/config/arm32.mk
> index 76e229d..aa79d22 100644
> --- a/config/arm32.mk
> +++ b/config/arm32.mk
> @@ -12,6 +12,7 @@ CFLAGS += -marm
>  HAS_PL011 := y
>  HAS_EXYNOS4210 := y
>  HAS_OMAP := y
> +HAS_NS16550 := y
>  
>  # Use only if calling $(LD) directly.
>  LDFLAGS_DIRECT += -EL
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index 736882a..df1428f 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -60,6 +60,9 @@ CFLAGS-$(lock_profile)  += -DLOCK_PROFILE
>  CFLAGS-$(HAS_ACPI)      += -DHAS_ACPI
>  CFLAGS-$(HAS_GDBSX)     += -DHAS_GDBSX
>  CFLAGS-$(HAS_PASSTHROUGH) += -DHAS_PASSTHROUGH
> +CFLAGS-$(HAS_DEVICE_TREE) += -DHAS_DEVICE_TREE
> +CFLAGS-$(HAS_PCI)       += -DHAS_PCI
> +CFLAGS-$(HAS_IOPORTS)   += -DHAS_IOPORTS
>  CFLAGS-$(frame_pointer) += -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER
>  
>  ifneq ($(max_phys_cpus),)
> diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
> index eb11b5b..c93d2af 100644
> --- a/xen/arch/x86/Rules.mk
> +++ b/xen/arch/x86/Rules.mk
> @@ -1,6 +1,7 @@
>  ########################################
>  # x86-specific definitions
>  
> +HAS_IOPORTS := y
>  HAS_ACPI := y
>  HAS_VGA  := y
>  HAS_VIDEO  := y
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index e0f80f6..854a572 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -13,14 +13,19 @@
>  #include <xen/init.h>
>  #include <xen/irq.h>
>  #include <xen/sched.h>
> -#include <xen/pci.h>
>  #include <xen/timer.h>
>  #include <xen/serial.h>
>  #include <xen/iocap.h>
> +#ifdef HAS_PCI
>  #include <xen/pci.h>
>  #include <xen/pci_regs.h>
> +#endif
>  #include <xen/8250-uart.h>
> +#include <xen/vmap.h>
>  #include <asm/io.h>
> +#ifdef HAS_DEVICE_TREE
> +#include <asm/device.h>
> +#endif
>  #ifdef CONFIG_X86
>  #include <asm/fixmap.h>
>  #endif
> @@ -40,15 +45,22 @@ 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;
> +    int reg_shift; /* Bits to shift register offset by */
> +    int reg_width; /* Number of bytes in each register */
>      char __iomem *remapped_io_base;  /* Remapped virtual address of MMIO. */
>      /* UART with IRQ line: interrupt-driven I/O. */
>      struct irqaction irqaction;
> +#ifdef CONFIG_ARM
> +    struct vuart_info vuart;
> +#endif
>      /* UART with no IRQ line: periodically-polled I/O. */
>      struct timer timer;
>      struct timer resume_timer;
>      unsigned int timeout_ms;
>      bool_t intr_works;
> +#ifdef HAS_PCI
>      /* PCI card parameters. */
>      unsigned int pb_bdf[3]; /* pci bridge BDF */
>      unsigned int ps_bdf[3]; /* pci serial port BDF */
> @@ -57,22 +69,46 @@ static struct ns16550 {
>      u32 bar;
>      u16 cr;
>      u8 bar_idx;
> +#endif
> +#ifdef HAS_DEVICE_TREE
> +    struct dt_irq dt_irq;
> +#endif
>  } ns16550_com[2] = { { 0 } };
>  
>  static void ns16550_delayed_resume(void *data);
>  
>  static char ns_read_reg(struct ns16550 *uart, int reg)
>  {
> +    void __iomem *addr = uart->remapped_io_base + (reg<<uart->reg_shift);
> +#ifdef HAS_IOPORTS
>      if ( uart->remapped_io_base == NULL )
>          return inb(uart->io_base + reg);
> -    return readb(uart->remapped_io_base + reg);
> +#endif
> +    switch (uart->reg_width) {
> +    default: /* assume single byte */
> +    case 1:
> +        return readb(addr);
> +    case 4:
> +        return readl(addr);
> +    }
>  }
>  
>  static void ns_write_reg(struct ns16550 *uart, int reg, char c)
>  {
> +    void __iomem *addr = uart->remapped_io_base + (reg<<uart->reg_shift);
> +#ifdef HAS_IOPORTS
>      if ( uart->remapped_io_base == NULL )
>          return outb(c, uart->io_base + reg);
> -    writeb(c, uart->remapped_io_base + reg);
> +#endif
> +    switch (uart->reg_width) {
> +    default: /* assume single byte */
> +    case 1:
> +        writeb(c, addr);
> +        break;
> +    case 4:
> +        writel(c, addr);
> +        break;
> +    }
>  }
>  
>  static int ns16550_ioport_invalid(struct ns16550 *uart)
> @@ -161,6 +197,7 @@ static int ns16550_getc(struct serial_port *port, char
> *pc)
>      return 1;
>  }
>  
> +#ifdef HAS_PCI
>  static void pci_serial_early_init(struct ns16550 *uart)
>  {
>      if ( !uart->ps_bdf_enable || uart->io_base >= 0x10000 )
> @@ -178,6 +215,9 @@ 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);
>  }
> +#else
> +static void pci_serial_early_init(struct ns16550 *uart) {}
> +#endif
>  
>  static void ns16550_setup_preirq(struct ns16550 *uart)
>  {
> @@ -223,8 +263,10 @@ static void __init ns16550_init_preirq(struct serial_port
> *port)
>  {
>      struct ns16550 *uart = port->uart;
>  
> +#ifdef HAS_IOPORTS
>      /* I/O ports are distinguished by their size (16 bits). */
>      if ( uart->io_base >= 0x10000 )
> +#endif
>      {
>  #ifdef CONFIG_X86
>          enum fixed_addresses idx = FIX_COM_BEGIN + (uart - ns16550_com);
> @@ -233,7 +275,7 @@ 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 = (char *)ioremap(uart->io_base,
> uart->io_size);
>  #endif
>      }
>  
> @@ -278,21 +320,27 @@ static void __init ns16550_init_postirq(struct
> serial_port *port)
>      bits = uart->data_bits + uart->stop_bits + !!uart->parity;
>      uart->timeout_ms = max_t(
>          unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
> -
>      if ( uart->irq > 0 )
>      {
>          uart->irqaction.handler = ns16550_interrupt;
>          uart->irqaction.name    = "ns16550";
>          uart->irqaction.dev_id  = port;
> +#ifdef HAS_DEVICE_TREE
> +        if ( (rc = setup_dt_irq(&uart->dt_irq, &uart->irqaction)) != 0 )
> +            printk("ERROR: Failed to allocate ns16550 DT IRQ.\n");
> +#else
>          if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
>              printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq);
> +#endif
>      }
>  
>      ns16550_setup_postirq(uart);
>  
> +#ifdef HAS_PCI
>      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)
> @@ -301,13 +349,16 @@ static void ns16550_suspend(struct serial_port *port)
>  
>      stop_timer(&uart->timer);
>  
> +#ifdef HAS_PCI
>      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 HAS_PCI
>      struct ns16550 *uart = port->uart;
>  
>      if ( uart->bar )
> @@ -317,6 +368,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);
> @@ -360,19 +412,17 @@ static void ns16550_resume(struct serial_port *port)
>          _ns16550_resume(port);
>  }
>  
> -#ifdef CONFIG_X86
>  static void __init ns16550_endboot(struct serial_port *port)
>  {
> +#ifdef HAS_IOPORTS
>      struct ns16550 *uart = port->uart;
>  
>      if ( uart->remapped_io_base )
>          return;
>      if ( ioports_deny_access(dom0, uart->io_base, uart->io_base + 7) != 0 )
>          BUG();
> -}
> -#else
> -#define ns16550_endboot NULL
>  #endif
> +}
>  
>  static int __init ns16550_irq(struct serial_port *port)
>  {
> @@ -380,6 +430,23 @@ static int __init ns16550_irq(struct serial_port *port)
>      return ((uart->irq > 0) ? uart->irq : -1);
>  }
>  
> +#ifdef HAS_DEVICE_TREE
> +static const struct dt_irq __init *ns16550_dt_irq(struct serial_port *port)
> +{
> +    struct ns16550 *uart = port->uart;
> +    return &uart->dt_irq;
> +}
> +#endif
> +
> +#ifdef CONFIG_ARM
> +static const struct vuart_info *ns16550_vuart_info(struct serial_port *port)
> +{
> +    struct ns16550 *uart = port->uart;
> +
> +    return &uart->vuart;
> +}
> +#endif
> +
>  static struct uart_driver __read_mostly ns16550_driver = {
>      .init_preirq  = ns16550_init_preirq,
>      .init_postirq = ns16550_init_postirq,
> @@ -389,7 +456,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,
> +#ifdef HAS_DEVICE_TREE
> +    .dt_irq_get   = ns16550_dt_irq,
> +#endif
> +#ifdef CONFIG_ARM
> +    .vuart_info   = ns16550_vuart_info,
> +#endif
>  };
>  
>  static int __init parse_parity_char(int c)
> @@ -414,15 +487,21 @@ static int __init check_existence(struct ns16550 *uart)
>  {
>      unsigned char status, scratch, scratch2, scratch3;
>  
> +#ifdef HAS_IO_PORTS
>      /*
>       * We can't poke MMIO UARTs until they get I/O remapped later. Assume
> that
>       * if we're getting MMIO UARTs, the arch code knows what it's doing.
>       */
>      if ( uart->io_base >= 0x10000 )
>          return 1;
> +#else
> +    return 1; /* Everything is MMIO */
> +#endif
>  
> +#ifdef HAS_PCI
>      pci_serial_early_init(uart);
> -    
> +#endif
> +
>      /*
>       * Do a simple existence test first; if we fail this,
>       * there's no point trying anything else.
> @@ -450,6 +529,7 @@ static int __init check_existence(struct ns16550 *uart)
>      return (status == 0x90);
>  }
>  
> +#ifdef HAS_PCI
>  static int
>  pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
>  {
> @@ -518,6 +598,7 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int
> bar_idx)
>  
>      return 0;
>  }
> +#endif
>  
>  #define PARSE_ERR(_f, _a...)                 \
>      do {                                     \
> @@ -564,6 +645,7 @@ static void __init ns16550_parse_port_config(
>  
>      if ( *conf == ',' && *++conf != ',' )
>      {
> +#ifdef HAS_PCI
>          if ( strncmp(conf, "pci", 3) == 0 )
>          {
>              if ( pci_uart_config(uart, 1/* skip AMT */, uart - ns16550_com) )
> @@ -577,6 +659,7 @@ static void __init ns16550_parse_port_config(
>              conf += 3;
>          }
>          else
> +#endif
>          {
>              uart->io_base = simple_strtoul(conf, &conf, 0);
>          }
> @@ -585,6 +668,7 @@ static void __init ns16550_parse_port_config(
>      if ( *conf == ',' && *++conf != ',' )
>          uart->irq = simple_strtol(conf, &conf, 10);
>  
> +#ifdef HAS_PCI
>      if ( *conf == ',' && *++conf != ',' )
>      {
>          conf = parse_pci(conf, NULL, &uart->ps_bdf[0],
> @@ -601,6 +685,7 @@ static void __init ns16550_parse_port_config(
>              PARSE_ERR("Bad bridge PCI coordinates");
>          uart->pb_bdf_enable = 1;
>      }
> +#endif
>  
>   config_parsed:
>      /* Sanity checks. */
> @@ -638,12 +723,86 @@ void __init ns16550_init(int index, struct
> ns16550_defaults *defaults)
>      uart->stop_bits = defaults->stop_bits;
>      uart->irq       = defaults->irq;
>      uart->io_base   = defaults->io_base;
> +    uart->io_size   = 8;
> +    uart->reg_width = 1;
> +    uart->reg_shift = 0;
> +
>      /* Default is no transmit FIFO. */
>      uart->fifo_size = 1;
>  
>      ns16550_parse_port_config(uart, (index == 0) ? opt_com1 : opt_com2);
>  }
>  
> +#ifdef HAS_DEVICE_TREE
> +static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
> +                                       const void *data)
> +{
> +    struct ns16550 *uart;
> +    int res;
> +    u32 reg_shift, reg_width;
> +
> +    uart = &ns16550_com[0];
> +
> +    uart->baud      = BAUD_AUTO;
> +    uart->clock_hz  = UART_CLOCK_HZ;
> +    uart->data_bits = 8;
> +    uart->parity    = UART_PARITY_NONE;
> +    uart->stop_bits = 1;
> +    /* Default is no transmit FIFO. */
> +    uart->fifo_size = 1;
> +
> +    res = dt_device_get_address(dev, 0, &uart->io_base, &uart->io_size);
> +    if ( res )
> +        return res;
> +
> +    res = dt_property_read_u32(dev, "reg-shift", &reg_shift);
> +    if ( !res )
> +        uart->reg_shift = 0;
> +    else
> +        uart->reg_shift = reg_shift;
> +
> +    res = dt_property_read_u32(dev, "reg-io-width", &reg_width);
> +    if ( !res )
> +        uart->reg_width = 1;
> +    else
> +        uart->reg_width = reg_width;
> +
> +    if ( uart->reg_width != 1 && uart->reg_width != 4 )
> +        return -EINVAL;
> +
> +    res = dt_device_get_irq(dev, 0, &uart->dt_irq);
> +    if ( res )
> +        return res;
> +
> +    /* The common bit of the driver mostly deals with irq not dt_irq. */
> +    uart->irq = uart->dt_irq.irq;
> +
> +    uart->vuart.base_addr = uart->io_base;
> +    uart->vuart.size = uart->io_size;
> +    uart->vuart.data_off = UART_THR <<uart->reg_shift;
> +    uart->vuart.status_off = UART_LSR<<uart->reg_shift;
> +    uart->vuart.status = UART_LSR_THRE|UART_LSR_TEMT;
> +
> +    /* Register with generic serial driver. */
> +    serial_register_uart(uart - ns16550_com, &ns16550_driver, uart);
> +
> +    dt_device_set_used_by(dev, DOMID_XEN);
> +
> +    return 0;
> +}
> +
> +static const char const *ns16550_dt_compat[] __initdata =
> +{
> +    "ns16550",
> +    NULL
> +};
> +
> +DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
> +        .compatible = ns16550_dt_compat,
> +        .init = ns16550_uart_dt_init,
> +DT_DEVICE_END
> +
> +#endif /* HAS_DEVICE_TREE */
>  /*
>   * Local variables:
>   * mode: C

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

* Re: [PATCH RFC 4/8] ns16550: support DesignWare 8250
  2013-09-10 14:18 ` [PATCH RFC 4/8] ns16550: support DesignWare 8250 Ian Campbell
@ 2013-09-10 14:36   ` Keir Fraser
  2013-09-10 15:02   ` Jan Beulich
  1 sibling, 0 replies; 34+ messages in thread
From: Keir Fraser @ 2013-09-10 14:36 UTC (permalink / raw)
  To: Ian Campbell, xen-devel
  Cc: keir, stefano.stabellini, julien.grall, tim, jbeulich, Josh Zhao

On 10/09/2013 07:18, "Ian Campbell" <ian.campbell@citrix.com> wrote:

> This hardware has an additional feature which signals an error if you try to
> write LCR while the UART is busy. We need to clear this error during setup,
> otherwise LCR.DLAB doesn't get set and we cannot read/write the divisor.
> 
> This has been tested on the cubieboard2
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: keir@xen.org
> Cc: jbeulich@suse.com

Acked-by: Keir Fraser <keir@xen.org>

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

* Re: [PATCH RFC 3/8] ns16550: make usable on ARM
  2013-09-10 14:36   ` Keir Fraser
@ 2013-09-10 14:45     ` Ian Campbell
  0 siblings, 0 replies; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 14:45 UTC (permalink / raw)
  To: Keir Fraser
  Cc: stefano.stabellini, julien.grall, tim, xen-devel, jbeulich, Josh Zhao

On Tue, 2013-09-10 at 07:36 -0700, Keir Fraser wrote:
> On 10/09/2013 07:18, "Ian Campbell" <ian.campbell@citrix.com> wrote:
> 
> > There are several aspects to this:
> > - Correctly conditionalise use of PCI
> > - Correctly conditionalise use of IO ports
> > - Add discovery via device tree
> > - Support different registers shift/stride and widths
> > - Add vuart hooks.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: keir@xen.org
> > Cc: jbeulich@suse.com
> 
> Bit of an ugly ifdef mess but it is only a serial driver.

Yeah, I'm not entirely happy with it either.

> Acked-by: Keir Fraser <keir@xen.org>

Thanks.

> 
> > ---
> >  config/arm32.mk            |    1 +
> >  xen/Rules.mk               |    3 +
> >  xen/arch/x86/Rules.mk      |    1 +
> >  xen/drivers/char/ns16550.c |  183
> > +++++++++++++++++++++++++++++++++++++++++---
> >  4 files changed, 176 insertions(+), 12 deletions(-)
> > 
> > diff --git a/config/arm32.mk b/config/arm32.mk
> > index 76e229d..aa79d22 100644
> > --- a/config/arm32.mk
> > +++ b/config/arm32.mk
> > @@ -12,6 +12,7 @@ CFLAGS += -marm
> >  HAS_PL011 := y
> >  HAS_EXYNOS4210 := y
> >  HAS_OMAP := y
> > +HAS_NS16550 := y
> >  
> >  # Use only if calling $(LD) directly.
> >  LDFLAGS_DIRECT += -EL
> > diff --git a/xen/Rules.mk b/xen/Rules.mk
> > index 736882a..df1428f 100644
> > --- a/xen/Rules.mk
> > +++ b/xen/Rules.mk
> > @@ -60,6 +60,9 @@ CFLAGS-$(lock_profile)  += -DLOCK_PROFILE
> >  CFLAGS-$(HAS_ACPI)      += -DHAS_ACPI
> >  CFLAGS-$(HAS_GDBSX)     += -DHAS_GDBSX
> >  CFLAGS-$(HAS_PASSTHROUGH) += -DHAS_PASSTHROUGH
> > +CFLAGS-$(HAS_DEVICE_TREE) += -DHAS_DEVICE_TREE
> > +CFLAGS-$(HAS_PCI)       += -DHAS_PCI
> > +CFLAGS-$(HAS_IOPORTS)   += -DHAS_IOPORTS
> >  CFLAGS-$(frame_pointer) += -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER
> >  
> >  ifneq ($(max_phys_cpus),)
> > diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
> > index eb11b5b..c93d2af 100644
> > --- a/xen/arch/x86/Rules.mk
> > +++ b/xen/arch/x86/Rules.mk
> > @@ -1,6 +1,7 @@
> >  ########################################
> >  # x86-specific definitions
> >  
> > +HAS_IOPORTS := y
> >  HAS_ACPI := y
> >  HAS_VGA  := y
> >  HAS_VIDEO  := y
> > diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> > index e0f80f6..854a572 100644
> > --- a/xen/drivers/char/ns16550.c
> > +++ b/xen/drivers/char/ns16550.c
> > @@ -13,14 +13,19 @@
> >  #include <xen/init.h>
> >  #include <xen/irq.h>
> >  #include <xen/sched.h>
> > -#include <xen/pci.h>
> >  #include <xen/timer.h>
> >  #include <xen/serial.h>
> >  #include <xen/iocap.h>
> > +#ifdef HAS_PCI
> >  #include <xen/pci.h>
> >  #include <xen/pci_regs.h>
> > +#endif
> >  #include <xen/8250-uart.h>
> > +#include <xen/vmap.h>
> >  #include <asm/io.h>
> > +#ifdef HAS_DEVICE_TREE
> > +#include <asm/device.h>
> > +#endif
> >  #ifdef CONFIG_X86
> >  #include <asm/fixmap.h>
> >  #endif
> > @@ -40,15 +45,22 @@ 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;
> > +    int reg_shift; /* Bits to shift register offset by */
> > +    int reg_width; /* Number of bytes in each register */
> >      char __iomem *remapped_io_base;  /* Remapped virtual address of MMIO. */
> >      /* UART with IRQ line: interrupt-driven I/O. */
> >      struct irqaction irqaction;
> > +#ifdef CONFIG_ARM
> > +    struct vuart_info vuart;
> > +#endif
> >      /* UART with no IRQ line: periodically-polled I/O. */
> >      struct timer timer;
> >      struct timer resume_timer;
> >      unsigned int timeout_ms;
> >      bool_t intr_works;
> > +#ifdef HAS_PCI
> >      /* PCI card parameters. */
> >      unsigned int pb_bdf[3]; /* pci bridge BDF */
> >      unsigned int ps_bdf[3]; /* pci serial port BDF */
> > @@ -57,22 +69,46 @@ static struct ns16550 {
> >      u32 bar;
> >      u16 cr;
> >      u8 bar_idx;
> > +#endif
> > +#ifdef HAS_DEVICE_TREE
> > +    struct dt_irq dt_irq;
> > +#endif
> >  } ns16550_com[2] = { { 0 } };
> >  
> >  static void ns16550_delayed_resume(void *data);
> >  
> >  static char ns_read_reg(struct ns16550 *uart, int reg)
> >  {
> > +    void __iomem *addr = uart->remapped_io_base + (reg<<uart->reg_shift);
> > +#ifdef HAS_IOPORTS
> >      if ( uart->remapped_io_base == NULL )
> >          return inb(uart->io_base + reg);
> > -    return readb(uart->remapped_io_base + reg);
> > +#endif
> > +    switch (uart->reg_width) {
> > +    default: /* assume single byte */
> > +    case 1:
> > +        return readb(addr);
> > +    case 4:
> > +        return readl(addr);
> > +    }
> >  }
> >  
> >  static void ns_write_reg(struct ns16550 *uart, int reg, char c)
> >  {
> > +    void __iomem *addr = uart->remapped_io_base + (reg<<uart->reg_shift);
> > +#ifdef HAS_IOPORTS
> >      if ( uart->remapped_io_base == NULL )
> >          return outb(c, uart->io_base + reg);
> > -    writeb(c, uart->remapped_io_base + reg);
> > +#endif
> > +    switch (uart->reg_width) {
> > +    default: /* assume single byte */
> > +    case 1:
> > +        writeb(c, addr);
> > +        break;
> > +    case 4:
> > +        writel(c, addr);
> > +        break;
> > +    }
> >  }
> >  
> >  static int ns16550_ioport_invalid(struct ns16550 *uart)
> > @@ -161,6 +197,7 @@ static int ns16550_getc(struct serial_port *port, char
> > *pc)
> >      return 1;
> >  }
> >  
> > +#ifdef HAS_PCI
> >  static void pci_serial_early_init(struct ns16550 *uart)
> >  {
> >      if ( !uart->ps_bdf_enable || uart->io_base >= 0x10000 )
> > @@ -178,6 +215,9 @@ 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);
> >  }
> > +#else
> > +static void pci_serial_early_init(struct ns16550 *uart) {}
> > +#endif
> >  
> >  static void ns16550_setup_preirq(struct ns16550 *uart)
> >  {
> > @@ -223,8 +263,10 @@ static void __init ns16550_init_preirq(struct serial_port
> > *port)
> >  {
> >      struct ns16550 *uart = port->uart;
> >  
> > +#ifdef HAS_IOPORTS
> >      /* I/O ports are distinguished by their size (16 bits). */
> >      if ( uart->io_base >= 0x10000 )
> > +#endif
> >      {
> >  #ifdef CONFIG_X86
> >          enum fixed_addresses idx = FIX_COM_BEGIN + (uart - ns16550_com);
> > @@ -233,7 +275,7 @@ 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 = (char *)ioremap(uart->io_base,
> > uart->io_size);
> >  #endif
> >      }
> >  
> > @@ -278,21 +320,27 @@ static void __init ns16550_init_postirq(struct
> > serial_port *port)
> >      bits = uart->data_bits + uart->stop_bits + !!uart->parity;
> >      uart->timeout_ms = max_t(
> >          unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
> > -
> >      if ( uart->irq > 0 )
> >      {
> >          uart->irqaction.handler = ns16550_interrupt;
> >          uart->irqaction.name    = "ns16550";
> >          uart->irqaction.dev_id  = port;
> > +#ifdef HAS_DEVICE_TREE
> > +        if ( (rc = setup_dt_irq(&uart->dt_irq, &uart->irqaction)) != 0 )
> > +            printk("ERROR: Failed to allocate ns16550 DT IRQ.\n");
> > +#else
> >          if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
> >              printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq);
> > +#endif
> >      }
> >  
> >      ns16550_setup_postirq(uart);
> >  
> > +#ifdef HAS_PCI
> >      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)
> > @@ -301,13 +349,16 @@ static void ns16550_suspend(struct serial_port *port)
> >  
> >      stop_timer(&uart->timer);
> >  
> > +#ifdef HAS_PCI
> >      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 HAS_PCI
> >      struct ns16550 *uart = port->uart;
> >  
> >      if ( uart->bar )
> > @@ -317,6 +368,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);
> > @@ -360,19 +412,17 @@ static void ns16550_resume(struct serial_port *port)
> >          _ns16550_resume(port);
> >  }
> >  
> > -#ifdef CONFIG_X86
> >  static void __init ns16550_endboot(struct serial_port *port)
> >  {
> > +#ifdef HAS_IOPORTS
> >      struct ns16550 *uart = port->uart;
> >  
> >      if ( uart->remapped_io_base )
> >          return;
> >      if ( ioports_deny_access(dom0, uart->io_base, uart->io_base + 7) != 0 )
> >          BUG();
> > -}
> > -#else
> > -#define ns16550_endboot NULL
> >  #endif
> > +}
> >  
> >  static int __init ns16550_irq(struct serial_port *port)
> >  {
> > @@ -380,6 +430,23 @@ static int __init ns16550_irq(struct serial_port *port)
> >      return ((uart->irq > 0) ? uart->irq : -1);
> >  }
> >  
> > +#ifdef HAS_DEVICE_TREE
> > +static const struct dt_irq __init *ns16550_dt_irq(struct serial_port *port)
> > +{
> > +    struct ns16550 *uart = port->uart;
> > +    return &uart->dt_irq;
> > +}
> > +#endif
> > +
> > +#ifdef CONFIG_ARM
> > +static const struct vuart_info *ns16550_vuart_info(struct serial_port *port)
> > +{
> > +    struct ns16550 *uart = port->uart;
> > +
> > +    return &uart->vuart;
> > +}
> > +#endif
> > +
> >  static struct uart_driver __read_mostly ns16550_driver = {
> >      .init_preirq  = ns16550_init_preirq,
> >      .init_postirq = ns16550_init_postirq,
> > @@ -389,7 +456,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,
> > +#ifdef HAS_DEVICE_TREE
> > +    .dt_irq_get   = ns16550_dt_irq,
> > +#endif
> > +#ifdef CONFIG_ARM
> > +    .vuart_info   = ns16550_vuart_info,
> > +#endif
> >  };
> >  
> >  static int __init parse_parity_char(int c)
> > @@ -414,15 +487,21 @@ static int __init check_existence(struct ns16550 *uart)
> >  {
> >      unsigned char status, scratch, scratch2, scratch3;
> >  
> > +#ifdef HAS_IO_PORTS
> >      /*
> >       * We can't poke MMIO UARTs until they get I/O remapped later. Assume
> > that
> >       * if we're getting MMIO UARTs, the arch code knows what it's doing.
> >       */
> >      if ( uart->io_base >= 0x10000 )
> >          return 1;
> > +#else
> > +    return 1; /* Everything is MMIO */
> > +#endif
> >  
> > +#ifdef HAS_PCI
> >      pci_serial_early_init(uart);
> > -    
> > +#endif
> > +
> >      /*
> >       * Do a simple existence test first; if we fail this,
> >       * there's no point trying anything else.
> > @@ -450,6 +529,7 @@ static int __init check_existence(struct ns16550 *uart)
> >      return (status == 0x90);
> >  }
> >  
> > +#ifdef HAS_PCI
> >  static int
> >  pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
> >  {
> > @@ -518,6 +598,7 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int
> > bar_idx)
> >  
> >      return 0;
> >  }
> > +#endif
> >  
> >  #define PARSE_ERR(_f, _a...)                 \
> >      do {                                     \
> > @@ -564,6 +645,7 @@ static void __init ns16550_parse_port_config(
> >  
> >      if ( *conf == ',' && *++conf != ',' )
> >      {
> > +#ifdef HAS_PCI
> >          if ( strncmp(conf, "pci", 3) == 0 )
> >          {
> >              if ( pci_uart_config(uart, 1/* skip AMT */, uart - ns16550_com) )
> > @@ -577,6 +659,7 @@ static void __init ns16550_parse_port_config(
> >              conf += 3;
> >          }
> >          else
> > +#endif
> >          {
> >              uart->io_base = simple_strtoul(conf, &conf, 0);
> >          }
> > @@ -585,6 +668,7 @@ static void __init ns16550_parse_port_config(
> >      if ( *conf == ',' && *++conf != ',' )
> >          uart->irq = simple_strtol(conf, &conf, 10);
> >  
> > +#ifdef HAS_PCI
> >      if ( *conf == ',' && *++conf != ',' )
> >      {
> >          conf = parse_pci(conf, NULL, &uart->ps_bdf[0],
> > @@ -601,6 +685,7 @@ static void __init ns16550_parse_port_config(
> >              PARSE_ERR("Bad bridge PCI coordinates");
> >          uart->pb_bdf_enable = 1;
> >      }
> > +#endif
> >  
> >   config_parsed:
> >      /* Sanity checks. */
> > @@ -638,12 +723,86 @@ void __init ns16550_init(int index, struct
> > ns16550_defaults *defaults)
> >      uart->stop_bits = defaults->stop_bits;
> >      uart->irq       = defaults->irq;
> >      uart->io_base   = defaults->io_base;
> > +    uart->io_size   = 8;
> > +    uart->reg_width = 1;
> > +    uart->reg_shift = 0;
> > +
> >      /* Default is no transmit FIFO. */
> >      uart->fifo_size = 1;
> >  
> >      ns16550_parse_port_config(uart, (index == 0) ? opt_com1 : opt_com2);
> >  }
> >  
> > +#ifdef HAS_DEVICE_TREE
> > +static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
> > +                                       const void *data)
> > +{
> > +    struct ns16550 *uart;
> > +    int res;
> > +    u32 reg_shift, reg_width;
> > +
> > +    uart = &ns16550_com[0];
> > +
> > +    uart->baud      = BAUD_AUTO;
> > +    uart->clock_hz  = UART_CLOCK_HZ;
> > +    uart->data_bits = 8;
> > +    uart->parity    = UART_PARITY_NONE;
> > +    uart->stop_bits = 1;
> > +    /* Default is no transmit FIFO. */
> > +    uart->fifo_size = 1;
> > +
> > +    res = dt_device_get_address(dev, 0, &uart->io_base, &uart->io_size);
> > +    if ( res )
> > +        return res;
> > +
> > +    res = dt_property_read_u32(dev, "reg-shift", &reg_shift);
> > +    if ( !res )
> > +        uart->reg_shift = 0;
> > +    else
> > +        uart->reg_shift = reg_shift;
> > +
> > +    res = dt_property_read_u32(dev, "reg-io-width", &reg_width);
> > +    if ( !res )
> > +        uart->reg_width = 1;
> > +    else
> > +        uart->reg_width = reg_width;
> > +
> > +    if ( uart->reg_width != 1 && uart->reg_width != 4 )
> > +        return -EINVAL;
> > +
> > +    res = dt_device_get_irq(dev, 0, &uart->dt_irq);
> > +    if ( res )
> > +        return res;
> > +
> > +    /* The common bit of the driver mostly deals with irq not dt_irq. */
> > +    uart->irq = uart->dt_irq.irq;
> > +
> > +    uart->vuart.base_addr = uart->io_base;
> > +    uart->vuart.size = uart->io_size;
> > +    uart->vuart.data_off = UART_THR <<uart->reg_shift;
> > +    uart->vuart.status_off = UART_LSR<<uart->reg_shift;
> > +    uart->vuart.status = UART_LSR_THRE|UART_LSR_TEMT;
> > +
> > +    /* Register with generic serial driver. */
> > +    serial_register_uart(uart - ns16550_com, &ns16550_driver, uart);
> > +
> > +    dt_device_set_used_by(dev, DOMID_XEN);
> > +
> > +    return 0;
> > +}
> > +
> > +static const char const *ns16550_dt_compat[] __initdata =
> > +{
> > +    "ns16550",
> > +    NULL
> > +};
> > +
> > +DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
> > +        .compatible = ns16550_dt_compat,
> > +        .init = ns16550_uart_dt_init,
> > +DT_DEVICE_END
> > +
> > +#endif /* HAS_DEVICE_TREE */
> >  /*
> >   * Local variables:
> >   * mode: C
> 
> 

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

* Re: [PATCH RFC 2/8] xen/arm: implement read[bl] and write[bl]
  2013-09-10 14:18 ` [PATCH RFC 2/8] xen/arm: implement read[bl] and write[bl] Ian Campbell
@ 2013-09-10 15:00   ` Julien Grall
  2013-09-10 15:09     ` Ian Campbell
  0 siblings, 1 reply; 34+ messages in thread
From: Julien Grall @ 2013-09-10 15:00 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Josh Zhao, stefano.stabellini, tim, xen-devel

On 09/10/2013 03:18 PM, Ian Campbell wrote:
> These are used in common driver code (specifically ns16550)

Can you implement read[bl] and write[bl] also for arm64?

> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/include/asm-arm/io.h |   48 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
> 
> 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:
> 


-- 
Julien Grall

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

* Re: [PATCH RFC 3/8] ns16550: make usable on ARM
  2013-09-10 14:18 ` [PATCH RFC 3/8] ns16550: make usable on ARM Ian Campbell
  2013-09-10 14:36   ` Keir Fraser
@ 2013-09-10 15:00   ` Jan Beulich
  2013-09-10 15:11     ` Ian Campbell
  1 sibling, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2013-09-10 15:00 UTC (permalink / raw)
  To: Ian Campbell
  Cc: keir, stefano.stabellini, julien.grall, tim, Josh Zhao, xen-devel

>>> On 10.09.13 at 16:18, Ian Campbell <ian.campbell@citrix.com> wrote:
> @@ -40,15 +45,22 @@ 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;

Does the size really need to be 64 bits?

>  static char ns_read_reg(struct ns16550 *uart, int reg)
>  {
> +    void __iomem *addr = uart->remapped_io_base + (reg<<uart->reg_shift);

Blanks around <<.

> +#ifdef HAS_IOPORTS
>      if ( uart->remapped_io_base == NULL )
>          return inb(uart->io_base + reg);
> -    return readb(uart->remapped_io_base + reg);
> +#endif
> +    switch (uart->reg_width) {

Coding style.

> +    default: /* assume single byte */

Is that really a good idea? I'd recommend returning all 1s here, and
dropping writes below.

> +    case 1:
> +        return readb(addr);
> +    case 4:
> +        return readl(addr);

This won't work without changing the return type of the function.
Or is this just mandating the access width, without the upper 24
bits carrying meaningful data?

> +    }

>  static void ns_write_reg(struct ns16550 *uart, int reg, char c)

Same/similar comments go for this function.

> +#ifdef HAS_PCI
>  static void pci_serial_early_init(struct ns16550 *uart)
>  {
>      if ( !uart->ps_bdf_enable || uart->io_base >= 0x10000 )
> @@ -178,6 +215,9 @@ 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);
>  }
> +#else
> +static void pci_serial_early_init(struct ns16550 *uart) {}
> +#endif

I'd prefer if #ifdef-s of this kind went inside the function body, thus
avoiding the duplication of the function "header". This particularly
reduces the churn on eventual future changes to the function type
(in particular people not building all architectures not noticing the
need to change more than one place).

> @@ -278,21 +320,27 @@ static void __init ns16550_init_postirq(struct serial_port *port)
>      bits = uart->data_bits + uart->stop_bits + !!uart->parity;
>      uart->timeout_ms = max_t(
>          unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
> -
>      if ( uart->irq > 0 )

Anything wrong with the blank line above?

> +static const char const *ns16550_dt_compat[] __initdata =

That ought to be __initconst now that committed that patch.

Jan

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

* Re: [PATCH RFC 4/8] ns16550: support DesignWare 8250
  2013-09-10 14:18 ` [PATCH RFC 4/8] ns16550: support DesignWare 8250 Ian Campbell
  2013-09-10 14:36   ` Keir Fraser
@ 2013-09-10 15:02   ` Jan Beulich
  2013-09-10 15:12     ` Ian Campbell
  1 sibling, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2013-09-10 15:02 UTC (permalink / raw)
  To: Ian Campbell
  Cc: keir, stefano.stabellini, julien.grall, tim, Josh Zhao, xen-devel

>>> On 10.09.13 at 16:18, Ian Campbell <ian.campbell@citrix.com> wrote:
> @@ -233,6 +234,16 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>      /* No interrupts. */
>      ns_write_reg(uart, UART_IER, 0);
>  
> +    if ( uart->dw_usr_bsy &&
> +         (ns_read_reg(uart, UART_IIR) & UART_IIR_BSY) == UART_IIR_BSY )
> +    {
> +        /* DesignWare 8250 detects if LCR is written while the UART is
> +         * busy and raises a "busy detect" interrupt. Read the UART
> +         * Status Register to clear this state.
> +         */
> +        (void)ns_read_reg(uart, UART_USR);

Pointless cast?

Jan

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

* Re: [PATCH RFC 1/8] xen/arm: Implement ioremap.
  2013-09-10 14:18 ` [PATCH RFC 1/8] xen/arm: Implement ioremap Ian Campbell
@ 2013-09-10 15:03   ` Julien Grall
  2013-09-10 15:14     ` Ian Campbell
  0 siblings, 1 reply; 34+ messages in thread
From: Julien Grall @ 2013-09-10 15:03 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Josh Zhao, stefano.stabellini, tim, xen-devel

On 09/10/2013 03:18 PM, Ian Campbell wrote:
> Common code uses this, it expects an uncached mapping.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/arch/arm/mm.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 69c157a..4521c8d 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -694,6 +694,11 @@ void *ioremap_attr(paddr_t pa, size_t len, unsigned int attributes)
>      return (__vmap(&pfn, nr, 1, 1, attributes) + offs);
>  }
>  
> +void *ioremap(paddr_t pa, size_t len)
> +{
> +    return ioremap_attr(pa, len, PAGE_HYPERVISOR_NOCACHE);
> +}
> +

Can you inline ioremap in the header (asm-arm/mm.h)?

>  static int create_xen_table(lpae_t *entry)
>  {
>      void *p;
> 

-- 
Julien Grall

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

* Re: [PATCH RFC 2/8] xen/arm: implement read[bl] and write[bl]
  2013-09-10 15:00   ` Julien Grall
@ 2013-09-10 15:09     ` Ian Campbell
  2013-09-10 15:12       ` Julien Grall
  0 siblings, 1 reply; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 15:09 UTC (permalink / raw)
  To: Julien Grall; +Cc: Josh Zhao, stefano.stabellini, tim, xen-devel

On Tue, 2013-09-10 at 16:00 +0100, Julien Grall wrote:
> On 09/10/2013 03:18 PM, Ian Campbell wrote:
> > These are used in common driver code (specifically ns16550)
> 
> Can you implement read[bl] and write[bl] also for arm64?

Uhm, yesm that's something of a prerequisite actually -- these functiosn
are in a common location so they need to be moved!

Well spotted.

I should also have mentioned the Linux heritage of these functions in
the commit message and/or in a code comment, which was remiss of me.

> 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  xen/include/asm-arm/io.h |   48 ++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 48 insertions(+)
> > 
> > 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] 34+ messages in thread

* Re: [PATCH RFC 3/8] ns16550: make usable on ARM
  2013-09-10 15:00   ` Jan Beulich
@ 2013-09-10 15:11     ` Ian Campbell
  2013-09-10 15:18       ` Jan Beulich
  0 siblings, 1 reply; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 15:11 UTC (permalink / raw)
  To: Jan Beulich
  Cc: keir, stefano.stabellini, julien.grall, tim, Josh Zhao, xen-devel

On Tue, 2013-09-10 at 16:00 +0100, Jan Beulich wrote:
> >>> On 10.09.13 at 16:18, Ian Campbell <ian.campbell@citrix.com> wrote:
> > @@ -40,15 +45,22 @@ 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;
> 
> Does the size really need to be 64 bits?

No, I just wrote 64 for both by mistake.

[...]
> > +    default: /* assume single byte */
> 
> Is that really a good idea? I'd recommend returning all 1s here, and
> dropping writes below.

OK.

> > +    case 1:
> > +        return readb(addr);
> > +    case 4:
> > +        return readl(addr);
> 
> This won't work without changing the return type of the function.
> Or is this just mandating the access width, without the upper 24
> bits carrying meaningful data?

The latter, the registers are still 8 bytes, it's just the accesses.
Maybe I should mask.

> > +#ifdef HAS_PCI
> >  static void pci_serial_early_init(struct ns16550 *uart)
> >  {
> >      if ( !uart->ps_bdf_enable || uart->io_base >= 0x10000 )
> > @@ -178,6 +215,9 @@ 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);
> >  }
> > +#else
> > +static void pci_serial_early_init(struct ns16550 *uart) {}
> > +#endif
> 
> I'd prefer if #ifdef-s of this kind went inside the function body, thus
> avoiding the duplication of the function "header". This particularly
> reduces the churn on eventual future changes to the function type
> (in particular people not building all architectures not noticing the
> need to change more than one place).

Me too normally, no idea why I did it this way!

> 
> > @@ -278,21 +320,27 @@ static void __init ns16550_init_postirq(struct serial_port *port)
> >      bits = uart->data_bits + uart->stop_bits + !!uart->parity;
> >      uart->timeout_ms = max_t(
> >          unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
> > -
> >      if ( uart->irq > 0 )
> 
> Anything wrong with the blank line above?

A victim of some debugging code I inserted then removed I think. I'll
but it back.

> 
> > +static const char const *ns16550_dt_compat[] __initdata =
> 
> That ought to be __initconst now that committed that patch.

Indeed yes, thanks.

Ian.

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

* Re: [PATCH RFC 2/8] xen/arm: implement read[bl] and write[bl]
  2013-09-10 15:09     ` Ian Campbell
@ 2013-09-10 15:12       ` Julien Grall
  2013-09-10 15:15         ` Ian Campbell
  0 siblings, 1 reply; 34+ messages in thread
From: Julien Grall @ 2013-09-10 15:12 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Josh Zhao, stefano.stabellini, tim, xen-devel

On 09/10/2013 04:09 PM, Ian Campbell wrote:
> On Tue, 2013-09-10 at 16:00 +0100, Julien Grall wrote:
>> On 09/10/2013 03:18 PM, Ian Campbell wrote:
>>> These are used in common driver code (specifically ns16550)
>>
>> Can you implement read[bl] and write[bl] also for arm64?
> 
> Uhm, yesm that's something of a prerequisite actually -- these functiosn
> are in a common location so they need to be moved!

BTW, there is already an implementation for readl/writel but called
ioreadl/iowritel.

You can either rename the functions or add some macros.

> Well spotted.
> 
> I should also have mentioned the Linux heritage of these functions in
> the commit message and/or in a code comment, which was remiss of me.
> 
>>
>>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>>> ---
>>>  xen/include/asm-arm/io.h |   48 ++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 48 insertions(+)
>>>
>>> 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:
>>>
>>
>>
> 
> 


-- 
Julien Grall

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

* Re: [PATCH RFC 4/8] ns16550: support DesignWare 8250
  2013-09-10 15:02   ` Jan Beulich
@ 2013-09-10 15:12     ` Ian Campbell
  2013-09-10 15:19       ` Jan Beulich
  0 siblings, 1 reply; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 15:12 UTC (permalink / raw)
  To: Jan Beulich
  Cc: keir, stefano.stabellini, julien.grall, tim, Josh Zhao, xen-devel

On Tue, 2013-09-10 at 16:02 +0100, Jan Beulich wrote:
> >>> On 10.09.13 at 16:18, Ian Campbell <ian.campbell@citrix.com> wrote:
> > @@ -233,6 +234,16 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
> >      /* No interrupts. */
> >      ns_write_reg(uart, UART_IER, 0);
> >  
> > +    if ( uart->dw_usr_bsy &&
> > +         (ns_read_reg(uart, UART_IIR) & UART_IIR_BSY) == UART_IIR_BSY )
> > +    {
> > +        /* DesignWare 8250 detects if LCR is written while the UART is
> > +         * busy and raises a "busy detect" interrupt. Read the UART
> > +         * Status Register to clear this state.
> > +         */
> > +        (void)ns_read_reg(uart, UART_USR);
> 
> Pointless cast?

It's a hint to the compiler/reader that the return value is deliberately
discarded. We have a handful of these in the tree already.

Ian.

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

* Re: [PATCH RFC 1/8] xen/arm: Implement ioremap.
  2013-09-10 15:03   ` Julien Grall
@ 2013-09-10 15:14     ` Ian Campbell
  2013-09-10 15:20       ` Julien Grall
  0 siblings, 1 reply; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 15:14 UTC (permalink / raw)
  To: Julien Grall; +Cc: Josh Zhao, stefano.stabellini, tim, xen-devel

On Tue, 2013-09-10 at 16:03 +0100, Julien Grall wrote:
> On 09/10/2013 03:18 PM, Ian Campbell wrote:
> > Common code uses this, it expects an uncached mapping.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  xen/arch/arm/mm.c |    5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> > index 69c157a..4521c8d 100644
> > --- a/xen/arch/arm/mm.c
> > +++ b/xen/arch/arm/mm.c
> > @@ -694,6 +694,11 @@ void *ioremap_attr(paddr_t pa, size_t len, unsigned int attributes)
> >      return (__vmap(&pfn, nr, 1, 1, attributes) + offs);
> >  }
> >  
> > +void *ioremap(paddr_t pa, size_t len)
> > +{
> > +    return ioremap_attr(pa, len, PAGE_HYPERVISOR_NOCACHE);
> > +}
> > +
> 
> Can you inline ioremap in the header (asm-arm/mm.h)?

It's prototyped in xen/include/xen/vmap.h so I don't think so, at least
not without introducing subtle header inclusion order traps.

It's unlikely to be performance critical so I don't think there is a
need to refactor the definition into arch code.

> >  static int create_xen_table(lpae_t *entry)
> >  {
> >      void *p;
> > 
> 

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

* Re: [PATCH RFC 2/8] xen/arm: implement read[bl] and write[bl]
  2013-09-10 15:12       ` Julien Grall
@ 2013-09-10 15:15         ` Ian Campbell
  0 siblings, 0 replies; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 15:15 UTC (permalink / raw)
  To: Julien Grall; +Cc: Josh Zhao, xen-devel, tim, stefano.stabellini

On Tue, 2013-09-10 at 16:12 +0100, Julien Grall wrote:
> On 09/10/2013 04:09 PM, Ian Campbell wrote:
> > On Tue, 2013-09-10 at 16:00 +0100, Julien Grall wrote:
> >> On 09/10/2013 03:18 PM, Ian Campbell wrote:
> >>> These are used in common driver code (specifically ns16550)
> >>
> >> Can you implement read[bl] and write[bl] also for arm64?
> > 
> > Uhm, yesm that's something of a prerequisite actually -- these functiosn
> > are in a common location so they need to be moved!
> 
> BTW, there is already an implementation for readl/writel but called
> ioreadl/iowritel.
> 
> You can either rename the functions or add some macros.

I'll double check them for equivalence and do that if possible.

> 
> > Well spotted.
> > 
> > I should also have mentioned the Linux heritage of these functions in
> > the commit message and/or in a code comment, which was remiss of me.
> > 
> >>
> >>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> >>> ---
> >>>  xen/include/asm-arm/io.h |   48 ++++++++++++++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 48 insertions(+)
> >>>
> >>> 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] 34+ messages in thread

* Re: [PATCH RFC 3/8] ns16550: make usable on ARM
  2013-09-10 15:11     ` Ian Campbell
@ 2013-09-10 15:18       ` Jan Beulich
  2013-09-10 15:21         ` Ian Campbell
  0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2013-09-10 15:18 UTC (permalink / raw)
  To: Ian Campbell
  Cc: keir, stefano.stabellini, julien.grall, tim, Josh Zhao, xen-devel

>>> On 10.09.13 at 17:11, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Tue, 2013-09-10 at 16:00 +0100, Jan Beulich wrote:
>> >>> On 10.09.13 at 16:18, Ian Campbell <ian.campbell@citrix.com> wrote:
>> > +    case 1:
>> > +        return readb(addr);
>> > +    case 4:
>> > +        return readl(addr);
>> 
>> This won't work without changing the return type of the function.
>> Or is this just mandating the access width, without the upper 24
>> bits carrying meaningful data?
> 
> The latter, the registers are still 8 bytes, it's just the accesses.
> Maybe I should mask.

No need to as long as the function return type is only 8 bits anyway.

Jan

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

* Re: [PATCH RFC 4/8] ns16550: support DesignWare 8250
  2013-09-10 15:12     ` Ian Campbell
@ 2013-09-10 15:19       ` Jan Beulich
  2013-09-10 15:21         ` Ian Campbell
  0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2013-09-10 15:19 UTC (permalink / raw)
  To: Ian Campbell
  Cc: keir, stefano.stabellini, julien.grall, tim, Josh Zhao, xen-devel

>>> On 10.09.13 at 17:12, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Tue, 2013-09-10 at 16:02 +0100, Jan Beulich wrote:
>> >>> On 10.09.13 at 16:18, Ian Campbell <ian.campbell@citrix.com> wrote:
>> > @@ -233,6 +234,16 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>> >      /* No interrupts. */
>> >      ns_write_reg(uart, UART_IER, 0);
>> >  
>> > +    if ( uart->dw_usr_bsy &&
>> > +         (ns_read_reg(uart, UART_IIR) & UART_IIR_BSY) == UART_IIR_BSY )
>> > +    {
>> > +        /* DesignWare 8250 detects if LCR is written while the UART is
>> > +         * busy and raises a "busy detect" interrupt. Read the UART
>> > +         * Status Register to clear this state.
>> > +         */
>> > +        (void)ns_read_reg(uart, UART_USR);
>> 
>> Pointless cast?
> 
> It's a hint to the compiler/reader that the return value is deliberately
> discarded. We have a handful of these in the tree already.

And over time I managed to remove another handful...

Jan

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

* Re: [PATCH RFC 1/8] xen/arm: Implement ioremap.
  2013-09-10 15:14     ` Ian Campbell
@ 2013-09-10 15:20       ` Julien Grall
  0 siblings, 0 replies; 34+ messages in thread
From: Julien Grall @ 2013-09-10 15:20 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Josh Zhao, stefano.stabellini, tim, xen-devel

On 09/10/2013 04:14 PM, Ian Campbell wrote:
> On Tue, 2013-09-10 at 16:03 +0100, Julien Grall wrote:
>> On 09/10/2013 03:18 PM, Ian Campbell wrote:
>>> Common code uses this, it expects an uncached mapping.
>>>
>>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>>> ---
>>>  xen/arch/arm/mm.c |    5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>>> index 69c157a..4521c8d 100644
>>> --- a/xen/arch/arm/mm.c
>>> +++ b/xen/arch/arm/mm.c
>>> @@ -694,6 +694,11 @@ void *ioremap_attr(paddr_t pa, size_t len, unsigned int attributes)
>>>      return (__vmap(&pfn, nr, 1, 1, attributes) + offs);
>>>  }
>>>  
>>> +void *ioremap(paddr_t pa, size_t len)
>>> +{
>>> +    return ioremap_attr(pa, len, PAGE_HYPERVISOR_NOCACHE);
>>> +}
>>> +
>>
>> Can you inline ioremap in the header (asm-arm/mm.h)?
> 
> It's prototyped in xen/include/xen/vmap.h so I don't think so, at least
> not without introducing subtle header inclusion order traps.
> 
> It's unlikely to be performance critical so I don't think there is a
> need to refactor the definition into arch code.

Right, so:

Reviewed-by: Julien Grall <julien.grall@linaro.org>

>>>  static int create_xen_table(lpae_t *entry)
>>>  {
>>>      void *p;
>>>
>>
> 
> 

-- 
Julien Grall

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

* Re: [PATCH RFC 4/8] ns16550: support DesignWare 8250
  2013-09-10 15:19       ` Jan Beulich
@ 2013-09-10 15:21         ` Ian Campbell
  2013-09-10 15:28           ` Jan Beulich
  0 siblings, 1 reply; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 15:21 UTC (permalink / raw)
  To: Jan Beulich
  Cc: keir, stefano.stabellini, julien.grall, tim, Josh Zhao, xen-devel

On Tue, 2013-09-10 at 16:19 +0100, Jan Beulich wrote:
> >>> On 10.09.13 at 17:12, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Tue, 2013-09-10 at 16:02 +0100, Jan Beulich wrote:
> >> >>> On 10.09.13 at 16:18, Ian Campbell <ian.campbell@citrix.com> wrote:
> >> > @@ -233,6 +234,16 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
> >> >      /* No interrupts. */
> >> >      ns_write_reg(uart, UART_IER, 0);
> >> >  
> >> > +    if ( uart->dw_usr_bsy &&
> >> > +         (ns_read_reg(uart, UART_IIR) & UART_IIR_BSY) == UART_IIR_BSY )
> >> > +    {
> >> > +        /* DesignWare 8250 detects if LCR is written while the UART is
> >> > +         * busy and raises a "busy detect" interrupt. Read the UART
> >> > +         * Status Register to clear this state.
> >> > +         */
> >> > +        (void)ns_read_reg(uart, UART_USR);
> >> 
> >> Pointless cast?
> > 
> > It's a hint to the compiler/reader that the return value is deliberately
> > discarded. We have a handful of these in the tree already.
> 
> And over time I managed to remove another handful...

Well, I guess I don't mind one way or another. What's the problem with
them?

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

* Re: [PATCH RFC 3/8] ns16550: make usable on ARM
  2013-09-10 15:18       ` Jan Beulich
@ 2013-09-10 15:21         ` Ian Campbell
  0 siblings, 0 replies; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 15:21 UTC (permalink / raw)
  To: Jan Beulich
  Cc: keir, stefano.stabellini, julien.grall, tim, Josh Zhao, xen-devel

On Tue, 2013-09-10 at 16:18 +0100, Jan Beulich wrote:
> >>> On 10.09.13 at 17:11, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Tue, 2013-09-10 at 16:00 +0100, Jan Beulich wrote:
> >> >>> On 10.09.13 at 16:18, Ian Campbell <ian.campbell@citrix.com> wrote:
> >> > +    case 1:
> >> > +        return readb(addr);
> >> > +    case 4:
> >> > +        return readl(addr);
> >> 
> >> This won't work without changing the return type of the function.
> >> Or is this just mandating the access width, without the upper 24
> >> bits carrying meaningful data?
> > 
> > The latter, the registers are still 8 bytes, it's just the accesses.
> > Maybe I should mask.
> 
> No need to as long as the function return type is only 8 bits anyway.

True. I think it is worth at least a comment, I'll add one.

Ian.

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

* Re: [PATCH RFC 4/8] ns16550: support DesignWare 8250
  2013-09-10 15:21         ` Ian Campbell
@ 2013-09-10 15:28           ` Jan Beulich
  2013-09-10 15:30             ` Ian Campbell
  0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2013-09-10 15:28 UTC (permalink / raw)
  To: Ian Campbell
  Cc: keir, stefano.stabellini, julien.grall, tim, Josh Zhao, xen-devel

>>> On 10.09.13 at 17:21, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Tue, 2013-09-10 at 16:19 +0100, Jan Beulich wrote:
>> >>> On 10.09.13 at 17:12, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> > On Tue, 2013-09-10 at 16:02 +0100, Jan Beulich wrote:
>> >> >>> On 10.09.13 at 16:18, Ian Campbell <ian.campbell@citrix.com> wrote:
>> >> > @@ -233,6 +234,16 @@ static void ns16550_setup_preirq(struct ns16550 
> *uart)
>> >> >      /* No interrupts. */
>> >> >      ns_write_reg(uart, UART_IER, 0);
>> >> >  
>> >> > +    if ( uart->dw_usr_bsy &&
>> >> > +         (ns_read_reg(uart, UART_IIR) & UART_IIR_BSY) == UART_IIR_BSY )
>> >> > +    {
>> >> > +        /* DesignWare 8250 detects if LCR is written while the UART is
>> >> > +         * busy and raises a "busy detect" interrupt. Read the UART
>> >> > +         * Status Register to clear this state.
>> >> > +         */
>> >> > +        (void)ns_read_reg(uart, UART_USR);
>> >> 
>> >> Pointless cast?
>> > 
>> > It's a hint to the compiler/reader that the return value is deliberately
>> > discarded. We have a handful of these in the tree already.
>> 
>> And over time I managed to remove another handful...
> 
> Well, I guess I don't mind one way or another. What's the problem with
> them?

I'm viewing casts as dangerous in general, and hence advocate
for removing them wherever not really needed. Casts to void I
view as warranted only when needed to silence false positive
compiler warnings.

And in the case here: Ignoring function return values isn't that
uncommon. And the comment already explains why the call is
being made.

Jan

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

* Re: [PATCH RFC 4/8] ns16550: support DesignWare 8250
  2013-09-10 15:28           ` Jan Beulich
@ 2013-09-10 15:30             ` Ian Campbell
  0 siblings, 0 replies; 34+ messages in thread
From: Ian Campbell @ 2013-09-10 15:30 UTC (permalink / raw)
  To: Jan Beulich
  Cc: keir, stefano.stabellini, julien.grall, tim, Josh Zhao, xen-devel

On Tue, 2013-09-10 at 16:28 +0100, Jan Beulich wrote:
> >>> On 10.09.13 at 17:21, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Tue, 2013-09-10 at 16:19 +0100, Jan Beulich wrote:
> >> >>> On 10.09.13 at 17:12, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> >> > On Tue, 2013-09-10 at 16:02 +0100, Jan Beulich wrote:
> >> >> >>> On 10.09.13 at 16:18, Ian Campbell <ian.campbell@citrix.com> wrote:
> >> >> > @@ -233,6 +234,16 @@ static void ns16550_setup_preirq(struct ns16550 
> > *uart)
> >> >> >      /* No interrupts. */
> >> >> >      ns_write_reg(uart, UART_IER, 0);
> >> >> >  
> >> >> > +    if ( uart->dw_usr_bsy &&
> >> >> > +         (ns_read_reg(uart, UART_IIR) & UART_IIR_BSY) == UART_IIR_BSY )
> >> >> > +    {
> >> >> > +        /* DesignWare 8250 detects if LCR is written while the UART is
> >> >> > +         * busy and raises a "busy detect" interrupt. Read the UART
> >> >> > +         * Status Register to clear this state.
> >> >> > +         */
> >> >> > +        (void)ns_read_reg(uart, UART_USR);
> >> >> 
> >> >> Pointless cast?
> >> > 
> >> > It's a hint to the compiler/reader that the return value is deliberately
> >> > discarded. We have a handful of these in the tree already.
> >> 
> >> And over time I managed to remove another handful...
> > 
> > Well, I guess I don't mind one way or another. What's the problem with
> > them?
> 
> I'm viewing casts as dangerous in general, and hence advocate
> for removing them wherever not really needed. Casts to void I
> view as warranted only when needed to silence false positive
> compiler warnings.
> 
> And in the case here: Ignoring function return values isn't that
> uncommon. And the comment already explains why the call is
> being made.

OK, I'll drop it then.

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

* Re: [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support.
  2013-09-10 14:18 [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support Ian Campbell
                   ` (7 preceding siblings ...)
  2013-09-10 14:18 ` [PATCH RFC 8/8] xen/arm: Some early trap logging Ian Campbell
@ 2013-09-11  2:05 ` Josh Zhao
  2013-09-11 10:15   ` Ian Campbell
  8 siblings, 1 reply; 34+ messages in thread
From: Josh Zhao @ 2013-09-11  2:05 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Keir Fraser, Stefano Stabellini, Julien Grall, Tim Deegan,
	xen-devel, jbuelich

2013/9/10 Ian Campbell <Ian.Campbell@citrix.com>:
> The following patches are a very early set of cubieboard patches, based
> on latest staging, eb786f709249666e1f7364706f94be1a6c4e04da
>
> Several are not to be applied because they can be done better using
> infrastructure from Julien's "Allow Xen to boot with a raw Device Tree"
> patch. They are included for completeness.
>
> With this I can boot up until dom0 panics due to lack of a root
> filesystem. This is because upstream Linux currently lacks any useful
> storage drivers (SATA, USB host, MMC). Hopefully this will resolve
> itself soon

>
> The bulk of this is the hacking around of the ns16550 driver to support
> ARM platforms and DTB.
>
> It's currently a bit of a faff to get going. Hopefully the following is
> helpful:
>
> u-boot
> ======
>
> https://github.com/linux-sunxi/u-boot-sunxi.git linux-sunxi/sunxi
> commit e94cff93c273b0825bea135e0f559f5580156fa6
>
> Plus git://git.linaro.org/people/aprzywara/u-boot.git hypmode_v4
> commit 5068b337518617586f2e51b6d616c54dbec4fa62
>
> Plus patches for hypmode on sunxi and to initialise CNTFRQ.
>
> All of which can be found in
>      git://xenbits.xen.org/people/ianc/u-boot.git devel/cubieboard2
>
> Built with:
>      make Cubieboard2_FEL CROSS_COMPILE=arm-linux-gnueabihf-
>
> Linux
> =====
>
> Linux tree:
> https://github.com/mripard/linux.git sunxi/dt-for-3.12
> commit 82abe5294aeadc42508c7944f3a9aec0eece214c
>
> Using in tree DTS arch/arm/boot/dts/sun7i-a20-cubieboard2.dts with the
> following patch.
>
> Build Image and dtbs in the usual way
>
> Xen
> ===
>
> Build in the usual way.
>
> Xen can use CONFIG_EARLY_PRINTK=sun7i.
>
> Booting
> =======
>
> I am using FEL boot per http://linux-sunxi.org/FEL/USBBoot and
> http://linux-sunxi.org/FEL using the fel-sdboot.sunxi on MMC
> mechanism.
>
> linux-sunxi tools from git://github.com/linux-sunxi/sunxi-tools.git
> commit b398456630b310dbf31c7515e8d6af37903c4975 and:
>
> ./usb-boot ~/devel/u-boot.git/spl/u-boot-spl.bin ~/devel/u-boot.git/u-boot.bin \
>            bootxen.scr ~/devel/xen.git/xen/xen \
>            ~/devel/linux.git/arch/arm/boot/dts/sun7i-a20-cubieboard2.dtb
>            ~/devel/linux.git/arch/arm/boot/zImage
>
> bootxen.scr is:
> $ cat bootxen
> setenv bootargs dtuart=serial0 earlyprint loglvl=all conswitch=x
> bootz 0x44000000 - 0x43000000
> $ mkimage -T script -A arm -d bootxen bootxen.scr
>
> DTS patch
> =========
>
>
> diff --git a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
>  31b76f0..6c0c387 100644
> --- a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> @@ -18,6 +18,21 @@
>         model = "Cubietech Cubieboard2";
>         compatible = "cubietech,cubieboard2", "allwinner,sun7i-a20";
>
> +       chosen {
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +
> +               module@0 {
> +                       compatible = "xen,linux-zimage", "xen,multiboot-module";
> +                       bootargs = "console=hvc0 earlyprintk ";
> +                       reg = <0x50000000 0x800000>;
> +               };
> +       };
> +
> +       aliases {
> +               serial0 = &uart0;
> +       };
> +
>         soc@01c00000 {
>                 pinctrl@01c20800 {
>                         led_pins_cubieboard2: led_pins@0 {
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> index f4e4524..bfc5dc2 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -26,11 +26,6 @@
>                         reg = <0>;
>                 };
>
> -               cpu@1 {
> -                       compatible = "arm,cortex-a7";
> -                       device_type = "cpu";
> -                       reg = <1>;
> -               };
>         };
>
>         memory {
> @@ -55,6 +55,14 @@
>                 };
>         };
>
> +        timer {
> +                compatible = "arm,armv7-timer";
> +                interrupts = <1 13 0xf08>,
> +                             <1 14 0xf08>,
> +                             <1 11 0xf08>,
> +                             <1 10 0xf08>;
> +        };
> +
>         soc@01c00000 {
>                 compatible = "simple-bus";
>                 #address-cells = <1>;
> @@ -94,17 +102,6 @@
>                         };
>                 };
>
> -               timer@01c20c00 {
> -                       compatible = "allwinner,sun4i-timer";
> -                       reg = <0x01c20c00 0x90>;
> -                       interrupts = <0 22 1>,
> -                                    <0 23 1>,
> -                                    <0 24 1>,
> -                                    <0 25 1>,
> -                                    <0 67 1>,
> -                                    <0 68 1>;
> -                       clocks = <&osc24M>;
> -               };
>
>                 wdt: watchdog@01c20c90 {
>                         compatible = "allwinner,sun4i-wdt";
>
>


Thanks Ian. I can boot dom0 until rootfs panic as yours. So I tried
booting dom0 with built-in initramfs, but it's failed.  I am not sure
whether the problem is the initramfs or not.



Starting kernel ...

- UART enabled -
- CPU 00000000 booting -
- Machine ID 000010bb -
- Started in Hyp mode -
- Zero BSS -
- Setting up control registers -
- Turning on paging -
- Ready -
-DTB R8 402d7700 -
- PADDR R9 40200000 -
- phys-offset R10 40000000 -
RAM: 0000000040000000 - 00000000bfffffff

MODULE[1]: 0000000060000000 - 0000000060800000
Placing Xen at 0x00000000bfe00000-0x00000000c0000000
Xen heap: 65536 pages  Dom heap: 458752 pages
Looking for UART console serial0
ns16550_uart_dt_init
ns16550 at 1c28000-1c28400
console done?
UART mapped at 10007000
divisor 0
 __  __            _  _   _  _                      _        _     _
 \ \/ /___ _ __   | || | | || |     _   _ _ __  ___| |_ __ _| |__ | | ___
  \  // _ \ '_ \  | || |_| || |_ __| | | | '_ \/ __| __/ _` | '_ \| |/ _ \
  /  \  __/ | | | |__   _|__   _|__| |_| | | | \__ \ || (_| | |_) | |  __/
 /_/\_\___|_| |_|    |_|(_) |_|     \__,_|_| |_|___/\__\__,_|_.__/|_|\___|

(XEN) Xen version 4.4-unstable (joshzhao@)
(arm-unknown-linux-gnueabi-gcc (GCC) 4.6.3) debug=y Wed Sep 11
09:40:18 CST 2013
(XEN) Latest ChangeSet: Mon Aug 26 12:40:44 2013 +0200 git:8a7769b-dirty
(XEN) Console output is synchronous.
(XEN) Processor: "ARM Limited", variant: 0x0, part 0xc07, rev 0x4
(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: 10101105 40000000 01240000 02102211
(XEN)  ISA Features: 02101110 13112111 21232041 11112131 10011142 00000000
(XEN) Platform: ALLWINNER SUN7I-A20
(XEN) Generic Timer IRQ: phys=55 hyp=57 virt=56
(XEN) clock-frequency res:0
(XEN) Using generic timer at 24000 KHz boot_count:0000000027b08aa2
(XEN) GIC initialization:
(XEN)         gic_dist_addr=0000000001c81000
(XEN)         gic_cpu_addr=0000000001c82000
(XEN)         gic_hyp_addr=0000000001c84000
(XEN)         gic_vcpu_addr=0000000001c86000
(XEN)         gic_maintenance_irq=25
(XEN) GIC: 160 lines, 1 cpu, secure (IID 0100143b).
(XEN) Waiting for 0 other CPUs to be ready
(XEN) Using scheduler: SMP Credit Scheduler (credit)
(XEN) Allocated console ring of 16 KiB.
(XEN) VFP implementer 0x41 architecture 2 part 0x30 variant 0x7 rev 0x4
(XEN) Brought up 1 CPUs
(XEN) *** LOADING DOMAIN 0 ***
(XEN) Populate P2M 0x90000000->0xa0000000 (1:1 mapping for dom0)
(XEN) Device-tree contains "xen,xen" node. Ignoring.
(XEN) Loading kernel from boot module 1
(XEN) Loading zImage from 0000000060000000 to 0000000090008000-0000000090534900
(XEN) Loading dom0 DTB to 0x000000009fe00000-0x000000009fe0105f
(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... 2... 1...
(XEN) *** Serial input -> DOM0 (type 'CTRL-a' three times to switch
input to Xen)
(XEN) Freed 216kB init memory.
[    0.000000] Booting Linux on physical CPU 0x0
[    0.000000] Linux version 3.11.0-rc4-01080-gad22d2d-dirty
(joshzhao@joshzhao-ThinkCentre-M58p) (gcc version 4.6.3 (GCC) ) #2 SMP
Tue Sep 10 16:00:01 CST 2013
[    0.000000] CPU: ARMv7 Processor [410fc074] revision 4 (ARMv7), cr=10c5387d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing
instruction cache
[    0.000000] Machine: Allwinner A1X (Device Tree), model: Cubietech
Cubieboard2
[    0.000000] debug: ignoring loglevel setting.
[    0.000000] Memory policy: ECC disabled, Data cache writealloc
[    0.000000] On node 0 totalpages: 65536
[    0.000000] free_area_init_node: node 0, pgdat c0915300,
node_mem_map c0960000
[    0.000000]   DMA zone: 512 pages used for memmap
[    0.000000]   DMA zone: 0 pages reserved
[    0.000000]   DMA zone: 65536 pages, LIFO batch:15
[    0.000000] PERCPU: Embedded 5 pages/cpu @c0b65000 s7808 r0 d12672 u32768
[    0.000000] pcpu-alloc: s7808 r0 d12672 u32768 alloc=8*4096
[    0.000000] pcpu-alloc: [0] 0
[    0.000000] Built 1 zonelists in Zone order, mobility grouping on.
Total pages: 65024
[    0.000000] Kernel command line: console=hvc0,115200n8 init=/init
debug  ignore_loglevel rw  earlyprintk=xen
[    0.000000] PID hash table entries: 1024 (order: 0, 4096 bytes)
[    0.000000] Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
[    0.000000] Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
[    0.000000] Memory: 250240K/262144K available (5052K kernel code,
602K rwdata, 1444K rodata, 2171K init, 290K bss, 11904K reserved, 0K
highmem)
[    0.000000] Virtual kernel memory layout:
[    0.000000]     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
[    0.000000]     fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
[    0.000000]     vmalloc : 0xd0800000 - 0xff000000   ( 744 MB)
[    0.000000]     lowmem  : 0xc0000000 - 0xd0000000   ( 256 MB)
[    0.000000]     pkmap   : 0xbfe00000 - 0xc0000000   (   2 MB)
[    0.000000]       .text : 0xc0008000 - 0xc06602e4   (6497 kB)
[    0.000000]       .init : 0xc0661000 - 0xc087fe80   (2172 kB)
[    0.000000]       .data : 0xc0880000 - 0xc0916a40   ( 603 kB)
[    0.000000]        .bss : 0xc0916a40 - 0xc095f3dc   ( 291 kB)
[    0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
[    0.000000] Hierarchical RCU implementation.
[    0.000000]     RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=1.
[    0.000000] NR_IRQS:16 nr_irqs:16 16
[    0.000000] Architected local timer running at 24.00MHz (virt).
[    0.000000] arch_timer: can't register interrupt 56 (-22)
[    0.000000] Switching to timer-based delay loop
[    0.000000] sched_clock: ARM arch timer >56 bits at 24000kHz, resolution 41ns
[    0.000000] sched_clock: 32 bits at 100 Hz, resolution 10000000ns,
wraps every 4294967286ms
[    0.000000] Console: colour dummy device 80x30
[    3.699313] Calibrating delay loop (skipped), value calculated
using timer frequency.. 48.00 BogoMIPS (lpj=240000)
[    3.699327] pid_max: default: 32768 minimum: 301
[    3.699497] Mount-cache hash table entries: 512
[    3.700917] CPU: Testing write buffer coherency: ok
[    3.701214] /cpus/cpu@0 missing clock-frequency property
[    3.701236] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
[    3.701271] Setting up static identity map for 0xc04c7830 - 0xc04c78c8
[    3.701330] unable to find compatible sirf rstc node in dtb
[    3.701896] Brought up 1 CPUs
[    3.701909] SMP: Total of 1 processors activated (48.00 BogoMIPS).
[    3.701916] CPU: All CPU(s) started in SVC mode.
[    3.702576] devtmpfs: initialized
[    3.706351] Xen 4.4 support found, events_irq=31 gnttab_frame_pfn=b0000
[    3.706437] xen:grant_table: Grant tables using version 1 layout
[    3.706501] Grant table initialized
[    3.706737] pinctrl core: initialized pinctrl subsystem
[    3.707125] regulator-dummy: no parameters
[    3.707449] NET: Registered protocol family 16
[    3.707786] Xen: initializing cpu0
[    3.708099] DMA: preallocated 256 KiB pool for atomic coherent allocations
[    3.708234] unable to find compatible sirf pwrc node in dtb
[    3.710859] Serial: AMBA PL011 UART driver
[    3.715594] bio: create slab <bio-0> at 0
[    3.716527] edma-dma-engine edma-dma-engine.0: Can't allocate PaRAM
dummy slot
[    3.716554] edma-dma-engine: probe of edma-dma-engine.0 failed with error -5
[    3.716661] xen:balloon: Initialising balloon driver
[    3.717367] vgaarb: loaded
[    3.717675] SCSI subsystem initialized
[    3.717852] libata version 3.00 loaded.
[    3.718081] usbcore: registered new interface driver usbfs
[    3.718130] usbcore: registered new interface driver hub
[    3.718233] usbcore: registered new device driver usb
[    3.718602] pps_core: LinuxPPS API ver. 1 registered
[    3.718611] pps_core: Software ver. 5.3.6 - Copyright 2005-2007
Rodolfo Giometti <giometti@linux.it>
[    3.718632] PTP clock support registered
[    3.718664] EDAC MC: Ver: 3.0.0
[    3.719670] Switched to clocksource arch_sys_counter
[    3.727609] NET: Registered protocol family 2
[    3.728177] TCP established hash table entries: 2048 (order: 2, 16384 bytes)
[    3.728223] TCP bind hash table entries: 2048 (order: 2, 16384 bytes)
[    3.728259] TCP: Hash tables configured (established 2048 bind 2048)
[    3.728327] TCP: reno registered
[    3.728342] UDP hash table entries: 256 (order: 1, 8192 bytes)
[    3.728375] UDP-Lite hash table entries: 256 (order: 1, 8192 bytes)
[    3.728599] NET: Registered protocol family 1
[    3.729001] RPC: Registered named UNIX socket transport module.
[    3.729014] RPC: Registered udp transport module.
[    3.729019] RPC: Registered tcp transport module.
[    3.729025] RPC: Registered tcp NFSv4.1 backchannel transport module.
[    3.729039] PCI: CLS 0 bytes, default 64
[    3.852329] NFS: Registering the id_resolver key type
[    3.852405] Key type id_resolver registered
[    3.852412] Key type id_legacy registered
[    3.852660] Block layer SCSI generic (bsg) driver version 0.4
loaded (major 251)
[    3.852670] io scheduler noop registered
[    3.852677] io scheduler deadline registered
[    3.852830] io scheduler cfq registered (default)
[    3.854795] sunxi-pinctrl 1c20800.pinctrl: initialized sunXi PIO driver
[    3.856031] xen:xen_evtchn: Event-channel device installed
[    4.458268] console [hvc0] enabled
[    4.461714] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
[    4.489650] 1c28000.serial: ttyS0 at MMIO 0x1c28000 (irq = 33) is a U6_16550A
[    4.496908] Serial: IMX driver
[    4.500283] serial: Freescale lpuart driver
[    4.504513] [drm] Initialized drm 1.1.0 20060810
[    4.510010] xen_netfront: Initialising Xen virtual ethernet driver
[    4.516291] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[    4.522633] ehci-pci: EHCI PCI platform driver
[    4.527115] ehci-platform: EHCI generic platform driver
[    4.532331] ehci-omap: OMAP-EHCI Host Controller driver
[    4.537513] ehci-orion: EHCI orion driver
[    4.541518] SPEAr-ehci: EHCI SPEAr driver
[    4.545525] tegra-ehci: Tegra EHCI driver
[    4.549747] usbcore: registered new interface driver usb-storage
[    4.556422] mousedev: PS/2 mouse device common for all mice
[    4.562533] sdhci: Secure Digital Host Controller Interface driver
[    4.568534] sdhci: Copyright(c) Pierre Ossman
[    4.573052] sdhci-pltfm: SDHCI platform and OF driver helper
[    4.579004] usbcore: registered new interface driver usbhid
[    4.584398] usbhid: USB HID core driver
[    4.588488] TCP: cubic registered
[    4.592223] NET: Registered protocol family 10
[    4.597186] sit: IPv6 over IPv4 tunneling driver
[    4.602338] Key type dns_resolver registered
[    4.606577] VFP support v0.3: implementor 41 architecture 2 part 30
variant 7 rev 4
[    4.614075] Registering SWP/SWPB emulation handler
[    4.619663] drivers/rtc/hctosys.c: unable to open rtc device (rtc0)
[    4.628188] Freeing unused kernel memory: 2168K (c0661000 - c087f000)
[    4.636010] Kernel panic - not syncing: Attempted to kill init!
exitcode=0x00007f00
[    4.636010]
[    4.645015] CPU: 0 PID: 1 Comm: init Not tainted
3.11.0-rc4-01080-gad22d2d-dirty #2
[    4.652633] [<c001598c>] (unwind_backtrace+0x0/0xf8) from
[<c0011900>] (show_stack+0x10/0x14)
[    4.661054] [<c0011900>] (show_stack+0x10/0x14) from [<c04c33d4>]
(dump_stack+0x6c/0x88)
[    4.669067] [<c04c33d4>] (dump_stack+0x6c/0x88) from [<c04c0e74>]
(panic+0x8c/0x1e4)
[    4.676741] [<c04c0e74>] (panic+0x8c/0x1e4) from [<c00411fc>]
(do_exit+0x7bc/0x8c8)
[    4.684329] [<c00411fc>] (do_exit+0x7bc/0x8c8) from [<c0041370>]
(do_group_exit+0x3c/0xc4)
[    4.692516] [<c0041370>] (do_group_exit+0x3c/0xc4) from
[<c0041408>] (__wake_up_parent+0x0/0x18)

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

* Re: [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support.
  2013-09-11  2:05 ` [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support Josh Zhao
@ 2013-09-11 10:15   ` Ian Campbell
  0 siblings, 0 replies; 34+ messages in thread
From: Ian Campbell @ 2013-09-11 10:15 UTC (permalink / raw)
  To: Josh Zhao
  Cc: Keir Fraser, Stefano Stabellini, Julien Grall, Tim Deegan,
	xen-devel, jbuelich

On Wed, 2013-09-11 at 10:05 +0800, Josh Zhao wrote:
[...]

Please trim your quotes.

> Thanks Ian. I can boot dom0 until rootfs panic as yours.

Excellent.

>  So I tried
> booting dom0 with built-in initramfs, but it's failed.  I am not sure
> whether the problem is the initramfs or not.

Did it work with just the kernel without Xen?

Are you going to dig into the issue?

Ian

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

* Re: [PATCH RFC 8/8] xen/arm: Some early trap logging
  2013-09-10 14:18 ` [PATCH RFC 8/8] xen/arm: Some early trap logging Ian Campbell
@ 2013-09-13 13:26   ` Julien Grall
  2013-09-13 13:35     ` Ian Campbell
  0 siblings, 1 reply; 34+ messages in thread
From: Julien Grall @ 2013-09-13 13:26 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Josh Zhao, stefano.stabellini, tim, xen-devel

On 09/10/2013 03:18 PM, Ian Campbell wrote:
> Not for really for application, might be useful to someone?

I think it could be useful for early debugging on new platform or until
init_traps initialized the correct handlers.

Can you update this patch with more information about the trap (reset,
undefined, ...)?

> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/arch/arm/arm32/head.S |   15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 79e95b6..06d53ad 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -366,6 +366,9 @@ paging:
>          bne   1b
>  
>  launch:
> +        adr   r0, early_trap_vector
> +        mcr   CP32(r0, VBAR_EL2)
> +        
>          ldr   r0, =init_stack        /* Find the boot-time stack */
>          ldr   sp, [r0]
>          add   sp, #STACK_SIZE        /* (which grows down from the top). */
> @@ -376,12 +379,24 @@ launch:
>          beq   start_xen              /* and disappear into the land of C */
>          b     start_secondary        /* (to the appropriate entry point) */
>  
> +trap_early:
> +        PRINT("\r\nEARLY TRAP\r\n")
>  /* Fail-stop
>   * r0: string explaining why */
>  fail:   PRINT("- Boot failed -\r\n")
>  1:      wfe
>          b     1b
>  
> +        .align 5
> +early_trap_vector:
> +        .word 0                         /* 0x00 - Reset */
> +        b trap_early                    /* 0x04 - Undefined Instruction */
> +        b trap_early                    /* 0x08 - Supervisor Call */
> +        b trap_early                    /* 0x0c - Prefetch Abort */
> +        b trap_early                    /* 0x10 - Data Abort */
> +        b trap_early                    /* 0x14 - Hypervisor */
> +        b trap_early                    /* 0x18 - IRQ */
> +        b trap_early                    /* 0x1c - FIQ */
>  
>  #ifdef EARLY_PRINTK
>  /* Bring up the UART.
> 


-- 
Julien Grall

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

* Re: [PATCH RFC 8/8] xen/arm: Some early trap logging
  2013-09-13 13:26   ` Julien Grall
@ 2013-09-13 13:35     ` Ian Campbell
  2013-09-13 13:44       ` Julien Grall
  0 siblings, 1 reply; 34+ messages in thread
From: Ian Campbell @ 2013-09-13 13:35 UTC (permalink / raw)
  To: Julien Grall; +Cc: Josh Zhao, stefano.stabellini, tim, xen-devel

On Fri, 2013-09-13 at 14:26 +0100, Julien Grall wrote:
> On 09/10/2013 03:18 PM, Ian Campbell wrote:
> > Not for really for application, might be useful to someone?
> 
> I think it could be useful for early debugging on new platform or until
> init_traps initialized the correct handlers.

True. Perhaps only if debug=y?

> Can you update this patch with more information about the trap (reset,
> undefined, ...)?

I will at some point, although not with too much urgency.

> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  xen/arch/arm/arm32/head.S |   15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> > index 79e95b6..06d53ad 100644
> > --- a/xen/arch/arm/arm32/head.S
> > +++ b/xen/arch/arm/arm32/head.S
> > @@ -366,6 +366,9 @@ paging:
> >          bne   1b
> >  
> >  launch:
> > +        adr   r0, early_trap_vector
> > +        mcr   CP32(r0, VBAR_EL2)
> > +        
> >          ldr   r0, =init_stack        /* Find the boot-time stack */
> >          ldr   sp, [r0]
> >          add   sp, #STACK_SIZE        /* (which grows down from the top). */
> > @@ -376,12 +379,24 @@ launch:
> >          beq   start_xen              /* and disappear into the land of C */
> >          b     start_secondary        /* (to the appropriate entry point) */
> >  
> > +trap_early:
> > +        PRINT("\r\nEARLY TRAP\r\n")
> >  /* Fail-stop
> >   * r0: string explaining why */
> >  fail:   PRINT("- Boot failed -\r\n")
> >  1:      wfe
> >          b     1b
> >  
> > +        .align 5
> > +early_trap_vector:
> > +        .word 0                         /* 0x00 - Reset */
> > +        b trap_early                    /* 0x04 - Undefined Instruction */
> > +        b trap_early                    /* 0x08 - Supervisor Call */
> > +        b trap_early                    /* 0x0c - Prefetch Abort */
> > +        b trap_early                    /* 0x10 - Data Abort */
> > +        b trap_early                    /* 0x14 - Hypervisor */
> > +        b trap_early                    /* 0x18 - IRQ */
> > +        b trap_early                    /* 0x1c - FIQ */
> >  
> >  #ifdef EARLY_PRINTK
> >  /* Bring up the UART.
> > 
> 
> 

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

* Re: [PATCH RFC 8/8] xen/arm: Some early trap logging
  2013-09-13 13:35     ` Ian Campbell
@ 2013-09-13 13:44       ` Julien Grall
  0 siblings, 0 replies; 34+ messages in thread
From: Julien Grall @ 2013-09-13 13:44 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Josh Zhao, stefano.stabellini, tim, xen-devel

On 09/13/2013 02:35 PM, Ian Campbell wrote:
> On Fri, 2013-09-13 at 14:26 +0100, Julien Grall wrote:
>> On 09/10/2013 03:18 PM, Ian Campbell wrote:
>>> Not for really for application, might be useful to someone?
>>
>> I think it could be useful for early debugging on new platform or until
>> init_traps initialized the correct handlers.
> 
> True. Perhaps only if debug=y?

Sounds good.

>> Can you update this patch with more information about the trap (reset,
>> undefined, ...)?
> 
> I will at some point, although not with too much urgency.

Ok. So with the modification above, I'm ok for this patch.

>>>
>>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>>> ---
>>>  xen/arch/arm/arm32/head.S |   15 +++++++++++++++
>>>  1 file changed, 15 insertions(+)
>>>
>>> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
>>> index 79e95b6..06d53ad 100644
>>> --- a/xen/arch/arm/arm32/head.S
>>> +++ b/xen/arch/arm/arm32/head.S
>>> @@ -366,6 +366,9 @@ paging:
>>>          bne   1b
>>>  
>>>  launch:
>>> +        adr   r0, early_trap_vector
>>> +        mcr   CP32(r0, VBAR_EL2)
>>> +        
>>>          ldr   r0, =init_stack        /* Find the boot-time stack */
>>>          ldr   sp, [r0]
>>>          add   sp, #STACK_SIZE        /* (which grows down from the top). */
>>> @@ -376,12 +379,24 @@ launch:
>>>          beq   start_xen              /* and disappear into the land of C */
>>>          b     start_secondary        /* (to the appropriate entry point) */
>>>  
>>> +trap_early:
>>> +        PRINT("\r\nEARLY TRAP\r\n")
>>>  /* Fail-stop
>>>   * r0: string explaining why */
>>>  fail:   PRINT("- Boot failed -\r\n")
>>>  1:      wfe
>>>          b     1b
>>>  
>>> +        .align 5
>>> +early_trap_vector:
>>> +        .word 0                         /* 0x00 - Reset */
>>> +        b trap_early                    /* 0x04 - Undefined Instruction */
>>> +        b trap_early                    /* 0x08 - Supervisor Call */
>>> +        b trap_early                    /* 0x0c - Prefetch Abort */
>>> +        b trap_early                    /* 0x10 - Data Abort */
>>> +        b trap_early                    /* 0x14 - Hypervisor */
>>> +        b trap_early                    /* 0x18 - IRQ */
>>> +        b trap_early                    /* 0x1c - FIQ */
>>>  
>>>  #ifdef EARLY_PRINTK
>>>  /* Bring up the UART.
>>>
>>
>>
> 
> 


-- 
Julien Grall

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

end of thread, other threads:[~2013-09-13 13:44 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-10 14:18 [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support Ian Campbell
2013-09-10 14:18 ` [PATCH RFC 1/8] xen/arm: Implement ioremap Ian Campbell
2013-09-10 15:03   ` Julien Grall
2013-09-10 15:14     ` Ian Campbell
2013-09-10 15:20       ` Julien Grall
2013-09-10 14:18 ` [PATCH RFC 2/8] xen/arm: implement read[bl] and write[bl] Ian Campbell
2013-09-10 15:00   ` Julien Grall
2013-09-10 15:09     ` Ian Campbell
2013-09-10 15:12       ` Julien Grall
2013-09-10 15:15         ` Ian Campbell
2013-09-10 14:18 ` [PATCH RFC 3/8] ns16550: make usable on ARM Ian Campbell
2013-09-10 14:36   ` Keir Fraser
2013-09-10 14:45     ` Ian Campbell
2013-09-10 15:00   ` Jan Beulich
2013-09-10 15:11     ` Ian Campbell
2013-09-10 15:18       ` Jan Beulich
2013-09-10 15:21         ` Ian Campbell
2013-09-10 14:18 ` [PATCH RFC 4/8] ns16550: support DesignWare 8250 Ian Campbell
2013-09-10 14:36   ` Keir Fraser
2013-09-10 15:02   ` Jan Beulich
2013-09-10 15:12     ` Ian Campbell
2013-09-10 15:19       ` Jan Beulich
2013-09-10 15:21         ` Ian Campbell
2013-09-10 15:28           ` Jan Beulich
2013-09-10 15:30             ` Ian Campbell
2013-09-10 14:18 ` [PATCH RFC 5/8] xen/arm: Support Cortex-A7 GIC Ian Campbell
2013-09-10 14:18 ` [PATCH RFC 6/8] xen/arm: Basic support for sunxi/sun7i platform Ian Campbell
2013-09-10 14:18 ` [PATCH RFC 7/8] xen/arm: Blacklist some sun7i UARTs Ian Campbell
2013-09-10 14:18 ` [PATCH RFC 8/8] xen/arm: Some early trap logging Ian Campbell
2013-09-13 13:26   ` Julien Grall
2013-09-13 13:35     ` Ian Campbell
2013-09-13 13:44       ` Julien Grall
2013-09-11  2:05 ` [PATCH RFC 0/8] xen/arm: very initial cubieboard2 support Josh Zhao
2013-09-11 10:15   ` Ian Campbell

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.