[RFC PATCH 1/4] tty: serial: 8250 core: provide a function to export uart_8250_port

[Sebastian Andrzej Siewior]

There is no access to access a struct uart_8250_port for a specific
line. This is only required outside of the 8250/uart callbacks like for
devices' suspend & remove callbacks. For those the 8250-core provides
wrapper like serial8250_unregister_port() which passes the struct
to the proper function based on the line argument.

For runtime suspend I need access to this struct not only to make
serial_out() work but also to properly restore up->ier and up->mcr.

Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>
---
drivers/tty/serial/8250/8250.h | 2 ++
drivers/tty/serial/8250/8250_core.c | 6 ++++++
2 files changed, 8 insertions(+)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 1ebf853..34c3cd1 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -110,6 +110,8 @@ static inline void serial_dl_write(struct uart_8250_port *up, int value)
up->dl_write(up, value);
}

+struct uart_8250_port *serial8250_get_port(int line);
+
#if defined(__alpha__) && !defined(CONFIG_PCI)
/*
* Digital did something really horribly wrong with the OUT1 and OUT2
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 7a91c6d..0c90160 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2755,6 +2755,12 @@ static struct uart_ops serial8250_pops = {

static struct uart_8250_port serial8250_ports[UART_NR];

+struct uart_8250_port *serial8250_get_port(int line)
+{
+ return &serial8250_ports[line];
+}
+EXPORT_SYMBOL_GPL(serial8250_get_port);
+
static void (*serial8250_isa_config)(int port, struct uart_port *up,
unsigned short *capabilities);

--
2.0.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Sebastian Andrzej Siewior]

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

Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>
---

drivers/tty/serial/8250/8250_core.c | 11 ++++++++++-
include/linux/serial_8250.h | 1 +
include/linux/serial_core.h | 1 +
3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 0c90160..f731fff 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1939,7 +1939,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 =
container_of(port, struct uart_8250_port, port);
@@ -2191,6 +2191,15 @@ static int serial8250_startup(struct uart_port *port)

return 0;
}
+EXPORT_SYMBOL_GPL(serial8250_do_startup);
+
+static int serial8250_startup(struct uart_port *port)
+{
+ if (port->startup)
+ return port->startup(port);
+ else
+ return serial8250_do_startup(port);
+}

static void serial8250_shutdown(struct uart_port *port)
{
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index af47a8a..9afed1e 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -114,6 +114,7 @@ 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_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 5bbb809..77acd73 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -122,6 +122,7 @@ struct uart_port {
void (*set_termios)(struct uart_port *,
struct ktermios *new,
struct ktermios *old);
+ int (*startup)(struct uart_port *port);
int (*handle_irq)(struct uart_port *);
void (*pm)(struct uart_port *, unsigned int state,
unsigned int old);

[Sebastian Andrzej Siewior]

While comparing the OMAP-serial and the 8250 part of this I noticed that
the the latter does not use runtime-pm. Here are the pieces. It is
basically a get before first register access and a last_busy + put after
last access.
If I understand this correct, it should do nothing as long as
pm_runtime_use_autosuspend() + pm_runtime_enable() isn't invoked on the
device.

Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>
---

drivers/tty/serial/8250/8250_core.c | 101 +++++++++++++++++++++++++++++++-----
1 file changed, 88 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index f731fff..b0b9abb 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -38,6 +38,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
@@ -553,10 +554,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.
*/
+ pm_runtime_get_sync(p->port.dev);
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) {
@@ -571,7 +573,17 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep)
serial_out(p, UART_EFR, 0);
serial_out(p, UART_LCR, 0);
}
+
+ if (!device_may_wakeup(p->port.dev)) {
+ if (sleep)
+ pm_runtime_forbid(p->port.dev);
+ else
+ pm_runtime_allow(p->port.dev);
+ }
}
+out:
+ pm_runtime_mark_last_busy(p->port.dev);
+ pm_runtime_put_autosuspend(p->port.dev);
}

#ifdef CONFIG_SERIAL_8250_RSA
@@ -1280,6 +1292,7 @@ static void serial8250_stop_tx(struct uart_port *port)

struct uart_8250_port *up =
container_of(port, struct uart_8250_port, port);

+ pm_runtime_get_sync(port->dev);
__stop_tx(up);

/*
@@ -1289,6 +1302,8 @@ static void serial8250_stop_tx(struct uart_port *port)
up->acr |= UART_ACR_TXDIS;
serial_icr_write(up, UART_ACR, up->acr);
}
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

static void serial8250_start_tx(struct uart_port *port)
@@ -1296,8 +1311,9 @@ static void serial8250_start_tx(struct uart_port *port)

struct uart_8250_port *up =
container_of(port, struct uart_8250_port, port);

+ pm_runtime_get_sync(port->dev);
if (up->dma && !serial8250_tx_dma(up)) {
- return;
+ goto out;
} else if (!(up->ier & UART_IER_THRI)) {
up->ier |= UART_IER_THRI;
serial_port_out(port, UART_IER, up->ier);
@@ -1318,6 +1334,9 @@ static void serial8250_start_tx(struct uart_port *port)
up->acr &= ~UART_ACR_TXDIS;
serial_icr_write(up, UART_ACR, up->acr);
}
+out:
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

static void serial8250_stop_rx(struct uart_port *port)
@@ -1325,9 +1344,14 @@ static void serial8250_stop_rx(struct uart_port *port)

struct uart_8250_port *up =
container_of(port, struct uart_8250_port, port);

+ pm_runtime_get_sync(port->dev);
+
up->ier &= ~UART_IER_RLSI;
up->port.read_status_mask &= ~UART_LSR_DR;
serial_port_out(port, UART_IER, up->ier);
+
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

static void serial8250_enable_ms(struct uart_port *port)
@@ -1340,7 +1364,10 @@ static void serial8250_enable_ms(struct uart_port *port)
return;

up->ier |= UART_IER_MSI;
+ pm_runtime_get_sync(port->dev);
serial_port_out(port, UART_IER, up->ier);
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

/*
@@ -1530,9 +1557,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);
+ unsigned int iir;
+ int ret;

- return serial8250_handle_irq(port, iir);
+ pm_runtime_get_sync(port->dev);
+
+ iir = serial_port_in(port, UART_IIR);
+ ret = serial8250_handle_irq(port, iir);
+
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
+ return ret;
}

/*
@@ -1790,11 +1825,16 @@ static unsigned int serial8250_tx_empty(struct uart_port *port)
unsigned long flags;
unsigned int lsr;

+ pm_runtime_get_sync(port->dev);
+
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);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
+
return (lsr & BOTH_EMPTY) == BOTH_EMPTY ? TIOCSER_TEMT : 0;
}

@@ -1805,7 +1845,10 @@ static unsigned int serial8250_get_mctrl(struct uart_port *port)
unsigned int status;
unsigned int ret;

+ pm_runtime_get_sync(port->dev);
status = serial8250_modem_status(up);
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);

ret = 0;
if (status & UART_MSR_DCD)
@@ -1838,7 +1881,10 @@ static void serial8250_set_mctrl(struct uart_port *port, unsigned int mctrl)

mcr = (mcr & up->mcr_mask) | up->mcr_force | up->mcr;

+ pm_runtime_get_sync(port->dev);
serial_port_out(port, UART_MCR, mcr);
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

static void serial8250_break_ctl(struct uart_port *port, int break_state)
@@ -1847,6 +1893,7 @@ static void serial8250_break_ctl(struct uart_port *port, int break_state)
container_of(port, struct uart_8250_port, port);
unsigned long flags;

+ pm_runtime_get_sync(port->dev);
spin_lock_irqsave(&port->lock, flags);
if (break_state == -1)
up->lcr |= UART_LCR_SBC;
@@ -1854,6 +1901,8 @@ 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);
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

/*
@@ -1898,12 +1947,23 @@ 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;
+
+ pm_runtime_get_sync(port->dev);

- 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:
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
+ return status;
}


@@ -1914,6 +1974,7 @@ static void serial8250_put_poll_char(struct uart_port *port,

struct uart_8250_port *up =
container_of(port, struct uart_8250_port, port);

+ pm_runtime_get_sync(up->dev);
/*
* First save the IER then disable the interrupts
*/
@@ -1935,6 +1996,9 @@ static void serial8250_put_poll_char(struct uart_port *port,
*/
wait_for_xmitr(up, BOTH_EMPTY);
serial_port_out(port, UART_IER, ier);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
+
}

#endif /* CONFIG_CONSOLE_POLL */
@@ -1961,6 +2025,7 @@ int serial8250_do_startup(struct uart_port *port)
if (port->iotype != up->cur_iotype)
set_io_from_upio(port);

+ pm_runtime_get_sync(port->dev);
if (port->type == PORT_16C950) {
/* Wake up and initialize UART */
up->acr = 0;
@@ -1981,7 +2046,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())
@@ -2005,7 +2069,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;
}

/*
@@ -2090,7 +2155,7 @@ int serial8250_do_startup(struct uart_port *port)
} else {
retval = serial_link_irq_chain(up);
if (retval)
- return retval;
+ goto out;
}

/*
@@ -2188,8 +2253,11 @@ int serial8250_do_startup(struct uart_port *port)
outb_p(0x80, icp);
inb_p(icp);
}
-
- return 0;
+ retval = 0;
+out:
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
+ return retval;
}
EXPORT_SYMBOL_GPL(serial8250_do_startup);

@@ -2207,6 +2275,7 @@ static void serial8250_shutdown(struct uart_port *port)
container_of(port, struct uart_8250_port, port);
unsigned long flags;

+ pm_runtime_get_sync(port->dev);
/*
* Disable interrupts from this port
*/
@@ -2246,6 +2315,8 @@ static void serial8250_shutdown(struct uart_port *port)
* the IRQ chain.
*/
serial_port_in(port, UART_RX);
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);

del_timer_sync(&up->timer);
up->timer.function = serial8250_timeout;
@@ -2356,6 +2427,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.
*/
+ pm_runtime_get_sync(port->dev);
spin_lock_irqsave(&port->lock, flags);

/*
@@ -2477,6 +2549,9 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
}
serial8250_set_mctrl(port, port->mctrl);
spin_unlock_irqrestore(&port->lock, flags);
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
+
/* Don't rewrite B0 */
if (tty_termios_baud_rate(termios))
tty_termios_encode_baud_rate(termios, baud, baud);

[Sebastian Andrzej Siewior]

This patch provides a 8250-core based UART driver for the internal OMAP
UART. The longterm goal is to provide the same functionality as the
current OMAP uart driver and hopefully DMA support which could borrowed
from the 8250-core.

It has been only tested as console UART.
The tty name is ttyS based instead of ttyO.

Missing:
- throttle callback
- custom startup + shutdown callback for wakeup-irq handling.
- RS485 handling.

v1…v2:
- added runtime PM. Could somebody could plese double check
this? I seems to be enabled and nothing explodes. However
serial_omap_get_context_loss_count() & enable_wakeup() are
NULL pointer (in the omap-serial driver). So I am not sure how
this supposed to work :)
- added omap_8250_set_termios()

Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>
---

drivers/tty/serial/8250/8250_core.c | 8 +
drivers/tty/serial/8250/8250_omap.c | 840 ++++++++++++++++++++++++++++++++++++
drivers/tty/serial/8250/Kconfig | 9 +
drivers/tty/serial/8250/Makefile | 1 +
include/uapi/linux/serial_core.h | 3 +-
5 files changed, 860 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 b0b9abb..3f2f533 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -261,6 +261,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,
@@ -1347,6 +1353,8 @@ static void serial8250_stop_rx(struct uart_port *port)
pm_runtime_get_sync(port->dev);

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..63f91fc
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -0,0 +1,840 @@
+/*
+ * 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/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_SW_CLR 0xF0
+#define OMAP_UART_TCR_TRIG 0x0F
+
+#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 serial8250_omap_priv {
+ int line;
+ u32 habit;
+ u32 fcr;
+ u32 mdr1;
+ u32 efr;
+ u32 quot;
+ u32 scr;
+ bool is_suspending;
+ int wakeirq;
+ int wakeups_enabled;
+ int context_loss_cnt;
+ 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));
+}
+
+static void uart_write(struct uart_8250_port *up, u32 reg, u32 val)
+{
+ writel(val, 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 serial8250_omap_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 serial8250_omap_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 serial8250_omap_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);
+
+ priv->scr = OMAP_UART_SCR_RX_TRIG_GRANU1_MASK | OMAP_UART_SCR_TX_EMPTY;
+ /*
+ * NOTE: Setting OMAP_UART_SCR_RX_TRIG_GRANU1_MASK sets Enables the
+ * granularity of 1 for TRIGGER RX level. Along with setting RX FIFO
+ * trigger level to 1 (as noted below, 16 characters) and TLR[3:0] to
+ * zero this will result RX FIFO threshold level to 1 character.
+ * Transmit FIFO threshold is set to 32 spaces. With SCR_TX_EMPTY set,
+ * we receive an interrupt once TX FIFO (and shift) is empty as this is
+ * what The irq routine currently expects to happen.
+ */
+ priv->fcr = UART_FCR6_R_TRIGGER_16 | UART_FCR6_T_TRIGGER_24 |
+ UART_FCR_ENABLE_FIFO;
+
+ 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]);
+
+ /* Enable access to TCR/TLR */
+ 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_TRIG);
+
+ 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, 0);
+ 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 serial8250_omap_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);
+
+ if (!device_may_wakeup(port->dev)) {
+ if (!state)
+ pm_runtime_forbid(port->dev);
+ else
+ pm_runtime_allow(port->dev);
+ }

+
+ 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 serial8250_omap_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 serial_omap_uart_qos_work(struct work_struct *work)
+{
+ struct serial8250_omap_priv *priv;
+
+ priv = container_of(work, struct serial8250_omap_priv, qos_work);
+ pm_qos_update_request(&priv->pm_qos_request, priv->latency);
+}
+
+#if 0
+static int omap_8250_startup(struct uart_port *port)
+{
+ struct uart_8250_port *up =
+ container_of(port, struct uart_8250_port, port);
+ struct serial8250_omap_priv *priv = port->private_data;
+
+ int ret;
+
+ if (priv->wakeirq) {
+ ret = request_irq(priv->wakeirq, port->handle_irq,
+ port->irqflags, "wakeup irq", port);
+ if (ret)
+ return ret;
+ disable_irq(up->wakeirq);
+ }
+
+ ret = serial8250_do_startup(port);
+ if (ret)
+ goto out;
+
+ /* Enable module level wake up */
+ priv->wer = OMAP_UART_WER_MOD_WKUP;
+ if (priv->features & OMAP_UART_WER_HAS_TX_WAKEUP)
+ priv->wer |= OMAP_UART_TX_WAKEUP_EN;
+out:
+ if (priv->wakeirq)
+ free(priv->wakeirq, port);
+ return ret;
+}
+#endif
+
+static int serial8250_omap_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 serial8250_omap_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 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;
+ 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;
+ /*
+ * XXX
+ * throttle
+ * 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);
+ } 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);
+ ret = -ENODEV;
+ /*XXX*/
+ BUG();
+ }
+ 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);
+ }
+
+ 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, serial_omap_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 serial8250_omap_remove(struct platform_device *pdev)
+{
+ struct serial8250_omap_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;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int serial_omap_prepare(struct device *dev)
+{
+ struct serial8250_omap_priv *priv = dev_get_drvdata(dev);
+
+ priv->is_suspending = true;
+ return 0;
+}
+
+static void serial_omap_complete(struct device *dev)
+{
+ struct serial8250_omap_priv *priv = dev_get_drvdata(dev);
+
+ priv->is_suspending = false;
+}
+
+static inline void serial_omap_enable_wakeirq(struct serial8250_omap_priv *priv,
+ bool enable)
+{
+ if (!priv->wakeirq)
+ return;
+
+ if (enable)
+ enable_irq(priv->wakeirq);
+ else
+ disable_irq_nosync(priv->wakeirq);
+}
+
+static void serial_omap_enable_wakeup(struct serial8250_omap_priv *priv,
+ bool enable)
+{
+ if (enable == priv->wakeups_enabled)
+ return;
+
+ serial_omap_enable_wakeirq(priv, enable);
+ priv->wakeups_enabled = enable;
+
+ /* XXX
+ * So
+ * omap_uart_enable_wakeup(dev, enable);
+ * could be assigned here but my
+ * pdata->enable_wakeup(up->dev, enable);
+ * NULL here…
+ */
+}
+
+static int serial_omap_suspend(struct device *dev)
+{
+ struct serial8250_omap_priv *priv = dev_get_drvdata(dev);
+
+ serial8250_suspend_port(priv->line);
+ flush_work(&priv->qos_work);
+
+ if (device_may_wakeup(dev))
+ serial_omap_enable_wakeup(priv, true);
+ else
+ serial_omap_enable_wakeup(priv, false);
+ return 0;
+}
+
+static int serial_omap_resume(struct device *dev)
+{
+ struct serial8250_omap_priv *priv = dev_get_drvdata(dev);
+
+ if (device_may_wakeup(dev))
+ serial_omap_enable_wakeup(priv, false);
+
+ serial8250_resume_port(priv->line);
+ return 0;
+}
+#else
+#define serial_omap_prepare NULL
+#define serial_omap_complete NULL
+#endif
+
+#ifdef CONFIG_PM_RUNTIME
+static int serial_omap_get_context_loss_count(struct serial8250_omap_priv *priv)
+{
+ /*
+ * XXX
+ * Is this still used? I have here a NULL pointer on my am335x-evm
+ */
+ return -EINVAL;
+}
+
+static int serial_omap_runtime_suspend(struct device *dev)
+{
+ struct serial8250_omap_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)
+ /*
+ * XXX Since I have no idea how to get to port right now it is
+ * just assumed true for uarts…
+ * && uart_console(&up->port))
+ */
+ return -EBUSY;
+
+ priv->context_loss_cnt = serial_omap_get_context_loss_count(priv);
+
+ serial_omap_enable_wakeup(priv, true);
+
+ priv->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
+ schedule_work(&priv->qos_work);
+ return 0;
+}
+
+static void serial_omap_restore_context(struct serial8250_omap_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);
+ /*
+ * XXX
+ * serial_out(up, UART_OMAP_WER, up->wer);
+ */
+}
+
+static int serial_omap_runtime_resume(struct device *dev)
+{
+ struct serial8250_omap_priv *priv = dev_get_drvdata(dev);
+ int loss_cnt = serial_omap_get_context_loss_count(priv);
+
+ serial_omap_enable_wakeup(priv, false);
+
+ if (loss_cnt < 0) {
+ dev_dbg(dev, "serial_omap_get_context_loss_count failed : %d\n",
+ loss_cnt);
+ serial_omap_restore_context(priv);
+ } else if (priv->context_loss_cnt != loss_cnt) {
+ serial_omap_restore_context(priv);
+ }
+ priv->latency = priv->calc_latency;
+ schedule_work(&priv->qos_work);
+
+ return 0;
+}
+#endif
+
+static const struct dev_pm_ops serial8250_omap_dev_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(serial_omap_suspend, serial_omap_resume)
+ SET_RUNTIME_PM_OPS(serial_omap_runtime_suspend,
+ serial_omap_runtime_resume, NULL)
+ .prepare = serial_omap_prepare,
+ .complete = serial_omap_complete,
+};
+
+static const struct of_device_id serial8250_omap_dt_ids[] = {
+ { .compatible = "ti,omap2-uart" },
+ { .compatible = "ti,omap3-uart" },
+ { .compatible = "ti,omap4-uart" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, serial8250_omap_dt_ids);
+
+static struct platform_driver serial8250_omap_platform_driver = {
+ .driver = {
+ .name = "serial8250-omap",
+ .pm = &serial8250_omap_dev_pm_ops,
+ .of_match_table = serial8250_omap_dt_ids,
+ .owner = THIS_MODULE,
+ },
+ .probe = serial8250_omap_probe,
+ .remove = serial8250_omap_remove,
+};
+module_platform_driver(serial8250_omap_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

[One Thousand Gnomes]

On Fri, 4 Jul 2014 18:34:08 +0200
Sebastian Andrzej Siewior <big...@linutronix.de> wrote:

> There is no access to access a struct uart_8250_port for a specific
> line. This is only required outside of the 8250/uart callbacks like for
> devices' suspend & remove callbacks. For those the 8250-core provides
> wrapper like serial8250_unregister_port() which passes the struct
> to the proper function based on the line argument.
>
> For runtime suspend I need access to this struct not only to make
> serial_out() work but also to properly restore up->ier and up->mcr.

Please update the patch to clearly describe the locking
assumptions/requirements for the caller (and not to use it unless you
have no other choice)

[One Thousand Gnomes]

On Fri, 4 Jul 2014 18:34:09 +0200

Sebastian Andrzej Siewior <big...@linutronix.de> wrote:

> The OMAP version of the 8250 can actually use 1:1 serial8250_startup().
> However it needs to be extended by a wakeup irq which should to be
> requested & enabled at ->startup() time.
>
> Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>

Acked-by: Alan Cox <al...@linux.intel.com>

[One Thousand Gnomes]

On Fri, 4 Jul 2014 18:34:10 +0200

Sebastian Andrzej Siewior <big...@linutronix.de> wrote:

> While comparing the OMAP-serial and the 8250 part of this I noticed that
> the the latter does not use runtime-pm.

Yes it does, but 8250 parts (generally - omap presumably is special
here ?) need to be powered on to transmit/receive not just for register
access. The core uart layer implements a "pm" operation for this.

As 8250_dw uses runtime pm to implement the pm operation it's not as
simple as assumign it won't get triggered.

I *think* this is ok because the designware and other cases would take a
reference on open and drop it on close, so avoiding any confusion, but
for the register accesses on a closed port it would benefit from a
further double check with Mika especially as the suspend/resume on the
LPSS block on some Intel devices is a little bit too "interesting" for
comfort.

Otherwise however I think this is good.

Alan

[Tony Lindgren]

* One Thousand Gnomes <gno...@lxorguk.ukuu.org.uk> [140707 06:22]:

> On Fri, 4 Jul 2014 18:34:10 +0200
> Sebastian Andrzej Siewior <big...@linutronix.de> wrote:
>
> > While comparing the OMAP-serial and the 8250 part of this I noticed that
> > the the latter does not use runtime-pm.
>
> Yes it does, but 8250 parts (generally - omap presumably is special
> here ?) need to be powered on to transmit/receive not just for register
> access. The core uart layer implements a "pm" operation for this.

At least for omaps the UARTs need to be idled for the SoC to
hit off-idle where the SoC is powered off and rebooted during
idle.

So we can certainly enable this in a generic way, however, this
can only be done under the following conditions:

1. We can save and restore the serial register context and detect
when context was lost in the serial driver. The context loss
can be detected by looking at some registers that are only
initialized during init.

2. There's a way for the serial RX pin to wake the SoC. On some
omaps there's a separate pin specific wake-up interrupt that
can be used for that.

3. The serial PM features must be only enabled if requested by
the user via sysfs. Typically extra latency and loss of the
first RX character occur which is not acceptable to most
applications.

Regards,

Tony

[Sebastian Andrzej Siewior]

On 07/09/2014 01:17 PM, Tony Lindgren wrote:
> * One Thousand Gnomes <gno...@lxorguk.ukuu.org.uk> [140707 06:22]:
>> On Fri, 4 Jul 2014 18:34:10 +0200
>> Sebastian Andrzej Siewior <big...@linutronix.de> wrote:
>>
>>> While comparing the OMAP-serial and the 8250 part of this I noticed that
>>> the the latter does not use runtime-pm.
>>
>> Yes it does, but 8250 parts (generally - omap presumably is special
>> here ?) need to be powered on to transmit/receive not just for register
>> access. The core uart layer implements a "pm" operation for this.
>
> At least for omaps the UARTs need to be idled for the SoC to
> hit off-idle where the SoC is powered off and rebooted during
> idle.
>
> So we can certainly enable this in a generic way, however, this
> can only be done under the following conditions:
>
> 1. We can save and restore the serial register context and detect
> when context was lost in the serial driver. The context loss
> can be detected by looking at some registers that are only
> initialized during init.

A register check on restore context? DLL+DLH might be a good hint here
since its value should be >0 in the running case.

>
> 2. There's a way for the serial RX pin to wake the SoC. On some
> omaps there's a separate pin specific wake-up interrupt that
> can be used for that.

That one would be handled by omap separately.

> 3. The serial PM features must be only enabled if requested by
> the user via sysfs. Typically extra latency and loss of the
> first RX character occur which is not acceptable to most
> applications.

Unless I'm mistaken the omap driver now initializes the timeout to -1.
So nothing happens unless this is enabled separately.

> Regards,
>
> Tony
>

Sebastian

[Tony Lindgren]

* Sebastian Andrzej Siewior <big...@linutronix.de> [140709 04:38]:

> On 07/09/2014 01:17 PM, Tony Lindgren wrote:
> > * One Thousand Gnomes <gno...@lxorguk.ukuu.org.uk> [140707 06:22]:
> >> On Fri, 4 Jul 2014 18:34:10 +0200
> >> Sebastian Andrzej Siewior <big...@linutronix.de> wrote:
> >>
> >>> While comparing the OMAP-serial and the 8250 part of this I noticed that
> >>> the the latter does not use runtime-pm.
> >>
> >> Yes it does, but 8250 parts (generally - omap presumably is special
> >> here ?) need to be powered on to transmit/receive not just for register
> >> access. The core uart layer implements a "pm" operation for this.
> >
> > At least for omaps the UARTs need to be idled for the SoC to
> > hit off-idle where the SoC is powered off and rebooted during
> > idle.
> >
> > So we can certainly enable this in a generic way, however, this
> > can only be done under the following conditions:

Sorry forgot to mention why I think this can now be done in a
generic way, that's because we have now runtime PM in Linux.

> > 1. We can save and restore the serial register context and detect
> > when context was lost in the serial driver. The context loss
> > can be detected by looking at some registers that are only
> > initialized during init.
>
> A register check on restore context? DLL+DLH might be a good hint here
> since its value should be >0 in the running case.

OK yeah that would be a generic way to do it potentially ;)

Note that all the context_loss_cnt stuff in omap-serial.c can
then be ignored, that's still only needed for the omap3 legacy
mode booting case and won't be needed much longer.

> > 2. There's a way for the serial RX pin to wake the SoC. On some
> > omaps there's a separate pin specific wake-up interrupt that
> > can be used for that.
>
> That one would be handled by omap separately.

OK great.

> > 3. The serial PM features must be only enabled if requested by
> > the user via sysfs. Typically extra latency and loss of the
> > first RX character occur which is not acceptable to most
> > applications.
>
> Unless I'm mistaken the omap driver now initializes the timeout to -1.
> So nothing happens unless this is enabled separately.

Yes that's the only safe approach so the serial port behaves in
the standard Linux way unless specifically requested by the
user.

Regards,

Tony

[Sebastian Andrzej Siewior]

On 07/09/2014 01:48 PM, Tony Lindgren wrote:
>>> So we can certainly enable this in a generic way, however, this
>>> can only be done under the following conditions:
>
> Sorry forgot to mention why I think this can now be done in a
> generic way, that's because we have now runtime PM in Linux.

I have a problem to follow here. I need set SET_RUNTIME_PM_OPS in
pm member of the driver. This is around for a while now maybe it
wasn't around by the omap-serial was written but this doesn't matter
now.

As for the dw driver (as one of the gnomes points out) it shouldn't
matter because it is enabled / disabled based on 8250's pm callback. So
while the device is active and the 8250 core grabs & puts the
reference, nothing should change. Still a double check from Mika
wouldn't hurt.

If I understand the omap case correct (and please correct me if I don't)
the 8250-omap driver assumes that the device can go to sleep
after the probe function completes. This does not happen
because pm_runtime_set_autosuspend_delay() sets the delay to -1.
If the user changes this via sysfs, then the device might go to sleep
and the IP block switched of. Therefore we need the runtime pm
callbacks in 8250-core before the register access because we can't
access the it otherwise.
The core should wake us up in case there is incoming RX action to be
handled so we can restore register's content. And for TX we have the
proper pm hooks.
The dw driver also only disables / enables the clock. So they don't
lose the register's content like omap does. Thus they don't need the
one struct I exported.

I don't think that it makes sense to make this restore/save part
generic if this what it is about. OMAP at least has a few registers
more to take care of and set-termios is already different.

>>> 1. We can save and restore the serial register context and detect
>>> when context was lost in the serial driver. The context loss
>>> can be detected by looking at some registers that are only
>>> initialized during init.
>>
>> A register check on restore context? DLL+DLH might be a good hint here
>> since its value should be >0 in the running case.
>
> OK yeah that would be a generic way to do it potentially ;)
>
> Note that all the context_loss_cnt stuff in omap-serial.c can
> then be ignored, that's still only needed for the omap3 legacy
> mode booting case and won't be needed much longer.

Ah. The way the code reads, it is just an optimization in case the core
didn't go off after all so we don't have to restore the registers.

>>> 2. There's a way for the serial RX pin to wake the SoC. On some
>>> omaps there's a separate pin specific wake-up interrupt that
>>> can be used for that.
>>
>> That one would be handled by omap separately.
>
> OK great.
>
>>> 3. The serial PM features must be only enabled if requested by
>>> the user via sysfs. Typically extra latency and loss of the
>>> first RX character occur which is not acceptable to most
>>> applications.
>>
>> Unless I'm mistaken the omap driver now initializes the timeout to -1.
>> So nothing happens unless this is enabled separately.
>
> Yes that's the only safe approach so the serial port behaves in
> the standard Linux way unless specifically requested by the
> user.
>
> Regards,
>
> Tony

Sebastian

[Tony Lindgren]

* Sebastian Andrzej Siewior <big...@linutronix.de> [140709 05:45]:

> On 07/09/2014 01:48 PM, Tony Lindgren wrote:
> >>> So we can certainly enable this in a generic way, however, this
> >>> can only be done under the following conditions:
> >
> > Sorry forgot to mention why I think this can now be done in a
> > generic way, that's because we have now runtime PM in Linux.
>
> I have a problem to follow here. I need set SET_RUNTIME_PM_OPS in

> .pm member of the driver. This is around for a while now maybe it

> wasn't around by the omap-serial was written but this doesn't matter
> now.

Right, the lack of runtime PM was one of the reasons for doing
omap-serial earlier.

> As for the dw driver (as one of the gnomes points out) it shouldn't
> matter because it is enabled / disabled based on 8250's pm callback. So
> while the device is active and the 8250 core grabs & puts the
> reference, nothing should change. Still a double check from Mika
> wouldn't hurt.

OK

> If I understand the omap case correct (and please correct me if I don't)
> the 8250-omap driver assumes that the device can go to sleep
> after the probe function completes. This does not happen
> because pm_runtime_set_autosuspend_delay() sets the delay to -1.

Correct. And once the timeout is enabled, the serial driver
autoidles itself using runtime PM based on the timeout value.

And also please note that for runtime PM the wake-up events need
to be always enabled, so the device_may_wakeup() checks should
be only implemented for suspend and resume. I think I got that
corrected for most part in omap-serial.c recently, but knowing
that might reduce the confusion a bit :)

> If the user changes this via sysfs, then the device might go to sleep
> and the IP block switched of. Therefore we need the runtime pm
> callbacks in 8250-core before the register access because we can't
> access the it otherwise.

Correct. And it's not just the 8250 idle state, it's the whole SoC
getting powered down during idle and rebooted when waking to a
timer interrupt or device interrupt.

> The core should wake us up in case there is incoming RX action to be
> handled so we can restore register's content. And for TX we have the
> proper pm hooks.

The piece of code that runs in that case is the interrupt handler
for the serial port. That handler could be separate from the
regular serial port handler if necessary though and do a callback
to the serial core code. But I don't know if that's needed.

> The dw driver also only disables / enables the clock. So they don't
> lose the register's content like omap does. Thus they don't need the
> one struct I exported.

Right, enabling and disabling the clock is done too for omaps
when the SoC hits retention idle.

> I don't think that it makes sense to make this restore/save part
> generic if this what it is about. OMAP at least has a few registers
> more to take care of and set-termios is already different.

Yeah, and that can be done later on if others need it.

> >>> 1. We can save and restore the serial register context and detect
> >>> when context was lost in the serial driver. The context loss
> >>> can be detected by looking at some registers that are only
> >>> initialized during init.
> >>
> >> A register check on restore context? DLL+DLH might be a good hint here
> >> since its value should be >0 in the running case.
> >
> > OK yeah that would be a generic way to do it potentially ;)
> >
> > Note that all the context_loss_cnt stuff in omap-serial.c can
> > then be ignored, that's still only needed for the omap3 legacy
> > mode booting case and won't be needed much longer.
>
> Ah. The way the code reads, it is just an optimization in case the core
> didn't go off after all so we don't have to restore the registers.

Well the interconnect code knows when the context was lost on omaps,
but there's currently no Linux generic API for that. And since we
can do it by checking some registers, it's best to implement it that
way.

[Sebastian Andrzej Siewior]

On 07/09/2014 05:12 PM, Tony Lindgren wrote:
> And also please note that for runtime PM the wake-up events need
> to be always enabled, so the device_may_wakeup() checks should
> be only implemented for suspend and resume. I think I got that
> corrected for most part in omap-serial.c recently, but knowing
> that might reduce the confusion a bit :)

Ehm. I also added it to omap_8250_pm() as it is done in omap-serial (in
serial_omap_pm()). Should I get rid of it in the latter?

[Sebastian Andrzej Siewior]

On 07/07/2014 03:09 PM, One Thousand Gnomes wrote:
> Please update the patch to clearly describe the locking
> assumptions/requirements for the caller (and not to use it unless you
> have no other choice)

done.

Sebastian

[Sebastian Andrzej Siewior]

On 07/07/2014 03:09 PM, One Thousand Gnomes wrote:

> On Fri, 4 Jul 2014 18:34:09 +0200
> Sebastian Andrzej Siewior <big...@linutronix.de> wrote:
>
>> The OMAP version of the 8250 can actually use 1:1 serial8250_startup().
>> However it needs to be extended by a wakeup irq which should to be
>> requested & enabled at ->startup() time.
>>
>> Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>
>
> Acked-by: Alan Cox <al...@linux.intel.com>

Thank you. I added to this patch the counterpart function (->shutdown)
and therefore not yet adding the acked-by.

Sebastian

[Sebastian Andrzej Siewior]

The OMAP version of the 8250 can actually use 1:1 serial8250_startup().
However it needs to be extended by a wakeup irq which should to be

requested & enabled at ->startup() time and disabled at ->shutdown() time.

v1…v2: add shutdown callback

Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>

---
drivers/tty/serial/8250/8250_core.c | 22 ++++++++++++++++++++--
include/linux/serial_8250.h | 2 ++
include/linux/serial_core.h | 2 ++
3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 6db2ee2..d37eb08 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c

@@ -1939,7 +1939,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 =
container_of(port, struct uart_8250_port, port);

@@ -2191,8 +2191,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 =
container_of(port, struct uart_8250_port, port);

@@ -2243,6 +2252,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)
{
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index af47a8a..0ec21ec 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -114,6 +114,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 5bbb809..6f20ff0 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

[Sebastian Andrzej Siewior]

While comparing the OMAP-serial and the 8250 part of this I noticed that

the the latter does not use runtime-pm. Here are the pieces. It is
basically a get before first register access and a last_busy + put after
last access.
If I understand this correct, it should do nothing as long as
pm_runtime_use_autosuspend() + pm_runtime_enable() isn't invoked on the
device.

Cc: mika.we...@linux.intel.com

Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>
---

drivers/tty/serial/8250/8250_core.c | 101 +++++++++++++++++++++++++++++++-----
1 file changed, 88 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index d37eb08..1a91a89 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c

@@ -38,6 +38,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
@@ -553,10 +554,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.
*/
+ pm_runtime_get_sync(p->port.dev);
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) {
@@ -571,7 +573,17 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep)
serial_out(p, UART_EFR, 0);

serial_out(p, UART_LCR, 0);
}
+

+ if (!device_may_wakeup(p->port.dev)) {
+ if (sleep)
+ pm_runtime_forbid(p->port.dev);
+ else
+ pm_runtime_allow(p->port.dev);
+ }
}
+out:
+ pm_runtime_mark_last_busy(p->port.dev);
+ pm_runtime_put_autosuspend(p->port.dev);
}

#ifdef CONFIG_SERIAL_8250_RSA

@@ -1280,6 +1292,7 @@ static void serial8250_stop_tx(struct uart_port *port)

struct uart_8250_port *up =
container_of(port, struct uart_8250_port, port);

+ pm_runtime_get_sync(port->dev);
__stop_tx(up);

/*
@@ -1289,6 +1302,8 @@ static void serial8250_stop_tx(struct uart_port *port)
up->acr |= UART_ACR_TXDIS;
serial_icr_write(up, UART_ACR, up->acr);
}

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

static void serial8250_start_tx(struct uart_port *port)

@@ -1296,8 +1311,9 @@ static void serial8250_start_tx(struct uart_port *port)

struct uart_8250_port *up =
container_of(port, struct uart_8250_port, port);

+ pm_runtime_get_sync(port->dev);
if (up->dma && !serial8250_tx_dma(up)) {
- return;
+ goto out;
} else if (!(up->ier & UART_IER_THRI)) {
up->ier |= UART_IER_THRI;
serial_port_out(port, UART_IER, up->ier);
@@ -1318,6 +1334,9 @@ static void serial8250_start_tx(struct uart_port *port)
up->acr &= ~UART_ACR_TXDIS;
serial_icr_write(up, UART_ACR, up->acr);
}
+out:

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

static void serial8250_stop_rx(struct uart_port *port)

@@ -1325,9 +1344,14 @@ static void serial8250_stop_rx(struct uart_port *port)

struct uart_8250_port *up =
container_of(port, struct uart_8250_port, port);

+ pm_runtime_get_sync(port->dev);
+
up->ier &= ~UART_IER_RLSI;

up->port.read_status_mask &= ~UART_LSR_DR;
serial_port_out(port, UART_IER, up->ier);

+
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

static void serial8250_enable_ms(struct uart_port *port)
@@ -1340,7 +1364,10 @@ static void serial8250_enable_ms(struct uart_port *port)
return;

up->ier |= UART_IER_MSI;

+ pm_runtime_get_sync(port->dev);
serial_port_out(port, UART_IER, up->ier);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

/*
@@ -1530,9 +1557,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);
+ unsigned int iir;
+ int ret;

- return serial8250_handle_irq(port, iir);
+ pm_runtime_get_sync(port->dev);
+
+ iir = serial_port_in(port, UART_IIR);
+ ret = serial8250_handle_irq(port, iir);

+
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);

+ return ret;
}

/*
@@ -1790,11 +1825,16 @@ static unsigned int serial8250_tx_empty(struct uart_port *port)
unsigned long flags;
unsigned int lsr;

+ pm_runtime_get_sync(port->dev);
+
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);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
+

return (lsr & BOTH_EMPTY) == BOTH_EMPTY ? TIOCSER_TEMT : 0;
}

@@ -1805,7 +1845,10 @@ static unsigned int serial8250_get_mctrl(struct uart_port *port)
unsigned int status;
unsigned int ret;

+ pm_runtime_get_sync(port->dev);
status = serial8250_modem_status(up);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);

ret = 0;
if (status & UART_MSR_DCD)
@@ -1838,7 +1881,10 @@ static void serial8250_set_mctrl(struct uart_port *port, unsigned int mctrl)

mcr = (mcr & up->mcr_mask) | up->mcr_force | up->mcr;

+ pm_runtime_get_sync(port->dev);
serial_port_out(port, UART_MCR, mcr);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

static void serial8250_break_ctl(struct uart_port *port, int break_state)
@@ -1847,6 +1893,7 @@ static void serial8250_break_ctl(struct uart_port *port, int break_state)
container_of(port, struct uart_8250_port, port);
unsigned long flags;

+ pm_runtime_get_sync(port->dev);
spin_lock_irqsave(&port->lock, flags);
if (break_state == -1)
up->lcr |= UART_LCR_SBC;
@@ -1854,6 +1901,8 @@ 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);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

/*
@@ -1898,12 +1947,23 @@ 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;
+
+ pm_runtime_get_sync(port->dev);

- 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:
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
+ return status;
}

@@ -1914,6 +1974,7 @@ static void serial8250_put_poll_char(struct uart_port *port,

struct uart_8250_port *up =
container_of(port, struct uart_8250_port, port);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);

+ return retval;
}
EXPORT_SYMBOL_GPL(serial8250_do_startup);

@@ -2207,6 +2275,7 @@ void serial8250_do_shutdown(struct uart_port *port)

container_of(port, struct uart_8250_port, port);
unsigned long flags;

+ pm_runtime_get_sync(port->dev);
/*
* Disable interrupts from this port
*/

@@ -2246,6 +2315,8 @@ void serial8250_do_shutdown(struct uart_port *port)

* the IRQ chain.
*/
serial_port_in(port, UART_RX);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);

del_timer_sync(&up->timer);
up->timer.function = serial8250_timeout;

@@ -2365,6 +2436,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.
*/
+ pm_runtime_get_sync(port->dev);
spin_lock_irqsave(&port->lock, flags);

/*

@@ -2486,6 +2558,9 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
}
serial8250_set_mctrl(port, port->mctrl);
spin_unlock_irqrestore(&port->lock, flags);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
+

/* Don't rewrite B0 */

if (tty_termios_baud_rate(termios))
tty_termios_encode_baud_rate(termios, baud, baud);

[Sebastian Andrzej Siewior]

This patch provides a 8250-core based UART driver for the internal OMAP
UART. The longterm goal is to provide the same functionality as the
current OMAP uart driver and hopefully DMA support which could borrowed
from the 8250-core.

It has been only tested as console UART on am335x-evm and dra7-evm.
The device name is ttyS based instead of ttyO. If a ttyO based node name
is required please ask udev for it.

Missing:
- throttle callback

v2…v3:
- wire up startup & shutdown for wakeup-irq handling.
- RS485 handling (well the core does).

v1…v2:
- added runtime PM. Could somebody could plese double check
this? I seems to be enabled and nothing explodes. However
serial_omap_get_context_loss_count() & enable_wakeup() are
NULL pointer (in the omap-serial driver). So I am not sure how
this supposed to work :)
- added omap_8250_set_termios()

Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>
---

drivers/tty/serial/8250/8250_core.c | 8 +
drivers/tty/serial/8250/8250_omap.c | 881 ++++++++++++++++++++++++++++++++++++

drivers/tty/serial/8250/Kconfig | 9 +
drivers/tty/serial/8250/Makefile | 1 +
include/uapi/linux/serial_core.h | 3 +-

5 files changed, 901 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 bf06a4c..1cbfc8c 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -263,6 +263,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,

@@ -1340,6 +1346,8 @@ static void serial8250_stop_rx(struct uart_port *port)

pm_runtime_get_sync(port->dev);

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..b7fdbf6
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -0,0 +1,881 @@

+/*
+ * 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>

+#define OMAP_UART_SW_CLR 0xf0
+#define OMAP_UART_TCR_TRIG 0x0f
+
+#define OMAP_UART_WER_MOD_WKUP 0x7f
+#define OMAP_UART_TX_WAKEUP_EN (1 << 7)

+
+#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 serial8250_omap_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;

+ pm_runtime_get_sync(port->dev);
+ spin_lock_irqsave(&port->lock, flags);

+ up->ier |= UART_IER_MSI;
+ serial_out(up, UART_IER, up->ier);
+

+ 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 serial8250_omap_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);
+

+ if (!device_may_wakeup(port->dev)) {
+ if (!state)
+ pm_runtime_forbid(port->dev);
+ else
+ pm_runtime_allow(port->dev);
+ }

+
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
+}

+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 serial8250_omap_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;

+
+ /* 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);
+ 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 serial8250_omap_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);
+}

+ up.port.startup = omap_8250_startup;
+ up.port.shutdown = omap_8250_shutdown;

+ /*
+ * XXX
+ * throttle
+ * 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);
+ ret = serial8250_probe_rs485(&up, &pdev->dev);

+ if (ret)
+ return ret;

+ } 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 defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM_RUNTIME)

+
+static inline void serial_omap_enable_wakeirq(struct serial8250_omap_priv *priv,
+ bool enable)
+{
+ if (!priv->wakeirq)
+ return;
+
+ if (enable)
+ enable_irq(priv->wakeirq);
+ else
+ disable_irq_nosync(priv->wakeirq);
+}
+
+static void serial_omap_enable_wakeup(struct serial8250_omap_priv *priv,
+ bool enable)
+{
+ if (enable == priv->wakeups_enabled)
+ return;
+
+ serial_omap_enable_wakeirq(priv, enable);
+ priv->wakeups_enabled = enable;
+}

+#endif

+
+#ifdef CONFIG_PM_SLEEP
+static int serial_omap_prepare(struct device *dev)
+{
+ struct serial8250_omap_priv *priv = dev_get_drvdata(dev);
+

+ pr_err("%s(%d)\n", __func__, __LINE__);
+ if (!priv)
+ return 0;

+ priv->is_suspending = true;
+ return 0;
+}
+
+static void serial_omap_complete(struct device *dev)
+{
+ struct serial8250_omap_priv *priv = dev_get_drvdata(dev);
+

+ pr_err("%s(%d)\n", __func__, __LINE__);
+ if (!priv)
+ return;

+ priv->is_suspending = false;
+}
+

+static int serial_omap_lost_context(struct serial8250_omap_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 serial_omap_runtime_suspend(struct device *dev)
+{
+ struct serial8250_omap_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;
+ }
+

+ serial_out(up, UART_OMAP_WER, priv->wer);

+}
+
+static int serial_omap_runtime_resume(struct device *dev)
+{
+ struct serial8250_omap_priv *priv = dev_get_drvdata(dev);

+ int loss_cntx;
+
+ /* In case runtime-pm tries this before we are setup */
+ if (!priv)
+ return 0;
+ serial_omap_enable_wakeup(priv, false);
+ loss_cntx = serial_omap_lost_context(priv);
+
+ if (loss_cntx)

[Sebastian Andrzej Siewior]

So after I stuffed the rs485 support from the omap-serial into
8250-omap, I've been looking at it and the only omap specific part was
the OMAP_UART_SCR_TX_EMPTY part. The driver has always TX_EMPTY set
because the 8250 core expects an interrupt after the TX fifo + shift
register is empty. The rs485 parts seems to wait for half fifo and then
again for the empty fifo. With this change gone it fits fine as generic
code and here it is.

It is expected that the potential rs485 user invokes
serial_omap_probe_rs485() to configure atleast RTS gpio which is a must
have property. The code has been taken from omap-serial driver and
received limited tested due to -ENODEV (the compiler said it works).

Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>
---

drivers/tty/serial/8250/8250_core.c | 171 ++++++++++++++++++++++++++++++++++++
include/linux/serial_8250.h | 6 ++
2 files changed, 177 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index c7c3bf7..bf06a4c 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -39,6 +39,8 @@
#include <linux/mutex.h>
#include <linux/slab.h>
#include <linux/pm_runtime.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>

#ifdef CONFIG_SPARC
#include <linux/sunserialcore.h>
#endif

@@ -1281,10 +1283,34 @@ static void autoconfig_irq(struct uart_8250_port *up)

static inline void __stop_tx(struct uart_8250_port *p)
{
+ if (p->rs485.flags & SER_RS485_ENABLED) {
+ int ret;
+
+ ret = (p->rs485.flags & SER_RS485_RTS_AFTER_SEND) ? 1 : 0;
+ if (gpio_get_value(p->rts_gpio) != ret) {
+ if (p->rs485.delay_rts_after_send > 0)
+ mdelay(p->rs485.delay_rts_after_send);
+ gpio_set_value(p->rts_gpio, ret);
+ }
+ }
+
if (p->ier & UART_IER_THRI) {
p->ier &= ~UART_IER_THRI;
serial_out(p, UART_IER, p->ier);
}
+
+ if ((p->rs485.flags & SER_RS485_ENABLED) &&
+ !(p->rs485.flags & SER_RS485_RX_DURING_TX)) {
+ /*
+ * Empty the RX FIFO, we are not interested in anything
+ * received during the half-duplex transmission.
+ */
+ serial_out(p, UART_FCR, p->fcr | UART_FCR_CLEAR_RCVR);
+ /* Re-enable RX interrupts */
+ p->ier |= UART_IER_RLSI | UART_IER_RDI;
+ p->port.read_status_mask |= UART_LSR_DR;
+ serial_out(p, UART_IER, p->ier);
+ }

}

static void serial8250_stop_tx(struct uart_port *port)

@@ -1330,6 +1356,20 @@ static void serial8250_start_tx(struct uart_port *port)

if (up->dma && !serial8250_tx_dma(up)) {

goto out;
} else if (!(up->ier & UART_IER_THRI)) {

+
+ if (up->rs485.flags & SER_RS485_ENABLED) {
+ int ret;
+
+ ret = (up->rs485.flags & SER_RS485_RTS_ON_SEND) ? 1 : 0;
+ if (gpio_get_value(up->rts_gpio) != ret) {
+ gpio_set_value(up->rts_gpio, ret);
+ if (up->rs485.delay_rts_before_send > 0)
+ mdelay(up->rs485.delay_rts_before_send);
+ }
+ if (!(up->rs485.flags & SER_RS485_RX_DURING_TX))
+ serial8250_stop_rx(port);
+ }
+

up->ier |= UART_IER_THRI;
serial_port_out(port, UART_IER, up->ier);

@@ -2556,6 +2596,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
serial_port_out(port, UART_FCR, UART_FCR_ENABLE_FIFO);
serial_port_out(port, UART_FCR, fcr); /* set fcr */
}
+ up->fcr = fcr;

serial8250_set_mctrl(port, port->mctrl);
spin_unlock_irqrestore(&port->lock, flags);

pm_runtime_mark_last_busy(port->dev);
@@ -2811,6 +2852,124 @@ serial8250_verify_port(struct uart_port *port, struct serial_struct *ser)
return 0;
}

+int serial8250_probe_rs485(struct uart_8250_port *up,
+ struct device *dev)
+{
+ struct serial_rs485 *rs485conf = &up->rs485;
+ struct device_node *np = dev->of_node;
+ u32 rs485_delay[2];
+ enum of_gpio_flags flags;
+ int ret;
+
+ rs485conf->flags = 0;
+ if (!np)
+ return 0;
+
+ /* check for tx enable gpio */
+ up->rts_gpio = of_get_named_gpio_flags(np, "rts-gpio", 0, &flags);
+ if (up->rts_gpio == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ if (!gpio_is_valid(up->rts_gpio))
+ return 0;
+
+ ret = devm_gpio_request(dev, up->rts_gpio, "serial_rts");
+ if (ret < 0)
+ return ret;
+ ret = gpio_direction_output(up->rts_gpio,
+ flags & SER_RS485_RTS_AFTER_SEND);
+ if (ret < 0)
+ return ret;
+
+ up->rts_gpio_valid = true;
+
+ if (of_property_read_bool(np, "rs485-rts-active-high"))
+ rs485conf->flags |= SER_RS485_RTS_ON_SEND;
+ else
+ rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
+
+ if (of_property_read_u32_array(np, "rs485-rts-delay",
+ rs485_delay, 2) == 0) {
+ rs485conf->delay_rts_before_send = rs485_delay[0];
+ rs485conf->delay_rts_after_send = rs485_delay[1];
+ }
+
+ if (of_property_read_bool(np, "rs485-rx-during-tx"))
+ rs485conf->flags |= SER_RS485_RX_DURING_TX;
+
+ if (of_property_read_bool(np, "linux,rs485-enabled-at-boot-time"))
+ rs485conf->flags |= SER_RS485_ENABLED;

+
+ return 0;
+}

+EXPORT_SYMBOL_GPL(serial8250_probe_rs485);
+
+static void serial8250_config_rs485(struct uart_port *port,
+ struct serial_rs485 *rs485conf)

+{
+ struct uart_8250_port *up =
+ container_of(port, struct uart_8250_port, port);

+ unsigned long flags;
+ unsigned int mode;
+ int val;
+
+ pm_runtime_get_sync(port->dev);
+ spin_lock_irqsave(&up->port.lock, flags);
+
+ /* Disable interrupts from this port */
+ mode = up->ier;
+ up->ier = 0;

+ serial_out(up, UART_IER, 0);
+

+ /* store new config */
+ up->rs485 = *rs485conf;
+
+ /* enable / disable rts */
+ val = (up->rs485.flags & SER_RS485_ENABLED) ?
+ SER_RS485_RTS_AFTER_SEND : SER_RS485_RTS_ON_SEND;
+ val = (up->rs485.flags & val) ? 1 : 0;
+ gpio_set_value(up->rts_gpio, val);
+
+ /* Enable interrupts */
+ up->ier = mode;

+ serial_out(up, UART_IER, up->ier);
+

+ spin_unlock_irqrestore(&up->port.lock, flags);
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
+}
+

+static int serial8250_ioctl(struct uart_port *port, unsigned int cmd,
+ unsigned long arg)
+{
+ struct serial_rs485 rs485conf;

+ struct uart_8250_port *up;
+

+ up = container_of(port, struct uart_8250_port, port);
+ switch (cmd) {
+ case TIOCSRS485:
+ if (!gpio_is_valid(up->rts_gpio))
+ return -ENODEV;
+ if (copy_from_user(&rs485conf, (void __user *) arg,
+ sizeof(rs485conf)))
+ return -EFAULT;
+
+ serial8250_config_rs485(port, &rs485conf);
+ break;
+
+ case TIOCGRS485:
+ if (!gpio_is_valid(up->rts_gpio))
+ return -ENODEV;
+ if (copy_to_user((void __user *) arg, &up->rs485,
+ sizeof(rs485conf)))
+ return -EFAULT;
+ break;
+
+ default:
+ return -ENOIOCTLCMD;
+ }

+ return 0;
+}
+

static const char *
serial8250_type(struct uart_port *port)
{
@@ -2840,6 +2999,7 @@ static struct uart_ops serial8250_pops = {
.request_port = serial8250_request_port,
.config_port = serial8250_config_port,
.verify_port = serial8250_verify_port,
+ .ioctl = serial8250_ioctl,
#ifdef CONFIG_CONSOLE_POLL
.poll_get_char = serial8250_get_poll_char,
.poll_put_char = serial8250_put_poll_char,
@@ -3375,6 +3535,17 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
uart->tx_loadsz = up->tx_loadsz;
uart->capabilities = up->capabilities;

+ /*
+ * gpio_is_valid() is nice but if this struct wasn't initialized
+ * then it is 0 which is considered as valid. With the ioctl()
+ * which can enable the rs485 routine we could abuse a gpio.
+ */
+ if (up->rts_gpio_valid) {
+ uart->rs485 = up->rs485;
+ uart->rts_gpio = up->rts_gpio;
+ } else
+ uart->rts_gpio = -EINVAL;
+
/* Take tx_loadsz from fifosize if it wasn't set separately */
if (uart->port.fifosize && !uart->tx_loadsz)
uart->tx_loadsz = uart->port.fifosize;
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 0ec21ec..056a73f 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -78,6 +78,7 @@ struct uart_8250_port {
unsigned char acr;
unsigned char ier;
unsigned char lcr;
+ unsigned char fcr;
unsigned char mcr;
unsigned char mcr_mask; /* mask of user bits */
unsigned char mcr_force; /* mask of forced bits */
@@ -94,6 +95,9 @@ struct uart_8250_port {
unsigned char msr_saved_flags;

struct uart_8250_dma *dma;
+ struct serial_rs485 rs485;
+ int rts_gpio;
+ bool rts_gpio_valid;

/* 8250 specific callbacks */
int (*dl_read)(struct uart_8250_port *);
@@ -107,6 +111,8 @@ void serial8250_resume_port(int line);

extern int early_serial_setup(struct uart_port *port);

+extern int serial8250_probe_rs485(struct uart_8250_port *up,
+ struct device *dev);
extern int serial8250_find_port(struct uart_port *p);
extern int serial8250_find_port_for_earlycon(void);
extern unsigned int serial8250_early_in(struct uart_port *port, int offset);

[Sebastian Andrzej Siewior]

With the upcomming rs485 support it is required that
serial8250_stop_rx() is called from serial8250_start_tx(). With this
reordering there is no need for a forward declaration of the former
function.

Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>
---

drivers/tty/serial/8250/8250_core.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 1a91a89..c7c3bf7 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1306,6 +1306,21 @@ static void serial8250_stop_tx(struct uart_port *port)
pm_runtime_put_autosuspend(port->dev);
}

+static void serial8250_stop_rx(struct uart_port *port)

+{
+ struct uart_8250_port *up =
+ container_of(port, struct uart_8250_port, port);
+

+ pm_runtime_get_sync(port->dev);
+
+ up->ier &= ~UART_IER_RLSI;
+ up->port.read_status_mask &= ~UART_LSR_DR;
+ serial_port_out(port, UART_IER, up->ier);
+

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
+}
+

static void serial8250_start_tx(struct uart_port *port)

{
struct uart_8250_port *up =
@@ -1339,21 +1354,6 @@ static void serial8250_start_tx(struct uart_port *port)
pm_runtime_put_autosuspend(port->dev);
}

-static void serial8250_stop_rx(struct uart_port *port)
-{
- struct uart_8250_port *up =
- container_of(port, struct uart_8250_port, port);
-
- pm_runtime_get_sync(port->dev);
-
- up->ier &= ~UART_IER_RLSI;
- up->port.read_status_mask &= ~UART_LSR_DR;
- serial_port_out(port, UART_IER, up->ier);
-
- pm_runtime_mark_last_busy(port->dev);
- pm_runtime_put_autosuspend(port->dev);
-}
-

static void serial8250_enable_ms(struct uart_port *port)

{
struct uart_8250_port *up =

[Sebastian Andrzej Siewior]

There is no access to access a struct uart_8250_port for a specific
line. This is only required outside of the 8250/uart callbacks like for
devices' suspend & remove callbacks. For those the 8250-core provides
wrapper like serial8250_unregister_port() which passes the struct
to the proper function based on the line argument.

For runtime suspend I need access to this struct not only to make
serial_out() work but also to properly restore up->ier and up->mcr.

Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>
---

drivers/tty/serial/8250/8250.h | 2 ++
drivers/tty/serial/8250/8250_core.c | 17 +++++++++++++++++
2 files changed, 19 insertions(+)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 1ebf853..34c3cd1 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -110,6 +110,8 @@ static inline void serial_dl_write(struct uart_8250_port *up, int value)
up->dl_write(up, value);
}

+struct uart_8250_port *serial8250_get_port(int line);
+
#if defined(__alpha__) && !defined(CONFIG_PCI)
/*
* Digital did something really horribly wrong with the OUT1 and OUT2

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 7a91c6d..6db2ee2 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2755,6 +2755,23 @@ static struct uart_ops serial8250_pops = {

static struct uart_8250_port serial8250_ports[UART_NR];

+/**
+ * serial8250_get_port - retrieve struct uart_8250_port
+ * @line: serial line number
+ *
+ * This function retrieves struct uart_8250_port for the specific line.
+ * This struct *must* *not* be used to perform a 8250 or serial core operation
+ * which is not accessible otherwise. Its only purpose is to make the struct
+ * accessible to the runtime-pm callbacks for context suspend/restore.
+ * The lock assumption made here is none because runtime-pm suspend/resume
+ * callbacks should not be invoked there is any operation performed on the port.
+ */

+struct uart_8250_port *serial8250_get_port(int line)
+{
+ return &serial8250_ports[line];
+}
+EXPORT_SYMBOL_GPL(serial8250_get_port);
+
static void (*serial8250_isa_config)(int port, struct uart_port *up,
unsigned short *capabilities);

[Sebastian Andrzej Siewior]

+Subject
On 2014-07-09 19:49:31 [+0200], Sebastian Andrzej Siewior wrote:
> This is version three of the patch set. Unless something serious comes up
> I would drop the RFC on the next post.
>
> So far I should have everything covered up comparing to the omap-serial
> driver except for the throttle callbacks. And now I would slowly start
> looking into DMA support…
>
Sebastian

[Lennart Sorensen]

Usually the delay for RS485 is done in bit times, not msec. Not sure
how you expect this to work. Not sure doing it in software is precise
enough either. It probably should be calculated based on the current
baudrate with a bit time rather than msec in the DT data. No one wants
to have to change the DT data to change the baud rate. After all this
is very often used with modbus and the modbus rules specify turn around
times in bit times.

I hope TI puts this into the UART in future designs where it belongs
(similar to what Exar and many others already did).

--
Len Sorensen

[Tony Lindgren]

* Sebastian Andrzej Siewior <big...@linutronix.de> [140709 10:52]:

> While comparing the OMAP-serial and the 8250 part of this I noticed that
> the the latter does not use runtime-pm. Here are the pieces. It is
> basically a get before first register access and a last_busy + put after
> last access.
> If I understand this correct, it should do nothing as long as
> pm_runtime_use_autosuspend() + pm_runtime_enable() isn't invoked on the
> device.

..

> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c

> @@ -571,7 +573,17 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep)
> serial_out(p, UART_EFR, 0);
> serial_out(p, UART_LCR, 0);
> }
> +
> + if (!device_may_wakeup(p->port.dev)) {
> + if (sleep)
> + pm_runtime_forbid(p->port.dev);
> + else
> + pm_runtime_allow(p->port.dev);
> + }
> }
> +out:
> + pm_runtime_mark_last_busy(p->port.dev);
> + pm_runtime_put_autosuspend(p->port.dev);
> }

The device_may_wakeup logic here is wrong as I described in the
earlier thread. For runtime PM, the wake-up events should be
always enabled. So the device_may_wakeup checks should be only
done for suspend and resume.

Regards,

Tony

[Tony Lindgren]

* Sebastian Andrzej Siewior <big...@linutronix.de> [140709 09:35]:

> On 07/09/2014 05:12 PM, Tony Lindgren wrote:
> > And also please note that for runtime PM the wake-up events need
> > to be always enabled, so the device_may_wakeup() checks should
> > be only implemented for suspend and resume. I think I got that
> > corrected for most part in omap-serial.c recently, but knowing
> > that might reduce the confusion a bit :)
>
> Ehm. I also added it to omap_8250_pm() as it is done in omap-serial (in
> serial_omap_pm()). Should I get rid of it in the latter?

Yes, I commented on that patch also.

Regards,

Tony

[Tony Lindgren]

* Sebastian Andrzej Siewior <big...@linutronix.de> [140709 10:52]:

>
> v1…v2:
> - added runtime PM. Could somebody could plese double check
> this? I seems to be enabled and nothing explodes. However
> serial_omap_get_context_loss_count() & enable_wakeup() are
> NULL pointer (in the omap-serial driver). So I am not sure how
> this supposed to work :)
> - added omap_8250_set_termios()

You can test this pretty easily on beagleboard xm for example
using v3.16-r4:

1. Compile the kernel using omap2plus_defconfig and enable your
driver. USB EHCI needs to be disabled and OTG port should
not have a USB cable connected.

2. Boot with init=/bin/sh to keep user space timers to minimum
at least until you have verified to hit off-idle

3. Enable UART timeouts with something like this. You may need
to update it for ttyS, I just changed ttyO to ttyS here:

#!/bin/bash
uarts=$(find /sys/class/tty/ttyS*/device/power/ -type d)
for uart in $uarts; do
echo 3000 > $uart/autosuspend_delay_ms
done

uarts=$(find /sys/class/tty/ttyS*/power/ -type d)
for uart in $uarts; do
echo enabled > $uart/wakeup
echo auto > $uart/control
done

echo 1 > /sys/kernel/debug/pm_debug/enable_off_mode

4. Wait for UART to time out and verify you hit off-idle by
looking at the debugfs entry:

# cat /sys/kernel/debug/pm_debug/count
...
core_pwrdm (ON),OFF:6,RET:0,INA:0,ON:7,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0
...

I just tried testing this, but did not get far on my omap3 evm:

[ 5.445953] Unhandled fault: external abort on non-linefetch (0x1028) at 0xfb020000
[ 5.453674] Internal error: : 1028 [#1] SMP ARM
[ 5.458221] Modules linked in:
[ 5.461334] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.16.0-rc4-00006-gaab2c6a #98
[ 5.469024] task: ce058b00 ti: ce05a000 task.ti: ce05a000
[ 5.474456] PC is at mem32_serial_in+0xc/0x1c
[ 5.478851] LR is at serial8250_do_startup+0xc8/0x89c
[ 5.483917] pc : [<c0346f90>] lr : [<c034ab2c>] psr: 60000113
[ 5.483917] sp : ce05bd10 ip : c0a0aba8 fp : ce275400
[ 5.495452] r10: 00000000 r9 : cda7a680 r8 : ce27568c
[ 5.500701] r7 : ce275400 r6 : 00000000 r5 : ce280408 r4 : c10b6234
[ 5.507263] r3 : fb020000 r2 : 00000002 r1 : fb020000 r0 : c10b6234
[ 5.513854] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
[ 5.521179] Control: 10c5387d Table: 80004019 DAC: 00000015
[ 5.526977] Process swapper/0 (pid: 1, stack limit = 0xce05a248)
[ 5.532989] Stack: (0xce05bd10 to 0xce05c000)
..
[ 5.734771] [<c0346f90>] (mem32_serial_in) from [<c034ab2c>] (serial8250_do_startup+0xc8/0x89c)
[ 5.743530] [<c034ab2c>] (serial8250_do_startup) from [<c03461f8>] (uart_startup.part.3+0x7c/0x1dc)
[ 5.752624] [<c03461f8>] (uart_startup.part.3) from [<c0346d68>] (uart_open+0xe4/0x124)
[ 5.760681] [<c0346d68>] (uart_open) from [<c032c19c>] (tty_open+0x130/0x58c)
[ 5.767852] [<c032c19c>] (tty_open) from [<c01216f4>] (chrdev_open+0x9c/0x174)
[ 5.775115] [<c01216f4>] (chrdev_open) from [<c011b8d4>] (do_dentry_open+0x1d0/0x310)
[ 5.783020] [<c011b8d4>] (do_dentry_open) from [<c011bd98>] (finish_open+0x34/0x4c)
[ 5.790710] [<c011bd98>] (finish_open) from [<c012a8f8>] (do_last.isra.27+0x5a4/0xb98)
[ 5.798675] [<c012a8f8>] (do_last.isra.27) from [<c012afa0>] (path_openat+0xb4/0x5e4)
[ 5.806549] [<c012afa0>] (path_openat) from [<c012b7dc>] (do_filp_open+0x2c/0x80)
[ 5.814086] [<c012b7dc>] (do_filp_open) from [<c011cd14>] (do_sys_open+0x100/0x1d0)
[ 5.821777] [<c011cd14>] (do_sys_open) from [<c07b7ce0>] (kernel_init_freeable+0x124/0x1c8)
[ 5.830200] [<c07b7ce0>] (kernel_init_freeable) from [<c054eb90>] (kernel_init+0x8/0xe4)
[ 5.838348] [<c054eb90>] (kernel_init) from [<c000e868>] (ret_from_fork+0x14/0x2c)

Sounds like the clocks are not enabled properly?

Regards,

Tony

[Olivier Galibert]

On Wed, Jul 9, 2014 at 7:49 PM, Sebastian Andrzej Siewior
<big...@linutronix.de> wrote:
> + * The lock assumption made here is none because runtime-pm suspend/resume
> + * callbacks should not be invoked there is any operation performed on the port.

I think there's a missing "if"?

Best,

OG.

[One Thousand Gnomes]

> static inline void __stop_tx(struct uart_8250_port *p)
> {
> + if (p->rs485.flags & SER_RS485_ENABLED) {
> + int ret;
> +
> + ret = (p->rs485.flags & SER_RS485_RTS_AFTER_SEND) ? 1 : 0;
> + if (gpio_get_value(p->rts_gpio) != ret) {
> + if (p->rs485.delay_rts_after_send > 0)
> + mdelay(p->rs485.delay_rts_after_send);
> + gpio_set_value(p->rts_gpio, ret);
> + }

RTS is not normally a GPIO. We should be controlling the UART RTS here,
and if the UART has a magic special case RTS wired to a GPIO then really
the hardware specific part should handle that gunge. I don't care whether
the drive does it via serial_out magic or a more explicit hook but it
doesn't belong here in core code.

Likewise the mdelay probably should be in the device specific bits or
controlled by a flag as not all hardware is so braindead.

> @@ -1330,6 +1356,20 @@ static void serial8250_start_tx(struct uart_port *port)
> if (up->dma && !serial8250_tx_dma(up)) {

Ditto

> +int serial8250_probe_rs485(struct uart_8250_port *up,
> + struct device *dev)
> +{
> + struct serial_rs485 *rs485conf = &up->rs485;
> + struct device_node *np = dev->of_node;
> + u32 rs485_delay[2];
> + enum of_gpio_flags flags;
> + int ret;
> +
> + rs485conf->flags = 0;
> + if (!np)
> + return 0;
> +
> + /* check for tx enable gpio */
> + up->rts_gpio = of_get_named_gpio_flags(np, "rts-gpio", 0, &flags);

No of_ dependencies in core 8250.c either please. That looks a perfectly
good implementation of serial8250_of_probe_rs485 however, just belongs in
the right place.

> +static int serial8250_ioctl(struct uart_port *port, unsigned int cmd,
> + unsigned long arg)
> +{
> + struct serial_rs485 rs485conf;
> + struct uart_8250_port *up;
> +
> + up = container_of(port, struct uart_8250_port, port);
> + switch (cmd) {
> + case TIOCSRS485:
> + if (!gpio_is_valid(up->rts_gpio))
> + return -ENODEV;

GPIO assumption again needs to go

> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> index 0ec21ec..056a73f 100644
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -78,6 +78,7 @@ struct uart_8250_port {
> unsigned char acr;
> unsigned char ier;
> unsigned char lcr;
> + unsigned char fcr;
> unsigned char mcr;
> unsigned char mcr_mask; /* mask of user bits */
> unsigned char mcr_force; /* mask of forced bits */
> @@ -94,6 +95,9 @@ struct uart_8250_port {
> unsigned char msr_saved_flags;
>
> struct uart_8250_dma *dma;
> + struct serial_rs485 rs485;
> + int rts_gpio;
> + bool rts_gpio_valid;

Keeping the gpio here doesn't look unreasonable if one is in use.

Alan

[One Thousand Gnomes]

On Wed, 9 Jul 2014 19:49:33 +0200

Sebastian Andrzej Siewior <big...@linutronix.de> wrote:

> The OMAP version of the 8250 can actually use 1:1 serial8250_startup().
> However it needs to be extended by a wakeup irq which should to be
> requested & enabled at ->startup() time and disabled at ->shutdown() time.
>
> v1…v2: add shutdown callback
>
> Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>

1,2 & 4

Acked by: Alan Cox <al...@linux.intel.com>

[Sebastian Andrzej Siewior]

On 07/09/2014 07:49 PM, Sebastian Andrzej Siewior wrote:
> The OMAP version of the 8250 can actually use 1:1 serial8250_startup().
> However it needs to be extended by a wakeup irq which should to be
> requested & enabled at ->startup() time and disabled at ->shutdown() time.
>
> v1…v2: add shutdown callback

forgot to copy the callbacks in serial8250_register_8250_port().

Sebastian

[Sebastian Andrzej Siewior]

On 07/10/2014 09:09 AM, Tony Lindgren wrote:
> You can test this pretty easily on beagleboard xm for example
> using v3.16-r4:

I tried this with am335x-evm, dra7-evm and beaglebone (omap5-uevm and
am335x-evmsk didn't want to boot a kernel and omap4-blaze didn't even
want to show MLO/U-boot) with the same result.

> 1. Compile the kernel using omap2plus_defconfig and enable your
> driver. USB EHCI needs to be disabled and OTG port should
> not have a USB cable connected.

EHCI was already disabled in the config. OTG was not connected.

> 2. Boot with init=/bin/sh to keep user space timers to minimum
> at least until you have verified to hit off-idle

I had network up and configured. Was that okay? I also tried
"ifconfig eth0 down; sleep 10; ifconfig eth0 up" to see if it works.

> 3. Enable UART timeouts with something like this. You may need
> to update it for ttyS, I just changed ttyO to ttyS here:
>
> #!/bin/bash
> uarts=$(find /sys/class/tty/ttyS*/device/power/ -type d)
> for uart in $uarts; do
> echo 3000 > $uart/autosuspend_delay_ms
> done
>
> uarts=$(find /sys/class/tty/ttyS*/power/ -type d)
> for uart in $uarts; do
> echo enabled > $uart/wakeup
> echo auto > $uart/control
> done
>
> echo 1 > /sys/kernel/debug/pm_debug/enable_off_mode
>
> 4. Wait for UART to time out and verify you hit off-idle by
> looking at the debugfs entry:
>
> # cat /sys/kernel/debug/pm_debug/count
> ...
> core_pwrdm (ON),OFF:6,RET:0,INA:0,ON:7,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0

That core_pwrdm shows only up on dra7. However with both drivers (mine
and the current omap serial) the UART went down after three secs (as
expected) and didn't accept any characters while writing on the
console. If I wrote something on it via network (like echo a >
/dev/ttyO0) it came back and was working as long as I kept it busy. The
thing is that RX does not wake it up. Any idea?
Also, while it was I checked the core_pwrdm and I had ON:1 and OFF:0.
So something is not right.
Since Dra7 has some things missing I tried it on am335x with the same
behavior. Should it work here?

> ...
>
> I just tried testing this, but did not get far on my omap3 evm:
>
> [ 5.445953] Unhandled fault: external abort on non-linefetch (0x1028) at 0xfb020000
> [ 5.453674] Internal error: : 1028 [#1] SMP ARM
> [ 5.458221] Modules linked in:
> [ 5.461334] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.16.0-rc4-00006-gaab2c6a #98
> [ 5.469024] task: ce058b00 ti: ce05a000 task.ti: ce05a000
> [ 5.474456] PC is at mem32_serial_in+0xc/0x1c
> [ 5.478851] LR is at serial8250_do_startup+0xc8/0x89c
> [ 5.483917] pc : [<c0346f90>] lr : [<c034ab2c>] psr: 60000113
> [ 5.483917] sp : ce05bd10 ip : c0a0aba8 fp : ce275400
> [ 5.495452] r10: 00000000 r9 : cda7a680 r8 : ce27568c
> [ 5.500701] r7 : ce275400 r6 : 00000000 r5 : ce280408 r4 : c10b6234
> [ 5.507263] r3 : fb020000 r2 : 00000002 r1 : fb020000 r0 : c10b6234
> [ 5.513854] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
> [ 5.521179] Control: 10c5387d Table: 80004019 DAC: 00000015
> [ 5.526977] Process swapper/0 (pid: 1, stack limit = 0xce05a248)
> [ 5.532989] Stack: (0xce05bd10 to 0xce05c000)

> ...

> [ 5.734771] [<c0346f90>] (mem32_serial_in) from [<c034ab2c>] (serial8250_do_startup+0xc8/0x89c)
> [ 5.743530] [<c034ab2c>] (serial8250_do_startup) from [<c03461f8>] (uart_startup.part.3+0x7c/0x1dc)
> [ 5.752624] [<c03461f8>] (uart_startup.part.3) from [<c0346d68>] (uart_open+0xe4/0x124)
> [ 5.760681] [<c0346d68>] (uart_open) from [<c032c19c>] (tty_open+0x130/0x58c)
> [ 5.767852] [<c032c19c>] (tty_open) from [<c01216f4>] (chrdev_open+0x9c/0x174)
> [ 5.775115] [<c01216f4>] (chrdev_open) from [<c011b8d4>] (do_dentry_open+0x1d0/0x310)
> [ 5.783020] [<c011b8d4>] (do_dentry_open) from [<c011bd98>] (finish_open+0x34/0x4c)
> [ 5.790710] [<c011bd98>] (finish_open) from [<c012a8f8>] (do_last.isra.27+0x5a4/0xb98)
> [ 5.798675] [<c012a8f8>] (do_last.isra.27) from [<c012afa0>] (path_openat+0xb4/0x5e4)
> [ 5.806549] [<c012afa0>] (path_openat) from [<c012b7dc>] (do_filp_open+0x2c/0x80)
> [ 5.814086] [<c012b7dc>] (do_filp_open) from [<c011cd14>] (do_sys_open+0x100/0x1d0)
> [ 5.821777] [<c011cd14>] (do_sys_open) from [<c07b7ce0>] (kernel_init_freeable+0x124/0x1c8)
> [ 5.830200] [<c07b7ce0>] (kernel_init_freeable) from [<c054eb90>] (kernel_init+0x8/0xe4)
> [ 5.838348] [<c054eb90>] (kernel_init) from [<c000e868>] (ret_from_fork+0x14/0x2c)
>
> Sounds like the clocks are not enabled properly?

puh. So after staring a while at your backtrace I realized that
shutdown & startup callbacks are not overwritten properly. Well, thanks
for that. Anyway, even serial8250_do_startup() has
pm_runtime_get_sync() before first register access so I have no idea
where this is coming from.

>
> Regards,
>
> Tony
>

Sebastian

[Sebastian Andrzej Siewior]

On 07/09/2014 09:01 PM, Lennart Sorensen wrote:
>> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
>> index c7c3bf7..bf06a4c 100644
>> --- a/drivers/tty/serial/8250/8250_core.c
>> +++ b/drivers/tty/serial/8250/8250_core.c

>> @@ -1281,10 +1283,34 @@ static void autoconfig_irq(struct uart_8250_port *up)
>>
>> static inline void __stop_tx(struct uart_8250_port *p)
>> {
>> + if (p->rs485.flags & SER_RS485_ENABLED) {
>> + int ret;
>> +
>> + ret = (p->rs485.flags & SER_RS485_RTS_AFTER_SEND) ? 1 : 0;
>> + if (gpio_get_value(p->rts_gpio) != ret) {
>> + if (p->rs485.delay_rts_after_send > 0)
>> + mdelay(p->rs485.delay_rts_after_send);
>> + gpio_set_value(p->rts_gpio, ret);
>
> Usually the delay for RS485 is done in bit times, not msec. Not sure
> how you expect this to work. Not sure doing it in software is precise
> enough either. It probably should be calculated based on the current
> baudrate with a bit time rather than msec in the DT data. No one wants
> to have to change the DT data to change the baud rate. After all this
> is very often used with modbus and the modbus rules specify turn around
> times in bit times.

My understanding is that this is not baudrate related. This is the
delay that the hardware (the transceiver) to perform the change and
work.
There is the ioctl() interface which can change the delay, too. The
only required thing is the gpio.

> I hope TI puts this into the UART in future designs where it belongs
> (similar to what Exar and many others already did).
>

Sebastian

[Carlos Hernandez]

On 07/10/2014 11:47 AM, Sebastian Andrzej Siewior wrote:
> That core_pwrdm shows only up on dra7. However with both drivers (mine
> and the current omap serial) the UART went down after three secs (as
> expected) and didn't accept any characters while writing on the
> console. If I wrote something on it via network (like echo a >
> /dev/ttyO0) it came back and was working as long as I kept it busy. The
> thing is that RX does not wake it up. Any idea?

To wakeup from UART you need:
1) Append no_console_suspend to bootargs
2) echo enabled > /sys/devices/ocp.3/4806a000.serial/tty/ttyO0/power/wakeup

[menon.n...@gmail.com]

On Thu, Jul 10, 2014 at 11:03 AM, Carlos Hernandez <c...@ti.com> wrote:
> 1) Append no_console_suspend to bootargs

Not needed. ideally with pinctrl wakeup capability, this should work.
if you do no_console_suspend, the module is not powered down.

[Tony Lindgren]

* Sebastian Andrzej Siewior <big...@linutronix.de> [140710 08:50]:

> On 07/10/2014 09:09 AM, Tony Lindgren wrote:
> > You can test this pretty easily on beagleboard xm for example
> > using v3.16-r4:
>
> I tried this with am335x-evm, dra7-evm and beaglebone (omap5-uevm and
> am335x-evmsk didn't want to boot a kernel and omap4-blaze didn't even
> want to show MLO/U-boot) with the same result.

None of these SoCs support off-idle with mainline kernel so testing
with those is not enough :) Best to use some omap3 based device for
testing this.

So far I have verified that beagleboard xm, n900, and omap3730-evm
all hit off-idle with v3.16-rc4.

> > 1. Compile the kernel using omap2plus_defconfig and enable your
> > driver. USB EHCI needs to be disabled and OTG port should
> > not have a USB cable connected.
>
> EHCI was already disabled in the config. OTG was not connected.

OK

> > 2. Boot with init=/bin/sh to keep user space timers to minimum
> > at least until you have verified to hit off-idle
>
> I had network up and configured. Was that okay? I also tried
> "ifconfig eth0 down; sleep 10; ifconfig eth0 up" to see if it works.

That's fine for GPMC connected devices, devices with Ethernet on
EHCI won't idle properly AFAIK.

> > 3. Enable UART timeouts with something like this. You may need
> > to update it for ttyS, I just changed ttyO to ttyS here:
> >
> > #!/bin/bash
> > uarts=$(find /sys/class/tty/ttyS*/device/power/ -type d)
> > for uart in $uarts; do
> > echo 3000 > $uart/autosuspend_delay_ms
> > done
> >
> > uarts=$(find /sys/class/tty/ttyS*/power/ -type d)
> > for uart in $uarts; do
> > echo enabled > $uart/wakeup
> > echo auto > $uart/control
> > done
> >
> > echo 1 > /sys/kernel/debug/pm_debug/enable_off_mode
> >
> > 4. Wait for UART to time out and verify you hit off-idle by
> > looking at the debugfs entry:
> >
> > # cat /sys/kernel/debug/pm_debug/count
> > ...
> > core_pwrdm (ON),OFF:6,RET:0,INA:0,ON:7,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0
>
> That core_pwrdm shows only up on dra7. However with both drivers (mine
> and the current omap serial) the UART went down after three secs (as
> expected) and didn't accept any characters while writing on the
> console. If I wrote something on it via network (like echo a >
> /dev/ttyO0) it came back and was working as long as I kept it busy. The
> thing is that RX does not wake it up. Any idea?

If the RX pin does not wake it up, you need to configure the
pinctrl-single entry for it, and configure that pin as a wake-up
interrupt. See the interrupts-extended entry in omap3-beagle-xm.dts.

> Also, while it was I checked the core_pwrdm and I had ON:1 and OFF:0.
> So something is not right.
> Since Dra7 has some things missing I tried it on am335x with the same
> behavior. Should it work here?

Yes only omap3 currently has the pieces needed for off-idle in the
mainline kernel.

Maybe because the console is enabled for that port?

Regards,

Tony

[Sebastian Andrzej Siewior]

On 07/10/2014 04:30 PM, Olivier Galibert wrote:
> On Wed, Jul 9, 2014 at 7:49 PM, Sebastian Andrzej Siewior
> <big...@linutronix.de> wrote:
>> + * The lock assumption made here is none because runtime-pm suspend/resume
>> + * callbacks should not be invoked there is any operation performed on the port.
>
> I think there's a missing "if"?

Thanks, fixed up.

> Best,
>
> OG.
>

Sebastian

[Sebastian Andrzej Siewior]

On 07/10/2014 08:28 AM, Tony Lindgren wrote:

>> --- a/drivers/tty/serial/8250/8250_core.c
>> +++ b/drivers/tty/serial/8250/8250_core.c
>> @@ -571,7 +573,17 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep)
>> serial_out(p, UART_EFR, 0);
>> serial_out(p, UART_LCR, 0);
>> }
>> +
>> + if (!device_may_wakeup(p->port.dev)) {
>> + if (sleep)
>> + pm_runtime_forbid(p->port.dev);
>> + else
>> + pm_runtime_allow(p->port.dev);
>> + }
>> }
>> +out:
>> + pm_runtime_mark_last_busy(p->port.dev);
>> + pm_runtime_put_autosuspend(p->port.dev);
>> }
>
> The device_may_wakeup logic here is wrong as I described in the
> earlier thread. For runtime PM, the wake-up events should be
> always enabled. So the device_may_wakeup checks should be only
> done for suspend and resume.

Okay. I dropped it from here.

>
> Regards,
>
> Tony
>

Sebastian

[Sebastian Andrzej Siewior]

On 07/11/2014 08:41 AM, Tony Lindgren wrote:
>> I tried this with am335x-evm, dra7-evm and beaglebone (omap5-uevm and
>> am335x-evmsk didn't want to boot a kernel and omap4-blaze didn't even
>> want to show MLO/U-boot) with the same result.
>
> None of these SoCs support off-idle with mainline kernel so testing
> with those is not enough :) Best to use some omap3 based device for
> testing this.
>
> So far I have verified that beagleboard xm, n900, and omap3730-evm
> all hit off-idle with v3.16-rc4.

Unfortunately I don't have access to any of those devices.

>> I had network up and configured. Was that okay? I also tried
>> "ifconfig eth0 down; sleep 10; ifconfig eth0 up" to see if it works.
>
> That's fine for GPMC connected devices, devices with Ethernet on
> EHCI won't idle properly AFAIK.

am33xx has proper ethernet (cpsw IP core on SoC and not something
behind USB).

>> That core_pwrdm shows only up on dra7. However with both drivers (mine
>> and the current omap serial) the UART went down after three secs (as
>> expected) and didn't accept any characters while writing on the
>> console. If I wrote something on it via network (like echo a >
>> /dev/ttyO0) it came back and was working as long as I kept it busy. The
>> thing is that RX does not wake it up. Any idea?
>
> If the RX pin does not wake it up, you need to configure the
> pinctrl-single entry for it, and configure that pin as a wake-up
> interrupt. See the interrupts-extended entry in omap3-beagle-xm.dts.

This does not help. Checking the manual, there is not something like
PIN_OFF_WAKEUPENABLE for am33xx. There is just INPUT/OUTPUT, pull
up/down + enabled/disabled and the mux_mode. For the interrupt, the HW
referenced as omap3_pmx_core touches some bits in the pinmux register
which do not exists on am33xx.

So I have nothing on HW around that could test wakeup scenario.

>> Also, while it was I checked the core_pwrdm and I had ON:1 and OFF:0.
>> So something is not right.
>> Since Dra7 has some things missing I tried it on am335x with the same
>> behavior. Should it work here?
>
> Yes only omap3 currently has the pieces needed for off-idle in the
> mainline kernel.
>

Good to know.

>> puh. So after staring a while at your backtrace I realized that
>> shutdown & startup callbacks are not overwritten properly. Well, thanks
>> for that. Anyway, even serial8250_do_startup() has
>> pm_runtime_get_sync() before first register access so I have no idea
>> where this is coming from.
>
> Maybe because the console is enabled for that port?

Maybe. But I would expect to explode around the console code. Anyway I
fixed it up and prepare next batch.

>
> Regards,
>
> Tony


Sebastian

[Tony Lindgren]

* Sebastian Andrzej Siewior <big...@linutronix.de> [140716 05:14]:

> On 07/11/2014 08:41 AM, Tony Lindgren wrote:
> >
> > If the RX pin does not wake it up, you need to configure the
> > pinctrl-single entry for it, and configure that pin as a wake-up
> > interrupt. See the interrupts-extended entry in omap3-beagle-xm.dts.
>
> This does not help. Checking the manual, there is not something like
> PIN_OFF_WAKEUPENABLE for am33xx. There is just INPUT/OUTPUT, pull
> up/down + enabled/disabled and the mux_mode. For the interrupt, the HW
> referenced as omap3_pmx_core touches some bits in the pinmux register
> which do not exists on am33xx.

Right, on am33xx there's no IO chain wake-up path. AFAIK the only way
to provide wake events on am33xx would be to mux the RX pin temporarily
to a GPIO input. And sounds like that may work only if the GPIO is
in the first GPIO bank that's always on. I don't know if the other
GPIO banks on am33xx can provide wake up events.

> So I have nothing on HW around that could test wakeup scenario.

Bummer :( I'll give it a try again for the next revision.

Regards,

Tony

[Sekhar Nori]

On Wednesday 16 July 2014 06:02 PM, Tony Lindgren wrote:
> * Sebastian Andrzej Siewior <big...@linutronix.de> [140716 05:14]:
>> On 07/11/2014 08:41 AM, Tony Lindgren wrote:
>>>
>>> If the RX pin does not wake it up, you need to configure the
>>> pinctrl-single entry for it, and configure that pin as a wake-up
>>> interrupt. See the interrupts-extended entry in omap3-beagle-xm.dts.
>>
>> This does not help. Checking the manual, there is not something like
>> PIN_OFF_WAKEUPENABLE for am33xx. There is just INPUT/OUTPUT, pull
>> up/down + enabled/disabled and the mux_mode. For the interrupt, the HW
>> referenced as omap3_pmx_core touches some bits in the pinmux register
>> which do not exists on am33xx.
>
> Right, on am33xx there's no IO chain wake-up path. AFAIK the only way
> to provide wake events on am33xx would be to mux the RX pin temporarily
> to a GPIO input. And sounds like that may work only if the GPIO is
> in the first GPIO bank that's always on. I don't know if the other
> GPIO banks on am33xx can provide wake up events.

No, they cannot.

Thanks,
Sekhar

[Sebastian Andrzej Siewior]

There is no access to access a struct uart_8250_port for a specific
line. This is only required outside of the 8250/uart callbacks like for
devices' suspend & remove callbacks. For those the 8250-core provides
wrapper like serial8250_unregister_port() which passes the struct
to the proper function based on the line argument.

For runtime suspend I need access to this struct not only to make
serial_out() work but also to properly restore up->ier and up->mcr.

Acked-by: Alan Cox <al...@linux.intel.com>

Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>

---
drivers/tty/serial/8250/8250.h | 2 ++
drivers/tty/serial/8250/8250_core.c | 18 ++++++++++++++++++
2 files changed, 20 insertions(+)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 1ebf853..34c3cd1 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -110,6 +110,8 @@ static inline void serial_dl_write(struct uart_8250_port *up, int value)
up->dl_write(up, value);
}

+struct uart_8250_port *serial8250_get_port(int line);
+
#if defined(__alpha__) && !defined(CONFIG_PCI)
/*
* Digital did something really horribly wrong with the OUT1 and OUT2

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 7a91c6d..5a6af19 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2755,6 +2755,24 @@ static struct uart_ops serial8250_pops = {

static struct uart_8250_port serial8250_ports[UART_NR];

+/**
+ * serial8250_get_port - retrieve struct uart_8250_port
+ * @line: serial line number
+ *
+ * This function retrieves struct uart_8250_port for the specific line.
+ * This struct *must* *not* be used to perform a 8250 or serial core operation
+ * which is not accessible otherwise. Its only purpose is to make the struct
+ * accessible to the runtime-pm callbacks for context suspend/restore.

+ * The lock assumption made here is none because runtime-pm suspend/resume

+ * callbacks should not be invoked if there is any operation performed on the
+ * port.

+ */
+struct uart_8250_port *serial8250_get_port(int line)
+{
+ return &serial8250_ports[line];
+}
+EXPORT_SYMBOL_GPL(serial8250_get_port);
+
static void (*serial8250_isa_config)(int port, struct uart_port *up,
unsigned short *capabilities);

--
2.0.1

[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 invovked from the serial_core (uart_throttle()) if the feature
flags are set.

Signed-off-by: Sebastian Andrzej Siewior <big...@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 bd0e1897..714f954 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1320,6 +1320,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 =
@@ -2752,6 +2762,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,
@@ -3300,6 +3312,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 6f20ff0..c380af2 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);

[Sebastian Andrzej Siewior]

This patch provides a 8250-core based UART driver for the internal OMAP
UART. The longterm goal is to provide the same functionality as the
current OMAP uart driver and hopefully DMA support which could borrowed
from the 8250-core.

It has been only tested as console UART on am335x-evm and dra7-evm.
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

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 plese double check
this?

- added omap_8250_set_termios()

Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>
---

drivers/tty/serial/8250/8250_core.c | 8 +
drivers/tty/serial/8250/8250_omap.c | 901 ++++++++++++++++++++++++++++++++++++

drivers/tty/serial/8250/Kconfig | 9 +
drivers/tty/serial/8250/Makefile | 1 +
include/uapi/linux/serial_core.h | 3 +-

5 files changed, 921 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 aceaea1..7a33c30 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -261,6 +261,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,

@@ -1350,6 +1356,8 @@ static void serial8250_stop_rx(struct uart_port *port)
pm_runtime_get_sync(port->dev);

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..4d01f45
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -0,0 +1,901 @@

+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)

+{
+ struct uart_8250_port *up =

+ container_of(port, struct uart_8250_port, port);
+ struct omap8250_priv *priv = up->port.private_data;

+ serial_out(up, UART_IER, up->ier);
+

+ 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);

+
+ 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)

+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;

+ * 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);
+ }
+
+ 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);

[Sebastian Andrzej Siewior]

While comparing the OMAP-serial and the 8250 part of this I noticed that
the the latter does not use runtime-pm. Here are the pieces. It is
basically a get before first register access and a last_busy + put after
last access.
If I understand this correct, it should do nothing as long as
pm_runtime_use_autosuspend() + pm_runtime_enable() isn't invoked on the
device.

v3…v4:
- added runtime to the console code
- removed device_may_wakeup() from serial8250_set_sleep()

Cc: mika.we...@linux.intel.com

Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>
---

drivers/tty/serial/8250/8250_core.c | 98 ++++++++++++++++++++++++++++++++-----
1 file changed, 85 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 714f954..aceaea1 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -38,6 +38,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

@@ -553,10 +554,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.
*/
+ pm_runtime_get_sync(p->port.dev);
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 +574,9 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep)
serial_out(p, UART_LCR, 0);

}
}
+out:
+ pm_runtime_mark_last_busy(p->port.dev);
+ pm_runtime_put_autosuspend(p->port.dev);
}

#ifdef CONFIG_SERIAL_8250_RSA
@@ -1280,6 +1285,7 @@ static void serial8250_stop_tx(struct uart_port *port)
struct uart_8250_port *up =
container_of(port, struct uart_8250_port, port);

+ pm_runtime_get_sync(port->dev);
__stop_tx(up);

/*
@@ -1289,6 +1295,8 @@ static void serial8250_stop_tx(struct uart_port *port)
up->acr |= UART_ACR_TXDIS;
serial_icr_write(up, UART_ACR, up->acr);

}
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

static void serial8250_start_tx(struct uart_port *port)

@@ -1296,8 +1304,9 @@ static void serial8250_start_tx(struct uart_port *port)
struct uart_8250_port *up =
container_of(port, struct uart_8250_port, port);

+ pm_runtime_get_sync(port->dev);

if (up->dma && !serial8250_tx_dma(up)) {

- return;
+ goto out;

} else if (!(up->ier & UART_IER_THRI)) {

up->ier |= UART_IER_THRI;
serial_port_out(port, UART_IER, up->ier);

@@ -1318,6 +1327,9 @@ static void serial8250_start_tx(struct uart_port *port)
up->acr &= ~UART_ACR_TXDIS;
serial_icr_write(up, UART_ACR, up->acr);
}
+out:

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

static void serial8250_throttle(struct uart_port *port)
@@ -1335,9 +1347,14 @@ static void serial8250_stop_rx(struct uart_port *port)
struct uart_8250_port *up =
container_of(port, struct uart_8250_port, port);

+ pm_runtime_get_sync(port->dev);

+
up->ier &= ~UART_IER_RLSI;

up->port.read_status_mask &= ~UART_LSR_DR;
serial_port_out(port, UART_IER, up->ier);

+
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

static void serial8250_enable_ms(struct uart_port *port)

@@ -1350,7 +1367,10 @@ static void serial8250_enable_ms(struct uart_port *port)
return;

up->ier |= UART_IER_MSI;
+ pm_runtime_get_sync(port->dev);
serial_port_out(port, UART_IER, up->ier);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

/*
@@ -1540,9 +1560,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);
+ unsigned int iir;
+ int ret;
+
+ pm_runtime_get_sync(port->dev);
+
+ iir = serial_port_in(port, UART_IIR);
+ ret = serial8250_handle_irq(port, iir);

- return serial8250_handle_irq(port, iir);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);

+ return ret;
}

/*
@@ -1800,11 +1828,16 @@ static unsigned int serial8250_tx_empty(struct uart_port *port)
unsigned long flags;
unsigned int lsr;

+ pm_runtime_get_sync(port->dev);
+
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);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
+

return (lsr & BOTH_EMPTY) == BOTH_EMPTY ? TIOCSER_TEMT : 0;
}

@@ -1815,7 +1848,10 @@ static unsigned int serial8250_get_mctrl(struct uart_port *port)
unsigned int status;
unsigned int ret;

+ pm_runtime_get_sync(port->dev);
status = serial8250_modem_status(up);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);

ret = 0;
if (status & UART_MSR_DCD)
@@ -1848,7 +1884,10 @@ static void serial8250_set_mctrl(struct uart_port *port, unsigned int mctrl)

mcr = (mcr & up->mcr_mask) | up->mcr_force | up->mcr;

+ pm_runtime_get_sync(port->dev);
serial_port_out(port, UART_MCR, mcr);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

static void serial8250_break_ctl(struct uart_port *port, int break_state)
@@ -1857,6 +1896,7 @@ static void serial8250_break_ctl(struct uart_port *port, int break_state)
container_of(port, struct uart_8250_port, port);
unsigned long flags;

+ pm_runtime_get_sync(port->dev);
spin_lock_irqsave(&port->lock, flags);
if (break_state == -1)
up->lcr |= UART_LCR_SBC;
@@ -1864,6 +1904,8 @@ 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);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

/*
@@ -1908,12 +1950,23 @@ 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;

- if (!(lsr & UART_LSR_DR))
- return NO_POLL_CHAR;
+ pm_runtime_get_sync(port->dev);

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


@@ -1924,6 +1977,7 @@ static void serial8250_put_poll_char(struct uart_port *port,
struct uart_8250_port *up =
container_of(port, struct uart_8250_port, port);

+ pm_runtime_get_sync(up->dev);
/*
* First save the IER then disable the interrupts
*/
@@ -1945,6 +1999,9 @@ static void serial8250_put_poll_char(struct uart_port *port,
*/
wait_for_xmitr(up, BOTH_EMPTY);
serial_port_out(port, UART_IER, ier);
+ pm_runtime_mark_last_busy(up->dev);
+ pm_runtime_put_autosuspend(up->dev);
+
}

#endif /* CONFIG_CONSOLE_POLL */
@@ -1971,6 +2028,7 @@ int serial8250_do_startup(struct uart_port *port)
if (port->iotype != up->cur_iotype)
set_io_from_upio(port);

+ pm_runtime_get_sync(port->dev);
if (port->type == PORT_16C950) {
/* Wake up and initialize UART */
up->acr = 0;
@@ -1991,7 +2049,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())
@@ -2015,7 +2072,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;
}

/*
@@ -2100,7 +2158,7 @@ int serial8250_do_startup(struct uart_port *port)
} else {
retval = serial_link_irq_chain(up);
if (retval)
- return retval;
+ goto out;
}

/*
@@ -2198,8 +2256,11 @@ int serial8250_do_startup(struct uart_port *port)
outb_p(0x80, icp);
inb_p(icp);
}
-
- return 0;
+ retval = 0;
+out:

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);

+ return retval;
}
EXPORT_SYMBOL_GPL(serial8250_do_startup);

@@ -2217,6 +2278,7 @@ void serial8250_do_shutdown(struct uart_port *port)
container_of(port, struct uart_8250_port, port);
unsigned long flags;

+ pm_runtime_get_sync(port->dev);
/*

* Disable interrupts from this port
*/

@@ -2256,6 +2318,8 @@ void serial8250_do_shutdown(struct uart_port *port)
* the IRQ chain.
*/
serial_port_in(port, UART_RX);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);

del_timer_sync(&up->timer);
up->timer.function = serial8250_timeout;
@@ -2375,6 +2439,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.
*/
+ pm_runtime_get_sync(port->dev);
spin_lock_irqsave(&port->lock, flags);

/*
@@ -2496,6 +2561,9 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
}
serial8250_set_mctrl(port, port->mctrl);
spin_unlock_irqrestore(&port->lock, flags);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
+

/* Don't rewrite B0 */

if (tty_termios_baud_rate(termios))
tty_termios_encode_baud_rate(termios, baud, baud);
@@ -2931,6 +2999,8 @@ serial8250_console_write(struct console *co, const char *s, unsigned int count)

touch_nmi_watchdog();

+ pm_runtime_get_sync(port->dev);
+
if (port->sysrq || oops_in_progress)
locked = spin_trylock_irqsave(&port->lock, flags);
else
@@ -2967,6 +3037,8 @@ serial8250_console_write(struct console *co, const char *s, unsigned int count)

if (locked)
spin_unlock_irqrestore(&port->lock, flags);

+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
}

static int __init serial8250_console_setup(struct console *co, char *options)

[Sebastian Andrzej Siewior]

The OMAP version of the 8250 can actually use 1:1 serial8250_startup().
However it needs to be extended by a wakeup 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 <al...@linux.intel.com>

Signed-off-by: Sebastian Andrzej Siewior <big...@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 5a6af19..bd0e1897 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1939,7 +1939,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 =
container_of(port, struct uart_8250_port, port);

@@ -2191,8 +2191,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 =
container_of(port, struct uart_8250_port, port);

@@ -2243,6 +2252,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)
{
@@ -3304,6 +3322,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 af47a8a..0ec21ec 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -114,6 +114,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 5bbb809..6f20ff0 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);

[Sebastian Andrzej Siewior]

I converted most of the omap-serial over to the 8250-core based code where it
once was forked from. I dropped the rs485 support for now.
The runtime-pm does not crash any machines because none of them shutdown the
IP core and/or enter deep idle where it would metter.

Sebastian

[Sebastian Andrzej Siewior]

On 07/16/2014 05:16 PM, Felipe Balbi wrote:
> Hi,

Hi Felipe,

> On Wed, Jul 16, 2014 at 04:45:02PM +0200, Sebastian Andrzej Siewior
> wrote:
>> @@ -1280,6 +1285,7 @@ static void serial8250_stop_tx(struct
>> uart_port *port) struct uart_8250_port *up = container_of(port,
>> struct uart_8250_port, port);
>>
>> + pm_runtime_get_sync(port->dev); __stop_tx(up);
>>
>> /* @@ -1289,6 +1295,8 @@ static void serial8250_stop_tx(struct
>> uart_port *port) up->acr |= UART_ACR_TXDIS; serial_icr_write(up,
>> UART_ACR, up->acr); } + pm_runtime_mark_last_busy(port->dev); +
>> pm_runtime_put_autosuspend(port->dev); }
>>
>> static void serial8250_start_tx(struct uart_port *port) @@
>> -1296,8 +1304,9 @@ static void serial8250_start_tx(struct
>> uart_port *port) struct uart_8250_port *up = container_of(port,
>> struct uart_8250_port, port);
>>
>> + pm_runtime_get_sync(port->dev); if (up->dma &&
>> !serial8250_tx_dma(up)) { - return; + goto out; } else if
>> (!(up->ier & UART_IER_THRI)) { up->ier |= UART_IER_THRI;
>> serial_port_out(port, UART_IER, up->ier); @@ -1318,6 +1327,9 @@
>> static void serial8250_start_tx(struct uart_port *port) up->acr
>> &= ~UART_ACR_TXDIS; serial_icr_write(up, UART_ACR, up->acr); }
>> +out: + pm_runtime_mark_last_busy(port->dev); +
>> pm_runtime_put_autosuspend(port->dev);
>

> I wonder if you should get_sync() on start_tx() and only
> put_autosuspend() at stop_tx(). I guess the outcome would be
> largely the same, no ?

I just opened minicom on ttyS0 and gave a try. start_tx() was invoked
each time I pressed a key (sent a character). I haven't seen stop_tx()
even after after I closed minicom. I guess stop_tx() is invoked if you
switch half-duplex communication.

> well, other than in probe and other functions which need to make
> sure clocks are on, but it seems unnecessary to enable/disable in
> every function.

What do you have in mind? Do you plan to let the uart on while the
minicom is attached but is doing nothing? In that case, ->startup() and
->shutdown() should be enough.
If you want to put the uart off while the port is open but idle then
you should cover the callbacks as they are independent of each other.
You could receive three ->set_termios() even without a single
->start_tx(). And as far as I understand the whole situation, you need
to make sure the UART is "up" while you touch a single register not
only sending characters, right?

Sebastian

[Sebastian Andrzej Siewior]

On 07/16/2014 06:06 PM, Felipe Balbi wrote:

>>> well, other than in probe and other functions which need to
>>> make sure clocks are on, but it seems unnecessary to
>>> enable/disable in every function.
>>
>> What do you have in mind? Do you plan to let the uart on while
>> the minicom is attached but is doing nothing? In that case,
>> ->startup() and ->shutdown() should be enough.
>

> no the idea was to keep it on for as long as it's transferring
> characters and idle it otherwise, if that can't be done easily,
> then I guess your way is the only way.

But maybe we have to add some additional logic here to keep it up for
the transfer. I've been just (maybe over)thinking: If you send 300
bytes over DMA via 300 baud it should take 10 seconds. The PM-timeout
could hit before the transfer is complete.
Same thing with hw-flowcontrol where you could get stalled for a few
seconds.
However it doesn't seem to be a problem in current omap-serial driver.

> cheers

[Tony Lindgren]

* Sebastian Andrzej Siewior <big...@linutronix.de> [140716 07:47]:

> This patch provides a 8250-core based UART driver for the internal OMAP
> UART. The longterm goal is to provide the same functionality as the
> current OMAP uart driver and hopefully DMA support which could borrowed
> from the 8250-core.
>
> It has been only tested as console UART on am335x-evm and dra7-evm.
> 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
>
> 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 plese double check
> this?
> - added omap_8250_set_termios()

Seems to boot a bit further now with output from serial console
initially, then I'm getting the following error again that's probably
related to clocks not enabled when the registers are accessed:

[ 1.050231] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[ 1.068664] 4806a000.serial: ttyS0 at MMIO 0x4806a000 (irq = 88, base_baud = 3000000) is a OMAP
[ 1.074676] 4806c000.serial: ttyS1 at MMIO 0x4806c000 (irq = 89, base_baud = 3000000) is a OMAP
[ 1.078613] console [ttyS2] disabled
[ 1.079193] 49020000.serial: ttyS2 at MMIO 0x49020000 (irq = 90, base_baud = 3000000) is a OMAP
[ 1.938110] console [ttyS2] enabled
[ 1.946533] 49042000.serial: ttyS3 at MMIO 0x49042000 (irq = 96, base_baud = 3000000) is a OMAP
..
[ 5.538421] Unhandled fault: external abort on non-linefetch (0x1028) at 0xfb020000
[ 5.546142] Internal error: : 1028 [#1] SMP ARM
[ 5.550720] Modules linked in:
[ 5.553802] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.16.0-rc5-00070-g1f95851 #129
[ 5.561584] task: ce058b00 ti: ce05a000 task.ti: ce05a000
[ 5.567016] PC is at mem32_serial_in+0xc/0x1c
[ 5.571411] LR is at serial8250_do_startup+0xc8/0x89c
[ 5.576507] pc : [<c034731c>] lr : [<c034a988>] psr: 60000113
[ 5.576507] sp : ce05bcf0 ip : c0a008e8 fp : ce46c400
[ 5.588043] r10: 00000000 r9 : cda7a680 r8 : ce46c68c
[ 5.593292] r7 : ce46c400 r6 : 00000000 r5 : ce548e10 r4 : c10abf34
[ 5.599853] r3 : fb020000 r2 : 00000002 r1 : fb020000 r0 : c10abf34
[ 5.606414] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
[ 5.613769] Control: 10c5387d Table: 80004019 DAC: 00000015
[ 5.619537] Process swapper/0 (pid: 1, stack limit = 0xce05a248)
..
[ 5.835601] [<c034731c>] (mem32_serial_in) from [<c034a988>] (serial8250_do_startup+0xc8/0x89c)
[ 5.844360] [<c034a988>] (serial8250_do_startup) from [<c034c58c>] (omap_8250_startup+0x5c/0xdc)
[ 5.853179] [<c034c58c>] (omap_8250_startup) from [<c034b170>] (serial8250_startup+0x14/0x20)
[ 5.861755] [<c034b170>] (serial8250_startup) from [<c0346584>] (uart_startup.part.3+0x7c/0x1dc)
[ 5.870605] [<c0346584>] (uart_startup.part.3) from [<c03470f4>] (uart_open+0xe4/0x124)
[ 5.878662] [<c03470f4>] (uart_open) from [<c032c528>] (tty_open+0x130/0x58c)
[ 5.885864] [<c032c528>] (tty_open) from [<c01216ec>] (chrdev_open+0x9c/0x174)
[ 5.893127] [<c01216ec>] (chrdev_open) from [<c011b8cc>] (do_dentry_open+0x1d0/0x310)
[ 5.901000] [<c011b8cc>] (do_dentry_open) from [<c011bd90>] (finish_open+0x34/0x4c)
[ 5.908721] [<c011bd90>] (finish_open) from [<c012a8f0>] (do_last.isra.27+0x5a4/0xb98)
[ 5.916687] [<c012a8f0>] (do_last.isra.27) from [<c012af98>] (path_openat+0xb4/0x5e4)
[ 5.924560] [<c012af98>] (path_openat) from [<c012b7d4>] (do_filp_open+0x2c/0x80)
[ 5.932098] [<c012b7d4>] (do_filp_open) from [<c011cd0c>] (do_sys_open+0x100/0x1d0)
[ 5.939788] [<c011cd0c>] (do_sys_open) from [<c07b6ce0>] (kernel_init_freeable+0x124/0x1c8)
[ 5.948211] [<c07b6ce0>] (kernel_init_freeable) from [<c054ed28>] (kernel_init+0x8/0xe4)
[ 5.956359] [<c054ed28>] (kernel_init) from [<c000e868>] (ret_from_fork+0x14/0x2c)
..
[ 5.974792] In-band Error seen by MPU at address 0
[ 5.979675] ------------[ cut here ]------------
[ 5.984344] WARNING: CPU: 0 PID: 1 at drivers/bus/omap_l3_smx.c:162 omap3_l3_app_irq+0xcc/0x124()
..

Regards,

Tony

[Sebastian Andrzej Siewior]

* Tony Lindgren | 2014-07-17 00:09:00 [-0700]:

>Seems to boot a bit further now with output from serial console
>initially, then I'm getting the following error again that's probably
>related to clocks not enabled when the registers are accessed:

It is (mostly) the same thing as before. We have additionally
omap_8250_startup() in the backtrace but it is the same thing.
So you say I miss a clock? Looking through serial8250_do_startup() I see:
- pm_runtime_get_sync(port->dev); which should get the clocks up
- serial8250_clear_fifos() which does a write at address + 8. Seems to
work.
- serial_port_in(port, UART_LSR); does a read at address + 0x14, seems
to work.
- serial_port_in(port, UART_RX); does a read at address + 0. This is
probably the bad one.

Now comparing with omap-serial I noticed that I do a 32bit access
instead a 16bit.
Could you please try the following hack:

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 2e4a93b..94af5a3 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -420,13 +420,13 @@ static void mem_serial_out(struct uart_port *p, int offset, int value)
static void mem32_serial_out(struct uart_port *p, int offset, int value)
{
offset = offset << p->regshift;
- writel(value, p->membase + offset);
+ writew(value, p->membase + offset);
}

static unsigned int mem32_serial_in(struct uart_port *p, int offset)
{
offset = offset << p->regshift;
- return readl(p->membase + offset);
+ return readw(p->membase + offset);
}

static unsigned int io_serial_in(struct uart_port *p, int offset)


besides that, I don't see what could be different.

>Regards,
>
>Tony

Sebastian

[Tony Lindgren]

* Sebastian Andrzej Siewior <big...@linutronix.de> [140717 00:44]:

> * Tony Lindgren | 2014-07-17 00:09:00 [-0700]:
>
> >Seems to boot a bit further now with output from serial console
> >initially, then I'm getting the following error again that's probably
> >related to clocks not enabled when the registers are accessed:
>
> It is (mostly) the same thing as before. We have additionally
> omap_8250_startup() in the backtrace but it is the same thing.
> So you say I miss a clock? Looking through serial8250_do_startup() I see:
> - pm_runtime_get_sync(port->dev); which should get the clocks up
> - serial8250_clear_fifos() which does a write at address + 8. Seems to
> work.
> - serial_port_in(port, UART_LSR); does a read at address + 0x14, seems
> to work.
> - serial_port_in(port, UART_RX); does a read at address + 0. This is
> probably the bad one.

Hmm it could be that it works for a while because the clocks are on
from the bootloader and pm_runtime calls won't do anything. This
could happen if the interconnect data based on the ti,hwmods entry
is not getting matched to the new driver. This gets initialized when
the device entry gets created in omap_device_build_from_dt().

Or maybe something now affects the clock aliases? It seems that we
are still missing the clocks entries in the .dtsi files, see the
mappings with $ git grep uart drivers/clk/ti/

> Now comparing with omap-serial I noticed that I do a 32bit access
> instead a 16bit.
> Could you please try the following hack:

No change with that :)

Regards,

Tony

[Sebastian Andrzej Siewior]

On 07/17/2014 10:12 AM, Tony Lindgren wrote:
> Hmm it could be that it works for a while because the clocks are on
> from the bootloader and pm_runtime calls won't do anything. This
> could happen if the interconnect data based on the ti,hwmods entry
> is not getting matched to the new driver. This gets initialized when
> the device entry gets created in omap_device_build_from_dt().
>
> Or maybe something now affects the clock aliases? It seems that we
> are still missing the clocks entries in the .dtsi files, see the
> mappings with $ git grep uart drivers/clk/ti/

I've been looking for something completely different while I noticed
this:

in drivers/tty/serial/omap-serial.c
| static struct platform_driver serial_omap_driver = {
| .driver = {
| .name = DRIVER_NAME,
| },
| };
|

and DRIVER_NAME should come from include/linux/platform_data/serial-omap.h
Looking further, I've found arch/arm/mach-omap2/serial.c:
| void __init omap_serial_init_port(struct omap_board_data *bdata,
| struct omap_uart_port_info *info)
| {
| char *name
�
| name = DRIVER_NAME;
�
| pdev = omap_device_build(name, uart->num, oh, pdata, pdata_size);
�
|

Would this explain it?

> Regards,
>
> Tony

Sebastian

[Sebastian Andrzej Siewior]

On 07/17/2014 04:54 PM, Felipe Balbi wrote:
> Hi,
>

> On Wed, Jul 16, 2014 at 04:45:03PM +0200, Sebastian Andrzej Siewior wrote:
>> +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);
>

> should this pm_runtime_get_sync() be placed above
> serial8250_do_startup() call ?

I don't think it matters since serial8250_do_startup() has its own
pm_runtime_get_sync().

->startup(), ->shutdown() are called via omap callbacks so we could
spare in the 8250-core if we do it in the omap code before invoking the
function. The same goes for serial8250_set_termios() which is not used
by omap but has those runtime-pm stuff, too.
It would be wrong if someone would use the serial8250_do_startup()
without his own runtime-pm get but it is omap only which does this
things.
So it is not used by anyone else (right now) and if you want to keep it
to a minimum I could remove them from those places.

Sebastian

[Peter Hurley]

On 07/16/2014 12:06 PM, Felipe Balbi wrote:

> On Wed, Jul 16, 2014 at 05:54:56PM +0200, Sebastian Andrzej Siewior wrote:
>> On 07/16/2014 05:16 PM, Felipe Balbi wrote:

>>> I wonder if you should get_sync() on start_tx() and only
>>> put_autosuspend() at stop_tx(). I guess the outcome would be
>>> largely the same, no ?
>>
>> I just opened minicom on ttyS0 and gave a try. start_tx() was invoked
>> each time I pressed a key (sent a character). I haven't seen stop_tx()
>> even after after I closed minicom. I guess stop_tx() is invoked if you
>> switch half-duplex communication.
>

> that's bad, I expected stop to be called also after each character.

The 8250 core auto-stops tx when the tx ring buffer is empty (except
in the case of dma, where stopping tx isn't necessary).

Regards,
Peter Hurley

[Sebastian Andrzej Siewior]

* Peter Hurley | 2014-07-17 11:31:59 [-0400]:

>On 07/16/2014 12:06 PM, Felipe Balbi wrote:
>>On Wed, Jul 16, 2014 at 05:54:56PM +0200, Sebastian Andrzej Siewior wrote:
>>>On 07/16/2014 05:16 PM, Felipe Balbi wrote:
>
>>>>I wonder if you should get_sync() on start_tx() and only
>>>>put_autosuspend() at stop_tx(). I guess the outcome would be
>>>>largely the same, no ?
>>>
>>>I just opened minicom on ttyS0 and gave a try. start_tx() was invoked
>>>each time I pressed a key (sent a character). I haven't seen stop_tx()
>>>even after after I closed minicom. I guess stop_tx() is invoked if you
>>>switch half-duplex communication.
>>
>>that's bad, I expected stop to be called also after each character.
>
>The 8250 core auto-stops tx when the tx ring buffer is empty (except
>in the case of dma, where stopping tx isn't necessary).

This is correct. So this is what I have now for the non-dma case:

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 2e4a93b..480a1c0 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1283,6 +1283,9 @@ 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);
+

+ pm_runtime_mark_last_busy(p->port.dev);
+ pm_runtime_put_autosuspend(p->port.dev);
}
}

@@ -1310,12 +1313,12 @@ static void serial8250_start_tx(struct uart_port *port)

struct uart_8250_port *up =
container_of(port, struct uart_8250_port, port);

- pm_runtime_get_sync(port->dev);

if (up->dma && !serial8250_tx_dma(up)) {

goto out;
} else if (!(up->ier & UART_IER_THRI)) {
up->ier |= UART_IER_THRI;

+ pm_runtime_get_sync(port->dev);
serial_port_out(port, UART_IER, up->ier);

if (up->bugs & UART_BUG_TXEN) {
unsigned char lsr;
@@ -1500,9 +1503,10 @@ void serial8250_tx_chars(struct uart_8250_port *up)
uart_write_wakeup(port);

DEBUG_INTR("THRE...");
-
+#if 0
if (uart_circ_empty(xmit))
__stop_tx(up);
+#endif
}
EXPORT_SYMBOL_GPL(serial8250_tx_chars);


and now I need to come up with something that is not if (port != omap)
for that #if 0 block. The code disables the TX FIFO empty interrupt once
the transfer is complete. I want to call __stop_tx() once the tx fifo is
empty.
Felipe, Would a check for dev->power.use_autosuspend be the right thing
to do?

>Regards,
>Peter Hurley

Sebastian

[Sebastian Andrzej Siewior]

On 07/17/2014 06:02 PM, Felipe Balbi wrote:
>> diff --git a/drivers/tty/serial/8250/8250_core.c
>> b/drivers/tty/serial/8250/8250_core.c index 2e4a93b..480a1c0
>> 100644 --- a/drivers/tty/serial/8250/8250_core.c +++
>> b/drivers/tty/serial/8250/8250_core.c @@ -1283,6 +1283,9 @@
>> 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); + + pm_runtime_mark_last_busy(p->port.dev); +
>> pm_runtime_put_autosuspend(p->port.dev); } }
>>
>> @@ -1310,12 +1313,12 @@ static void serial8250_start_tx(struct
>> uart_port *port) struct uart_8250_port *up = container_of(port,
>> struct uart_8250_port, port);
>>
>> - pm_runtime_get_sync(port->dev); if (up->dma &&
>> !serial8250_tx_dma(up)) { goto out; } else if (!(up->ier &
>> UART_IER_THRI)) { up->ier |= UART_IER_THRI; +
>> pm_runtime_get_sync(port->dev); serial_port_out(port, UART_IER,
>> up->ier);
>>
>> if (up->bugs & UART_BUG_TXEN) { unsigned char lsr;
>

> this looks better. So we get on start_tx() and put on stop_tx().

>
>> @@ -1500,9 +1503,10 @@ void serial8250_tx_chars(struct
>> uart_8250_port *up) uart_write_wakeup(port);
>>
>> DEBUG_INTR("THRE..."); - +#if 0 if (uart_circ_empty(xmit))
>> __stop_tx(up); +#endif } EXPORT_SYMBOL_GPL(serial8250_tx_chars);
>

> is it so that start_tx() gets called one and stop_tx() might be
> called N times ? That looks unbalanced to me. If the calls are
> balanced, then you shouldn't need to care because pm_runtime will
> handle reference counting for you, right?

No, this is okay. If you look, it checks for "up->ier &
UART_IER_THRI". On the second invocation it will see that this bit is
already set and therefore won't call get_sync() for the second time.
That bit is removed in the _stop_tx() path.

>> and now I need to come up with something that is not if (port !=
>> omap) for that #if 0 block. The code disables the TX FIFO empty
>> interrupt once the transfer is complete. I want to call
>> __stop_tx() once the tx fifo is empty. Felipe, Would a check for
>> dev->power.use_autosuspend be the right thing to do?
>

> probably not, as that's internal to the pm_runtime code. But I
> wonder if start/stop tx calls are balanced, if they are then we're
> good. Unless I'm missing something else.

Do you have other ideas? It doesn't look like this is exported at all.
If we call _stop_tx() right away, then we have 64 bytes in the TX fifo
in the worst case. They should be gone "soon" but the HW-flow control
may delay it (in theory for a long time)).

[Tony Lindgren]

* Sebastian Andrzej Siewior <big...@linutronix.de> [140717 03:09]:

> On 07/17/2014 10:12 AM, Tony Lindgren wrote:
> > Hmm it could be that it works for a while because the clocks are on
> > from the bootloader and pm_runtime calls won't do anything. This
> > could happen if the interconnect data based on the ti,hwmods entry
> > is not getting matched to the new driver. This gets initialized when
> > the device entry gets created in omap_device_build_from_dt().
> >
> > Or maybe something now affects the clock aliases? It seems that we
> > are still missing the clocks entries in the .dtsi files, see the
> > mappings with $ git grep uart drivers/clk/ti/
>
> I've been looking for something completely different while I noticed
> this:
>
> in drivers/tty/serial/omap-serial.c
> | static struct platform_driver serial_omap_driver = {
> | .driver = {
> | .name = DRIVER_NAME,
> | },
> | };
> |
>
> and DRIVER_NAME should come from include/linux/platform_data/serial-omap.h
> Looking further, I've found arch/arm/mach-omap2/serial.c:
> | void __init omap_serial_init_port(struct omap_board_data *bdata,
> | struct omap_uart_port_info *info)
> | {
> | char *name

> …
> | name = DRIVER_NAME;
> …

> | pdev = omap_device_build(name, uart->num, oh, pdata, pdata_size);

> …
> |
>
> Would this explain it?

That would explain it for legacy booting, but not for device tree
based booting. I can try to debug it further on Monday.

Regards,

Tony

[Sebastian Andrzej Siewior]

On 07/17/2014 06:18 PM, Felipe Balbi wrote:

>> No, this is okay. If you look, it checks for "up->ier &
>> UART_IER_THRI". On the second invocation it will see that this
>> bit is already set and therefore won't call get_sync() for the
>> second time. That bit is removed in the _stop_tx() path.
>

> oh, right. But that's actually unnecessary. Calling
> pm_runtime_get() multiple times will just increment the usage
> counter multiple times, which means you can call __stop_tx()
> multiple times too and everything gets balanced, right ?

No. start_tx() will be called multiple times but only the first
invocation invoke pm_runtime_get(). Now I noticed that I forgot to
remove pm_runtime_put_autosuspend() at the bottom of it. But you get
the idea right?
pm_get() on the while the UART_IER_THRI is not yet set. pm_put() once
the fifo is completely empty.

>> Do you have other ideas? It doesn't look like this is exported at
>> all. If we call _stop_tx() right away, then we have 64 bytes in
>> the TX fifo in the worst case. They should be gone "soon" but the
>> HW-flow control may delay it (in theory for a long time)).
>

> this can be problematic, specially for OMAP which can go into OFF
> while idle. Whatever is in the FIFO would get lost. It seems like
> omap-serial solved this within transmit_chars().

No, it didn't.

> See how transmit_chars() is called from within IRQ handler with
> clocks enabled then it conditionally calls serial_omap_stop_tx()
> which will pm_runtime_get_sync() -> do_the_harlem_shake() ->
> pm_runtime_put_autosuspend(). That leaves one unbalanced
> pm_runtime_get() which is balanced when we're exitting the IRQ
> handler.

omap-serial and the 8250 do the following on tx path:
- start_tx()
-> sets UART_IER_THRI. This will generate an interrupt once the FIFO
is empty.
- interrupt, notices the empty fifo, invokes serial8250_start_tx()/
transmit_chars().
Both have a while loop that fills the FIFO. This loop is left once
the tty-buffer is empty (uart_circ_empty() is true) or the FIFO full.

Lets say you filled 64 bytes into the FIFO and then left because your
FIFO is full and tty-buffer is empty. That means you will invoke
serial_omap_stop_tx() and remove UART_IER_THRI bit.
This is okay because you are not interested in further FIFO empty
interrupts because you don't have any TX-bytes to be sent. However,
once you leave the transmit_chars() you leave serial_omap_irq() which
does the last pm_put(). That means you have data in the TX FIFO that is
about to be sent and the device is in auto-suspend.
This is "fine" as long as the timeout is greater then the time required
for the data be sent (plus assuming HW-float control does not stall it
for too long) so nobody notices a thing.

For that reason I added the hack / #if0 block in the 8250 driver. To
ensure we do not disable the TX-FIFO-empty interrupt even if there is
nothing to send. Instead we enter serial8250_tx_chars() once again with
empty FIFO and empty tty-buffer and will invoke _stop_tx() which also
finally does the pm_put().
That is the plan. The problem I have is how to figure out that the
device is using auto-suspend. If I don't then I would have to remove
the #if0 block and that would mean for everybody an extra interrupt
(which I wanted to avoid).

> This seems work fine and dandy without DMA, but for DMA work, I
> think we need to make sure this IP stays powered until we get DMA
> completion callback. But that's future, I guess.

Yes, probably. That means one get at dma start, one put at dma complete
callback. And I assume we get that callbacks once the DMA transfer is
complete, not when the FIFO is empty :) So lets leave it to the future
for now�

[Peter Hurley]

On 07/18/2014 11:31 AM, Felipe Balbi wrote:

> On Fri, Jul 18, 2014 at 10:35:10AM +0200, Sebastian Andrzej Siewior wrote:
>> >On 07/17/2014 06:18 PM, Felipe Balbi wrote:
>> >
>>>> > >>No, this is okay. If you look, it checks for "up->ier &
>>>> > >>UART_IER_THRI". On the second invocation it will see that this
>>>> > >>bit is already set and therefore won't call get_sync() for the
>>>> > >>second time. That bit is removed in the _stop_tx() path.
>>> > >
>>> > >oh, right. But that's actually unnecessary. Calling
>>> > >pm_runtime_get() multiple times will just increment the usage
>>> > >counter multiple times, which means you can call __stop_tx()
>>> > >multiple times too and everything gets balanced, right ?
>> >
>> >No. start_tx() will be called multiple times but only the first
>> >invocation invoke pm_runtime_get(). Now I noticed that I forgot to

> right, but that's unnecessary. You can pm_runtime_get() every time
> start_tx() is called. Just make sure to put everytime stop_tx() is
> called too.

The interface is asymmetric.

start_tx() may be invoked multiple times for which only 1 interrupt
will occur, and thus only invoke __stop_tx() once.

Regards,
Peter Hurley

[Tony Lindgren]

* Tony Lindgren <to...@atomide.com> [140717 23:28]:

Looks like the following is needed to avoid the error, no idea why..
But this is what we also do in omap-serial.c. It may be needed in
other places too?

Also, no luck yet on runtime PM. If the console is enabled omap won't
idle, and if no serial console is enabled, it just hangs. Will try
to debug this part further, it may be also related to the above.

Regards,

Tony

--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2066,8 +2066,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);

@@ -2228,8 +2228,8 @@ dont_test_tx_en:
* 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;

[Mika Westerberg]

On Wed, Jul 09, 2014 at 07:49:34PM +0200, Sebastian Andrzej Siewior wrote:
> While comparing the OMAP-serial and the 8250 part of this I noticed that
> the the latter does not use runtime-pm. Here are the pieces. It is
> basically a get before first register access and a last_busy + put after
> last access.
> If I understand this correct, it should do nothing as long as
> pm_runtime_use_autosuspend() + pm_runtime_enable() isn't invoked on the
> device.
>

> Cc: mika.we...@linux.intel.com

Sorry for the delay, just came back from vacation.

Adding Heikki, who knows the 8250_dw driver much better than me.
Unfortunately he is still on vacation for next two weeks.

> Signed-off-by: Sebastian Andrzej Siewior <big...@linutronix.de>
> ---

> drivers/tty/serial/8250/8250_core.c | 101 +++++++++++++++++++++++++++++++-----
> 1 file changed, 88 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index d37eb08..1a91a89 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c

> @@ -38,6 +38,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
> @@ -553,10 +554,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.
> */
> + pm_runtime_get_sync(p->port.dev);
> 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) {

> @@ -571,7 +573,17 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep)
> serial_out(p, UART_EFR, 0);
> serial_out(p, UART_LCR, 0);
> }
> +
> + if (!device_may_wakeup(p->port.dev)) {
> + if (sleep)
> + pm_runtime_forbid(p->port.dev);
> + else
> + pm_runtime_allow(p->port.dev);
> + }
> }
> +out:

> + pm_runtime_mark_last_busy(p->port.dev);
> + pm_runtime_put_autosuspend(p->port.dev);
> }
>

> #ifdef CONFIG_SERIAL_8250_RSA
> @@ -1280,6 +1292,7 @@ static void serial8250_stop_tx(struct uart_port *port)

> struct uart_8250_port *up =
> container_of(port, struct uart_8250_port, port);
>

> + pm_runtime_get_sync(port->dev);
> __stop_tx(up);
>
> /*

> @@ -1289,6 +1302,8 @@ static void serial8250_stop_tx(struct uart_port *port)

> up->acr |= UART_ACR_TXDIS;
> serial_icr_write(up, UART_ACR, up->acr);
> }
> + pm_runtime_mark_last_busy(port->dev);
> + pm_runtime_put_autosuspend(port->dev);
> }
>

> static void serial8250_start_tx(struct uart_port *port)

> @@ -1296,8 +1311,9 @@ static void serial8250_start_tx(struct uart_port *port)

> struct uart_8250_port *up =
> container_of(port, struct uart_8250_port, port);
>

> + pm_runtime_get_sync(port->dev);

> if (up->dma && !serial8250_tx_dma(up)) {

> - return;
> + goto out;

> } else if (!(up->ier & UART_IER_THRI)) {
> up->ier |= UART_IER_THRI;

> serial_port_out(port, UART_IER, up->ier);
> @@ -1318,6 +1334,9 @@ static void serial8250_start_tx(struct uart_port *port)

> up->acr &= ~UART_ACR_TXDIS;
> serial_icr_write(up, UART_ACR, up->acr);
> }
> +out:
> + pm_runtime_mark_last_busy(port->dev);
> + pm_runtime_put_autosuspend(port->dev);
> }
>

> static void serial8250_stop_rx(struct uart_port *port)

> @@ -1325,9 +1344,14 @@ static void serial8250_stop_rx(struct uart_port *port)

> struct uart_8250_port *up =
> container_of(port, struct uart_8250_port, port);
>

> + pm_runtime_get_sync(port->dev);
> +
> up->ier &= ~UART_IER_RLSI;
> up->port.read_status_mask &= ~UART_LSR_DR;
> serial_port_out(port, UART_IER, up->ier);
> +

> + pm_runtime_mark_last_busy(port->dev);
> + pm_runtime_put_autosuspend(port->dev);
> }
>

> static void serial8250_enable_ms(struct uart_port *port)
> @@ -1340,7 +1364,10 @@ static void serial8250_enable_ms(struct uart_port *port)

> return;
>
> up->ier |= UART_IER_MSI;

> + pm_runtime_get_sync(port->dev);
> serial_port_out(port, UART_IER, up->ier);

> + pm_runtime_mark_last_busy(port->dev);
> + pm_runtime_put_autosuspend(port->dev);
> }
>

> /*
> @@ -1530,9 +1557,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);
> + unsigned int iir;
> + int ret;
>

> - return serial8250_handle_irq(port, iir);
> + pm_runtime_get_sync(port->dev);

Is this function executed in interrupt context? Calling _sync() variant
might sleep here. At least if the RTPM of the device is backed by ACPI
methods.

> +
> + iir = serial_port_in(port, UART_IIR);
> + ret = serial8250_handle_irq(port, iir);

> +

> + pm_runtime_mark_last_busy(port->dev);
> + pm_runtime_put_autosuspend(port->dev);

> + return ret;
> }
>
> /*
> @@ -1790,11 +1825,16 @@ static unsigned int serial8250_tx_empty(struct uart_port *port)

> unsigned long flags;
> unsigned int lsr;
>
> + pm_runtime_get_sync(port->dev);
> +
> 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);
>

> + pm_runtime_mark_last_busy(port->dev);
> + pm_runtime_put_autosuspend(port->dev);

> +
> return (lsr & BOTH_EMPTY) == BOTH_EMPTY ? TIOCSER_TEMT : 0;
> }
>

> @@ -1805,7 +1845,10 @@ static unsigned int serial8250_get_mctrl(struct uart_port *port)

> unsigned int status;
> unsigned int ret;
>
> + pm_runtime_get_sync(port->dev);
> status = serial8250_modem_status(up);

> + pm_runtime_mark_last_busy(port->dev);
> + pm_runtime_put_autosuspend(port->dev);
>

> ret = 0;
> if (status & UART_MSR_DCD)

> @@ -1838,7 +1881,10 @@ static void serial8250_set_mctrl(struct uart_port *port, unsigned int mctrl)

>
> mcr = (mcr & up->mcr_mask) | up->mcr_force | up->mcr;
>
> + pm_runtime_get_sync(port->dev);
> serial_port_out(port, UART_MCR, mcr);

> + pm_runtime_mark_last_busy(port->dev);
> + pm_runtime_put_autosuspend(port->dev);
> }
>

> static void serial8250_break_ctl(struct uart_port *port, int break_state)

> @@ -1847,6 +1893,7 @@ static void serial8250_break_ctl(struct uart_port *port, int break_state)

> container_of(port, struct uart_8250_port, port);
> unsigned long flags;
>
> + pm_runtime_get_sync(port->dev);
> spin_lock_irqsave(&port->lock, flags);
> if (break_state == -1)
> up->lcr |= UART_LCR_SBC;

> @@ -1854,6 +1901,8 @@ 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);

> + pm_runtime_mark_last_busy(port->dev);
> + pm_runtime_put_autosuspend(port->dev);
> }
>

> /*
> @@ -1898,12 +1947,23 @@ 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;

> +
> + pm_runtime_get_sync(port->dev);

>
> - 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:
> + pm_runtime_mark_last_busy(up->dev);
> + pm_runtime_put_autosuspend(up->dev);
> + return status;
> }
>
>

> @@ -1914,6 +1974,7 @@ static void serial8250_put_poll_char(struct uart_port *port,

> struct uart_8250_port *up =
> container_of(port, struct uart_8250_port, port);
>

> + pm_runtime_get_sync(up->dev);
> /*
> * First save the IER then disable the interrupts
> */

> @@ -1935,6 +1996,9 @@ static void serial8250_put_poll_char(struct uart_port *port,

> */
> wait_for_xmitr(up, BOTH_EMPTY);
> serial_port_out(port, UART_IER, ier);
> + pm_runtime_mark_last_busy(up->dev);
> + pm_runtime_put_autosuspend(up->dev);
> +
> }
>
> #endif /* CONFIG_CONSOLE_POLL */

> @@ -1961,6 +2025,7 @@ int serial8250_do_startup(struct uart_port *port)

> if (port->iotype != up->cur_iotype)
> set_io_from_upio(port);
>
> + pm_runtime_get_sync(port->dev);
> if (port->type == PORT_16C950) {
> /* Wake up and initialize UART */
> up->acr = 0;

> @@ -1981,7 +2046,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())

> @@ -2005,7 +2069,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;
> }
>
> /*

> @@ -2090,7 +2155,7 @@ int serial8250_do_startup(struct uart_port *port)

> } else {
> retval = serial_link_irq_chain(up);
> if (retval)
> - return retval;
> + goto out;
> }
>
> /*

> @@ -2188,8 +2253,11 @@ int serial8250_do_startup(struct uart_port *port)

> outb_p(0x80, icp);
> inb_p(icp);
> }
> -
> - return 0;
> + retval = 0;

> +out:
> + pm_runtime_mark_last_busy(port->dev);
> + pm_runtime_put_autosuspend(port->dev);

> + return retval;
> }
> EXPORT_SYMBOL_GPL(serial8250_do_startup);
>
> @@ -2207,6 +2275,7 @@ void serial8250_do_shutdown(struct uart_port *port)

> container_of(port, struct uart_8250_port, port);
> unsigned long flags;
>
> + pm_runtime_get_sync(port->dev);
> /*
> * Disable interrupts from this port
> */

> @@ -2246,6 +2315,8 @@ void serial8250_do_shutdown(struct uart_port *port)

> * the IRQ chain.
> */
> serial_port_in(port, UART_RX);

> + pm_runtime_mark_last_busy(port->dev);
> + pm_runtime_put_autosuspend(port->dev);
>

> del_timer_sync(&up->timer);
> up->timer.function = serial8250_timeout;

> @@ -2365,6 +2436,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.
> */
> + pm_runtime_get_sync(port->dev);
> spin_lock_irqsave(&port->lock, flags);
>
> /*

> @@ -2486,6 +2558,9 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,

> }
> serial8250_set_mctrl(port, port->mctrl);
> spin_unlock_irqrestore(&port->lock, flags);

> + pm_runtime_mark_last_busy(port->dev);
> + pm_runtime_put_autosuspend(port->dev);

> +
> /* Don't rewrite B0 */
> if (tty_termios_baud_rate(termios))
> tty_termios_encode_baud_rate(termios, baud, baud);

> --
> 2.0.1

[Heikki Krogerus]

On Wed, Jul 09, 2014 at 07:49:37PM +0200, Sebastian Andrzej Siewior wrote:
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c

> index bf06a4c..1cbfc8c 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -263,6 +263,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,

> @@ -1340,6 +1346,8 @@ static void serial8250_stop_rx(struct uart_port *port)

> pm_runtime_get_sync(port->dev);
>
> 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);

Alan couldn't UART_IER_RDI be always cleared here with all port types?
Actually, shouldn't it be?

Then the custom port type PORT_OMAP_16750 would not be needed.

--
heikki

[Sebastian Andrzej Siewior]

On 07/21/2014 11:35 AM, Tony Lindgren wrote:

> Looks like the following is needed to avoid the error, no idea why..
> But this is what we also do in omap-serial.c. It may be needed in
> other places too?

Yes, there was a third one on shutdown.

> Also, no luck yet on runtime PM. If the console is enabled omap won't
> idle, and if no serial console is enabled, it just hangs. Will try
> to debug this part further, it may be also related to the above.

My beaglebone xm won't do idle:

| omap3_pm_off_mode_enable: Core OFF disabled due to errata i583

but it would if it could, right? That means that I can't test the wake
via UART on this one?

So I will try to fix the tx-pm, fold patches and then prepare another
batch.
And I could verify that DMA works on that omap3 :)

> Regards,
>
> Tony

Sebastian

[Tony Lindgren]

* Sebastian Andrzej Siewior <big...@linutronix.de> [140813 09:23]:

> On 07/21/2014 11:35 AM, Tony Lindgren wrote:
>
> > Looks like the following is needed to avoid the error, no idea why..
> > But this is what we also do in omap-serial.c. It may be needed in
> > other places too?
>
> Yes, there was a third one on shutdown.

OK

> > Also, no luck yet on runtime PM. If the console is enabled omap won't
> > idle, and if no serial console is enabled, it just hangs. Will try
> > to debug this part further, it may be also related to the above.
>
> My beaglebone xm won't do idle:
>
> | omap3_pm_off_mode_enable: Core OFF disabled due to errata i583
>
> but it would if it could, right? That means that I can't test the wake
> via UART on this one?

Hmm OK yes that's disabled for some earlier ES revisions. I guess you
could comment out the limitations for PM_SDRC_WAKEUP_ERRATUM_i583
in both cpuidle34xx.c and pm34xx.c and see if off-idle is usable for
your tests :) It might be flakey but still usable for testing.

> So I will try to fix the tx-pm, fold patches and then prepare another
> batch.

OK great, let me know if you need more help.

> And I could verify that DMA works on that omap3 :)

OK

Regards,

Tony