[PATCH v7] 8250-core based serial driver for OMAP + DMA

[PATCH v7] 8250-core based serial driver for OMAP + DMA

[Sebastian Andrzej Siewior]

This is my complete queue fo the omap serial driver based on the 8250 core
code. I played with it on beagle bone, am335x-evm and dra7xx including DMA.
The uncertain remain the runtime-pm pieces.
I hacked a small serial testing application which sent 10x 4KiB of data in
raw mode. The number of interrupts in comparison:

    serial-omap | 8250 omap | 8250 omap + dma |
   --------------------------------------------
TX |       2558 |       641 |         0 +  30 |
RX |      40960 |       854 |         1 + 853 |

So the 8250 version uses less interrupts for the same amount of data.
The consequence is that in TX mode there should be "short" periods where
no data is sent (before the CPU gets to re-fill the FIFO). On RX we have
a smaller time frame where we have to start to purge the FIFO before it
overflows.

Sebastian

[PATCH 01/15] tty: serial: 8250_core: allow to overwrite & export serial8250_startup()

[Sebastian Andrzej Siewior]

The OMAP version of the 8250 can actually use 1:1 serial8250_startup().
However it needs to be extended by a wake up irq which should to be
requested & enabled at ->startup() time and disabled at ->shutdown() time.

v2…v3: properly copy callbacks
v1…v2: add shutdown callback

Acked-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/tty/serial/8250/8250_core.c | 26 ++++++++++++++++++++++++--
 include/linux/serial_8250.h         |  2 ++
 include/linux/serial_core.h         |  2 ++
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index e2ad13a..dde1331 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1929,7 +1929,7 @@ static void serial8250_put_poll_char(struct uart_port *port,
 
 #endif /* CONFIG_CONSOLE_POLL */
 
-static int serial8250_startup(struct uart_port *port)
+int serial8250_do_startup(struct uart_port *port)
 {
 	struct uart_8250_port *up = up_to_u8250p(port);
 	unsigned long flags;
@@ -2180,8 +2180,17 @@ static int serial8250_startup(struct uart_port *port)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(serial8250_do_startup);
 
-static void serial8250_shutdown(struct uart_port *port)
+static int serial8250_startup(struct uart_port *port)
+{
+	if (port->startup)
+		return port->startup(port);
+	else
+		return serial8250_do_startup(port);
+}
+
+void serial8250_do_shutdown(struct uart_port *port)
 {
 	struct uart_8250_port *up = up_to_u8250p(port);
 	unsigned long flags;
@@ -2231,6 +2240,15 @@ static void serial8250_shutdown(struct uart_port *port)
 	if (port->irq)
 		serial_unlink_irq_chain(up);
 }
+EXPORT_SYMBOL_GPL(serial8250_do_shutdown);
+
+static void serial8250_shutdown(struct uart_port *port)
+{
+	if (port->shutdown)
+		port->shutdown(port);
+	else
+		serial8250_do_shutdown(port);
+}
 
 static unsigned int serial8250_get_divisor(struct uart_port *port, unsigned int baud)
 {
@@ -3428,6 +3446,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
 		/*  Possibly override set_termios call */
 		if (up->port.set_termios)
 			uart->port.set_termios = up->port.set_termios;
+		if (up->port.startup)
+			uart->port.startup = up->port.startup;
+		if (up->port.shutdown)
+			uart->port.shutdown = up->port.shutdown;
 		if (up->port.pm)
 			uart->port.pm = up->port.pm;
 		if (up->port.handle_break)
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index f93649e..c3aa004 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -121,6 +121,8 @@ extern void serial8250_early_out(struct uart_port *port, int offset, int value);
 extern int setup_early_serial8250_console(char *cmdline);
 extern void serial8250_do_set_termios(struct uart_port *port,
 		struct ktermios *termios, struct ktermios *old);
+extern int serial8250_do_startup(struct uart_port *port);
+extern void serial8250_do_shutdown(struct uart_port *port);
 extern void serial8250_do_pm(struct uart_port *port, unsigned int state,
 			     unsigned int oldstate);
 extern int fsl8250_handle_irq(struct uart_port *port);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index cf3a1e7..f3ea531 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -122,6 +122,8 @@ struct uart_port {
 	void			(*set_termios)(struct uart_port *,
 				               struct ktermios *new,
 				               struct ktermios *old);
+	int			(*startup)(struct uart_port *port);
+	void			(*shutdown)(struct uart_port *port);
 	int			(*handle_irq)(struct uart_port *);
 	void			(*pm)(struct uart_port *, unsigned int state,
 				      unsigned int old);
-- 
2.0.1

[PATCH 02/15] tty: serial: 8250_core: allow to set ->throttle / ->unthrottle callbacks

[Sebastian Andrzej Siewior]

The OMAP UART provides support for HW assisted flow control. What is
missing is the support to throttle / unthrottle callbacks which are used
by the omap-serial driver at the moment.
This patch adds the callbacks. It should be safe to add them since they
are only invoked from the serial_core (uart_throttle()) if the feature
flags are set.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/tty/serial/8250/8250_core.c | 14 ++++++++++++++
 include/linux/serial_core.h         |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index dde1331..35fe96e 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1318,6 +1318,16 @@ static void serial8250_start_tx(struct uart_port *port)
 	}
 }
 
+static void serial8250_throttle(struct uart_port *port)
+{
+	port->throttle(port);
+}
+
+static void serial8250_unthrottle(struct uart_port *port)
+{
+	port->unthrottle(port);
+}
+
 static void serial8250_stop_rx(struct uart_port *port)
 {
 	struct uart_8250_port *up = up_to_u8250p(port);
@@ -2877,6 +2887,8 @@ static struct uart_ops serial8250_pops = {
 	.get_mctrl	= serial8250_get_mctrl,
 	.stop_tx	= serial8250_stop_tx,
 	.start_tx	= serial8250_start_tx,
+	.throttle	= serial8250_throttle,
+	.unthrottle	= serial8250_unthrottle,
 	.stop_rx	= serial8250_stop_rx,
 	.enable_ms	= serial8250_enable_ms,
 	.break_ctl	= serial8250_break_ctl,
@@ -3424,6 +3436,8 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
 		uart->port.fifosize	= up->port.fifosize;
 		uart->tx_loadsz		= up->tx_loadsz;
 		uart->capabilities	= up->capabilities;
+		uart->port.throttle	= up->port.throttle;
+		uart->port.unthrottle	= up->port.unthrottle;
 
 		/* Take tx_loadsz from fifosize if it wasn't set separately */
 		if (uart->port.fifosize && !uart->tx_loadsz)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index f3ea531..edaaaa0 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -124,6 +124,8 @@ struct uart_port {
 				               struct ktermios *old);
 	int			(*startup)(struct uart_port *port);
 	void			(*shutdown)(struct uart_port *port);
+	void			(*throttle)(struct uart_port *port);
+	void			(*unthrottle)(struct uart_port *port);
 	int			(*handle_irq)(struct uart_port *);
 	void			(*pm)(struct uart_port *, unsigned int state,
 				      unsigned int old);
-- 
2.0.1

[PATCH 03/15] tty: serial: 8250_core: add run time pm

[Sebastian Andrzej Siewior]

While comparing the OMAP-serial and the 8250 part of this I noticed that
the latter does not use run time-pm. Here are the pieces. It is
basically a get before first register access and a last_busy + put after
last access. This has to be enabled from userland _and_ UART_CAP_RPM is
required for this.
The runtime PM can usually work transparently in the background however
there is one exception to this: After serial8250_tx_chars() completes
there still may be unsent bytes in the FIFO (depending on CPU speed vs
baud rate + flow control). Even if the TTY-buffer is empty we do not
want RPM to disable the device because it won't send the remaining
bytes. Instead we leave serial8250_tx_chars() with RPM enabled and wait
for the FIFO empty interrupt. Once we enter serial8250_tx_chars() with
an empty buffer we know that the FIFO is empty and since we are not going
to send anything, we can disable the device.
That xchg() is to ensure that serial8250_tx_chars() can be called
multiple times and only the first invocation will actually invoke the
runtime PM function. So that the last invocation of __stop_tx() will
disable runtime pm.

v4…v5:
	- add a wrapper around rpm function and introduce UART_CAP_RPM
	  to ensure RPM put is invoked after the TX FIFO is empty.
v3…v4:
	- added runtime to the console code
	- removed device_may_wakeup() from serial8250_set_sleep()

Cc: mika.westerberg@linux.intel.com
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/tty/serial/8250/8250.h      |   1 +
 drivers/tty/serial/8250/8250_core.c | 134 ++++++++++++++++++++++++++++++++----
 include/linux/serial_8250.h         |   1 +
 3 files changed, 123 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 85bfec5..1bcb4b2 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -72,6 +72,7 @@ struct serial8250_config {
 #define UART_CAP_UUE	(1 << 12)	/* UART needs IER bit 6 set (Xscale) */
 #define UART_CAP_RTOIE	(1 << 13)	/* UART needs IER bit 4 set (Xscale, Tegra) */
 #define UART_CAP_HFIFO	(1 << 14)	/* UART has a "hidden" FIFO */
+#define UART_CAP_RPM	(1 << 15)	/* Runtime PM is active while idle */
 
 #define UART_BUG_QUOT	(1 << 0)	/* UART has buggy quot LSB */
 #define UART_BUG_TXEN	(1 << 1)	/* UART has buggy TX IIR status */
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 35fe96e..ae268e3 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -37,6 +37,7 @@
 #include <linux/nmi.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
 #ifdef CONFIG_SPARC
 #include <linux/sunserialcore.h>
 #endif
@@ -539,6 +540,53 @@ void serial8250_clear_and_reinit_fifos(struct uart_8250_port *p)
 }
 EXPORT_SYMBOL_GPL(serial8250_clear_and_reinit_fifos);
 
+static void serial8250_rpm_get(struct uart_8250_port *p)
+{
+	if (!(p->capabilities & UART_CAP_RPM))
+		return;
+	pm_runtime_get_sync(p->port.dev);
+}
+
+static void serial8250_rpm_put(struct uart_8250_port *p)
+{
+	if (!(p->capabilities & UART_CAP_RPM))
+		return;
+	pm_runtime_mark_last_busy(p->port.dev);
+	pm_runtime_put_autosuspend(p->port.dev);
+}
+
+/*
+ * This two wrapper ensure, that enable_runtime_pm_tx() can be called more than
+ * once and disable_runtime_pm_tx() will still disable RPM because the fifo is
+ * empty and the HW can idle again.
+ */
+static void serial8250_rpm_get_tx(struct uart_8250_port *p)
+{
+	unsigned char rpm_active;
+
+	if (!(p->capabilities & UART_CAP_RPM))
+		return;
+
+	rpm_active = xchg(&p->rpm_tx_active, 1);
+	if (rpm_active)
+		return;
+	pm_runtime_get_sync(p->port.dev);
+}
+
+static void serial8250_rpm_put_tx(struct uart_8250_port *p)
+{
+	unsigned char rpm_active;
+
+	if (!(p->capabilities & UART_CAP_RPM))
+		return;
+
+	rpm_active = xchg(&p->rpm_tx_active, 0);
+	if (!rpm_active)
+		return;
+	pm_runtime_mark_last_busy(p->port.dev);
+	pm_runtime_put_autosuspend(p->port.dev);
+}
+
 /*
  * IER sleep support.  UARTs which have EFRs need the "extended
  * capability" bit enabled.  Note that on XR16C850s, we need to
@@ -553,10 +601,11 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep)
 	 * offset but the UART channel may only write to the corresponding
 	 * bit.
 	 */
+	serial8250_rpm_get(p);
 	if ((p->port.type == PORT_XR17V35X) ||
 	   (p->port.type == PORT_XR17D15X)) {
 		serial_out(p, UART_EXAR_SLEEP, sleep ? 0xff : 0);
-		return;
+		goto out;
 	}
 
 	if (p->capabilities & UART_CAP_SLEEP) {
@@ -572,6 +621,8 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep)
 			serial_out(p, UART_LCR, 0);
 		}
 	}
+out:
+	serial8250_rpm_put(p);
 }
 
 #ifdef CONFIG_SERIAL_8250_RSA
@@ -1272,6 +1323,7 @@ static inline void __stop_tx(struct uart_8250_port *p)
 	if (p->ier & UART_IER_THRI) {
 		p->ier &= ~UART_IER_THRI;
 		serial_out(p, UART_IER, p->ier);
+		serial8250_rpm_put_tx(p);
 	}
 }
 
@@ -1279,6 +1331,7 @@ static void serial8250_stop_tx(struct uart_port *port)
 {
 	struct uart_8250_port *up = up_to_u8250p(port);
 
+	serial8250_rpm_get(up);
 	__stop_tx(up);
 
 	/*
@@ -1288,12 +1341,14 @@ static void serial8250_stop_tx(struct uart_port *port)
 		up->acr |= UART_ACR_TXDIS;
 		serial_icr_write(up, UART_ACR, up->acr);
 	}
+	serial8250_rpm_put(up);
 }
 
 static void serial8250_start_tx(struct uart_port *port)
 {
 	struct uart_8250_port *up = up_to_u8250p(port);
 
+	serial8250_rpm_get_tx(up);
 	if (up->dma && !serial8250_tx_dma(up)) {
 		return;
 	} else if (!(up->ier & UART_IER_THRI)) {
@@ -1332,9 +1387,13 @@ static void serial8250_stop_rx(struct uart_port *port)
 {
 	struct uart_8250_port *up = up_to_u8250p(port);
 
+	serial8250_rpm_get(up);
+
 	up->ier &= ~UART_IER_RLSI;
 	up->port.read_status_mask &= ~UART_LSR_DR;
 	serial_port_out(port, UART_IER, up->ier);
+
+	serial8250_rpm_put(up);
 }
 
 static void serial8250_enable_ms(struct uart_port *port)
@@ -1346,7 +1405,10 @@ static void serial8250_enable_ms(struct uart_port *port)
 		return;
 
 	up->ier |= UART_IER_MSI;
+
+	serial8250_rpm_get(up);
 	serial_port_out(port, UART_IER, up->ier);
+	serial8250_rpm_put(up);
 }
 
 /*
@@ -1468,7 +1530,12 @@ void serial8250_tx_chars(struct uart_8250_port *up)
 
 	DEBUG_INTR("THRE...");
 
-	if (uart_circ_empty(xmit))
+	/*
+	 * With RPM enabled, we have to wait once the FIFO is empty before the
+	 * HW can go idle. So we get here once again with empty FIFO and disable
+	 * the interrupt and RPM in __stop_tx()
+	 */
+	if (uart_circ_empty(xmit) && !(up->capabilities & UART_CAP_RPM))
 		__stop_tx(up);
 }
 EXPORT_SYMBOL_GPL(serial8250_tx_chars);
@@ -1535,9 +1602,17 @@ EXPORT_SYMBOL_GPL(serial8250_handle_irq);
 
 static int serial8250_default_handle_irq(struct uart_port *port)
 {
-	unsigned int iir = serial_port_in(port, UART_IIR);
+	struct uart_8250_port *up = up_to_u8250p(port);
+	unsigned int iir;
+	int ret;
 
-	return serial8250_handle_irq(port, iir);
+	serial8250_rpm_get(up);
+
+	iir = serial_port_in(port, UART_IIR);
+	ret = serial8250_handle_irq(port, iir);
+
+	serial8250_rpm_put(up);
+	return ret;
 }
 
 /*
@@ -1794,11 +1869,15 @@ static unsigned int serial8250_tx_empty(struct uart_port *port)
 	unsigned long flags;
 	unsigned int lsr;
 
+	serial8250_rpm_get(up);
+
 	spin_lock_irqsave(&port->lock, flags);
 	lsr = serial_port_in(port, UART_LSR);
 	up->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
 	spin_unlock_irqrestore(&port->lock, flags);
 
+	serial8250_rpm_put(up);
+
 	return (lsr & BOTH_EMPTY) == BOTH_EMPTY ? TIOCSER_TEMT : 0;
 }
 
@@ -1808,7 +1887,9 @@ static unsigned int serial8250_get_mctrl(struct uart_port *port)
 	unsigned int status;
 	unsigned int ret;
 
+	serial8250_rpm_get(up);
 	status = serial8250_modem_status(up);
+	serial8250_rpm_put(up);
 
 	ret = 0;
 	if (status & UART_MSR_DCD)
@@ -1840,7 +1921,9 @@ static void serial8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
 
 	mcr = (mcr & up->mcr_mask) | up->mcr_force | up->mcr;
 
+	serial8250_rpm_get(up);
 	serial_port_out(port, UART_MCR, mcr);
+	serial8250_rpm_put(up);
 }
 
 static void serial8250_break_ctl(struct uart_port *port, int break_state)
@@ -1848,6 +1931,7 @@ static void serial8250_break_ctl(struct uart_port *port, int break_state)
 	struct uart_8250_port *up = up_to_u8250p(port);
 	unsigned long flags;
 
+	serial8250_rpm_get(up);
 	spin_lock_irqsave(&port->lock, flags);
 	if (break_state == -1)
 		up->lcr |= UART_LCR_SBC;
@@ -1855,6 +1939,7 @@ static void serial8250_break_ctl(struct uart_port *port, int break_state)
 		up->lcr &= ~UART_LCR_SBC;
 	serial_port_out(port, UART_LCR, up->lcr);
 	spin_unlock_irqrestore(&port->lock, flags);
+	serial8250_rpm_put(up);
 }
 
 /*
@@ -1899,12 +1984,22 @@ static void wait_for_xmitr(struct uart_8250_port *up, int bits)
 
 static int serial8250_get_poll_char(struct uart_port *port)
 {
-	unsigned char lsr = serial_port_in(port, UART_LSR);
+	unsigned char lsr;
+	int status;
+
+	serial8250_rpm_get(up);
 
-	if (!(lsr & UART_LSR_DR))
-		return NO_POLL_CHAR;
+	lsr = serial_port_in(port, UART_LSR);
 
-	return serial_port_in(port, UART_RX);
+	if (!(lsr & UART_LSR_DR)) {
+		status = NO_POLL_CHAR;
+		goto out;
+	}
+
+	status = serial_port_in(port, UART_RX);
+out:
+	serial8250_rpm_put(up);
+	return status;
 }
 
 
@@ -1914,6 +2009,7 @@ static void serial8250_put_poll_char(struct uart_port *port,
 	unsigned int ier;
 	struct uart_8250_port *up = up_to_u8250p(port);
 
+	serial8250_rpm_get(up);
 	/*
 	 *	First save the IER then disable the interrupts
 	 */
@@ -1935,6 +2031,7 @@ static void serial8250_put_poll_char(struct uart_port *port,
 	 */
 	wait_for_xmitr(up, BOTH_EMPTY);
 	serial_port_out(port, UART_IER, ier);
+	serial8250_rpm_put(up);
 }
 
 #endif /* CONFIG_CONSOLE_POLL */
@@ -1960,6 +2057,7 @@ int serial8250_do_startup(struct uart_port *port)
 	if (port->iotype != up->cur_iotype)
 		set_io_from_upio(port);
 
+	serial8250_rpm_get(up);
 	if (port->type == PORT_16C950) {
 		/* Wake up and initialize UART */
 		up->acr = 0;
@@ -1980,7 +2078,6 @@ int serial8250_do_startup(struct uart_port *port)
 	 */
 	enable_rsa(up);
 #endif
-
 	/*
 	 * Clear the FIFO buffers and disable them.
 	 * (they will be reenabled in set_termios())
@@ -2004,7 +2101,8 @@ int serial8250_do_startup(struct uart_port *port)
 	    (serial_port_in(port, UART_LSR) == 0xff)) {
 		printk_ratelimited(KERN_INFO "ttyS%d: LSR safety check engaged!\n",
 				   serial_index(port));
-		return -ENODEV;
+		retval = -ENODEV;
+		goto out;
 	}
 
 	/*
@@ -2089,7 +2187,7 @@ int serial8250_do_startup(struct uart_port *port)
 	} else {
 		retval = serial_link_irq_chain(up);
 		if (retval)
-			return retval;
+			goto out;
 	}
 
 	/*
@@ -2187,8 +2285,10 @@ int serial8250_do_startup(struct uart_port *port)
 		outb_p(0x80, icp);
 		inb_p(icp);
 	}
-
-	return 0;
+	retval = 0;
+out:
+	serial8250_rpm_put(up);
+	return retval;
 }
 EXPORT_SYMBOL_GPL(serial8250_do_startup);
 
@@ -2205,6 +2305,7 @@ void serial8250_do_shutdown(struct uart_port *port)
 	struct uart_8250_port *up = up_to_u8250p(port);
 	unsigned long flags;
 
+	serial8250_rpm_get(up);
 	/*
 	 * Disable interrupts from this port
 	 */
@@ -2244,6 +2345,7 @@ void serial8250_do_shutdown(struct uart_port *port)
 	 * the IRQ chain.
 	 */
 	serial_port_in(port, UART_RX);
+	serial8250_rpm_put(up);
 
 	del_timer_sync(&up->timer);
 	up->timer.function = serial8250_timeout;
@@ -2361,6 +2463,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 	 * Ok, we're now changing the port state.  Do it with
 	 * interrupts disabled.
 	 */
+	serial8250_rpm_get(up);
 	spin_lock_irqsave(&port->lock, flags);
 
 	/*
@@ -2482,6 +2585,8 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 	}
 	serial8250_set_mctrl(port, port->mctrl);
 	spin_unlock_irqrestore(&port->lock, flags);
+	serial8250_rpm_put(up);
+
 	/* Don't rewrite B0 */
 	if (tty_termios_baud_rate(termios))
 		tty_termios_encode_baud_rate(termios, baud, baud);
@@ -3055,6 +3160,8 @@ serial8250_console_write(struct console *co, const char *s, unsigned int count)
 
 	touch_nmi_watchdog();
 
+	serial8250_rpm_get(up);
+
 	if (port->sysrq || oops_in_progress)
 		locked = spin_trylock_irqsave(&port->lock, flags);
 	else
@@ -3091,6 +3198,7 @@ serial8250_console_write(struct console *co, const char *s, unsigned int count)
 
 	if (locked)
 		spin_unlock_irqrestore(&port->lock, flags);
+	serial8250_rpm_put(up);
 }
 
 static int __init serial8250_console_setup(struct console *co, char *options)
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index c3aa004..b161eee 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -84,6 +84,7 @@ struct uart_8250_port {
 	unsigned char		mcr_mask;	/* mask of user bits */
 	unsigned char		mcr_force;	/* mask of forced bits */
 	unsigned char		cur_iotype;	/* Running I/O type */
+	unsigned char		rpm_tx_active;
 
 	/*
 	 * Some bits in registers are cleared on a read, so they must
-- 
2.0.1

Re: [PATCH 03/15] tty: serial: 8250_core: add run time pm

[Frans Klaver]

Hi,

On Fri, Aug 15, 2014 at 07:42:31PM +0200, Sebastian Andrzej Siewior wrote:
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -1899,12 +1984,22 @@ static void wait_for_xmitr(struct uart_8250_port *up, int bits)
>  
>  static int serial8250_get_poll_char(struct uart_port *port)
>  {
> -	unsigned char lsr = serial_port_in(port, UART_LSR);
> +	unsigned char lsr;
> +	int status;
> +
> +	serial8250_rpm_get(up);

or up won't be defined below. You probably need 
+	struct uart_8250_port *up = up_to_u8250p(port);
somewhere in there.

> -	if (!(lsr & UART_LSR_DR))
> -		return NO_POLL_CHAR;
> +	lsr = serial_port_in(port, UART_LSR);
>  
> -	return serial_port_in(port, UART_RX);
> +	if (!(lsr & UART_LSR_DR)) {
> +		status = NO_POLL_CHAR;
> +		goto out;
> +	}
> +
> +	status = serial_port_in(port, UART_RX);
> +out:
> +	serial8250_rpm_put(up);
> +	return status;
>  }

Cheers,
Frans

Re: [PATCH 03/15] tty: serial: 8250_core: add run time pm

[Frans Klaver]

On Wed, Aug 20, 2014 at 11:23:21AM +0200, Frans Klaver wrote:
> Hi,
> 
> On Fri, Aug 15, 2014 at 07:42:31PM +0200, Sebastian Andrzej Siewior wrote:
> > --- a/drivers/tty/serial/8250/8250_core.c
> > +++ b/drivers/tty/serial/8250/8250_core.c
> > @@ -1899,12 +1984,22 @@ static void wait_for_xmitr(struct uart_8250_port *up, int bits)
> >  
> >  static int serial8250_get_poll_char(struct uart_port *port)
> >  {
> > -	unsigned char lsr = serial_port_in(port, UART_LSR);
> > +	unsigned char lsr;
> > +	int status;
> > +
> > +	serial8250_rpm_get(up);
> 
> or up won't be defined below. You probably need 

Obviously I meant to say that 'up' won't be defined here.


> +	struct uart_8250_port *up = up_to_u8250p(port);
> somewhere in there.
> 
> > -	if (!(lsr & UART_LSR_DR))
> > -		return NO_POLL_CHAR;
> > +	lsr = serial_port_in(port, UART_LSR);
> >  
> > -	return serial_port_in(port, UART_RX);
> > +	if (!(lsr & UART_LSR_DR)) {
> > +		status = NO_POLL_CHAR;
> > +		goto out;
> > +	}
> > +
> > +	status = serial_port_in(port, UART_RX);
> > +out:
> > +	serial8250_rpm_put(up);
> > +	return status;
> >  }
> 
> Cheers,
> Frans

Re: [PATCH 03/15] tty: serial: 8250_core: add run time pm

[Sebastian Andrzej Siewior]

On 08/20/2014 11:39 AM, Frans Klaver wrote:

>>>  static int serial8250_get_poll_char(struct uart_port *port)
>>>  {
>>> -	unsigned char lsr = serial_port_in(port, UART_LSR);
>>> +	unsigned char lsr;
>>> +	int status;
>>> +
>>> +	serial8250_rpm_get(up);
>>
>> or up won't be defined below. You probably need 
> 
> Obviously I meant to say that 'up' won't be defined here.

Good catch, thanks. However I wouldn't bet my money it that won't be
defined here. The semaphore code provides up() and down() so it is
makes it kind of defined :) But it doesn't compile due to wrong pointer
types which is good (I run into this myself and looked confused at
first).
I didn't notice this at all because only kgdb uses this
CONFIG_CONSOLE_POLL which I had off. Once again, thank you.

>> +	struct uart_8250_port *up = up_to_u8250p(port);
>> somewhere in there.

Sebastian

[PATCH 04/15] tty: serial: 8250_core: read only RX if there is something in the FIFO

[Sebastian Andrzej Siewior]

The serial8250_do_startup() function unconditionally clears the
interrupts and for that it reads from the RX-FIFO without checking if
there is a byte in the FIFO or not. This works fine on OMAP4+ HW like
AM335x or DRA7.
OMAP3630 ES1.1 (which means probably all OMAP3 and earlier) does not like
this:

|Unhandled fault: external abort on non-linefetch (0x1028) at 0xfb020000
|Internal error: : 1028 [#1] ARM
|Modules linked in:
|CPU: 0 PID: 1 Comm: swapper Not tainted 3.16.0-00022-g7edcb57-dirty #1213
|task: de0572c0 ti: de058000 task.ti: de058000
|PC is at mem32_serial_in+0xc/0x1c
|LR is at serial8250_do_startup+0x220/0x85c
|Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
|Control: 10c5387d  Table: 80004019  DAC: 00000015
|[<c03051d4>] (mem32_serial_in) from [<c0307fe8>] (serial8250_do_startup+0x220/0x85c)
|[<c0307fe8>] (serial8250_do_startup) from [<c0309e00>] (omap_8250_startup+0x5c/0xe0)
|[<c0309e00>] (omap_8250_startup) from [<c030863c>] (serial8250_startup+0x18/0x2c)
|[<c030863c>] (serial8250_startup) from [<c030394c>] (uart_startup+0x78/0x1d8)
|[<c030394c>] (uart_startup) from [<c0304678>] (uart_open+0xe8/0x114)
|[<c0304678>] (uart_open) from [<c02e9e10>] (tty_open+0x1a8/0x5a4)

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/tty/serial/8250/8250_core.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index ae268e3..cc90c19 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2087,8 +2087,8 @@ int serial8250_do_startup(struct uart_port *port)
 	/*
 	 * Clear the interrupt registers.
 	 */
-	serial_port_in(port, UART_LSR);
-	serial_port_in(port, UART_RX);
+	if (serial_port_in(port, UART_LSR) & UART_LSR_DR)
+		serial_port_in(port, UART_RX);
 	serial_port_in(port, UART_IIR);
 	serial_port_in(port, UART_MSR);
 
@@ -2249,8 +2249,8 @@ int serial8250_do_startup(struct uart_port *port)
 	 * saved flags to avoid getting false values from polling
 	 * routines or the previous session.
 	 */
-	serial_port_in(port, UART_LSR);
-	serial_port_in(port, UART_RX);
+	if (serial_port_in(port, UART_LSR) & UART_LSR_DR)
+		serial_port_in(port, UART_RX);
 	serial_port_in(port, UART_IIR);
 	serial_port_in(port, UART_MSR);
 	up->lsr_saved_flags = 0;
@@ -2344,7 +2344,8 @@ void serial8250_do_shutdown(struct uart_port *port)
 	 * Read data port to reset things, and then unlink from
 	 * the IRQ chain.
 	 */
-	serial_port_in(port, UART_RX);
+	if (serial_port_in(port, UART_LSR) & UART_LSR_DR)
+		serial_port_in(port, UART_RX);
 	serial8250_rpm_put(up);
 
 	del_timer_sync(&up->timer);
-- 
2.0.1

[PATCH 05/15] tty: serial: Add 8250-core based omap driver

[Sebastian Andrzej Siewior]

This patch provides a 8250-core based UART driver for the internal OMAP
UART. The long term goal is to provide the same functionality as the
current OMAP uart driver and DMA support.
I tried to merge omap-serial code together with the 8250-core code.
There should should be hardly a noticable difference. The trigger levels
are different compared to omap-serial:
- omap serial
  TX: Interrupt comes after TX FIFO has room for 16 bytes.
      TX of 4096 bytes in one go results in 256 interrupts

  RX: Interrupt comes after there is on byte in the FIFO.
      RX of 4096 bytes results in 4096 interrupts.

- this driver
  TX: Interrupt comes once the TX FIFO is empty.
      TX of 4096 bytes results in 65 interrupts. That means there will
      be gaps on the line while the driver reloads the FIFO.

  RX: Interrupt comes once there are 48 bytes in the FIFO or less over
      "longer" time frame. We have
          1 / 11520 * 10^3 * 16 => 1.38… ms
      1.38ms to react and purge the FIFO on 115200,8N1. Since the other
      driver fired after each byte it had ~5.47ms time to react. This
      _may_ cause problems if one relies on no missing bytes and has no
      flow control. On the other hand we get only 85 interrupts for the
      same amount of data.

It has been only tested as console UART on am335x-evm, dra7-evm and
beagle bone. I also did some longer raw-transfers to meassure the load.

The device name is ttyS based instead of ttyO. If a ttyO based node name
is required please ask udev for it. If both driver are activated (this
and omap-serial) then this serial driver will take control over the
device due to the link order

v4…v7:
	- change trigger levels after some tests with raw transfers.
v3…v4:
	- drop RS485 support
	- wire up ->throttle / ->unthrottle
v2…v3:
	- wire up startup & shutdown for wakeup-irq handling.
	- RS485 handling (well the core does).

v1…v2:
	- added runtime PM. Could somebody could please double check
	  this?
	- added omap_8250_set_termios()

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/tty/serial/8250/8250_core.c |   8 +
 drivers/tty/serial/8250/8250_omap.c | 911 ++++++++++++++++++++++++++++++++++++
 drivers/tty/serial/8250/Kconfig     |   9 +
 drivers/tty/serial/8250/Makefile    |   1 +
 include/uapi/linux/serial_core.h    |   3 +-
 5 files changed, 931 insertions(+), 1 deletion(-)
 create mode 100644 drivers/tty/serial/8250/8250_omap.c

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index cc90c19..ab003b6 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -264,6 +264,12 @@ static const struct serial8250_config uart_config[] = {
 		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
 		.flags		= UART_CAP_FIFO | UART_CAP_AFE,
 	},
+	[PORT_OMAP_16750] = {
+		.name		= "OMAP",
+		.fifo_size	= 64,
+		.tx_loadsz	= 64,
+		.flags		= UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP,
+	},
 	[PORT_TEGRA] = {
 		.name		= "Tegra",
 		.fifo_size	= 32,
@@ -1390,6 +1396,8 @@ static void serial8250_stop_rx(struct uart_port *port)
 	serial8250_rpm_get(up);
 
 	up->ier &= ~UART_IER_RLSI;
+	if (port->type == PORT_OMAP_16750)
+		up->ier &= ~UART_IER_RDI;
 	up->port.read_status_mask &= ~UART_LSR_DR;
 	serial_port_out(port, UART_IER, up->ier);
 
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
new file mode 100644
index 0000000..368e9d8
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -0,0 +1,911 @@
+/*
+ * 8250-core based driver for the OMAP internal UART
+ *
+ *  Copyright (C) 2014 Sebastian Andrzej Siewior
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/serial_8250.h>
+#include <linux/serial_core.h>
+#include <linux/serial_reg.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/of_irq.h>
+#include <linux/delay.h>
+#include <linux/pm_runtime.h>
+#include <linux/console.h>
+#include <linux/pm_qos.h>
+
+#include "8250.h"
+
+#define UART_DLL_EM 9
+#define UART_DLM_EM 10
+
+#define DEFAULT_CLK_SPEED	48000000 /* 48 Mhz*/
+
+#define UART_ERRATA_i202_MDR1_ACCESS	(1 << 0)
+#define OMAP_UART_WER_HAS_TX_WAKEUP	(1 << 1)
+
+/* SCR register bitmasks */
+#define OMAP_UART_SCR_RX_TRIG_GRANU1_MASK	(1 << 7)
+#define OMAP_UART_SCR_TX_TRIG_GRANU1_MASK	(1 << 6)
+#define OMAP_UART_SCR_TX_EMPTY			(1 << 3)
+
+/* MVR register bitmasks */
+#define OMAP_UART_MVR_SCHEME_SHIFT	30
+#define OMAP_UART_LEGACY_MVR_MAJ_MASK	0xf0
+#define OMAP_UART_LEGACY_MVR_MAJ_SHIFT	4
+#define OMAP_UART_LEGACY_MVR_MIN_MASK	0x0f
+#define OMAP_UART_MVR_MAJ_MASK		0x700
+#define OMAP_UART_MVR_MAJ_SHIFT		8
+#define OMAP_UART_MVR_MIN_MASK		0x3f
+
+/* Enable XON/XOFF flow control on output */
+#define OMAP_UART_SW_TX		0x08
+/* Enable XON/XOFF flow control on input */
+#define OMAP_UART_SW_RX		0x02
+
+#define OMAP_UART_WER_MOD_WKUP	0x7f
+#define OMAP_UART_TX_WAKEUP_EN	(1 << 7)
+
+#define TX_TRIGGER	1
+#define RX_TRIGGER	48
+
+#define OMAP_UART_TCR_RESTORE(x)	((x / 4) << 4)
+#define OMAP_UART_TCR_HALT(x)		((x / 4) << 0)
+
+#define UART_BUILD_REVISION(x, y)	(((x) << 8) | (y))
+
+#define OMAP_UART_REV_46 0x0406
+#define OMAP_UART_REV_52 0x0502
+#define OMAP_UART_REV_63 0x0603
+
+struct omap8250_priv {
+	int line;
+	u32 habit;
+	u32 fcr;
+	u32 mdr1;
+	u32 efr;
+	u32 quot;
+	u32 scr;
+	u32 wer;
+
+	bool is_suspending;
+	int wakeirq;
+	int wakeups_enabled;
+	u32 latency;
+	u32 calc_latency;
+	struct pm_qos_request pm_qos_request;
+	struct work_struct qos_work;
+};
+
+static u32 uart_read(struct uart_8250_port *up, u32 reg)
+{
+	return readl(up->port.membase + (reg << up->port.regshift));
+}
+
+/*
+ * Work Around for Errata i202 (2430, 3430, 3630, 4430 and 4460)
+ * The access to uart register after MDR1 Access
+ * causes UART to corrupt data.
+ *
+ * Need a delay =
+ * 5 L4 clock cycles + 5 UART functional clock cycle (@48MHz = ~0.2uS)
+ * give 10 times as much
+ */
+static void omap_8250_mdr1_errataset(struct uart_8250_port *up, u8 mdr1)
+{
+	struct omap8250_priv *priv = up->port.private_data;
+	u8 timeout = 255;
+
+	serial_out(up, UART_OMAP_MDR1, mdr1);
+	udelay(2);
+	serial_out(up, UART_FCR, priv->fcr | UART_FCR_CLEAR_XMIT |
+			UART_FCR_CLEAR_RCVR);
+	/*
+	 * Wait for FIFO to empty: when empty, RX_FIFO_E bit is 0 and
+	 * TX_FIFO_E bit is 1.
+	 */
+	while (UART_LSR_THRE != (serial_in(up, UART_LSR) &
+				(UART_LSR_THRE | UART_LSR_DR))) {
+		timeout--;
+		if (!timeout) {
+			/* Should *never* happen. we warn and carry on */
+			dev_crit(up->port.dev, "Errata i202: timedout %x\n",
+						serial_in(up, UART_LSR));
+			break;
+		}
+		udelay(1);
+	}
+}
+
+static void omap_8250_get_divisor(struct uart_port *port, unsigned int baud,
+		struct omap8250_priv *priv)
+{
+	unsigned int uartclk = port->uartclk;
+	unsigned int div_13, div_16;
+	unsigned int abs_d13, abs_d16;
+
+	/*
+	 * Old custom speed handling.
+	 */
+	if (baud == 38400 && (port->flags & UPF_SPD_MASK) == UPF_SPD_CUST) {
+		priv->quot = port->custom_divisor & 0xffff;
+		/*
+		 * I assume that nobody is using this. But hey, if somebody
+		 * would like to specify the divisor _and_ the mode then the
+		 * driver is ready and waiting for it.
+		 */
+		if (port->custom_divisor & (1 << 16))
+			priv->mdr1 = UART_OMAP_MDR1_13X_MODE;
+		else
+			priv->mdr1 = UART_OMAP_MDR1_16X_MODE;
+		return;
+	}
+	div_13 = DIV_ROUND_CLOSEST(uartclk, 13 * baud);
+	div_16 = DIV_ROUND_CLOSEST(uartclk, 16 * baud);
+
+	abs_d13 = abs(baud - port->uartclk / 13 / div_13);
+	abs_d16 = abs(baud - port->uartclk / 16 / div_16);
+
+	if (abs_d13 >= abs_d16) {
+		priv->mdr1 = UART_OMAP_MDR1_16X_MODE;
+		priv->quot = div_16;
+	} else {
+		priv->mdr1 = UART_OMAP_MDR1_13X_MODE;
+		priv->quot = div_13;
+	}
+}
+
+/*
+ * OMAP can use "CLK / (16 or 13) / div" for baud rate. And then we have have
+ * some differences in how we want to handle flow control.
+ */
+static void omap_8250_set_termios(struct uart_port *port,
+		struct ktermios *termios, struct ktermios *old)
+{
+	struct uart_8250_port *up =
+		container_of(port, struct uart_8250_port, port);
+	struct omap8250_priv *priv = up->port.private_data;
+	unsigned char cval = 0;
+	unsigned long flags = 0;
+	unsigned int baud;
+
+	switch (termios->c_cflag & CSIZE) {
+	case CS5:
+		cval = UART_LCR_WLEN5;
+		break;
+	case CS6:
+		cval = UART_LCR_WLEN6;
+		break;
+	case CS7:
+		cval = UART_LCR_WLEN7;
+		break;
+	default:
+	case CS8:
+		cval = UART_LCR_WLEN8;
+		break;
+	}
+
+	if (termios->c_cflag & CSTOPB)
+		cval |= UART_LCR_STOP;
+	if (termios->c_cflag & PARENB)
+		cval |= UART_LCR_PARITY;
+	if (!(termios->c_cflag & PARODD))
+		cval |= UART_LCR_EPAR;
+	if (termios->c_cflag & CMSPAR)
+		cval |= UART_LCR_SPAR;
+
+	/*
+	 * Ask the core to calculate the divisor for us.
+	 */
+	baud = uart_get_baud_rate(port, termios, old,
+			port->uartclk / 16 / 0xffff,
+			port->uartclk / 13);
+	omap_8250_get_divisor(port, baud, priv);
+
+	/*
+	 * Ok, we're now changing the port state. Do it with
+	 * interrupts disabled.
+	 */
+	pm_runtime_get_sync(port->dev);
+	spin_lock_irqsave(&port->lock, flags);
+
+	/*
+	 * Update the per-port timeout.
+	 */
+	uart_update_timeout(port, termios->c_cflag, baud);
+
+	up->port.read_status_mask = UART_LSR_OE | UART_LSR_THRE | UART_LSR_DR;
+	if (termios->c_iflag & INPCK)
+		up->port.read_status_mask |= UART_LSR_FE | UART_LSR_PE;
+	if (termios->c_iflag & (BRKINT | PARMRK))
+		up->port.read_status_mask |= UART_LSR_BI;
+
+	/*
+	 * Characters to ignore
+	 */
+	up->port.ignore_status_mask = 0;
+	if (termios->c_iflag & IGNPAR)
+		up->port.ignore_status_mask |= UART_LSR_PE | UART_LSR_FE;
+	if (termios->c_iflag & IGNBRK) {
+		up->port.ignore_status_mask |= UART_LSR_BI;
+		/*
+		 * If we're ignoring parity and break indicators,
+		 * ignore overruns too (for real raw support).
+		 */
+		if (termios->c_iflag & IGNPAR)
+			up->port.ignore_status_mask |= UART_LSR_OE;
+	}
+
+	/*
+	 * ignore all characters if CREAD is not set
+	 */
+	if ((termios->c_cflag & CREAD) == 0)
+		up->port.ignore_status_mask |= UART_LSR_DR;
+
+	/*
+	 * Modem status interrupts
+	 */
+	up->ier &= ~UART_IER_MSI;
+	if (UART_ENABLE_MS(&up->port, termios->c_cflag))
+		up->ier |= UART_IER_MSI;
+	serial_out(up, UART_IER, up->ier);
+
+	serial_out(up, UART_LCR, cval);         /* reset DLAB */
+	up->lcr = cval;
+
+	/* Up to here it was mostly serial8250_do_set_termios() */
+
+	/* FCR can be changed only when the
+	 * baud clock is not running
+	 * DLL_REG and DLH_REG set to 0.
+	 */
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
+	serial_out(up, UART_DLL, 0);
+	serial_out(up, UART_DLM, 0);
+	serial_out(up, UART_LCR, 0);
+
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+	serial_out(up, UART_EFR, UART_EFR_ECB);
+
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
+	serial_out(up, UART_MCR, up->mcr | UART_MCR_TCRTLR);
+
+	serial_out(up, UART_TI752_TCR,
+			OMAP_UART_TCR_RESTORE(16) | OMAP_UART_TCR_HALT(52));
+
+	priv->scr = OMAP_UART_SCR_RX_TRIG_GRANU1_MASK | OMAP_UART_SCR_TX_EMPTY |
+		OMAP_UART_SCR_TX_TRIG_GRANU1_MASK;
+
+	serial_out(up, UART_TI752_TLR,
+			TRIGGER_TLR_MASK(TX_TRIGGER) << UART_TI752_TLR_TX |
+			TRIGGER_TLR_MASK(RX_TRIGGER) << UART_TI752_TLR_RX);
+	/*
+	 * We enable TRIG_GRANU for RX and TX and additionaly we set
+	 * SCR_TX_EMPTY bit. The result is the following:
+	 * - RX_TRIGGER amount of bytes in the FIFO will cause an interrupt.
+	 * - less than RX_TRIGGER number of bytes will also cause an interrupt
+	 *   once the UART decides that there no new bytes arriving.
+	 * - Once THRE is enabled, the interrupt will be fired once the FIFO is
+	 *   empty - the trigger level is ignored here.
+	 */
+	priv->fcr = UART_FCR_ENABLE_FIFO;
+	priv->fcr |= TRIGGER_FCR_MASK(TX_TRIGGER) << OMAP_UART_FCR_TX_TRIG;
+	priv->fcr |= TRIGGER_FCR_MASK(RX_TRIGGER) << OMAP_UART_FCR_RX_TRIG;
+
+	serial_out(up, UART_FCR, priv->fcr);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+
+	serial_out(up, UART_OMAP_SCR, priv->scr);
+
+	/* Reset UART_MCR_TCRTLR: this must be done with the EFR_ECB bit set */
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
+	serial_out(up, UART_MCR, up->mcr);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+	serial_out(up, UART_EFR, 0);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
+
+	/* Protocol, Baud Rate, and Interrupt Settings */
+
+	if (priv->habit & UART_ERRATA_i202_MDR1_ACCESS)
+		omap_8250_mdr1_errataset(up, UART_OMAP_MDR1_DISABLE);
+	else
+		serial_out(up, UART_OMAP_MDR1, UART_OMAP_MDR1_DISABLE);
+
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+	serial_out(up, UART_EFR, UART_EFR_ECB);
+
+	serial_out(up, UART_LCR, 0);
+	serial_out(up, UART_IER, 0);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+
+	serial_dl_write(up, priv->quot);
+
+	serial_out(up, UART_LCR, 0);
+	serial_out(up, UART_IER, up->ier);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+
+	serial_out(up, UART_EFR, 0);
+	serial_out(up, UART_LCR, up->lcr);
+
+	if (priv->habit & UART_ERRATA_i202_MDR1_ACCESS)
+		omap_8250_mdr1_errataset(up, priv->mdr1);
+	else
+		serial_out(up, UART_OMAP_MDR1, priv->mdr1);
+
+	/* Configure flow control */
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+
+	/* XON1/XOFF1 accessible mode B, TCRTLR=0, ECB=0 */
+	serial_out(up, UART_XON1, termios->c_cc[VSTART]);
+	serial_out(up, UART_XOFF1, termios->c_cc[VSTOP]);
+
+	priv->efr = 0;
+	if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW) {
+		/* Enable AUTORTS and AUTOCTS */
+		priv->efr |= UART_EFR_CTS | UART_EFR_RTS;
+
+		/* Ensure MCR RTS is asserted */
+		up->mcr |= UART_MCR_RTS;
+	}
+
+	if (up->port.flags & UPF_SOFT_FLOW) {
+		/*
+		 * IXON Flag:
+		 * Enable XON/XOFF flow control on input.
+		 * Receiver compares XON1, XOFF1.
+		 */
+		if (termios->c_iflag & IXON)
+			priv->efr |= OMAP_UART_SW_RX;
+
+		/*
+		 * IXOFF Flag:
+		 * Enable XON/XOFF flow control on output.
+		 * Transmit XON1, XOFF1
+		 */
+		if (termios->c_iflag & IXOFF)
+			priv->efr |= OMAP_UART_SW_TX;
+
+		/*
+		 * IXANY Flag:
+		 * Enable any character to restart output.
+		 * Operation resumes after receiving any
+		 * character after recognition of the XOFF character
+		 */
+		if (termios->c_iflag & IXANY)
+			up->mcr |= UART_MCR_XONANY;
+		else
+			up->mcr &= ~UART_MCR_XONANY;
+	}
+	serial_out(up, UART_MCR, up->mcr);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+	serial_out(up, UART_EFR, priv->efr);
+	serial_out(up, UART_LCR, up->lcr);
+
+	port->ops->set_mctrl(port, port->mctrl);
+
+	spin_unlock_irqrestore(&up->port.lock, flags);
+	pm_runtime_mark_last_busy(port->dev);
+	pm_runtime_put_autosuspend(port->dev);
+
+	/* calculate wakeup latency constraint */
+	priv->calc_latency = (USEC_PER_SEC * up->port.fifosize) / (baud / 8);
+	priv->latency = priv->calc_latency;
+	schedule_work(&priv->qos_work);
+
+	/* Don't rewrite B0 */
+	if (tty_termios_baud_rate(termios))
+		tty_termios_encode_baud_rate(termios, baud, baud);
+}
+
+/* same as 8250 except that we may have extra flow bits set in EFR */
+static void omap_8250_pm(struct uart_port *port, unsigned int state,
+		unsigned int oldstate)
+{
+	struct uart_8250_port *up =
+		container_of(port, struct uart_8250_port, port);
+	struct omap8250_priv *priv = up->port.private_data;
+
+	pm_runtime_get_sync(port->dev);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+	serial_out(up, UART_EFR, priv->efr | UART_EFR_ECB);
+	serial_out(up, UART_LCR, 0);
+
+	serial_out(up, UART_IER, (state != 0) ? UART_IERX_SLEEP : 0);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+	serial_out(up, UART_EFR, priv->efr);
+	serial_out(up, UART_LCR, 0);
+
+	pm_runtime_mark_last_busy(port->dev);
+	pm_runtime_put_autosuspend(port->dev);
+}
+
+static void omap_serial_fill_features_erratas(struct uart_8250_port *up,
+		struct omap8250_priv *priv)
+{
+	u32 mvr, scheme;
+	u16 revision, major, minor;
+
+	mvr = uart_read(up, UART_OMAP_MVER);
+
+	/* Check revision register scheme */
+	scheme = mvr >> OMAP_UART_MVR_SCHEME_SHIFT;
+
+	switch (scheme) {
+	case 0: /* Legacy Scheme: OMAP2/3 */
+		/* MINOR_REV[0:4], MAJOR_REV[4:7] */
+		major = (mvr & OMAP_UART_LEGACY_MVR_MAJ_MASK) >>
+			OMAP_UART_LEGACY_MVR_MAJ_SHIFT;
+		minor = (mvr & OMAP_UART_LEGACY_MVR_MIN_MASK);
+		break;
+	case 1:
+		/* New Scheme: OMAP4+ */
+		/* MINOR_REV[0:5], MAJOR_REV[8:10] */
+		major = (mvr & OMAP_UART_MVR_MAJ_MASK) >>
+			OMAP_UART_MVR_MAJ_SHIFT;
+		minor = (mvr & OMAP_UART_MVR_MIN_MASK);
+		break;
+	default:
+		dev_warn(up->port.dev,
+				"Unknown revision, defaulting to highest\n");
+		/* highest possible revision */
+		major = 0xff;
+		minor = 0xff;
+	}
+	/* normalize revision for the driver */
+	revision = UART_BUILD_REVISION(major, minor);
+
+	switch (revision) {
+	case OMAP_UART_REV_46:
+		priv->habit = UART_ERRATA_i202_MDR1_ACCESS;
+		break;
+	case OMAP_UART_REV_52:
+		priv->habit = UART_ERRATA_i202_MDR1_ACCESS |
+				OMAP_UART_WER_HAS_TX_WAKEUP;
+		break;
+	case OMAP_UART_REV_63:
+		priv->habit = UART_ERRATA_i202_MDR1_ACCESS |
+			OMAP_UART_WER_HAS_TX_WAKEUP;
+		break;
+	default:
+		break;
+	}
+}
+
+static void omap8250_uart_qos_work(struct work_struct *work)
+{
+	struct omap8250_priv *priv;
+
+	priv = container_of(work, struct omap8250_priv, qos_work);
+	pm_qos_update_request(&priv->pm_qos_request, priv->latency);
+}
+
+static irqreturn_t omap_wake_irq(int irq, void *dev_id)
+{
+	struct uart_port *port = dev_id;
+	int ret;
+
+	ret = port->handle_irq(port);
+	if (ret)
+		return IRQ_HANDLED;
+	return IRQ_NONE;
+}
+
+static int omap_8250_startup(struct uart_port *port)
+{
+	struct uart_8250_port *up =
+		container_of(port, struct uart_8250_port, port);
+	struct omap8250_priv *priv = port->private_data;
+
+	int ret;
+
+	if (priv->wakeirq) {
+		ret = request_irq(priv->wakeirq, omap_wake_irq,
+				port->irqflags, "wakeup irq", port);
+		if (ret)
+			return ret;
+		disable_irq(priv->wakeirq);
+	}
+
+	ret = serial8250_do_startup(port);
+	if (ret)
+		goto err;
+
+	pm_runtime_get_sync(port->dev);
+
+	/* Enable module level wake up */
+	priv->wer = OMAP_UART_WER_MOD_WKUP;
+	if (priv->habit & OMAP_UART_WER_HAS_TX_WAKEUP)
+		priv->wer |= OMAP_UART_TX_WAKEUP_EN;
+	serial_out(up, UART_OMAP_WER, priv->wer);
+
+	pm_runtime_mark_last_busy(port->dev);
+	pm_runtime_put_autosuspend(port->dev);
+	return 0;
+err:
+	if (priv->wakeirq)
+		free_irq(priv->wakeirq, port);
+	return ret;
+}
+
+static void omap_8250_shutdown(struct uart_port *port)
+{
+	struct uart_8250_port *up =
+		container_of(port, struct uart_8250_port, port);
+	struct omap8250_priv *priv = port->private_data;
+
+	pm_runtime_get_sync(port->dev);
+
+	serial_out(up, UART_OMAP_WER, 0);
+	serial8250_do_shutdown(port);
+
+	pm_runtime_mark_last_busy(port->dev);
+	pm_runtime_put_autosuspend(port->dev);
+
+	if (priv->wakeirq)
+		free_irq(priv->wakeirq, port);
+}
+
+static void omap_8250_throttle(struct uart_port *port)
+{
+	unsigned long flags;
+	struct uart_8250_port *up =
+		container_of(port, struct uart_8250_port, port);
+
+	pm_runtime_get_sync(port->dev);
+
+	spin_lock_irqsave(&port->lock, flags);
+	up->ier &= ~(UART_IER_RLSI | UART_IER_RDI);
+	serial_out(up, UART_IER, up->ier);
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	pm_runtime_mark_last_busy(port->dev);
+	pm_runtime_put_autosuspend(port->dev);
+}
+
+static void omap_8250_unthrottle(struct uart_port *port)
+{
+	unsigned long flags;
+	struct uart_8250_port *up =
+		container_of(port, struct uart_8250_port, port);
+
+	pm_runtime_get_sync(port->dev);
+
+	spin_lock_irqsave(&port->lock, flags);
+	up->ier |= UART_IER_RLSI | UART_IER_RDI;
+	serial_out(up, UART_IER, up->ier);
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	pm_runtime_mark_last_busy(port->dev);
+	pm_runtime_put_autosuspend(port->dev);
+}
+
+static int omap8250_probe(struct platform_device *pdev)
+{
+	struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	struct resource *irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	struct omap8250_priv *priv;
+	struct uart_8250_port up;
+	int ret;
+	void __iomem *membase;
+
+	if (!regs || !irq) {
+		dev_err(&pdev->dev, "missing registers or irq\n");
+		return -EINVAL;
+	}
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		dev_err(&pdev->dev, "unable to allocate private data\n");
+		return -ENOMEM;
+	}
+	membase = devm_ioremap_nocache(&pdev->dev, regs->start,
+			resource_size(regs));
+	if (!membase)
+		return -ENODEV;
+
+	memset(&up, 0, sizeof(up));
+	up.port.dev = &pdev->dev;
+	up.port.mapbase = regs->start;
+	up.port.membase = membase;
+	up.port.irq = irq->start;
+	/*
+	 * It claims to be 16C750 compatible however it is a little different.
+	 * It has EFR and has no FCR7_64byte bit. The AFE which it claims to
+	 * have is enabled via EFR instead of MCR.
+	 */
+	up.port.type = PORT_OMAP_16750;
+	up.port.iotype = UPIO_MEM32;
+	up.port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE |
+		UPF_SOFT_FLOW | UPF_HARD_FLOW;
+	up.port.private_data = priv;
+
+	up.port.regshift = 2;
+	up.port.fifosize = 64;
+
+	up.port.set_termios = omap_8250_set_termios;
+	up.port.pm = omap_8250_pm;
+	up.port.startup = omap_8250_startup;
+	up.port.shutdown = omap_8250_shutdown;
+	up.port.throttle = omap_8250_throttle;
+	up.port.unthrottle = omap_8250_unthrottle;
+
+	if (pdev->dev.of_node) {
+		up.port.line = of_alias_get_id(pdev->dev.of_node, "serial");
+		of_property_read_u32(pdev->dev.of_node, "clock-frequency",
+				&up.port.uartclk);
+		priv->wakeirq = irq_of_parse_and_map(pdev->dev.of_node, 1);
+	} else {
+		up.port.line = pdev->id;
+	}
+
+	if (up.port.line < 0) {
+		dev_err(&pdev->dev, "failed to get alias/pdev id, errno %d\n",
+				up.port.line);
+		return -ENODEV;
+	}
+	if (!up.port.uartclk) {
+		up.port.uartclk = DEFAULT_CLK_SPEED;
+		dev_warn(&pdev->dev,
+				"No clock speed specified: using default: %d\n",
+				DEFAULT_CLK_SPEED);
+	}
+
+#ifdef CONFIG_PM_RUNTIME
+	up.capabilities |= UART_CAP_RPM;
+#endif
+
+	priv->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
+	priv->calc_latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
+	pm_qos_add_request(&priv->pm_qos_request,
+			PM_QOS_CPU_DMA_LATENCY, priv->latency);
+	INIT_WORK(&priv->qos_work, omap8250_uart_qos_work);
+
+	device_init_wakeup(&pdev->dev, true);
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, -1);
+
+	pm_runtime_irq_safe(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
+	pm_runtime_get_sync(&pdev->dev);
+
+	omap_serial_fill_features_erratas(&up, priv);
+
+	ret = serial8250_register_8250_port(&up);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "unable to register 8250 port\n");
+		goto err;
+	}
+	priv->line = ret;
+	platform_set_drvdata(pdev, priv);
+	pm_runtime_mark_last_busy(&pdev->dev);
+	pm_runtime_put_autosuspend(&pdev->dev);
+	return 0;
+err:
+	pm_runtime_put(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+	return ret;
+}
+
+static int omap8250_remove(struct platform_device *pdev)
+{
+	struct omap8250_priv *priv = platform_get_drvdata(pdev);
+
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+	serial8250_unregister_port(priv->line);
+	pm_qos_remove_request(&priv->pm_qos_request);
+	device_init_wakeup(&pdev->dev, false);
+	return 0;
+}
+
+#if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM_RUNTIME)
+
+static inline void omap8250_enable_wakeirq(struct omap8250_priv *priv,
+		bool enable)
+{
+	if (!priv->wakeirq)
+		return;
+
+	if (enable)
+		enable_irq(priv->wakeirq);
+	else
+		disable_irq_nosync(priv->wakeirq);
+}
+
+static void omap8250_enable_wakeup(struct omap8250_priv *priv,
+		bool enable)
+{
+	if (enable == priv->wakeups_enabled)
+		return;
+
+	omap8250_enable_wakeirq(priv, enable);
+	priv->wakeups_enabled = enable;
+}
+#endif
+
+#ifdef CONFIG_PM_SLEEP
+static int omap8250_prepare(struct device *dev)
+{
+	struct omap8250_priv *priv = dev_get_drvdata(dev);
+
+	if (!priv)
+		return 0;
+	priv->is_suspending = true;
+	return 0;
+}
+
+static void omap8250_complete(struct device *dev)
+{
+	struct omap8250_priv *priv = dev_get_drvdata(dev);
+
+	if (!priv)
+		return;
+	priv->is_suspending = false;
+}
+
+static int omap8250_suspend(struct device *dev)
+{
+	struct omap8250_priv *priv = dev_get_drvdata(dev);
+
+	serial8250_suspend_port(priv->line);
+	flush_work(&priv->qos_work);
+
+	if (device_may_wakeup(dev))
+		omap8250_enable_wakeup(priv, true);
+	else
+		omap8250_enable_wakeup(priv, false);
+	return 0;
+}
+
+static int omap8250_resume(struct device *dev)
+{
+	struct omap8250_priv *priv = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		omap8250_enable_wakeup(priv, false);
+
+	serial8250_resume_port(priv->line);
+	return 0;
+}
+#else
+#define omap8250_prepare NULL
+#define omap8250_complete NULL
+#endif
+
+#ifdef CONFIG_PM_RUNTIME
+static int omap8250_lost_context(struct omap8250_priv *priv)
+{
+	struct uart_8250_port *up;
+	u32 val;
+
+	up = serial8250_get_port(priv->line);
+	val = serial_in(up, UART_OMAP_MDR1);
+	/*
+	 * If we lose context, then MDR1 is set to its reset value which is
+	 * UART_OMAP_MDR1_DISABLE. After set_termios() we set it either to 13x
+	 * or 16x but never to disable again.
+	 */
+	if (val == UART_OMAP_MDR1_DISABLE)
+		return 1;
+	return 0;
+}
+
+static int omap8250_runtime_suspend(struct device *dev)
+{
+	struct omap8250_priv *priv = dev_get_drvdata(dev);
+
+	/*
+	 * When using 'no_console_suspend', the console UART must not be
+	 * suspended. Since driver suspend is managed by runtime suspend,
+	 * preventing runtime suspend (by returning error) will keep device
+	 * active during suspend.
+	 */
+	if (priv->is_suspending && !console_suspend_enabled) {
+		struct uart_8250_port *up;
+
+		up = serial8250_get_port(priv->line);
+		if (uart_console(&up->port))
+			return -EBUSY;
+	}
+
+	omap8250_enable_wakeup(priv, true);
+
+	priv->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
+	schedule_work(&priv->qos_work);
+	return 0;
+}
+
+static void omap8250_restore_context(struct omap8250_priv *priv)
+{
+	struct uart_8250_port *up;
+
+	up = serial8250_get_port(priv->line);
+
+	if (priv->habit & UART_ERRATA_i202_MDR1_ACCESS)
+		omap_8250_mdr1_errataset(up, UART_OMAP_MDR1_DISABLE);
+	else
+		serial_out(up, UART_OMAP_MDR1, UART_OMAP_MDR1_DISABLE);
+
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); /* Config B mode */
+	serial_out(up, UART_EFR, UART_EFR_ECB);
+	serial_out(up, UART_LCR, 0x0); /* Operational mode */
+	serial_out(up, UART_IER, 0x0);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); /* Config B mode */
+
+	serial_dl_write(up, priv->quot);
+
+	serial_out(up, UART_LCR, 0x0); /* Operational mode */
+	serial_out(up, UART_IER, up->ier);
+	serial_out(up, UART_FCR, priv->fcr);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
+	serial_out(up, UART_MCR, up->mcr);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); /* Config B mode */
+	serial_out(up, UART_OMAP_SCR, priv->scr);
+	serial_out(up, UART_EFR, priv->efr);
+	serial_out(up, UART_LCR, up->lcr);
+	if (priv->habit & UART_ERRATA_i202_MDR1_ACCESS)
+		omap_8250_mdr1_errataset(up, priv->mdr1);
+	else
+		serial_out(up, UART_OMAP_MDR1, priv->mdr1);
+	serial_out(up, UART_OMAP_WER, priv->wer);
+}
+
+static int omap8250_runtime_resume(struct device *dev)
+{
+	struct omap8250_priv *priv = dev_get_drvdata(dev);
+	int loss_cntx;
+
+	/* In case runtime-pm tries this before we are setup */
+	if (!priv)
+		return 0;
+	omap8250_enable_wakeup(priv, false);
+	loss_cntx = omap8250_lost_context(priv);
+
+	if (loss_cntx)
+		omap8250_restore_context(priv);
+
+	priv->latency = priv->calc_latency;
+	schedule_work(&priv->qos_work);
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops omap8250_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(omap8250_suspend, omap8250_resume)
+	SET_RUNTIME_PM_OPS(omap8250_runtime_suspend,
+			omap8250_runtime_resume, NULL)
+	.prepare        = omap8250_prepare,
+	.complete       = omap8250_complete,
+};
+
+static const struct of_device_id omap8250_dt_ids[] = {
+	{ .compatible = "ti,omap2-uart" },
+	{ .compatible = "ti,omap3-uart" },
+	{ .compatible = "ti,omap4-uart" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, omap8250_dt_ids);
+
+static struct platform_driver omap8250_platform_driver = {
+	.driver = {
+		.name		= "omap8250",
+		.pm		= &omap8250_dev_pm_ops,
+		.of_match_table = omap8250_dt_ids,
+		.owner		= THIS_MODULE,
+	},
+	.probe			= omap8250_probe,
+	.remove			= omap8250_remove,
+};
+module_platform_driver(omap8250_platform_driver);
+
+MODULE_AUTHOR("Sebastian Andrzej Siewior");
+MODULE_DESCRIPTION("OMAP 8250 Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 349ee59..7a5073b 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -298,3 +298,12 @@ config SERIAL_8250_RT288X
 	  If you have a Ralink RT288x/RT305x SoC based board and want to use the
 	  serial port, say Y to this option. The driver can handle up to 2 serial
 	  ports. If unsure, say N.
+
+config SERIAL_8250_OMAP
+	tristate "Support for OMAP internal UART (8250 based driver)"
+	depends on SERIAL_8250 && ARCH_OMAP2PLUS
+	help
+	  If you have a machine based on an Texas Instruments OMAP CPU you
+	  can enable its onboard serial ports by enabling this option.
+
+	  This driver is in early stage and uses ttyS instead of ttyO.
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index 36d68d0..4bac392 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_SERIAL_8250_HUB6)		+= 8250_hub6.o
 obj-$(CONFIG_SERIAL_8250_FSL)		+= 8250_fsl.o
 obj-$(CONFIG_SERIAL_8250_DW)		+= 8250_dw.o
 obj-$(CONFIG_SERIAL_8250_EM)		+= 8250_em.o
+obj-$(CONFIG_SERIAL_8250_OMAP)		+= 8250_omap.o
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index 5820269..74f9b11 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -54,7 +54,8 @@
 #define PORT_ALTR_16550_F32 26	/* Altera 16550 UART with 32 FIFOs */
 #define PORT_ALTR_16550_F64 27	/* Altera 16550 UART with 64 FIFOs */
 #define PORT_ALTR_16550_F128 28 /* Altera 16550 UART with 128 FIFOs */
-#define PORT_MAX_8250	28	/* max port ID */
+#define PORT_OMAP_16750	29	/* TI's OMAP internal 16C750 compatible UART */
+#define PORT_MAX_8250	29	/* max port ID */
 
 /*
  * ARM specific type numbers.  These are not currently guaranteed
-- 
2.0.1

Re: [PATCH 05/15] tty: serial: Add 8250-core based omap driver

[Lennart Sorensen]

On Fri, Aug 15, 2014 at 07:42:33PM +0200, Sebastian Andrzej Siewior wrote:
> This patch provides a 8250-core based UART driver for the internal OMAP
> UART. The long term goal is to provide the same functionality as the
> current OMAP uart driver and DMA support.
> I tried to merge omap-serial code together with the 8250-core code.
> There should should be hardly a noticable difference. The trigger levels
> are different compared to omap-serial:
> - omap serial
>   TX: Interrupt comes after TX FIFO has room for 16 bytes.
>       TX of 4096 bytes in one go results in 256 interrupts
> 
>   RX: Interrupt comes after there is on byte in the FIFO.
                                       one
>       RX of 4096 bytes results in 4096 interrupts.
> 
> - this driver
>   TX: Interrupt comes once the TX FIFO is empty.
>       TX of 4096 bytes results in 65 interrupts. That means there will
>       be gaps on the line while the driver reloads the FIFO.

Any idea how long the gap is likely to be?  Probably not much.  I like
the reduction in the number of interrupts.

I suppose if you did an interrupt when half empty or 3/4 empty, you
would avoid the gap, and only increase the interrupt amount a little bit.
Waiting until completely empty gives you larger dma transfers and less
interrupts, but reduces your effective bandwidth on the port.  Is that
really the right tradeoff?  I think the original driver behaviour there
was fairly sane, although the 16 byte value could perhaps be increased
to 32 or 48.

>   RX: Interrupt comes once there are 48 bytes in the FIFO or less over
>       "longer" time frame. We have
>           1 / 11520 * 10^3 * 16 => 1.38… ms
>       1.38ms to react and purge the FIFO on 115200,8N1. Since the other
>       driver fired after each byte it had ~5.47ms time to react. This
>       _may_ cause problems if one relies on no missing bytes and has no
>       flow control. On the other hand we get only 85 interrupts for the
>       same amount of data.

Hmm, so if this was 32 instead of 48, it would double the amount of
time you have to react, while only increasing the interrupt rate by 50%
(1 every 32 rather than 1 every 48).  Could be interesting to tweak to
get the balance just right.  Maybe it could have an optional dtb entry
to control it if you don't like the default or is there a way to change
it from user space already?

I know for our system we would like to be able to tolerate 1ms at 230400
without data loss.

-- 
Len Sorensen

Re: [PATCH 05/15] tty: serial: Add 8250-core based omap driver

[Sebastian Andrzej Siewior]

On 08/15/2014 08:37 PM, Lennart Sorensen wrote:
> On Fri, Aug 15, 2014 at 07:42:33PM +0200, Sebastian Andrzej Siewior wrote:
>> This patch provides a 8250-core based UART driver for the internal OMAP
>> UART. The long term goal is to provide the same functionality as the
>> current OMAP uart driver and DMA support.
>> I tried to merge omap-serial code together with the 8250-core code.
>> There should should be hardly a noticable difference. The trigger levels
>> are different compared to omap-serial:
>> - omap serial
>>   TX: Interrupt comes after TX FIFO has room for 16 bytes.
>>       TX of 4096 bytes in one go results in 256 interrupts
>>
>>   RX: Interrupt comes after there is on byte in the FIFO.
>                                        one
>>       RX of 4096 bytes results in 4096 interrupts.
>>
>> - this driver
>>   TX: Interrupt comes once the TX FIFO is empty.
>>       TX of 4096 bytes results in 65 interrupts. That means there will
>>       be gaps on the line while the driver reloads the FIFO.
> 
> Any idea how long the gap is likely to be?  Probably not much.  I like
> the reduction in the number of interrupts.

If you want to change this to reduce the gap, then you have first
change 8250 core code. Currently it waits until the shift register is
empty.
On the other hand if you use DMA then it can handle transfers > 64bytes
in one go and you can start transfers while the FIFO is not completely
empty.

> I suppose if you did an interrupt when half empty or 3/4 empty, you
> would avoid the gap, and only increase the interrupt amount a little bit.
> Waiting until completely empty gives you larger dma transfers and less
> interrupts, but reduces your effective bandwidth on the port.  Is that
> really the right tradeoff?  I think the original driver behaviour there
> was fairly sane, although the 16 byte value could perhaps be increased
> to 32 or 48.

If you use DMA. You program one transfer says 100 bytes. You get an
dma-transfer complete once the 100 bytes are transfered which means the
FIFO has 63 bytes. From this point on you could enqueue the next
transfer with say another 100 bytes. In that scenario you don't see the
gap.

You get only to the gap if you use the non-DMA mode (and not UARTs
support DMA). In that case, yes waiting till there only 16 bytes before
starting the refill would make sense if you want to utilize the port by
100%. But as I said in 0/15, you need to teach the core this first.
Otherwise it will return doing nothing until the shift register is
empty (i.e. until the FIFO is completely empty).

>>   RX: Interrupt comes once there are 48 bytes in the FIFO or less over
>>       "longer" time frame. We have
>>           1 / 11520 * 10^3 * 16 => 1.38… ms
>>       1.38ms to react and purge the FIFO on 115200,8N1. Since the other
>>       driver fired after each byte it had ~5.47ms time to react. This
>>       _may_ cause problems if one relies on no missing bytes and has no
>>       flow control. On the other hand we get only 85 interrupts for the
>>       same amount of data.
> 
> Hmm, so if this was 32 instead of 48, it would double the amount of
> time you have to react, while only increasing the interrupt rate by 50%
> (1 every 32 rather than 1 every 48).  Could be interesting to tweak to
> get the balance just right.  Maybe it could have an optional dtb entry
> to control it if you don't like the default or is there a way to change
> it from user space already?

There is patch in Greg's tty tree already where you are able to
configure the RX trigger level. We could wire this up once we agree
which levels we want support. The OMAP supports all levels from 1…63.

> I know for our system we would like to be able to tolerate 1ms at 230400
> without data loss.

Yes, true. However this is only an issue without HW control. With DMA
the buffer is slightly larger. The DMA engine starts the transfer on
its own once there 48 bytes in the FIFO (except in the few cases where
it does not).

Sebastian

Re: [PATCH 05/15] tty: serial: Add 8250-core based omap driver

[Lennart Sorensen]

On Fri, Aug 15, 2014 at 09:27:59PM +0200, Sebastian Andrzej Siewior wrote:
> If you want to change this to reduce the gap, then you have first
> change 8250 core code. Currently it waits until the shift register is
> empty.

Oh the 8250 normally works this way?  I didn't know that.

> On the other hand if you use DMA then it can handle transfers > 64bytes
> in one go and you can start transfers while the FIFO is not completely
> empty.

You can dma more than the fifo size?

> If you use DMA. You program one transfer says 100 bytes. You get an
> dma-transfer complete once the 100 bytes are transfered which means the
> FIFO has 63 bytes. From this point on you could enqueue the next
> transfer with say another 100 bytes. In that scenario you don't see the
> gap.
> 
> You get only to the gap if you use the non-DMA mode (and not UARTs
> support DMA). In that case, yes waiting till there only 16 bytes before
> starting the refill would make sense if you want to utilize the port by
> 100%. But as I said in 0/15, you need to teach the core this first.
> Otherwise it will return doing nothing until the shift register is
> empty (i.e. until the FIFO is completely empty).

Well if DMA takes care of it, and the normal 8250 is already like this,
then I suppose it is already better than the typical case.

> There is patch in Greg's tty tree already where you are able to
> configure the RX trigger level. We could wire this up once we agree
> which levels we want support. The OMAP supports all levels from 1…63.

All? or just every 4 (that's what I just read in the DRA7xx docs).

> Yes, true. However this is only an issue without HW control. With DMA
> the buffer is slightly larger. The DMA engine starts the transfer on
> its own once there 48 bytes in the FIFO (except in the few cases where
> it does not).

That's nice of it.  I will have to give this a try.

-- 
Len Sorensen

Re: [PATCH 05/15] tty: serial: Add 8250-core based omap driver

[Sebastian Andrzej Siewior]

On 08/15/2014 09:33 PM, Lennart Sorensen wrote:
>> On the other hand if you use DMA then it can handle transfers > 64bytes
>> in one go and you can start transfers while the FIFO is not completely
>> empty.
> 
> You can dma more than the fifo size?

Yes. The UART asserts the DMA line as long as there is room for
$TRESHOLD number of bytes. So we never overflow the FIFO. That is why we
can't take any DMA channel but only *the* one.

>> There is patch in Greg's tty tree already where you are able to
>> configure the RX trigger level. We could wire this up once we agree
>> which levels we want support. The OMAP supports all levels from 1…63.
> 
> All? or just every 4 (that's what I just read in the DRA7xx docs).

All. Take a look at the RX_TRIGGER constant while comparing source vs
manual :)

Sebastian

Re: [PATCH 05/15] tty: serial: Add 8250-core based omap driver

[Tony Lindgren]

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140815 10:46]:
> This patch provides a 8250-core based UART driver for the internal OMAP
> UART. The long term goal is to provide the same functionality as the
> current OMAP uart driver and DMA support.
> I tried to merge omap-serial code together with the 8250-core code.
> There should should be hardly a noticable difference. The trigger levels
> are different compared to omap-serial:

Nice, now it mostly works for me with off-idle too :) That is as long
as I have the DMA channels commented out in the .dts file.

And I'm still seeing an occasional hang with pstore console just
showing:

[  289.076538] In-band Error seen by MPU  at address 0
[  289.076538] ------------[ cut here ]------------
[  289.076568] WARNING: CPU: 0 PID: 99 at drivers/bus/omap_l3_smx.c:162 omap3_l3_app_irq+0xdc/0x134()
[  289.076599] Modules linked in:
[  289.076599] CPU: 0 PID: 99 Comm: test-idle-off-8 Tainted: G        W      3.16.0+ #510
[  289.076629] [<c0016c44>] (unwind_backtrace) from [<c00129c8>] (show_stack+0x20/0x24)
[  289.076660] [<c00129c8>] (show_stack) from [<c0714cd4>] (dump_stack+0x88/0xa4)

Which most likely means there's still some glitch with the
runtime PM somewhere and registers are being accessed when
not clocked. I _think_ I did not see it when I did not have
console=ttyS2,115200 in my cmdline but was using just pstore
console.

> The device name is ttyS based instead of ttyO. If a ttyO based node name
> is required please ask udev for it. If both driver are activated (this
> and omap-serial) then this serial driver will take control over the
> device due to the link order

That's still not going to help with the existing kernel cmdlines
and existing installs.. I wonder if we can just do a minimal
dummy serial-omap.c that just proxies all the ttyO read/write
access to ttyS?

Regards,

Tony

Re: [PATCH 05/15] tty: serial: Add 8250-core based omap driver

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [140815 14:10]:
> * Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140815 10:46]:
> > This patch provides a 8250-core based UART driver for the internal OMAP
> > UART. The long term goal is to provide the same functionality as the
> > current OMAP uart driver and DMA support.
> > I tried to merge omap-serial code together with the 8250-core code.
> > There should should be hardly a noticable difference. The trigger levels
> > are different compared to omap-serial:
> 
> Nice, now it mostly works for me with off-idle too :) That is as long
> as I have the DMA channels commented out in the .dts file.
> 
> And I'm still seeing an occasional hang with pstore console just
> showing:
> 
> [  289.076538] In-band Error seen by MPU  at address 0
> [  289.076538] ------------[ cut here ]------------
> [  289.076568] WARNING: CPU: 0 PID: 99 at drivers/bus/omap_l3_smx.c:162 omap3_l3_app_irq+0xdc/0x134()
> [  289.076599] Modules linked in:
> [  289.076599] CPU: 0 PID: 99 Comm: test-idle-off-8 Tainted: G        W      3.16.0+ #510
> [  289.076629] [<c0016c44>] (unwind_backtrace) from [<c00129c8>] (show_stack+0x20/0x24)
> [  289.076660] [<c00129c8>] (show_stack) from [<c0714cd4>] (dump_stack+0x88/0xa4)
> 
> Which most likely means there's still some glitch with the
> runtime PM somewhere and registers are being accessed when
> not clocked. I _think_ I did not see it when I did not have
> console=ttyS2,115200 in my cmdline but was using just pstore
> console.

Oh and echo mem > /sys/power/state and then hitting a key on the serial
console won't wake the system. Does that need to be manually configured
for device_may_wakeup()?
 
> > The device name is ttyS based instead of ttyO. If a ttyO based node name
> > is required please ask udev for it. If both driver are activated (this
> > and omap-serial) then this serial driver will take control over the
> > device due to the link order
> 
> That's still not going to help with the existing kernel cmdlines
> and existing installs.. I wonder if we can just do a minimal
> dummy serial-omap.c that just proxies all the ttyO read/write
> access to ttyS?
> 
> Regards,
> 
> Tony
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 05/15] tty: serial: Add 8250-core based omap driver

[Sebastian Andrzej Siewior]

On 08/16/2014 12:44 AM, Tony Lindgren wrote:

> Oh and echo mem > /sys/power/state and then hitting a key on the serial
> console won't wake the system. Does that need to be manually configured
> for device_may_wakeup()?

This is what it looks like:

/# echo enabled >
/sys/devices/68000000.ocp/49020000.serial/tty/ttyS2/power/wakeup

/ # date; echo mem > /sys/power/state; date
Sat Jan  1 07:01:41 UTC 2000
[  249.916503] PM: Syncing filesystems ... done.
[  252.674652] Freezing user space processes ... (elapsed 0.001 seconds)
done.
[  252.683563] Freezing remaining freezable tasks ... (elapsed 0.001
seconds) done.
[  252.693084] Suspending console(s) (use no_console_suspend to debug)
[  252.715820] PM: suspend of devices complete after 11.688 msecs
[  252.722015] PM: late suspend of devices complete after 6.195 msecs
[  252.729187] PM: noirq suspend of devices complete after 7.110 msecs
[  252.729278] Successfully put all powerdomains to target state
[  252.733306] PM: noirq resume of devices complete after 3.967 msecs
[  252.738708] PM: early resume of devices complete after 4.425 msecs
[  252.910400] PM: resume of devices complete after 171.569 msecs
[  252.957855] Restarting tasks ... done.
Sat Jan  1 07:01:51 UTC 2000


>  

Sebastian

Re: [PATCH 05/15] tty: serial: Add 8250-core based omap driver

[Tony Lindgren]

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140829 08:50]:
> On 08/16/2014 12:44 AM, Tony Lindgren wrote:
> 
> > Oh and echo mem > /sys/power/state and then hitting a key on the serial
> > console won't wake the system. Does that need to be manually configured
> > for device_may_wakeup()?
> 
> This is what it looks like:
> 
> /# echo enabled >
> /sys/devices/68000000.ocp/49020000.serial/tty/ttyS2/power/wakeup
> 
> / # date; echo mem > /sys/power/state; date
> Sat Jan  1 07:01:41 UTC 2000
> [  249.916503] PM: Syncing filesystems ... done.
> [  252.674652] Freezing user space processes ... (elapsed 0.001 seconds)
> done.
> [  252.683563] Freezing remaining freezable tasks ... (elapsed 0.001
> seconds) done.
> [  252.693084] Suspending console(s) (use no_console_suspend to debug)
> [  252.715820] PM: suspend of devices complete after 11.688 msecs
> [  252.722015] PM: late suspend of devices complete after 6.195 msecs
> [  252.729187] PM: noirq suspend of devices complete after 7.110 msecs
> [  252.729278] Successfully put all powerdomains to target state
> [  252.733306] PM: noirq resume of devices complete after 3.967 msecs
> [  252.738708] PM: early resume of devices complete after 4.425 msecs
> [  252.910400] PM: resume of devices complete after 171.569 msecs
> [  252.957855] Restarting tasks ... done.
> Sat Jan  1 07:01:51 UTC 2000

Yes works for me too now thanks.

Tony

Re: [PATCH 05/15] tty: serial: Add 8250-core based omap driver

[Sebastian Andrzej Siewior]

On 08/15/2014 11:07 PM, Tony Lindgren wrote:
> Nice, now it mostly works for me with off-idle too :) That is as long
> as I have the DMA channels commented out in the .dts file.
> 
> And I'm still seeing an occasional hang with pstore console just
> showing:
> 
> [  289.076538] In-band Error seen by MPU  at address 0
> [  289.076538] ------------[ cut here ]------------
> [  289.076568] WARNING: CPU: 0 PID: 99 at drivers/bus/omap_l3_smx.c:162 omap3_l3_app_irq+0xdc/0x134()
> [  289.076599] Modules linked in:
> [  289.076599] CPU: 0 PID: 99 Comm: test-idle-off-8 Tainted: G        W      3.16.0+ #510
> [  289.076629] [<c0016c44>] (unwind_backtrace) from [<c00129c8>] (show_stack+0x20/0x24)
> [  289.076660] [<c00129c8>] (show_stack) from [<c0714cd4>] (dump_stack+0x88/0xa4)

Okay. So this backtrace does not show more like from where the access
is happening?

> Which most likely means there's still some glitch with the
> runtime PM somewhere and registers are being accessed when
> not clocked. I _think_ I did not see it when I did not have
> console=ttyS2,115200 in my cmdline but was using just pstore
> console.
> 
>> The device name is ttyS based instead of ttyO. If a ttyO based node name
>> is required please ask udev for it. If both driver are activated (this
>> and omap-serial) then this serial driver will take control over the
>> device due to the link order
> 
> That's still not going to help with the existing kernel cmdlines
> and existing installs.. I wonder if we can just do a minimal
> dummy serial-omap.c that just proxies all the ttyO read/write
> access to ttyS?

Hmm. So you are not a friend of the udev solution?. For now the driver
is "default n" and you have to explicit enable it and _then_ you should
be able to update your command line, etc.
If I introduce a kernel proxy for compatibility I assume that people
will wake up once that compatibility piece is gone.

However, if you insist… I tried to make a symlink but nobody does this
in kernel. The "rtc -> rtc0" and friends seem to come from udev _or_
distro. So I sff could a second device node with the same major/minor.
That would work for userland but not for the kernel console… So we need
a proxy-console for this.

Before I spent time on this proxy-console I would like to hear from Greg
that he is okay with this.

> 
> Regards,
> 
> Tony

Sebastian

Re: [PATCH 05/15] tty: serial: Add 8250-core based omap driver

[Tony Lindgren]

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140821 04:04]:
> On 08/15/2014 11:07 PM, Tony Lindgren wrote:
> > Nice, now it mostly works for me with off-idle too :) That is as long
> > as I have the DMA channels commented out in the .dts file.
> > 
> > And I'm still seeing an occasional hang with pstore console just
> > showing:
> > 
> > [  289.076538] In-band Error seen by MPU  at address 0
> > [  289.076538] ------------[ cut here ]------------
> > [  289.076568] WARNING: CPU: 0 PID: 99 at drivers/bus/omap_l3_smx.c:162 omap3_l3_app_irq+0xdc/0x134()
> > [  289.076599] Modules linked in:
> > [  289.076599] CPU: 0 PID: 99 Comm: test-idle-off-8 Tainted: G        W      3.16.0+ #510
> > [  289.076629] [<c0016c44>] (unwind_backtrace) from [<c00129c8>] (show_stack+0x20/0x24)
> > [  289.076660] [<c00129c8>] (show_stack) from [<c0714cd4>] (dump_stack+0x88/0xa4)
> 
> Okay. So this backtrace does not show more like from where the access
> is happening?

No, or if the omap_l3_smx.c can show the address it's not implemented.
 
> > Which most likely means there's still some glitch with the
> > runtime PM somewhere and registers are being accessed when
> > not clocked. I _think_ I did not see it when I did not have
> > console=ttyS2,115200 in my cmdline but was using just pstore
> > console.
> > 
> >> The device name is ttyS based instead of ttyO. If a ttyO based node name
> >> is required please ask udev for it. If both driver are activated (this
> >> and omap-serial) then this serial driver will take control over the
> >> device due to the link order
> > 
> > That's still not going to help with the existing kernel cmdlines
> > and existing installs.. I wonder if we can just do a minimal
> > dummy serial-omap.c that just proxies all the ttyO read/write
> > access to ttyS?
> 
> Hmm. So you are not a friend of the udev solution?. For now the driver
> is "default n" and you have to explicit enable it and _then_ you should
> be able to update your command line, etc.
> If I introduce a kernel proxy for compatibility I assume that people
> will wake up once that compatibility piece is gone.

The udev solution will not help with all the existing cases of
console=ttyO2,115200. But maybe we just need to translate the
cmdline to ttS2 in those cases?

Then the udev rules could fix up most of the distro issues, but
not all of them. And still requires the userspace to be updated.
 
> However, if you insist… I tried to make a symlink but nobody does this
> in kernel. The "rtc -> rtc0" and friends seem to come from udev _or_
> distro. So I sff could a second device node with the same major/minor.
> That would work for userland but not for the kernel console… So we need
> a proxy-console for this.

Yeah the symlinks won't work, think read-only rootfs for example :)
 
> Before I spent time on this proxy-console I would like to hear from Greg
> that he is okay with this.

Yeah me too.

Tony

Re: [PATCH 05/15] tty: serial: Add 8250-core based omap driver

[Heikki Krogerus]

Hi,

On Fri, Aug 15, 2014 at 07:42:33PM +0200, Sebastian Andrzej Siewior wrote:
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index cc90c19..ab003b6 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -264,6 +264,12 @@ static const struct serial8250_config uart_config[] = {
>  		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
>  		.flags		= UART_CAP_FIFO | UART_CAP_AFE,
>  	},
> +	[PORT_OMAP_16750] = {
> +		.name		= "OMAP",
> +		.fifo_size	= 64,
> +		.tx_loadsz	= 64,
> +		.flags		= UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP,
> +	},

I don't think you need this. Reasons below...

>  	[PORT_TEGRA] = {
>  		.name		= "Tegra",
>  		.fifo_size	= 32,
> @@ -1390,6 +1396,8 @@ static void serial8250_stop_rx(struct uart_port *port)
>  	serial8250_rpm_get(up);
>  
>  	up->ier &= ~UART_IER_RLSI;
> +	if (port->type == PORT_OMAP_16750)
> +		up->ier &= ~UART_IER_RDI;

I don't see any reason why this could not be always cleared regardless
of the type:

        up->ier &= ~(UART_IER_RLSI | UART_IRE_RDI);

>  	up->port.read_status_mask &= ~UART_LSR_DR;
>  	serial_port_out(port, UART_IER, up->ier);
>  
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> new file mode 100644
> index 0000000..368e9d8
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -0,0 +1,911 @@
> +/*
> + * 8250-core based driver for the OMAP internal UART
> + *
> + *  Copyright (C) 2014 Sebastian Andrzej Siewior
> + *
> + */

[cut]

> +static int omap8250_probe(struct platform_device *pdev)
> +{
> +	struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	struct resource *irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +	struct omap8250_priv *priv;
> +	struct uart_8250_port up;
> +	int ret;
> +	void __iomem *membase;
> +
> +	if (!regs || !irq) {
> +		dev_err(&pdev->dev, "missing registers or irq\n");
> +		return -EINVAL;
> +	}
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv) {
> +		dev_err(&pdev->dev, "unable to allocate private data\n");
> +		return -ENOMEM;
> +	}
> +	membase = devm_ioremap_nocache(&pdev->dev, regs->start,
> +			resource_size(regs));
> +	if (!membase)
> +		return -ENODEV;
> +
> +	memset(&up, 0, sizeof(up));
> +	up.port.dev = &pdev->dev;
> +	up.port.mapbase = regs->start;
> +	up.port.membase = membase;
> +	up.port.irq = irq->start;
> +	/*
> +	 * It claims to be 16C750 compatible however it is a little different.
> +	 * It has EFR and has no FCR7_64byte bit. The AFE which it claims to
> +	 * have is enabled via EFR instead of MCR.
> +	 */
> +	up.port.type = PORT_OMAP_16750;

Since you are not calling serial8250_do_set_termios, 8250_core.c newer
overrides the FCR set in this driver. However, if the FCR is a
problem, since Yoshihiro added the member for it to struct
uart_8250_port (commit aef9a7bd9), just make it possible for the probe
drivers to provide also it's value:

--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2829,7 +2829,9 @@ static void serial8250_config_port(struct uart_port *port, int flags)
                port->handle_irq = exar_handle_irq;
 
        register_dev_spec_attr_grp(up);
-       up->fcr = uart_config[up->port.type].fcr;
+
+       if (!up->fcr)
+               up->fcr = uart_config[up->port.type].fcr;
 }
 
 static int

So instead of using PORT_OMAP_16750:
        up.port.type = PORT_16750;
        up.capabilities = UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP;

and the fcr if needed.
        up.fcr = ...

> +	up.port.iotype = UPIO_MEM32;
> +	up.port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE |
> +		UPF_SOFT_FLOW | UPF_HARD_FLOW;
> +	up.port.private_data = priv;
> +
> +	up.port.regshift = 2;
> +	up.port.fifosize = 64;

You don't need to set the fifosize unless you want to replace the
value you get from uart_config array.

> +	up.port.set_termios = omap_8250_set_termios;
> +	up.port.pm = omap_8250_pm;
> +	up.port.startup = omap_8250_startup;
> +	up.port.shutdown = omap_8250_shutdown;
> +	up.port.throttle = omap_8250_throttle;
> +	up.port.unthrottle = omap_8250_unthrottle;
> +
> +	if (pdev->dev.of_node) {
> +		up.port.line = of_alias_get_id(pdev->dev.of_node, "serial");
> +		of_property_read_u32(pdev->dev.of_node, "clock-frequency",
> +				&up.port.uartclk);
> +		priv->wakeirq = irq_of_parse_and_map(pdev->dev.of_node, 1);
> +	} else {
> +		up.port.line = pdev->id;
> +	}
> +
> +	if (up.port.line < 0) {
> +		dev_err(&pdev->dev, "failed to get alias/pdev id, errno %d\n",
> +				up.port.line);
> +		return -ENODEV;
> +	}
> +	if (!up.port.uartclk) {
> +		up.port.uartclk = DEFAULT_CLK_SPEED;
> +		dev_warn(&pdev->dev,
> +				"No clock speed specified: using default: %d\n",
> +				DEFAULT_CLK_SPEED);
> +	}
> +
> +#ifdef CONFIG_PM_RUNTIME
> +	up.capabilities |= UART_CAP_RPM;
> +#endif

By setting this here, you will not get the capabilities from the
uart_config[type].flags if runtime PM is enabled in any case, right?

[cut]

> diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
> index 36d68d0..4bac392 100644
> --- a/drivers/tty/serial/8250/Makefile
> +++ b/drivers/tty/serial/8250/Makefile
> @@ -20,3 +20,4 @@ obj-$(CONFIG_SERIAL_8250_HUB6)		+= 8250_hub6.o
>  obj-$(CONFIG_SERIAL_8250_FSL)		+= 8250_fsl.o
>  obj-$(CONFIG_SERIAL_8250_DW)		+= 8250_dw.o
>  obj-$(CONFIG_SERIAL_8250_EM)		+= 8250_em.o
> +obj-$(CONFIG_SERIAL_8250_OMAP)		+= 8250_omap.o
> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> index 5820269..74f9b11 100644
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -54,7 +54,8 @@
>  #define PORT_ALTR_16550_F32 26	/* Altera 16550 UART with 32 FIFOs */
>  #define PORT_ALTR_16550_F64 27	/* Altera 16550 UART with 64 FIFOs */
>  #define PORT_ALTR_16550_F128 28 /* Altera 16550 UART with 128 FIFOs */
> -#define PORT_MAX_8250	28	/* max port ID */
> +#define PORT_OMAP_16750	29	/* TI's OMAP internal 16C750 compatible UART */
> +#define PORT_MAX_8250	29	/* max port ID */

So in this case I see no reason for new type.

Cheers,

-- 
heikki

Re: [PATCH 05/15] tty: serial: Add 8250-core based omap driver

[Sebastian Andrzej Siewior]

On 08/18/2014 03:46 PM, Heikki Krogerus wrote:
> On Fri, Aug 15, 2014 at 07:42:33PM +0200, Sebastian Andrzej Siewior wrote:
>> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
>> index cc90c19..ab003b6 100644
>> --- a/drivers/tty/serial/8250/8250_core.c
>> +++ b/drivers/tty/serial/8250/8250_core.c
>> @@ -264,6 +264,12 @@ static const struct serial8250_config uart_config[] = {
>>  		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
>>  		.flags		= UART_CAP_FIFO | UART_CAP_AFE,
>>  	},
>> +	[PORT_OMAP_16750] = {
>> +		.name		= "OMAP",
>> +		.fifo_size	= 64,
>> +		.tx_loadsz	= 64,
>> +		.flags		= UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP,
>> +	},
> 
> I don't think you need this. Reasons below...

For those it works. However I have to this value to something because
it can't stay PORT_UNKNOWN. So for now I took PORT_8250 because it is
not used anywhere in the code. The only side effect of this is that I
can't specify the name. I can live with this…

> 
>>  	[PORT_TEGRA] = {
>>  		.name		= "Tegra",
>>  		.fifo_size	= 32,
>> @@ -1390,6 +1396,8 @@ static void serial8250_stop_rx(struct uart_port *port)
>>  	serial8250_rpm_get(up);
>>  
>>  	up->ier &= ~UART_IER_RLSI;
>> +	if (port->type == PORT_OMAP_16750)
>> +		up->ier &= ~UART_IER_RDI;
> 
> I don't see any reason why this could not be always cleared regardless
> of the type:
> 
>         up->ier &= ~(UART_IER_RLSI | UART_IRE_RDI);
> 

I remember you brought that up recently asking Alan if it is okay doing
so. Since it looks sane to revert that bit on RX-stop, I will drop that
omap check here.

> [cut]
> 
> Since you are not calling serial8250_do_set_termios, 8250_core.c newer
> overrides the FCR set in this driver. However, if the FCR is a
> problem, since Yoshihiro added the member for it to struct
> uart_8250_port (commit aef9a7bd9), just make it possible for the probe
> drivers to provide also it's value:
> 
>  static int
> 
> So instead of using PORT_OMAP_16750:
>         up.port.type = PORT_16750;
>         up.capabilities = UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP;
> 
> and the fcr if needed.
>         up.fcr = ...
> 

That fcr value looks nice so I can't drop my private copy of it.  But
this FCR works different for RX trigger (the way it is used). Which
means to support user configurable RX-level I would need to overwrite
that callback. However since PORT_8250 does not supply any FCR values,
I can just ignore it for now.

>> +	up.port.iotype = UPIO_MEM32;
>> +	up.port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE |
>> +		UPF_SOFT_FLOW | UPF_HARD_FLOW;
>> +	up.port.private_data = priv;
>> +
>> +	up.port.regshift = 2;
>> +	up.port.fifosize = 64;
> 
> You don't need to set the fifosize unless you want to replace the
> value you get from uart_config array.
Since you made me drop my uart_config array entry I keep this and add
the other values, too

>> +	up.port.set_termios = omap_8250_set_termios;
>> +	up.port.pm = omap_8250_pm;
>> +	up.port.startup = omap_8250_startup;
>> +	up.port.shutdown = omap_8250_shutdown;
>> +	up.port.throttle = omap_8250_throttle;
>> +	up.port.unthrottle = omap_8250_unthrottle;
>> +
>> +	if (pdev->dev.of_node) {
>> +		up.port.line = of_alias_get_id(pdev->dev.of_node, "serial");
>> +		of_property_read_u32(pdev->dev.of_node, "clock-frequency",
>> +				&up.port.uartclk);
>> +		priv->wakeirq = irq_of_parse_and_map(pdev->dev.of_node, 1);
>> +	} else {
>> +		up.port.line = pdev->id;
>> +	}
>> +
>> +	if (up.port.line < 0) {
>> +		dev_err(&pdev->dev, "failed to get alias/pdev id, errno %d\n",
>> +				up.port.line);
>> +		return -ENODEV;
>> +	}
>> +	if (!up.port.uartclk) {
>> +		up.port.uartclk = DEFAULT_CLK_SPEED;
>> +		dev_warn(&pdev->dev,
>> +				"No clock speed specified: using default: %d\n",
>> +				DEFAULT_CLK_SPEED);
>> +	}
>> +
>> +#ifdef CONFIG_PM_RUNTIME
>> +	up.capabilities |= UART_CAP_RPM;
>> +#endif
> 
> By setting this here, you will not get the capabilities from the
> uart_config[type].flags if runtime PM is enabled in any case, right?

Yes. It was not plan to behave like this and is fixed, thanks.

> [cut]
> 
>> diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
>> index 36d68d0..4bac392 100644
>> --- a/drivers/tty/serial/8250/Makefile
>> +++ b/drivers/tty/serial/8250/Makefile
>> @@ -20,3 +20,4 @@ obj-$(CONFIG_SERIAL_8250_HUB6)		+= 8250_hub6.o
>>  obj-$(CONFIG_SERIAL_8250_FSL)		+= 8250_fsl.o
>>  obj-$(CONFIG_SERIAL_8250_DW)		+= 8250_dw.o
>>  obj-$(CONFIG_SERIAL_8250_EM)		+= 8250_em.o
>> +obj-$(CONFIG_SERIAL_8250_OMAP)		+= 8250_omap.o
>> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
>> index 5820269..74f9b11 100644
>> --- a/include/uapi/linux/serial_core.h
>> +++ b/include/uapi/linux/serial_core.h
>> @@ -54,7 +54,8 @@
>>  #define PORT_ALTR_16550_F32 26	/* Altera 16550 UART with 32 FIFOs */
>>  #define PORT_ALTR_16550_F64 27	/* Altera 16550 UART with 64 FIFOs */
>>  #define PORT_ALTR_16550_F128 28 /* Altera 16550 UART with 128 FIFOs */
>> -#define PORT_MAX_8250	28	/* max port ID */
>> +#define PORT_OMAP_16750	29	/* TI's OMAP internal 16C750 compatible UART */
>> +#define PORT_MAX_8250	29	/* max port ID */
> 
> So in this case I see no reason for new type.

gone.

> 
> Cheers,
> 


Sebastian

[PATCH 06/15] tty: serial: 8250_dma: handle error on TX submit

[Sebastian Andrzej Siewior]

Right now it is possible that serial8250_tx_dma() fails and returns
-EBUSY. The caller (serial8250_start_tx()) will then enable
UART_IER_THRI which will generate an interrupt once the TX FIFO is
empty.
In serial8250_handle_irq() nothing will happen because up->dma is set
and so serial8250_tx_chars() won't be invoked. We end up with plenty of
interrupts and some "too much work for irq" output.

This patch introduces dma_tx_err in struct uart_8250_port to signal that
the last invocation of serial8250_tx_dma() failed so we can fill the TX
FIFO manually. Should the next invocation of serial8250_start_tx()
succeed then the dma_tx_err flag along with the THRI bit is removed and
DMA only usage may continue.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/tty/serial/8250/8250_core.c |  2 +-
 drivers/tty/serial/8250/8250_dma.c  | 30 +++++++++++++++++++++++++-----
 include/linux/serial_8250.h         |  1 +
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index ab003b6..7498ce1 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1600,7 +1600,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
 			status = serial8250_rx_chars(up, status);
 	}
 	serial8250_modem_status(up);
-	if (!up->dma && (status & UART_LSR_THRE))
+	if ((!up->dma || up->dma_tx_err) && (status & UART_LSR_THRE))
 		serial8250_tx_chars(up);
 
 	spin_unlock_irqrestore(&port->lock, flags);
diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index 148ffe4..5c59b15 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -36,8 +36,16 @@ static void __dma_tx_complete(void *param)
 	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
 		uart_write_wakeup(&p->port);
 
-	if (!uart_circ_empty(xmit) && !uart_tx_stopped(&p->port))
-		serial8250_tx_dma(p);
+	if (!uart_circ_empty(xmit) && !uart_tx_stopped(&p->port)) {
+		int ret;
+
+		ret = serial8250_tx_dma(p);
+		if (ret) {
+			p->dma_tx_err = 1;
+			p->ier |= UART_IER_THRI;
+			serial_port_out(&p->port, UART_IER, p->ier);
+		}
+	}
 
 	spin_unlock_irqrestore(&p->port.lock, flags);
 }
@@ -69,6 +77,7 @@ int serial8250_tx_dma(struct uart_8250_port *p)
 	struct uart_8250_dma		*dma = p->dma;
 	struct circ_buf			*xmit = &p->port.state->xmit;
 	struct dma_async_tx_descriptor	*desc;
+	int ret;
 
 	if (uart_tx_stopped(&p->port) || dma->tx_running ||
 	    uart_circ_empty(xmit))
@@ -80,8 +89,10 @@ int serial8250_tx_dma(struct uart_8250_port *p)
 					   dma->tx_addr + xmit->tail,
 					   dma->tx_size, DMA_MEM_TO_DEV,
 					   DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
-	if (!desc)
-		return -EBUSY;
+	if (!desc) {
+		ret = -EBUSY;
+		goto err;
+	}
 
 	dma->tx_running = 1;
 
@@ -94,8 +105,17 @@ int serial8250_tx_dma(struct uart_8250_port *p)
 				   UART_XMIT_SIZE, DMA_TO_DEVICE);
 
 	dma_async_issue_pending(dma->txchan);
-
+	if (p->dma_tx_err) {
+		p->dma_tx_err = 0;
+		if (p->ier & UART_IER_THRI) {
+			p->ier &= ~UART_IER_THRI;
+			serial_out(p, UART_IER, p->ier);
+		}
+	}
 	return 0;
+err:
+	p->dma_tx_err = 1;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(serial8250_tx_dma);
 
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index b161eee..02e82dc 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -85,6 +85,7 @@ struct uart_8250_port {
 	unsigned char		mcr_force;	/* mask of forced bits */
 	unsigned char		cur_iotype;	/* Running I/O type */
 	unsigned char		rpm_tx_active;
+	unsigned char		dma_tx_err;
 
 	/*
 	 * Some bits in registers are cleared on a read, so they must
-- 
2.0.1

Re: [PATCH 06/15] tty: serial: 8250_dma: handle error on TX submit

[Heikki Krogerus]

On Fri, Aug 15, 2014 at 07:42:34PM +0200, Sebastian Andrzej Siewior wrote:
> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> index b161eee..02e82dc 100644
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -85,6 +85,7 @@ struct uart_8250_port {
>  	unsigned char		mcr_force;	/* mask of forced bits */
>  	unsigned char		cur_iotype;	/* Running I/O type */
>  	unsigned char		rpm_tx_active;
> +	unsigned char		dma_tx_err;

Why not make this member of uart_8250_dma instead?

-- 
heikki

Re: [PATCH 06/15] tty: serial: 8250_dma: handle error on TX submit

[Sebastian Andrzej Siewior]

On 08/18/2014 03:57 PM, Heikki Krogerus wrote:
> On Fri, Aug 15, 2014 at 07:42:34PM +0200, Sebastian Andrzej Siewior wrote:
>> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
>> index b161eee..02e82dc 100644
>> --- a/include/linux/serial_8250.h
>> +++ b/include/linux/serial_8250.h
>> @@ -85,6 +85,7 @@ struct uart_8250_port {
>>  	unsigned char		mcr_force;	/* mask of forced bits */
>>  	unsigned char		cur_iotype;	/* Running I/O type */
>>  	unsigned char		rpm_tx_active;
>> +	unsigned char		dma_tx_err;
> 
> Why not make this member of uart_8250_dma instead?

Why not indeed. Fixed.

Sebastian

[PATCH 07/15] tty: serial: 8250_dma: enqueue RX dma again on completion.

[Sebastian Andrzej Siewior]

The omap needs a DMA request pending right away. If it is enqueued once
the bytes are in the FIFO then nothing will happen and the FIFO will be
later purged via RX-timeout interrupt.
This patch enqueues RX-DMA request on completion but not if it was
aborted on error. The first enqueue will happen in the driver in
startup.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/tty/serial/8250/8250_core.c |  3 +++
 drivers/tty/serial/8250/8250_dma.c  | 12 +++++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 7498ce1..ab2de27 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1598,6 +1598,9 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
 
 		if (!up->dma || dma_err)
 			status = serial8250_rx_chars(up, status);
+
+		if (dma_err && port->type == PORT_OMAP_16750)
+			serial8250_rx_dma(up, 0);
 	}
 	serial8250_modem_status(up);
 	if ((!up->dma || up->dma_tx_err) && (status & UART_LSR_THRE))
diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index 5c59b15..7417ed5 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -50,9 +50,8 @@ static void __dma_tx_complete(void *param)
 	spin_unlock_irqrestore(&p->port.lock, flags);
 }
 
-static void __dma_rx_complete(void *param)
+static void __dma_rx_do_complete(struct uart_8250_port *p, bool error)
 {
-	struct uart_8250_port	*p = param;
 	struct uart_8250_dma	*dma = p->dma;
 	struct tty_port		*tty_port = &p->port.state->port;
 	struct dma_tx_state	state;
@@ -68,10 +67,17 @@ static void __dma_rx_complete(void *param)
 
 	tty_insert_flip_string(tty_port, dma->rx_buf, count);
 	p->port.icount.rx += count;
+	if (!error && p->port.type == PORT_OMAP_16750)
+		serial8250_rx_dma(p, 0);
 
 	tty_flip_buffer_push(tty_port);
 }
 
+static void __dma_rx_complete(void *param)
+{
+	__dma_rx_do_complete(param, false);
+}
+
 int serial8250_tx_dma(struct uart_8250_port *p)
 {
 	struct uart_8250_dma		*dma = p->dma;
@@ -139,7 +145,7 @@ int serial8250_rx_dma(struct uart_8250_port *p, unsigned int iir)
 		 */
 		if (dma_status == DMA_IN_PROGRESS) {
 			dmaengine_pause(dma->rxchan);
-			__dma_rx_complete(p);
+			__dma_rx_do_complete(p, true);
 		}
 		return -ETIMEDOUT;
 	default:
-- 
2.0.1

Re: [PATCH 07/15] tty: serial: 8250_dma: enqueue RX dma again on completion.

[One Thousand Gnomes]

>  		if (!up->dma || dma_err)
>  			status = serial8250_rx_chars(up, status);
> +
> +		if (dma_err && port->type == PORT_OMAP_16750)
> +			serial8250_rx_dma(up, 0);

Can we stick to a 'has dma' flag and port->rx_dma() type usages so that
we don't have to rewrite it again to add them the next slightly odd DMA
user we add 8)

Re: [PATCH 07/15] tty: serial: 8250_dma: enqueue RX dma again on completion.

[Sebastian Andrzej Siewior]

On 08/18/2014 12:52 PM, One Thousand Gnomes wrote:
> 
>>  		if (!up->dma || dma_err)
>>  			status = serial8250_rx_chars(up, status);
>> +
>> +		if (dma_err && port->type == PORT_OMAP_16750)
>> +			serial8250_rx_dma(up, 0);
> 
> Can we stick to a 'has dma' flag and port->rx_dma() type usages so that
> we don't have to rewrite it again to add them the next slightly odd DMA
> user we add 8)

I hide this behind a bug flag, something like UART_NEEDS_DMA_RX_PENDING.

Sebastian

[PATCH 08/15] dmaengine: edma: fix two faults which happen with the 8250_dma user

[Sebastian Andrzej Siewior]

The rx path of the 8250_dma user in the RX-timeout case:
- it starts the RX transfer
  if the rx-timeout interrupt occurs, it dmaengine_pause() the transfer
- step two is dmaengine_terminate_all() on this channel.
- based on dmaengine_tx_status() it learns the number of transferred
  bytes.
- the rx interrupt occurs, it does not start the RX-transfer because
  according to dmaengine_tx_status() the status of the current transfer
  is still DMA_IN_PROGRESS because the earlier dmaengine_terminate_all()
  did not reset this.
- on rx-timeout it invokes dmaengine_pause() again. This time, it
  segfaults because the channel has no descriptor yet.

To make the upper case work better, this patch adds dma_cookie_complete()
to complete the cookie. Also it adds is an additional check for echan->edesc
in case the channel has no descriptor assigned.

Cc: Joel Fernandes <joelf@ti.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/dma/edma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index d08c4de..ff05dd4 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -256,6 +256,7 @@ static int edma_terminate_all(struct edma_chan *echan)
 	 * echan->edesc is NULL and exit.)
 	 */
 	if (echan->edesc) {
+		dma_cookie_complete(&echan->edesc->vdesc.tx);
 		echan->edesc = NULL;
 		edma_stop(echan->ch_num);
 	}
@@ -282,7 +283,7 @@ static int edma_slave_config(struct edma_chan *echan,
 static int edma_dma_pause(struct edma_chan *echan)
 {
 	/* Pause/Resume only allowed with cyclic mode */
-	if (!echan->edesc->cyclic)
+	if (!echan->edesc || !echan->edesc->cyclic)
 		return -EINVAL;
 
 	edma_pause(echan->ch_num);
-- 
2.0.1

[PATCH 09/15] dmaengine: omap-dma: complete the transfer on terminate_all

[Sebastian Andrzej Siewior]

The rx path of the 8250_dma user in the RX-timeout case:
- it starts the RX transfer
- if the rx-timeout interrupt occurs, it dmaengine_pause() the transfer
- step two is dmaengine_terminate_all() on this channel.
- the rx interrupt occurs, it does not start the RX-transfer because
  according to dmaengine_tx_status() the status of the current transfer
  is still DMA_IN_PROGRESS because the earlier dmaengine_terminate_all()
  did not reset this.

To fix this problem, the patch adds dma_cookie_complete() "complete" the
current cookie.

Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/dma/omap-dma.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index b19f04f..81ede01 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -970,6 +970,12 @@ static int omap_dma_terminate_all(struct omap_chan *c)
 
 	/* Prevent this channel being scheduled */
 	spin_lock(&d->lock);
+	if (!c->desc && !list_empty(&c->node)) {
+		struct virt_dma_desc *vd = vchan_next_desc(&c->vc);
+
+		if (vd)
+			dma_cookie_complete(&vd->tx);
+	}
 	list_del_init(&c->node);
 	spin_unlock(&d->lock);
 
@@ -979,6 +985,7 @@ static int omap_dma_terminate_all(struct omap_chan *c)
 	 * c->desc is NULL and exit.)
 	 */
 	if (c->desc) {
+		dma_cookie_complete(&c->desc->vd.tx);
 		c->desc = NULL;
 		/* Avoid stopping the dma twice */
 		if (!c->paused)
-- 
2.0.1

[PATCH 10/15] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx

[Sebastian Andrzej Siewior]

At least on AM335x the following problem exists: Even if the TX FIFO is
empty and a TX transfer is programmed (and started) the UART does not
trigger the DMA transfer.
After $TRESHOLD number of bytes have been written to the FIFO manually the
UART reevaluates the whole situation and decides that now there is enough
room in the FIFO and so the transfer begins.
This problem has not been seen on DRA7 or beaglebone (OMAP3).

The workaround is to use a threshold of one byte, program the DMA
transfer minus one byte and then to put the first byte into the FIFO to
kick start the transfer.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/tty/serial/8250/8250.h     |  1 +
 drivers/tty/serial/8250/8250_dma.c | 18 +++++++++++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 1bcb4b2..d6a3f5c 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -79,6 +79,7 @@ struct serial8250_config {
 #define UART_BUG_NOMSR	(1 << 2)	/* UART has buggy MSR status bits (Au1x00) */
 #define UART_BUG_THRE	(1 << 3)	/* UART has buggy THRE reassertion */
 #define UART_BUG_PARITY	(1 << 4)	/* UART mishandles parity if FIFO enabled */
+#define UART_BUG_DMATX	(1 << 5)	/* UART needs one byte in FIFO for kickstart */
 
 #define PROBE_RSA	(1 << 0)
 #define PROBE_ANY	(~0)
diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index 7417ed5..dc1294b 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -83,6 +83,7 @@ int serial8250_tx_dma(struct uart_8250_port *p)
 	struct uart_8250_dma		*dma = p->dma;
 	struct circ_buf			*xmit = &p->port.state->xmit;
 	struct dma_async_tx_descriptor	*desc;
+	unsigned int			skip_byte = 0;
 	int ret;
 
 	if (uart_tx_stopped(&p->port) || dma->tx_running ||
@@ -91,10 +92,18 @@ int serial8250_tx_dma(struct uart_8250_port *p)
 
 	dma->tx_size = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
 
+	if (p->bugs & UART_BUG_DMATX) {
+		if (dma->tx_size < 4) {
+			ret = -EINVAL;
+			goto err;
+		}
+		skip_byte = 1;
+	}
+
 	desc = dmaengine_prep_slave_single(dma->txchan,
-					   dma->tx_addr + xmit->tail,
-					   dma->tx_size, DMA_MEM_TO_DEV,
-					   DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+			dma->tx_addr + xmit->tail + skip_byte,
+			dma->tx_size - skip_byte, DMA_MEM_TO_DEV,
+			DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
 	if (!desc) {
 		ret = -EBUSY;
 		goto err;
@@ -118,6 +127,9 @@ int serial8250_tx_dma(struct uart_8250_port *p)
 			serial_out(p, UART_IER, p->ier);
 		}
 	}
+	if (skip_byte)
+		serial_out(p, UART_TX, xmit->buf[xmit->tail]);
+
 	return 0;
 err:
 	p->dma_tx_err = 1;
-- 
2.0.1

[PATCH 11/15] tty: serial: 8250_dma: handle the when UART response while DMA remains idle

[Sebastian Andrzej Siewior]

The OMAP UART needs the RX-DMA programmed right away. If we receive a BREAK
then we have to cancel the DMA transfer before we can handle the break
condition. Otherwise, after we handled the "break condition" the DMA
engine will start reading bytes from the FIFO while we also purge the
FIFO manually. The order of what we receive is random.

Also sometimes the OMAP UART does not signal the DMA engine to unload
the FIFO. Usually this happens when we have >threshold bytes in the FIFO
and start the DMA transfer. It seems that in those cases the UART won't
trigger the transfer once the requested threshold is reached. In some
rare cases the UART does not trigger the DMA transfer even if programmed
while the FIFO was empty.
In those cases the UART drops an RDI event and we have to empty the FIFO
manually. If we ignore it because the DMA transfer is programmed then we
will enter the function a few times until we receive the RX_TIMEOUT
event. At that point the FIFO is usually full and we risk to overflow
the FIFO.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/tty/serial/8250/8250_dma.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index dc1294b..cfc83d4 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -149,6 +149,13 @@ int serial8250_rx_dma(struct uart_8250_port *p, unsigned int iir)
 	switch (iir & 0x3f) {
 	case UART_IIR_RLSI:
 		/* 8250_core handles errors and break interrupts */
+		if (p->port.type == PORT_OMAP_16750)
+			/*
+			 * The OMAP has its DMA transfer started before we get
+			 * here. Keeping it enabled will result in data beeing
+			 * read out of order (DMA + manuall FIFO read).
+			 */
+			__dma_rx_do_complete(p, true);
 		return -EIO;
 	case UART_IIR_RX_TIMEOUT:
 		/*
@@ -160,6 +167,21 @@ int serial8250_rx_dma(struct uart_8250_port *p, unsigned int iir)
 			__dma_rx_do_complete(p, true);
 		}
 		return -ETIMEDOUT;
+	case UART_IIR_RDI:
+		if (p->port.type != PORT_OMAP_16750)
+			break;
+		/*
+		 * The OMAP UART is a special BEAST. If we receive RDI we _have_
+		 * a DMA transfer programmed but it didn't worked. One reason is
+		 * that we were too slow and there were too many bytes in the
+		 * FIFO, the UART counted wrong and never kicked the DMA engine
+		 * to do anything. That means once we receive RDI on OMAP than
+		 * the DMA won't do anything soon so we have to cancel the DMA
+		 * transfer and purge the FIFO manually.
+		 */
+		__dma_rx_do_complete(p, true);
+		return -ETIMEDOUT;
+
 	default:
 		break;
 	}
-- 
2.0.1

[PATCH 12/15] tty: serial: 8250_dma: add pm runtime

[Sebastian Andrzej Siewior]

There is nothing to do for RPM in the RX path. If the HW goes off then it
won't assert the DMA line and the transfer won't happen. So we hope that
the HW does not go off for RX to work (DMA or PIO makes no difference
here).

For TX the situation is slightly different. RPM is enabled on
start_tx(). We can't disable RPM on DMA complete callback because there
is still data in the FIFO which is being sent. We have to wait until
the FIFO is empty before we disable it.
For this to happen we fake a TX sent error and enable THRI. Once the
FIFO is empty we receive an interrupt and since the TTY-buffer is still
empty we "put RPM" via __stop_tx(). Should it been filed then in the
start_tx() path we should program the DMA transfer and remove the error
flag and the THRI bit.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/tty/serial/8250/8250_dma.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index cfc83d4..8a26024 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -21,6 +21,7 @@ static void __dma_tx_complete(void *param)
 	struct uart_8250_dma	*dma = p->dma;
 	struct circ_buf		*xmit = &p->port.state->xmit;
 	unsigned long	flags;
+	bool en_thri = false;
 
 	dma_sync_single_for_cpu(dma->txchan->device->dev, dma->tx_addr,
 				UART_XMIT_SIZE, DMA_TO_DEVICE);
@@ -40,11 +41,16 @@ static void __dma_tx_complete(void *param)
 		int ret;
 
 		ret = serial8250_tx_dma(p);
-		if (ret) {
-			p->dma_tx_err = 1;
-			p->ier |= UART_IER_THRI;
-			serial_port_out(&p->port, UART_IER, p->ier);
-		}
+		if (ret)
+			en_thri = true;
+
+	} else if (p->capabilities & UART_CAP_RPM)
+		en_thri = true;
+
+	if (en_thri) {
+		p->dma_tx_err = 1;
+		p->ier |= UART_IER_THRI;
+		serial_port_out(&p->port, UART_IER, p->ier);
 	}
 
 	spin_unlock_irqrestore(&p->port.lock, flags);
-- 
2.0.1

[PATCH 13/15] arm: dts: am33xx: add DMA properties for UART

[Sebastian Andrzej Siewior]

Cc: devicetree@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/arm/boot/dts/am33xx.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 4a4e02d..cdccbd6 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -200,6 +200,8 @@
 			reg = <0x44e09000 0x2000>;
 			interrupts = <72>;
 			status = "disabled";
+			dmas = <&edma 26>, <&edma 27>;
+			dma-names = "tx", "rx";
 		};
 
 		uart1: serial@48022000 {
@@ -209,6 +211,8 @@
 			reg = <0x48022000 0x2000>;
 			interrupts = <73>;
 			status = "disabled";
+			dmas = <&edma 28>, <&edma 29>;
+			dma-names = "tx", "rx";
 		};
 
 		uart2: serial@48024000 {
@@ -218,6 +222,8 @@
 			reg = <0x48024000 0x2000>;
 			interrupts = <74>;
 			status = "disabled";
+			dmas = <&edma 30>, <&edma 31>;
+			dma-names = "tx", "rx";
 		};
 
 		uart3: serial@481a6000 {
-- 
2.0.1

[PATCH 14/15] arm: dts: dra7: add DMA properties for UART

[Sebastian Andrzej Siewior]

Cc: devicetree@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/arm/boot/dts/dra7.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 10066f4..a904561 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -259,6 +259,8 @@
 			ti,hwmods = "uart1";
 			clock-frequency = <48000000>;
 			status = "disabled";
+			dmas = <&sdma 49>, <&sdma 50>;
+			dma-names = "tx", "rx";
 		};
 
 		uart2: serial@4806c000 {
@@ -268,6 +270,8 @@
 			ti,hwmods = "uart2";
 			clock-frequency = <48000000>;
 			status = "disabled";
+			dmas = <&sdma 51>, <&sdma 52>;
+			dma-names = "tx", "rx";
 		};
 
 		uart3: serial@48020000 {
@@ -277,6 +281,8 @@
 			ti,hwmods = "uart3";
 			clock-frequency = <48000000>;
 			status = "disabled";
+			dmas = <&sdma 53>, <&sdma 54>;
+			dma-names = "tx", "rx";
 		};
 
 		uart4: serial@4806e000 {
@@ -286,6 +292,8 @@
 			ti,hwmods = "uart4";
 			clock-frequency = <48000000>;
                         status = "disabled";
+			dmas = <&sdma 55>, <&sdma 56>;
+			dma-names = "tx", "rx";
 		};
 
 		uart5: serial@48066000 {
@@ -295,6 +303,8 @@
 			ti,hwmods = "uart5";
 			clock-frequency = <48000000>;
 			status = "disabled";
+			dmas = <&sdma 63>, <&sdma 64>;
+			dma-names = "tx", "rx";
 		};
 
 		uart6: serial@48068000 {
@@ -304,6 +314,8 @@
 			ti,hwmods = "uart6";
 			clock-frequency = <48000000>;
 			status = "disabled";
+			dmas = <&sdma 79>, <&sdma 80>;
+			dma-names = "tx", "rx";
 		};
 
 		uart7: serial@48420000 {
-- 
2.0.1

[PATCH 15/15] tty: serial: 8250: omap: add dma support

[Sebastian Andrzej Siewior]

This patch adds the required pieces to 8250-OMAP UART driver for DMA
support. The TX burst size is set to 1 so we can send an arbitrary
amount of bytes.

The RX burst is currently set to 48 which means we receive an DMA
interrupt every 48 bytes and have to reprogram everything. Less bytes in
the RX-FIFO mean that no DMA transfer will happen and the UART will send a
RX-timeout _or_ RDI event at which point the FIFO will be manually purged.
There is a workaround for TX-DMA on AM33xx where we put the first byte
into the FIFO to kick start the DMA process. Haven't seen this problem on
OMAP3 (beagle bone) or DRA7xx.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/tty/serial/8250/8250_omap.c | 79 +++++++++++++++++++++++++++++++++++--
 1 file changed, 76 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 368e9d8..dfd2ddd 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -31,10 +31,16 @@
 #define UART_ERRATA_i202_MDR1_ACCESS	(1 << 0)
 #define OMAP_UART_WER_HAS_TX_WAKEUP	(1 << 1)
 
+#define OMAP_UART_FCR_RX_TRIG		6
+#define OMAP_UART_FCR_TX_TRIG		4
+
 /* SCR register bitmasks */
 #define OMAP_UART_SCR_RX_TRIG_GRANU1_MASK	(1 << 7)
 #define OMAP_UART_SCR_TX_TRIG_GRANU1_MASK	(1 << 6)
 #define OMAP_UART_SCR_TX_EMPTY			(1 << 3)
+#define OMAP_UART_SCR_DMAMODE_MASK		(3 << 1)
+#define OMAP_UART_SCR_DMAMODE_1			(1 << 1)
+#define OMAP_UART_SCR_DMAMODE_CTL		(1 << 0)
 
 /* MVR register bitmasks */
 #define OMAP_UART_MVR_SCHEME_SHIFT	30
@@ -45,6 +51,12 @@
 #define OMAP_UART_MVR_MAJ_SHIFT		8
 #define OMAP_UART_MVR_MIN_MASK		0x3f
 
+#define UART_TI752_TLR_TX	0
+#define UART_TI752_TLR_RX	4
+
+#define TRIGGER_TLR_MASK(x)	((x & 0x3c) >> 2)
+#define TRIGGER_FCR_MASK(x)	(x & 3)
+
 /* Enable XON/XOFF flow control on output */
 #define OMAP_UART_SW_TX		0x08
 /* Enable XON/XOFF flow control on input */
@@ -82,6 +94,7 @@ struct omap8250_priv {
 	u32 calc_latency;
 	struct pm_qos_request pm_qos_request;
 	struct work_struct qos_work;
+	struct uart_8250_dma omap8250_dma;
 };
 
 static u32 uart_read(struct uart_8250_port *up, u32 reg)
@@ -162,6 +175,20 @@ static void omap_8250_get_divisor(struct uart_port *port, unsigned int baud,
 	}
 }
 
+static void omap8250_update_scr(struct uart_8250_port *up,
+		struct omap8250_priv *priv)
+{
+	/*
+	 * The manual recommends not to enable the DMA mode selector in the SCR
+	 * (instead of the FCR) register _and_ selecting the DMA mode as one
+	 * register write because this may lead to malfunction.
+	 */
+	if (priv->scr & OMAP_UART_SCR_DMAMODE_MASK)
+		serial_out(up, UART_OMAP_SCR,
+				priv->scr & ~OMAP_UART_SCR_DMAMODE_MASK);
+	serial_out(up, UART_OMAP_SCR, priv->scr);
+}
+
 /*
  * OMAP can use "CLK / (16 or 13) / div" for baud rate. And then we have have
  * some differences in how we want to handle flow control.
@@ -286,6 +313,9 @@ static void omap_8250_set_termios(struct uart_port *port,
 	serial_out(up, UART_TI752_TLR,
 			TRIGGER_TLR_MASK(TX_TRIGGER) << UART_TI752_TLR_TX |
 			TRIGGER_TLR_MASK(RX_TRIGGER) << UART_TI752_TLR_RX);
+	if (up->dma)
+		priv->scr |= OMAP_UART_SCR_DMAMODE_1 |
+			OMAP_UART_SCR_DMAMODE_CTL;
 	/*
 	 * We enable TRIG_GRANU for RX and TX and additionaly we set
 	 * SCR_TX_EMPTY bit. The result is the following:
@@ -294,6 +324,14 @@ static void omap_8250_set_termios(struct uart_port *port,
 	 *   once the UART decides that there no new bytes arriving.
 	 * - Once THRE is enabled, the interrupt will be fired once the FIFO is
 	 *   empty - the trigger level is ignored here.
+	 *
+	 * Once DMA is enabled:
+	 * - UART will assert the TX DMA line once there is room for TX_TRIGGER
+	 *   bytes in the TX FIFO. On each assert the DMA engine will move
+	 *   TX_TRIGGER bytes into the FIFO.
+	 * - UART will assert the RX DMA line once there are RX_TRIGGER bytes in
+	 *   the FIFO and move RX_TRIGGER bytes.
+	 * This is because treshold and trigger values are the same.
 	 */
 	priv->fcr = UART_FCR_ENABLE_FIFO;
 	priv->fcr |= TRIGGER_FCR_MASK(TX_TRIGGER) << OMAP_UART_FCR_TX_TRIG;
@@ -302,7 +340,7 @@ static void omap_8250_set_termios(struct uart_port *port,
 	serial_out(up, UART_FCR, priv->fcr);
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
 
-	serial_out(up, UART_OMAP_SCR, priv->scr);
+	omap8250_update_scr(up, priv);
 
 	/* Reset UART_MCR_TCRTLR: this must be done with the EFR_ECB bit set */
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
@@ -525,6 +563,9 @@ static int omap_8250_startup(struct uart_port *port)
 		priv->wer |= OMAP_UART_TX_WAKEUP_EN;
 	serial_out(up, UART_OMAP_WER, priv->wer);
 
+	if (up->dma)
+		serial8250_rx_dma(up, 0);
+
 	pm_runtime_mark_last_busy(port->dev);
 	pm_runtime_put_autosuspend(port->dev);
 	return 0;
@@ -540,6 +581,8 @@ static void omap_8250_shutdown(struct uart_port *port)
 		container_of(port, struct uart_8250_port, port);
 	struct omap8250_priv *priv = port->private_data;
 
+	if (up->dma)
+		dmaengine_terminate_all(up->dma->rxchan);
 	pm_runtime_get_sync(port->dev);
 
 	serial_out(up, UART_OMAP_WER, 0);
@@ -586,6 +629,13 @@ static void omap_8250_unthrottle(struct uart_port *port)
 	pm_runtime_put_autosuspend(port->dev);
 }
 
+#ifdef CONFIG_SERIAL_8250_DMA
+static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
+{
+	return false;
+}
+#endif
+
 static int omap8250_probe(struct platform_device *pdev)
 {
 	struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -677,7 +727,29 @@ static int omap8250_probe(struct platform_device *pdev)
 	pm_runtime_get_sync(&pdev->dev);
 
 	omap_serial_fill_features_erratas(&up, priv);
-
+#ifdef CONFIG_SERIAL_8250_DMA
+	if (pdev->dev.of_node) {
+		/*
+		 * Oh DMA support. If there are no DMA properties in the DT then
+		 * we will fall back to a generic DMA channel which does not
+		 * really work here. To ensure that we do not get a generic DMA
+		 * channel assigned, we have the the_no_dma_filter_fn() here.
+		 * To avoid "failed to request DMA" messages we check for DMA
+		 * properties in DT.
+		 */
+		ret = of_property_count_strings(pdev->dev.of_node, "dma-names");
+		if (ret == 2) {
+			up.dma = &priv->omap8250_dma;
+			priv->omap8250_dma.fn = the_no_dma_filter_fn;
+			priv->omap8250_dma.rx_size = RX_TRIGGER;
+			priv->omap8250_dma.rxconf.src_maxburst = RX_TRIGGER;
+			priv->omap8250_dma.txconf.dst_maxburst = TX_TRIGGER;
+
+			if (of_machine_is_compatible("ti,am33xx"))
+				up.bugs |= UART_BUG_DMATX;
+		}
+	}
+#endif
 	ret = serial8250_register_8250_port(&up);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "unable to register 8250 port\n");
@@ -848,7 +920,8 @@ static void omap8250_restore_context(struct omap8250_priv *priv)
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
 	serial_out(up, UART_MCR, up->mcr);
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); /* Config B mode */
-	serial_out(up, UART_OMAP_SCR, priv->scr);
+	omap8250_update_scr(up, priv);
+
 	serial_out(up, UART_EFR, priv->efr);
 	serial_out(up, UART_LCR, up->lcr);
 	if (priv->habit & UART_ERRATA_i202_MDR1_ACCESS)
-- 
2.0.1

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Tony Lindgren]

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140815 11:13]:
> +#ifdef CONFIG_SERIAL_8250_DMA
> +	if (pdev->dev.of_node) {
> +		/*
> +		 * Oh DMA support. If there are no DMA properties in the DT then
> +		 * we will fall back to a generic DMA channel which does not
> +		 * really work here. To ensure that we do not get a generic DMA
> +		 * channel assigned, we have the the_no_dma_filter_fn() here.
> +		 * To avoid "failed to request DMA" messages we check for DMA
> +		 * properties in DT.
> +		 */
> +		ret = of_property_count_strings(pdev->dev.of_node, "dma-names");
> +		if (ret == 2) {
> +			up.dma = &priv->omap8250_dma;
> +			priv->omap8250_dma.fn = the_no_dma_filter_fn;
> +			priv->omap8250_dma.rx_size = RX_TRIGGER;
> +			priv->omap8250_dma.rxconf.src_maxburst = RX_TRIGGER;
> +			priv->omap8250_dma.txconf.dst_maxburst = TX_TRIGGER;
> +
> +			if (of_machine_is_compatible("ti,am33xx"))
> +				up.bugs |= UART_BUG_DMATX;
> +		}
> +	}
> +#endif

Hmm I wonder if there's a more generic solution to this?

It would be also nice to be able to specify the use of DMA from
the board specific .dts file.

Also, with DMA enabled, looks like omap deeper idle states are
blocked as the DMA stays reserved. After I commented out the
DMA info for my console UART, PM started working.

Regards,

Tony

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Sebastian Andrzej Siewior]

On 08/15/2014 11:02 PM, Tony Lindgren wrote:
> * Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140815 11:13]:
>> +#ifdef CONFIG_SERIAL_8250_DMA
>> +	if (pdev->dev.of_node) {
>> +		/*
>> +		 * Oh DMA support. If there are no DMA properties in the DT then
>> +		 * we will fall back to a generic DMA channel which does not
>> +		 * really work here. To ensure that we do not get a generic DMA
>> +		 * channel assigned, we have the the_no_dma_filter_fn() here.
>> +		 * To avoid "failed to request DMA" messages we check for DMA
>> +		 * properties in DT.
>> +		 */
>> +		ret = of_property_count_strings(pdev->dev.of_node, "dma-names");
>> +		if (ret == 2) {
>> +			up.dma = &priv->omap8250_dma;
>> +			priv->omap8250_dma.fn = the_no_dma_filter_fn;
>> +			priv->omap8250_dma.rx_size = RX_TRIGGER;
>> +			priv->omap8250_dma.rxconf.src_maxburst = RX_TRIGGER;
>> +			priv->omap8250_dma.txconf.dst_maxburst = TX_TRIGGER;
>> +
>> +			if (of_machine_is_compatible("ti,am33xx"))
>> +				up.bugs |= UART_BUG_DMATX;
>> +		}
>> +	}
>> +#endif
> 
> Hmm I wonder if there's a more generic solution to this?

To what exactly? The trigger level, the TX-bug or using DMA in the
first place?

> It would be also nice to be able to specify the use of DMA from
> the board specific .dts file.

Really? I don't see this requirement for MMC for instance. However you
still could provide an empty "dma-names" property in your board .dts
file if you do not want use DMA. Would this work?

> Also, with DMA enabled, looks like omap deeper idle states are
> blocked as the DMA stays reserved. After I commented out the
> DMA info for my console UART, PM started working.

Hmm. This would explain something. This would mean that I should cancel
the RX DMA transfer in the PM-suspend routine. Let me see how that
works.

> Regards,
> 
> Tony

Sebastian

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Tony Lindgren]

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140821 01:37]:
> On 08/15/2014 11:02 PM, Tony Lindgren wrote:
> > * Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140815 11:13]:
> >> +#ifdef CONFIG_SERIAL_8250_DMA
> >> +	if (pdev->dev.of_node) {
> >> +		/*
> >> +		 * Oh DMA support. If there are no DMA properties in the DT then
> >> +		 * we will fall back to a generic DMA channel which does not
> >> +		 * really work here. To ensure that we do not get a generic DMA
> >> +		 * channel assigned, we have the the_no_dma_filter_fn() here.
> >> +		 * To avoid "failed to request DMA" messages we check for DMA
> >> +		 * properties in DT.
> >> +		 */
> >> +		ret = of_property_count_strings(pdev->dev.of_node, "dma-names");
> >> +		if (ret == 2) {
> >> +			up.dma = &priv->omap8250_dma;
> >> +			priv->omap8250_dma.fn = the_no_dma_filter_fn;
> >> +			priv->omap8250_dma.rx_size = RX_TRIGGER;
> >> +			priv->omap8250_dma.rxconf.src_maxburst = RX_TRIGGER;
> >> +			priv->omap8250_dma.txconf.dst_maxburst = TX_TRIGGER;
> >> +
> >> +			if (of_machine_is_compatible("ti,am33xx"))
> >> +				up.bugs |= UART_BUG_DMATX;
> >> +		}
> >> +	}
> >> +#endif
> > 
> > Hmm I wonder if there's a more generic solution to this?
> 
> To what exactly? The trigger level, the TX-bug or using DMA in the
> first place?

Oh sorry, I meant to having to do of_property_count_strings to
figure out if it's correct or not. 
 
> > It would be also nice to be able to specify the use of DMA from
> > the board specific .dts file.
> 
> Really? I don't see this requirement for MMC for instance. However you
> still could provide an empty "dma-names" property in your board .dts
> file if you do not want use DMA. Would this work?

OK yeah that works. And that's needed mostly because of the issue
below.
 
> > Also, with DMA enabled, looks like omap deeper idle states are
> > blocked as the DMA stays reserved. After I commented out the
> > DMA info for my console UART, PM started working.
> 
> Hmm. This would explain something. This would mean that I should cancel
> the RX DMA transfer in the PM-suspend routine. Let me see how that
> works.

OK and if the DMA works with PM, then I don't see why we would not
want to have it automatically enabled.

BTW, looks like the ports move around now though. If set a port
to disabled with status = "disabled"; in the .dts file, you'll get
a different console which does not happen with omap-serial I believe.

Regards,

Tony

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Sebastian Andrzej Siewior]

On 08/21/2014 08:44 PM, Tony Lindgren wrote:
>>> Also, with DMA enabled, looks like omap deeper idle states are
>>> blocked as the DMA stays reserved. After I commented out the
>>> DMA info for my console UART, PM started working.
>>
>> Hmm. This would explain something. This would mean that I should cancel
>> the RX DMA transfer in the PM-suspend routine. Let me see how that
>> works.
> 
> OK and if the DMA works with PM, then I don't see why we would not
> want to have it automatically enabled.

I re-did that part where the registers are restored. Mostly for that
reason to use function in runtime_resume() as in set_termios(). I think
that is cute :) _And_ if somebody changes here something and breaks it
then it doesn't work with and without runtime-pm. It looks like the
omap-serial doesn't restore the XON1 & XOFF1 registers.

While at it I made sure that it works as good as I could and that means:

core_pwrdm
(ON),OFF:182,RET:21,INA:131,ON:335,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0

The core off part with DMA looks like a no no:
I #if 0 the block in where it assigned up.dma. With this I hit
core-off. Step two was

|static void omap8250_update_scr(struct uart_8250_port *up,
|                 struct omap8250_priv *priv)
|{
|serial_out(up, UART_OMAP_SCR, priv->scr | OMAP_UART_SCR_DMAMODE_CTL);
|serial_out(up, UART_OMAP_SCR, priv->scr | OMAP_UART_SCR_DMAMODE_CTL |
OMAP_UART_SCR_DMAMODE_1);
|serial_out(up, UART_OMAP_SCR, priv->scr | |OMAP_UART_SCR_DMAMODE_CTL);
|serial_out(up, UART_OMAP_SCR, priv->scr);
|}

which means I just enable DMA mode in UART and disable it. No DMA
operations were performed.
With this change I see a lost character now and then which means the
UART-IP goes into off and loses its context. Good. However I don't see
core off anymore. This looks like a bug beyond my responsibilities :)

I added code to cancel & and start DMA transfers in runtime suspend
callbacks.
However core-off with DMA won't work. I think we could document this in
the binding document. What do you think?

> BTW, looks like the ports move around now though. If set a port
> to disabled with status = "disabled"; in the .dts file, you'll get
> a different console which does not happen with omap-serial I believe.

You a right. I fixed it in the 8250-core code.

> Regards,
> 
> Tony

Sebastian

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Tony Lindgren]

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140827 12:54]:
> On 08/21/2014 08:44 PM, Tony Lindgren wrote:
> >>> Also, with DMA enabled, looks like omap deeper idle states are
> >>> blocked as the DMA stays reserved. After I commented out the
> >>> DMA info for my console UART, PM started working.
> >>
> >> Hmm. This would explain something. This would mean that I should cancel
> >> the RX DMA transfer in the PM-suspend routine. Let me see how that
> >> works.
> > 
> > OK and if the DMA works with PM, then I don't see why we would not
> > want to have it automatically enabled.
> 
> I re-did that part where the registers are restored. Mostly for that
> reason to use function in runtime_resume() as in set_termios(). I think
> that is cute :) _And_ if somebody changes here something and breaks it
> then it doesn't work with and without runtime-pm. It looks like the
> omap-serial doesn't restore the XON1 & XOFF1 registers.
> 
> While at it I made sure that it works as good as I could and that means:
> 
> core_pwrdm
> (ON),OFF:182,RET:21,INA:131,ON:335,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0

Hey that's great, that's the ultimate torture test here!
There's nothing like rebooting the system every time you hit
idle and still have drivers working :)
 
> The core off part with DMA looks like a no no:
> I #if 0 the block in where it assigned up.dma. With this I hit
> core-off. Step two was
> 
> |static void omap8250_update_scr(struct uart_8250_port *up,
> |                 struct omap8250_priv *priv)
> |{
> |serial_out(up, UART_OMAP_SCR, priv->scr | OMAP_UART_SCR_DMAMODE_CTL);
> |serial_out(up, UART_OMAP_SCR, priv->scr | OMAP_UART_SCR_DMAMODE_CTL |
> OMAP_UART_SCR_DMAMODE_1);
> |serial_out(up, UART_OMAP_SCR, priv->scr | |OMAP_UART_SCR_DMAMODE_CTL);
> |serial_out(up, UART_OMAP_SCR, priv->scr);
> |}
> 
> which means I just enable DMA mode in UART and disable it. No DMA
> operations were performed.
> With this change I see a lost character now and then which means the
> UART-IP goes into off and loses its context. Good. However I don't see
> core off anymore. This looks like a bug beyond my responsibilities :)

OK, that sounds like a bug still lurking around somewhere. The core
domain won't hit idle if there are any hardware pieces blocking.

> I added code to cancel & and start DMA transfers in runtime suspend
> callbacks.

Do you mean just the OMAP_UART_SCR_DMAMODE_CTL related code, or
also the dmaengine calls?

> However core-off with DMA won't work. I think we could document this in
> the binding document. What do you think?

There should not be such a limitation though. Maybe dump out the values
of cm_idlest_per and cm_idlest1_core for working and failing off idle
cases and see what the difference is?

It could be the either the dma or the uart hardware blocking. I guess
it could be also an issue with runtime pm use somewhere.

> > BTW, looks like the ports move around now though. If set a port
> > to disabled with status = "disabled"; in the .dts file, you'll get
> > a different console which does not happen with omap-serial I believe.
> 
> You a right. I fixed it in the 8250-core code.

OK thanks.

Tony

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Sebastian Andrzej Siewior]

* Tony Lindgren | 2014-08-27 13:23:14 [-0700]:

>> which means I just enable DMA mode in UART and disable it. No DMA
>> operations were performed.
>> With this change I see a lost character now and then which means the
>> UART-IP goes into off and loses its context. Good. However I don't see
>> core off anymore. This looks like a bug beyond my responsibilities :)
>
>OK, that sounds like a bug still lurking around somewhere. The core
>domain won't hit idle if there are any hardware pieces blocking.

Yes.

>> I added code to cancel & and start DMA transfers in runtime suspend
>> callbacks.
>
>Do you mean just the OMAP_UART_SCR_DMAMODE_CTL related code, or
>also the dmaengine calls?

dmaengine calls are unused because up.dma is not assigned. It is 
basically like you wouldn't have the dma properties in the devicetree.
And while in that non-DMA mode I just set and unset the DMAMODE_CTL + 
DMAMODE_1 bits in the SCR register. Nothing else. Based on some testing
I just did, DMAMODE_CTL does not make the difference. DMAMODE_CTL +
DMAMODE_1 does.

>> However core-off with DMA won't work. I think we could document this in
>> the binding document. What do you think?
>
>There should not be such a limitation though. Maybe dump out the values
>of cm_idlest_per and cm_idlest1_core for working and failing off idle
>cases and see what the difference is?

I can't follow here. This is the working case:

usbhost_pwrdm (OFF),OFF:1,RET:1,INA:0,ON:2,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
sgx_pwrdm (OFF),OFF:1,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
core_pwrdm (ON),OFF:109,RET:22,INA:71,ON:203,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0
per_pwrdm (ON),OFF:137,RET:126,INA:0,ON:264,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
dss_pwrdm (ON),OFF:383,RET:861,INA:0,ON:1245,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
cam_pwrdm (OFF),OFF:1,RET:1,INA:0,ON:2,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
neon_pwrdm (ON),OFF:311,RET:889,INA:44,ON:1245,RET-LOGIC-OFF:0
mpu_pwrdm (ON),OFF:311,RET:889,INA:44,ON:1245,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
iva2_pwrdm (OFF),OFF:1,RET:1,INA:0,ON:2,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0,RET-MEMBANK3-OFF:0,RET-MEMBANK4-OFF:0
usbhost_clkdm->usbhost_pwrdm (0)
sgx_clkdm->sgx_pwrdm (0)
per_clkdm->per_pwrdm (15)
cam_clkdm->cam_pwrdm (0)
dss_clkdm->dss_pwrdm (1)
d2d_clkdm->core_pwrdm (0)
iva2_clkdm->iva2_pwrdm (0)
mpu_clkdm->mpu_pwrdm (0)
core_l4_clkdm->core_pwrdm (20)
core_l3_clkdm->core_pwrdm (1)
neon_clkdm->neon_pwrdm (0)

non-working:

usbhost_pwrdm (OFF),OFF:1,RET:1,INA:0,ON:2,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
sgx_pwrdm (OFF),OFF:1,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
core_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0
per_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
dss_pwrdm (ON),OFF:253,RET:159,INA:0,ON:413,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
cam_pwrdm (OFF),OFF:1,RET:1,INA:0,ON:2,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
neon_pwrdm (ON),OFF:135,RET:242,INA:35,ON:413,RET-LOGIC-OFF:0
mpu_pwrdm (ON),OFF:135,RET:242,INA:35,ON:413,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
iva2_pwrdm (OFF),OFF:1,RET:1,INA:0,ON:2,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0,RET-MEMBANK3-OFF:0,RET-MEMBANK4-OFF:0
usbhost_clkdm->usbhost_pwrdm (0)
sgx_clkdm->sgx_pwrdm (0)
per_clkdm->per_pwrdm (15)
cam_clkdm->cam_pwrdm (0)
dss_clkdm->dss_pwrdm (1)
d2d_clkdm->core_pwrdm (0)
iva2_clkdm->iva2_pwrdm (0)
mpu_clkdm->mpu_pwrdm (0)
core_l4_clkdm->core_pwrdm (21)
core_l3_clkdm->core_pwrdm (1)
neon_clkdm->neon_pwrdm (0)

so per_pwrdm and core_pwrdm remain on and I guess the former is where
the UART sits.

>It could be the either the dma or the uart hardware blocking. I guess
>it could be also an issue with runtime pm use somewhere.

So I just toggle the two DMA bits in SCR and the UART seems to block
since the DMA hw is not involved. Reading SCR back says that those bits
are not set.
To use DMA you don't have to enable it in SCR register you can also use
the FCR register. The manual says that you can only write this DMA
enable bit in the FCR register if the baud clock is not running. And
guess what: same thing: I only *toggle* the DMA enable bit here (it
remains 0 later) and the core won't hit idle.
Same effect if I toggle this bit while the baud clock is running (the
manual says that this bit can only be written if the baud clock is not
running). Seems like the UART is following its own specification and it
remains blocking once the DMA was enabled.
It would be nice if someone from the UART-IP team could ACK this.

Bah. Does it make sense to use runtime-PM if we can't hit core-off? I'm
thinking to add a printk once dma is enabled says that runtime-pm is
switched off.

>Tony

Sebastian

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Tony Lindgren]

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140828 01:24]:
> * Tony Lindgren | 2014-08-27 13:23:14 [-0700]:
> >
> >Do you mean just the OMAP_UART_SCR_DMAMODE_CTL related code, or
> >also the dmaengine calls?
> 
> dmaengine calls are unused because up.dma is not assigned. It is 
> basically like you wouldn't have the dma properties in the devicetree.
> And while in that non-DMA mode I just set and unset the DMAMODE_CTL + 
> DMAMODE_1 bits in the SCR register. Nothing else. Based on some testing
> I just did, DMAMODE_CTL does not make the difference. DMAMODE_CTL +
> DMAMODE_1 does.

OK
 
> >> However core-off with DMA won't work. I think we could document this in
> >> the binding document. What do you think?
> >
> >There should not be such a limitation though. Maybe dump out the values
> >of cm_idlest_per and cm_idlest1_core for working and failing off idle
> >cases and see what the difference is?
> 
> I can't follow here. This is the working case:
...
 
> so per_pwrdm and core_pwrdm remain on and I guess the former is where
> the UART sits.

That won't show the cm_idlest_per and cm_idlest1_core currently though.

Below is a test patch that allows you to dump those too. Then you
can see which bits show up as blockers. Sounds like it will be the
serial port with dma enabled based on your description.
 
> >It could be the either the dma or the uart hardware blocking. I guess
> >it could be also an issue with runtime pm use somewhere.
> 
> So I just toggle the two DMA bits in SCR and the UART seems to block
> since the DMA hw is not involved. Reading SCR back says that those bits
> are not set.

Oh that's interesting.

> To use DMA you don't have to enable it in SCR register you can also use
> the FCR register. The manual says that you can only write this DMA
> enable bit in the FCR register if the baud clock is not running. And
> guess what: same thing: I only *toggle* the DMA enable bit here (it
> remains 0 later) and the core won't hit idle.
> Same effect if I toggle this bit while the baud clock is running (the
> manual says that this bit can only be written if the baud clock is not
> running). Seems like the UART is following its own specification and it
> remains blocking once the DMA was enabled.
> It would be nice if someone from the UART-IP team could ACK this.

Sounds like there should be some way to clear that state.. I wonder
if omap-serial.c had something before it's DMA support was removed?

I'd assume when the UART is powered down by runtime PM it's state
is completetely reset and we could restore the non-DMA state?

Maybe post your current patches and a test patch to try to toggle
the DMA on and off?

> Bah. Does it make sense to use runtime-PM if we can't hit core-off? I'm
> thinking to add a printk once dma is enabled says that runtime-pm is
> switched off.

Well if we can't find a way to unset the DMA registers in the UART,
how about only enable it if a kernel cmdline option is specified?

We do have runtime PM working without it, and the serial console
super important for any kind of debugging no matter what idle
mode the SoC hits.. So let's not break that.

Regards,

Tony

8<-----------------
From: Tony Lindgren <tony@atomide.com>
Date: Tue, 26 Aug 2014 14:25:33 -0700
Subject: [PATCH] Test patch for dumping omap3 off idle blocking bits

Allows seeing the deeper idle state blockers in
/sys/kernel/debug/pm_debug/count. For example, when
off idle is working on beaglboard xm, this is what
i see:

# sleep 5; cat /sys/kernel/debug/pm_debug/count
...
0006ffff 48005020 (fa005020) cm_idlest_per blocking bits: 00010000
e7ffffbd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00000042
0000000d 48004a28 (fa004a28) cm_idlest3_core

--- a/arch/arm/mach-omap2/pm-debug.c
+++ b/arch/arm/mach-omap2/pm-debug.c
@@ -142,10 +142,76 @@ static int pwrdm_dbg_show_timer(struct powerdomain *pwrdm, void *user)
 	return 0;
 }
 
+#include "iomap.h"
+
+struct dregs {
+	const char	*desc;
+	u32		phys;
+	void __iomem	*virt;
+	u32		mask;
+};
+
+#define PER_CM_BASE	0x48005000
+#define PER_CM_REG(name, offset, mask)				\
+	{ name, PER_CM_BASE + offset,				\
+	OMAP2_L4_IO_ADDRESS(PER_CM_BASE + offset), mask, }
+
+static struct dregs cm_per[] = {
+	PER_CM_REG("cm_idlest_per", 0x20, 0xfff80000), /* p 513 */
+	{ NULL, },
+};
+
+#define CORE_CM_BASE	0x48004a00
+#define CORE_CM_REG(name, offset, mask)				\
+	{ name, CORE_CM_BASE + offset,				\
+	OMAP2_L4_IO_ADDRESS(CORE_CM_BASE + offset), mask, }
+
+static struct dregs cm_core[] = {
+	CORE_CM_REG("cm_idlest1_core", 0x20, 0x9c800109), /* p 467 */
+	CORE_CM_REG("cm_idlest3_core", 0x28, 0xfffffffb),
+	{ NULL, },
+};
+
+void __dregs_dump(struct dregs *dregs, struct seq_file *s)
+{
+	for (; dregs->desc; dregs++) {
+		u32 val, blockers;
+
+		val = __raw_readl(dregs->virt);
+
+		seq_printf(s, "%08x %08x (%p) %s",
+			   val, dregs->phys, dregs->virt,
+			   dregs->desc);
+
+		if (dregs->mask) {
+			blockers = ~val;
+			blockers &= ~dregs->mask;
+
+			if (blockers)
+				seq_printf(s, " blocking bits: %08x",
+					   blockers);
+		}
+
+		seq_printf(s, "\n");
+	}
+}
+
+void cm_per_dump(struct seq_file *s)
+{
+	__dregs_dump(cm_per, s);
+}
+
+void cm_core_dump(struct seq_file *s)
+{
+	__dregs_dump(cm_core, s);
+}
+
 static int pm_dbg_show_counters(struct seq_file *s, void *unused)
 {
 	pwrdm_for_each(pwrdm_dbg_show_counter, s);
 	clkdm_for_each(clkdm_dbg_show_counter, s);
+	cm_per_dump(s);
+	cm_core_dump(s);
 
 	return 0;
 }

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Sebastian Andrzej Siewior]

On 08/28/2014 06:46 PM, Tony Lindgren wrote:
>> To use DMA you don't have to enable it in SCR register you can also use
>> the FCR register. The manual says that you can only write this DMA
>> enable bit in the FCR register if the baud clock is not running. And
>> guess what: same thing: I only *toggle* the DMA enable bit here (it
>> remains 0 later) and the core won't hit idle.
>> Same effect if I toggle this bit while the baud clock is running (the
>> manual says that this bit can only be written if the baud clock is not
>> running). Seems like the UART is following its own specification and it
>> remains blocking once the DMA was enabled.
>> It would be nice if someone from the UART-IP team could ACK this.
> 
> Sounds like there should be some way to clear that state.. I wonder
> if omap-serial.c had something before it's DMA support was removed?

Its DMA was removed? Like there was DMA support?

> I'd assume when the UART is powered down by runtime PM it's state
> is completetely reset and we could restore the non-DMA state?

I tried that by canceling the RX-DMA request and removing the DMA-enable
bits from UART but it didn't help. Then I noticed that once that DMA en
bit is set, the UART won't do any idle.

> Maybe post your current patches and a test patch to try to toggle
> the DMA on and off?

Oh. I pushed my dirty tree to
 git://git.breakpoint.cc/bigeasy/linux.git uart_v8_wip

The top most commit does not setup dma at all and adds commented out
code how to enable DMA via SCR or FCR register. With this I hit
core-off. Once _one_ of the modes are enabled, it doesn't work anymore.

I will try to address review comments tomorrow and hopefully post a v8
based on -rc2. The same goes for your patch (which I will try tomorrow).

>> Bah. Does it make sense to use runtime-PM if we can't hit core-off? I'm
>> thinking to add a printk once dma is enabled says that runtime-pm is
>> switched off.
> 
> Well if we can't find a way to unset the DMA registers in the UART,
> how about only enable it if a kernel cmdline option is specified?

Hmmm. So removing DMA properties from DT is not an option? Currently I
added a message that DMA might forbid low power mode so people might
remove "dma-names" from DT (or assign "") and DMA is off. However if
this is no good, then I guess I could add kernel module which enables
DMA.

> 
> We do have runtime PM working without it, and the serial console
> super important for any kind of debugging no matter what idle
> mode the SoC hits.. So let's not break that.

Yeah, I know. I've been debugging while I broke something which means
uart wasn't working :)

> 
> Regards,
> 
> Tony

Sebastian

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Tony Lindgren]

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140828 12:37]:
> On 08/28/2014 06:46 PM, Tony Lindgren wrote:
> > 
> > Sounds like there should be some way to clear that state.. I wonder
> > if omap-serial.c had something before it's DMA support was removed?
> 
> Its DMA was removed? Like there was DMA support?

Yeah see commit 494574304711a333386e7dd5fd3ebbc3b7024994...
 
> > I'd assume when the UART is powered down by runtime PM it's state
> > is completetely reset and we could restore the non-DMA state?
> 
> I tried that by canceling the RX-DMA request and removing the DMA-enable
> bits from UART but it didn't help. Then I noticed that once that DMA en
> bit is set, the UART won't do any idle.
> 
> > Maybe post your current patches and a test patch to try to toggle
> > the DMA on and off?
> 
> Oh. I pushed my dirty tree to
>  git://git.breakpoint.cc/bigeasy/linux.git uart_v8_wip
> 
> The top most commit does not setup dma at all and adds commented out
> code how to enable DMA via SCR or FCR register. With this I hit
> core-off. Once _one_ of the modes are enabled, it doesn't work anymore.
> 
> I will try to address review comments tomorrow and hopefully post a v8
> based on -rc2. The same goes for your patch (which I will try tomorrow).

OK thanks, I'm seeing the same issue as you. And the idlest registers
don't show any blockers. Looking at the errata docs, seems like
"Usage Note 2.7" in sprz318f.pdf says:

 Details When configured for DMA operations using smartidle mode (SYSC[4:3].IDLEMODE =
 0x2), the UART module will not acknowledge incoming idle requests. As a consequence,
 it can prevent L4 from going to idle.
 When there are additional expected transfers, the UART should be placed in force-idle
 mode.

So I've added also Paul to Cc, he may have better suggestions for the
hwmod flags to use. The experimental patch below seems to allow idling
for me, care to give it a try?

Regards,

Tony

8< --------------------
From: Tony Lindgren <tony@atomide.com>
Date: Thu, 28 Aug 2014 15:41:21 -0700
Subject: [PATCH] ARM: OMAP3: Use force-idle for UARTs because of DMA errata

In sprz318f.pdf Usage Note 2.7 says that UARTs cannot acknowledge
idle requests in smartidle mode when configure for DMA operations.
This prevents L4 from going idle. Sol et's use force-idle mode
instead.

--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -490,7 +490,9 @@ static struct omap_hwmod omap3xxx_uart1_hwmod = {
 	.mpu_irqs	= omap2_uart1_mpu_irqs,
 	.sdma_reqs	= omap2_uart1_sdma_reqs,
 	.main_clk	= "uart1_fck",
-	.flags		= DEBUG_TI81XXUART1_FLAGS | HWMOD_SWSUP_SIDLE_ACT,
+	.flags		= DEBUG_TI81XXUART1_FLAGS |
+				HWMOD_NO_OCP_AUTOIDLE | HWMOD_SWSUP_SIDLE |
+				HWMOD_FORCE_MSTANDBY,
 	.prcm		= {
 		.omap2 = {
 			.module_offs = CORE_MOD,
@@ -509,7 +511,9 @@ static struct omap_hwmod omap3xxx_uart2_hwmod = {
 	.mpu_irqs	= omap2_uart2_mpu_irqs,
 	.sdma_reqs	= omap2_uart2_sdma_reqs,
 	.main_clk	= "uart2_fck",
-	.flags		= DEBUG_TI81XXUART2_FLAGS | HWMOD_SWSUP_SIDLE_ACT,
+	.flags		= DEBUG_TI81XXUART2_FLAGS |
+				HWMOD_NO_OCP_AUTOIDLE | HWMOD_SWSUP_SIDLE |
+				HWMOD_FORCE_MSTANDBY,
 	.prcm		= {
 		.omap2 = {
 			.module_offs = CORE_MOD,
@@ -529,7 +533,8 @@ static struct omap_hwmod omap3xxx_uart3_hwmod = {
 	.sdma_reqs	= omap2_uart3_sdma_reqs,
 	.main_clk	= "uart3_fck",
 	.flags		= DEBUG_OMAP3UART3_FLAGS | DEBUG_TI81XXUART3_FLAGS |
-				HWMOD_SWSUP_SIDLE_ACT,
+				HWMOD_NO_OCP_AUTOIDLE | HWMOD_SWSUP_SIDLE |
+				HWMOD_FORCE_MSTANDBY,
 	.prcm		= {
 		.omap2 = {
 			.module_offs = OMAP3430_PER_MOD,
@@ -559,7 +564,9 @@ static struct omap_hwmod omap36xx_uart4_hwmod = {
 	.mpu_irqs	= uart4_mpu_irqs,
 	.sdma_reqs	= uart4_sdma_reqs,
 	.main_clk	= "uart4_fck",
-	.flags		= DEBUG_OMAP3UART4_FLAGS | HWMOD_SWSUP_SIDLE_ACT,
+	.flags		= DEBUG_OMAP3UART4_FLAGS |
+				HWMOD_NO_OCP_AUTOIDLE | HWMOD_SWSUP_SIDLE |
+				HWMOD_FORCE_MSTANDBY,
 	.prcm		= {
 		.omap2 = {
 			.module_offs = OMAP3430_PER_MOD,

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Sebastian Andrzej Siewior]

On 08/29/2014 12:54 AM, Tony Lindgren wrote:
> * Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140828 12:37]:
>> On 08/28/2014 06:46 PM, Tony Lindgren wrote:
>>>
>>> Sounds like there should be some way to clear that state.. I wonder
>>> if omap-serial.c had something before it's DMA support was removed?
>>
>> Its DMA was removed? Like there was DMA support?
> 
> Yeah see commit 494574304711a333386e7dd5fd3ebbc3b7024994...

Interesting. I've been browsing that file and checking other trees and
never noticed that it was there at some point. I only saw the pieces
which looked it was there.

> OK thanks, I'm seeing the same issue as you. And the idlest registers
> don't show any blockers. Looking at the errata docs, seems like
> "Usage Note 2.7" in sprz318f.pdf says:
> 
>  Details When configured for DMA operations using smartidle mode (SYSC[4:3].IDLEMODE =
>  0x2), the UART module will not acknowledge incoming idle requests. As a consequence,
>  it can prevent L4 from going to idle.
>  When there are additional expected transfers, the UART should be placed in force-idle
>  mode.

Ehm. So I haven't found an errata document for omap3630, there is
nothing like that in that one I have for am335x or dra7. The document
you mentioned is for AM3715. Interesting…

> So I've added also Paul to Cc, he may have better suggestions for the
> hwmod flags to use. The experimental patch below seems to allow idling
> for me, care to give it a try?

Yep, this one works. And I see DMA transfers (for RX side) after the
core hit idle so it seems to look good :)

> 
> Regards,
> 
> Tony

Sebastian

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 816 bytes --]

On Fri, Aug 29, 2014 at 11:32:36AM +0200, Sebastian Andrzej Siewior wrote:
> On 08/29/2014 12:54 AM, Tony Lindgren wrote:
> > * Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140828 12:37]:
> >> On 08/28/2014 06:46 PM, Tony Lindgren wrote:
> >>>
> >>> Sounds like there should be some way to clear that state.. I wonder
> >>> if omap-serial.c had something before it's DMA support was removed?
> >>
> >> Its DMA was removed? Like there was DMA support?
> > 
> > Yeah see commit 494574304711a333386e7dd5fd3ebbc3b7024994...
> 
> Interesting. I've been browsing that file and checking other trees and
> never noticed that it was there at some point. I only saw the pieces
> which looked it was there.

it was known to be broken and unused. There was no way to even enable
that code.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Tony Lindgren]

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140829 02:32]:
> On 08/29/2014 12:54 AM, Tony Lindgren wrote:
> > OK thanks, I'm seeing the same issue as you. And the idlest registers
> > don't show any blockers. Looking at the errata docs, seems like
> > "Usage Note 2.7" in sprz318f.pdf says:
> > 
> >  Details When configured for DMA operations using smartidle mode (SYSC[4:3].IDLEMODE =
> >  0x2), the UART module will not acknowledge incoming idle requests. As a consequence,
> >  it can prevent L4 from going to idle.
> >  When there are additional expected transfers, the UART should be placed in force-idle
> >  mode.
> 
> Ehm. So I haven't found an errata document for omap3630, there is
> nothing like that in that one I have for am335x or dra7. The document
> you mentioned is for AM3715. Interesting…

Yes I would not be suprised if that same bug is in all of them though..
 
> > So I've added also Paul to Cc, he may have better suggestions for the
> > hwmod flags to use. The experimental patch below seems to allow idling
> > for me, care to give it a try?
> 
> Yep, this one works. And I see DMA transfers (for RX side) after the
> core hit idle so it seems to look good :)

OK great :)

Looks like the paste bug is there for sure, doing off idle and pasting
240 characters to the console can hang the UART RX after few attempts,
and pasting 16 charactes won't show up at all if the system is idling.
So you may want to play with that too a bit :)

Regards,

Tony

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Sebastian Andrzej Siewior]

On 08/29/2014 06:12 PM, Tony Lindgren wrote:
> Looks like the paste bug is there for sure, doing off idle and pasting
> 240 characters to the console can hang the UART RX after few attempts,
> and pasting 16 charactes won't show up at all if the system is idling.
> So you may want to play with that too a bit :)

Oh. perfect. Please note that we the threshold moved from 16 to 48.
However I see that usually we lose a lot of characters before the uart
wakes up and does its job. Usually I see a couple characters but
sometimes is see broken characters which suggests that the clock was
not (yet) perfect. And I managed not get get it woken once.
So let me look at this once I am through with the review responses…

> 
> Regards,
> 
> Tony
> 
Sebastian

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Sebastian Andrzej Siewior]

On 08/29/2014 06:12 PM, Tony Lindgren wrote:
> Looks like the paste bug is there for sure, doing off idle and pasting
> 240 characters to the console can hang the UART RX after few attempts,
> and pasting 16 charactes won't show up at all if the system is idling.
> So you may want to play with that too a bit :)

One character wakes it up. After that you can send 16, 64 and you see
them. Right away. No delay.

If you send "a lot" data in one-go it takes approx 152 characters until
the first one is displayed properly at 115200,8N1. That is approx 13ms.
Could it take that long to get up and be ready?

Comparing it with serial-omap I see the same thing: I takes approx the
same amount of data until the first one is displayed. After a lot of
"long" writes which wake the chip up from idle I manage to freeze both,
the serial-omap driver and mine driver.

One thing that is probably a dumb idea is that printk in
omap_8250_mdr1_errataset().
Would it be possible that when I hit a printk in the resume path that I
may deadlock and box will freeze?

> 
> Regards,
> 
> Tony
> 

Sebastian

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1391 bytes --]

Hi,

On Mon, Sep 01, 2014 at 07:47:53PM +0200, Sebastian Andrzej Siewior wrote:
> On 08/29/2014 06:12 PM, Tony Lindgren wrote:
> > Looks like the paste bug is there for sure, doing off idle and pasting
> > 240 characters to the console can hang the UART RX after few attempts,
> > and pasting 16 charactes won't show up at all if the system is idling.
> > So you may want to play with that too a bit :)
> 
> One character wakes it up. After that you can send 16, 64 and you see
> them. Right away. No delay.
> 
> If you send "a lot" data in one-go it takes approx 152 characters until
> the first one is displayed properly at 115200,8N1. That is approx 13ms.
> Could it take that long to get up and be ready?

I noticed the same behaviour when I tested the runtime PM stuff on
my N900 with the existing serial-omap driver and I also assumed,
that the chip needs that long to get up.

> Comparing it with serial-omap I see the same thing: I takes approx the
> same amount of data until the first one is displayed. After a lot of
> "long" writes which wake the chip up from idle I manage to freeze both,
> the serial-omap driver and mine driver.
> 
> One thing that is probably a dumb idea is that printk in
> omap_8250_mdr1_errataset().
> Would it be possible that when I hit a printk in the resume path that I
> may deadlock and box will freeze?

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [140901 20:06]:
> Hi,
> 
> On Mon, Sep 01, 2014 at 07:47:53PM +0200, Sebastian Andrzej Siewior wrote:
> > On 08/29/2014 06:12 PM, Tony Lindgren wrote:
> > > Looks like the paste bug is there for sure, doing off idle and pasting
> > > 240 characters to the console can hang the UART RX after few attempts,
> > > and pasting 16 charactes won't show up at all if the system is idling.
> > > So you may want to play with that too a bit :)
> > 
> > One character wakes it up. After that you can send 16, 64 and you see
> > them. Right away. No delay.
> > 
> > If you send "a lot" data in one-go it takes approx 152 characters until
> > the first one is displayed properly at 115200,8N1. That is approx 13ms.
> > Could it take that long to get up and be ready?
> 
> I noticed the same behaviour when I tested the runtime PM stuff on
> my N900 with the existing serial-omap driver and I also assumed,
> that the chip needs that long to get up.

OK yeah I've confirmed that serial-omap also won't do anything with the
pasted data until woken up. It takes some time to get things powered up
again, there's nothing we can do beyond having the autoidle disabled by
default.
 
> > Comparing it with serial-omap I see the same thing: I takes approx the
> > same amount of data until the first one is displayed. After a lot of
> > "long" writes which wake the chip up from idle I manage to freeze both,
> > the serial-omap driver and mine driver.

Hmm I have not seen the RX hang with serial-omap when pasting data to
the console to wake it up.
 
> > One thing that is probably a dumb idea is that printk in
> > omap_8250_mdr1_errataset().
> > Would it be possible that when I hit a printk in the resume path that I
> > may deadlock and box will freeze?

I guess yeah. You may want to use pstore as console to debug that. You
need to use this patch to prevent pstore from hanging:

https://lkml.org/lkml/2013/4/9/831

Then enable:

CONFIG_PSTORE=y
CONFIG_PSTORE_CONSOLE=y
CONFIG_PSTORE_RAM=y
CONFIG_FUNCTION_TRACER=y
CONFIG_DEBUG_FS=y
CONFIG_PSTORE_FTRACE=y

Then at kernel cmdline, specify something like this:

memmap=2M$0x88000000 ramoops.mem_address=0x88000000 ramoops.mem_size=0x200000 ramoops.record_size=0x40000

And leave out console=ttyS line and spin up a getty on the serial
line.

After booting, you should just need to do:

# mount -t pstore - /sys/fs/pstore

And then you see dmesg in /sys/fs/pstore. To debug hangs, set up the
PMIC watchdog and do not set up omap watchdog, and you should see the
last dmesg in /sys/fs/pstore after a warm reset.

Regards,

Tony

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Sebastian Andrzej Siewior]

On 09/01/2014 07:47 PM, Sebastian Andrzej Siewior wrote:
> Comparing it with serial-omap I see the same thing: I takes approx the
> same amount of data until the first one is displayed. After a lot of
> "long" writes which wake the chip up from idle I manage to freeze both,
> the serial-omap driver and mine driver.

So after some testing:
- it happens with omap-serial as well. Especially after disabling the
  LED trigger for both LEDs.

- it seemed that disabling the MDR1 check whether or not we lost
  context made the problem appear less often but it was a trick. Even
  with restoring the context each time I see the same problem.

- it seems to be easier to trigger with the LED trigger switched off.
  However sometimes it works for 10 minutes, sometimes it triggers
  after one.

- I see to face two kind of "deaths":
  - the LED still goes on and off and the uart just does not respond
    even if I tell the button print something on the screen (the button
    also changes the frequency of the LED so I know that the button is
    doing something).
    Also from dumping the content of /proc/interrupts it seems that a
    wake up is made, the uart should have restored the registers.

  - one where the system is dead and the LED does not blink anymore.
    Also my button is dead.

- disabling DMA makes the problem not go away.

- mdelay(25) in omap8250_lost_context() is long enough to drop the 403
  bytes I send in my testcase. That means I see only "good" characters.
  With this the box remained alive for 2h. However the uart died anyway.

>>
>> Regards,
>>
>> Tony
>>
> 

Sebastian

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Tony Lindgren]

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140902 11:40]:
> On 09/01/2014 07:47 PM, Sebastian Andrzej Siewior wrote:
> > Comparing it with serial-omap I see the same thing: I takes approx the
> > same amount of data until the first one is displayed. After a lot of
> > "long" writes which wake the chip up from idle I manage to freeze both,
> > the serial-omap driver and mine driver.
> 
> So after some testing:
> - it happens with omap-serial as well. Especially after disabling the
>   LED trigger for both LEDs.
> 
> - it seemed that disabling the MDR1 check whether or not we lost
>   context made the problem appear less often but it was a trick. Even
>   with restoring the context each time I see the same problem.
> 
> - it seems to be easier to trigger with the LED trigger switched off.
>   However sometimes it works for 10 minutes, sometimes it triggers
>   after one.
> 
> - I see to face two kind of "deaths":
>   - the LED still goes on and off and the uart just does not respond
>     even if I tell the button print something on the screen (the button
>     also changes the frequency of the LED so I know that the button is
>     doing something).
>     Also from dumping the content of /proc/interrupts it seems that a
>     wake up is made, the uart should have restored the registers.

OK yeah this is the case I was seeing too. So do you just set the
LED triggers to none in sysfs to make it easier to reproduce?

>   - one where the system is dead and the LED does not blink anymore.
>     Also my button is dead.

This I don't think I've seen. This could also be the errata issue on
your earlier rev beagleboard-xm with off-idle.
 
> - disabling DMA makes the problem not go away.

OK
 
> - mdelay(25) in omap8250_lost_context() is long enough to drop the 403
>   bytes I send in my testcase. That means I see only "good" characters.
>   With this the box remained alive for 2h. However the uart died anyway.

I wonder if doing some TX on the uart restores it? So maybe try

$ while [ 1 ]; do date; sleep 10 done &

To have it occasionally print something in the background.

Regards,

Tony

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2460 bytes --]

On Tue, Sep 02, 2014 at 01:15:37PM -0700, Tony Lindgren wrote:
> * Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140902 11:40]:
> > On 09/01/2014 07:47 PM, Sebastian Andrzej Siewior wrote:
> > > Comparing it with serial-omap I see the same thing: I takes approx the
> > > same amount of data until the first one is displayed. After a lot of
> > > "long" writes which wake the chip up from idle I manage to freeze both,
> > > the serial-omap driver and mine driver.
> > 
> > So after some testing:
> > - it happens with omap-serial as well. Especially after disabling the
> >   LED trigger for both LEDs.
> > 
> > - it seemed that disabling the MDR1 check whether or not we lost
> >   context made the problem appear less often but it was a trick. Even
> >   with restoring the context each time I see the same problem.
> > 
> > - it seems to be easier to trigger with the LED trigger switched off.
> >   However sometimes it works for 10 minutes, sometimes it triggers
> >   after one.
> > 
> > - I see to face two kind of "deaths":
> >   - the LED still goes on and off and the uart just does not respond
> >     even if I tell the button print something on the screen (the button
> >     also changes the frequency of the LED so I know that the button is
> >     doing something).
> >     Also from dumping the content of /proc/interrupts it seems that a
> >     wake up is made, the uart should have restored the registers.
> 
> OK yeah this is the case I was seeing too. So do you just set the
> LED triggers to none in sysfs to make it easier to reproduce?
> 
> >   - one where the system is dead and the LED does not blink anymore.
> >     Also my button is dead.
> 
> This I don't think I've seen. This could also be the errata issue on
> your earlier rev beagleboard-xm with off-idle.
>  
> > - disabling DMA makes the problem not go away.
> 
> OK
>  
> > - mdelay(25) in omap8250_lost_context() is long enough to drop the 403
> >   bytes I send in my testcase. That means I see only "good" characters.
> >   With this the box remained alive for 2h. However the uart died anyway.
> 
> I wonder if doing some TX on the uart restores it? So maybe try
> 
> $ while [ 1 ]; do date; sleep 10 done &
> 
> To have it occasionally print something in the background.

If there is a way to detect the hangup you may try to recover by
resetting the serial device using omap_hwmod_reset().

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Sebastian Andrzej Siewior]

On 09/02/2014 10:15 PM, Tony Lindgren wrote:
>> - I see to face two kind of "deaths":
>>   - the LED still goes on and off and the uart just does not respond
>>     even if I tell the button print something on the screen (the button
>>     also changes the frequency of the LED so I know that the button is
>>     doing something).
>>     Also from dumping the content of /proc/interrupts it seems that a
>>     wake up is made, the uart should have restored the registers.
> 
> OK yeah this is the case I was seeing too. So do you just set the
> LED triggers to none in sysfs to make it easier to reproduce?

Yes.

>>   - one where the system is dead and the LED does not blink anymore.
>>     Also my button is dead.
> 
> This I don't think I've seen. This could also be the errata issue on
> your earlier rev beagleboard-xm with off-idle.

might be.

Your pstore hint gave me something. I tried that earlier but somehow
assumed that dram content was killed on init. But the content is even
there are pressing the reset button :)

However, I was able to capture the case where the LED was not blinking:
The IIR register says 0xc6 (=> line status error). That is okay. At the
same time LSR register says 0xe0. This is not okay. It means that there
is some kind of error and at least one error bit is set in this
register which is not the case. Also those bits are cleared on read
which does not happen here. And we loop forever so the LED does blink
anymore.

The RX-count register says that it is empty which sense because bit 0
is not set (in LSR). However I can read multiple times from the RX FIFO
until I get the "unhandled bus access" error which usually happens
right away if the empty FIFO is read on omap3 HW. In the last test I
mange to read 91 times before the crash. I hoped that this FIFO read
would make the interrupt go away but it did not.

The HW seems to be in a strange state. It might be either the errata
or something else. I even took the resume routine from omap-serial in
case I did something wrong. In my last test it worked for 10minues
before the interrupt storm came.

This is probably the same thing I see on the omap-serial driver where I
got from pstore:

[   32.659271] random: nonblocking pool is initialized
[  212.170623] NMI watchdog: BUG: soft lockup - CPU#0 stuck for 22s!
[swapper:0]

So I *guess* the interrupt routine is looping. This is problem one, no
idea what is going on (the register status captured on 8250-omap makes
no sense).

Problem two, where the UART does not wakeup:
What I observed is that sometimes the UART does not wake up properly
i.e. it does not write anything on the console, even where it should. I
can't tell if the read is working properly, the write does not.
>From my capture I see that the resume routine was running and the
register should have been written. That means the UART should be up and
running but nothing happens.
It often works again after the system comes out of resume again (i.e.
RPM suspens and resumes the UART). So it is okay on the next wakeup. Or
the wakeup after next.
>From the script:

| while ((1))
| do
|
|         echo -n 409-chars >/dev/ttyUSB0
|
|         sleep 1
|         a=$(date)
|         echo -e "\n#$a" >/dev/ttyUSB0
|         echo $a
|         sleep 13;
| done

I see that sometimes one or two sequential timestamps are missing. And
the it continues like nothing happened.

> Tony

Sebastian

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Tony Lindgren]

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140903 09:46]:
> On 09/02/2014 10:15 PM, Tony Lindgren wrote:
> >> - I see to face two kind of "deaths":
> >>   - the LED still goes on and off and the uart just does not respond
> >>     even if I tell the button print something on the screen (the button
> >>     also changes the frequency of the LED so I know that the button is
> >>     doing something).
> >>     Also from dumping the content of /proc/interrupts it seems that a
> >>     wake up is made, the uart should have restored the registers.
> > 
> > OK yeah this is the case I was seeing too. So do you just set the
> > LED triggers to none in sysfs to make it easier to reproduce?
> 
> Yes.
> 
> >>   - one where the system is dead and the LED does not blink anymore.
> >>     Also my button is dead.
> > 
> > This I don't think I've seen. This could also be the errata issue on
> > your earlier rev beagleboard-xm with off-idle.
> 
> might be.
> 
> Your pstore hint gave me something. I tried that earlier but somehow
> assumed that dram content was killed on init. But the content is even
> there are pressing the reset button :)

Yeah pstore is very nice for debugging mystery hangs :)
 
> However, I was able to capture the case where the LED was not blinking:
> The IIR register says 0xc6 (=> line status error). That is okay. At the
> same time LSR register says 0xe0. This is not okay. It means that there
> is some kind of error and at least one error bit is set in this
> register which is not the case. Also those bits are cleared on read
> which does not happen here. And we loop forever so the LED does blink
> anymore.

OK
 
> The RX-count register says that it is empty which sense because bit 0
> is not set (in LSR). However I can read multiple times from the RX FIFO
> until I get the "unhandled bus access" error which usually happens
> right away if the empty FIFO is read on omap3 HW. In the last test I
> mange to read 91 times before the crash. I hoped that this FIFO read
> would make the interrupt go away but it did not.
> 
> The HW seems to be in a strange state. It might be either the errata
> or something else. I even took the resume routine from omap-serial in
> case I did something wrong. In my last test it worked for 10minues
> before the interrupt storm came.
> 
> This is probably the same thing I see on the omap-serial driver where I
> got from pstore:
> 
> [   32.659271] random: nonblocking pool is initialized
> [  212.170623] NMI watchdog: BUG: soft lockup - CPU#0 stuck for 22s!
> [swapper:0]
> 
> So I *guess* the interrupt routine is looping. This is problem one, no
> idea what is going on (the register status captured on 8250-omap makes
> no sense).

See recent commit cc824534d4fe, and try commenting out the check for
HWMOD_FORCE_MSTANDBY in omap_hwmod.c so _reconfigure_io_chain() is always
called. If that changes something, we at least have some idea.

It could be also the wake-up interrupt looping. So you may also want to
try adding some printks (pstore only) into omap_prcm_irq_handler() and
omap3xxx_prm_clear_mod_irqs() as that's handling the wake-up event
interrupts.
 
> Problem two, where the UART does not wakeup:
> What I observed is that sometimes the UART does not wake up properly
> i.e. it does not write anything on the console, even where it should. I
> can't tell if the read is working properly, the write does not.
> From my capture I see that the resume routine was running and the
> register should have been written. That means the UART should be up and
> running but nothing happens.

This seems also be hinting to something needing _reconfigure_io_chain()
to be called along the lines of commit cc824534d4fe.

> It often works again after the system comes out of resume again (i.e.
> RPM suspens and resumes the UART). So it is okay on the next wakeup. Or
> the wakeup after next.
> From the script:
> 
> | while ((1))
> | do
> |
> |         echo -n 409-chars >/dev/ttyUSB0
> |
> |         sleep 1
> |         a=$(date)
> |         echo -e "\n#$a" >/dev/ttyUSB0
> |         echo $a
> |         sleep 13;
> | done
> 
> I see that sometimes one or two sequential timestamps are missing. And
> the it continues like nothing happened.

OK. At least it's starting to now sound that the bugs are pretty much
the same with 8250 and serial-omap :)

Regards,

Tony

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Sebastian Andrzej Siewior]

On 09/03/2014 07:48 PM, Tony Lindgren wrote:

I will check this upper part of this email soon…

> OK. At least it's starting to now sound that the bugs are pretty much
> the same with 8250 and serial-omap :)

Do you see a reason for not posting the entire series again since now I
am bug compatible in the pm-runtime part? We have -rc3 and I would like
to get this merge the series I have now and this seems to be the only
problem I am aware of.

> 
> Regards,
> 
> Tony

Sebastian

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Tony Lindgren]

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140904 06:46]:
> On 09/03/2014 07:48 PM, Tony Lindgren wrote:
> 
> I will check this upper part of this email soon…
> 
> > OK. At least it's starting to now sound that the bugs are pretty much
> > the same with 8250 and serial-omap :)
> 
> Do you see a reason for not posting the entire series again since now I
> am bug compatible in the pm-runtime part? We have -rc3 and I would like
> to get this merge the series I have now and this seems to be the only
> problem I am aware of.

Yeah makes sense to me. Thanks for ensuring the bug
compability :)

Regards,

Tony

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Sebastian Andrzej Siewior]

On 09/04/2014 04:52 PM, Tony Lindgren wrote:
> Yeah makes sense to me. Thanks for ensuring the bug
> compability :)

Thanks. What about the ttyOx vs ttySx. Do you want me to add kernel
line fixup somewhere arch/arm or we ignore this for now? Greg didn't
say anything…

> 
> Regards,
> 
> Tony
> 

Sebastian

Re: [PATCH 15/15] tty: serial: 8250: omap: add dma support

[Tony Lindgren]

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140904 07:56]:
> On 09/04/2014 04:52 PM, Tony Lindgren wrote:
> > Yeah makes sense to me. Thanks for ensuring the bug
> > compability :)
> 
> Thanks. What about the ttyOx vs ttySx. Do you want me to add kernel
> line fixup somewhere arch/arm or we ignore this for now? Greg didn't
> say anything…

I suggest we have both drivers just available first and then attempt
to deal with the removal of most of omap-serial later on.

If both drivers are present, we should try to ensure 8250 gets probed
first which I believe it already does based on the Makefile order.

Once the 8250 based driver is known to work, it might make sense to
remove most of the functions in omap-serial and have it use the 8250
functions too to remove duplicate code.

That might be already enough and no proxying is needed and we get
rid of the duplicate code :)

Regards,

Tony

Re: [PATCH v7] 8250-core based serial driver for OMAP + DMA

[Lennart Sorensen]

On Fri, Aug 15, 2014 at 07:42:28PM +0200, Sebastian Andrzej Siewior wrote:
> This is my complete queue fo the omap serial driver based on the 8250 core
> code. I played with it on beagle bone, am335x-evm and dra7xx including DMA.
> The uncertain remain the runtime-pm pieces.
> I hacked a small serial testing application which sent 10x 4KiB of data in
> raw mode. The number of interrupts in comparison:
> 
>     serial-omap | 8250 omap | 8250 omap + dma |
>    --------------------------------------------
> TX |       2558 |       641 |         0 +  30 |
> RX |      40960 |       854 |         1 + 853 |
> 
> So the 8250 version uses less interrupts for the same amount of data.
> The consequence is that in TX mode there should be "short" periods where
> no data is sent (before the CPU gets to re-fill the FIFO). On RX we have
> a smaller time frame where we have to start to purge the FIFO before it
> overflows.

Are you saying that with the new driver you have to respond to the RX
irq faster than before to avoid overflows?  It is not quite clear.

I do think 40000 interrupts to handle 40000 bytes of date does seem a
tad inefficient, so dropping to 854 looks a lot nicer.  Was the omap
driver not using the fifo trigger levels at all?

-- 
Len Sorensen

Re: [PATCH v7] 8250-core based serial driver for OMAP + DMA

[Sebastian Andrzej Siewior]

On 08/15/2014 08:17 PM, Lennart Sorensen wrote:

> Are you saying that with the new driver you have to respond to the RX
> irq faster than before to avoid overflows?  It is not quite clear.

Yes. The irq fires 46 bytes giving you 16 bytes buffer before overflow
vs 63 bytes buffer the old one had.

> I do think 40000 interrupts to handle 40000 bytes of date does seem a
> tad inefficient, so dropping to 854 looks a lot nicer.  Was the omap
> driver not using the fifo trigger levels at all?

It configured the trigger levels to 1 for RX and 16 for TX.

Sebastian

Re: [PATCH v7] 8250-core based serial driver for OMAP + DMA

[Tony Lindgren]

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140815 12:16]:
> On 08/15/2014 08:17 PM, Lennart Sorensen wrote:
> 
> > Are you saying that with the new driver you have to respond to the RX
> > irq faster than before to avoid overflows?  It is not quite clear.
> 
> Yes. The irq fires 46 bytes giving you 16 bytes buffer before overflow
> vs 63 bytes buffer the old one had.
> 
> > I do think 40000 interrupts to handle 40000 bytes of date does seem a
> > tad inefficient, so dropping to 854 looks a lot nicer.  Was the omap
> > driver not using the fifo trigger levels at all?
> 
> It configured the trigger levels to 1 for RX and 16 for TX.

Hmm that weird RX trigger level is a workaround for lost characters.

See commit 0ba5f66836 (tty: serial: OMAP: use a 1-byte RX FIFO
threshold in PIO mode :)

There's paste test in that commit, I wonder if the 8250 drivers
can deal with it any better?

Regards,

Tony

Re: [PATCH v7] 8250-core based serial driver for OMAP + DMA

[Sebastian Andrzej Siewior]

* Tony Lindgren | 2014-08-15 13:28:27 [-0700]:

>> It configured the trigger levels to 1 for RX and 16 for TX.
>
>Hmm that weird RX trigger level is a workaround for lost characters.
>
>See commit 0ba5f66836 (tty: serial: OMAP: use a 1-byte RX FIFO
>threshold in PIO mode :)
>
>There's paste test in that commit, I wonder if the 8250 drivers
>can deal with it any better?

I recall that I used it as a console and didn't see any missing chars.
I will try test but I am traveling this week…

>Regards,
>
>Tony

Sebastian

Re: [PATCH v7] 8250-core based serial driver for OMAP + DMA

[Peter Hurley]

On 08/15/2014 04:28 PM, Tony Lindgren wrote:
> * Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140815 12:16]:
>> On 08/15/2014 08:17 PM, Lennart Sorensen wrote:
>>
>>> Are you saying that with the new driver you have to respond to the RX
>>> irq faster than before to avoid overflows?  It is not quite clear.
>>
>> Yes. The irq fires 46 bytes giving you 16 bytes buffer before overflow
>> vs 63 bytes buffer the old one had.
>>
>>> I do think 40000 interrupts to handle 40000 bytes of date does seem a
>>> tad inefficient, so dropping to 854 looks a lot nicer.  Was the omap
>>> driver not using the fifo trigger levels at all?
>>
>> It configured the trigger levels to 1 for RX and 16 for TX.
> 
> Hmm that weird RX trigger level is a workaround for lost characters.
> 
> See commit 0ba5f66836 (tty: serial: OMAP: use a 1-byte RX FIFO
> threshold in PIO mode :)

That commit looks like it should have been specific to the silicon
exhibiting the rx timeout bug.

Regards,
Peter Hurley

Re: [PATCH v7] 8250-core based serial driver for OMAP + DMA

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1144 bytes --]

On Mon, Aug 18, 2014 at 11:15:17AM -0400, Peter Hurley wrote:
> On 08/15/2014 04:28 PM, Tony Lindgren wrote:
> > * Sebastian Andrzej Siewior <bigeasy@linutronix.de> [140815 12:16]:
> >> On 08/15/2014 08:17 PM, Lennart Sorensen wrote:
> >>
> >>> Are you saying that with the new driver you have to respond to the RX
> >>> irq faster than before to avoid overflows?  It is not quite clear.
> >>
> >> Yes. The irq fires 46 bytes giving you 16 bytes buffer before overflow
> >> vs 63 bytes buffer the old one had.
> >>
> >>> I do think 40000 interrupts to handle 40000 bytes of date does seem a
> >>> tad inefficient, so dropping to 854 looks a lot nicer.  Was the omap
> >>> driver not using the fifo trigger levels at all?
> >>
> >> It configured the trigger levels to 1 for RX and 16 for TX.
> > 
> > Hmm that weird RX trigger level is a workaround for lost characters.
> > 
> > See commit 0ba5f66836 (tty: serial: OMAP: use a 1-byte RX FIFO
> > threshold in PIO mode :)
> 
> That commit looks like it should have been specific to the silicon
> exhibiting the rx timeout bug.

yeah, I'll agree with that.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]