* [PATCH 0/4] Support 8250 compatible UART for OMAP5432
@ 2013-07-19 12:57 Chen Baozi
2013-07-19 12:57 ` [PATCH 1/4] xen: extract register definitions from ns16550 into a separated header Chen Baozi
` (3 more replies)
0 siblings, 4 replies; 27+ messages in thread
From: Chen Baozi @ 2013-07-19 12:57 UTC (permalink / raw)
To: Ian Campbell; +Cc: Chen Baozi, xen-devel
This patchset enables UART support for OMAP5432 ES2.0 devboard.
Chen Baozi (4):
xen: extract register definitions from ns16550 into a separated header
xen/arm: add OMAP5432 UART support for early_printk
xen: set the right flags when enabling interrupts for 8250
xen/arm: enable ns16550 uart driver for OMAP5432
docs/misc/arm/early-printk.txt | 1 +
xen/arch/arm/Rules.mk | 5 +
xen/arch/arm/arm32/debug-8250.inc | 48 ++++++
xen/drivers/char/ns16550.c | 312 ++++++++++++++++++++++----------------
xen/include/asm-arm/io.h | 49 ++++++
xen/include/xen/8250.h | 104 +++++++++++++
xen/include/xen/serial.h | 7 +-
7 files changed, 393 insertions(+), 133 deletions(-)
create mode 100644 xen/arch/arm/arm32/debug-8250.inc
create mode 100644 xen/include/xen/8250.h
--
1.8.1.4
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/4] xen: extract register definitions from ns16550 into a separated header
2013-07-19 12:57 [PATCH 0/4] Support 8250 compatible UART for OMAP5432 Chen Baozi
@ 2013-07-19 12:57 ` Chen Baozi
2013-07-19 15:35 ` Julien Grall
2013-07-19 12:57 ` [PATCH 2/4] xen/arm: add OMAP5432 UART support for early_printk Chen Baozi
` (2 subsequent siblings)
3 siblings, 1 reply; 27+ messages in thread
From: Chen Baozi @ 2013-07-19 12:57 UTC (permalink / raw)
To: Ian Campbell; +Cc: Chen Baozi, bjzhang, Keir Fraser, xen-devel
Since both UART driver codes on Allwinner A31, OMAP5 and x86 would use
these definitions, we refactor the codes into a separated header to avoid
unnecessary duplication.
Signed-off-by: Chen Baozi <baozich@gmail.com>
---
xen/drivers/char/ns16550.c | 155 +++++++++++++--------------------------------
xen/include/xen/8250.h | 104 ++++++++++++++++++++++++++++++
2 files changed, 148 insertions(+), 111 deletions(-)
create mode 100644 xen/include/xen/8250.h
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index e0c87bb..9248330 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -19,6 +19,7 @@
#include <xen/iocap.h>
#include <xen/pci.h>
#include <xen/pci_regs.h>
+#include <xen/8250.h>
#include <asm/io.h>
#ifdef CONFIG_X86
#include <asm/fixmap.h>
@@ -58,76 +59,6 @@ static struct ns16550 {
u8 bar_idx;
} ns16550_com[2] = { { 0 } };
-/* Register offsets */
-#define RBR 0x00 /* receive buffer */
-#define THR 0x00 /* transmit holding */
-#define IER 0x01 /* interrupt enable */
-#define IIR 0x02 /* interrupt identity */
-#define FCR 0x02 /* FIFO control */
-#define LCR 0x03 /* line control */
-#define MCR 0x04 /* Modem control */
-#define LSR 0x05 /* line status */
-#define MSR 0x06 /* Modem status */
-#define DLL 0x00 /* divisor latch (ls) (DLAB=1) */
-#define DLM 0x01 /* divisor latch (ms) (DLAB=1) */
-
-/* Interrupt Enable Register */
-#define IER_ERDAI 0x01 /* rx data recv'd */
-#define IER_ETHREI 0x02 /* tx reg. empty */
-#define IER_ELSI 0x04 /* rx line status */
-#define IER_EMSI 0x08 /* MODEM status */
-
-/* Interrupt Identification Register */
-#define IIR_NOINT 0x01 /* no interrupt pending */
-#define IIR_IMASK 0x06 /* interrupt identity: */
-#define IIR_LSI 0x06 /* - rx line status */
-#define IIR_RDAI 0x04 /* - rx data recv'd */
-#define IIR_THREI 0x02 /* - tx reg. empty */
-#define IIR_MSI 0x00 /* - MODEM status */
-
-/* FIFO Control Register */
-#define FCR_ENABLE 0x01 /* enable FIFO */
-#define FCR_CLRX 0x02 /* clear Rx FIFO */
-#define FCR_CLTX 0x04 /* clear Tx FIFO */
-#define FCR_DMA 0x10 /* enter DMA mode */
-#define FCR_TRG1 0x00 /* Rx FIFO trig lev 1 */
-#define FCR_TRG4 0x40 /* Rx FIFO trig lev 4 */
-#define FCR_TRG8 0x80 /* Rx FIFO trig lev 8 */
-#define FCR_TRG14 0xc0 /* Rx FIFO trig lev 14 */
-
-/* Line Control Register */
-#define LCR_DLAB 0x80 /* Divisor Latch Access */
-
-/* Modem Control Register */
-#define MCR_DTR 0x01 /* Data Terminal Ready */
-#define MCR_RTS 0x02 /* Request to Send */
-#define MCR_OUT2 0x08 /* OUT2: interrupt mask */
-#define MCR_LOOP 0x10 /* Enable loopback test mode */
-
-/* Line Status Register */
-#define LSR_DR 0x01 /* Data ready */
-#define LSR_OE 0x02 /* Overrun */
-#define LSR_PE 0x04 /* Parity error */
-#define LSR_FE 0x08 /* Framing error */
-#define LSR_BI 0x10 /* Break */
-#define LSR_THRE 0x20 /* Xmit hold reg empty */
-#define LSR_TEMT 0x40 /* Xmitter empty */
-#define LSR_ERR 0x80 /* Error */
-
-/* These parity settings can be ORed directly into the LCR. */
-#define PARITY_NONE (0<<3)
-#define PARITY_ODD (1<<3)
-#define PARITY_EVEN (3<<3)
-#define PARITY_MARK (5<<3)
-#define PARITY_SPACE (7<<3)
-
-/* Frequency of external clock source. This definition assumes PC platform. */
-#define UART_CLOCK_HZ 1843200
-
-/* Resume retry settings */
-#define RESUME_DELAY MILLISECS(10)
-#define RESUME_RETRIES 100
-
static char ns_read_reg(struct ns16550 *uart, int reg)
{
if ( uart->remapped_io_base == NULL )
@@ -150,12 +81,12 @@ static void ns16550_interrupt(
uart->intr_works = 1;
- while ( !(ns_read_reg(uart, IIR) & IIR_NOINT) )
+ while ( !(ns_read_reg(uart, UART_IIR) & UART_IIR_NOINT) )
{
- char lsr = ns_read_reg(uart, LSR);
- if ( lsr & LSR_THRE )
+ char lsr = ns_read_reg(uart, UART_LSR);
+ if ( lsr & UART_LSR_THRE )
serial_tx_interrupt(port, regs);
- if ( lsr & LSR_DR )
+ if ( lsr & UART_LSR_DR )
serial_rx_interrupt(port, regs);
}
}
@@ -171,10 +102,10 @@ static void __ns16550_poll(struct cpu_user_regs *regs)
if ( uart->intr_works )
return; /* Interrupts work - no more polling */
- while ( ns_read_reg(uart, LSR) & LSR_DR )
+ while ( ns_read_reg(uart, UART_LSR) & UART_LSR_DR )
serial_rx_interrupt(port, regs);
- if ( ns_read_reg(uart, LSR) & LSR_THRE )
+ if ( ns_read_reg(uart, UART_LSR) & UART_LSR_THRE )
serial_tx_interrupt(port, regs);
set_timer(&uart->timer, NOW() + MILLISECS(uart->timeout_ms));
@@ -194,23 +125,23 @@ static unsigned int ns16550_tx_ready(struct serial_port *port)
{
struct ns16550 *uart = port->uart;
- return ns_read_reg(uart, LSR) & LSR_THRE ? uart->fifo_size : 0;
+ return ns_read_reg(uart, UART_LSR) & UART_LSR_THRE ? uart->fifo_size : 0;
}
static void ns16550_putc(struct serial_port *port, char c)
{
struct ns16550 *uart = port->uart;
- ns_write_reg(uart, THR, c);
+ ns_write_reg(uart, UART_THR, c);
}
static int ns16550_getc(struct serial_port *port, char *pc)
{
struct ns16550 *uart = port->uart;
- if ( !(ns_read_reg(uart, LSR) & LSR_DR) )
+ if ( !(ns_read_reg(uart, UART_LSR) & UART_LSR_DR) )
return 0;
- *pc = ns_read_reg(uart, RBR);
+ *pc = ns_read_reg(uart, UART_RBR);
return 1;
}
@@ -244,31 +175,32 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
lcr = (uart->data_bits - 5) | ((uart->stop_bits - 1) << 2) | uart->parity;
/* No interrupts. */
- ns_write_reg(uart, IER, 0);
+ ns_write_reg(uart, UART_IER, 0);
/* Line control and baud-rate generator. */
- ns_write_reg(uart, LCR, lcr | LCR_DLAB);
+ ns_write_reg(uart, UART_LCR, lcr | UART_LCR_DLAB);
if ( uart->baud != BAUD_AUTO )
{
/* Baud rate specified: program it into the divisor latch. */
divisor = uart->clock_hz / (uart->baud << 4);
- ns_write_reg(uart, DLL, (char)divisor);
- ns_write_reg(uart, DLM, (char)(divisor >> 8));
+ ns_write_reg(uart, UART_DLL, (char)divisor);
+ ns_write_reg(uart, UART_DLM, (char)(divisor >> 8));
}
else
{
/* Baud rate already set: read it out from the divisor latch. */
- divisor = ns_read_reg(uart, DLL);
- divisor |= ns_read_reg(uart, DLM) << 8;
+ divisor = ns_read_reg(uart, UART_DLL);
+ divisor |= ns_read_reg(uart, UART_DLM) << 8;
uart->baud = uart->clock_hz / (divisor << 4);
}
- ns_write_reg(uart, LCR, lcr);
+ ns_write_reg(uart, UART_LCR, lcr);
/* No flow ctrl: DTR and RTS are both wedged high to keep remote happy. */
- ns_write_reg(uart, MCR, MCR_DTR | MCR_RTS);
+ ns_write_reg(uart, UART_MCR, UART_MCR_DTR | UART_MCR_RTS);
/* Enable and clear the FIFOs. Set a large trigger threshold. */
- ns_write_reg(uart, FCR, FCR_ENABLE | FCR_CLRX | FCR_CLTX | FCR_TRG14);
+ ns_write_reg(uart, UART_FCR,
+ UART_FCR_ENABLE | UART_FCR_CLRX | UART_FCR_CLTX | UART_FCR_TRG14);
}
static void __init ns16550_init_preirq(struct serial_port *port)
@@ -292,8 +224,8 @@ static void __init ns16550_init_preirq(struct serial_port *port)
ns16550_setup_preirq(uart);
/* Check this really is a 16550+. Otherwise we have no FIFOs. */
- if ( ((ns_read_reg(uart, IIR) & 0xc0) == 0xc0) &&
- ((ns_read_reg(uart, FCR) & FCR_TRG14) == FCR_TRG14) )
+ if ( ((ns_read_reg(uart, UART_IIR) & 0xc0) == 0xc0) &&
+ ((ns_read_reg(uart, UART_FCR) & UART_FCR_TRG14) == UART_FCR_TRG14) )
uart->fifo_size = 16;
}
@@ -302,10 +234,11 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
if ( uart->irq > 0 )
{
/* Master interrupt enable; also keep DTR/RTS asserted. */
- ns_write_reg(uart, MCR, MCR_OUT2 | MCR_DTR | MCR_RTS);
+ ns_write_reg(uart,
+ UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
/* Enable receive and transmit interrupts. */
- ns_write_reg(uart, IER, IER_ERDAI | IER_ETHREI);
+ ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ETHREI);
}
if ( uart->irq >= 0 )
@@ -374,11 +307,11 @@ static void _ns16550_resume(struct serial_port *port)
static int ns16550_ioport_invalid(struct ns16550 *uart)
{
- return ((((unsigned char)ns_read_reg(uart, LSR)) == 0xff) &&
- (((unsigned char)ns_read_reg(uart, MCR)) == 0xff) &&
- (((unsigned char)ns_read_reg(uart, IER)) == 0xff) &&
- (((unsigned char)ns_read_reg(uart, IIR)) == 0xff) &&
- (((unsigned char)ns_read_reg(uart, LCR)) == 0xff));
+ return ((((unsigned char)ns_read_reg(uart, UART_LSR)) == 0xff) &&
+ (((unsigned char)ns_read_reg(uart, UART_MCR)) == 0xff) &&
+ (((unsigned char)ns_read_reg(uart, UART_IER)) == 0xff) &&
+ (((unsigned char)ns_read_reg(uart, UART_IIR)) == 0xff) &&
+ (((unsigned char)ns_read_reg(uart, UART_LCR)) == 0xff));
}
static int delayed_resume_tries;
@@ -457,15 +390,15 @@ static int __init parse_parity_char(int c)
switch ( c )
{
case 'n':
- return PARITY_NONE;
+ return UART_PARITY_NONE;
case 'o':
- return PARITY_ODD;
+ return UART_PARITY_ODD;
case 'e':
- return PARITY_EVEN;
+ return UART_PARITY_EVEN;
case 'm':
- return PARITY_MARK;
+ return UART_PARITY_MARK;
case 's':
- return PARITY_SPACE;
+ return UART_PARITY_SPACE;
}
return 0;
}
@@ -500,17 +433,17 @@ static int __init check_existence(struct ns16550 *uart)
* Do a simple existence test first; if we fail this,
* there's no point trying anything else.
*/
- scratch = ns_read_reg(uart, IER);
- ns_write_reg(uart, IER, 0);
+ scratch = ns_read_reg(uart, UART_IER);
+ ns_write_reg(uart, UART_IER, 0);
/*
* Mask out IER[7:4] bits for test as some UARTs (e.g. TL
* 16C754B) allow only to modify them if an EFR bit is set.
*/
- scratch2 = ns_read_reg(uart, IER) & 0x0f;
- ns_write_reg(uart, IER, 0x0F);
- scratch3 = ns_read_reg(uart, IER) & 0x0f;
- ns_write_reg(uart, IER, scratch);
+ scratch2 = ns_read_reg(uart, UART_IER) & 0x0f;
+ ns_write_reg(uart,UART_IER, 0x0F);
+ scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
+ ns_write_reg(uart, UART_IER, scratch);
if ( (scratch2 != 0) || (scratch3 != 0x0F) )
return 0;
@@ -518,8 +451,8 @@ static int __init check_existence(struct ns16550 *uart)
* Check to see if a UART is really there.
* Use loopback test mode.
*/
- ns_write_reg(uart, MCR, MCR_LOOP | 0x0A);
- status = ns_read_reg(uart, MSR) & 0xF0;
+ ns_write_reg(uart, UART_MCR, UART_MCR_LOOP | 0x0A);
+ status = ns_read_reg(uart, UART_MSR) & 0xF0;
return (status == 0x90);
}
diff --git a/xen/include/xen/8250.h b/xen/include/xen/8250.h
new file mode 100644
index 0000000..84f35bc
--- /dev/null
+++ b/xen/include/xen/8250.h
@@ -0,0 +1,104 @@
+/*
+ * xen/include/xen/8250.h
+ *
+ * This header is extracted from driver/char/ns16550.c
+ *
+ * Common constant definition between early printk and the UART driver
+ * for the 16550-series UART
+ *
+ * Copyright (c) 2003-2005, K A Fraser
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __XEN_NS16550_H__
+#define __XEN_NS16550_H__
+
+/* Register offsets */
+#define UART_RBR 0x00 /* receive buffer */
+#define UART_THR 0x00 /* transmit holding */
+#define UART_IER 0x01 /* interrupt enable */
+#define UART_IIR 0x02 /* interrupt identity */
+#define UART_FCR 0x02 /* FIFO control */
+#define UART_LCR 0x03 /* line control */
+#define UART_MCR 0x04 /* Modem control */
+#define UART_LSR 0x05 /* line status */
+#define UART_MSR 0x06 /* Modem status */
+#define UART_DLL 0x00 /* divisor latch (ls) (DLAB=1) */
+#define UART_DLM 0x01 /* divisor latch (ms) (DLAB=1) */
+
+/* Interrupt Enable Register */
+#define UART_IER_ERDAI 0x01 /* rx data recv'd */
+#define UART_IER_ETHREI 0x02 /* tx reg. empty */
+#define UART_IER_ELSI 0x04 /* rx line status */
+#define UART_IER_EMSI 0x08 /* MODEM status */
+
+/* Interrupt Identificatiegister */
+#define UART_IIR_NOINT 0x01 /* no interrupt pending */
+#define UART_IIR_IMA 0x06 /* interrupt identity: */
+#define UART_IIR_LSI 0x06 /* - rx line status */
+#define UART_IIR_RDA 0x04 /* - rx data recv'd */
+#define UART_IIR_THR 0x02 /* - tx reg. empty */
+#define UART_IIR_MSI 0x00 /* - MODEM status */
+
+/* FIFO Control Register */
+#define UART_FCR_ENABLE 0x01 /* enable FIFO */
+#define UART_FCR_CLRX 0x02 /* clear Rx FIFO */
+#define UART_FCR_CLTX 0x04 /* clear Tx FIFO */
+#define UART_FCR_DMA 0x10 /* enter DMA mode */
+#define UART_FCR_TRG1 0x00 /* Rx FIFO trig lev 1 */
+#define UART_FCR_TRG4 0x40 /* Rx FIFO trig lev 4 */
+#define UART_FCR_TRG8 0x80 /* Rx FIFO trig lev 8 */
+#define UART_FCR_TRG14 0xc0 /* Rx FIFO trig lev 14 */
+
+/* Line Control Register */
+#define UART_LCR_DLAB 0x80 /* Divisor Latch Access */
+
+/* Modem Control Register */
+#define UART_MCR_DTR 0x01 /* Data Terminal Ready */
+#define UART_MCR_RTS 0x02 /* Request to Send */
+#define UART_MCR_OUT2 0x08 /* OUT2: interrupt mask */
+#define UART_MCR_LOOP 0x10 /* Enable loopback test mode */
+
+/* Line Status Register */
+#define UART_LSR_DR 0x01 /* Data ready */
+#define UART_LSR_OE 0x02 /* Overrun */
+#define UART_LSR_PE 0x04 /* Parity error */
+#define UART_LSR_FE 0x08 /* Framing error */
+#define UART_LSR_BI 0x10 /* Break */
+#define UART_LSR_THRE 0x20 /* Xmit hold reg empty */
+#define UART_LSR_TEMT 0x40 /* Xmitter empty */
+#define UART_LSR_ERR 0x80 /* Error */
+
+/* These parity settings can be ORed directly into the LCR. */
+#define UART_PARITY_NONE (0<<3)
+#define UART_PARITY_ODD (1<<3)
+#define UART_PARITY_EVEN (3<<3)
+#define UART_PARITY_MARK (5<<3)
+#define UART_PARITY_SPACE (7<<3)
+
+/* Frequency of external clock source. This definition assumes PC platform. */
+#define UART_CLOCK_HZ 1843200
+
+/* Resume retry settings */
+#define RESUME_DELAY MILLISECS(10)
+#define RESUME_RETRIES 100
+
+#endif /* __XEN_NS16550_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/4] xen/arm: add OMAP5432 UART support for early_printk
2013-07-19 12:57 [PATCH 0/4] Support 8250 compatible UART for OMAP5432 Chen Baozi
2013-07-19 12:57 ` [PATCH 1/4] xen: extract register definitions from ns16550 into a separated header Chen Baozi
@ 2013-07-19 12:57 ` Chen Baozi
2013-07-19 15:49 ` Julien Grall
2013-07-19 12:57 ` [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250 Chen Baozi
2013-07-19 12:57 ` [PATCH 4/4] xen/arm: enable ns16550 uart driver for OMAP5432 Chen Baozi
3 siblings, 1 reply; 27+ messages in thread
From: Chen Baozi @ 2013-07-19 12:57 UTC (permalink / raw)
To: Ian Campbell; +Cc: Chen Baozi, bjzhang, xen-devel
Signed-off-by: Chen Baozi <baozich@gmail.com>
---
docs/misc/arm/early-printk.txt | 1 +
xen/arch/arm/Rules.mk | 4 ++++
xen/arch/arm/arm32/debug-8250.inc | 48 +++++++++++++++++++++++++++++++++++++++
3 files changed, 53 insertions(+)
create mode 100644 xen/arch/arm/arm32/debug-8250.inc
diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt
index fbc3208..874488f 100644
--- a/docs/misc/arm/early-printk.txt
+++ b/docs/misc/arm/early-printk.txt
@@ -13,6 +13,7 @@ where mach is the name of the machine:
- exynos5250: printk with the second UART
- midway: printk with the pl011 on Calxeda Midway processors
- fastmodel: printk on ARM Fastmodel software emulators
+ - omap5432: printk with UART3 on TI OMAP5432 processors
The base address and baud rate is hardcoded in xen/arch/arm/Rules.mk,
see there when adding support for new machines.
diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index 422ed04..eaa03fb 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -64,6 +64,10 @@ EARLY_PRINTK_INC := pl011
EARLY_PRINTK_BAUD := 115200
EARLY_UART_BASE_ADDRESS := 0xfff36000
endif
+ifeq ($(CONFIG_EARLY_PRINTK), omap5432)
+EARLY_PRINTK_INC := 8250
+EARLY_UART_BASE_ADDRESS := 0x48020000
+endif
ifneq ($(EARLY_PRINTK_INC),)
EARLY_PRINTK := y
diff --git a/xen/arch/arm/arm32/debug-8250.inc b/xen/arch/arm/arm32/debug-8250.inc
new file mode 100644
index 0000000..cbb63ca
--- /dev/null
+++ b/xen/arch/arm/arm32/debug-8250.inc
@@ -0,0 +1,48 @@
+/*
+ * xen/arch/arm/arm32/debug-8250.inc
+ *
+ * 8250 specific debug code
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <xen/8250.h>
+
+/* OMAP UART initialization
+ * rb: register which contains the UART base address
+ * rc: scratch register 1
+ * rd: scratch register 2 */
+.macro early_uart_init rb rc rd
+.endm
+
+/* OMAP UART wait UART to be ready to transmit
+ * rb: register which contains the UART base address
+ * rc: scratch register */
+.macro early_uart_ready rb rc
+1:
+ ldr \rc, [\rb, #(UART_LSR<<2)] /* Read line status register */
+ tst \rc, #UART_LSR_THRE /* Check Xmit holding register flag */
+ beq 1b /* Wait for the UART to be ready */
+.endm
+
+/* OMAP UART transmit character
+ * rb: register which contains the UART base address
+ * rt: register which contains the character to transmit */
+.macro early_uart_transmit rb rt
+ str \rt, [\rb, #UART_THR] /* Write Transmit buffer */
+.endm
+
+/*
+ * Local variables:
+ * mode: ASM
+ * indent-tabs-mode: nil
+ * End:
+ */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-19 12:57 [PATCH 0/4] Support 8250 compatible UART for OMAP5432 Chen Baozi
2013-07-19 12:57 ` [PATCH 1/4] xen: extract register definitions from ns16550 into a separated header Chen Baozi
2013-07-19 12:57 ` [PATCH 2/4] xen/arm: add OMAP5432 UART support for early_printk Chen Baozi
@ 2013-07-19 12:57 ` Chen Baozi
2013-07-19 14:15 ` Ian Campbell
2013-07-19 12:57 ` [PATCH 4/4] xen/arm: enable ns16550 uart driver for OMAP5432 Chen Baozi
3 siblings, 1 reply; 27+ messages in thread
From: Chen Baozi @ 2013-07-19 12:57 UTC (permalink / raw)
To: Ian Campbell; +Cc: Chen Baozi, Keir Fraser, xen-devel
Previous bits setting would cause generating THRE interrupts, which cannot
be cleared and make system loop in gic_interrupt endlessly. Set 'received
data available' and 'receiver line status' bits of IER when enabling
interrupts as what Linux 8250 driver does.
Signed-off-by: Chen Baozi <baozich@gmail.com>
---
xen/drivers/char/ns16550.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 9248330..60512be 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -237,8 +237,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
ns_write_reg(uart,
UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
- /* Enable receive and transmit interrupts. */
- ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ETHREI);
+ /* Enable receive and line status interrupts. */
+ ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ELSI);
}
if ( uart->irq >= 0 )
--
1.8.1.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 4/4] xen/arm: enable ns16550 uart driver for OMAP5432
2013-07-19 12:57 [PATCH 0/4] Support 8250 compatible UART for OMAP5432 Chen Baozi
` (2 preceding siblings ...)
2013-07-19 12:57 ` [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250 Chen Baozi
@ 2013-07-19 12:57 ` Chen Baozi
2013-07-19 17:06 ` Julien Grall
3 siblings, 1 reply; 27+ messages in thread
From: Chen Baozi @ 2013-07-19 12:57 UTC (permalink / raw)
To: Ian Campbell; +Cc: Chen Baozi, Keir Fraser, xen-devel
Signed-off-by: Chen Baozi <baozich@gmail.com>
---
xen/arch/arm/Rules.mk | 1 +
xen/drivers/char/ns16550.c | 155 +++++++++++++++++++++++++++++++++++++++------
xen/include/asm-arm/io.h | 49 ++++++++++++++
xen/include/xen/serial.h | 7 +-
4 files changed, 191 insertions(+), 21 deletions(-)
diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index eaa03fb..8604e34 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -9,6 +9,7 @@
HAS_DEVICE_TREE := y
HAS_VIDEO := y
HAS_ARM_HDLCD := y
+HAS_NS16550 := y
CFLAGS += -fno-builtin -fno-common -Wredundant-decls
CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 60512be..b8690eb 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -1,10 +1,10 @@
/******************************************************************************
* ns16550.c
- *
+ *
* Driver for 16550-series UARTs. This driver is to be kept within Xen as
* it permits debugging of seriously-toasted machines (e.g., in situations
* where a device driver within a guest OS would be inaccessible).
- *
+ *
* Copyright (c) 2003-2005, K A Fraser
*/
@@ -25,6 +25,13 @@
#include <asm/fixmap.h>
#endif
+#ifdef CONFIG_ARM
+#include <xen/device_tree.h>
+#include <xen/errno.h>
+#include <asm/early_printk.h>
+#include <asm/device.h>
+#endif
+
/*
* Configure serial port with a string:
* <baud>[/<clock_hz>][,DPS[,<io-base>[,<irq>[,<port-bdf>[,<bridge-bdf>]]]]].
@@ -39,8 +46,11 @@ string_param("com1", opt_com1);
string_param("com2", opt_com2);
static struct ns16550 {
- int baud, clock_hz, data_bits, parity, stop_bits, fifo_size, irq;
- unsigned long io_base; /* I/O port or memory-mapped I/O address. */
+ int baud, data_bits, parity, stop_bits, fifo_size;
+ u32 clock_hz;
+ struct dt_irq irq;
+ u64 io_base; /* I/O port or memory-mapped I/O address. */
+ u64 io_size;
char __iomem *remapped_io_base; /* Remapped virtual address of MMIO. */
/* UART with IRQ line: interrupt-driven I/O. */
struct irqaction irqaction;
@@ -61,16 +71,24 @@ static struct ns16550 {
static char ns_read_reg(struct ns16550 *uart, int reg)
{
+#ifdef CONFIG_X86
if ( uart->remapped_io_base == NULL )
return inb(uart->io_base + reg);
return readb(uart->remapped_io_base + reg);
+#else
+ return readb(uart->remapped_io_base + (reg << 2));
+#endif
}
static void ns_write_reg(struct ns16550 *uart, int reg, char c)
{
+#ifdef CONFIG_X86
if ( uart->remapped_io_base == NULL )
return outb(c, uart->io_base + reg);
writeb(c, uart->remapped_io_base + reg);
+#else
+ writeb(c, uart->remapped_io_base + (reg << 2));
+#endif
}
static void ns16550_interrupt(
@@ -145,11 +163,12 @@ static int ns16550_getc(struct serial_port *port, char *pc)
return 1;
}
+#ifdef CONFIG_X86
static void pci_serial_early_init(struct ns16550 *uart)
{
if ( !uart->ps_bdf_enable || uart->io_base >= 0x10000 )
return;
-
+
if ( uart->pb_bdf_enable )
pci_conf_write16(0, uart->pb_bdf[0], uart->pb_bdf[1], uart->pb_bdf[2],
PCI_IO_BASE,
@@ -161,7 +180,13 @@ static void pci_serial_early_init(struct ns16550 *uart)
uart->io_base | PCI_BASE_ADDRESS_SPACE_IO);
pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1], uart->ps_bdf[2],
PCI_COMMAND, PCI_COMMAND_IO);
+
+}
+#else
+static void pci_serial_early_init(struct ns16550 *uart)
+{
}
+#endif
static void ns16550_setup_preirq(struct ns16550 *uart)
{
@@ -217,7 +242,9 @@ static void __init ns16550_init_preirq(struct serial_port *port)
uart->remapped_io_base = (void __iomem *)fix_to_virt(idx);
uart->remapped_io_base += uart->io_base & ~PAGE_MASK;
#else
- uart->remapped_io_base = (char *)ioremap(uart->io_base, 8);
+ uart->remapped_io_base = ioremap_attr(uart->io_base,
+ uart->io_size,
+ PAGE_HYPERVISOR_NOCACHE);
#endif
}
@@ -231,7 +258,7 @@ static void __init ns16550_init_preirq(struct serial_port *port)
static void ns16550_setup_postirq(struct ns16550 *uart)
{
- if ( uart->irq > 0 )
+ if ( uart->irq.irq > 0 )
{
/* Master interrupt enable; also keep DTR/RTS asserted. */
ns_write_reg(uart,
@@ -241,7 +268,7 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ELSI);
}
- if ( uart->irq >= 0 )
+ if ( uart->irq.irq >= 0 )
set_timer(&uart->timer, NOW() + MILLISECS(uart->timeout_ms));
}
@@ -250,7 +277,7 @@ static void __init ns16550_init_postirq(struct serial_port *port)
struct ns16550 *uart = port->uart;
int rc, bits;
- if ( uart->irq < 0 )
+ if ( uart->irq.irq < 0 )
return;
serial_async_transmit(port);
@@ -262,20 +289,26 @@ static void __init ns16550_init_postirq(struct serial_port *port)
uart->timeout_ms = max_t(
unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
- if ( uart->irq > 0 )
+ if ( uart->irq.irq > 0 )
{
uart->irqaction.handler = ns16550_interrupt;
uart->irqaction.name = "ns16550";
uart->irqaction.dev_id = port;
- if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
- printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq);
+#ifdef CONFIG_X86
+ if ( (rc = setup_irq(uart->irq.irq, &uart->irqaction)) != 0 )
+#else
+ if ( (rc = setup_dt_irq(&uart->irq, &uart->irqaction)) != 0 )
+#endif
+ printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq.irq);
}
ns16550_setup_postirq(uart);
+#ifdef CONFIG_X86
if ( uart->bar || uart->ps_bdf_enable )
pci_hide_device(uart->ps_bdf[0], PCI_DEVFN(uart->ps_bdf[1],
uart->ps_bdf[2]));
+#endif
}
static void ns16550_suspend(struct serial_port *port)
@@ -284,13 +317,16 @@ static void ns16550_suspend(struct serial_port *port)
stop_timer(&uart->timer);
+#ifdef CONFIG_X86
if ( uart->bar )
uart->cr = pci_conf_read16(0, uart->ps_bdf[0], uart->ps_bdf[1],
uart->ps_bdf[2], PCI_COMMAND);
+#endif
}
static void _ns16550_resume(struct serial_port *port)
{
+#ifdef CONFIG_X86
struct ns16550 *uart = port->uart;
if ( uart->bar )
@@ -300,6 +336,7 @@ static void _ns16550_resume(struct serial_port *port)
pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1], uart->ps_bdf[2],
PCI_COMMAND, uart->cr);
}
+#endif
ns16550_setup_preirq(port->uart);
ns16550_setup_postirq(port->uart);
@@ -370,9 +407,17 @@ static void __init ns16550_endboot(struct serial_port *port)
static int __init ns16550_irq(struct serial_port *port)
{
struct ns16550 *uart = port->uart;
- return ((uart->irq > 0) ? uart->irq : -1);
+ return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
}
+#ifdef CONFIG_ARM
+static const struct dt_irq __init *ns16550_dt_irq(struct serial_port *port)
+{
+ struct ns16550 *uart = port->uart;
+ return &uart->irq;
+}
+#endif
+
static struct uart_driver __read_mostly ns16550_driver = {
.init_preirq = ns16550_init_preirq,
.init_postirq = ns16550_init_postirq,
@@ -382,7 +427,10 @@ static struct uart_driver __read_mostly ns16550_driver = {
.tx_ready = ns16550_tx_ready,
.putc = ns16550_putc,
.getc = ns16550_getc,
- .irq = ns16550_irq
+ .irq = ns16550_irq,
+#ifdef CONFIG_ARM
+ .dt_irq_get = ns16550_dt_irq
+#endif
};
static int __init parse_parity_char(int c)
@@ -403,6 +451,7 @@ static int __init parse_parity_char(int c)
return 0;
}
+#ifdef CONFIG_X86
static void __init parse_pci_bdf(const char **conf, unsigned int bdf[3])
{
bdf[0] = simple_strtoul(*conf, conf, 16);
@@ -415,6 +464,7 @@ static void __init parse_pci_bdf(const char **conf, unsigned int bdf[3])
(*conf)++;
bdf[2] = simple_strtoul(*conf, conf, 16);
}
+#endif
static int __init check_existence(struct ns16550 *uart)
{
@@ -428,7 +478,7 @@ static int __init check_existence(struct ns16550 *uart)
return 1;
pci_serial_early_init(uart);
-
+
/*
* Do a simple existence test first; if we fail this,
* there's no point trying anything else.
@@ -445,7 +495,7 @@ static int __init check_existence(struct ns16550 *uart)
scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
ns_write_reg(uart, UART_IER, scratch);
if ( (scratch2 != 0) || (scratch3 != 0x0F) )
- return 0;
+ return 0;
/*
* Check to see if a UART is really there.
@@ -456,6 +506,7 @@ static int __init check_existence(struct ns16550 *uart)
return (status == 0x90);
}
+#ifdef CONFIG_X86
static int
pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
{
@@ -507,7 +558,7 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
uart->bar = bar;
uart->bar_idx = bar_idx;
uart->io_base = bar & ~PCI_BASE_ADDRESS_SPACE_IO;
- uart->irq = pci_conf_read8(0, b, d, f, PCI_INTERRUPT_PIN) ?
+ uart->irq.irq = pci_conf_read8(0, b, d, f, PCI_INTERRUPT_PIN) ?
pci_conf_read8(0, b, d, f, PCI_INTERRUPT_LINE) : 0;
return 0;
@@ -519,11 +570,12 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
return -1;
uart->io_base = 0x3f8;
- uart->irq = 0;
+ uart->irq.irq = 0;
uart->clock_hz = UART_CLOCK_HZ;
return 0;
}
+#endif
#define PARSE_ERR(_f, _a...) \
do { \
@@ -534,6 +586,7 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
static void __init ns16550_parse_port_config(
struct ns16550 *uart, const char *conf)
{
+#ifdef CONFIG_X86
int baud;
/* No user-specified configuration? */
@@ -589,7 +642,7 @@ static void __init ns16550_parse_port_config(
}
if ( *conf == ',' && *++conf != ',' )
- uart->irq = simple_strtol(conf, &conf, 10);
+ uart->irq.irq = simple_strtol(conf, &conf, 10);
if ( *conf == ',' && *++conf != ',' )
{
@@ -604,6 +657,8 @@ static void __init ns16550_parse_port_config(
}
config_parsed:
+#endif
+
/* Sanity checks. */
if ( (uart->baud != BAUD_AUTO) &&
((uart->baud < 1200) || (uart->baud > 115200)) )
@@ -633,18 +688,80 @@ void __init ns16550_init(int index, struct ns16550_defaults *defaults)
uart->baud = (defaults->baud ? :
console_has((index == 0) ? "com1" : "com2")
? BAUD_AUTO : 0);
+#ifdef CONFIG_ARM
+ uart->clock_hz = defaults->clock_hz;
+#else
uart->clock_hz = UART_CLOCK_HZ;
+#endif
uart->data_bits = defaults->data_bits;
uart->parity = parse_parity_char(defaults->parity);
uart->stop_bits = defaults->stop_bits;
uart->irq = defaults->irq;
uart->io_base = defaults->io_base;
+#ifdef CONFIG_ARM
+ uart->io_size = defaults->io_size;
+#endif
+
/* Default is no transmit FIFO. */
uart->fifo_size = 1;
ns16550_parse_port_config(uart, (index == 0) ? opt_com1 : opt_com2);
+
}
+#ifdef CONFIG_ARM
+
+static int __init ns16550_dt_init(struct dt_device_node *dev,
+ const void *data)
+{
+ struct ns16550_defaults uart;
+ int res;
+ const __be32 *clkspec;
+
+ uart.baud = 115200;
+ uart.data_bits = 8;
+ uart.parity = 'n';
+ uart.stop_bits = 1;
+
+ res = dt_device_get_address(dev, 0, &uart.io_base, &uart.io_size);
+ if (res) {
+ early_printk("ns16550: Unable to retrieve the base"
+ " address of the UART\n");
+ return res;
+ }
+
+ res = dt_device_get_irq(dev, 0, &uart.irq);
+ if (res) {
+ early_printk("ns16550: Unable to retrieve the IRQ\n");
+ return res;
+ }
+
+ clkspec = dt_get_property(dev, "clock-frequency", NULL);
+ if (clkspec == NULL) {
+ early_printk("ns16550: Unable to retrieve the clock frequency\n");
+ return -EINVAL;
+ }
+ uart.clock_hz = be32_to_cpup(clkspec);
+
+ ns16550_init(0, &uart);
+ dt_device_set_used_by(dev, DOMID_XEN);
+
+ return 0;
+}
+
+static const char const *ns16550_dt_compat[] __initdata =
+{
+ "ti,omap4-uart",
+ NULL
+};
+
+DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
+ .compatible = ns16550_dt_compat,
+ .init = ns16550_dt_init,
+DT_DEVICE_END
+
+#endif /* CONFIG_ARM */
+
/*
* Local variables:
* mode: C
diff --git a/xen/include/asm-arm/io.h b/xen/include/asm-arm/io.h
index aea5233..33674bc 100644
--- a/xen/include/asm-arm/io.h
+++ b/xen/include/asm-arm/io.h
@@ -1,6 +1,55 @@
#ifndef _ASM_IO_H
#define _ASM_IO_H
+static inline void __raw_writeb(u8 val, volatile void __iomem *addr)
+{
+ asm volatile("strb %1, %0"
+ : "+Qo" (*(volatile u8 __force *)addr)
+ : "r" (val));
+}
+
+static inline void __raw_writel(u32 val, volatile void __iomem *addr)
+{
+ asm volatile("str %1, %0"
+ : "+Qo" (*(volatile u32 __force *)addr)
+ : "r" (val));
+}
+
+static inline u8 __raw_readb(const volatile void __iomem *addr)
+{
+ u8 val;
+ asm volatile("ldrb %1, %0"
+ : "+Qo" (*(volatile u8 __force *)addr),
+ "=r" (val));
+ return val;
+}
+
+static inline u32 __raw_readl(const volatile void __iomem *addr)
+{
+ u32 val;
+ asm volatile("ldr %1, %0"
+ : "+Qo" (*(volatile u32 __force *)addr),
+ "=r" (val));
+ return val;
+}
+
+#define __iormb() rmb()
+#define __iowmb() wmb()
+
+#define readb_relaxed(c) ({ u8 __r = __raw_readb(c); __r; })
+#define readl_relaxed(c) ({ u32 __r = le32_to_cpu((__force __le32) \
+ __raw_readl(c)); __r; })
+
+#define writeb_relaxed(v,c) __raw_writeb(v,c)
+#define writel_relaxed(v,c) __raw_writel((__force u32) cpu_to_le32(v),c)
+
+#define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(); __v; })
+#define readl(c) ({ u32 __v = readl_relaxed(c); __iormb(); __v; })
+
+#define writeb(v,c) ({ __iowmb(); writeb_relaxed(v,c); })
+#define writel(v,c) ({ __iowmb(); writel_relaxed(v,c); })
+
+
#endif
/*
* Local variables:
diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
index 9caf776..09ca855 100644
--- a/xen/include/xen/serial.h
+++ b/xen/include/xen/serial.h
@@ -11,6 +11,7 @@
#include <xen/init.h>
#include <xen/spinlock.h>
+#include <xen/device_tree.h>
struct cpu_user_regs;
@@ -151,8 +152,10 @@ struct ns16550_defaults {
int data_bits; /* default data bits (5, 6, 7 or 8) */
int parity; /* default parity (n, o, e, m or s) */
int stop_bits; /* default stop bits (1 or 2) */
- int irq; /* default irq */
- unsigned long io_base; /* default io_base address */
+ struct dt_irq irq; /* default irq */
+ u64 io_base; /* default io_base address */
+ u64 io_size;
+ u32 clock_hz; /* UART clock rate */
};
void ns16550_init(int index, struct ns16550_defaults *defaults);
void ehci_dbgp_init(void);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-19 12:57 ` [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250 Chen Baozi
@ 2013-07-19 14:15 ` Ian Campbell
2013-07-19 15:18 ` Chen Baozi
0 siblings, 1 reply; 27+ messages in thread
From: Ian Campbell @ 2013-07-19 14:15 UTC (permalink / raw)
To: Chen Baozi; +Cc: Keir Fraser, xen-devel
On Fri, 2013-07-19 at 20:57 +0800, Chen Baozi wrote:
> Previous bits setting would cause generating THRE interrupts, which cannot
> be cleared and make system loop in gic_interrupt endlessly. Set 'received
> data available' and 'receiver line status' bits of IER when enabling
> interrupts as what Linux 8250 driver does.
We do actually want THRE interrupts though, don't we? Otherwise how do
we know when we can send more stuff?
Are you using console_sync? It might mask any issue arising from not
getting these interrupts (or maybe even cause your initial issue?)
The LSI register is only of use if we care about RTS/CTS or something. I
don't know if we do, but I'd be surprised if just asking for the
interrupt was sufficient (i.e. we'd need to do something with the irq)
How did you configure this interrupt line, level or edge trigger? I'd
have expected it to need to be edge, and the issue sounds a bit like it
might be level.
Ian.
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> ---
> xen/drivers/char/ns16550.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index 9248330..60512be 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -237,8 +237,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
> ns_write_reg(uart,
> UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
>
> - /* Enable receive and transmit interrupts. */
> - ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ETHREI);
> + /* Enable receive and line status interrupts. */
> + ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ELSI);
> }
>
> if ( uart->irq >= 0 )
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-19 14:15 ` Ian Campbell
@ 2013-07-19 15:18 ` Chen Baozi
2013-07-22 12:59 ` Chen Baozi
0 siblings, 1 reply; 27+ messages in thread
From: Chen Baozi @ 2013-07-19 15:18 UTC (permalink / raw)
To: Ian Campbell; +Cc: Keir Fraser, xen-devel
On Fri, Jul 19, 2013 at 10:15 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>
> On Fri, 2013-07-19 at 20:57 +0800, Chen Baozi wrote:
> > Previous bits setting would cause generating THRE interrupts, which cannot
> > be cleared and make system loop in gic_interrupt endlessly. Set 'received
> > data available' and 'receiver line status' bits of IER when enabling
> > interrupts as what Linux 8250 driver does.
>
> We do actually want THRE interrupts though, don't we? Otherwise how do
> we know when we can send more stuff?
I'm not very sure about it. In Linux 8250 driver THRE is not set at this stage.
And following it does work for OMAP5. I'll try to dig it next to see
whether this
fix is reasonable.
>
> Are you using console_sync? It might mask any issue arising from not
> getting these interrupts (or maybe even cause your initial issue?)
Yes. I have tried to shut this down to test whether it is the cause of my
issue. But it seems not.
>
> The LSI register is only of use if we care about RTS/CTS or something. I
> don't know if we do, but I'd be surprised if just asking for the
> interrupt was sufficient (i.e. we'd need to do something with the irq)
>
> How did you configure this interrupt line, level or edge trigger? I'd
> have expected it to need to be edge, and the issue sounds a bit like it
> might be level.
It is high-level read from the DTS. I've also changed to it to edge trigger
manually and every other possibilities when debugging. And it seems
edge trigger doesn't help.
Anyway, some research must be done for this patch and my fixes to the
issue next.
Thanks.
Baozi
>
> Ian.
>
> > Signed-off-by: Chen Baozi <baozich@gmail.com>
> > ---
> > xen/drivers/char/ns16550.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> > index 9248330..60512be 100644
> > --- a/xen/drivers/char/ns16550.c
> > +++ b/xen/drivers/char/ns16550.c
> > @@ -237,8 +237,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
> > ns_write_reg(uart,
> > UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
> >
> > - /* Enable receive and transmit interrupts. */
> > - ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ETHREI);
> > + /* Enable receive and line status interrupts. */
> > + ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ELSI);
> > }
> >
> > if ( uart->irq >= 0 )
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] xen: extract register definitions from ns16550 into a separated header
2013-07-19 12:57 ` [PATCH 1/4] xen: extract register definitions from ns16550 into a separated header Chen Baozi
@ 2013-07-19 15:35 ` Julien Grall
2013-07-19 15:37 ` Chen Baozi
0 siblings, 1 reply; 27+ messages in thread
From: Julien Grall @ 2013-07-19 15:35 UTC (permalink / raw)
To: Chen Baozi; +Cc: xen-devel, Keir Fraser, Ian Campbell, bjzhang
On 07/19/2013 01:57 PM, Chen Baozi wrote:
> Since both UART driver codes on Allwinner A31, OMAP5 and x86 would use
> these definitions, we refactor the codes into a separated header to avoid
> unnecessary duplication.
>
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> ---
> xen/drivers/char/ns16550.c | 155 +++++++++++++--------------------------------
> xen/include/xen/8250.h | 104 ++++++++++++++++++++++++++++++
> 2 files changed, 148 insertions(+), 111 deletions(-)
> create mode 100644 xen/include/xen/8250.h
>
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index e0c87bb..9248330 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -19,6 +19,7 @@
> #include <xen/iocap.h>
> #include <xen/pci.h>
> #include <xen/pci_regs.h>
> +#include <xen/8250.h>
> #include <asm/io.h>
> #ifdef CONFIG_X86
> #include <asm/fixmap.h>
> @@ -58,76 +59,6 @@ static struct ns16550 {
> u8 bar_idx;
> } ns16550_com[2] = { { 0 } };
>
> -/* Register offsets */
> -#define RBR 0x00 /* receive buffer */
> -#define THR 0x00 /* transmit holding */
> -#define IER 0x01 /* interrupt enable */
> -#define IIR 0x02 /* interrupt identity */
> -#define FCR 0x02 /* FIFO control */
> -#define LCR 0x03 /* line control */
> -#define MCR 0x04 /* Modem control */
> -#define LSR 0x05 /* line status */
> -#define MSR 0x06 /* Modem status */
> -#define DLL 0x00 /* divisor latch (ls) (DLAB=1) */
> -#define DLM 0x01 /* divisor latch (ms) (DLAB=1) */
> -
> -/* Interrupt Enable Register */
> -#define IER_ERDAI 0x01 /* rx data recv'd */
> -#define IER_ETHREI 0x02 /* tx reg. empty */
> -#define IER_ELSI 0x04 /* rx line status */
> -#define IER_EMSI 0x08 /* MODEM status */
> -
> -/* Interrupt Identification Register */
> -#define IIR_NOINT 0x01 /* no interrupt pending */
> -#define IIR_IMASK 0x06 /* interrupt identity: */
> -#define IIR_LSI 0x06 /* - rx line status */
> -#define IIR_RDAI 0x04 /* - rx data recv'd */
> -#define IIR_THREI 0x02 /* - tx reg. empty */
> -#define IIR_MSI 0x00 /* - MODEM status */
> -
> -/* FIFO Control Register */
> -#define FCR_ENABLE 0x01 /* enable FIFO */
> -#define FCR_CLRX 0x02 /* clear Rx FIFO */
> -#define FCR_CLTX 0x04 /* clear Tx FIFO */
> -#define FCR_DMA 0x10 /* enter DMA mode */
> -#define FCR_TRG1 0x00 /* Rx FIFO trig lev 1 */
> -#define FCR_TRG4 0x40 /* Rx FIFO trig lev 4 */
> -#define FCR_TRG8 0x80 /* Rx FIFO trig lev 8 */
> -#define FCR_TRG14 0xc0 /* Rx FIFO trig lev 14 */
> -
> -/* Line Control Register */
> -#define LCR_DLAB 0x80 /* Divisor Latch Access */
> -
> -/* Modem Control Register */
> -#define MCR_DTR 0x01 /* Data Terminal Ready */
> -#define MCR_RTS 0x02 /* Request to Send */
> -#define MCR_OUT2 0x08 /* OUT2: interrupt mask */
> -#define MCR_LOOP 0x10 /* Enable loopback test mode */
> -
> -/* Line Status Register */
> -#define LSR_DR 0x01 /* Data ready */
> -#define LSR_OE 0x02 /* Overrun */
> -#define LSR_PE 0x04 /* Parity error */
> -#define LSR_FE 0x08 /* Framing error */
> -#define LSR_BI 0x10 /* Break */
> -#define LSR_THRE 0x20 /* Xmit hold reg empty */
> -#define LSR_TEMT 0x40 /* Xmitter empty */
> -#define LSR_ERR 0x80 /* Error */
> -
> -/* These parity settings can be ORed directly into the LCR. */
> -#define PARITY_NONE (0<<3)
> -#define PARITY_ODD (1<<3)
> -#define PARITY_EVEN (3<<3)
> -#define PARITY_MARK (5<<3)
> -#define PARITY_SPACE (7<<3)
> -
> -/* Frequency of external clock source. This definition assumes PC platform. */
> -#define UART_CLOCK_HZ 1843200
> -
> -/* Resume retry settings */
> -#define RESUME_DELAY MILLISECS(10)
> -#define RESUME_RETRIES 100
> -
> static char ns_read_reg(struct ns16550 *uart, int reg)
> {
> if ( uart->remapped_io_base == NULL )
> @@ -150,12 +81,12 @@ static void ns16550_interrupt(
>
> uart->intr_works = 1;
>
> - while ( !(ns_read_reg(uart, IIR) & IIR_NOINT) )
> + while ( !(ns_read_reg(uart, UART_IIR) & UART_IIR_NOINT) )
> {
> - char lsr = ns_read_reg(uart, LSR);
> - if ( lsr & LSR_THRE )
> + char lsr = ns_read_reg(uart, UART_LSR);
> + if ( lsr & UART_LSR_THRE )
> serial_tx_interrupt(port, regs);
> - if ( lsr & LSR_DR )
> + if ( lsr & UART_LSR_DR )
> serial_rx_interrupt(port, regs);
> }
> }
> @@ -171,10 +102,10 @@ static void __ns16550_poll(struct cpu_user_regs *regs)
> if ( uart->intr_works )
> return; /* Interrupts work - no more polling */
>
> - while ( ns_read_reg(uart, LSR) & LSR_DR )
> + while ( ns_read_reg(uart, UART_LSR) & UART_LSR_DR )
> serial_rx_interrupt(port, regs);
>
> - if ( ns_read_reg(uart, LSR) & LSR_THRE )
> + if ( ns_read_reg(uart, UART_LSR) & UART_LSR_THRE )
> serial_tx_interrupt(port, regs);
>
> set_timer(&uart->timer, NOW() + MILLISECS(uart->timeout_ms));
> @@ -194,23 +125,23 @@ static unsigned int ns16550_tx_ready(struct serial_port *port)
> {
> struct ns16550 *uart = port->uart;
>
> - return ns_read_reg(uart, LSR) & LSR_THRE ? uart->fifo_size : 0;
> + return ns_read_reg(uart, UART_LSR) & UART_LSR_THRE ? uart->fifo_size : 0;
> }
>
> static void ns16550_putc(struct serial_port *port, char c)
> {
> struct ns16550 *uart = port->uart;
> - ns_write_reg(uart, THR, c);
> + ns_write_reg(uart, UART_THR, c);
> }
>
> static int ns16550_getc(struct serial_port *port, char *pc)
> {
> struct ns16550 *uart = port->uart;
>
> - if ( !(ns_read_reg(uart, LSR) & LSR_DR) )
> + if ( !(ns_read_reg(uart, UART_LSR) & UART_LSR_DR) )
> return 0;
>
> - *pc = ns_read_reg(uart, RBR);
> + *pc = ns_read_reg(uart, UART_RBR);
> return 1;
> }
>
> @@ -244,31 +175,32 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
> lcr = (uart->data_bits - 5) | ((uart->stop_bits - 1) << 2) | uart->parity;
>
> /* No interrupts. */
> - ns_write_reg(uart, IER, 0);
> + ns_write_reg(uart, UART_IER, 0);
>
> /* Line control and baud-rate generator. */
> - ns_write_reg(uart, LCR, lcr | LCR_DLAB);
> + ns_write_reg(uart, UART_LCR, lcr | UART_LCR_DLAB);
> if ( uart->baud != BAUD_AUTO )
> {
> /* Baud rate specified: program it into the divisor latch. */
> divisor = uart->clock_hz / (uart->baud << 4);
> - ns_write_reg(uart, DLL, (char)divisor);
> - ns_write_reg(uart, DLM, (char)(divisor >> 8));
> + ns_write_reg(uart, UART_DLL, (char)divisor);
> + ns_write_reg(uart, UART_DLM, (char)(divisor >> 8));
> }
> else
> {
> /* Baud rate already set: read it out from the divisor latch. */
> - divisor = ns_read_reg(uart, DLL);
> - divisor |= ns_read_reg(uart, DLM) << 8;
> + divisor = ns_read_reg(uart, UART_DLL);
> + divisor |= ns_read_reg(uart, UART_DLM) << 8;
> uart->baud = uart->clock_hz / (divisor << 4);
> }
> - ns_write_reg(uart, LCR, lcr);
> + ns_write_reg(uart, UART_LCR, lcr);
>
> /* No flow ctrl: DTR and RTS are both wedged high to keep remote happy. */
> - ns_write_reg(uart, MCR, MCR_DTR | MCR_RTS);
> + ns_write_reg(uart, UART_MCR, UART_MCR_DTR | UART_MCR_RTS);
>
> /* Enable and clear the FIFOs. Set a large trigger threshold. */
> - ns_write_reg(uart, FCR, FCR_ENABLE | FCR_CLRX | FCR_CLTX | FCR_TRG14);
> + ns_write_reg(uart, UART_FCR,
> + UART_FCR_ENABLE | UART_FCR_CLRX | UART_FCR_CLTX | UART_FCR_TRG14);
> }
>
> static void __init ns16550_init_preirq(struct serial_port *port)
> @@ -292,8 +224,8 @@ static void __init ns16550_init_preirq(struct serial_port *port)
> ns16550_setup_preirq(uart);
>
> /* Check this really is a 16550+. Otherwise we have no FIFOs. */
> - if ( ((ns_read_reg(uart, IIR) & 0xc0) == 0xc0) &&
> - ((ns_read_reg(uart, FCR) & FCR_TRG14) == FCR_TRG14) )
> + if ( ((ns_read_reg(uart, UART_IIR) & 0xc0) == 0xc0) &&
> + ((ns_read_reg(uart, UART_FCR) & UART_FCR_TRG14) == UART_FCR_TRG14) )
> uart->fifo_size = 16;
> }
>
> @@ -302,10 +234,11 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
> if ( uart->irq > 0 )
> {
> /* Master interrupt enable; also keep DTR/RTS asserted. */
> - ns_write_reg(uart, MCR, MCR_OUT2 | MCR_DTR | MCR_RTS);
> + ns_write_reg(uart,
> + UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
>
> /* Enable receive and transmit interrupts. */
> - ns_write_reg(uart, IER, IER_ERDAI | IER_ETHREI);
> + ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ETHREI);
> }
>
> if ( uart->irq >= 0 )
> @@ -374,11 +307,11 @@ static void _ns16550_resume(struct serial_port *port)
>
> static int ns16550_ioport_invalid(struct ns16550 *uart)
> {
> - return ((((unsigned char)ns_read_reg(uart, LSR)) == 0xff) &&
> - (((unsigned char)ns_read_reg(uart, MCR)) == 0xff) &&
> - (((unsigned char)ns_read_reg(uart, IER)) == 0xff) &&
> - (((unsigned char)ns_read_reg(uart, IIR)) == 0xff) &&
> - (((unsigned char)ns_read_reg(uart, LCR)) == 0xff));
> + return ((((unsigned char)ns_read_reg(uart, UART_LSR)) == 0xff) &&
> + (((unsigned char)ns_read_reg(uart, UART_MCR)) == 0xff) &&
> + (((unsigned char)ns_read_reg(uart, UART_IER)) == 0xff) &&
> + (((unsigned char)ns_read_reg(uart, UART_IIR)) == 0xff) &&
> + (((unsigned char)ns_read_reg(uart, UART_LCR)) == 0xff));
> }
>
> static int delayed_resume_tries;
> @@ -457,15 +390,15 @@ static int __init parse_parity_char(int c)
> switch ( c )
> {
> case 'n':
> - return PARITY_NONE;
> + return UART_PARITY_NONE;
> case 'o':
> - return PARITY_ODD;
> + return UART_PARITY_ODD;
> case 'e':
> - return PARITY_EVEN;
> + return UART_PARITY_EVEN;
> case 'm':
> - return PARITY_MARK;
> + return UART_PARITY_MARK;
> case 's':
> - return PARITY_SPACE;
> + return UART_PARITY_SPACE;
> }
> return 0;
> }
> @@ -500,17 +433,17 @@ static int __init check_existence(struct ns16550 *uart)
> * Do a simple existence test first; if we fail this,
> * there's no point trying anything else.
> */
> - scratch = ns_read_reg(uart, IER);
> - ns_write_reg(uart, IER, 0);
> + scratch = ns_read_reg(uart, UART_IER);
> + ns_write_reg(uart, UART_IER, 0);
>
> /*
> * Mask out IER[7:4] bits for test as some UARTs (e.g. TL
> * 16C754B) allow only to modify them if an EFR bit is set.
> */
> - scratch2 = ns_read_reg(uart, IER) & 0x0f;
> - ns_write_reg(uart, IER, 0x0F);
> - scratch3 = ns_read_reg(uart, IER) & 0x0f;
> - ns_write_reg(uart, IER, scratch);
> + scratch2 = ns_read_reg(uart, UART_IER) & 0x0f;
> + ns_write_reg(uart,UART_IER, 0x0F);
> + scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
> + ns_write_reg(uart, UART_IER, scratch);
> if ( (scratch2 != 0) || (scratch3 != 0x0F) )
> return 0;
>
> @@ -518,8 +451,8 @@ static int __init check_existence(struct ns16550 *uart)
> * Check to see if a UART is really there.
> * Use loopback test mode.
> */
> - ns_write_reg(uart, MCR, MCR_LOOP | 0x0A);
> - status = ns_read_reg(uart, MSR) & 0xF0;
> + ns_write_reg(uart, UART_MCR, UART_MCR_LOOP | 0x0A);
> + status = ns_read_reg(uart, UART_MSR) & 0xF0;
> return (status == 0x90);
> }
>
> diff --git a/xen/include/xen/8250.h b/xen/include/xen/8250.h
> new file mode 100644
> index 0000000..84f35bc
> --- /dev/null
> +++ b/xen/include/xen/8250.h
When you look to the name, it's not clear what this file is. Could you
rename it to 8250-uart.h?
> @@ -0,0 +1,104 @@
> +/*
> + * xen/include/xen/8250.h
> + *
> + * This header is extracted from driver/char/ns16550.c
> + *
> + * Common constant definition between early printk and the UART driver
> + * for the 16550-series UART
> + *
> + * Copyright (c) 2003-2005, K A Fraser
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __XEN_NS16550_H__
> +#define __XEN_NS16550_H__
> +
> +/* Register offsets */
> +#define UART_RBR 0x00 /* receive buffer */
> +#define UART_THR 0x00 /* transmit holding */
> +#define UART_IER 0x01 /* interrupt enable */
> +#define UART_IIR 0x02 /* interrupt identity */
> +#define UART_FCR 0x02 /* FIFO control */
> +#define UART_LCR 0x03 /* line control */
> +#define UART_MCR 0x04 /* Modem control */
> +#define UART_LSR 0x05 /* line status */
> +#define UART_MSR 0x06 /* Modem status */
> +#define UART_DLL 0x00 /* divisor latch (ls) (DLAB=1) */
> +#define UART_DLM 0x01 /* divisor latch (ms) (DLAB=1) */
> +
> +/* Interrupt Enable Register */
> +#define UART_IER_ERDAI 0x01 /* rx data recv'd */
> +#define UART_IER_ETHREI 0x02 /* tx reg. empty */
> +#define UART_IER_ELSI 0x04 /* rx line status */
> +#define UART_IER_EMSI 0x08 /* MODEM status */
> +
> +/* Interrupt Identificatiegister */
> +#define UART_IIR_NOINT 0x01 /* no interrupt pending */
> +#define UART_IIR_IMA 0x06 /* interrupt identity: */
> +#define UART_IIR_LSI 0x06 /* - rx line status */
> +#define UART_IIR_RDA 0x04 /* - rx data recv'd */
> +#define UART_IIR_THR 0x02 /* - tx reg. empty */
> +#define UART_IIR_MSI 0x00 /* - MODEM status */
> +
> +/* FIFO Control Register */
> +#define UART_FCR_ENABLE 0x01 /* enable FIFO */
> +#define UART_FCR_CLRX 0x02 /* clear Rx FIFO */
> +#define UART_FCR_CLTX 0x04 /* clear Tx FIFO */
> +#define UART_FCR_DMA 0x10 /* enter DMA mode */
> +#define UART_FCR_TRG1 0x00 /* Rx FIFO trig lev 1 */
> +#define UART_FCR_TRG4 0x40 /* Rx FIFO trig lev 4 */
> +#define UART_FCR_TRG8 0x80 /* Rx FIFO trig lev 8 */
> +#define UART_FCR_TRG14 0xc0 /* Rx FIFO trig lev 14 */
> +
> +/* Line Control Register */
> +#define UART_LCR_DLAB 0x80 /* Divisor Latch Access */
> +
> +/* Modem Control Register */
> +#define UART_MCR_DTR 0x01 /* Data Terminal Ready */
> +#define UART_MCR_RTS 0x02 /* Request to Send */
> +#define UART_MCR_OUT2 0x08 /* OUT2: interrupt mask */
> +#define UART_MCR_LOOP 0x10 /* Enable loopback test mode */
> +
> +/* Line Status Register */
> +#define UART_LSR_DR 0x01 /* Data ready */
> +#define UART_LSR_OE 0x02 /* Overrun */
> +#define UART_LSR_PE 0x04 /* Parity error */
> +#define UART_LSR_FE 0x08 /* Framing error */
> +#define UART_LSR_BI 0x10 /* Break */
> +#define UART_LSR_THRE 0x20 /* Xmit hold reg empty */
> +#define UART_LSR_TEMT 0x40 /* Xmitter empty */
> +#define UART_LSR_ERR 0x80 /* Error */
> +
> +/* These parity settings can be ORed directly into the LCR. */
> +#define UART_PARITY_NONE (0<<3)
> +#define UART_PARITY_ODD (1<<3)
> +#define UART_PARITY_EVEN (3<<3)
> +#define UART_PARITY_MARK (5<<3)
> +#define UART_PARITY_SPACE (7<<3)
> +
> +/* Frequency of external clock source. This definition assumes PC platform. */
> +#define UART_CLOCK_HZ 1843200
> +
> +/* Resume retry settings */
> +#define RESUME_DELAY MILLISECS(10)
> +#define RESUME_RETRIES 100
> +
> +#endif /* __XEN_NS16550_H__ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
>
--
Julien
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] xen: extract register definitions from ns16550 into a separated header
2013-07-19 15:35 ` Julien Grall
@ 2013-07-19 15:37 ` Chen Baozi
2013-07-19 15:43 ` Julien Grall
0 siblings, 1 reply; 27+ messages in thread
From: Chen Baozi @ 2013-07-19 15:37 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, Keir Fraser, Ian Campbell, bjzhang
On Fri, Jul 19, 2013 at 11:35 PM, Julien Grall <julien.grall@linaro.org> wrote:
> On 07/19/2013 01:57 PM, Chen Baozi wrote:
>> Since both UART driver codes on Allwinner A31, OMAP5 and x86 would use
>> these definitions, we refactor the codes into a separated header to avoid
>> unnecessary duplication.
>>
>> Signed-off-by: Chen Baozi <baozich@gmail.com>
>> ---
>> xen/drivers/char/ns16550.c | 155 +++++++++++++--------------------------------
>> xen/include/xen/8250.h | 104 ++++++++++++++++++++++++++++++
>> 2 files changed, 148 insertions(+), 111 deletions(-)
>> create mode 100644 xen/include/xen/8250.h
>>
>> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
>> index e0c87bb..9248330 100644
>> --- a/xen/drivers/char/ns16550.c
>> +++ b/xen/drivers/char/ns16550.c
>> @@ -19,6 +19,7 @@
>> #include <xen/iocap.h>
>> #include <xen/pci.h>
>> #include <xen/pci_regs.h>
>> +#include <xen/8250.h>
>> #include <asm/io.h>
>> #ifdef CONFIG_X86
>> #include <asm/fixmap.h>
>> @@ -58,76 +59,6 @@ static struct ns16550 {
>> u8 bar_idx;
>> } ns16550_com[2] = { { 0 } };
>>
>> -/* Register offsets */
>> -#define RBR 0x00 /* receive buffer */
>> -#define THR 0x00 /* transmit holding */
>> -#define IER 0x01 /* interrupt enable */
>> -#define IIR 0x02 /* interrupt identity */
>> -#define FCR 0x02 /* FIFO control */
>> -#define LCR 0x03 /* line control */
>> -#define MCR 0x04 /* Modem control */
>> -#define LSR 0x05 /* line status */
>> -#define MSR 0x06 /* Modem status */
>> -#define DLL 0x00 /* divisor latch (ls) (DLAB=1) */
>> -#define DLM 0x01 /* divisor latch (ms) (DLAB=1) */
>> -
>> -/* Interrupt Enable Register */
>> -#define IER_ERDAI 0x01 /* rx data recv'd */
>> -#define IER_ETHREI 0x02 /* tx reg. empty */
>> -#define IER_ELSI 0x04 /* rx line status */
>> -#define IER_EMSI 0x08 /* MODEM status */
>> -
>> -/* Interrupt Identification Register */
>> -#define IIR_NOINT 0x01 /* no interrupt pending */
>> -#define IIR_IMASK 0x06 /* interrupt identity: */
>> -#define IIR_LSI 0x06 /* - rx line status */
>> -#define IIR_RDAI 0x04 /* - rx data recv'd */
>> -#define IIR_THREI 0x02 /* - tx reg. empty */
>> -#define IIR_MSI 0x00 /* - MODEM status */
>> -
>> -/* FIFO Control Register */
>> -#define FCR_ENABLE 0x01 /* enable FIFO */
>> -#define FCR_CLRX 0x02 /* clear Rx FIFO */
>> -#define FCR_CLTX 0x04 /* clear Tx FIFO */
>> -#define FCR_DMA 0x10 /* enter DMA mode */
>> -#define FCR_TRG1 0x00 /* Rx FIFO trig lev 1 */
>> -#define FCR_TRG4 0x40 /* Rx FIFO trig lev 4 */
>> -#define FCR_TRG8 0x80 /* Rx FIFO trig lev 8 */
>> -#define FCR_TRG14 0xc0 /* Rx FIFO trig lev 14 */
>> -
>> -/* Line Control Register */
>> -#define LCR_DLAB 0x80 /* Divisor Latch Access */
>> -
>> -/* Modem Control Register */
>> -#define MCR_DTR 0x01 /* Data Terminal Ready */
>> -#define MCR_RTS 0x02 /* Request to Send */
>> -#define MCR_OUT2 0x08 /* OUT2: interrupt mask */
>> -#define MCR_LOOP 0x10 /* Enable loopback test mode */
>> -
>> -/* Line Status Register */
>> -#define LSR_DR 0x01 /* Data ready */
>> -#define LSR_OE 0x02 /* Overrun */
>> -#define LSR_PE 0x04 /* Parity error */
>> -#define LSR_FE 0x08 /* Framing error */
>> -#define LSR_BI 0x10 /* Break */
>> -#define LSR_THRE 0x20 /* Xmit hold reg empty */
>> -#define LSR_TEMT 0x40 /* Xmitter empty */
>> -#define LSR_ERR 0x80 /* Error */
>> -
>> -/* These parity settings can be ORed directly into the LCR. */
>> -#define PARITY_NONE (0<<3)
>> -#define PARITY_ODD (1<<3)
>> -#define PARITY_EVEN (3<<3)
>> -#define PARITY_MARK (5<<3)
>> -#define PARITY_SPACE (7<<3)
>> -
>> -/* Frequency of external clock source. This definition assumes PC platform. */
>> -#define UART_CLOCK_HZ 1843200
>> -
>> -/* Resume retry settings */
>> -#define RESUME_DELAY MILLISECS(10)
>> -#define RESUME_RETRIES 100
>> -
>> static char ns_read_reg(struct ns16550 *uart, int reg)
>> {
>> if ( uart->remapped_io_base == NULL )
>> @@ -150,12 +81,12 @@ static void ns16550_interrupt(
>>
>> uart->intr_works = 1;
>>
>> - while ( !(ns_read_reg(uart, IIR) & IIR_NOINT) )
>> + while ( !(ns_read_reg(uart, UART_IIR) & UART_IIR_NOINT) )
>> {
>> - char lsr = ns_read_reg(uart, LSR);
>> - if ( lsr & LSR_THRE )
>> + char lsr = ns_read_reg(uart, UART_LSR);
>> + if ( lsr & UART_LSR_THRE )
>> serial_tx_interrupt(port, regs);
>> - if ( lsr & LSR_DR )
>> + if ( lsr & UART_LSR_DR )
>> serial_rx_interrupt(port, regs);
>> }
>> }
>> @@ -171,10 +102,10 @@ static void __ns16550_poll(struct cpu_user_regs *regs)
>> if ( uart->intr_works )
>> return; /* Interrupts work - no more polling */
>>
>> - while ( ns_read_reg(uart, LSR) & LSR_DR )
>> + while ( ns_read_reg(uart, UART_LSR) & UART_LSR_DR )
>> serial_rx_interrupt(port, regs);
>>
>> - if ( ns_read_reg(uart, LSR) & LSR_THRE )
>> + if ( ns_read_reg(uart, UART_LSR) & UART_LSR_THRE )
>> serial_tx_interrupt(port, regs);
>>
>> set_timer(&uart->timer, NOW() + MILLISECS(uart->timeout_ms));
>> @@ -194,23 +125,23 @@ static unsigned int ns16550_tx_ready(struct serial_port *port)
>> {
>> struct ns16550 *uart = port->uart;
>>
>> - return ns_read_reg(uart, LSR) & LSR_THRE ? uart->fifo_size : 0;
>> + return ns_read_reg(uart, UART_LSR) & UART_LSR_THRE ? uart->fifo_size : 0;
>> }
>>
>> static void ns16550_putc(struct serial_port *port, char c)
>> {
>> struct ns16550 *uart = port->uart;
>> - ns_write_reg(uart, THR, c);
>> + ns_write_reg(uart, UART_THR, c);
>> }
>>
>> static int ns16550_getc(struct serial_port *port, char *pc)
>> {
>> struct ns16550 *uart = port->uart;
>>
>> - if ( !(ns_read_reg(uart, LSR) & LSR_DR) )
>> + if ( !(ns_read_reg(uart, UART_LSR) & UART_LSR_DR) )
>> return 0;
>>
>> - *pc = ns_read_reg(uart, RBR);
>> + *pc = ns_read_reg(uart, UART_RBR);
>> return 1;
>> }
>>
>> @@ -244,31 +175,32 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>> lcr = (uart->data_bits - 5) | ((uart->stop_bits - 1) << 2) | uart->parity;
>>
>> /* No interrupts. */
>> - ns_write_reg(uart, IER, 0);
>> + ns_write_reg(uart, UART_IER, 0);
>>
>> /* Line control and baud-rate generator. */
>> - ns_write_reg(uart, LCR, lcr | LCR_DLAB);
>> + ns_write_reg(uart, UART_LCR, lcr | UART_LCR_DLAB);
>> if ( uart->baud != BAUD_AUTO )
>> {
>> /* Baud rate specified: program it into the divisor latch. */
>> divisor = uart->clock_hz / (uart->baud << 4);
>> - ns_write_reg(uart, DLL, (char)divisor);
>> - ns_write_reg(uart, DLM, (char)(divisor >> 8));
>> + ns_write_reg(uart, UART_DLL, (char)divisor);
>> + ns_write_reg(uart, UART_DLM, (char)(divisor >> 8));
>> }
>> else
>> {
>> /* Baud rate already set: read it out from the divisor latch. */
>> - divisor = ns_read_reg(uart, DLL);
>> - divisor |= ns_read_reg(uart, DLM) << 8;
>> + divisor = ns_read_reg(uart, UART_DLL);
>> + divisor |= ns_read_reg(uart, UART_DLM) << 8;
>> uart->baud = uart->clock_hz / (divisor << 4);
>> }
>> - ns_write_reg(uart, LCR, lcr);
>> + ns_write_reg(uart, UART_LCR, lcr);
>>
>> /* No flow ctrl: DTR and RTS are both wedged high to keep remote happy. */
>> - ns_write_reg(uart, MCR, MCR_DTR | MCR_RTS);
>> + ns_write_reg(uart, UART_MCR, UART_MCR_DTR | UART_MCR_RTS);
>>
>> /* Enable and clear the FIFOs. Set a large trigger threshold. */
>> - ns_write_reg(uart, FCR, FCR_ENABLE | FCR_CLRX | FCR_CLTX | FCR_TRG14);
>> + ns_write_reg(uart, UART_FCR,
>> + UART_FCR_ENABLE | UART_FCR_CLRX | UART_FCR_CLTX | UART_FCR_TRG14);
>> }
>>
>> static void __init ns16550_init_preirq(struct serial_port *port)
>> @@ -292,8 +224,8 @@ static void __init ns16550_init_preirq(struct serial_port *port)
>> ns16550_setup_preirq(uart);
>>
>> /* Check this really is a 16550+. Otherwise we have no FIFOs. */
>> - if ( ((ns_read_reg(uart, IIR) & 0xc0) == 0xc0) &&
>> - ((ns_read_reg(uart, FCR) & FCR_TRG14) == FCR_TRG14) )
>> + if ( ((ns_read_reg(uart, UART_IIR) & 0xc0) == 0xc0) &&
>> + ((ns_read_reg(uart, UART_FCR) & UART_FCR_TRG14) == UART_FCR_TRG14) )
>> uart->fifo_size = 16;
>> }
>>
>> @@ -302,10 +234,11 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
>> if ( uart->irq > 0 )
>> {
>> /* Master interrupt enable; also keep DTR/RTS asserted. */
>> - ns_write_reg(uart, MCR, MCR_OUT2 | MCR_DTR | MCR_RTS);
>> + ns_write_reg(uart,
>> + UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
>>
>> /* Enable receive and transmit interrupts. */
>> - ns_write_reg(uart, IER, IER_ERDAI | IER_ETHREI);
>> + ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ETHREI);
>> }
>>
>> if ( uart->irq >= 0 )
>> @@ -374,11 +307,11 @@ static void _ns16550_resume(struct serial_port *port)
>>
>> static int ns16550_ioport_invalid(struct ns16550 *uart)
>> {
>> - return ((((unsigned char)ns_read_reg(uart, LSR)) == 0xff) &&
>> - (((unsigned char)ns_read_reg(uart, MCR)) == 0xff) &&
>> - (((unsigned char)ns_read_reg(uart, IER)) == 0xff) &&
>> - (((unsigned char)ns_read_reg(uart, IIR)) == 0xff) &&
>> - (((unsigned char)ns_read_reg(uart, LCR)) == 0xff));
>> + return ((((unsigned char)ns_read_reg(uart, UART_LSR)) == 0xff) &&
>> + (((unsigned char)ns_read_reg(uart, UART_MCR)) == 0xff) &&
>> + (((unsigned char)ns_read_reg(uart, UART_IER)) == 0xff) &&
>> + (((unsigned char)ns_read_reg(uart, UART_IIR)) == 0xff) &&
>> + (((unsigned char)ns_read_reg(uart, UART_LCR)) == 0xff));
>> }
>>
>> static int delayed_resume_tries;
>> @@ -457,15 +390,15 @@ static int __init parse_parity_char(int c)
>> switch ( c )
>> {
>> case 'n':
>> - return PARITY_NONE;
>> + return UART_PARITY_NONE;
>> case 'o':
>> - return PARITY_ODD;
>> + return UART_PARITY_ODD;
>> case 'e':
>> - return PARITY_EVEN;
>> + return UART_PARITY_EVEN;
>> case 'm':
>> - return PARITY_MARK;
>> + return UART_PARITY_MARK;
>> case 's':
>> - return PARITY_SPACE;
>> + return UART_PARITY_SPACE;
>> }
>> return 0;
>> }
>> @@ -500,17 +433,17 @@ static int __init check_existence(struct ns16550 *uart)
>> * Do a simple existence test first; if we fail this,
>> * there's no point trying anything else.
>> */
>> - scratch = ns_read_reg(uart, IER);
>> - ns_write_reg(uart, IER, 0);
>> + scratch = ns_read_reg(uart, UART_IER);
>> + ns_write_reg(uart, UART_IER, 0);
>>
>> /*
>> * Mask out IER[7:4] bits for test as some UARTs (e.g. TL
>> * 16C754B) allow only to modify them if an EFR bit is set.
>> */
>> - scratch2 = ns_read_reg(uart, IER) & 0x0f;
>> - ns_write_reg(uart, IER, 0x0F);
>> - scratch3 = ns_read_reg(uart, IER) & 0x0f;
>> - ns_write_reg(uart, IER, scratch);
>> + scratch2 = ns_read_reg(uart, UART_IER) & 0x0f;
>> + ns_write_reg(uart,UART_IER, 0x0F);
>> + scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
>> + ns_write_reg(uart, UART_IER, scratch);
>> if ( (scratch2 != 0) || (scratch3 != 0x0F) )
>> return 0;
>>
>> @@ -518,8 +451,8 @@ static int __init check_existence(struct ns16550 *uart)
>> * Check to see if a UART is really there.
>> * Use loopback test mode.
>> */
>> - ns_write_reg(uart, MCR, MCR_LOOP | 0x0A);
>> - status = ns_read_reg(uart, MSR) & 0xF0;
>> + ns_write_reg(uart, UART_MCR, UART_MCR_LOOP | 0x0A);
>> + status = ns_read_reg(uart, UART_MSR) & 0xF0;
>> return (status == 0x90);
>> }
>>
>> diff --git a/xen/include/xen/8250.h b/xen/include/xen/8250.h
>> new file mode 100644
>> index 0000000..84f35bc
>> --- /dev/null
>> +++ b/xen/include/xen/8250.h
>
> When you look to the name, it's not clear what this file is. Could you
> rename it to 8250-uart.h?
No problem. Thanks.
>
>> @@ -0,0 +1,104 @@
>> +/*
>> + * xen/include/xen/8250.h
>> + *
>> + * This header is extracted from driver/char/ns16550.c
>> + *
>> + * Common constant definition between early printk and the UART driver
>> + * for the 16550-series UART
>> + *
>> + * Copyright (c) 2003-2005, K A Fraser
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#ifndef __XEN_NS16550_H__
>> +#define __XEN_NS16550_H__
>> +
>> +/* Register offsets */
>> +#define UART_RBR 0x00 /* receive buffer */
>> +#define UART_THR 0x00 /* transmit holding */
>> +#define UART_IER 0x01 /* interrupt enable */
>> +#define UART_IIR 0x02 /* interrupt identity */
>> +#define UART_FCR 0x02 /* FIFO control */
>> +#define UART_LCR 0x03 /* line control */
>> +#define UART_MCR 0x04 /* Modem control */
>> +#define UART_LSR 0x05 /* line status */
>> +#define UART_MSR 0x06 /* Modem status */
>> +#define UART_DLL 0x00 /* divisor latch (ls) (DLAB=1) */
>> +#define UART_DLM 0x01 /* divisor latch (ms) (DLAB=1) */
>> +
>> +/* Interrupt Enable Register */
>> +#define UART_IER_ERDAI 0x01 /* rx data recv'd */
>> +#define UART_IER_ETHREI 0x02 /* tx reg. empty */
>> +#define UART_IER_ELSI 0x04 /* rx line status */
>> +#define UART_IER_EMSI 0x08 /* MODEM status */
>> +
>> +/* Interrupt Identificatiegister */
>> +#define UART_IIR_NOINT 0x01 /* no interrupt pending */
>> +#define UART_IIR_IMA 0x06 /* interrupt identity: */
>> +#define UART_IIR_LSI 0x06 /* - rx line status */
>> +#define UART_IIR_RDA 0x04 /* - rx data recv'd */
>> +#define UART_IIR_THR 0x02 /* - tx reg. empty */
>> +#define UART_IIR_MSI 0x00 /* - MODEM status */
>> +
>> +/* FIFO Control Register */
>> +#define UART_FCR_ENABLE 0x01 /* enable FIFO */
>> +#define UART_FCR_CLRX 0x02 /* clear Rx FIFO */
>> +#define UART_FCR_CLTX 0x04 /* clear Tx FIFO */
>> +#define UART_FCR_DMA 0x10 /* enter DMA mode */
>> +#define UART_FCR_TRG1 0x00 /* Rx FIFO trig lev 1 */
>> +#define UART_FCR_TRG4 0x40 /* Rx FIFO trig lev 4 */
>> +#define UART_FCR_TRG8 0x80 /* Rx FIFO trig lev 8 */
>> +#define UART_FCR_TRG14 0xc0 /* Rx FIFO trig lev 14 */
>> +
>> +/* Line Control Register */
>> +#define UART_LCR_DLAB 0x80 /* Divisor Latch Access */
>> +
>> +/* Modem Control Register */
>> +#define UART_MCR_DTR 0x01 /* Data Terminal Ready */
>> +#define UART_MCR_RTS 0x02 /* Request to Send */
>> +#define UART_MCR_OUT2 0x08 /* OUT2: interrupt mask */
>> +#define UART_MCR_LOOP 0x10 /* Enable loopback test mode */
>> +
>> +/* Line Status Register */
>> +#define UART_LSR_DR 0x01 /* Data ready */
>> +#define UART_LSR_OE 0x02 /* Overrun */
>> +#define UART_LSR_PE 0x04 /* Parity error */
>> +#define UART_LSR_FE 0x08 /* Framing error */
>> +#define UART_LSR_BI 0x10 /* Break */
>> +#define UART_LSR_THRE 0x20 /* Xmit hold reg empty */
>> +#define UART_LSR_TEMT 0x40 /* Xmitter empty */
>> +#define UART_LSR_ERR 0x80 /* Error */
>> +
>> +/* These parity settings can be ORed directly into the LCR. */
>> +#define UART_PARITY_NONE (0<<3)
>> +#define UART_PARITY_ODD (1<<3)
>> +#define UART_PARITY_EVEN (3<<3)
>> +#define UART_PARITY_MARK (5<<3)
>> +#define UART_PARITY_SPACE (7<<3)
>> +
>> +/* Frequency of external clock source. This definition assumes PC platform. */
>> +#define UART_CLOCK_HZ 1843200
>> +
>> +/* Resume retry settings */
>> +#define RESUME_DELAY MILLISECS(10)
>> +#define RESUME_RETRIES 100
>> +
>> +#endif /* __XEN_NS16550_H__ */
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>>
>
> --
> Julien
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] xen: extract register definitions from ns16550 into a separated header
2013-07-19 15:37 ` Chen Baozi
@ 2013-07-19 15:43 ` Julien Grall
0 siblings, 0 replies; 27+ messages in thread
From: Julien Grall @ 2013-07-19 15:43 UTC (permalink / raw)
To: Chen Baozi; +Cc: xen-devel, Keir Fraser, Ian Campbell, bjzhang
On 07/19/2013 04:37 PM, Chen Baozi wrote:
> On Fri, Jul 19, 2013 at 11:35 PM, Julien Grall <julien.grall@linaro.org> wrote:
>> On 07/19/2013 01:57 PM, Chen Baozi wrote:
>>> Since both UART driver codes on Allwinner A31, OMAP5 and x86 would use
>>> these definitions, we refactor the codes into a separated header to avoid
>>> unnecessary duplication.
>>>
>>> Signed-off-by: Chen Baozi <baozich@gmail.com>
>>> ---
>>> xen/drivers/char/ns16550.c | 155 +++++++++++++--------------------------------
>>> xen/include/xen/8250.h | 104 ++++++++++++++++++++++++++++++
>>> 2 files changed, 148 insertions(+), 111 deletions(-)
>>> create mode 100644 xen/include/xen/8250.h
>>>
>>> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
>>> index e0c87bb..9248330 100644
>>> --- a/xen/drivers/char/ns16550.c
>>> +++ b/xen/drivers/char/ns16550.c
>>> @@ -19,6 +19,7 @@
>>> #include <xen/iocap.h>
>>> #include <xen/pci.h>
>>> #include <xen/pci_regs.h>
>>> +#include <xen/8250.h>
>>> #include <asm/io.h>
>>> #ifdef CONFIG_X86
>>> #include <asm/fixmap.h>
>>> @@ -58,76 +59,6 @@ static struct ns16550 {
>>> u8 bar_idx;
>>> } ns16550_com[2] = { { 0 } };
>>>
>>> -/* Register offsets */
>>> -#define RBR 0x00 /* receive buffer */
>>> -#define THR 0x00 /* transmit holding */
>>> -#define IER 0x01 /* interrupt enable */
>>> -#define IIR 0x02 /* interrupt identity */
>>> -#define FCR 0x02 /* FIFO control */
>>> -#define LCR 0x03 /* line control */
>>> -#define MCR 0x04 /* Modem control */
>>> -#define LSR 0x05 /* line status */
>>> -#define MSR 0x06 /* Modem status */
>>> -#define DLL 0x00 /* divisor latch (ls) (DLAB=1) */
>>> -#define DLM 0x01 /* divisor latch (ms) (DLAB=1) */
>>> -
>>> -/* Interrupt Enable Register */
>>> -#define IER_ERDAI 0x01 /* rx data recv'd */
>>> -#define IER_ETHREI 0x02 /* tx reg. empty */
>>> -#define IER_ELSI 0x04 /* rx line status */
>>> -#define IER_EMSI 0x08 /* MODEM status */
>>> -
>>> -/* Interrupt Identification Register */
>>> -#define IIR_NOINT 0x01 /* no interrupt pending */
>>> -#define IIR_IMASK 0x06 /* interrupt identity: */
>>> -#define IIR_LSI 0x06 /* - rx line status */
>>> -#define IIR_RDAI 0x04 /* - rx data recv'd */
>>> -#define IIR_THREI 0x02 /* - tx reg. empty */
>>> -#define IIR_MSI 0x00 /* - MODEM status */
>>> -
>>> -/* FIFO Control Register */
>>> -#define FCR_ENABLE 0x01 /* enable FIFO */
>>> -#define FCR_CLRX 0x02 /* clear Rx FIFO */
>>> -#define FCR_CLTX 0x04 /* clear Tx FIFO */
>>> -#define FCR_DMA 0x10 /* enter DMA mode */
>>> -#define FCR_TRG1 0x00 /* Rx FIFO trig lev 1 */
>>> -#define FCR_TRG4 0x40 /* Rx FIFO trig lev 4 */
>>> -#define FCR_TRG8 0x80 /* Rx FIFO trig lev 8 */
>>> -#define FCR_TRG14 0xc0 /* Rx FIFO trig lev 14 */
>>> -
>>> -/* Line Control Register */
>>> -#define LCR_DLAB 0x80 /* Divisor Latch Access */
>>> -
>>> -/* Modem Control Register */
>>> -#define MCR_DTR 0x01 /* Data Terminal Ready */
>>> -#define MCR_RTS 0x02 /* Request to Send */
>>> -#define MCR_OUT2 0x08 /* OUT2: interrupt mask */
>>> -#define MCR_LOOP 0x10 /* Enable loopback test mode */
>>> -
>>> -/* Line Status Register */
>>> -#define LSR_DR 0x01 /* Data ready */
>>> -#define LSR_OE 0x02 /* Overrun */
>>> -#define LSR_PE 0x04 /* Parity error */
>>> -#define LSR_FE 0x08 /* Framing error */
>>> -#define LSR_BI 0x10 /* Break */
>>> -#define LSR_THRE 0x20 /* Xmit hold reg empty */
>>> -#define LSR_TEMT 0x40 /* Xmitter empty */
>>> -#define LSR_ERR 0x80 /* Error */
>>> -
>>> -/* These parity settings can be ORed directly into the LCR. */
>>> -#define PARITY_NONE (0<<3)
>>> -#define PARITY_ODD (1<<3)
>>> -#define PARITY_EVEN (3<<3)
>>> -#define PARITY_MARK (5<<3)
>>> -#define PARITY_SPACE (7<<3)
>>> -
>>> -/* Frequency of external clock source. This definition assumes PC platform. */
>>> -#define UART_CLOCK_HZ 1843200
>>> -
>>> -/* Resume retry settings */
>>> -#define RESUME_DELAY MILLISECS(10)
>>> -#define RESUME_RETRIES 100
>>> -
>>> static char ns_read_reg(struct ns16550 *uart, int reg)
>>> {
>>> if ( uart->remapped_io_base == NULL )
>>> @@ -150,12 +81,12 @@ static void ns16550_interrupt(
>>>
>>> uart->intr_works = 1;
>>>
>>> - while ( !(ns_read_reg(uart, IIR) & IIR_NOINT) )
>>> + while ( !(ns_read_reg(uart, UART_IIR) & UART_IIR_NOINT) )
>>> {
>>> - char lsr = ns_read_reg(uart, LSR);
>>> - if ( lsr & LSR_THRE )
>>> + char lsr = ns_read_reg(uart, UART_LSR);
>>> + if ( lsr & UART_LSR_THRE )
>>> serial_tx_interrupt(port, regs);
>>> - if ( lsr & LSR_DR )
>>> + if ( lsr & UART_LSR_DR )
>>> serial_rx_interrupt(port, regs);
>>> }
>>> }
>>> @@ -171,10 +102,10 @@ static void __ns16550_poll(struct cpu_user_regs *regs)
>>> if ( uart->intr_works )
>>> return; /* Interrupts work - no more polling */
>>>
>>> - while ( ns_read_reg(uart, LSR) & LSR_DR )
>>> + while ( ns_read_reg(uart, UART_LSR) & UART_LSR_DR )
>>> serial_rx_interrupt(port, regs);
>>>
>>> - if ( ns_read_reg(uart, LSR) & LSR_THRE )
>>> + if ( ns_read_reg(uart, UART_LSR) & UART_LSR_THRE )
>>> serial_tx_interrupt(port, regs);
>>>
>>> set_timer(&uart->timer, NOW() + MILLISECS(uart->timeout_ms));
>>> @@ -194,23 +125,23 @@ static unsigned int ns16550_tx_ready(struct serial_port *port)
>>> {
>>> struct ns16550 *uart = port->uart;
>>>
>>> - return ns_read_reg(uart, LSR) & LSR_THRE ? uart->fifo_size : 0;
>>> + return ns_read_reg(uart, UART_LSR) & UART_LSR_THRE ? uart->fifo_size : 0;
>>> }
>>>
>>> static void ns16550_putc(struct serial_port *port, char c)
>>> {
>>> struct ns16550 *uart = port->uart;
>>> - ns_write_reg(uart, THR, c);
>>> + ns_write_reg(uart, UART_THR, c);
>>> }
>>>
>>> static int ns16550_getc(struct serial_port *port, char *pc)
>>> {
>>> struct ns16550 *uart = port->uart;
>>>
>>> - if ( !(ns_read_reg(uart, LSR) & LSR_DR) )
>>> + if ( !(ns_read_reg(uart, UART_LSR) & UART_LSR_DR) )
>>> return 0;
>>>
>>> - *pc = ns_read_reg(uart, RBR);
>>> + *pc = ns_read_reg(uart, UART_RBR);
>>> return 1;
>>> }
>>>
>>> @@ -244,31 +175,32 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>>> lcr = (uart->data_bits - 5) | ((uart->stop_bits - 1) << 2) | uart->parity;
>>>
>>> /* No interrupts. */
>>> - ns_write_reg(uart, IER, 0);
>>> + ns_write_reg(uart, UART_IER, 0);
>>>
>>> /* Line control and baud-rate generator. */
>>> - ns_write_reg(uart, LCR, lcr | LCR_DLAB);
>>> + ns_write_reg(uart, UART_LCR, lcr | UART_LCR_DLAB);
>>> if ( uart->baud != BAUD_AUTO )
>>> {
>>> /* Baud rate specified: program it into the divisor latch. */
>>> divisor = uart->clock_hz / (uart->baud << 4);
>>> - ns_write_reg(uart, DLL, (char)divisor);
>>> - ns_write_reg(uart, DLM, (char)(divisor >> 8));
>>> + ns_write_reg(uart, UART_DLL, (char)divisor);
>>> + ns_write_reg(uart, UART_DLM, (char)(divisor >> 8));
>>> }
>>> else
>>> {
>>> /* Baud rate already set: read it out from the divisor latch. */
>>> - divisor = ns_read_reg(uart, DLL);
>>> - divisor |= ns_read_reg(uart, DLM) << 8;
>>> + divisor = ns_read_reg(uart, UART_DLL);
>>> + divisor |= ns_read_reg(uart, UART_DLM) << 8;
>>> uart->baud = uart->clock_hz / (divisor << 4);
>>> }
>>> - ns_write_reg(uart, LCR, lcr);
>>> + ns_write_reg(uart, UART_LCR, lcr);
>>>
>>> /* No flow ctrl: DTR and RTS are both wedged high to keep remote happy. */
>>> - ns_write_reg(uart, MCR, MCR_DTR | MCR_RTS);
>>> + ns_write_reg(uart, UART_MCR, UART_MCR_DTR | UART_MCR_RTS);
>>>
>>> /* Enable and clear the FIFOs. Set a large trigger threshold. */
>>> - ns_write_reg(uart, FCR, FCR_ENABLE | FCR_CLRX | FCR_CLTX | FCR_TRG14);
>>> + ns_write_reg(uart, UART_FCR,
>>> + UART_FCR_ENABLE | UART_FCR_CLRX | UART_FCR_CLTX | UART_FCR_TRG14);
>>> }
>>>
>>> static void __init ns16550_init_preirq(struct serial_port *port)
>>> @@ -292,8 +224,8 @@ static void __init ns16550_init_preirq(struct serial_port *port)
>>> ns16550_setup_preirq(uart);
>>>
>>> /* Check this really is a 16550+. Otherwise we have no FIFOs. */
>>> - if ( ((ns_read_reg(uart, IIR) & 0xc0) == 0xc0) &&
>>> - ((ns_read_reg(uart, FCR) & FCR_TRG14) == FCR_TRG14) )
>>> + if ( ((ns_read_reg(uart, UART_IIR) & 0xc0) == 0xc0) &&
>>> + ((ns_read_reg(uart, UART_FCR) & UART_FCR_TRG14) == UART_FCR_TRG14) )
>>> uart->fifo_size = 16;
>>> }
>>>
>>> @@ -302,10 +234,11 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
>>> if ( uart->irq > 0 )
>>> {
>>> /* Master interrupt enable; also keep DTR/RTS asserted. */
>>> - ns_write_reg(uart, MCR, MCR_OUT2 | MCR_DTR | MCR_RTS);
>>> + ns_write_reg(uart,
>>> + UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
>>>
>>> /* Enable receive and transmit interrupts. */
>>> - ns_write_reg(uart, IER, IER_ERDAI | IER_ETHREI);
>>> + ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ETHREI);
>>> }
>>>
>>> if ( uart->irq >= 0 )
>>> @@ -374,11 +307,11 @@ static void _ns16550_resume(struct serial_port *port)
>>>
>>> static int ns16550_ioport_invalid(struct ns16550 *uart)
>>> {
>>> - return ((((unsigned char)ns_read_reg(uart, LSR)) == 0xff) &&
>>> - (((unsigned char)ns_read_reg(uart, MCR)) == 0xff) &&
>>> - (((unsigned char)ns_read_reg(uart, IER)) == 0xff) &&
>>> - (((unsigned char)ns_read_reg(uart, IIR)) == 0xff) &&
>>> - (((unsigned char)ns_read_reg(uart, LCR)) == 0xff));
>>> + return ((((unsigned char)ns_read_reg(uart, UART_LSR)) == 0xff) &&
>>> + (((unsigned char)ns_read_reg(uart, UART_MCR)) == 0xff) &&
>>> + (((unsigned char)ns_read_reg(uart, UART_IER)) == 0xff) &&
>>> + (((unsigned char)ns_read_reg(uart, UART_IIR)) == 0xff) &&
>>> + (((unsigned char)ns_read_reg(uart, UART_LCR)) == 0xff));
>>> }
>>>
>>> static int delayed_resume_tries;
>>> @@ -457,15 +390,15 @@ static int __init parse_parity_char(int c)
>>> switch ( c )
>>> {
>>> case 'n':
>>> - return PARITY_NONE;
>>> + return UART_PARITY_NONE;
>>> case 'o':
>>> - return PARITY_ODD;
>>> + return UART_PARITY_ODD;
>>> case 'e':
>>> - return PARITY_EVEN;
>>> + return UART_PARITY_EVEN;
>>> case 'm':
>>> - return PARITY_MARK;
>>> + return UART_PARITY_MARK;
>>> case 's':
>>> - return PARITY_SPACE;
>>> + return UART_PARITY_SPACE;
>>> }
>>> return 0;
>>> }
>>> @@ -500,17 +433,17 @@ static int __init check_existence(struct ns16550 *uart)
>>> * Do a simple existence test first; if we fail this,
>>> * there's no point trying anything else.
>>> */
>>> - scratch = ns_read_reg(uart, IER);
>>> - ns_write_reg(uart, IER, 0);
>>> + scratch = ns_read_reg(uart, UART_IER);
>>> + ns_write_reg(uart, UART_IER, 0);
>>>
>>> /*
>>> * Mask out IER[7:4] bits for test as some UARTs (e.g. TL
>>> * 16C754B) allow only to modify them if an EFR bit is set.
>>> */
>>> - scratch2 = ns_read_reg(uart, IER) & 0x0f;
>>> - ns_write_reg(uart, IER, 0x0F);
>>> - scratch3 = ns_read_reg(uart, IER) & 0x0f;
>>> - ns_write_reg(uart, IER, scratch);
>>> + scratch2 = ns_read_reg(uart, UART_IER) & 0x0f;
>>> + ns_write_reg(uart,UART_IER, 0x0F);
>>> + scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
>>> + ns_write_reg(uart, UART_IER, scratch);
>>> if ( (scratch2 != 0) || (scratch3 != 0x0F) )
>>> return 0;
>>>
>>> @@ -518,8 +451,8 @@ static int __init check_existence(struct ns16550 *uart)
>>> * Check to see if a UART is really there.
>>> * Use loopback test mode.
>>> */
>>> - ns_write_reg(uart, MCR, MCR_LOOP | 0x0A);
>>> - status = ns_read_reg(uart, MSR) & 0xF0;
>>> + ns_write_reg(uart, UART_MCR, UART_MCR_LOOP | 0x0A);
>>> + status = ns_read_reg(uart, UART_MSR) & 0xF0;
>>> return (status == 0x90);
>>> }
>>>
>>> diff --git a/xen/include/xen/8250.h b/xen/include/xen/8250.h
>>> new file mode 100644
>>> index 0000000..84f35bc
>>> --- /dev/null
>>> +++ b/xen/include/xen/8250.h
>>
>> When you look to the name, it's not clear what this file is. Could you
>> rename it to 8250-uart.h?
>
> No problem. Thanks.
BTW, this patch was already committed on 16 july: 3726b10.
The file was named ns16550-uart.h
>>
>>> @@ -0,0 +1,104 @@
>>> +/*
>>> + * xen/include/xen/8250.h
>>> + *
>>> + * This header is extracted from driver/char/ns16550.c
>>> + *
>>> + * Common constant definition between early printk and the UART driver
>>> + * for the 16550-series UART
>>> + *
>>> + * Copyright (c) 2003-2005, K A Fraser
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + */
>>> +
>>> +#ifndef __XEN_NS16550_H__
>>> +#define __XEN_NS16550_H__
>>> +
>>> +/* Register offsets */
>>> +#define UART_RBR 0x00 /* receive buffer */
>>> +#define UART_THR 0x00 /* transmit holding */
>>> +#define UART_IER 0x01 /* interrupt enable */
>>> +#define UART_IIR 0x02 /* interrupt identity */
>>> +#define UART_FCR 0x02 /* FIFO control */
>>> +#define UART_LCR 0x03 /* line control */
>>> +#define UART_MCR 0x04 /* Modem control */
>>> +#define UART_LSR 0x05 /* line status */
>>> +#define UART_MSR 0x06 /* Modem status */
>>> +#define UART_DLL 0x00 /* divisor latch (ls) (DLAB=1) */
>>> +#define UART_DLM 0x01 /* divisor latch (ms) (DLAB=1) */
>>> +
>>> +/* Interrupt Enable Register */
>>> +#define UART_IER_ERDAI 0x01 /* rx data recv'd */
>>> +#define UART_IER_ETHREI 0x02 /* tx reg. empty */
>>> +#define UART_IER_ELSI 0x04 /* rx line status */
>>> +#define UART_IER_EMSI 0x08 /* MODEM status */
>>> +
>>> +/* Interrupt Identificatiegister */
>>> +#define UART_IIR_NOINT 0x01 /* no interrupt pending */
>>> +#define UART_IIR_IMA 0x06 /* interrupt identity: */
>>> +#define UART_IIR_LSI 0x06 /* - rx line status */
>>> +#define UART_IIR_RDA 0x04 /* - rx data recv'd */
>>> +#define UART_IIR_THR 0x02 /* - tx reg. empty */
>>> +#define UART_IIR_MSI 0x00 /* - MODEM status */
>>> +
>>> +/* FIFO Control Register */
>>> +#define UART_FCR_ENABLE 0x01 /* enable FIFO */
>>> +#define UART_FCR_CLRX 0x02 /* clear Rx FIFO */
>>> +#define UART_FCR_CLTX 0x04 /* clear Tx FIFO */
>>> +#define UART_FCR_DMA 0x10 /* enter DMA mode */
>>> +#define UART_FCR_TRG1 0x00 /* Rx FIFO trig lev 1 */
>>> +#define UART_FCR_TRG4 0x40 /* Rx FIFO trig lev 4 */
>>> +#define UART_FCR_TRG8 0x80 /* Rx FIFO trig lev 8 */
>>> +#define UART_FCR_TRG14 0xc0 /* Rx FIFO trig lev 14 */
>>> +
>>> +/* Line Control Register */
>>> +#define UART_LCR_DLAB 0x80 /* Divisor Latch Access */
>>> +
>>> +/* Modem Control Register */
>>> +#define UART_MCR_DTR 0x01 /* Data Terminal Ready */
>>> +#define UART_MCR_RTS 0x02 /* Request to Send */
>>> +#define UART_MCR_OUT2 0x08 /* OUT2: interrupt mask */
>>> +#define UART_MCR_LOOP 0x10 /* Enable loopback test mode */
>>> +
>>> +/* Line Status Register */
>>> +#define UART_LSR_DR 0x01 /* Data ready */
>>> +#define UART_LSR_OE 0x02 /* Overrun */
>>> +#define UART_LSR_PE 0x04 /* Parity error */
>>> +#define UART_LSR_FE 0x08 /* Framing error */
>>> +#define UART_LSR_BI 0x10 /* Break */
>>> +#define UART_LSR_THRE 0x20 /* Xmit hold reg empty */
>>> +#define UART_LSR_TEMT 0x40 /* Xmitter empty */
>>> +#define UART_LSR_ERR 0x80 /* Error */
>>> +
>>> +/* These parity settings can be ORed directly into the LCR. */
>>> +#define UART_PARITY_NONE (0<<3)
>>> +#define UART_PARITY_ODD (1<<3)
>>> +#define UART_PARITY_EVEN (3<<3)
>>> +#define UART_PARITY_MARK (5<<3)
>>> +#define UART_PARITY_SPACE (7<<3)
>>> +
>>> +/* Frequency of external clock source. This definition assumes PC platform. */
>>> +#define UART_CLOCK_HZ 1843200
>>> +
>>> +/* Resume retry settings */
>>> +#define RESUME_DELAY MILLISECS(10)
>>> +#define RESUME_RETRIES 100
>>> +
>>> +#endif /* __XEN_NS16550_H__ */
>>> +
>>> +/*
>>> + * Local variables:
>>> + * mode: C
>>> + * c-file-style: "BSD"
>>> + * c-basic-offset: 4
>>> + * indent-tabs-mode: nil
>>> + * End:
>>> + */
>>>
>>
>> --
>> Julien
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] xen/arm: add OMAP5432 UART support for early_printk
2013-07-19 12:57 ` [PATCH 2/4] xen/arm: add OMAP5432 UART support for early_printk Chen Baozi
@ 2013-07-19 15:49 ` Julien Grall
0 siblings, 0 replies; 27+ messages in thread
From: Julien Grall @ 2013-07-19 15:49 UTC (permalink / raw)
To: Chen Baozi; +Cc: xen-devel, Ian Campbell, bjzhang
On 07/19/2013 01:57 PM, Chen Baozi wrote:
>
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> ---
> docs/misc/arm/early-printk.txt | 1 +
> xen/arch/arm/Rules.mk | 4 ++++
> xen/arch/arm/arm32/debug-8250.inc | 48 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 53 insertions(+)
> create mode 100644 xen/arch/arm/arm32/debug-8250.inc
>
> diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt
> index fbc3208..874488f 100644
> --- a/docs/misc/arm/early-printk.txt
> +++ b/docs/misc/arm/early-printk.txt
> @@ -13,6 +13,7 @@ where mach is the name of the machine:
> - exynos5250: printk with the second UART
> - midway: printk with the pl011 on Calxeda Midway processors
> - fastmodel: printk on ARM Fastmodel software emulators
> + - omap5432: printk with UART3 on TI OMAP5432 processors
>
> The base address and baud rate is hardcoded in xen/arch/arm/Rules.mk,
> see there when adding support for new machines.
> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index 422ed04..eaa03fb 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -64,6 +64,10 @@ EARLY_PRINTK_INC := pl011
> EARLY_PRINTK_BAUD := 115200
> EARLY_UART_BASE_ADDRESS := 0xfff36000
> endif
> +ifeq ($(CONFIG_EARLY_PRINTK), omap5432)
> +EARLY_PRINTK_INC := 8250
> +EARLY_UART_BASE_ADDRESS := 0x48020000
> +endif
>
> ifneq ($(EARLY_PRINTK_INC),)
> EARLY_PRINTK := y
> diff --git a/xen/arch/arm/arm32/debug-8250.inc b/xen/arch/arm/arm32/debug-8250.inc
> new file mode 100644
> index 0000000..cbb63ca
> --- /dev/null
> +++ b/xen/arch/arm/arm32/debug-8250.inc
> @@ -0,0 +1,48 @@
> +/*
> + * xen/arch/arm/arm32/debug-8250.inc
> + *
> + * 8250 specific debug code
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <xen/8250.h>
> +
> +/* OMAP UART initialization
> + * rb: register which contains the UART base address
> + * rc: scratch register 1
> + * rd: scratch register 2 */
> +.macro early_uart_init rb rc rd
> +.endm
You don't need to define early_uart_init. The debug code will not use it
if EARLY_PRINTK_INIT_UART is not enabled.
> +
> +/* OMAP UART wait UART to be ready to transmit
> + * rb: register which contains the UART base address
> + * rc: scratch register */
> +.macro early_uart_ready rb rc
> +1:
> + ldr \rc, [\rb, #(UART_LSR<<2)] /* Read line status register */
Where does "<<2" come from?
> + tst \rc, #UART_LSR_THRE /* Check Xmit holding register flag */
> + beq 1b /* Wait for the UART to be ready */
> +.endm
> +
> +/* OMAP UART transmit character
> + * rb: register which contains the UART base address
> + * rt: register which contains the character to transmit */
> +.macro early_uart_transmit rb rt
> + str \rt, [\rb, #UART_THR] /* Write Transmit buffer */
> +.endm
> +
> +/*
> + * Local variables:
> + * mode: ASM
> + * indent-tabs-mode: nil
> + * End:
> + */
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/4] xen/arm: enable ns16550 uart driver for OMAP5432
2013-07-19 12:57 ` [PATCH 4/4] xen/arm: enable ns16550 uart driver for OMAP5432 Chen Baozi
@ 2013-07-19 17:06 ` Julien Grall
2013-07-22 13:06 ` Chen Baozi
0 siblings, 1 reply; 27+ messages in thread
From: Julien Grall @ 2013-07-19 17:06 UTC (permalink / raw)
To: Chen Baozi; +Cc: Keir Fraser, Ian Campbell, xen-devel
On 07/19/2013 01:57 PM, Chen Baozi wrote:
>
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> ---
> xen/arch/arm/Rules.mk | 1 +
> xen/drivers/char/ns16550.c | 155 +++++++++++++++++++++++++++++++++++++++------
> xen/include/asm-arm/io.h | 49 ++++++++++++++
> xen/include/xen/serial.h | 7 +-
> 4 files changed, 191 insertions(+), 21 deletions(-)
>
> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index eaa03fb..8604e34 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -9,6 +9,7 @@
> HAS_DEVICE_TREE := y
> HAS_VIDEO := y
> HAS_ARM_HDLCD := y
> +HAS_NS16550 := y
>
> CFLAGS += -fno-builtin -fno-common -Wredundant-decls
> CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index 60512be..b8690eb 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -1,10 +1,10 @@
> /******************************************************************************
> * ns16550.c
> - *
> + *
Spurious change.
> * Driver for 16550-series UARTs. This driver is to be kept within Xen as
> * it permits debugging of seriously-toasted machines (e.g., in situations
> * where a device driver within a guest OS would be inaccessible).
> - *
> + *
Spurious change.
> * Copyright (c) 2003-2005, K A Fraser
> */
>
> @@ -25,6 +25,13 @@
> #include <asm/fixmap.h>
> #endif
>
> +#ifdef CONFIG_ARM
> +#include <xen/device_tree.h>
> +#include <xen/errno.h>
The above 2 includes can be moved outside the ifdef.
> +#include <asm/early_printk.h>
> +#include <asm/device.h>
> +#endif
> +
> /*
> * Configure serial port with a string:
> * <baud>[/<clock_hz>][,DPS[,<io-base>[,<irq>[,<port-bdf>[,<bridge-bdf>]]]]].
> @@ -39,8 +46,11 @@ string_param("com1", opt_com1);
> string_param("com2", opt_com2);
>
> static struct ns16550 {
> - int baud, clock_hz, data_bits, parity, stop_bits, fifo_size, irq;
> - unsigned long io_base; /* I/O port or memory-mapped I/O address. */
> + int baud, data_bits, parity, stop_bits, fifo_size;
> + u32 clock_hz;
> + struct dt_irq irq;
> + u64 io_base; /* I/O port or memory-mapped I/O address. */
> + u64 io_size;
> char __iomem *remapped_io_base; /* Remapped virtual address of MMIO. */
> /* UART with IRQ line: interrupt-driven I/O. */
> struct irqaction irqaction;
> @@ -61,16 +71,24 @@ static struct ns16550 {
>
> static char ns_read_reg(struct ns16550 *uart, int reg)
> {
> +#ifdef CONFIG_X86
> if ( uart->remapped_io_base == NULL )
> return inb(uart->io_base + reg);
> return readb(uart->remapped_io_base + reg);
> +#else
> + return readb(uart->remapped_io_base + (reg << 2));
> +#endif
> }
>
Is it possible to add a new private field "shift"? So the both readb can
be merged.
> static void ns_write_reg(struct ns16550 *uart, int reg, char c)
> {
> +#ifdef CONFIG_X86
> if ( uart->remapped_io_base == NULL )
> return outb(c, uart->io_base + reg);
> writeb(c, uart->remapped_io_base + reg);
> +#else
> + writeb(c, uart->remapped_io_base + (reg << 2));
> +#endif
Same here for readb.
> }
>
> static void ns16550_interrupt(
> @@ -145,11 +163,12 @@ static int ns16550_getc(struct serial_port *port, char *pc)
> return 1;
> }
>
> +#ifdef CONFIG_X86
> static void pci_serial_early_init(struct ns16550 *uart)
> {
> if ( !uart->ps_bdf_enable || uart->io_base >= 0x10000 )
> return;
> -
> +
Spurious change.
> if ( uart->pb_bdf_enable )
> pci_conf_write16(0, uart->pb_bdf[0], uart->pb_bdf[1], uart->pb_bdf[2],
> PCI_IO_BASE,
> @@ -161,7 +180,13 @@ static void pci_serial_early_init(struct ns16550 *uart)
> uart->io_base | PCI_BASE_ADDRESS_SPACE_IO);
> pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1], uart->ps_bdf[2],
> PCI_COMMAND, PCI_COMMAND_IO);
> +
> +}
> +#else
> +static void pci_serial_early_init(struct ns16550 *uart)
> +{
> }
> +#endif
>
> static void ns16550_setup_preirq(struct ns16550 *uart)
> {
> @@ -217,7 +242,9 @@ static void __init ns16550_init_preirq(struct serial_port *port)
> uart->remapped_io_base = (void __iomem *)fix_to_virt(idx);
> uart->remapped_io_base += uart->io_base & ~PAGE_MASK;
> #else
> - uart->remapped_io_base = (char *)ioremap(uart->io_base, 8);
> + uart->remapped_io_base = ioremap_attr(uart->io_base,
> + uart->io_size,
> + PAGE_HYPERVISOR_NOCACHE);
> #endif
> }
>
> @@ -231,7 +258,7 @@ static void __init ns16550_init_preirq(struct serial_port *port)
>
> static void ns16550_setup_postirq(struct ns16550 *uart)
> {
> - if ( uart->irq > 0 )
> + if ( uart->irq.irq > 0 )
> {
> /* Master interrupt enable; also keep DTR/RTS asserted. */
> ns_write_reg(uart,
> @@ -241,7 +268,7 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
> ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ELSI);
> }
>
> - if ( uart->irq >= 0 )
> + if ( uart->irq.irq >= 0 )
> set_timer(&uart->timer, NOW() + MILLISECS(uart->timeout_ms));
> }
>
> @@ -250,7 +277,7 @@ static void __init ns16550_init_postirq(struct serial_port *port)
> struct ns16550 *uart = port->uart;
> int rc, bits;
>
> - if ( uart->irq < 0 )
> + if ( uart->irq.irq < 0 )
> return;
>
> serial_async_transmit(port);
> @@ -262,20 +289,26 @@ static void __init ns16550_init_postirq(struct serial_port *port)
> uart->timeout_ms = max_t(
> unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
>
> - if ( uart->irq > 0 )
> + if ( uart->irq.irq > 0 )
> {
> uart->irqaction.handler = ns16550_interrupt;
> uart->irqaction.name = "ns16550";
> uart->irqaction.dev_id = port;
> - if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
> - printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq);
> +#ifdef CONFIG_X86
> + if ( (rc = setup_irq(uart->irq.irq, &uart->irqaction)) != 0 )
> +#else
> + if ( (rc = setup_dt_irq(&uart->irq, &uart->irqaction)) != 0 )
> +#endif
> + printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq.irq);
> }
>
> ns16550_setup_postirq(uart);
>
> +#ifdef CONFIG_X86
> if ( uart->bar || uart->ps_bdf_enable )
> pci_hide_device(uart->ps_bdf[0], PCI_DEVFN(uart->ps_bdf[1],
> uart->ps_bdf[2]));
> +#endif
> }
>
> static void ns16550_suspend(struct serial_port *port)
> @@ -284,13 +317,16 @@ static void ns16550_suspend(struct serial_port *port)
>
> stop_timer(&uart->timer);
>
> +#ifdef CONFIG_X86
> if ( uart->bar )
> uart->cr = pci_conf_read16(0, uart->ps_bdf[0], uart->ps_bdf[1],
> uart->ps_bdf[2], PCI_COMMAND);
> +#endif
> }
>
> static void _ns16550_resume(struct serial_port *port)
> {
> +#ifdef CONFIG_X86
> struct ns16550 *uart = port->uart;
>
> if ( uart->bar )
> @@ -300,6 +336,7 @@ static void _ns16550_resume(struct serial_port *port)
> pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1], uart->ps_bdf[2],
> PCI_COMMAND, uart->cr);
> }
> +#endif
>
> ns16550_setup_preirq(port->uart);
> ns16550_setup_postirq(port->uart);
> @@ -370,9 +407,17 @@ static void __init ns16550_endboot(struct serial_port *port)
> static int __init ns16550_irq(struct serial_port *port)
> {
> struct ns16550 *uart = port->uart;
> - return ((uart->irq > 0) ? uart->irq : -1);
> + return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
> }
>
> +#ifdef CONFIG_ARM
> +static const struct dt_irq __init *ns16550_dt_irq(struct serial_port *port)
> +{
> + struct ns16550 *uart = port->uart;
> + return &uart->irq;
> +}
> +#endif
You don't need to surround this function by ifdef.
> static struct uart_driver __read_mostly ns16550_driver = {
> .init_preirq = ns16550_init_preirq,
> .init_postirq = ns16550_init_postirq,
> @@ -382,7 +427,10 @@ static struct uart_driver __read_mostly ns16550_driver = {
> .tx_ready = ns16550_tx_ready,
> .putc = ns16550_putc,
> .getc = ns16550_getc,
> - .irq = ns16550_irq
> + .irq = ns16550_irq,
> +#ifdef CONFIG_ARM
> + .dt_irq_get = ns16550_dt_irq
> +#endif
> };
same here.
> static int __init parse_parity_char(int c)
> @@ -403,6 +451,7 @@ static int __init parse_parity_char(int c)
> return 0;
> }
>
> +#ifdef CONFIG_X86
> static void __init parse_pci_bdf(const char **conf, unsigned int bdf[3])
> {
> bdf[0] = simple_strtoul(*conf, conf, 16);
> @@ -415,6 +464,7 @@ static void __init parse_pci_bdf(const char **conf, unsigned int bdf[3])
> (*conf)++;
> bdf[2] = simple_strtoul(*conf, conf, 16);
> }
> +#endif
>
> static int __init check_existence(struct ns16550 *uart)
> {
> @@ -428,7 +478,7 @@ static int __init check_existence(struct ns16550 *uart)
> return 1;
>
> pci_serial_early_init(uart);
> -
> +
spurious change.
> /*
> * Do a simple existence test first; if we fail this,
> * there's no point trying anything else.
> @@ -445,7 +495,7 @@ static int __init check_existence(struct ns16550 *uart)
> scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
> ns_write_reg(uart, UART_IER, scratch);
> if ( (scratch2 != 0) || (scratch3 != 0x0F) )
> - return 0;
> + return 0;
spurious change.
>
> /*
> * Check to see if a UART is really there.
> @@ -456,6 +506,7 @@ static int __init check_existence(struct ns16550 *uart)
> return (status == 0x90);
> }
>
> +#ifdef CONFIG_X86
> static int
> pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
> {
> @@ -507,7 +558,7 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
> uart->bar = bar;
> uart->bar_idx = bar_idx;
> uart->io_base = bar & ~PCI_BASE_ADDRESS_SPACE_IO;
> - uart->irq = pci_conf_read8(0, b, d, f, PCI_INTERRUPT_PIN) ?
> + uart->irq.irq = pci_conf_read8(0, b, d, f, PCI_INTERRUPT_PIN) ?
> pci_conf_read8(0, b, d, f, PCI_INTERRUPT_LINE) : 0;
>
> return 0;
> @@ -519,11 +570,12 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
> return -1;
>
> uart->io_base = 0x3f8;
> - uart->irq = 0;
> + uart->irq.irq = 0;
> uart->clock_hz = UART_CLOCK_HZ;
>
> return 0;
> }
> +#endif
>
> #define PARSE_ERR(_f, _a...) \
> do { \
> @@ -534,6 +586,7 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
> static void __init ns16550_parse_port_config(
> struct ns16550 *uart, const char *conf)
> {
> +#ifdef CONFIG_X86
> int baud;
>
> /* No user-specified configuration? */
> @@ -589,7 +642,7 @@ static void __init ns16550_parse_port_config(
> }
>
> if ( *conf == ',' && *++conf != ',' )
> - uart->irq = simple_strtol(conf, &conf, 10);
> + uart->irq.irq = simple_strtol(conf, &conf, 10);
>
> if ( *conf == ',' && *++conf != ',' )
> {
> @@ -604,6 +657,8 @@ static void __init ns16550_parse_port_config(
> }
>
> config_parsed:
> +#endif
> +
You have disabled nearly 80% of the code for ARM. Perhaps you can split
this function in 2:
- Parsing pci/user configuration
- Register the UART
> /* Sanity checks. */
> if ( (uart->baud != BAUD_AUTO) &&
> ((uart->baud < 1200) || (uart->baud > 115200)) )
> @@ -633,18 +688,80 @@ void __init ns16550_init(int index, struct ns16550_defaults *defaults)
> uart->baud = (defaults->baud ? :
> console_has((index == 0) ? "com1" : "com2")
> ? BAUD_AUTO : 0);
> +#ifdef CONFIG_ARM
> + uart->clock_hz = defaults->clock_hz;
> +#else
> uart->clock_hz = UART_CLOCK_HZ;
> +#endif
> uart->data_bits = defaults->data_bits;
> uart->parity = parse_parity_char(defaults->parity);
> uart->stop_bits = defaults->stop_bits;
> uart->irq = defaults->irq;
> uart->io_base = defaults->io_base;
> +#ifdef CONFIG_ARM
> + uart->io_size = defaults->io_size;
> +#endif
> +
> /* Default is no transmit FIFO. */
> uart->fifo_size = 1;
>
> ns16550_parse_port_config(uart, (index == 0) ? opt_com1 : opt_com2);
> +
> }
>
> +#ifdef CONFIG_ARM
> +
> +static int __init ns16550_dt_init(struct dt_device_node *dev,
> + const void *data)
> +{
> + struct ns16550_defaults uart;
> + int res;
> + const __be32 *clkspec;
> +
> + uart.baud = 115200;
> + uart.data_bits = 8;
> + uart.parity = 'n';
> + uart.stop_bits = 1;
> +
> + res = dt_device_get_address(dev, 0, &uart.io_base, &uart.io_size);
> + if (res) {
> + early_printk("ns16550: Unable to retrieve the base"
> + " address of the UART\n");
> + return res;
> + }
> +
> + res = dt_device_get_irq(dev, 0, &uart.irq);
> + if (res) {
> + early_printk("ns16550: Unable to retrieve the IRQ\n");
> + return res;
> + }
> +
> + clkspec = dt_get_property(dev, "clock-frequency", NULL);
> + if (clkspec == NULL) {
> + early_printk("ns16550: Unable to retrieve the clock frequency\n");
> + return -EINVAL;
> + }
> + uart.clock_hz = be32_to_cpup(clkspec);
> +
> + ns16550_init(0, &uart);
> + dt_device_set_used_by(dev, DOMID_XEN);
> +
> + return 0;
> +}
> +
> +static const char const *ns16550_dt_compat[] __initdata =
> +{
> + "ti,omap4-uart",
> + NULL
> +};
> +
> +DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
> + .compatible = ns16550_dt_compat,
> + .init = ns16550_dt_init,
> +DT_DEVICE_END
> +
> +#endif /* CONFIG_ARM */
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/include/asm-arm/io.h b/xen/include/asm-arm/io.h
> index aea5233..33674bc 100644
> --- a/xen/include/asm-arm/io.h
> +++ b/xen/include/asm-arm/io.h
> @@ -1,6 +1,55 @@
> #ifndef _ASM_IO_H
> #define _ASM_IO_H
>
> +static inline void __raw_writeb(u8 val, volatile void __iomem *addr)
> +{
> + asm volatile("strb %1, %0"
> + : "+Qo" (*(volatile u8 __force *)addr)
> + : "r" (val));
> +}
> +
> +static inline void __raw_writel(u32 val, volatile void __iomem *addr)
> +{
> + asm volatile("str %1, %0"
> + : "+Qo" (*(volatile u32 __force *)addr)
> + : "r" (val));
> +}
> +
> +static inline u8 __raw_readb(const volatile void __iomem *addr)
> +{
> + u8 val;
> + asm volatile("ldrb %1, %0"
> + : "+Qo" (*(volatile u8 __force *)addr),
> + "=r" (val));
> + return val;
> +}
> +
> +static inline u32 __raw_readl(const volatile void __iomem *addr)
> +{
> + u32 val;
> + asm volatile("ldr %1, %0"
> + : "+Qo" (*(volatile u32 __force *)addr),
> + "=r" (val));
> + return val;
> +}
> +
> +#define __iormb() rmb()
> +#define __iowmb() wmb()
> +
> +#define readb_relaxed(c) ({ u8 __r = __raw_readb(c); __r; })
> +#define readl_relaxed(c) ({ u32 __r = le32_to_cpu((__force __le32) \
> + __raw_readl(c)); __r; })
> +
> +#define writeb_relaxed(v,c) __raw_writeb(v,c)
> +#define writel_relaxed(v,c) __raw_writel((__force u32) cpu_to_le32(v),c)
> +
> +#define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(); __v; })
> +#define readl(c) ({ u32 __v = readl_relaxed(c); __iormb(); __v; })
> +
> +#define writeb(v,c) ({ __iowmb(); writeb_relaxed(v,c); })
> +#define writel(v,c) ({ __iowmb(); writel_relaxed(v,c); })
> +
> +
> #endif
> /*
> * Local variables:
Theses changes are not in the right place. The asm-arm/io.h must only
contain common code between arm32 and arm64. Your code is arm32
specific, so it should be moved in asm-arm/arm32/io.h.
There is already an implementation for (io)readl and (io)writel with
another name. I will be happy if you rename/alias these 2 functions, it
will avoid ifdef... in ns16550 code.
> diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
> index 9caf776..09ca855 100644
> --- a/xen/include/xen/serial.h
> +++ b/xen/include/xen/serial.h
> @@ -11,6 +11,7 @@
>
> #include <xen/init.h>
> #include <xen/spinlock.h>
> +#include <xen/device_tree.h>
>
> struct cpu_user_regs;
>
> @@ -151,8 +152,10 @@ struct ns16550_defaults {
> int data_bits; /* default data bits (5, 6, 7 or 8) */
> int parity; /* default parity (n, o, e, m or s) */
> int stop_bits; /* default stop bits (1 or 2) */
> - int irq; /* default irq */
> - unsigned long io_base; /* default io_base address */
> + struct dt_irq irq; /* default irq */
This change will break build on x86. This structure is filled by
arch/x86/setup.c
> + u64 io_base; /* default io_base address */
> + u64 io_size;
> + u32 clock_hz; /* UART clock rate */
> };
As I understand, you use these modifications only in ns16550.c to fill
it with device tree value. Considering the number of ifdef you have
added in ns16550_init, I think it's better to clone the function and
directly fill ns16550, instead of ns16550_defaults. Anyone got any other
thoughts?
> void ns16550_init(int index, struct ns16550_defaults *defaults);
> void ehci_dbgp_init(void);
>
Cheers,
--
Julien
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-19 15:18 ` Chen Baozi
@ 2013-07-22 12:59 ` Chen Baozi
2013-07-23 11:47 ` Chen Baozi
0 siblings, 1 reply; 27+ messages in thread
From: Chen Baozi @ 2013-07-22 12:59 UTC (permalink / raw)
To: Ian Campbell; +Cc: Keir Fraser, xen-devel
On Jul 19, 2013, at 11:18 PM, Chen Baozi <baozich@gmail.com> wrote:
> On Fri, Jul 19, 2013 at 10:15 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>>
>> On Fri, 2013-07-19 at 20:57 +0800, Chen Baozi wrote:
>>> Previous bits setting would cause generating THRE interrupts, which cannot
>>> be cleared and make system loop in gic_interrupt endlessly. Set 'received
>>> data available' and 'receiver line status' bits of IER when enabling
>>> interrupts as what Linux 8250 driver does.
>>
>> We do actually want THRE interrupts though, don't we? Otherwise how do
>> we know when we can send more stuff?
>
> I'm not very sure about it. In Linux 8250 driver THRE is not set at this stage.
> And following it does work for OMAP5. I'll try to dig it next to see
> whether this fix is reasonable.
I rechecked the Linux codes today. In Linux, THRE is only enabled during tx
(enable at start_tx and disable at stop_tx) when enabling Modem Status interrupt.
Every time when handling irq, the Linux driver will check the MSR. If Modem
Status interrupt is enabled and delta-CTS flag is set, it will enter the codes
switching THRE status (by start_tx or stop_tx). So at least, I think, THRE should
not be enabled all the time once 8250 driver is initialized. Since Xen UART driver
doesn't seem to be complex as Linux does, I haven't found a proper pointer to insert
"start_tx/stop_tx" logic in Xen UART driver yet. And I think THRE might not be set
at ns16550_setup_postirq actually.
For "how do we know when we can send more stuff?", I think the while loop in
ns16550_interrupt checking UART_LSR_THRE flag can guarantee it. UART_LSR_THRE
is set when the THR (or transmitter FIFO) and the TSR are both empty regardless
of status of THRE flag in IER register.
BTW, I've also re-checked about console_sync and irq type. Neither of them is
the cause of my issue.
Any ideas?
Cheers,
Baozi
>
>>
>> Are you using console_sync? It might mask any issue arising from not
>> getting these interrupts (or maybe even cause your initial issue?)
>
> Yes. I have tried to shut this down to test whether it is the cause of my
> issue. But it seems not.
>
>>
>> The LSI register is only of use if we care about RTS/CTS or something. I
>> don't know if we do, but I'd be surprised if just asking for the
>> interrupt was sufficient (i.e. we'd need to do something with the irq)
>>
>> How did you configure this interrupt line, level or edge trigger? I'd
>> have expected it to need to be edge, and the issue sounds a bit like it
>> might be level.
>
> It is high-level read from the DTS. I've also changed to it to edge trigger
> manually and every other possibilities when debugging. And it seems
> edge trigger doesn't help.
>
> Anyway, some research must be done for this patch and my fixes to the
> issue next.
>
> Thanks.
>
> Baozi
>
>>
>> Ian.
>>
>>> Signed-off-by: Chen Baozi <baozich@gmail.com>
>>> ---
>>> xen/drivers/char/ns16550.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
>>> index 9248330..60512be 100644
>>> --- a/xen/drivers/char/ns16550.c
>>> +++ b/xen/drivers/char/ns16550.c
>>> @@ -237,8 +237,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
>>> ns_write_reg(uart,
>>> UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
>>>
>>> - /* Enable receive and transmit interrupts. */
>>> - ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ETHREI);
>>> + /* Enable receive and line status interrupts. */
>>> + ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ELSI);
>>> }
>>>
>>> if ( uart->irq >= 0 )
>>
>>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/4] xen/arm: enable ns16550 uart driver for OMAP5432
2013-07-19 17:06 ` Julien Grall
@ 2013-07-22 13:06 ` Chen Baozi
0 siblings, 0 replies; 27+ messages in thread
From: Chen Baozi @ 2013-07-22 13:06 UTC (permalink / raw)
To: Julien Grall; +Cc: Keir Fraser, Ian Campbell, xen-devel
On Jul 20, 2013, at 1:06 AM, Julien Grall <julien.grall@citrix.com> wrote:
> On 07/19/2013 01:57 PM, Chen Baozi wrote:
>>
>> Signed-off-by: Chen Baozi <baozich@gmail.com>
>> ---
>> xen/arch/arm/Rules.mk | 1 +
>> xen/drivers/char/ns16550.c | 155 +++++++++++++++++++++++++++++++++++++++------
>> xen/include/asm-arm/io.h | 49 ++++++++++++++
>> xen/include/xen/serial.h | 7 +-
>> 4 files changed, 191 insertions(+), 21 deletions(-)
>>
>> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
>> index eaa03fb..8604e34 100644
>> --- a/xen/arch/arm/Rules.mk
>> +++ b/xen/arch/arm/Rules.mk
>> @@ -9,6 +9,7 @@
>> HAS_DEVICE_TREE := y
>> HAS_VIDEO := y
>> HAS_ARM_HDLCD := y
>> +HAS_NS16550 := y
>>
>> CFLAGS += -fno-builtin -fno-common -Wredundant-decls
>> CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe
>> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
>> index 60512be..b8690eb 100644
>> --- a/xen/drivers/char/ns16550.c
>> +++ b/xen/drivers/char/ns16550.c
>> @@ -1,10 +1,10 @@
>> /******************************************************************************
>> * ns16550.c
>> - *
>> + *
>
> Spurious change.
>> * Driver for 16550-series UARTs. This driver is to be kept within Xen as
>> * it permits debugging of seriously-toasted machines (e.g., in situations
>> * where a device driver within a guest OS would be inaccessible).
>> - *
>> + *
> Spurious change.
>
>> * Copyright (c) 2003-2005, K A Fraser
>> */
>>
>> @@ -25,6 +25,13 @@
>> #include <asm/fixmap.h>
>> #endif
>>
>> +#ifdef CONFIG_ARM
>> +#include <xen/device_tree.h>
>> +#include <xen/errno.h>
>
> The above 2 includes can be moved outside the ifdef.
>
>> +#include <asm/early_printk.h>
>> +#include <asm/device.h>
>> +#endif
>> +
>> /*
>> * Configure serial port with a string:
>> * <baud>[/<clock_hz>][,DPS[,<io-base>[,<irq>[,<port-bdf>[,<bridge-bdf>]]]]].
>> @@ -39,8 +46,11 @@ string_param("com1", opt_com1);
>> string_param("com2", opt_com2);
>>
>> static struct ns16550 {
>> - int baud, clock_hz, data_bits, parity, stop_bits, fifo_size, irq;
>> - unsigned long io_base; /* I/O port or memory-mapped I/O address. */
>> + int baud, data_bits, parity, stop_bits, fifo_size;
>> + u32 clock_hz;
>> + struct dt_irq irq;
>> + u64 io_base; /* I/O port or memory-mapped I/O address. */
>> + u64 io_size;
>> char __iomem *remapped_io_base; /* Remapped virtual address of MMIO. */
>> /* UART with IRQ line: interrupt-driven I/O. */
>> struct irqaction irqaction;
>> @@ -61,16 +71,24 @@ static struct ns16550 {
>>
>> static char ns_read_reg(struct ns16550 *uart, int reg)
>> {
>> +#ifdef CONFIG_X86
>> if ( uart->remapped_io_base == NULL )
>> return inb(uart->io_base + reg);
>> return readb(uart->remapped_io_base + reg);
>> +#else
>> + return readb(uart->remapped_io_base + (reg << 2));
>> +#endif
>> }
>>
>
> Is it possible to add a new private field "shift"? So the both readb can
> be merged.
>
>> static void ns_write_reg(struct ns16550 *uart, int reg, char c)
>> {
>> +#ifdef CONFIG_X86
>> if ( uart->remapped_io_base == NULL )
>> return outb(c, uart->io_base + reg);
>> writeb(c, uart->remapped_io_base + reg);
>> +#else
>> + writeb(c, uart->remapped_io_base + (reg << 2));
>> +#endif
>
> Same here for readb.
>
>> }
>>
>> static void ns16550_interrupt(
>> @@ -145,11 +163,12 @@ static int ns16550_getc(struct serial_port *port, char *pc)
>> return 1;
>> }
>>
>> +#ifdef CONFIG_X86
>> static void pci_serial_early_init(struct ns16550 *uart)
>> {
>> if ( !uart->ps_bdf_enable || uart->io_base >= 0x10000 )
>> return;
>> -
>> +
> Spurious change.
>
>> if ( uart->pb_bdf_enable )
>> pci_conf_write16(0, uart->pb_bdf[0], uart->pb_bdf[1], uart->pb_bdf[2],
>> PCI_IO_BASE,
>> @@ -161,7 +180,13 @@ static void pci_serial_early_init(struct ns16550 *uart)
>> uart->io_base | PCI_BASE_ADDRESS_SPACE_IO);
>> pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1], uart->ps_bdf[2],
>> PCI_COMMAND, PCI_COMMAND_IO);
>> +
>> +}
>> +#else
>> +static void pci_serial_early_init(struct ns16550 *uart)
>> +{
>> }
>> +#endif
>>
>> static void ns16550_setup_preirq(struct ns16550 *uart)
>> {
>> @@ -217,7 +242,9 @@ static void __init ns16550_init_preirq(struct serial_port *port)
>> uart->remapped_io_base = (void __iomem *)fix_to_virt(idx);
>> uart->remapped_io_base += uart->io_base & ~PAGE_MASK;
>> #else
>> - uart->remapped_io_base = (char *)ioremap(uart->io_base, 8);
>> + uart->remapped_io_base = ioremap_attr(uart->io_base,
>> + uart->io_size,
>> + PAGE_HYPERVISOR_NOCACHE);
>> #endif
>> }
>>
>> @@ -231,7 +258,7 @@ static void __init ns16550_init_preirq(struct serial_port *port)
>>
>> static void ns16550_setup_postirq(struct ns16550 *uart)
>> {
>> - if ( uart->irq > 0 )
>> + if ( uart->irq.irq > 0 )
>> {
>> /* Master interrupt enable; also keep DTR/RTS asserted. */
>> ns_write_reg(uart,
>> @@ -241,7 +268,7 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
>> ns_write_reg(uart, UART_IER, UART_IER_ERDAI | UART_IER_ELSI);
>> }
>>
>> - if ( uart->irq >= 0 )
>> + if ( uart->irq.irq >= 0 )
>> set_timer(&uart->timer, NOW() + MILLISECS(uart->timeout_ms));
>> }
>>
>> @@ -250,7 +277,7 @@ static void __init ns16550_init_postirq(struct serial_port *port)
>> struct ns16550 *uart = port->uart;
>> int rc, bits;
>>
>> - if ( uart->irq < 0 )
>> + if ( uart->irq.irq < 0 )
>> return;
>>
>> serial_async_transmit(port);
>> @@ -262,20 +289,26 @@ static void __init ns16550_init_postirq(struct serial_port *port)
>> uart->timeout_ms = max_t(
>> unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
>>
>> - if ( uart->irq > 0 )
>> + if ( uart->irq.irq > 0 )
>> {
>> uart->irqaction.handler = ns16550_interrupt;
>> uart->irqaction.name = "ns16550";
>> uart->irqaction.dev_id = port;
>> - if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
>> - printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq);
>> +#ifdef CONFIG_X86
>> + if ( (rc = setup_irq(uart->irq.irq, &uart->irqaction)) != 0 )
>> +#else
>> + if ( (rc = setup_dt_irq(&uart->irq, &uart->irqaction)) != 0 )
>> +#endif
>> + printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq.irq);
>> }
>>
>> ns16550_setup_postirq(uart);
>>
>> +#ifdef CONFIG_X86
>> if ( uart->bar || uart->ps_bdf_enable )
>> pci_hide_device(uart->ps_bdf[0], PCI_DEVFN(uart->ps_bdf[1],
>> uart->ps_bdf[2]));
>> +#endif
>> }
>>
>> static void ns16550_suspend(struct serial_port *port)
>> @@ -284,13 +317,16 @@ static void ns16550_suspend(struct serial_port *port)
>>
>> stop_timer(&uart->timer);
>>
>> +#ifdef CONFIG_X86
>> if ( uart->bar )
>> uart->cr = pci_conf_read16(0, uart->ps_bdf[0], uart->ps_bdf[1],
>> uart->ps_bdf[2], PCI_COMMAND);
>> +#endif
>> }
>>
>> static void _ns16550_resume(struct serial_port *port)
>> {
>> +#ifdef CONFIG_X86
>> struct ns16550 *uart = port->uart;
>>
>> if ( uart->bar )
>> @@ -300,6 +336,7 @@ static void _ns16550_resume(struct serial_port *port)
>> pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1], uart->ps_bdf[2],
>> PCI_COMMAND, uart->cr);
>> }
>> +#endif
>>
>> ns16550_setup_preirq(port->uart);
>> ns16550_setup_postirq(port->uart);
>> @@ -370,9 +407,17 @@ static void __init ns16550_endboot(struct serial_port *port)
>> static int __init ns16550_irq(struct serial_port *port)
>> {
>> struct ns16550 *uart = port->uart;
>> - return ((uart->irq > 0) ? uart->irq : -1);
>> + return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
>> }
>>
>> +#ifdef CONFIG_ARM
>> +static const struct dt_irq __init *ns16550_dt_irq(struct serial_port *port)
>> +{
>> + struct ns16550 *uart = port->uart;
>> + return &uart->irq;
>> +}
>> +#endif
>
> You don't need to surround this function by ifdef.
>
>> static struct uart_driver __read_mostly ns16550_driver = {
>> .init_preirq = ns16550_init_preirq,
>> .init_postirq = ns16550_init_postirq,
>> @@ -382,7 +427,10 @@ static struct uart_driver __read_mostly ns16550_driver = {
>> .tx_ready = ns16550_tx_ready,
>> .putc = ns16550_putc,
>> .getc = ns16550_getc,
>> - .irq = ns16550_irq
>> + .irq = ns16550_irq,
>> +#ifdef CONFIG_ARM
>> + .dt_irq_get = ns16550_dt_irq
>> +#endif
>> };
>
> same here.
>
>> static int __init parse_parity_char(int c)
>> @@ -403,6 +451,7 @@ static int __init parse_parity_char(int c)
>> return 0;
>> }
>>
>> +#ifdef CONFIG_X86
>> static void __init parse_pci_bdf(const char **conf, unsigned int bdf[3])
>> {
>> bdf[0] = simple_strtoul(*conf, conf, 16);
>> @@ -415,6 +464,7 @@ static void __init parse_pci_bdf(const char **conf, unsigned int bdf[3])
>> (*conf)++;
>> bdf[2] = simple_strtoul(*conf, conf, 16);
>> }
>> +#endif
>>
>> static int __init check_existence(struct ns16550 *uart)
>> {
>> @@ -428,7 +478,7 @@ static int __init check_existence(struct ns16550 *uart)
>> return 1;
>>
>> pci_serial_early_init(uart);
>> -
>> +
>
> spurious change.
>
>> /*
>> * Do a simple existence test first; if we fail this,
>> * there's no point trying anything else.
>> @@ -445,7 +495,7 @@ static int __init check_existence(struct ns16550 *uart)
>> scratch3 = ns_read_reg(uart, UART_IER) & 0x0f;
>> ns_write_reg(uart, UART_IER, scratch);
>> if ( (scratch2 != 0) || (scratch3 != 0x0F) )
>> - return 0;
>> + return 0;
>
> spurious change.
>
>>
>> /*
>> * Check to see if a UART is really there.
>> @@ -456,6 +506,7 @@ static int __init check_existence(struct ns16550 *uart)
>> return (status == 0x90);
>> }
>>
>> +#ifdef CONFIG_X86
>> static int
>> pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
>> {
>> @@ -507,7 +558,7 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
>> uart->bar = bar;
>> uart->bar_idx = bar_idx;
>> uart->io_base = bar & ~PCI_BASE_ADDRESS_SPACE_IO;
>> - uart->irq = pci_conf_read8(0, b, d, f, PCI_INTERRUPT_PIN) ?
>> + uart->irq.irq = pci_conf_read8(0, b, d, f, PCI_INTERRUPT_PIN) ?
>> pci_conf_read8(0, b, d, f, PCI_INTERRUPT_LINE) : 0;
>>
>> return 0;
>> @@ -519,11 +570,12 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
>> return -1;
>>
>> uart->io_base = 0x3f8;
>> - uart->irq = 0;
>> + uart->irq.irq = 0;
>> uart->clock_hz = UART_CLOCK_HZ;
>>
>> return 0;
>> }
>> +#endif
>>
>> #define PARSE_ERR(_f, _a...) \
>> do { \
>> @@ -534,6 +586,7 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
>> static void __init ns16550_parse_port_config(
>> struct ns16550 *uart, const char *conf)
>> {
>> +#ifdef CONFIG_X86
>> int baud;
>>
>> /* No user-specified configuration? */
>> @@ -589,7 +642,7 @@ static void __init ns16550_parse_port_config(
>> }
>>
>> if ( *conf == ',' && *++conf != ',' )
>> - uart->irq = simple_strtol(conf, &conf, 10);
>> + uart->irq.irq = simple_strtol(conf, &conf, 10);
>>
>> if ( *conf == ',' && *++conf != ',' )
>> {
>> @@ -604,6 +657,8 @@ static void __init ns16550_parse_port_config(
>> }
>>
>> config_parsed:
>> +#endif
>> +
>
> You have disabled nearly 80% of the code for ARM. Perhaps you can split
> this function in 2:
> - Parsing pci/user configuration
> - Register the UART
>
>> /* Sanity checks. */
>> if ( (uart->baud != BAUD_AUTO) &&
>> ((uart->baud < 1200) || (uart->baud > 115200)) )
>> @@ -633,18 +688,80 @@ void __init ns16550_init(int index, struct ns16550_defaults *defaults)
>> uart->baud = (defaults->baud ? :
>> console_has((index == 0) ? "com1" : "com2")
>> ? BAUD_AUTO : 0);
>> +#ifdef CONFIG_ARM
>> + uart->clock_hz = defaults->clock_hz;
>> +#else
>> uart->clock_hz = UART_CLOCK_HZ;
>> +#endif
>> uart->data_bits = defaults->data_bits;
>> uart->parity = parse_parity_char(defaults->parity);
>> uart->stop_bits = defaults->stop_bits;
>> uart->irq = defaults->irq;
>> uart->io_base = defaults->io_base;
>> +#ifdef CONFIG_ARM
>> + uart->io_size = defaults->io_size;
>> +#endif
>> +
>> /* Default is no transmit FIFO. */
>> uart->fifo_size = 1;
>>
>> ns16550_parse_port_config(uart, (index == 0) ? opt_com1 : opt_com2);
>> +
>> }
>>
>> +#ifdef CONFIG_ARM
>> +
>> +static int __init ns16550_dt_init(struct dt_device_node *dev,
>> + const void *data)
>> +{
>> + struct ns16550_defaults uart;
>> + int res;
>> + const __be32 *clkspec;
>> +
>> + uart.baud = 115200;
>> + uart.data_bits = 8;
>> + uart.parity = 'n';
>> + uart.stop_bits = 1;
>> +
>> + res = dt_device_get_address(dev, 0, &uart.io_base, &uart.io_size);
>> + if (res) {
>> + early_printk("ns16550: Unable to retrieve the base"
>> + " address of the UART\n");
>> + return res;
>> + }
>> +
>> + res = dt_device_get_irq(dev, 0, &uart.irq);
>> + if (res) {
>> + early_printk("ns16550: Unable to retrieve the IRQ\n");
>> + return res;
>> + }
>> +
>> + clkspec = dt_get_property(dev, "clock-frequency", NULL);
>> + if (clkspec == NULL) {
>> + early_printk("ns16550: Unable to retrieve the clock frequency\n");
>> + return -EINVAL;
>> + }
>> + uart.clock_hz = be32_to_cpup(clkspec);
>> +
>> + ns16550_init(0, &uart);
>> + dt_device_set_used_by(dev, DOMID_XEN);
>> +
>> + return 0;
>> +}
>> +
>> +static const char const *ns16550_dt_compat[] __initdata =
>> +{
>> + "ti,omap4-uart",
>> + NULL
>> +};
>> +
>> +DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
>> + .compatible = ns16550_dt_compat,
>> + .init = ns16550_dt_init,
>> +DT_DEVICE_END
>> +
>> +#endif /* CONFIG_ARM */
>> +
>> /*
>> * Local variables:
>> * mode: C
>> diff --git a/xen/include/asm-arm/io.h b/xen/include/asm-arm/io.h
>> index aea5233..33674bc 100644
>> --- a/xen/include/asm-arm/io.h
>> +++ b/xen/include/asm-arm/io.h
>> @@ -1,6 +1,55 @@
>> #ifndef _ASM_IO_H
>> #define _ASM_IO_H
>>
>> +static inline void __raw_writeb(u8 val, volatile void __iomem *addr)
>> +{
>> + asm volatile("strb %1, %0"
>> + : "+Qo" (*(volatile u8 __force *)addr)
>> + : "r" (val));
>> +}
>> +
>> +static inline void __raw_writel(u32 val, volatile void __iomem *addr)
>> +{
>> + asm volatile("str %1, %0"
>> + : "+Qo" (*(volatile u32 __force *)addr)
>> + : "r" (val));
>> +}
>> +
>> +static inline u8 __raw_readb(const volatile void __iomem *addr)
>> +{
>> + u8 val;
>> + asm volatile("ldrb %1, %0"
>> + : "+Qo" (*(volatile u8 __force *)addr),
>> + "=r" (val));
>> + return val;
>> +}
>> +
>> +static inline u32 __raw_readl(const volatile void __iomem *addr)
>> +{
>> + u32 val;
>> + asm volatile("ldr %1, %0"
>> + : "+Qo" (*(volatile u32 __force *)addr),
>> + "=r" (val));
>> + return val;
>> +}
>> +
>> +#define __iormb() rmb()
>> +#define __iowmb() wmb()
>> +
>> +#define readb_relaxed(c) ({ u8 __r = __raw_readb(c); __r; })
>> +#define readl_relaxed(c) ({ u32 __r = le32_to_cpu((__force __le32) \
>> + __raw_readl(c)); __r; })
>> +
>> +#define writeb_relaxed(v,c) __raw_writeb(v,c)
>> +#define writel_relaxed(v,c) __raw_writel((__force u32) cpu_to_le32(v),c)
>> +
>> +#define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(); __v; })
>> +#define readl(c) ({ u32 __v = readl_relaxed(c); __iormb(); __v; })
>> +
>> +#define writeb(v,c) ({ __iowmb(); writeb_relaxed(v,c); })
>> +#define writel(v,c) ({ __iowmb(); writel_relaxed(v,c); })
>> +
>> +
>> #endif
>> /*
>> * Local variables:
>
> Theses changes are not in the right place. The asm-arm/io.h must only
> contain common code between arm32 and arm64. Your code is arm32
> specific, so it should be moved in asm-arm/arm32/io.h.
>
> There is already an implementation for (io)readl and (io)writel with
> another name. I will be happy if you rename/alias these 2 functions, it
> will avoid ifdef... in ns16550 code.
>
>> diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
>> index 9caf776..09ca855 100644
>> --- a/xen/include/xen/serial.h
>> +++ b/xen/include/xen/serial.h
>> @@ -11,6 +11,7 @@
>>
>> #include <xen/init.h>
>> #include <xen/spinlock.h>
>> +#include <xen/device_tree.h>
>
>>
>> struct cpu_user_regs;
>>
>> @@ -151,8 +152,10 @@ struct ns16550_defaults {
>> int data_bits; /* default data bits (5, 6, 7 or 8) */
>> int parity; /* default parity (n, o, e, m or s) */
>> int stop_bits; /* default stop bits (1 or 2) */
>> - int irq; /* default irq */
>> - unsigned long io_base; /* default io_base address */
>> + struct dt_irq irq; /* default irq */
>
> This change will break build on x86. This structure is filled by
> arch/x86/setup.c
>
>> + u64 io_base; /* default io_base address */
>> + u64 io_size;
>> + u32 clock_hz; /* UART clock rate */
>> };
>
> As I understand, you use these modifications only in ns16550.c to fill
> it with device tree value. Considering the number of ifdef you have
> added in ns16550_init, I think it's better to clone the function and
> directly fill ns16550, instead of ns16550_defaults. Anyone got any other
> thoughts?
>
>> void ns16550_init(int index, struct ns16550_defaults *defaults);
>> void ehci_dbgp_init(void);
>>
>
> Cheers,
>
Hi Julien
Thanks for reviewing. I'll rework my patchset according to your suggestions
and resend it later.
Cheers,
Baozi
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-22 12:59 ` Chen Baozi
@ 2013-07-23 11:47 ` Chen Baozi
2013-07-24 10:04 ` Chen Baozi
0 siblings, 1 reply; 27+ messages in thread
From: Chen Baozi @ 2013-07-23 11:47 UTC (permalink / raw)
To: Ian Campbell; +Cc: Keir Fraser, xen-devel
On Jul 22, 2013, at 8:59 PM, Chen Baozi <baozich@gmail.com> wrote:
>
> On Jul 19, 2013, at 11:18 PM, Chen Baozi <baozich@gmail.com> wrote:
>
>> On Fri, Jul 19, 2013 at 10:15 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>>>
>>> On Fri, 2013-07-19 at 20:57 +0800, Chen Baozi wrote:
>>>> Previous bits setting would cause generating THRE interrupts, which cannot
>>>> be cleared and make system loop in gic_interrupt endlessly. Set 'received
>>>> data available' and 'receiver line status' bits of IER when enabling
>>>> interrupts as what Linux 8250 driver does.
>>>
>>> We do actually want THRE interrupts though, don't we? Otherwise how do
>>> we know when we can send more stuff?
>>
>> I'm not very sure about it. In Linux 8250 driver THRE is not set at this stage.
>> And following it does work for OMAP5. I'll try to dig it next to see
>> whether this fix is reasonable.
>
> I rechecked the Linux codes today. In Linux, THRE is only enabled during tx
> (enable at start_tx and disable at stop_tx) when enabling Modem Status interrupt.
> Every time when handling irq, the Linux driver will check the MSR. If Modem
> Status interrupt is enabled and delta-CTS flag is set, it will enter the codes
> switching THRE status (by start_tx or stop_tx). So at least, I think, THRE should
> not be enabled all the time once 8250 driver is initialized. Since Xen UART driver
> doesn't seem to be complex as Linux does, I haven't found a proper pointer to insert
> "start_tx/stop_tx" logic in Xen UART driver yet. And I think THRE might not be set
> at ns16550_setup_postirq actually.
>
> For "how do we know when we can send more stuff?", I think the while loop in
> ns16550_interrupt checking UART_LSR_THRE flag can guarantee it. UART_LSR_THRE
> is set when the THR (or transmitter FIFO) and the TSR are both empty regardless
> of status of THRE flag in IER register.
>
> BTW, I've also re-checked about console_sync and irq type. Neither of them is
> the cause of my issue.
>
> Any ideas?
According to my test today, when disable sync_console，THRE interrupt is needed
to transmit info. However, enabling it all the time since postirq is not
appropriate because it would keep on interrupting system endless after the tx
is completed.
Baozi
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-23 11:47 ` Chen Baozi
@ 2013-07-24 10:04 ` Chen Baozi
2013-07-25 9:14 ` Chen Baozi
0 siblings, 1 reply; 27+ messages in thread
From: Chen Baozi @ 2013-07-24 10:04 UTC (permalink / raw)
To: Ian Campbell; +Cc: Keir Fraser, xen-devel
On Jul 23, 2013, at 7:47 PM, Chen Baozi <baozich@gmail.com> wrote:
>
> On Jul 22, 2013, at 8:59 PM, Chen Baozi <baozich@gmail.com> wrote:
>
>>
>> On Jul 19, 2013, at 11:18 PM, Chen Baozi <baozich@gmail.com> wrote:
>>
>>> On Fri, Jul 19, 2013 at 10:15 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>>>>
>>>> On Fri, 2013-07-19 at 20:57 +0800, Chen Baozi wrote:
>>>>> Previous bits setting would cause generating THRE interrupts, which cannot
>>>>> be cleared and make system loop in gic_interrupt endlessly. Set 'received
>>>>> data available' and 'receiver line status' bits of IER when enabling
>>>>> interrupts as what Linux 8250 driver does.
>>>>
>>>> We do actually want THRE interrupts though, don't we? Otherwise how do
>>>> we know when we can send more stuff?
>>>
>>> I'm not very sure about it. In Linux 8250 driver THRE is not set at this stage.
>>> And following it does work for OMAP5. I'll try to dig it next to see
>>> whether this fix is reasonable.
>>
>> I rechecked the Linux codes today. In Linux, THRE is only enabled during tx
>> (enable at start_tx and disable at stop_tx) when enabling Modem Status interrupt.
>> Every time when handling irq, the Linux driver will check the MSR. If Modem
>> Status interrupt is enabled and delta-CTS flag is set, it will enter the codes
>> switching THRE status (by start_tx or stop_tx). So at least, I think, THRE should
>> not be enabled all the time once 8250 driver is initialized. Since Xen UART driver
>> doesn't seem to be complex as Linux does, I haven't found a proper pointer to insert
>> "start_tx/stop_tx" logic in Xen UART driver yet. And I think THRE might not be set
>> at ns16550_setup_postirq actually.
>>
>> For "how do we know when we can send more stuff?", I think the while loop in
>> ns16550_interrupt checking UART_LSR_THRE flag can guarantee it. UART_LSR_THRE
>> is set when the THR (or transmitter FIFO) and the TSR are both empty regardless
>> of status of THRE flag in IER register.
>>
>> BTW, I've also re-checked about console_sync and irq type. Neither of them is
>> the cause of my issue.
>>
>> Any ideas?
>
> According to my test today, when disable sync_console，THRE interrupt is needed
> to transmit info. However, enabling it all the time since postirq is not
> appropriate because it would keep on interrupting system endless after the tx
> is completed.
There is a very weird phenomenon of my OMAP5 UART.
According to its manual, when the transmitter FIFO and THRE interrupt are enabled,
transmit interrupts occur as follows:
1. The transmitter holding register interrupt (IIR & 0xf == 0x2) occurs when the
transmit FIFO is empty. The transmit FIFO is cleared (IIR & 0xf == 0x1) when the
THR is written to or the IIR is read.
2. ...
What is weird is that I found the transmit FIFO is not cleared (IIR & 0xf == 0x2)
no matter I write THR or read IIR.
I add the following piece of codes to test in ns16550_interrupt:
+ while (ns_read_reg(hart, UART_IIR) & 0x2) {
+ ns_write_reg(uart, UART_THR, 'a');
+ }
The result shows that it will never end the loop even if I've both written THR
and read IIR.
PS, I re-confirmed about the interrupt type in the manual and the latest dts
description in Linux kernel. It is a level triggered interrupt rather than the
edge.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-24 10:04 ` Chen Baozi
@ 2013-07-25 9:14 ` Chen Baozi
2013-07-25 11:17 ` Julien Grall
0 siblings, 1 reply; 27+ messages in thread
From: Chen Baozi @ 2013-07-25 9:14 UTC (permalink / raw)
To: Ian Campbell, Keir Fraser, <julien.grall@linaro.org>; +Cc: xen-devel
On Jul 24, 2013, at 6:04 PM, Chen Baozi <baozich@gmail.com> wrote:
>
> On Jul 23, 2013, at 7:47 PM, Chen Baozi <baozich@gmail.com> wrote:
>
>>
>> On Jul 22, 2013, at 8:59 PM, Chen Baozi <baozich@gmail.com> wrote:
>>
>>>
>>> On Jul 19, 2013, at 11:18 PM, Chen Baozi <baozich@gmail.com> wrote:
>>>
>>>> On Fri, Jul 19, 2013 at 10:15 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>>>>>
>>>>> On Fri, 2013-07-19 at 20:57 +0800, Chen Baozi wrote:
>>>>>> Previous bits setting would cause generating THRE interrupts, which cannot
>>>>>> be cleared and make system loop in gic_interrupt endlessly. Set 'received
>>>>>> data available' and 'receiver line status' bits of IER when enabling
>>>>>> interrupts as what Linux 8250 driver does.
>>>>>
>>>>> We do actually want THRE interrupts though, don't we? Otherwise how do
>>>>> we know when we can send more stuff?
>>>>
>>>> I'm not very sure about it. In Linux 8250 driver THRE is not set at this stage.
>>>> And following it does work for OMAP5. I'll try to dig it next to see
>>>> whether this fix is reasonable.
>>>
>>> I rechecked the Linux codes today. In Linux, THRE is only enabled during tx
>>> (enable at start_tx and disable at stop_tx) when enabling Modem Status interrupt.
>>> Every time when handling irq, the Linux driver will check the MSR. If Modem
>>> Status interrupt is enabled and delta-CTS flag is set, it will enter the codes
>>> switching THRE status (by start_tx or stop_tx). So at least, I think, THRE should
>>> not be enabled all the time once 8250 driver is initialized. Since Xen UART driver
>>> doesn't seem to be complex as Linux does, I haven't found a proper pointer to insert
>>> "start_tx/stop_tx" logic in Xen UART driver yet. And I think THRE might not be set
>>> at ns16550_setup_postirq actually.
>>>
>>> For "how do we know when we can send more stuff?", I think the while loop in
>>> ns16550_interrupt checking UART_LSR_THRE flag can guarantee it. UART_LSR_THRE
>>> is set when the THR (or transmitter FIFO) and the TSR are both empty regardless
>>> of status of THRE flag in IER register.
>>>
>>> BTW, I've also re-checked about console_sync and irq type. Neither of them is
>>> the cause of my issue.
>>>
>>> Any ideas?
>>
>> According to my test today, when disable sync_console，THRE interrupt is needed
>> to transmit info. However, enabling it all the time since postirq is not
>> appropriate because it would keep on interrupting system endless after the tx
>> is completed.
>
> There is a very weird phenomenon of my OMAP5 UART.
>
> According to its manual, when the transmitter FIFO and THRE interrupt are enabled,
> transmit interrupts occur as follows:
>
> 1. The transmitter holding register interrupt (IIR & 0xf == 0x2) occurs when the
> transmit FIFO is empty. The transmit FIFO is cleared (IIR & 0xf == 0x1) when the
> THR is written to or the IIR is read.
>
> 2. ...
>
> What is weird is that I found the transmit FIFO is not cleared (IIR & 0xf == 0x2)
> no matter I write THR or read IIR.
>
> I add the following piece of codes to test in ns16550_interrupt:
>
> + while (ns_read_reg(hart, UART_IIR) & 0x2) {
> + ns_write_reg(uart, UART_THR, 'a');
> + }
>
> The result shows that it will never end the loop even if I've both written THR
> and read IIR.
According to my test today, this dead loop above only happens in case that I have
loaded both xen-uImage & zImage(dom0). Once I loaded both two images, reset the
board and boot the system with xen-uImage only at the same address, the phenomenon
keeps the same (dead loop). However, if I powered down the board and then start
the system with xen-uImage only, or if I reset the board and load xen-uImage at
the address where zImage should be, this would not happen. Instead, there would
be a bunch of 'a' printed after interrupt enabled. And the following output info
after interrupt enabled would be print as expected. In other word, the while
loop does clear the transmit FIFO.
Interestingly, the phenomenon when IER_ETHREI is not enabled is similar. If both
xen-uImage and zImage are loaded at the right address (zImage is loaded at
the address specified in DTS module@0), some external events need to be sent to
trigger the driver to continue outputting infos.
All the test described above is done without 'sync_console' bootargs.
Besides this test, I noticed that we set a timer to poll UART after enabling
the interrupt. And the timer initialization codes are platform specific, which
should be implemented in xen/arch/arm/platforms/omap5.c for example. However,
I haven't done this yet. (I used to plan to turn to this work after UART porting
has been done.) Is it a possible factor that may cause my issues?
Any ideas?
Cheers,
Baozi
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-25 9:14 ` Chen Baozi
@ 2013-07-25 11:17 ` Julien Grall
2013-07-25 11:31 ` Chen Baozi
0 siblings, 1 reply; 27+ messages in thread
From: Julien Grall @ 2013-07-25 11:17 UTC (permalink / raw)
To: Chen Baozi; +Cc: Keir Fraser, Ian Campbell, xen-devel
On 07/25/2013 10:14 AM, Chen Baozi wrote:
> Besides this test, I noticed that we set a timer to poll UART after enabling
> the interrupt. And the timer initialization codes are platform specific, which
> should be implemented in xen/arch/arm/platforms/omap5.c for example. However,
> I haven't done this yet. (I used to plan to turn to this work after UART porting
> has been done.) Is it a possible factor that may cause my issues?
Xen uses the arch timers to handle the time. On some platform (for
instance the Arndale), it's not enabled by default.
If the omap5 boot with the arch timers enabled, you don't need to
implement the init_time callback.
Do you know if Xen receives timer interrupts?
--
Julien
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-25 11:17 ` Julien Grall
@ 2013-07-25 11:31 ` Chen Baozi
2013-07-25 12:33 ` Julien Grall
0 siblings, 1 reply; 27+ messages in thread
From: Chen Baozi @ 2013-07-25 11:31 UTC (permalink / raw)
To: Julien Grall; +Cc: Keir Fraser, Ian Campbell, xen-devel
On Jul 25, 2013, at 7:17 PM, Julien Grall <julien.grall@linaro.org> wrote:
> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>
>> Besides this test, I noticed that we set a timer to poll UART after enabling
>> the interrupt. And the timer initialization codes are platform specific, which
>> should be implemented in xen/arch/arm/platforms/omap5.c for example. However,
>> I haven't done this yet. (I used to plan to turn to this work after UART porting
>> has been done.) Is it a possible factor that may cause my issues?
>
> Xen uses the arch timers to handle the time. On some platform (for
> instance the Arndale), it's not enabled by default.
>
> If the omap5 boot with the arch timers enabled, you don't need to
> implement the init_time callback.
>
> Do you know if Xen receives timer interrupts?
Any way that I could confirm this?
There is a line of boot message:
"Using generic timer at 0 KHz."
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-25 11:31 ` Chen Baozi
@ 2013-07-25 12:33 ` Julien Grall
2013-07-26 1:14 ` Chen Baozi
0 siblings, 1 reply; 27+ messages in thread
From: Julien Grall @ 2013-07-25 12:33 UTC (permalink / raw)
To: Chen Baozi; +Cc: Keir Fraser, Ian Campbell, xen-devel
On 07/25/2013 12:31 PM, Chen Baozi wrote:
>
> On Jul 25, 2013, at 7:17 PM, Julien Grall <julien.grall@linaro.org> wrote:
>
>> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>>
>>> Besides this test, I noticed that we set a timer to poll UART after enabling
>>> the interrupt. And the timer initialization codes are platform specific, which
>>> should be implemented in xen/arch/arm/platforms/omap5.c for example. However,
>>> I haven't done this yet. (I used to plan to turn to this work after UART porting
>>> has been done.) Is it a possible factor that may cause my issues?
>>
>> Xen uses the arch timers to handle the time. On some platform (for
>> instance the Arndale), it's not enabled by default.
>>
>> If the omap5 boot with the arch timers enabled, you don't need to
>> implement the init_time callback.
>>
>> Do you know if Xen receives timer interrupts?
>
> Any way that I could confirm this?
>
> There is a line of boot message:
>
> "Using generic timer at 0 KHz."
>
It seems the arch timer is not configured/enabled. Do you know if U-boot
enables it?
It's not clear to me, how you need to implement init_time. Do you have a
datasheet with the board? If yes, is there a section for the section timer?
Perhaps the good start is linux/arch/arm/mach-omap2/timer.c.
--
Julien
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-25 12:33 ` Julien Grall
@ 2013-07-26 1:14 ` Chen Baozi
2013-07-26 3:31 ` Chen Baozi
0 siblings, 1 reply; 27+ messages in thread
From: Chen Baozi @ 2013-07-26 1:14 UTC (permalink / raw)
To: Julien Grall; +Cc: Keir Fraser, Ian Campbell, xen-devel
On Jul 25, 2013, at 8:33 PM, Julien Grall <julien.grall@linaro.org> wrote:
> On 07/25/2013 12:31 PM, Chen Baozi wrote:
>>
>> On Jul 25, 2013, at 7:17 PM, Julien Grall <julien.grall@linaro.org> wrote:
>>
>>> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>>>
>>>> Besides this test, I noticed that we set a timer to poll UART after enabling
>>>> the interrupt. And the timer initialization codes are platform specific, which
>>>> should be implemented in xen/arch/arm/platforms/omap5.c for example. However,
>>>> I haven't done this yet. (I used to plan to turn to this work after UART porting
>>>> has been done.) Is it a possible factor that may cause my issues?
>>>
>>> Xen uses the arch timers to handle the time. On some platform (for
>>> instance the Arndale), it's not enabled by default.
>>>
>>> If the omap5 boot with the arch timers enabled, you don't need to
>>> implement the init_time callback.
>>>
>>> Do you know if Xen receives timer interrupts?
>>
>> Any way that I could confirm this?
>>
>> There is a line of boot message:
>>
>> "Using generic timer at 0 KHz."
>>
>
> It seems the arch timer is not configured/enabled. Do you know if U-boot
> enables it?
I could see clocks.c/clocks.o in its u-boot sources, but I don't think U-boot has already enabled it, for I could read such lines from Linux kernel boot messages:
"""
OMAP clockevent source: GPTIMER1 at 32768 Hz
sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999s
OMAP clocksource: 32k_counter at 32768 Hz
arch_timer: No interrupt available, giving up
omap5_realtime_timer_init: arch_timer_register failed -22
>
> It's not clear to me, how you need to implement init_time. Do you have a
> datasheet with the board? If yes, is there a section for the section timer?
Yes, TI has published an 49.6MB Technical Reference Manual online with 88 pages section about timer.
>
> Perhaps the good start is linux/arch/arm/mach-omap2/timer.c.
Thanks a lot, it should be helpful!
Baozi
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-26 1:14 ` Chen Baozi
@ 2013-07-26 3:31 ` Chen Baozi
2013-07-26 9:30 ` Chen Baozi
2013-07-26 10:42 ` Julien Grall
0 siblings, 2 replies; 27+ messages in thread
From: Chen Baozi @ 2013-07-26 3:31 UTC (permalink / raw)
To: Julien Grall; +Cc: Keir Fraser, Ian Campbell, xen-devel
On Jul 26, 2013, at 9:14 AM, Chen Baozi <baozich@gmail.com> wrote:
>
> On Jul 25, 2013, at 8:33 PM, Julien Grall <julien.grall@linaro.org> wrote:
>
>> On 07/25/2013 12:31 PM, Chen Baozi wrote:
>>>
>>> On Jul 25, 2013, at 7:17 PM, Julien Grall <julien.grall@linaro.org> wrote:
>>>
>>>> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>>>>
>>>>> Besides this test, I noticed that we set a timer to poll UART after enabling
>>>>> the interrupt. And the timer initialization codes are platform specific, which
>>>>> should be implemented in xen/arch/arm/platforms/omap5.c for example. However,
>>>>> I haven't done this yet. (I used to plan to turn to this work after UART porting
>>>>> has been done.) Is it a possible factor that may cause my issues?
>>>>
>>>> Xen uses the arch timers to handle the time. On some platform (for
>>>> instance the Arndale), it's not enabled by default.
>>>>
>>>> If the omap5 boot with the arch timers enabled, you don't need to
>>>> implement the init_time callback.
>>>>
>>>> Do you know if Xen receives timer interrupts?
>>>
>>> Any way that I could confirm this?
>>>
>>> There is a line of boot message:
>>>
>>> "Using generic timer at 0 KHz."
>>>
>>
>> It seems the arch timer is not configured/enabled. Do you know if U-boot
>> enables it?
>
> I could see clocks.c/clocks.o in its u-boot sources, but I don't think U-boot has already enabled it, for I could read such lines from Linux kernel boot messages:
>
> """
> OMAP clockevent source: GPTIMER1 at 32768 Hz
> sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999s
> OMAP clocksource: 32k_counter at 32768 Hz
> arch_timer: No interrupt available, giving up
> omap5_realtime_timer_init: arch_timer_register failed -22
>
>>
>> It's not clear to me, how you need to implement init_time. Do you have a
>> datasheet with the board? If yes, is there a section for the section timer?
>
> Yes, TI has published an 49.6MB Technical Reference Manual online with 88 pages section about timer.
Wait. I think I might make a mistake. The "arch timer" is different from "timer" as devices, right? The timer section in Technical Reference Manual seems to be the latter one, for it is connect to the Level-4 interconnect of OMAP5432 and described as "ti,omap5430-timer" in DTS. I think the "arch timer" you mentioned should refer to "arm,armv7-timer", right?
>
>>
>> Perhaps the good start is linux/arch/arm/mach-omap2/timer.c.
>
> Thanks a lot, it should be helpful!
>
> Baozi
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-26 3:31 ` Chen Baozi
@ 2013-07-26 9:30 ` Chen Baozi
2013-07-26 10:42 ` Julien Grall
1 sibling, 0 replies; 27+ messages in thread
From: Chen Baozi @ 2013-07-26 9:30 UTC (permalink / raw)
To: Julien Grall; +Cc: Keir Fraser, Ian Campbell, xen-devel
On Jul 26, 2013, at 11:31 AM, Chen Baozi <baozich@gmail.com> wrote:
>
> On Jul 26, 2013, at 9:14 AM, Chen Baozi <baozich@gmail.com> wrote:
>
>>
>> On Jul 25, 2013, at 8:33 PM, Julien Grall <julien.grall@linaro.org> wrote:
>>
>>> On 07/25/2013 12:31 PM, Chen Baozi wrote:
>>>>
>>>> On Jul 25, 2013, at 7:17 PM, Julien Grall <julien.grall@linaro.org> wrote:
>>>>
>>>>> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>>>>>
>>>>>> Besides this test, I noticed that we set a timer to poll UART after enabling
>>>>>> the interrupt. And the timer initialization codes are platform specific, which
>>>>>> should be implemented in xen/arch/arm/platforms/omap5.c for example. However,
>>>>>> I haven't done this yet. (I used to plan to turn to this work after UART porting
>>>>>> has been done.) Is it a possible factor that may cause my issues?
>>>>>
>>>>> Xen uses the arch timers to handle the time. On some platform (for
>>>>> instance the Arndale), it's not enabled by default.
>>>>>
>>>>> If the omap5 boot with the arch timers enabled, you don't need to
>>>>> implement the init_time callback.
>>>>>
>>>>> Do you know if Xen receives timer interrupts?
>>>>
>>>> Any way that I could confirm this?
>>>>
>>>> There is a line of boot message:
>>>>
>>>> "Using generic timer at 0 KHz."
>>>>
>>>
>>> It seems the arch timer is not configured/enabled. Do you know if U-boot
>>> enables it?
>>
>> I could see clocks.c/clocks.o in its u-boot sources, but I don't think U-boot has already enabled it, for I could read such lines from Linux kernel boot messages:
>>
>> """
>> OMAP clockevent source: GPTIMER1 at 32768 Hz
>> sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999s
>> OMAP clocksource: 32k_counter at 32768 Hz
>> arch_timer: No interrupt available, giving up
>> omap5_realtime_timer_init: arch_timer_register failed -22
>>
>>>
>>> It's not clear to me, how you need to implement init_time. Do you have a
>>> datasheet with the board? If yes, is there a section for the section timer?
>>
>> Yes, TI has published an 49.6MB Technical Reference Manual online with 88 pages section about timer.
>
> Wait. I think I might make a mistake. The "arch timer" is different from "timer" as devices, right? The timer section in Technical Reference Manual seems to be the latter one, for it is connect to the Level-4 interconnect of OMAP5432 and described as "ti,omap5430-timer" in DTS. I think the "arch timer" you mentioned should refer to "arm,armv7-timer", right?
According to my understanding, the "arch timer" refers to the Generic Timer inside the Cortex-A15 MPCore. And it is configured through the memory-mapped interface. Right?
>
>>
>>>
>>> Perhaps the good start is linux/arch/arm/mach-omap2/timer.c.
>>
>> Thanks a lot, it should be helpful!
>>
>> Baozi
>>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-26 3:31 ` Chen Baozi
2013-07-26 9:30 ` Chen Baozi
@ 2013-07-26 10:42 ` Julien Grall
2013-07-26 11:01 ` Chen Baozi
1 sibling, 1 reply; 27+ messages in thread
From: Julien Grall @ 2013-07-26 10:42 UTC (permalink / raw)
To: Chen Baozi; +Cc: Keir Fraser, Ian Campbell, xen-devel
On 07/26/2013 04:31 AM, Chen Baozi wrote:
>
> On Jul 26, 2013, at 9:14 AM, Chen Baozi <baozich@gmail.com> wrote:
>
>>
>> On Jul 25, 2013, at 8:33 PM, Julien Grall <julien.grall@linaro.org> wrote:
>>
>>> On 07/25/2013 12:31 PM, Chen Baozi wrote:
>>>>
>>>> On Jul 25, 2013, at 7:17 PM, Julien Grall <julien.grall@linaro.org> wrote:
>>>>
>>>>> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>>>>>
>>>>>> Besides this test, I noticed that we set a timer to poll UART after enabling
>>>>>> the interrupt. And the timer initialization codes are platform specific, which
>>>>>> should be implemented in xen/arch/arm/platforms/omap5.c for example. However,
>>>>>> I haven't done this yet. (I used to plan to turn to this work after UART porting
>>>>>> has been done.) Is it a possible factor that may cause my issues?
>>>>>
>>>>> Xen uses the arch timers to handle the time. On some platform (for
>>>>> instance the Arndale), it's not enabled by default.
>>>>>
>>>>> If the omap5 boot with the arch timers enabled, you don't need to
>>>>> implement the init_time callback.
>>>>>
>>>>> Do you know if Xen receives timer interrupts?
>>>>
>>>> Any way that I could confirm this?
>>>>
>>>> There is a line of boot message:
>>>>
>>>> "Using generic timer at 0 KHz."
>>>>
>>>
>>> It seems the arch timer is not configured/enabled. Do you know if U-boot
>>> enables it?
>>
>> I could see clocks.c/clocks.o in its u-boot sources, but I don't think U-boot has already enabled it, for I could read such lines from Linux kernel boot messages:
Are you trying to boot Linux on bare metal or as dom0?
>> """
>> OMAP clockevent source: GPTIMER1 at 32768 Hz
>> sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999s
>> OMAP clocksource: 32k_counter at 32768 Hz
>> arch_timer: No interrupt available, giving up
Do you have a "arm,armv7-timer" compatible node in your DTS with
interrupts? Here Linux can't find an appropriate interrupt.
>> omap5_realtime_timer_init: arch_timer_register failed -22
>>
>>>
>>> It's not clear to me, how you need to implement init_time. Do you have a
>>> datasheet with the board? If yes, is there a section for the section timer?
>>
>> Yes, TI has published an 49.6MB Technical Reference Manual online with 88 pages section about timer.
>
> Wait. I think I might make a mistake. The "arch timer" is different from "timer" as devices, right? The timer section in Technical Reference Manual seems to be the latter one, for it is connect to the Level-4 interconnect of OMAP5432 and described as "ti,omap5430-timer" in DTS. I think the "arch timer" you mentioned should refer to "arm,armv7-timer", right?
By "arch timer" I mean "arm,armv7-timer".
--
Julien
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-26 10:42 ` Julien Grall
@ 2013-07-26 11:01 ` Chen Baozi
2013-07-26 12:01 ` Julien Grall
0 siblings, 1 reply; 27+ messages in thread
From: Chen Baozi @ 2013-07-26 11:01 UTC (permalink / raw)
To: Julien Grall; +Cc: Keir Fraser, Ian Campbell, xen-devel
On Jul 26, 2013, at 6:42 PM, Julien Grall <julien.grall@linaro.org> wrote:
> On 07/26/2013 04:31 AM, Chen Baozi wrote:
>>
>> On Jul 26, 2013, at 9:14 AM, Chen Baozi <baozich@gmail.com> wrote:
>>
>>>
>>> On Jul 25, 2013, at 8:33 PM, Julien Grall <julien.grall@linaro.org> wrote:
>>>
>>>> On 07/25/2013 12:31 PM, Chen Baozi wrote:
>>>>>
>>>>> On Jul 25, 2013, at 7:17 PM, Julien Grall <julien.grall@linaro.org> wrote:
>>>>>
>>>>>> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>>>>>>
>>>>>>> Besides this test, I noticed that we set a timer to poll UART after enabling
>>>>>>> the interrupt. And the timer initialization codes are platform specific, which
>>>>>>> should be implemented in xen/arch/arm/platforms/omap5.c for example. However,
>>>>>>> I haven't done this yet. (I used to plan to turn to this work after UART porting
>>>>>>> has been done.) Is it a possible factor that may cause my issues?
>>>>>>
>>>>>> Xen uses the arch timers to handle the time. On some platform (for
>>>>>> instance the Arndale), it's not enabled by default.
>>>>>>
>>>>>> If the omap5 boot with the arch timers enabled, you don't need to
>>>>>> implement the init_time callback.
>>>>>>
>>>>>> Do you know if Xen receives timer interrupts?
>>>>>
>>>>> Any way that I could confirm this?
>>>>>
>>>>> There is a line of boot message:
>>>>>
>>>>> "Using generic timer at 0 KHz."
>>>>>
>>>>
>>>> It seems the arch timer is not configured/enabled. Do you know if U-boot
>>>> enables it?
>>>
>>> I could see clocks.c/clocks.o in its u-boot sources, but I don't think U-boot has already enabled it, for I could read such lines from Linux kernel boot messages:
>
> Are you trying to boot Linux on bare metal or as dom0?
bare metal.
>
>>> """
>>> OMAP clockevent source: GPTIMER1 at 32768 Hz
>>> sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999s
>>> OMAP clocksource: 32k_counter at 32768 Hz
>>> arch_timer: No interrupt available, giving up
>
> Do you have a "arm,armv7-timer" compatible node in your DTS with
> interrupts? Here Linux can't find an appropriate interrupt.
You reminds me that the DTS shipped along with the board has issues on "arm,armv7-timer". I have corrected it for xen but leave the bare metal booting dtb the original status.
>
>>> omap5_realtime_timer_init: arch_timer_register failed -22
>>>
>>>>
>>>> It's not clear to me, how you need to implement init_time. Do you have a
>>>> datasheet with the board? If yes, is there a section for the section timer?
>>>
>>> Yes, TI has published an 49.6MB Technical Reference Manual online with 88 pages section about timer.
>>
>> Wait. I think I might make a mistake. The "arch timer" is different from "timer" as devices, right? The timer section in Technical Reference Manual seems to be the latter one, for it is connect to the Level-4 interconnect of OMAP5432 and described as "ti,omap5430-timer" in DTS. I think the "arch timer" you mentioned should refer to "arm,armv7-timer", right?
>
> By "arch timer" I mean "arm,armv7-timer".
Thanks. So I think the codes to start with it is linux/drivers/clocksource/arm_arch_timer.c: arch_timer_init(). The linux/arch/arm/mach-omap2/timer.c is for the timer device "ti,omap543-timer".
Again, Thanks a lot.
Baozi
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-26 11:01 ` Chen Baozi
@ 2013-07-26 12:01 ` Julien Grall
2013-07-26 12:32 ` Chen Baozi
0 siblings, 1 reply; 27+ messages in thread
From: Julien Grall @ 2013-07-26 12:01 UTC (permalink / raw)
To: Chen Baozi; +Cc: Keir Fraser, Ian Campbell, xen-devel
On 07/26/2013 12:01 PM, Chen Baozi wrote:
>
> On Jul 26, 2013, at 6:42 PM, Julien Grall <julien.grall@linaro.org> wrote:
>
>> On 07/26/2013 04:31 AM, Chen Baozi wrote:
>>>
>>> On Jul 26, 2013, at 9:14 AM, Chen Baozi <baozich@gmail.com> wrote:
>>>
>>>>
>>>> On Jul 25, 2013, at 8:33 PM, Julien Grall <julien.grall@linaro.org> wrote:
>>>>
>>>>> On 07/25/2013 12:31 PM, Chen Baozi wrote:
>>>>>>
>>>>>> On Jul 25, 2013, at 7:17 PM, Julien Grall <julien.grall@linaro.org> wrote:
>>>>>>
>>>>>>> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>>>>>>>
>>>>>>>> Besides this test, I noticed that we set a timer to poll UART after enabling
>>>>>>>> the interrupt. And the timer initialization codes are platform specific, which
>>>>>>>> should be implemented in xen/arch/arm/platforms/omap5.c for example. However,
>>>>>>>> I haven't done this yet. (I used to plan to turn to this work after UART porting
>>>>>>>> has been done.) Is it a possible factor that may cause my issues?
>>>>>>>
>>>>>>> Xen uses the arch timers to handle the time. On some platform (for
>>>>>>> instance the Arndale), it's not enabled by default.
>>>>>>>
>>>>>>> If the omap5 boot with the arch timers enabled, you don't need to
>>>>>>> implement the init_time callback.
>>>>>>>
>>>>>>> Do you know if Xen receives timer interrupts?
>>>>>>
>>>>>> Any way that I could confirm this?
>>>>>>
>>>>>> There is a line of boot message:
>>>>>>
>>>>>> "Using generic timer at 0 KHz."
>>>>>>
>>>>>
>>>>> It seems the arch timer is not configured/enabled. Do you know if U-boot
>>>>> enables it?
>>>>
>>>> I could see clocks.c/clocks.o in its u-boot sources, but I don't think U-boot has already enabled it, for I could read such lines from Linux kernel boot messages:
>>
>> Are you trying to boot Linux on bare metal or as dom0?
>
> bare metal.
>
>>
>>>> """
>>>> OMAP clockevent source: GPTIMER1 at 32768 Hz
>>>> sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999s
>>>> OMAP clocksource: 32k_counter at 32768 Hz
>>>> arch_timer: No interrupt available, giving up
>>
>> Do you have a "arm,armv7-timer" compatible node in your DTS with
>> interrupts? Here Linux can't find an appropriate interrupt.
>
> You reminds me that the DTS shipped along with the board has issues on "arm,armv7-timer". I have corrected it for xen but leave the bare metal booting dtb the original status.
>
>>
>>>> omap5_realtime_timer_init: arch_timer_register failed -22
>>>>
>>>>>
>>>>> It's not clear to me, how you need to implement init_time. Do you have a
>>>>> datasheet with the board? If yes, is there a section for the section timer?
>>>>
>>>> Yes, TI has published an 49.6MB Technical Reference Manual online with 88 pages section about timer.
>>>
>>> Wait. I think I might make a mistake. The "arch timer" is different from "timer" as devices, right? The timer section in Technical Reference Manual seems to be the latter one, for it is connect to the Level-4 interconnect of OMAP5432 and described as "ti,omap5430-timer" in DTS. I think the "arch timer" you mentioned should refer to "arm,armv7-timer", right?
>>
>> By "arch timer" I mean "arm,armv7-timer".
>
> Thanks. So I think the codes to start with it is linux/drivers/clocksource/arm_arch_timer.c: arch_timer_init(). The linux/arch/arm/mach-omap2/timer.c is for the timer device "ti,omap543-timer".
arch_timer_init is already implemented in init_xen_time
(xen/arch/arm/time.c).
On some board, the arch timer is not enabled by default and need a
"workaround" for this purpose.
For instance on the Arndale, we need to enable the MCTimer. I think you
need to do something like that for the OMAP.
Do you know if the boot CPU already start in HYP mode? If no, did you
set up correctly the timer in mode_switch.S?
--
Julien
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250
2013-07-26 12:01 ` Julien Grall
@ 2013-07-26 12:32 ` Chen Baozi
0 siblings, 0 replies; 27+ messages in thread
From: Chen Baozi @ 2013-07-26 12:32 UTC (permalink / raw)
To: Julien Grall; +Cc: Keir Fraser, Ian Campbell, xen-devel@lists.xen.org
在 2013-7-26，20:01，Julien Grall <julien.grall@linaro.org> 写道：
> On 07/26/2013 12:01 PM, Chen Baozi wrote:
>>
>> On Jul 26, 2013, at 6:42 PM, Julien Grall <julien.grall@linaro.org> wrote:
>>
>>> On 07/26/2013 04:31 AM, Chen Baozi wrote:
>>>>
>>>> On Jul 26, 2013, at 9:14 AM, Chen Baozi <baozich@gmail.com> wrote:
>>>>
>>>>>
>>>>> On Jul 25, 2013, at 8:33 PM, Julien Grall <julien.grall@linaro.org> wrote:
>>>>>
>>>>>> On 07/25/2013 12:31 PM, Chen Baozi wrote:
>>>>>>>
>>>>>>> On Jul 25, 2013, at 7:17 PM, Julien Grall <julien.grall@linaro.org> wrote:
>>>>>>>
>>>>>>>> On 07/25/2013 10:14 AM, Chen Baozi wrote:
>>>>>>>>
>>>>>>>>> Besides this test, I noticed that we set a timer to poll UART after enabling
>>>>>>>>> the interrupt. And the timer initialization codes are platform specific, which
>>>>>>>>> should be implemented in xen/arch/arm/platforms/omap5.c for example. However,
>>>>>>>>> I haven't done this yet. (I used to plan to turn to this work after UART porting
>>>>>>>>> has been done.) Is it a possible factor that may cause my issues?
>>>>>>>>
>>>>>>>> Xen uses the arch timers to handle the time. On some platform (for
>>>>>>>> instance the Arndale), it's not enabled by default.
>>>>>>>>
>>>>>>>> If the omap5 boot with the arch timers enabled, you don't need to
>>>>>>>> implement the init_time callback.
>>>>>>>>
>>>>>>>> Do you know if Xen receives timer interrupts?
>>>>>>>
>>>>>>> Any way that I could confirm this?
>>>>>>>
>>>>>>> There is a line of boot message:
>>>>>>>
>>>>>>> "Using generic timer at 0 KHz."
>>>>>>
>>>>>> It seems the arch timer is not configured/enabled. Do you know if U-boot
>>>>>> enables it?
>>>>>
>>>>> I could see clocks.c/clocks.o in its u-boot sources, but I don't think U-boot has already enabled it, for I could read such lines from Linux kernel boot messages:
>>>
>>> Are you trying to boot Linux on bare metal or as dom0?
>>
>> bare metal.
>>
>>>
>>>>> """
>>>>> OMAP clockevent source: GPTIMER1 at 32768 Hz
>>>>> sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999s
>>>>> OMAP clocksource: 32k_counter at 32768 Hz
>>>>> arch_timer: No interrupt available, giving up
>>>
>>> Do you have a "arm,armv7-timer" compatible node in your DTS with
>>> interrupts? Here Linux can't find an appropriate interrupt.
>>
>> You reminds me that the DTS shipped along with the board has issues on "arm,armv7-timer". I have corrected it for xen but leave the bare metal booting dtb the original status.
>>
>>>
>>>>> omap5_realtime_timer_init: arch_timer_register failed -22
>>>>>
>>>>>>
>>>>>> It's not clear to me, how you need to implement init_time. Do you have a
>>>>>> datasheet with the board? If yes, is there a section for the section timer?
>>>>>
>>>>> Yes, TI has published an 49.6MB Technical Reference Manual online with 88 pages section about timer.
>>>>
>>>> Wait. I think I might make a mistake. The "arch timer" is different from "timer" as devices, right? The timer section in Technical Reference Manual seems to be the latter one, for it is connect to the Level-4 interconnect of OMAP5432 and described as "ti,omap5430-timer" in DTS. I think the "arch timer" you mentioned should refer to "arm,armv7-timer", right?
>>>
>>> By "arch timer" I mean "arm,armv7-timer".
>>
>> Thanks. So I think the codes to start with it is linux/drivers/clocksource/arm_arch_timer.c: arch_timer_init(). The linux/arch/arm/mach-omap2/timer.c is for the timer device "ti,omap543-timer".
>
> arch_timer_init is already implemented in init_xen_time
> (xen/arch/arm/time.c).
> On some board, the arch timer is not enabled by default and need a
> "workaround" for this purpose.
>
> For instance on the Arndale, we need to enable the MCTimer. I think you
> need to do something like that for the OMAP.
>
> Do you know if the boot CPU already start in HYP mode? If no, did you
> set up correctly the timer in mode_switch.S?
I have patched the u-boot. The boot CPU now starts in HYP mode.
Cheers,
Baozi
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2013-07-26 12:32 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-19 12:57 [PATCH 0/4] Support 8250 compatible UART for OMAP5432 Chen Baozi
2013-07-19 12:57 ` [PATCH 1/4] xen: extract register definitions from ns16550 into a separated header Chen Baozi
2013-07-19 15:35 ` Julien Grall
2013-07-19 15:37 ` Chen Baozi
2013-07-19 15:43 ` Julien Grall
2013-07-19 12:57 ` [PATCH 2/4] xen/arm: add OMAP5432 UART support for early_printk Chen Baozi
2013-07-19 15:49 ` Julien Grall
2013-07-19 12:57 ` [PATCH 3/4] xen: set the right flags when enabling interrupts for 8250 Chen Baozi
2013-07-19 14:15 ` Ian Campbell
2013-07-19 15:18 ` Chen Baozi
2013-07-22 12:59 ` Chen Baozi
2013-07-23 11:47 ` Chen Baozi
2013-07-24 10:04 ` Chen Baozi
2013-07-25 9:14 ` Chen Baozi
2013-07-25 11:17 ` Julien Grall
2013-07-25 11:31 ` Chen Baozi
2013-07-25 12:33 ` Julien Grall
2013-07-26 1:14 ` Chen Baozi
2013-07-26 3:31 ` Chen Baozi
2013-07-26 9:30 ` Chen Baozi
2013-07-26 10:42 ` Julien Grall
2013-07-26 11:01 ` Chen Baozi
2013-07-26 12:01 ` Julien Grall
2013-07-26 12:32 ` Chen Baozi
2013-07-19 12:57 ` [PATCH 4/4] xen/arm: enable ns16550 uart driver for OMAP5432 Chen Baozi
2013-07-19 17:06 ` Julien Grall
2013-07-22 13:06 ` Chen Baozi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).