Discussion:
[PATCH 00/16 v9] omap 8250 based uart + DMA
Sebastian Andrzej Siewior
2014-09-10 19:29:55 UTC
Permalink
the diff of v8=E2=80=A6v9 is small:
- rebased on top's of Greg's tty-next branch
- fixed #10 where we might have THRE interrupt enabled for longer than
needed
- re-did register setup in #10. Before this "less file" could freeze th=
e
am335x-evm.

Sebastian

--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-10 19:29:58 UTC
Permalink
The serial8250_do_startup() function unconditionally clears the
interrupts and for that it reads from the RX-FIFO without checking if
there is a byte in the FIFO or not. This works fine on OMAP4+ HW like
AM335x or DRA7.
OMAP3630 ES1.1 (which means probably all OMAP3 and earlier) does not like
this:

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

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

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 3cf5c98013e4..547afde9fdda 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2088,8 +2088,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);

@@ -2250,8 +2250,8 @@ int serial8250_do_startup(struct uart_port *port)
* saved flags to avoid getting false values from polling
* routines or the previous session.
*/
- serial_port_in(port, UART_LSR);
- serial_port_in(port, UART_RX);
+ if (serial_port_in(port, UART_LSR) & UART_LSR_DR)
+ serial_port_in(port, UART_RX);
serial_port_in(port, UART_IIR);
serial_port_in(port, UART_MSR);
up->lsr_saved_flags = 0;
@@ -2344,7 +2344,8 @@ void serial8250_do_shutdown(struct uart_port *port)
* Read data port to reset things, and then unlink from
* the IRQ chain.
*/
- serial_port_in(port, UART_RX);
+ if (serial_port_in(port, UART_LSR) & UART_LSR_DR)
+ serial_port_in(port, UART_RX);
serial8250_rpm_put(up);

del_timer_sync(&up->timer);
--
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-10 19:29:56 UTC
Permalink
The OMAP UART provides support for HW assisted flow control. What is
missing is the support to throttle / unthrottle callbacks which are used
by the omap-serial driver at the moment.
This patch adds the callbacks. It should be safe to add them since they
are only invoked from the serial_core (uart_throttle()) if the feature
flags are set.

Reviewed-by: Tony Lindgren <***@atomide.com>
Tested-by: Tony Lindgren <***@atomide.com>
Signed-off-by: Sebastian Andrzej Siewior <***@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 a0c1d64f34c5..68c44d97091b 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1319,6 +1319,16 @@ static void serial8250_start_tx(struct uart_port *port)
}
}

+static void serial8250_throttle(struct uart_port *port)
+{
+ port->throttle(port);
+}
+
+static void serial8250_unthrottle(struct uart_port *port)
+{
+ port->unthrottle(port);
+}
+
static void serial8250_stop_rx(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
@@ -2912,6 +2922,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,
@@ -3462,6 +3474,8 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
uart->capabilities = up->capabilities;
uart->rs485_config = up->rs485_config;
uart->rs485 = up->rs485;
+ 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 3bd7d55eebce..80936c68a7d3 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -124,6 +124,8 @@ struct uart_port {
struct ktermios *old);
int (*startup)(struct uart_port *port);
void (*shutdown)(struct uart_port *port);
+ void (*throttle)(struct uart_port *port);
+ void (*unthrottle)(struct uart_port *port);
int (*handle_irq)(struct uart_port *);
void (*pm)(struct uart_port *, unsigned int state,
unsigned int old);
--
2.1.0
Sebastian Andrzej Siewior
2014-09-10 19:30:02 UTC
Permalink
Right now it is possible that serial8250_tx_dma() fails and returns
-EBUSY. The caller (serial8250_start_tx()) will then enable
UART_IER_THRI which will generate an interrupt once the TX FIFO is
empty.
In serial8250_handle_irq() nothing will happen because up->dma is set
and so serial8250_tx_chars() won't be invoked. We end up with plenty of
interrupts and some "too much work for irq" output.

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

Reviewed-by: Tony Lindgren <***@atomide.com>
Tested-by: Tony Lindgren <***@atomide.com>
Signed-off-by: Sebastian Andrzej Siewior <***@linutronix.de>
---
drivers/tty/serial/8250/8250.h | 1 +
drivers/tty/serial/8250/8250_core.c | 4 +++-
drivers/tty/serial/8250/8250_dma.c | 30 +++++++++++++++++++++++++-----
3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 1bcb4b2141a6..a63d198f8d03 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -41,6 +41,7 @@ struct uart_8250_dma {
size_t tx_size;

unsigned char tx_running:1;
+ unsigned char tx_err: 1;
};

struct old_serial_port {
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 139f3d2b8aa9..9f961ea82564 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1594,8 +1594,10 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
status = serial8250_rx_chars(up, status);
}
serial8250_modem_status(up);
- if (!up->dma && (status & UART_LSR_THRE))
+ if ((!up->dma || (up->dma && up->dma->tx_err)) &&
+ (status & UART_LSR_THRE)) {
serial8250_tx_chars(up);
+ }

spin_unlock_irqrestore(&port->lock, flags);
return 1;
diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index 148ffe4c232f..69e54abb6e71 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -36,8 +36,16 @@ static void __dma_tx_complete(void *param)
if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
uart_write_wakeup(&p->port);

- if (!uart_circ_empty(xmit) && !uart_tx_stopped(&p->port))
- serial8250_tx_dma(p);
+ if (!uart_circ_empty(xmit) && !uart_tx_stopped(&p->port)) {
+ int ret;
+
+ ret = serial8250_tx_dma(p);
+ if (ret) {
+ dma->tx_err = 1;
+ p->ier |= UART_IER_THRI;
+ serial_port_out(&p->port, UART_IER, p->ier);
+ }
+ }

spin_unlock_irqrestore(&p->port.lock, flags);
}
@@ -69,6 +77,7 @@ int serial8250_tx_dma(struct uart_8250_port *p)
struct uart_8250_dma *dma = p->dma;
struct circ_buf *xmit = &p->port.state->xmit;
struct dma_async_tx_descriptor *desc;
+ int ret;

if (uart_tx_stopped(&p->port) || dma->tx_running ||
uart_circ_empty(xmit))
@@ -80,8 +89,10 @@ int serial8250_tx_dma(struct uart_8250_port *p)
dma->tx_addr + xmit->tail,
dma->tx_size, DMA_MEM_TO_DEV,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
- if (!desc)
- return -EBUSY;
+ if (!desc) {
+ ret = -EBUSY;
+ goto err;
+ }

dma->tx_running = 1;

@@ -94,8 +105,17 @@ int serial8250_tx_dma(struct uart_8250_port *p)
UART_XMIT_SIZE, DMA_TO_DEVICE);

dma_async_issue_pending(dma->txchan);
-
+ if (dma->tx_err) {
+ dma->tx_err = 0;
+ if (p->ier & UART_IER_THRI) {
+ p->ier &= ~UART_IER_THRI;
+ serial_out(p, UART_IER, p->ier);
+ }
+ }
return 0;
+err:
+ dma->tx_err = 1;
+ return ret;
}
EXPORT_SYMBOL_GPL(serial8250_tx_dma);
--
2.1.0
Sebastian Andrzej Siewior
2014-09-10 19:30:01 UTC
Permalink
This patch provides a 8250-core based UART driver for the internal OMAP
UART. The long term goal is to provide the same functionality as the
current OMAP uart driver and DMA support.
I tried to merge omap-serial code together with the 8250-core code.
There should should be hardly a noticable difference. The trigger level=
s
are different compared to omap-serial:
- omap serial
TX: Interrupt comes after TX FIFO has room for 16 bytes.
TX of 4096 bytes in one go results in 256 interrupts

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

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

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

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

The device name is ttyS based instead of ttyO. If a ttyO based node nam=
e
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

v8=E2=80=A6v9:
- less on a file seems to hang the am335x after a while. I
believe I introduce this bug a while ago since I can reproduce
this prior to v8. Fixed by redoing the omap8250_restore_regs()
v7=E2=80=A6v8:
- redo the register write. There is now one function for that
which is used from set_termios() and runtime-resume.
- drop PORT_OMAP_16750 and move the setup to the omap file. We
have our own set termios function anyway (Heikki Krogerus)
- use MEM instead of MEM32. TRM of AM/DM37x says that 32bit
access on THR might result in data abort. We only need 32bit
access in the errata function which is before we use 8250's
read function so it doesn't matter.
v4=E2=80=A6v7:
- change trigger levels after some tests with raw transfers.
v3=E2=80=A6v4:
- drop RS485 support
- wire up ->throttle / ->unthrottle
v2=E2=80=A6v3:
- wire up startup & shutdown for wakeup-irq handling.
- RS485 handling (well the core does).

v1=E2=80=A6v2:
- added runtime PM. Could somebody could please double check
this?
- added omap_8250_set_termios()

Reviewed-by: Tony Lindgren <***@atomide.com>
Tested-by: Tony Lindgren <***@atomide.com>
Signed-off-by: Sebastian Andrzej Siewior <***@linutronix.de>
---
drivers/tty/serial/8250/8250_omap.c | 911 ++++++++++++++++++++++++++++=
++++++++
drivers/tty/serial/8250/Kconfig | 9 +
drivers/tty/serial/8250/Makefile | 1 +
3 files changed, 921 insertions(+)
create mode 100644 drivers/tty/serial/8250/8250_omap.c

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8=
250/8250_omap.c
new file mode 100644
index 000000000000..2a187b00ed0a
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -0,0 +1,911 @@
+/*
+ * 8250-core based driver for the OMAP internal UART
+ *
+ * Copyright (C) 2014 Sebastian Andrzej Siewior
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/serial_8250.h>
+#include <linux/serial_core.h>
+#include <linux/serial_reg.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/of_irq.h>
+#include <linux/delay.h>
+#include <linux/pm_runtime.h>
+#include <linux/console.h>
+#include <linux/pm_qos.h>
+
+#include "8250.h"
+
+#define DEFAULT_CLK_SPEED 48000000
+
+#define UART_ERRATA_i202_MDR1_ACCESS (1 << 0)
+#define OMAP_UART_WER_HAS_TX_WAKEUP (1 << 1)
+
+#define OMAP_UART_FCR_RX_TRIG 6
+#define OMAP_UART_FCR_TX_TRIG 4
+
+/* SCR register bitmasks */
+#define OMAP_UART_SCR_RX_TRIG_GRANU1_MASK (1 << 7)
+#define OMAP_UART_SCR_TX_TRIG_GRANU1_MASK (1 << 6)
+#define OMAP_UART_SCR_TX_EMPTY (1 << 3)
+#define OMAP_UART_SCR_DMAMODE_MASK (3 << 1)
+#define OMAP_UART_SCR_DMAMODE_1 (1 << 1)
+#define OMAP_UART_SCR_DMAMODE_CTL (1 << 0)
+
+/* MVR register bitmasks */
+#define OMAP_UART_MVR_SCHEME_SHIFT 30
+#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
+
+#define UART_TI752_TLR_TX 0
+#define UART_TI752_TLR_RX 4
+
+#define TRIGGER_TLR_MASK(x) ((x & 0x3c) >> 2)
+#define TRIGGER_FCR_MASK(x) (x & 3)
+
+/* Enable XON/XOFF flow control on output */
+#define OMAP_UART_SW_TX 0x08
+/* Enable XON/XOFF flow control on input */
+#define OMAP_UART_SW_RX 0x02
+
+#define OMAP_UART_WER_MOD_WKUP 0x7f
+#define OMAP_UART_TX_WAKEUP_EN (1 << 7)
+
+#define TX_TRIGGER 1
+#define RX_TRIGGER 48
+
+#define OMAP_UART_TCR_RESTORE(x) ((x / 4) << 4)
+#define OMAP_UART_TCR_HALT(x) ((x / 4) << 0)
+
+#define UART_BUILD_REVISION(x, y) (((x) << 8) | (y))
+
+#define OMAP_UART_REV_46 0x0406
+#define OMAP_UART_REV_52 0x0502
+#define OMAP_UART_REV_63 0x0603
+
+struct omap8250_priv {
+ int line;
+ u32 habit;
+ u32 mdr1;
+ u32 efr;
+ u32 quot;
+ u32 scr;
+ u32 wer;
+ u32 xon;
+ u32 xoff;
+
+ 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;
+ struct uart_8250_dma omap8250_dma;
+ bool dma_active;
+};
+
+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 =3D
+ * 5 L4 clock cycles + 5 UART functional clock cycle (@48MHz =3D ~0.2u=
S)
+ * give 10 times as much
+ */
+static void omap_8250_mdr1_errataset(struct uart_8250_port *up, u8 mdr=
1)
+{
+ u8 timeout =3D 255;
+
+ serial_out(up, UART_OMAP_MDR1, mdr1);
+ udelay(2);
+ serial_out(up, UART_FCR, up->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 !=3D (serial_in(up, UART_LSR) &
+ (UART_LSR_THRE | UART_LSR_DR))) {
+ timeout--;
+ if (!timeout) {
+ /* Should *never* happen. we warn and carry on */
+ dev_crit(up->port.dev, "Errata i202: timedout %x\n",
+ serial_in(up, UART_LSR));
+ break;
+ }
+ udelay(1);
+ }
+}
+
+static void omap_8250_get_divisor(struct uart_port *port, unsigned int=
baud,
+ struct omap8250_priv *priv)
+{
+ unsigned int uartclk =3D port->uartclk;
+ unsigned int div_13, div_16;
+ unsigned int abs_d13, abs_d16;
+
+ /*
+ * Old custom speed handling.
+ */
+ if (baud =3D=3D 38400 && (port->flags & UPF_SPD_MASK) =3D=3D UPF_SPD_=
CUST) {
+ priv->quot =3D 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 =3D UART_OMAP_MDR1_13X_MODE;
+ else
+ priv->mdr1 =3D UART_OMAP_MDR1_16X_MODE;
+ return;
+ }
+ div_13 =3D DIV_ROUND_CLOSEST(uartclk, 13 * baud);
+ div_16 =3D DIV_ROUND_CLOSEST(uartclk, 16 * baud);
+
+ abs_d13 =3D abs(baud - port->uartclk / 13 / div_13);
+ abs_d16 =3D abs(baud - port->uartclk / 16 / div_16);
+
+ if (abs_d13 >=3D abs_d16) {
+ priv->mdr1 =3D UART_OMAP_MDR1_16X_MODE;
+ priv->quot =3D div_16;
+ } else {
+ priv->mdr1 =3D UART_OMAP_MDR1_13X_MODE;
+ priv->quot =3D div_13;
+ }
+}
+
+static void omap8250_update_scr(struct uart_8250_port *up,
+ struct omap8250_priv *priv)
+{
+ /*
+ * The manual recommends not to enable the DMA mode selector in the S=
CR
+ * (instead of the FCR) register _and_ selecting the DMA mode as one
+ * register write because this may lead to malfunction.
+ */
+ if (priv->scr & OMAP_UART_SCR_DMAMODE_MASK)
+ serial_out(up, UART_OMAP_SCR,
+ priv->scr & ~OMAP_UART_SCR_DMAMODE_MASK);
+ serial_out(up, UART_OMAP_SCR, priv->scr);
+}
+
+static void omap8250_restore_regs(struct uart_8250_port *up)
+{
+ struct omap8250_priv *priv =3D up->port.private_data;
+
+ 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, 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, UART_MCR_TCRTLR);
+ serial_out(up, UART_FCR, up->fcr);
+
+ omap8250_update_scr(up, priv);
+
+ /* Protocol, Baud Rate, and Interrupt Settings */
+ /* need mode A for FCR */
+ 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_TI752_TCR, OMAP_UART_TCR_RESTORE(16) |
+ OMAP_UART_TCR_HALT(52));
+ serial_out(up, UART_TI752_TLR,
+ TRIGGER_TLR_MASK(TX_TRIGGER) << UART_TI752_TLR_TX |
+ TRIGGER_TLR_MASK(RX_TRIGGER) << UART_TI752_TLR_RX);
+
+ serial_out(up, UART_LCR, 0);
+
+ /* drop TCR + TLR access, we setup XON/XOFF later */
+ serial_out(up, UART_MCR, up->mcr);
+ 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_EFR, priv->efr);
+
+ serial_out(up, UART_LCR, up->lcr);
+ /* need mode A for FCR */
+ 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);
+ serial_out(up, UART_XON1, priv->xon);
+ serial_out(up, UART_XOFF1, priv->xoff);
+
+ serial_out(up, UART_LCR, up->lcr);
+ up->port.ops->set_mctrl(&up->port, up->port.mctrl);
+}
+/*
+ * OMAP can use "CLK / (16 or 13) / div" for baud rate. And then we ha=
ve 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 =3D
+ container_of(port, struct uart_8250_port, port);
+ struct omap8250_priv *priv =3D up->port.private_data;
+ unsigned char cval =3D 0;
+ unsigned long flags =3D 0;
+ unsigned int baud;
+
+ switch (termios->c_cflag & CSIZE) {
+ case CS5:
+ cval =3D UART_LCR_WLEN5;
+ break;
+ case CS6:
+ cval =3D UART_LCR_WLEN6;
+ break;
+ case CS7:
+ cval =3D UART_LCR_WLEN7;
+ break;
+ default:
+ case CS8:
+ cval =3D UART_LCR_WLEN8;
+ break;
+ }
+
+ if (termios->c_cflag & CSTOPB)
+ cval |=3D UART_LCR_STOP;
+ if (termios->c_cflag & PARENB)
+ cval |=3D UART_LCR_PARITY;
+ if (!(termios->c_cflag & PARODD))
+ cval |=3D UART_LCR_EPAR;
+ if (termios->c_cflag & CMSPAR)
+ cval |=3D UART_LCR_SPAR;
+
+ /*
+ * Ask the core to calculate the divisor for us.
+ */
+ baud =3D 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 =3D UART_LSR_OE | UART_LSR_THRE | UART_LSR_=
DR;
+ if (termios->c_iflag & INPCK)
+ up->port.read_status_mask |=3D UART_LSR_FE | UART_LSR_PE;
+ if (termios->c_iflag & (BRKINT | PARMRK))
+ up->port.read_status_mask |=3D UART_LSR_BI;
+
+ /*
+ * Characters to ignore
+ */
+ up->port.ignore_status_mask =3D 0;
+ if (termios->c_iflag & IGNPAR)
+ up->port.ignore_status_mask |=3D UART_LSR_PE | UART_LSR_FE;
+ if (termios->c_iflag & IGNBRK) {
+ up->port.ignore_status_mask |=3D 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 |=3D UART_LSR_OE;
+ }
+
+ /*
+ * ignore all characters if CREAD is not set
+ */
+ if ((termios->c_cflag & CREAD) =3D=3D 0)
+ up->port.ignore_status_mask |=3D UART_LSR_DR;
+
+ /*
+ * Modem status interrupts
+ */
+ up->ier &=3D ~UART_IER_MSI;
+ if (UART_ENABLE_MS(&up->port, termios->c_cflag))
+ up->ier |=3D UART_IER_MSI;
+
+ up->lcr =3D cval;
+ /* Up to here it was mostly serial8250_do_set_termios() */
+
+ /*
+ * We enable TRIG_GRANU for RX and TX and additionaly we set
+ * SCR_TX_EMPTY bit. The result is the following:
+ * - RX_TRIGGER amount of bytes in the FIFO will cause an interrupt.
+ * - less than RX_TRIGGER number of bytes will also cause an interrup=
t
+ * once the UART decides that there no new bytes arriving.
+ * - Once THRE is enabled, the interrupt will be fired once the FIFO =
is
+ * empty - the trigger level is ignored here.
+ *
+ * Once DMA is enabled:
+ * - UART will assert the TX DMA line once there is room for TX_TRIGG=
ER
+ * bytes in the TX FIFO. On each assert the DMA engine will move
+ * TX_TRIGGER bytes into the FIFO.
+ * - UART will assert the RX DMA line once there are RX_TRIGGER bytes=
in
+ * the FIFO and move RX_TRIGGER bytes.
+ * This is because treshold and trigger values are the same.
+ */
+ up->fcr =3D UART_FCR_ENABLE_FIFO;
+ up->fcr |=3D TRIGGER_FCR_MASK(TX_TRIGGER) << OMAP_UART_FCR_TX_TRIG;
+ up->fcr |=3D TRIGGER_FCR_MASK(RX_TRIGGER) << OMAP_UART_FCR_RX_TRIG;
+
+ priv->scr =3D OMAP_UART_SCR_RX_TRIG_GRANU1_MASK | OMAP_UART_SCR_TX_EM=
PTY |
+ OMAP_UART_SCR_TX_TRIG_GRANU1_MASK;
+
+ priv->xon =3D termios->c_cc[VSTART];
+ priv->xoff =3D termios->c_cc[VSTOP];
+
+ priv->efr =3D 0;
+ if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW) {
+ /* Enable AUTORTS and AUTOCTS */
+ priv->efr |=3D UART_EFR_CTS | UART_EFR_RTS;
+
+ /* Ensure MCR RTS is asserted */
+ up->mcr |=3D 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 |=3D OMAP_UART_SW_RX;
+
+ /*
+ * IXOFF Flag:
+ * Enable XON/XOFF flow control on output.
+ * Transmit XON1, XOFF1
+ */
+ if (termios->c_iflag & IXOFF)
+ priv->efr |=3D 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 |=3D UART_MCR_XONANY;
+ else
+ up->mcr &=3D ~UART_MCR_XONANY;
+ }
+ omap8250_restore_regs(up);
+
+ 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 =3D USEC_PER_SEC * 64 * 8 / baud;
+ priv->latency =3D 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 =3D
+ container_of(port, struct uart_8250_port, port);
+ struct omap8250_priv *priv =3D 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 !=3D 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 *u=
p,
+ struct omap8250_priv *priv)
+{
+ u32 mvr, scheme;
+ u16 revision, major, minor;
+
+ mvr =3D uart_read(up, UART_OMAP_MVER);
+
+ /* Check revision register scheme */
+ scheme =3D mvr >> OMAP_UART_MVR_SCHEME_SHIFT;
+
+ switch (scheme) {
+ case 0: /* Legacy Scheme: OMAP2/3 */
+ /* MINOR_REV[0:4], MAJOR_REV[4:7] */
+ major =3D (mvr & OMAP_UART_LEGACY_MVR_MAJ_MASK) >>
+ OMAP_UART_LEGACY_MVR_MAJ_SHIFT;
+ minor =3D (mvr & OMAP_UART_LEGACY_MVR_MIN_MASK);
+ break;
+ case 1:
+ /* New Scheme: OMAP4+ */
+ /* MINOR_REV[0:5], MAJOR_REV[8:10] */
+ major =3D (mvr & OMAP_UART_MVR_MAJ_MASK) >>
+ OMAP_UART_MVR_MAJ_SHIFT;
+ minor =3D (mvr & OMAP_UART_MVR_MIN_MASK);
+ break;
+ default:
+ dev_warn(up->port.dev,
+ "Unknown revision, defaulting to highest\n");
+ /* highest possible revision */
+ major =3D 0xff;
+ minor =3D 0xff;
+ }
+ /* normalize revision for the driver */
+ revision =3D UART_BUILD_REVISION(major, minor);
+
+ switch (revision) {
+ case OMAP_UART_REV_46:
+ priv->habit =3D UART_ERRATA_i202_MDR1_ACCESS;
+ break;
+ case OMAP_UART_REV_52:
+ priv->habit =3D UART_ERRATA_i202_MDR1_ACCESS |
+ OMAP_UART_WER_HAS_TX_WAKEUP;
+ break;
+ case OMAP_UART_REV_63:
+ priv->habit =3D UART_ERRATA_i202_MDR1_ACCESS |
+ OMAP_UART_WER_HAS_TX_WAKEUP;
+ break;
+ default:
+ break;
+ }
+}
+
+static void omap8250_uart_qos_work(struct work_struct *work)
+{
+ struct omap8250_priv *priv;
+
+ priv =3D 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 =3D dev_id;
+ int ret;
+
+ ret =3D 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 =3D
+ container_of(port, struct uart_8250_port, port);
+ struct omap8250_priv *priv =3D port->private_data;
+
+ int ret;
+
+ if (priv->wakeirq) {
+ ret =3D request_irq(priv->wakeirq, omap_wake_irq,
+ port->irqflags, "wakeup irq", port);
+ if (ret)
+ return ret;
+ disable_irq(priv->wakeirq);
+ }
+
+ pm_runtime_get_sync(port->dev);
+
+ ret =3D serial8250_do_startup(port);
+ if (ret)
+ goto err;
+
+#ifdef CONFIG_PM_RUNTIME
+ up->capabilities |=3D UART_CAP_RPM;
+#endif
+
+ /* Enable module level wake up */
+ priv->wer =3D OMAP_UART_WER_MOD_WKUP;
+ if (priv->habit & OMAP_UART_WER_HAS_TX_WAKEUP)
+ priv->wer |=3D 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:
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
+ if (priv->wakeirq)
+ free_irq(priv->wakeirq, port);
+ return ret;
+}
+
+static void omap_8250_shutdown(struct uart_port *port)
+{
+ struct uart_8250_port *up =3D
+ container_of(port, struct uart_8250_port, port);
+ struct omap8250_priv *priv =3D port->private_data;
+
+ flush_work(&priv->qos_work);
+
+ 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 =3D
+ container_of(port, struct uart_8250_port, port);
+
+ pm_runtime_get_sync(port->dev);
+
+ spin_lock_irqsave(&port->lock, flags);
+ up->ier &=3D ~(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 =3D
+ container_of(port, struct uart_8250_port, port);
+
+ pm_runtime_get_sync(port->dev);
+
+ spin_lock_irqsave(&port->lock, flags);
+ up->ier |=3D 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 =3D platform_get_resource(pdev, IORESOURCE_MEM,=
0);
+ struct resource *irq =3D platform_get_resource(pdev, IORESOURCE_IRQ, =
0);
+ struct omap8250_priv *priv;
+ struct uart_8250_port up;
+ int ret;
+ void __iomem *membase;
+
+ if (!regs || !irq) {
+ dev_err(&pdev->dev, "missing registers or irq\n");
+ return -EINVAL;
+ }
+
+ priv =3D devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ membase =3D devm_ioremap_nocache(&pdev->dev, regs->start,
+ resource_size(regs));
+ if (!membase)
+ return -ENODEV;
+
+ memset(&up, 0, sizeof(up));
+ up.port.dev =3D &pdev->dev;
+ up.port.mapbase =3D regs->start;
+ up.port.membase =3D membase;
+ up.port.irq =3D irq->start;
+ /*
+ * It claims to be 16C750 compatible however it is a little different=
=2E
+ * It has EFR and has no FCR7_64byte bit. The AFE (which it claims to
+ * have) is enabled via EFR instead of MCR. The type is set here 8250
+ * just to get things going. UNKNOWN does not work for a few reasons =
and
+ * we don't need our own type since we don't use 8250's set_termios()
+ * and our "bugs" are handeld via the bug member.
+ */
+ up.port.type =3D PORT_8250;
+ up.port.iotype =3D UPIO_MEM;
+ up.port.flags =3D UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SOFT_FLOW |
+ UPF_HARD_FLOW;
+ up.port.private_data =3D priv;
+
+ up.port.regshift =3D 2;
+ up.port.fifosize =3D 64;
+ up.tx_loadsz =3D 64;
+ up.capabilities =3D UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP;
+#ifdef CONFIG_PM_RUNTIME
+ /*
+ * PM_RUNTIME is mostly transparent. However to do it right we need t=
o a
+ * TX empty interrupt before we can put the device to auto idle. So i=
f
+ * PM_RUNTIME is not enabled we don't add that flag and can spare tha=
t
+ * one extra interrupt in the TX path.
+ */
+ up.capabilities |=3D UART_CAP_RPM;
+#endif
+ up.port.set_termios =3D omap_8250_set_termios;
+ up.port.pm =3D omap_8250_pm;
+ up.port.startup =3D omap_8250_startup;
+ up.port.shutdown =3D omap_8250_shutdown;
+ up.port.throttle =3D omap_8250_throttle;
+ up.port.unthrottle =3D omap_8250_unthrottle;
+
+ if (pdev->dev.of_node) {
+ up.port.line =3D of_alias_get_id(pdev->dev.of_node, "serial");
+ of_property_read_u32(pdev->dev.of_node, "clock-frequency",
+ &up.port.uartclk);
+ priv->wakeirq =3D irq_of_parse_and_map(pdev->dev.of_node, 1);
+ } else {
+ up.port.line =3D 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 =3D DEFAULT_CLK_SPEED;
+ dev_warn(&pdev->dev,
+ "No clock speed specified: using default: %d\n",
+ DEFAULT_CLK_SPEED);
+ }
+
+ priv->latency =3D PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
+ priv->calc_latency =3D 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 =3D serial8250_register_8250_port(&up);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "unable to register 8250 port\n");
+ goto err;
+ }
+ priv->line =3D 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 =3D 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 =3D=3D priv->wakeups_enabled)
+ return;
+
+ omap8250_enable_wakeirq(priv, enable);
+ priv->wakeups_enabled =3D enable;
+}
+#endif
+
+#ifdef CONFIG_PM_SLEEP
+static int omap8250_prepare(struct device *dev)
+{
+ struct omap8250_priv *priv =3D dev_get_drvdata(dev);
+
+ if (!priv)
+ return 0;
+ priv->is_suspending =3D true;
+ return 0;
+}
+
+static void omap8250_complete(struct device *dev)
+{
+ struct omap8250_priv *priv =3D dev_get_drvdata(dev);
+
+ if (!priv)
+ return;
+ priv->is_suspending =3D false;
+}
+
+static int omap8250_suspend(struct device *dev)
+{
+ struct omap8250_priv *priv =3D 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 =3D 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 uart_8250_port *up)
+{
+ u32 val;
+
+ val =3D 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 13=
x
+ * or 16x but never to disable again.
+ */
+ if (val =3D=3D UART_OMAP_MDR1_DISABLE)
+ return 1;
+ return 0;
+}
+
+static int omap8250_runtime_suspend(struct device *dev)
+{
+ struct omap8250_priv *priv =3D dev_get_drvdata(dev);
+ struct uart_8250_port *up;
+
+ up =3D serial8250_get_port(priv->line);
+ /*
+ * 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) {
+ if (uart_console(&up->port))
+ return -EBUSY;
+ }
+
+ omap8250_enable_wakeup(priv, true);
+
+ priv->latency =3D PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
+ schedule_work(&priv->qos_work);
+
+ return 0;
+}
+
+static int omap8250_runtime_resume(struct device *dev)
+{
+ struct omap8250_priv *priv =3D dev_get_drvdata(dev);
+ struct uart_8250_port *up;
+ int loss_cntx;
+
+ /* In case runtime-pm tries this before we are setup */
+ if (!priv)
+ return 0;
+
+ up =3D serial8250_get_port(priv->line);
+ omap8250_enable_wakeup(priv, false);
+ loss_cntx =3D omap8250_lost_context(up);
+
+ if (loss_cntx)
+ omap8250_restore_regs(up);
+
+ priv->latency =3D priv->calc_latency;
+ schedule_work(&priv->qos_work);
+ return 0;
+}
+#endif
+
+static const struct dev_pm_ops omap8250_dev_pm_ops =3D {
+ SET_SYSTEM_SLEEP_PM_OPS(omap8250_suspend, omap8250_resume)
+ SET_RUNTIME_PM_OPS(omap8250_runtime_suspend,
+ omap8250_runtime_resume, NULL)
+ .prepare =3D omap8250_prepare,
+ .complete =3D omap8250_complete,
+};
+
+static const struct of_device_id omap8250_dt_ids[] =3D {
+ { .compatible =3D "ti,omap2-uart" },
+ { .compatible =3D "ti,omap3-uart" },
+ { .compatible =3D "ti,omap4-uart" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, omap8250_dt_ids);
+
+static struct platform_driver omap8250_platform_driver =3D {
+ .driver =3D {
+ .name =3D "omap8250",
+ .pm =3D &omap8250_dev_pm_ops,
+ .of_match_table =3D omap8250_dt_ids,
+ .owner =3D THIS_MODULE,
+ },
+ .probe =3D omap8250_probe,
+ .remove =3D omap8250_remove,
+};
+module_platform_driver(omap8250_platform_driver);
+
+MODULE_AUTHOR("Sebastian Andrzej Siewior");
+MODULE_DESCRIPTION("OMAP 8250 Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/=
Kconfig
index 21eca79224e4..bb1b7119ecf9 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -299,6 +299,15 @@ config SERIAL_8250_RT288X
serial port, say Y to this option. The driver can handle up to 2 se=
rial
ports. If unsure, say N.
=20
+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.
+
config SERIAL_8250_FINTEK
tristate "Support for Fintek F81216A LPC to 4 UART"
depends on SERIAL_8250 && PNP
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250=
/Makefile
index 5256b894e46a..31e7cdc6865c 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -20,5 +20,6 @@ obj-$(CONFIG_SERIAL_8250_HUB6) +=3D 8250_hub6.o
obj-$(CONFIG_SERIAL_8250_FSL) +=3D 8250_fsl.o
obj-$(CONFIG_SERIAL_8250_DW) +=3D 8250_dw.o
obj-$(CONFIG_SERIAL_8250_EM) +=3D 8250_em.o
+obj-$(CONFIG_SERIAL_8250_OMAP) +=3D 8250_omap.o
obj-$(CONFIG_SERIAL_8250_FINTEK) +=3D 8250_fintek.o
obj-$(CONFIG_SERIAL_8250_MT6577) +=3D 8250_mtk.o
--=20
2.1.0
Peter Hurley
2014-09-11 11:57:15 UTC
Permalink
Hi Sebastian,

Nice work. Minor comments within.
This patch provides a 8250-core based UART driver for the internal OM=
AP
UART. The long term goal is to provide the same functionality as the
current OMAP uart driver and DMA support.
I tried to merge omap-serial code together with the 8250-core code.
There should should be hardly a noticable difference. The trigger lev=
els
- omap serial
TX: Interrupt comes after TX FIFO has room for 16 bytes.
TX of 4096 bytes in one go results in 256 interrupts
=20
RX: Interrupt comes after there is on byte in the FIFO.
RX of 4096 bytes results in 4096 interrupts.
=20
- this driver
TX: Interrupt comes once the TX FIFO is empty.
TX of 4096 bytes results in 65 interrupts. That means there wil=
l
be gaps on the line while the driver reloads the FIFO.
=20
RX: Interrupt comes once there are 48 bytes in the FIFO or less ove=
r
"longer" time frame. We have
1 / 11520 * 10^3 * 16 =3D> 1.38=E2=80=A6 ms
1.38ms to react and purge the FIFO on 115200,8N1. Since the oth=
er
driver fired after each byte it had ~5.47ms time to react. This
_may_ cause problems if one relies on no missing bytes and has =
no
flow control. On the other hand we get only 85 interrupts for t=
he
same amount of data.
After this is merged, it may be worth investigating how to use Yoshihir=
o's
newly-added 8250-based tunable RX trigger interface for omap.
It has been only tested as console UART on am335x-evm, dra7-evm and
beagle bone. I also did some longer raw-transfers to meassure the loa=
d.
=20
The device name is ttyS based instead of ttyO. If a ttyO based node n=
ame
is required please ask udev for it. If both driver are activated (thi=
s
and omap-serial) then this serial driver will take control over the
device due to the link order
=20
- less on a file seems to hang the am335x after a while. I
believe I introduce this bug a while ago since I can reproduce
this prior to v8. Fixed by redoing the omap8250_restore_regs()
- redo the register write. There is now one function for that
which is used from set_termios() and runtime-resume.
- drop PORT_OMAP_16750 and move the setup to the omap file. We
have our own set termios function anyway (Heikki Krogerus)
- use MEM instead of MEM32. TRM of AM/DM37x says that 32bit
access on THR might result in data abort. We only need 32bit
access in the errata function which is before we use 8250's
read function so it doesn't matter.
- change trigger levels after some tests with raw transfers.
- drop RS485 support
- wire up ->throttle / ->unthrottle
- wire up startup & shutdown for wakeup-irq handling.
- RS485 handling (well the core does).
=20
- added runtime PM. Could somebody could please double check
this?
- added omap_8250_set_termios()
=20
---
drivers/tty/serial/8250/8250_omap.c | 911 ++++++++++++++++++++++++++=
++++++++++
drivers/tty/serial/8250/Kconfig | 9 +
drivers/tty/serial/8250/Makefile | 1 +
3 files changed, 921 insertions(+)
create mode 100644 drivers/tty/serial/8250/8250_omap.c
=20
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial=
/8250/8250_omap.c
new file mode 100644
index 000000000000..2a187b00ed0a
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -0,0 +1,911 @@
+/*
+ * 8250-core based driver for the OMAP internal UART
+ *
+ * Copyright (C) 2014 Sebastian Andrzej Siewior
+ * based on omap-serial.c, Copyright (C) 2010 Texas Instruments.

or something like that, since this is (partly) based on omap-serial.c
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/serial_8250.h>
+#include <linux/serial_core.h>
+#include <linux/serial_reg.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/of_irq.h>
+#include <linux/delay.h>
+#include <linux/pm_runtime.h>
+#include <linux/console.h>
+#include <linux/pm_qos.h>
+
+#include "8250.h"
+
+#define DEFAULT_CLK_SPEED 48000000
+
+#define UART_ERRATA_i202_MDR1_ACCESS (1 << 0)
+#define OMAP_UART_WER_HAS_TX_WAKEUP (1 << 1)
+
+#define OMAP_UART_FCR_RX_TRIG 6
+#define OMAP_UART_FCR_TX_TRIG 4
+
+/* SCR register bitmasks */
+#define OMAP_UART_SCR_RX_TRIG_GRANU1_MASK (1 << 7)
+#define OMAP_UART_SCR_TX_TRIG_GRANU1_MASK (1 << 6)
+#define OMAP_UART_SCR_TX_EMPTY (1 << 3)
+#define OMAP_UART_SCR_DMAMODE_MASK (3 << 1)
+#define OMAP_UART_SCR_DMAMODE_1 (1 << 1)
+#define OMAP_UART_SCR_DMAMODE_CTL (1 << 0)
+
+/* MVR register bitmasks */
+#define OMAP_UART_MVR_SCHEME_SHIFT 30
+#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
+
+#define UART_TI752_TLR_TX 0
+#define UART_TI752_TLR_RX 4
+
+#define TRIGGER_TLR_MASK(x) ((x & 0x3c) >> 2)
+#define TRIGGER_FCR_MASK(x) (x & 3)
+
+/* Enable XON/XOFF flow control on output */
+#define OMAP_UART_SW_TX 0x08
+/* Enable XON/XOFF flow control on input */
+#define OMAP_UART_SW_RX 0x02
+
+#define OMAP_UART_WER_MOD_WKUP 0x7f
+#define OMAP_UART_TX_WAKEUP_EN (1 << 7)
+
+#define TX_TRIGGER 1
+#define RX_TRIGGER 48
+
+#define OMAP_UART_TCR_RESTORE(x) ((x / 4) << 4)
+#define OMAP_UART_TCR_HALT(x) ((x / 4) << 0)
+
+#define UART_BUILD_REVISION(x, y) (((x) << 8) | (y))
+
+#define OMAP_UART_REV_46 0x0406
+#define OMAP_UART_REV_52 0x0502
+#define OMAP_UART_REV_63 0x0603
+
+struct omap8250_priv {
+ int line;
+ u32 habit;
+ u32 mdr1;
+ u32 efr;
+ u32 quot;
+ u32 scr;
+ u32 wer;
+ u32 xon;
+ u32 xoff;
+
+ 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;
+ struct uart_8250_dma omap8250_dma;
+ bool dma_active;
+};
+
+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 =3D
2uS)
+ * give 10 times as much
+ */
+static void omap_8250_mdr1_errataset(struct uart_8250_port *up, u8 m=
dr1)
+{
+ u8 timeout =3D 255;
+
+ serial_out(up, UART_OMAP_MDR1, mdr1);
+ udelay(2);
+ serial_out(up, UART_FCR, up->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 !=3D (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 i=
nt baud,
+ struct omap8250_priv *priv)
+{
+ unsigned int uartclk =3D port->uartclk;
+ unsigned int div_13, div_16;
+ unsigned int abs_d13, abs_d16;
+
+ /*
+ * Old custom speed handling.
+ */
+ if (baud =3D=3D 38400 && (port->flags & UPF_SPD_MASK) =3D=3D UPF_SP=
D_CUST) {
+ priv->quot =3D 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 =3D UART_OMAP_MDR1_13X_MODE;
+ else
+ priv->mdr1 =3D UART_OMAP_MDR1_16X_MODE;
+ return;
+ }
+ div_13 =3D DIV_ROUND_CLOSEST(uartclk, 13 * baud);
+ div_16 =3D DIV_ROUND_CLOSEST(uartclk, 16 * baud);
+
+ abs_d13 =3D abs(baud - port->uartclk / 13 / div_13);
+ abs_d16 =3D abs(baud - port->uartclk / 16 / div_16);
+
+ if (abs_d13 >=3D abs_d16) {
+ priv->mdr1 =3D UART_OMAP_MDR1_16X_MODE;
+ priv->quot =3D div_16;
+ } else {
+ priv->mdr1 =3D UART_OMAP_MDR1_13X_MODE;
+ priv->quot =3D div_13;
+ }
+}
+
+static void omap8250_update_scr(struct uart_8250_port *up,
+ struct omap8250_priv *priv)
+{
+ /*
+ * The manual recommends not to enable the DMA mode selector in the=
SCR
+ * (instead of the FCR) register _and_ selecting the DMA mode as on=
e
+ * register write because this may lead to malfunction.
+ */
+ if (priv->scr & OMAP_UART_SCR_DMAMODE_MASK)
+ serial_out(up, UART_OMAP_SCR,
+ priv->scr & ~OMAP_UART_SCR_DMAMODE_MASK);
+ serial_out(up, UART_OMAP_SCR, priv->scr);
+}
+
+static void omap8250_restore_regs(struct uart_8250_port *up)
+{
+ struct omap8250_priv *priv =3D up->port.private_data;
+
+ 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, 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, UART_MCR_TCRTLR);
+ serial_out(up, UART_FCR, up->fcr);
+
+ omap8250_update_scr(up, priv);
+
+ /* Protocol, Baud Rate, and Interrupt Settings */
+ /* need mode A for FCR */
+ 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_TI752_TCR, OMAP_UART_TCR_RESTORE(16) |
+ OMAP_UART_TCR_HALT(52));
+ serial_out(up, UART_TI752_TLR,
+ TRIGGER_TLR_MASK(TX_TRIGGER) << UART_TI752_TLR_TX |
+ TRIGGER_TLR_MASK(RX_TRIGGER) << UART_TI752_TLR_RX);
+
+ serial_out(up, UART_LCR, 0);
+
+ /* drop TCR + TLR access, we setup XON/XOFF later */
+ serial_out(up, UART_MCR, up->mcr);
+ 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_EFR, priv->efr);
+
+ serial_out(up, UART_LCR, up->lcr);
+ /* need mode A for FCR */
+ 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);
+ serial_out(up, UART_XON1, priv->xon);
+ serial_out(up, UART_XOFF1, priv->xoff);
+
+ serial_out(up, UART_LCR, up->lcr);
+ up->port.ops->set_mctrl(&up->port, up->port.mctrl);
+}
+/*
+ * 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 =3D
+ container_of(port, struct uart_8250_port, port);
+ struct omap8250_priv *priv =3D up->port.private_data;
+ unsigned char cval =3D 0;
+ unsigned long flags =3D 0;
+ unsigned int baud;
+
+ switch (termios->c_cflag & CSIZE) {
+ cval =3D UART_LCR_WLEN5;
+ break;
+ cval =3D UART_LCR_WLEN6;
+ break;
+ cval =3D UART_LCR_WLEN7;
+ break;
+ cval =3D UART_LCR_WLEN8;
+ break;
+ }
+
+ if (termios->c_cflag & CSTOPB)
+ cval |=3D UART_LCR_STOP;
+ if (termios->c_cflag & PARENB)
+ cval |=3D UART_LCR_PARITY;
+ if (!(termios->c_cflag & PARODD))
+ cval |=3D UART_LCR_EPAR;
+ if (termios->c_cflag & CMSPAR)
+ cval |=3D UART_LCR_SPAR;
+
+ /*
+ * Ask the core to calculate the divisor for us.
+ */
+ baud =3D 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);
^^^
spin_lock_irq(&port->lock);

The serial core calls the ->set_termios() method with interrupts enable=
d.
+
+ /*
+ * Update the per-port timeout.
+ */
+ uart_update_timeout(port, termios->c_cflag, baud);
+
+ up->port.read_status_mask =3D UART_LSR_OE | UART_LSR_THRE | UART_LS=
R_DR;
+ if (termios->c_iflag & INPCK)
+ up->port.read_status_mask |=3D UART_LSR_FE | UART_LSR_PE;
+ if (termios->c_iflag & (BRKINT | PARMRK))
^
IGNBRK |

Otherwise, the read_status_mask will mask out the BI condition, so
uart_insert_char() will send '\0' byte as TTY_NORMAL.

The 8250 and omap RX path differed so the omap driver didn't need this
change, whereas the 8250 driver does.
+ up->port.read_status_mask |=3D UART_LSR_BI;
+
+ /*
+ * Characters to ignore
+ */
+ up->port.ignore_status_mask =3D 0;
+ if (termios->c_iflag & IGNPAR)
+ up->port.ignore_status_mask |=3D UART_LSR_PE | UART_LSR_FE;
+ if (termios->c_iflag & IGNBRK) {
+ up->port.ignore_status_mask |=3D 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 |=3D UART_LSR_OE;
+ }
+
+ /*
+ * ignore all characters if CREAD is not set
+ */
+ if ((termios->c_cflag & CREAD) =3D=3D 0)
+ up->port.ignore_status_mask |=3D UART_LSR_DR;
+
+ /*
+ * Modem status interrupts
+ */
+ up->ier &=3D ~UART_IER_MSI;
+ if (UART_ENABLE_MS(&up->port, termios->c_cflag))
+ up->ier |=3D UART_IER_MSI;
+
+ up->lcr =3D cval;
+ /* Up to here it was mostly serial8250_do_set_termios() */
+
+ /*
+ * We enable TRIG_GRANU for RX and TX and additionaly we set
+ * - RX_TRIGGER amount of bytes in the FIFO will cause an interrupt=
=2E
+ * - less than RX_TRIGGER number of bytes will also cause an interr=
upt
+ * once the UART decides that there no new bytes arriving.
+ * - Once THRE is enabled, the interrupt will be fired once the FIF=
O is
+ * empty - the trigger level is ignored here.
+ *
+ * - UART will assert the TX DMA line once there is room for TX_TRI=
GGER
+ * bytes in the TX FIFO. On each assert the DMA engine will move
+ * TX_TRIGGER bytes into the FIFO.
+ * - UART will assert the RX DMA line once there are RX_TRIGGER byt=
es in
+ * the FIFO and move RX_TRIGGER bytes.
+ * This is because treshold and trigger values are the same.
+ */
+ up->fcr =3D UART_FCR_ENABLE_FIFO;
+ up->fcr |=3D TRIGGER_FCR_MASK(TX_TRIGGER) << OMAP_UART_FCR_TX_TRIG;
+ up->fcr |=3D TRIGGER_FCR_MASK(RX_TRIGGER) << OMAP_UART_FCR_RX_TRIG;
+
+ priv->scr =3D OMAP_UART_SCR_RX_TRIG_GRANU1_MASK | OMAP_UART_SCR_TX_=
EMPTY |
+ OMAP_UART_SCR_TX_TRIG_GRANU1_MASK;
+
+ priv->xon =3D termios->c_cc[VSTART];
+ priv->xoff =3D termios->c_cc[VSTOP];
+
+ priv->efr =3D 0;
+ if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW) {
+ /* Enable AUTORTS and AUTOCTS */
+ priv->efr |=3D UART_EFR_CTS | UART_EFR_RTS;
+
+ /* Ensure MCR RTS is asserted */
+ up->mcr |=3D UART_MCR_RTS;
+ }
+
+ if (up->port.flags & UPF_SOFT_FLOW) {
I'm aware that this is basically from the omap driver but can someone c=
lear
up if omap hardware can actually do UPF_HARD_FLOW and UPF_SOFT_FLOW
simultaneously? The datasheets that I've looked at say no.

Regards,
Peter Hurley
+ /*
+ * Enable XON/XOFF flow control on input.
+ * Receiver compares XON1, XOFF1.
+ */
+ if (termios->c_iflag & IXON)
+ priv->efr |=3D OMAP_UART_SW_RX;
+
+ /*
+ * Enable XON/XOFF flow control on output.
+ * Transmit XON1, XOFF1
+ */
+ if (termios->c_iflag & IXOFF)
+ priv->efr |=3D OMAP_UART_SW_TX;
+
+ /*
+ * 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 |=3D UART_MCR_XONANY;
+ else
+ up->mcr &=3D ~UART_MCR_XONANY;
+ }
+ omap8250_restore_regs(up);
+
+ 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 =3D USEC_PER_SEC * 64 * 8 / baud;
+ priv->latency =3D 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 =3D
+ container_of(port, struct uart_8250_port, port);
+ struct omap8250_priv *priv =3D 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 !=3D 0) ? UART_IERX_SLEEP : 0);
+ serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+ serial_out(up, UART_EFR, priv->efr);
+ serial_out(up, UART_LCR, 0);
+
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
+}
+
+static void omap_serial_fill_features_erratas(struct uart_8250_port =
*up,
+ struct omap8250_priv *priv)
+{
+ u32 mvr, scheme;
+ u16 revision, major, minor;
+
+ mvr =3D uart_read(up, UART_OMAP_MVER);
+
+ /* Check revision register scheme */
+ scheme =3D mvr >> OMAP_UART_MVR_SCHEME_SHIFT;
+
+ switch (scheme) {
+ case 0: /* Legacy Scheme: OMAP2/3 */
+ /* MINOR_REV[0:4], MAJOR_REV[4:7] */
+ major =3D (mvr & OMAP_UART_LEGACY_MVR_MAJ_MASK) >>
+ OMAP_UART_LEGACY_MVR_MAJ_SHIFT;
+ minor =3D (mvr & OMAP_UART_LEGACY_MVR_MIN_MASK);
+ break;
+ /* New Scheme: OMAP4+ */
+ /* MINOR_REV[0:5], MAJOR_REV[8:10] */
+ major =3D (mvr & OMAP_UART_MVR_MAJ_MASK) >>
+ OMAP_UART_MVR_MAJ_SHIFT;
+ minor =3D (mvr & OMAP_UART_MVR_MIN_MASK);
+ break;
+ dev_warn(up->port.dev,
+ "Unknown revision, defaulting to highest\n");
+ /* highest possible revision */
+ major =3D 0xff;
+ minor =3D 0xff;
+ }
+ /* normalize revision for the driver */
+ revision =3D UART_BUILD_REVISION(major, minor);
+
+ switch (revision) {
+ priv->habit =3D UART_ERRATA_i202_MDR1_ACCESS;
+ break;
+ priv->habit =3D UART_ERRATA_i202_MDR1_ACCESS |
+ OMAP_UART_WER_HAS_TX_WAKEUP;
+ break;
+ priv->habit =3D UART_ERRATA_i202_MDR1_ACCESS |
+ OMAP_UART_WER_HAS_TX_WAKEUP;
+ break;
+ break;
+ }
+}
+
+static void omap8250_uart_qos_work(struct work_struct *work)
+{
+ struct omap8250_priv *priv;
+
+ priv =3D 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 =3D dev_id;
+ int ret;
+
+ ret =3D 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 =3D
+ container_of(port, struct uart_8250_port, port);
+ struct omap8250_priv *priv =3D port->private_data;
+
+ int ret;
+
+ if (priv->wakeirq) {
+ ret =3D request_irq(priv->wakeirq, omap_wake_irq,
+ port->irqflags, "wakeup irq", port);
+ if (ret)
+ return ret;
+ disable_irq(priv->wakeirq);
+ }
+
+ pm_runtime_get_sync(port->dev);
+
+ ret =3D serial8250_do_startup(port);
+ if (ret)
+ goto err;
+
+#ifdef CONFIG_PM_RUNTIME
+ up->capabilities |=3D UART_CAP_RPM;
+#endif
+
+ /* Enable module level wake up */
+ priv->wer =3D OMAP_UART_WER_MOD_WKUP;
+ if (priv->habit & OMAP_UART_WER_HAS_TX_WAKEUP)
+ priv->wer |=3D 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;
+ pm_runtime_mark_last_busy(port->dev);
+ pm_runtime_put_autosuspend(port->dev);
+ if (priv->wakeirq)
+ free_irq(priv->wakeirq, port);
+ return ret;
+}
+
+static void omap_8250_shutdown(struct uart_port *port)
+{
+ struct uart_8250_port *up =3D
+ container_of(port, struct uart_8250_port, port);
+ struct omap8250_priv *priv =3D port->private_data;
+
+ flush_work(&priv->qos_work);
+
+ 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 =3D
+ container_of(port, struct uart_8250_port, port);
+
+ pm_runtime_get_sync(port->dev);
+
+ spin_lock_irqsave(&port->lock, flags);
+ up->ier &=3D ~(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 =3D
+ container_of(port, struct uart_8250_port, port);
+
+ pm_runtime_get_sync(port->dev);
+
+ spin_lock_irqsave(&port->lock, flags);
+ up->ier |=3D 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 =3D platform_get_resource(pdev, IORESOURCE_ME=
M, 0);
+ struct resource *irq =3D platform_get_resource(pdev, IORESOURCE_IRQ=
, 0);
+ struct omap8250_priv *priv;
+ struct uart_8250_port up;
+ int ret;
+ void __iomem *membase;
+
+ if (!regs || !irq) {
+ dev_err(&pdev->dev, "missing registers or irq\n");
+ return -EINVAL;
+ }
+
+ priv =3D devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ membase =3D devm_ioremap_nocache(&pdev->dev, regs->start,
+ resource_size(regs));
+ if (!membase)
+ return -ENODEV;
+
+ memset(&up, 0, sizeof(up));
+ up.port.dev =3D &pdev->dev;
+ up.port.mapbase =3D regs->start;
+ up.port.membase =3D membase;
+ up.port.irq =3D irq->start;
+ /*
+ * It claims to be 16C750 compatible however it is a little differe=
nt.
+ * It has EFR and has no FCR7_64byte bit. The AFE (which it claims =
to
+ * have) is enabled via EFR instead of MCR. The type is set here 82=
50
+ * just to get things going. UNKNOWN does not work for a few reason=
s and
+ * we don't need our own type since we don't use 8250's set_termios=
()
+ * and our "bugs" are handeld via the bug member.
+ */
+ up.port.type =3D PORT_8250;
+ up.port.iotype =3D UPIO_MEM;
+ up.port.flags =3D UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_SOFT_FLOW |
+ UPF_HARD_FLOW;
+ up.port.private_data =3D priv;
+
+ up.port.regshift =3D 2;
+ up.port.fifosize =3D 64;
+ up.tx_loadsz =3D 64;
+ up.capabilities =3D UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP;
+#ifdef CONFIG_PM_RUNTIME
+ /*
+ * PM_RUNTIME is mostly transparent. However to do it right we need=
to a
+ * TX empty interrupt before we can put the device to auto idle. So=
if
+ * PM_RUNTIME is not enabled we don't add that flag and can spare t=
hat
+ * one extra interrupt in the TX path.
+ */
+ up.capabilities |=3D UART_CAP_RPM;
+#endif
+ up.port.set_termios =3D omap_8250_set_termios;
+ up.port.pm =3D omap_8250_pm;
+ up.port.startup =3D omap_8250_startup;
+ up.port.shutdown =3D omap_8250_shutdown;
+ up.port.throttle =3D omap_8250_throttle;
+ up.port.unthrottle =3D omap_8250_unthrottle;
+
+ if (pdev->dev.of_node) {
+ up.port.line =3D of_alias_get_id(pdev->dev.of_node, "serial");
+ of_property_read_u32(pdev->dev.of_node, "clock-frequency",
+ &up.port.uartclk);
+ priv->wakeirq =3D irq_of_parse_and_map(pdev->dev.of_node, 1);
+ } else {
+ up.port.line =3D 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 =3D DEFAULT_CLK_SPEED;
+ dev_warn(&pdev->dev,
+ "No clock speed specified: using default: %d\n",
+ DEFAULT_CLK_SPEED);
+ }
+
+ priv->latency =3D PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
+ priv->calc_latency =3D 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 =3D serial8250_register_8250_port(&up);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "unable to register 8250 port\n");
+ goto err;
+ }
+ priv->line =3D ret;
+ platform_set_drvdata(pdev, priv);
+ pm_runtime_mark_last_busy(&pdev->dev);
+ pm_runtime_put_autosuspend(&pdev->dev);
+ return 0;
+ pm_runtime_put(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
+ return ret;
+}
+
+static int omap8250_remove(struct platform_device *pdev)
+{
+ struct omap8250_priv *priv =3D 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 *pri=
v,
+ 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 =3D=3D priv->wakeups_enabled)
+ return;
+
+ omap8250_enable_wakeirq(priv, enable);
+ priv->wakeups_enabled =3D enable;
+}
+#endif
+
+#ifdef CONFIG_PM_SLEEP
+static int omap8250_prepare(struct device *dev)
+{
+ struct omap8250_priv *priv =3D dev_get_drvdata(dev);
+
+ if (!priv)
+ return 0;
+ priv->is_suspending =3D true;
+ return 0;
+}
+
+static void omap8250_complete(struct device *dev)
+{
+ struct omap8250_priv *priv =3D dev_get_drvdata(dev);
+
+ if (!priv)
+ return;
+ priv->is_suspending =3D false;
+}
+
+static int omap8250_suspend(struct device *dev)
+{
+ struct omap8250_priv *priv =3D 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 =3D 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 uart_8250_port *up)
+{
+ u32 val;
+
+ val =3D 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 =3D=3D UART_OMAP_MDR1_DISABLE)
+ return 1;
+ return 0;
+}
+
+static int omap8250_runtime_suspend(struct device *dev)
+{
+ struct omap8250_priv *priv =3D dev_get_drvdata(dev);
+ struct uart_8250_port *up;
+
+ up =3D serial8250_get_port(priv->line);
+ /*
+ * 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) {
+ if (uart_console(&up->port))
+ return -EBUSY;
+ }
+
+ omap8250_enable_wakeup(priv, true);
+
+ priv->latency =3D PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
+ schedule_work(&priv->qos_work);
+
+ return 0;
+}
+
+static int omap8250_runtime_resume(struct device *dev)
+{
+ struct omap8250_priv *priv =3D dev_get_drvdata(dev);
+ struct uart_8250_port *up;
+ int loss_cntx;
+
+ /* In case runtime-pm tries this before we are setup */
+ if (!priv)
+ return 0;
+
+ up =3D serial8250_get_port(priv->line);
+ omap8250_enable_wakeup(priv, false);
+ loss_cntx =3D omap8250_lost_context(up);
+
+ if (loss_cntx)
+ omap8250_restore_regs(up);
+
+ priv->latency =3D priv->calc_latency;
+ schedule_work(&priv->qos_work);
+ return 0;
+}
+#endif
+
+static const struct dev_pm_ops omap8250_dev_pm_ops =3D {
+ SET_SYSTEM_SLEEP_PM_OPS(omap8250_suspend, omap8250_resume)
+ SET_RUNTIME_PM_OPS(omap8250_runtime_suspend,
+ omap8250_runtime_resume, NULL)
+ .prepare =3D omap8250_prepare,
+ .complete =3D omap8250_complete,
+};
+
+static const struct of_device_id omap8250_dt_ids[] =3D {
+ { .compatible =3D "ti,omap2-uart" },
+ { .compatible =3D "ti,omap3-uart" },
+ { .compatible =3D "ti,omap4-uart" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, omap8250_dt_ids);
+
+static struct platform_driver omap8250_platform_driver =3D {
+ .driver =3D {
+ .name =3D "omap8250",
+ .pm =3D &omap8250_dev_pm_ops,
+ .of_match_table =3D omap8250_dt_ids,
+ .owner =3D THIS_MODULE,
+ },
+ .probe =3D omap8250_probe,
+ .remove =3D omap8250_remove,
+};
+module_platform_driver(omap8250_platform_driver);
+
+MODULE_AUTHOR("Sebastian Andrzej Siewior");
+MODULE_DESCRIPTION("OMAP 8250 Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/825=
0/Kconfig
index 21eca79224e4..bb1b7119ecf9 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -299,6 +299,15 @@ config SERIAL_8250_RT288X
serial port, say Y to this option. The driver can handle up to 2 =
serial
ports. If unsure, say N.
=20
+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.
+
config SERIAL_8250_FINTEK
tristate "Support for Fintek F81216A LPC to 4 UART"
depends on SERIAL_8250 && PNP
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/82=
50/Makefile
index 5256b894e46a..31e7cdc6865c 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -20,5 +20,6 @@ obj-$(CONFIG_SERIAL_8250_HUB6) +=3D 8250_hub6.o
obj-$(CONFIG_SERIAL_8250_FSL) +=3D 8250_fsl.o
obj-$(CONFIG_SERIAL_8250_DW) +=3D 8250_dw.o
obj-$(CONFIG_SERIAL_8250_EM) +=3D 8250_em.o
+obj-$(CONFIG_SERIAL_8250_OMAP) +=3D 8250_omap.o
obj-$(CONFIG_SERIAL_8250_FINTEK) +=3D 8250_fintek.o
obj-$(CONFIG_SERIAL_8250_MT6577) +=3D 8250_mtk.o
=20
--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-16 17:01:29 UTC
Permalink
Post by Peter Hurley
Hi Sebastian,
Hi Peter,
Post by Peter Hurley
Nice work. Minor comments within.
Thanks.
Post by Peter Hurley
After this is merged, it may be worth investigating how to use Yoshih=
iro's
Post by Peter Hurley
newly-added 8250-based tunable RX trigger interface for omap.
We need to overwrite the FCR callback. First because we can support
trigger levels 1=8564 and it it split across two registers and second
because a change here results also in different DMA attributes.
Post by Peter Hurley
Post by Sebastian Andrzej Siewior
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -0,0 +1,911 @@
+/*
+ * 8250-core based driver for the OMAP internal UART
+ *
+ * Copyright (C) 2014 Sebastian Andrzej Siewior
=20
+ * based on omap-serial.c, Copyright (C) 2010 Texas Instruments.
=20
or something like that, since this is (partly) based on omap-serial.c
of course.
Post by Peter Hurley
Post by Sebastian Andrzej Siewior
+ *
+ */
+
=85
Post by Peter Hurley
Post by Sebastian Andrzej Siewior
+ /*
+ * Ask the core to calculate the divisor for us.
+ */
+ baud =3D 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);
^^^
spin_lock_irq(&port->lock);
=20
The serial core calls the ->set_termios() method with interrupts enab=
led.

Okay, this could work.
Post by Peter Hurley
Post by Sebastian Andrzej Siewior
+
+ /*
+ * Update the per-port timeout.
+ */
+ uart_update_timeout(port, termios->c_cflag, baud);
+
+ up->port.read_status_mask =3D UART_LSR_OE | UART_LSR_THRE | UART_L=
SR_DR;
Post by Peter Hurley
Post by Sebastian Andrzej Siewior
+ if (termios->c_iflag & INPCK)
+ up->port.read_status_mask |=3D UART_LSR_FE | UART_LSR_PE;
+ if (termios->c_iflag & (BRKINT | PARMRK))
^
IGNBRK |
=20
Otherwise, the read_status_mask will mask out the BI condition, so
uart_insert_char() will send '\0' byte as TTY_NORMAL.
=20
The 8250 and omap RX path differed so the omap driver didn't need thi=
s
Post by Peter Hurley
change, whereas the 8250 driver does.
Updated.
Post by Peter Hurley
Post by Sebastian Andrzej Siewior
+ up->port.read_status_mask |=3D UART_LSR_BI;
+
+ /*
=85
Post by Peter Hurley
Post by Sebastian Andrzej Siewior
+
+ priv->efr =3D 0;
+ if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW) =
{
Post by Peter Hurley
Post by Sebastian Andrzej Siewior
+ /* Enable AUTORTS and AUTOCTS */
+ priv->efr |=3D UART_EFR_CTS | UART_EFR_RTS;
+
+ /* Ensure MCR RTS is asserted */
+ up->mcr |=3D UART_MCR_RTS;
+ }
+
+ if (up->port.flags & UPF_SOFT_FLOW) {
=20
I'm aware that this is basically from the omap driver but can someone=
clear
Post by Peter Hurley
up if omap hardware can actually do UPF_HARD_FLOW and UPF_SOFT_FLOW
simultaneously? The datasheets that I've looked at say no.
The one that I have here for am335x says:
"The UART module can use hardware or software flow control to manage
transmission and reception".
So yes, you are right about this. I changed this to do UPF_HARD_FLOW if
possible + else UPF_SOFT_FLOW.
Post by Peter Hurley
Regards,
Peter Hurley
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Frans Klaver
2014-09-29 09:38:23 UTC
Permalink
Post by Sebastian Andrzej Siewior
+ /*
+ * We enable TRIG_GRANU for RX and TX and additionaly we set
+ * - RX_TRIGGER amount of bytes in the FIFO will cause an interrupt.
+ * - less than RX_TRIGGER number of bytes will also cause an interrupt
+ * once the UART decides that there no new bytes arriving.
+ * - Once THRE is enabled, the interrupt will be fired once the FIFO is
+ * empty - the trigger level is ignored here.
+ *
+ * - UART will assert the TX DMA line once there is room for TX_TRIGGER
+ * bytes in the TX FIFO. On each assert the DMA engine will move
+ * TX_TRIGGER bytes into the FIFO.
+ * - UART will assert the RX DMA line once there are RX_TRIGGER bytes in
+ * the FIFO and move RX_TRIGGER bytes.
+ * This is because treshold and trigger values are the same.
threshold
Post by Sebastian Andrzej Siewior
+ /*
+ * It claims to be 16C750 compatible however it is a little different.
+ * It has EFR and has no FCR7_64byte bit. The AFE (which it claims to
+ * have) is enabled via EFR instead of MCR. The type is set here 8250
+ * just to get things going. UNKNOWN does not work for a few reasons and
+ * we don't need our own type since we don't use 8250's set_termios()
+ * and our "bugs" are handeld via the bug member.
handled
Post by Sebastian Andrzej Siewior
+ */
+ up.port.type = PORT_8250;
+ up.port.iotype = UPIO_MEM;
+ up.port.flags = 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.tx_loadsz = 64;
+ up.capabilities = UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP;
+#ifdef CONFIG_PM_RUNTIME
+ /*
+ * PM_RUNTIME is mostly transparent. However to do it right we need to a
need to _do_ a ...?
Post by Sebastian Andrzej Siewior
+ * TX empty interrupt before we can put the device to auto idle. So if
+ * PM_RUNTIME is not enabled we don't add that flag and can spare that
+ * one extra interrupt in the TX path.
+ */
<snip>
Post by Sebastian Andrzej Siewior
+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.
+
I just wondered if this driver should be marked experimental?


Thanks,
Frans
Sebastian Andrzej Siewior
2014-09-29 13:27:12 UTC
Permalink
Post by Sebastian Andrzej Siewior
threshold
fixed
Post by Sebastian Andrzej Siewior
Post by Sebastian Andrzej Siewior
+ /*
+ * It claims to be 16C750 compatible however it is a little different.
+ * It has EFR and has no FCR7_64byte bit. The AFE (which it claims to
+ * have) is enabled via EFR instead of MCR. The type is set here 8250
+ * just to get things going. UNKNOWN does not work for a few reasons and
+ * we don't need our own type since we don't use 8250's set_termios()
+ * and our "bugs" are handeld via the bug member.
handled
replaced that last line with
or pm callback.

since there no bugs member anymore.
Post by Sebastian Andrzej Siewior
Post by Sebastian Andrzej Siewior
+ */
+ up.port.type = PORT_8250;
+ up.port.iotype = UPIO_MEM;
+ up.port.flags = 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.tx_loadsz = 64;
+ up.capabilities = UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP;
+#ifdef CONFIG_PM_RUNTIME
+ /*
+ * PM_RUNTIME is mostly transparent. However to do it right we need to a
need to _do_ a ...?
I think dropping that 'to' should fix it.
Post by Sebastian Andrzej Siewior
Post by Sebastian Andrzej Siewior
+ * TX empty interrupt before we can put the device to auto idle. So if
+ * PM_RUNTIME is not enabled we don't add that flag and can spare that
+ * one extra interrupt in the TX path.
+ */
<snip>
Post by Sebastian Andrzej Siewior
+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.
+
I just wondered if this driver should be marked experimental?
What did you have in mind? CONFIG_EXPERIMENTAL is gone. After all that
debuging that I had in the meantime I was thinking about dropping that
"early stage".
Post by Sebastian Andrzej Siewior
Thanks,
Frans
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Frans Klaver
2014-09-29 13:34:12 UTC
Permalink
On Mon, Sep 29, 2014 at 3:27 PM, Sebastian Andrzej Siewior
Post by Sebastian Andrzej Siewior
Post by Sebastian Andrzej Siewior
threshold
fixed
Post by Sebastian Andrzej Siewior
+ /*
+ * It claims to be 16C750 compatible however it is a little different.
+ * It has EFR and has no FCR7_64byte bit. The AFE (which it claims to
+ * have) is enabled via EFR instead of MCR. The type is set here 8250
+ * just to get things going. UNKNOWN does not work for a few reasons and
+ * we don't need our own type since we don't use 8250's set_termios()
+ * and our "bugs" are handeld via the bug member.
handled
replaced that last line with
or pm callback.
since there no bugs member anymore.
Post by Sebastian Andrzej Siewior
+ */
+ up.port.type = PORT_8250;
+ up.port.iotype = UPIO_MEM;
+ up.port.flags = 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.tx_loadsz = 64;
+ up.capabilities = UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP;
+#ifdef CONFIG_PM_RUNTIME
+ /*
+ * PM_RUNTIME is mostly transparent. However to do it right we need to a
need to _do_ a ...?
I think dropping that 'to' should fix it.
Yup.
Post by Sebastian Andrzej Siewior
Post by Sebastian Andrzej Siewior
+ * TX empty interrupt before we can put the device to auto idle. So if
+ * PM_RUNTIME is not enabled we don't add that flag and can spare that
+ * one extra interrupt in the TX path.
+ */
<snip>
+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.
+
I just wondered if this driver should be marked experimental?
What did you have in mind? CONFIG_EXPERIMENTAL is gone. After all that
debuging that I had in the meantime I was thinking about dropping that
"early stage".
That was the other option. I'm good with that. Also, I never noticed
CONFIG_EXPERIMENTAL being gone, so that's down the drain ;).
Post by Sebastian Andrzej Siewior
Post by Sebastian Andrzej Siewior
Thanks,
Frans
Sebastian
_______________________________________________
linux-arm-kernel mailing list
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Sebastian Andrzej Siewior
2014-09-10 19:29:59 UTC
Permalink
Tony noticed that the old omap-serial driver picked the uart "number"
based on the hint given from device tree or platform device's id.
The 8250 based omap driver doesn't do this because the core code does
not honour the ->line argument which is passed by the driver.

This patch aims to keep the same behaviour as with omap-serial. The
function will first try to use the line suggested ->line argument and
then fallback to the old strategy in case the port is taken.

That means the the third uart will always be ttyS2 even if the previous
two have not been enabled in DT.

Reviewed-by: Tony Lindgren <***@atomide.com>
Tested-by: Tony Lindgren <***@atomide.com>
Signed-off-by: Sebastian Andrzej Siewior <***@linutronix.de>
---
drivers/tty/serial/8250/8250_core.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 547afde9fdda..ac88e66df65d 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3516,6 +3516,11 @@ static struct uart_8250_port *serial8250_find_match_or_unused(struct uart_port *
if (uart_match_port(&serial8250_ports[i].port, port))
return &serial8250_ports[i];

+ /* try line number first if still available */
+ i = port->line;
+ if (i < nr_uarts && serial8250_ports[i].port.type == PORT_UNKNOWN &&
+ serial8250_ports[i].port.iobase == 0)
+ return &serial8250_ports[i];
/*
* We didn't find a matching entry, so look for the first
* free entry. We look for one which hasn't been previously
--
2.1.0
Sebastian Andrzej Siewior
2014-09-10 19:30:00 UTC
Permalink
serial8250_do_startup() adds UART_IER_RDI and UART_IER_RLSI to ier.
serial8250_stop_rx() should remove both.
This is what the serial-omap driver has been doing and is now moved to
the 8250-core since it does no look to be *that* omap specific.

Cc: ***@linux.intel.com
Reviewed-by: Tony Lindgren <***@atomide.com>
Tested-by: Tony Lindgren <***@atomide.com>
Signed-off-by: Sebastian Andrzej Siewior <***@linutronix.de>
---
drivers/tty/serial/8250/8250_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index ac88e66df65d..139f3d2b8aa9 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1390,7 +1390,7 @@ static void serial8250_stop_rx(struct uart_port *port)

serial8250_rpm_get(up);

- up->ier &= ~UART_IER_RLSI;
+ up->ier &= ~(UART_IER_RLSI | UART_IER_RDI);
up->port.read_status_mask &= ~UART_LSR_DR;
serial_port_out(port, UART_IER, up->ier);
--
2.1.0
Heikki Krogerus
2014-09-11 11:19:31 UTC
Permalink
Post by Sebastian Andrzej Siewior
serial8250_do_startup() adds UART_IER_RDI and UART_IER_RLSI to ier.
serial8250_stop_rx() should remove both.
This is what the serial-omap driver has been doing and is now moved to
the 8250-core since it does no look to be *that* omap specific.
Looks good to me. FWIW...
Post by Sebastian Andrzej Siewior
---
drivers/tty/serial/8250/8250_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index ac88e66df65d..139f3d2b8aa9 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1390,7 +1390,7 @@ static void serial8250_stop_rx(struct uart_port *port)
serial8250_rpm_get(up);
- up->ier &= ~UART_IER_RLSI;
+ up->ier &= ~(UART_IER_RLSI | UART_IER_RDI);
up->port.read_status_mask &= ~UART_LSR_DR;
serial_port_out(port, UART_IER, up->ier);
--
2.1.0
Thanks,
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-10 19:30:03 UTC
Permalink
The omap needs a DMA request pending right away. If it is enqueued once
the bytes are in the FIFO then nothing will happen and the FIFO will be
later purged via RX-timeout interrupt.
This patch enqueues RX-DMA request on completion but not if it was
aborted on error. The first enqueue will happen in the driver in
startup.

Reviewed-by: Tony Lindgren <***@atomide.com>
Tested-by: Tony Lindgren <***@atomide.com>
Signed-off-by: Sebastian Andrzej Siewior <***@linutronix.de>
---
drivers/tty/serial/8250/8250.h | 3 ++-
drivers/tty/serial/8250/8250_core.c | 3 +++
drivers/tty/serial/8250/8250_dma.c | 12 +++++++++---
3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index a63d198f8d03..fbed1636e9c4 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -80,7 +80,8 @@ struct serial8250_config {
#define UART_BUG_NOMSR (1 << 2) /* UART has buggy MSR status bits (Au1x00) */
#define UART_BUG_THRE (1 << 3) /* UART has buggy THRE reassertion */
#define UART_BUG_PARITY (1 << 4) /* UART mishandles parity if FIFO enabled */
-
+#define UART_BUG_DMA_RX (1 << 5) /* UART needs DMA RX req before there is
+ data in FIFO */
#define PROBE_RSA (1 << 0)
#define PROBE_ANY (~0)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 9f961ea82564..39b1c7318f17 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1592,6 +1592,9 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)

if (!up->dma || dma_err)
status = serial8250_rx_chars(up, status);
+
+ if (dma_err && up->bugs & UART_BUG_DMA_RX)
+ serial8250_rx_dma(up, 0);
}
serial8250_modem_status(up);
if ((!up->dma || (up->dma && up->dma->tx_err)) &&
diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index 69e54abb6e71..3674900a1f14 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -50,9 +50,8 @@ static void __dma_tx_complete(void *param)
spin_unlock_irqrestore(&p->port.lock, flags);
}

-static void __dma_rx_complete(void *param)
+static void __dma_rx_do_complete(struct uart_8250_port *p, bool error)
{
- struct uart_8250_port *p = param;
struct uart_8250_dma *dma = p->dma;
struct tty_port *tty_port = &p->port.state->port;
struct dma_tx_state state;
@@ -68,10 +67,17 @@ static void __dma_rx_complete(void *param)

tty_insert_flip_string(tty_port, dma->rx_buf, count);
p->port.icount.rx += count;
+ if (!error && p->bugs & UART_BUG_DMA_RX)
+ serial8250_rx_dma(p, 0);

tty_flip_buffer_push(tty_port);
}

+static void __dma_rx_complete(void *param)
+{
+ __dma_rx_do_complete(param, false);
+}
+
int serial8250_tx_dma(struct uart_8250_port *p)
{
struct uart_8250_dma *dma = p->dma;
@@ -139,7 +145,7 @@ int serial8250_rx_dma(struct uart_8250_port *p, unsigned int iir)
*/
if (dma_status == DMA_IN_PROGRESS) {
dmaengine_pause(dma->rxchan);
- __dma_rx_complete(p);
+ __dma_rx_do_complete(p, true);
}
return -ETIMEDOUT;
default:
--
2.1.0
Sebastian Andrzej Siewior
2014-09-10 19:30:04 UTC
Permalink
At least on AM335x the following problem exists: Even if the TX FIFO is
empty and a TX transfer is programmed (and started) the UART does not
trigger the DMA transfer.
After $TRESHOLD number of bytes have been written to the FIFO manually =
the
UART reevaluates the whole situation and decides that now there is enou=
gh
room in the FIFO and so the transfer begins.
This problem has not been seen on DRA7 or beagle board xm (OMAP3). I am=
not
sure if this is UART-IP core specific or DMA engine.

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

v7=E2=80=A6v8:
- fix the problem when get invoked and the FIFO is full.

Reviewed-by: Tony Lindgren <***@atomide.com>
Tested-by: Tony Lindgren <***@atomide.com>
Signed-off-by: Sebastian Andrzej Siewior <***@linutronix.de>
---
drivers/tty/serial/8250/8250.h | 3 +++
drivers/tty/serial/8250/8250_dma.c | 39 ++++++++++++++++++++++++++++++=
+++++---
include/uapi/linux/serial_reg.h | 1 +
3 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8=
250.h
index fbed1636e9c4..09489b391568 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -82,6 +82,9 @@ struct serial8250_config {
#define UART_BUG_PARITY (1 << 4) /* UART mishandles parity if FIFO ena=
bled */
#define UART_BUG_DMA_RX (1 << 5) /* UART needs DMA RX req before there=
is
data in FIFO */
+#define UART_BUG_DMA_TX (1 << 6) /* UART needs one byte in FIFO for
+ kickstart */
+
#define PROBE_RSA (1 << 0)
#define PROBE_ANY (~0)
=20
diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/82=
50/8250_dma.c
index 3674900a1f14..48dc57aad0dd 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -83,6 +83,7 @@ int serial8250_tx_dma(struct uart_8250_port *p)
struct uart_8250_dma *dma =3D p->dma;
struct circ_buf *xmit =3D &p->port.state->xmit;
struct dma_async_tx_descriptor *desc;
+ unsigned int skip_byte =3D 0;
int ret;
=20
if (uart_tx_stopped(&p->port) || dma->tx_running ||
@@ -91,10 +92,40 @@ int serial8250_tx_dma(struct uart_8250_port *p)
=20
dma->tx_size =3D CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SI=
ZE);
=20
+ if (p->bugs & UART_BUG_DMA_TX) {
+ u8 tx_lvl;
+
+ /*
+ * We need to put the first byte into the FIFO in order to start
+ * the DMA transfer. For transfers smaller than four bytes we
+ * don't bother doing DMA at all. It seem not matter if there
+ * are still bytes in the FIFO from the last transfer (in case
+ * we got here directly from __dma_tx_complete()). Bytes leaving
+ * the FIFO seem not to trigger the DMA transfer. It is really
+ * the byte that we put into the FIFO.
+ * If the FIFO is already full then we most likely got here from
+ * __dma_tx_complete(). And this means the DMA engine just
+ * completed its work. We don't have to wait the complete 86us
+ * at 115200,8n1 but around 60us (not to mention lower
+ * baudrates). So in that case we take the interrupt and try
+ * again with an empty FIFO.
+ */
+ tx_lvl =3D serial_in(p, UART_OMAP_TX_LVL);
+ if (tx_lvl =3D=3D p->tx_loadsz) {
+ ret =3D -EBUSY;
+ goto err;
+ }
+ if (dma->tx_size < 4) {
+ ret =3D -EINVAL;
+ goto err;
+ }
+ skip_byte =3D 1;
+ }
+
desc =3D dmaengine_prep_slave_single(dma->txchan,
- dma->tx_addr + xmit->tail,
- dma->tx_size, DMA_MEM_TO_DEV,
- DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ dma->tx_addr + xmit->tail + skip_byte,
+ dma->tx_size - skip_byte, DMA_MEM_TO_DEV,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!desc) {
ret =3D -EBUSY;
goto err;
@@ -118,6 +149,8 @@ int serial8250_tx_dma(struct uart_8250_port *p)
serial_out(p, UART_IER, p->ier);
}
}
+ if (skip_byte)
+ serial_out(p, UART_TX, xmit->buf[xmit->tail]);
return 0;
err:
dma->tx_err =3D 1;
diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/seria=
l_reg.h
index df6c9ab6b0cd..53af3b790129 100644
--- a/include/uapi/linux/serial_reg.h
+++ b/include/uapi/linux/serial_reg.h
@@ -359,6 +359,7 @@
#define UART_OMAP_SYSC 0x15 /* System configuration register */
#define UART_OMAP_SYSS 0x16 /* System status register */
#define UART_OMAP_WER 0x17 /* Wake-up enable register */
+#define UART_OMAP_TX_LVL 0x1a /* TX FIFO level register */
=20
/*
* These are the definitions for the MDR1 register
--=20
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Heikki Krogerus
2014-09-11 11:17:21 UTC
Permalink
On Wed, Sep 10, 2014 at 09:30:04PM +0200, Sebastian Andrzej Siewior wro=
At least on AM335x the following problem exists: Even if the TX FIFO =
is
empty and a TX transfer is programmed (and started) the UART does not
trigger the DMA transfer.
After $TRESHOLD number of bytes have been written to the FIFO manuall=
y the
UART reevaluates the whole situation and decides that now there is en=
ough
room in the FIFO and so the transfer begins.
This problem has not been seen on DRA7 or beagle board xm (OMAP3). I =
am not
sure if this is UART-IP core specific or DMA engine.
=20
The workaround is to use a threshold of one byte, program the DMA
transfer minus one byte and then to put the first byte into the FIFO =
to
kick start the transfer.
=20
- fix the problem when get invoked and the FIFO is full.
=20
---
drivers/tty/serial/8250/8250.h | 3 +++
drivers/tty/serial/8250/8250_dma.c | 39 ++++++++++++++++++++++++++++=
+++++++---
include/uapi/linux/serial_reg.h | 1 +
3 files changed, 40 insertions(+), 3 deletions(-)
=20
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250=
/8250.h
index fbed1636e9c4..09489b391568 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -82,6 +82,9 @@ struct serial8250_config {
#define UART_BUG_PARITY (1 << 4) /* UART mishandles parity if FIFO e=
nabled */
#define UART_BUG_DMA_RX (1 << 5) /* UART needs DMA RX req before the=
re is
data in FIFO */
+#define UART_BUG_DMA_TX (1 << 6) /* UART needs one byte in FIFO for
+ kickstart */
I don't think we should go ahead with this patch. I'm pretty sure
this is AM335 specific problem, or at least limited to only few
platforms. And I don't think we should take any more "BUG" flags.

We should add hooks like tx_dma and rx_dma to struct uart_8250_dma so
that the probe drivers can replace serial8250_tx_dma and
seria8250_rx_dma, like I think Alan already suggested.

Let's keep serial8250_tx_dma/rx_dma as the default, and not add any
quirks to them. Only if there is a very common case should it be
handled in those. The case of RX req needing to be sent before data in
=46IFO maybe one of those, but I'm no sure.
#define PROBE_RSA (1 << 0)
#define PROBE_ANY (~0)
=20
diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/=
8250/8250_dma.c
index 3674900a1f14..48dc57aad0dd 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -83,6 +83,7 @@ int serial8250_tx_dma(struct uart_8250_port *p)
struct uart_8250_dma *dma =3D p->dma;
struct circ_buf *xmit =3D &p->port.state->xmit;
struct dma_async_tx_descriptor *desc;
+ unsigned int skip_byte =3D 0;
int ret;
=20
if (uart_tx_stopped(&p->port) || dma->tx_running ||
@@ -91,10 +92,40 @@ int serial8250_tx_dma(struct uart_8250_port *p)
=20
dma->tx_size =3D CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_=
SIZE);
=20
+ if (p->bugs & UART_BUG_DMA_TX) {
+ u8 tx_lvl;
+
+ /*
+ * We need to put the first byte into the FIFO in order to start
+ * the DMA transfer. For transfers smaller than four bytes we
+ * don't bother doing DMA at all. It seem not matter if there
+ * are still bytes in the FIFO from the last transfer (in case
+ * we got here directly from __dma_tx_complete()). Bytes leaving
+ * the FIFO seem not to trigger the DMA transfer. It is really
+ * the byte that we put into the FIFO.
+ * If the FIFO is already full then we most likely got here from
+ * __dma_tx_complete(). And this means the DMA engine just
+ * completed its work. We don't have to wait the complete 86us
+ * at 115200,8n1 but around 60us (not to mention lower
+ * baudrates). So in that case we take the interrupt and try
+ * again with an empty FIFO.
+ */
+ tx_lvl =3D serial_in(p, UART_OMAP_TX_LVL);
+ if (tx_lvl =3D=3D p->tx_loadsz) {
+ ret =3D -EBUSY;
+ goto err;
+ }
+ if (dma->tx_size < 4) {
+ ret =3D -EINVAL;
+ goto err;
+ }
+ skip_byte =3D 1;
+ }
+
desc =3D dmaengine_prep_slave_single(dma->txchan,
- dma->tx_addr + xmit->tail,
- dma->tx_size, DMA_MEM_TO_DEV,
- DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ dma->tx_addr + xmit->tail + skip_byte,
+ dma->tx_size - skip_byte, DMA_MEM_TO_DEV,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!desc) {
ret =3D -EBUSY;
goto err;
@@ -118,6 +149,8 @@ int serial8250_tx_dma(struct uart_8250_port *p)
serial_out(p, UART_IER, p->ier);
}
}
+ if (skip_byte)
+ serial_out(p, UART_TX, xmit->buf[xmit->tail]);
return 0;
dma->tx_err =3D 1;
diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/ser=
ial_reg.h
index df6c9ab6b0cd..53af3b790129 100644
--- a/include/uapi/linux/serial_reg.h
+++ b/include/uapi/linux/serial_reg.h
@@ -359,6 +359,7 @@
#define UART_OMAP_SYSC 0x15 /* System configuration register */
#define UART_OMAP_SYSS 0x16 /* System status register */
#define UART_OMAP_WER 0x17 /* Wake-up enable register */
+#define UART_OMAP_TX_LVL 0x1a /* TX FIFO level register */
=20
/*
* These are the definitions for the MDR1 register
--=20
2.1.0
Cheers,

--=20
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-11 11:42:35 UTC
Permalink
Post by Heikki Krogerus
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/825=
0/8250.h
Post by Heikki Krogerus
index fbed1636e9c4..09489b391568 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -82,6 +82,9 @@ struct serial8250_config {
#define UART_BUG_PARITY (1 << 4) /* UART mishandles parity if FIFO =
enabled */
Post by Heikki Krogerus
#define UART_BUG_DMA_RX (1 << 5) /* UART needs DMA RX req before th=
ere is
Post by Heikki Krogerus
data in FIFO */
+#define UART_BUG_DMA_TX (1 << 6) /* UART needs one byte in FIFO for
+ kickstart */
=20
I don't think we should go ahead with this patch. I'm pretty sure
this is AM335 specific problem, or at least limited to only few
platforms. And I don't think we should take any more "BUG" flags.
=20
We should add hooks like tx_dma and rx_dma to struct uart_8250_dma so
that the probe drivers can replace serial8250_tx_dma and
seria8250_rx_dma, like I think Alan already suggested.
Okay. Wasn't aware that Alan already suggested that.
I also need a watchdog timer for TX since it seems that on omap3 the
DMA engine suddenly forgets to continue with DMA=E2=80=A6

If this is really what we want, I would need to refactor a few things=E2=
=80=A6
Post by Heikki Krogerus
Let's keep serial8250_tx_dma/rx_dma as the default, and not add any
quirks to them. Only if there is a very common case should it be
handled in those. The case of RX req needing to be sent before data i=
n
Post by Heikki Krogerus
FIFO maybe one of those, but I'm no sure.
keep in mind that both (RX & TX bugs/hacks) need also a bit of handling
in the 8250-core so it works together (like the tx_err member so we
fall back to manual xmit)

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Peter Hurley
2014-09-11 12:32:58 UTC
Permalink
Post by Sebastian Andrzej Siewior
I also need a watchdog timer for TX since it seems that on omap3 the
DMA engine suddenly forgets to continue with DMA=E2=80=A6
One difference I noticed between the omap driver and the 8250 driver is
the way modem status interrupts are handled.

The omap driver only checks modem status for the UART_IIR_MSI interrupt=
type.
The 8250 driver checks modem status at every interrupt (other than NO_I=
NT).

I think the UART_MSR_DCTS bit always reflects that the CTS input has ch=
anged
between reads of the MSR _even if auto CTS is on_. So perhaps the hardw=
are
is being stopped by uart_handle_cts_change() when auto CTS is on?

Regards,
Peter Hurley

[The UPF_HARD_FLOW thing was pretty much just done for omap even though
8250 already had auto CTS/auto RTS. Serial core hardware flow control s=
upport
needs a redo as drivers have pretty much tacked stuff on randomly.]
Sebastian Andrzej Siewior
2014-09-11 12:50:50 UTC
Permalink
Post by Sebastian Andrzej Siewior
I also need a watchdog timer for TX since it seems that on omap3 the
DMA engine suddenly forgets to continue with DMA=E2=80=A6
=20
One difference I noticed between the omap driver and the 8250 driver =
is
the way modem status interrupts are handled.
=20
The omap driver only checks modem status for the UART_IIR_MSI interru=
pt type.
The 8250 driver checks modem status at every interrupt (other than NO=
_INT).
=20
I think the UART_MSR_DCTS bit always reflects that the CTS input has =
changed
between reads of the MSR _even if auto CTS is on_. So perhaps the har=
dware
is being stopped by uart_handle_cts_change() when auto CTS is on?
I doubt that. What I see from a timer debug is that the TX-FIFO level
is at 0, the DMA transfer for say 1024 bytes start.
The FIFO is filled to 64bytes and refilled so it doesn't drop below 50.
At the time of the stall I see that the DMA engine has outstanding
bytes which it should transfer and the TX FIFO is empty. If hardware
flow control stops the transfer, I would expect that the DMA engine
still fills the TX-FIFO until 64 and then waits. But it doesn't.
Writing bytes into the FIFO leads to bytes beeing sent (and I see them
on the other side) but the DMA transfer is still on hold. Canceling the
DMA transfer and re-programming the remaining bytes transfers the
remaining bytes.

The odd thing is that I only triggered it with "less file". It doesn't
happen on regular console interaction or "cat large-file". And it only
triggers on beagle board xm (omap34xx) and I wasn't able to reproduce
it on am335x or dra7. The latter shares the same DMA engine as beagle
board xm.

I remember also that I disabled the HW/SW float control just to make
sure it is not it.
=20
Regards,
Peter Hurley
=20
[The UPF_HARD_FLOW thing was pretty much just done for omap even thou=
gh
8250 already had auto CTS/auto RTS. Serial core hardware flow control=
support
needs a redo as drivers have pretty much tacked stuff on randomly.]
Sebastian
Peter Hurley
2014-09-11 14:35:43 UTC
Permalink
Post by Sebastian Andrzej Siewior
I also need a watchdog timer for TX since it seems that on omap3 th=
e
Post by Sebastian Andrzej Siewior
DMA engine suddenly forgets to continue with DMA=E2=80=A6
One difference I noticed between the omap driver and the 8250 driver=
is
Post by Sebastian Andrzej Siewior
the way modem status interrupts are handled.
The omap driver only checks modem status for the UART_IIR_MSI interr=
upt type.
Post by Sebastian Andrzej Siewior
The 8250 driver checks modem status at every interrupt (other than N=
O_INT).
Post by Sebastian Andrzej Siewior
I think the UART_MSR_DCTS bit always reflects that the CTS input has=
changed
Post by Sebastian Andrzej Siewior
between reads of the MSR _even if auto CTS is on_. So perhaps the ha=
rdware
Post by Sebastian Andrzej Siewior
is being stopped by uart_handle_cts_change() when auto CTS is on?
=20
I doubt that. What I see from a timer debug is that the TX-FIFO level
is at 0, the DMA transfer for say 1024 bytes start.
The FIFO is filled to 64bytes and refilled so it doesn't drop below 5=
0.
Post by Sebastian Andrzej Siewior
At the time of the stall I see that the DMA engine has outstanding
bytes which it should transfer and the TX FIFO is empty. If hardware
flow control stops the transfer, I would expect that the DMA engine
still fills the TX-FIFO until 64 and then waits. But it doesn't.
Writing bytes into the FIFO leads to bytes beeing sent (and I see the=
m
Post by Sebastian Andrzej Siewior
on the other side) but the DMA transfer is still on hold. Canceling t=
he
Post by Sebastian Andrzej Siewior
DMA transfer and re-programming the remaining bytes transfers the
remaining bytes.
=20
The odd thing is that I only triggered it with "less file". It doesn'=
t
Post by Sebastian Andrzej Siewior
happen on regular console interaction or "cat large-file". And it onl=
y
Post by Sebastian Andrzej Siewior
triggers on beagle board xm (omap34xx) and I wasn't able to reproduce
it on am335x or dra7. The latter shares the same DMA engine as beagle
board xm.
=20
I remember also that I disabled the HW/SW float control just to make
sure it is not it.
Ok.

I do need to find out if omap hardware sets UART_MSR_DCTS when auto CTS
is on. Would you mind running the debug patch below with HW flow contro=
l on?

Regards,
Peter Hurley

--- >% ---
Subject: [DEBUG] serial: does OMAP set UART_LSR_DCTS while autoCTS is o=
n?

** debug patch only **

Signed-off-by: Peter Hurley <***@hurleysoftware.com>
---
drivers/tty/serial/serial_core.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/seri=
al_core.c
index 5a78f69..1579a20 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2783,6 +2783,9 @@ void uart_handle_cts_change(struct uart_port *upo=
rt, unsigned int status)
uport->icount.cts++;
=20
if (tty_port_cts_enabled(port)) {
+
+ WARN_ON_ONCE(uport->flags & UPF_HARD_FLOW);
+
if (tty->hw_stopped) {
if (status) {
tty->hw_stopped =3D 0;
--=20
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-15 17:01:57 UTC
Permalink
Post by Peter Hurley
I do need to find out if omap hardware sets UART_MSR_DCTS when auto CTS
is on. Would you mind running the debug patch below with HW flow control on?
I didn't forget about this. However I told minicom to use hardware flow
control (and I see the driver set the HW bit) but I haven't seen that
uart_handle_cts_change() has been invoked at all. I'm going to check
two other boards and report then.
Post by Peter Hurley
Regards,
Peter Hurley
Sebastian
Sebastian Andrzej Siewior
2014-09-16 16:55:37 UTC
Permalink
I do need to find out if omap hardware sets UART_MSR_DCTS when auto =
CTS
is on. Would you mind running the debug patch below with HW flow con=
trol on?
=20
I didn't forget about this. However I told minicom to use hardware fl=
ow
control (and I see the driver set the HW bit) but I haven't seen that
uart_handle_cts_change() has been invoked at all. I'm going to check
two other boards and report then.
No, I don't get into this at all function. So I connected my am335x-evm
with beagle board xm because both of them have an old fashion UART
connector (instead those uart-to-usb). Both configured with HW-Flow and
I haven't seen the function invoked but I saw "port->icount.overrun"
being incremented. This shouldn't happen. So I am a little puzzled here=
=E2=80=A6
Regards,
Peter Hurley
=20
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Peter Hurley
2014-09-17 12:20:41 UTC
Permalink
Post by Sebastian Andrzej Siewior
I do need to find out if omap hardware sets UART_MSR_DCTS when auto=
CTS
Post by Sebastian Andrzej Siewior
is on. Would you mind running the debug patch below with HW flow co=
ntrol on?
Post by Sebastian Andrzej Siewior
I didn't forget about this. However I told minicom to use hardware f=
low
Post by Sebastian Andrzej Siewior
control (and I see the driver set the HW bit) but I haven't seen tha=
t
Post by Sebastian Andrzej Siewior
uart_handle_cts_change() has been invoked at all. I'm going to check
two other boards and report then.
=20
No, I don't get into this at all function.
Ok, good to know. Thanks for testing that.
Post by Sebastian Andrzej Siewior
So I connected my am335x-evm
with beagle board xm because both of them have an old fashion UART
connector (instead those uart-to-usb). Both configured with HW-Flow a=
nd
Post by Sebastian Andrzej Siewior
I haven't seen the function invoked but I saw "port->icount.overrun"
being incremented. This shouldn't happen. So I am a little puzzled he=
re=E2=80=A6

Yeah, that's weird. Do you have a break-out box to confirm that RTS/CTS=
are
being driven?

Regards,
Peter Hurley
Sebastian Andrzej Siewior
2014-09-17 16:25:07 UTC
Permalink
Post by Sebastian Andrzej Siewior
So I connected my am335x-evm
with beagle board xm because both of them have an old fashion UART
connector (instead those uart-to-usb). Both configured with HW-Flow =
and
Post by Sebastian Andrzej Siewior
I haven't seen the function invoked but I saw "port->icount.overrun"
being incremented. This shouldn't happen. So I am a little puzzled h=
ere=E2=80=A6
=20
Yeah, that's weird. Do you have a break-out box to confirm that RTS/C=
TS are
being driven?
Yeah, I've been thinking about that, too. I will be gone next week and
hopefully when I get back I will something around to test this.
=20
Regards,
Peter Hurley
=20
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-29 16:15:05 UTC
Permalink
Post by Sebastian Andrzej Siewior
So I connected my am335x-evm
with beagle board xm because both of them have an old fashion UART
connector (instead those uart-to-usb). Both configured with HW-Flow =
and
Post by Sebastian Andrzej Siewior
I haven't seen the function invoked but I saw "port->icount.overrun"
being incremented. This shouldn't happen. So I am a little puzzled h=
ere=E2=80=A6
Yeah, that's weird. Do you have a break-out box to confirm that RTS/CT=
S are
being driven?
=3D>
- beagle board
According to schematics the board has only RX and TX connected. No
RTS/CTS

- am335x-evm
The schematics say "DNI" next to a resistor on RTS/CTS. DNI stands
most likely for "Do Not Install". With a scope it looks like the the
wire behind the MAX is open.

- beagle bone black
Only RX and TX are wired towards the USB2serial device.

In short: each device I have has RTS/CTS not working/connected and I
can't test HW handshake with HW available.
Regards,
Peter Hurley
Sebastian
Frans Klaver
2014-09-11 15:11:14 UTC
Permalink
On Thu, Sep 11, 2014 at 02:50:50PM +0200, Sebastian Andrzej Siewior wro=
Post by Sebastian Andrzej Siewior
I also need a watchdog timer for TX since it seems that on omap3 t=
he
Post by Sebastian Andrzej Siewior
DMA engine suddenly forgets to continue with DMA=E2=80=A6
=20
One difference I noticed between the omap driver and the 8250 drive=
r is
Post by Sebastian Andrzej Siewior
the way modem status interrupts are handled.
=20
The omap driver only checks modem status for the UART_IIR_MSI inter=
rupt type.
Post by Sebastian Andrzej Siewior
The 8250 driver checks modem status at every interrupt (other than =
NO_INT).
Post by Sebastian Andrzej Siewior
=20
I think the UART_MSR_DCTS bit always reflects that the CTS input ha=
s changed
Post by Sebastian Andrzej Siewior
between reads of the MSR _even if auto CTS is on_. So perhaps the h=
ardware
Post by Sebastian Andrzej Siewior
is being stopped by uart_handle_cts_change() when auto CTS is on?
=20
I doubt that. What I see from a timer debug is that the TX-FIFO level
is at 0, the DMA transfer for say 1024 bytes start.
The FIFO is filled to 64bytes and refilled so it doesn't drop below 5=
0.
Post by Sebastian Andrzej Siewior
At the time of the stall I see that the DMA engine has outstanding
bytes which it should transfer and the TX FIFO is empty. If hardware
flow control stops the transfer, I would expect that the DMA engine
still fills the TX-FIFO until 64 and then waits. But it doesn't.
Writing bytes into the FIFO leads to bytes beeing sent (and I see the=
m
Post by Sebastian Andrzej Siewior
on the other side) but the DMA transfer is still on hold. Canceling t=
he
Post by Sebastian Andrzej Siewior
DMA transfer and re-programming the remaining bytes transfers the
remaining bytes.
=20
The odd thing is that I only triggered it with "less file". It doesn'=
t
Post by Sebastian Andrzej Siewior
happen on regular console interaction or "cat large-file". And it onl=
y
Post by Sebastian Andrzej Siewior
triggers on beagle board xm (omap34xx) and I wasn't able to reproduce
it on am335x or dra7. The latter shares the same DMA engine as beagle
board xm.
I can still reproduce it on am335x. I can get out of it as soon as
something else gets written to the console though.

# echo "<3>something" >/dev/kmsg

=46rans
Post by Sebastian Andrzej Siewior
=20
I remember also that I disabled the HW/SW float control just to make
sure it is not it.
=20
=20
Regards,
Peter Hurley
=20
[The UPF_HARD_FLOW thing was pretty much just done for omap even th=
ough
Post by Sebastian Andrzej Siewior
8250 already had auto CTS/auto RTS. Serial core hardware flow contr=
ol support
Post by Sebastian Andrzej Siewior
needs a redo as drivers have pretty much tacked stuff on randomly.]
=20
Sebastian
=20
_______________________________________________
linux-arm-kernel mailing list
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-11 16:04:32 UTC
Permalink
Post by Frans Klaver
I can still reproduce it on am335x. I can get out of it as soon as
something else gets written to the console though.
# echo "<3>something" >/dev/kmsg
Is this to stall it or to get out of it?
Post by Frans Klaver
Frans
Sebastian
Frans Klaver
2014-09-11 17:04:03 UTC
Permalink
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
I can still reproduce it on am335x. I can get out of it as soon as
something else gets written to the console though.
# echo "<3>something" >/dev/kmsg
Is this to stall it or to get out of it?
This is to get out of it. I do this from an ssh connection after the console got stuck. The 3 kind of ensures the message is actually sent to ttyS0.
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
Frans
Sebastian
_______________________________________________
linux-arm-kernel mailing list
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Sebastian Andrzej Siewior
2014-09-12 07:23:24 UTC
Permalink
Post by Frans Klaver
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
I can still reproduce it on am335x. I can get out of it as soon as
something else gets written to the console though.
# echo "<3>something" >/dev/kmsg
Is this to stall it or to get out of it?
This is to get out of it. I do this from an ssh connection after the console got stuck. The 3 kind of ensures the message is actually sent to ttyS0.
Interesting. This shouldn't do anything. If there is a TX operation in
progress then ->start_tx() will simply return and the xmit buffer will
be send once the current TX operation completes.

Is there anything I can do to reproduce this behavior?
This problem only pops-up if you use DMA. With disabled DMA you don't
see this, right?

Sebastian

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Frans Klaver
2014-09-12 09:40:10 UTC
Permalink
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
I can still reproduce it on am335x. I can get out of it as soon as
something else gets written to the console though.
# echo "<3>something" >/dev/kmsg
Is this to stall it or to get out of it?
This is to get out of it. I do this from an ssh connection after the console got stuck. The 3 kind of ensures the message is actually sent to ttyS0.
Interesting. This shouldn't do anything. If there is a TX operation in
progress then ->start_tx() will simply return and the xmit buffer will
be send once the current TX operation completes.
Is there anything I can do to reproduce this behavior?
I'm not sure. I just reproduced this on a boneblack, using your uart_v9
branch.
Post by Sebastian Andrzej Siewior
This problem only pops-up if you use DMA. With disabled DMA you don't
see this, right?
I get the lockup both with and without DMA enabled. Here's the 8250
config I use. Our full .config is attached in case it may provide
something relevant.

CONFIG_SERIAL_8250=y
CONFIG_SERIAL_8250_DEPRECATED_OPTIONS=y
CONFIG_SERIAL_8250_CONSOLE=y
CONFIG_SERIAL_8250_DMA=y
CONFIG_SERIAL_8250_NR_UARTS=4
CONFIG_SERIAL_8250_RUNTIME_UARTS=4
CONFIG_SERIAL_8250_EXTENDED=y
# CONFIG_SERIAL_8250_MANY_PORTS is not set
# CONFIG_SERIAL_8250_SHARE_IRQ is not set
# CONFIG_SERIAL_8250_DETECT_IRQ is not set
# CONFIG_SERIAL_8250_RSA is not set
# CONFIG_SERIAL_8250_DW is not set
# CONFIG_SERIAL_8250_EM is not set
CONFIG_SERIAL_8250_OMAP=y
# Non-8250 serial port support
# CONFIG_DEBUG_LL_UART_8250 is not set
# CONFIG_DEBUG_UART_8250 is not set

FWIW:

***@boneblack:~# cat /proc/device-tree/compatible
ti,am335x-boneti,am33xx
***@boneblack:~# udevadm info -a -p $(udevadm info -q path -n /dev/ttyS0)
...
looking at device '/devices/ocp/44e09000.serial/tty/ttyS0':
KERNEL=="ttyS0"
SUBSYSTEM=="tty"
DRIVER==""
ATTR{irq}=="88"
ATTR{line}=="0"
ATTR{port}=="0x0"
ATTR{type}=="1"
ATTR{flags}=="0x38600000"
ATTR{iomem_base}=="0x44E09000"
ATTR{custom_divisor}=="0"
ATTR{iomem_reg_shift}=="2"
ATTR{uartclk}=="48000000"
ATTR{xmit_fifo_size}=="64"
ATTR{close_delay}=="50"
ATTR{closing_wait}=="3000"
ATTR{io_type}=="2"

looking at parent device '/devices/ocp/44e09000.serial':
KERNELS=="44e09000.serial"
SUBSYSTEMS=="platform"
DRIVERS=="omap8250"
ATTRS{driver_override}=="(null)"

looking at parent device '/devices/ocp':
KERNELS=="ocp"
SUBSYSTEMS=="platform"
DRIVERS==""
ATTRS{driver_override}=="(null)"

Something that may also help: when I have a lockup on the boneblack, dma
is enabled and something is written to console like I described earlier,
I get the following bad irq:

[ 34.197045] irq 30: nobody cared (try booting with the "irqpoll" option)
[ 34.197078] CPU: 0 PID: 314 Comm: sh Tainted: G W 3.17.0-rc3-00075-g3ef5e6d #69
[ 34.197179] [<c0013454>] (unwind_backtrace) from [<c00113b0>] (show_stack+0x10/0x14)
[ 34.197239] [<c00113b0>] (show_stack) from [<c005ca80>] (__report_bad_irq+0x28/0xb8)
[ 34.197282] [<c005ca80>] (__report_bad_irq) from [<c005cecc>] (note_interrupt+0x1cc/0x270)
[ 34.197324] [<c005cecc>] (note_interrupt) from [<c005b30c>] (handle_irq_event_percpu+0x110/0x130)
[ 34.197364] [<c005b30c>] (handle_irq_event_percpu) from [<c005b368>] (handle_irq_event+0x3c/0x5c)
[ 34.197401] [<c005b368>] (handle_irq_event) from [<c005d88c>] (handle_level_irq+0xd0/0x108)
[ 34.197437] [<c005d88c>] (handle_level_irq) from [<c005ac0c>] (generic_handle_irq+0x20/0x30)
[ 34.197474] [<c005ac0c>] (generic_handle_irq) from [<c000ee2c>] (handle_IRQ+0x60/0x80)
[ 34.197509] [<c000ee2c>] (handle_IRQ) from [<c0008654>] (omap3_intc_handle_irq+0x64/0x8c)
[ 34.197548] [<c0008654>] (omap3_intc_handle_irq) from [<c0011dc0>] (__irq_svc+0x40/0x54)
[ 34.197562] Exception stack(0xdf5c9cf8 to 0xdf5c9d40)not ready
[ 34.197581] 9ce0: df5c9d40 00400100
[ 34.197611] 9d00: 00000100 ffffffc0 00000040 df5c8000 00000000 c077cf00 c077d4c0 c077cec0
[ 34.197641] 9d20: c077d4c0 600f0013 0000000a df5c9d40 c00369c0 c0036a20 200f0113 ffffffff
[ 34.197688] [<c0011dc0>] (__irq_svc) from [<c0036a20>] (__do_softirq+0x78/0x1f4)s/Full - flow control rx/tx
[ 34.197723] [<c0036a20>] (__do_softirq) from [<c0036db8>] (irq_exit+0x80/0xdc)
[ 34.197756] [<c0036db8>] (irq_exit) from [<c000ee30>] (handle_IRQ+0x64/0x80)
[ 34.197789] [<c000ee30>] (handle_IRQ) from [<c0008654>] (omap3_intc_handle_irq+0x64/0x8c)
[ 34.197825] [<c0008654>] (omap3_intc_handle_irq) from [<c0011dc0>] (__irq_svc+0x40/0x54)
[ 34.197837] Exception stack(0xdf5c9db8 to 0xdf5c9e00):Link
[ 34.197853] 9da0: 00000000 00000000
[ 34.197881] 9dc0: 00000002 df0b9e10 00000003 000008d0 00000030 00000062 c077d4c0 c077d4c0
[ 34.197911] 9de0: c077d4c0 600f0013 00000000 df5c9e00 c0058670 c00594fc 600f0013 ffffffff
[ 34.197952] [<c0011dc0>] (__irq_svc) from [<c00594fc>] (console_unlock+0x1c8/0x368)
[ 34.197989] [<c00594fc>] (console_unlock) from [<c0059ac0>] (vprintk_emit+0x424/0x46c)
[ 34.198029] [<c0059ac0>] (vprintk_emit) from [<c04cd420>] (printk_emit+0x1c/0x24)
[ 34.198067] [<c04cd420>] (printk_emit) from [<c0059c7c>] (devkmsg_writev+0x150/0x184)
[ 34.198109] [<c0059c7c>] (devkmsg_writev) from [<c00bf730>] (do_sync_write+0x6c/0x90)
[ 34.198145] [<c00bf730>] (do_sync_write) from [<c00bffe0>] (vfs_write+0xd4/0x1bc)
[ 34.198179] [<c00bffe0>] (vfs_write) from [<c00c05d4>] (SyS_write+0x3c/0x7c)
[ 34.198213] [<c00c05d4>] (SyS_write) from [<c000e540>] (ret_fast_syscall+0x0/0x30)
[ 34.198222] handlers:tained, lease time 86400
[ 34.198249] [<c001a378>] dma_ccerr_handler.1
[ 34.198258] Disabling IRQ #30.

Full dmesg is also attached.

Hope you find something useful in there.

Thanks,
Frans
Sebastian Andrzej Siewior
2014-09-12 09:51:22 UTC
Permalink
Post by Frans Klaver
I'm not sure. I just reproduced this on a boneblack, using your uart_v9
branch.
Post by Sebastian Andrzej Siewior
This problem only pops-up if you use DMA. With disabled DMA you don't
see this, right?
I get the lockup both with and without DMA enabled. Here's the 8250
config I use. Our full .config is attached in case it may provide
something relevant.
Hmm. I have a bone black here. Can you tell what you did to get this
lockup? The port configuration (unless 115200 8N1) and what you did to
get this lockup.
Post by Frans Klaver
Something that may also help: when I have a lockup on the boneblack, dma
is enabled and something is written to console like I described earlier,
Full dmesg is also attached.
This one should be stuffed by this:
"[RFC] ARM: edma: unconditionally ack the error interrupt"
https://lkml.org/lkml/2014/9/10/714
Post by Frans Klaver
Hope you find something useful in there.
Thanks,
Frans
Sebastian
Frans Klaver
2014-09-12 10:28:16 UTC
Permalink
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
I'm not sure. I just reproduced this on a boneblack, using your uart_v9
branch.
Post by Sebastian Andrzej Siewior
This problem only pops-up if you use DMA. With disabled DMA you don't
see this, right?
I get the lockup both with and without DMA enabled. Here's the 8250
config I use. Our full .config is attached in case it may provide
something relevant.
Hmm. I have a bone black here. Can you tell what you did to get this
lockup? The port configuration (unless 115200 8N1) and what you did to
get this lockup.
port config is 115200 8N1. I don't recall doing anything special. I
boot, login, less file and get a lock.
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
Something that may also help: when I have a lockup on the boneblack, dma
is enabled and something is written to console like I described earlier,
Full dmesg is also attached.
"[RFC] ARM: edma: unconditionally ack the error interrupt"
https://lkml.org/lkml/2014/9/10/714
OK, that makes the console usable again after I write something to kmsg.
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
Hope you find something useful in there.
Thanks,
Frans
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-15 16:42:04 UTC
Permalink
Post by Frans Klaver
port config is 115200 8N1. I don't recall doing anything special. I
boot, login, less file and get a lock.
So I booted my mini Debian 7.6 (basic system + openssh) on my beagle
bone black which is:

[ 0.000000] AM335X ES2.0 (neon )

configured a console, login, invoked "less file". The file was shown, I
hit on the space key so less shows me more of the file. No lock-up.
I tried booting via NFS and MMC. I tried various files with less.

My dot config is here
https://breakpoint.cc/config-am335x-bb.txt.xz

If there is nothing specific to the file you do less on I have no idea
what else it could if it is not the config.

Sebastian

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Frans Klaver
2014-09-16 09:05:40 UTC
Permalink
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
port config is 115200 8N1. I don't recall doing anything special. I
boot, login, less file and get a lock.
So I booted my mini Debian 7.6 (basic system + openssh) on my beagle
[ 0.000000] AM335X ES2.0 (neon )
configured a console, login, invoked "less file". The file was shown, I
hit on the space key so less shows me more of the file. No lock-up.
I tried booting via NFS and MMC. I tried various files with less.
My dot config is here
https://breakpoint.cc/config-am335x-bb.txt.xz
If there is nothing specific to the file you do less on I have no idea
what else it could if it is not the config.
I'll test your config and go through the differences.

Thanks,
Frans
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Frans Klaver
2014-09-16 12:42:01 UTC
Permalink
Post by Frans Klaver
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
port config is 115200 8N1. I don't recall doing anything special. I
boot, login, less file and get a lock.
So I booted my mini Debian 7.6 (basic system + openssh) on my beagle
[ 0.000000] AM335X ES2.0 (neon )
configured a console, login, invoked "less file". The file was shown, I
hit on the space key so less shows me more of the file. No lock-up.
I tried booting via NFS and MMC. I tried various files with less.
My dot config is here
https://breakpoint.cc/config-am335x-bb.txt.xz
If there is nothing specific to the file you do less on I have no idea
what else it could if it is not the config.
I'll test your config and go through the differences.
So far it looks like it isn't in the config. I've tested both your and
my config on debian wheezy on a boneblack. No lock ups.

Less doesn't fill my entire screen, so it looks a bit funky when
scrolling, but I'm not sure if that's related to the driver.

I'll have to look further for things we did that may have impact here.
Post by Frans Klaver
Thanks,
Frans
_______________________________________________
linux-arm-kernel mailing list
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Frans Klaver
2014-09-16 14:23:11 UTC
Permalink
Post by Frans Klaver
Post by Frans Klaver
Post by Sebastian Andrzej Siewior
If there is nothing specific to the file you do less on I have no idea
what else it could if it is not the config.
I'll test your config and go through the differences.
So far it looks like it isn't in the config. I've tested both your and
my config on debian wheezy on a boneblack. No lock ups.
No, skip that. It is in the config. I'll hunt it down.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Frans Klaver
2014-09-17 10:28:12 UTC
Permalink
Hi,

Yesterday's testing was a bit messy. So here goes again.

On Mon, Sep 15, 2014 at 06:42:04PM +0200, Sebastian Andrzej Siewior wro=
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
port config is 115200 8N1. I don't recall doing anything special. I
boot, login, less file and get a lock.
=20
So I booted my mini Debian 7.6 (basic system + openssh) on my beagle
=20
[ 0.000000] AM335X ES2.0 (neon )
Mine's the same.
Post by Sebastian Andrzej Siewior
configured a console, login, invoked "less file". The file was shown,=
I
Post by Sebastian Andrzej Siewior
hit on the space key so less shows me more of the file. No lock-up.
I tried booting via NFS and MMC. I tried various files with less.
=20
My dot config is here
https://breakpoint.cc/config-am335x-bb.txt.xz
=20
If there is nothing specific to the file you do less on I have no ide=
a
Post by Sebastian Andrzej Siewior
what else it could if it is not the config.
It could be environmental. I have three test cases right now. Two of
them on the beagle bone black, the third on our custom am335x based
platform.

- All test cases run the same kernel built from uart_v10-pre1.
- For the black, the board, dtb, and u-boot environment are equal for
the test cases.

- Bone Black: Debian 7.5
Login, "less file" doesn't lock up. Scrolling down looks sensible.
Scrolling up leaves me with a crooked display, provided the minicom
window is more than 24 lines high. Condensed example:

Normal less looks like:
=20
Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do
eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad
minim veniam, quis nostrud exercitation ullamco laboris nisi ut
aliquip ex ea commodo consequat. Duis aute irure dolor in
:

While after scrolling up it looks like

minim veniam, quis nostrud exercitation ullamco laboris nisi ut
aliquip ex ea commodo consequat. Duis aute irure dolor in
:

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do
eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad

vi works sensibly, but only occupies part of the total screen estate
in minicom. After quitting, minicom doesn't use the rest of the scree=
n
estate anymore.

After running vi, less doesn't show the weird scrolling behavior
anymore, since the console has just been limited to 24x80.

- Bone Black: Yocto poky, core-image-minimal
Login, "less file" locks up, doesn't show anything. I can exit using
Ctrl-C.

vi runs normally, only occupies part of the total screen estate in
minicom. After quitting, a weird character shows up (typically I see
=C3=BF there), but minicom can use the rest of the screen estate agai=
n.
If we disregard the odd character, this is much like the behavior we
have on the omap-serial driver.

- Custom board: Yocto poky, custom image
Login, "less file" locks up, showing only "=C3=BF" in the top left co=
rner
of the screen. Can get out of there by having something dumped throug=
h
/dev/kmsg.

vi: see "Bone Black: Yocto poky, core-image-minimal"

Having it summed up like this, I think we're back at ncurses and its
interaction with the serial driver.

Hope this helps. Thanks for your effort so far,
=46rans
--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-21 20:41:00 UTC
Permalink
Post by Frans Klaver
- Bone Black: Yocto poky, core-image-minimal
Login, "less file" locks up, doesn't show anything. I can exit using
Ctrl-C.
So I have the same with my and the serial-omap driver. No difference
here. The trace looks like this:
| <idle>-0 [000] d.h. 444.393585: serial8250_handle_irq=
: iir cc lsr 61
| <idle>-0 [000] d.h. 444.393605: serial8250_rx_chars: =
get 0d
received the enter key

| <idle>-0 [000] d.h. 444.393609: serial8250_rx_chars: =
insert d lsr 61
| <idle>-0 [000] d.h. 444.393614: uart_insert_char: 1
| <idle>-0 [000] d.h. 444.393617: uart_insert_char: 2
| <idle>-0 [000] dnh. 444.393636: serial8250_tx_chars: =
empty
| kworker/0:2-753 [000] d... 444.393686: serial8250_start_tx: =
empty?1
| kworker/0:2-753 [000] d.h. 444.393699: serial8250_handle_irq=
: iir c2 lsr 60
| kworker/0:2-753 [000] d.h. 444.393705: serial8250_tx_chars: =
empty
| sh-1042 [000] d... 444.393822: serial8250_start_tx: =
empty?1
| sh-1042 [000] d.h. 444.393836: serial8250_handle_irq=
: iir c2 lsr 60
| sh-1042 [000] d.h. 444.393842: serial8250_tx_chars: =
empty
| sh-1042 [000] d... 444.393855: serial8250_start_tx: =
empty?0
| sh-1042 [000] d.h. 444.393863: serial8250_handle_irq=
: iir c2 lsr 60
| sh-1042 [000] d.h. 444.393867: serial8250_tx_chars: =
put 0d
| sh-1042 [000] d.h. 444.393871: serial8250_tx_chars: =
put 0a

shell responded with "\r\n" which I see and then

| sh-1042 [000] d.h. 444.394057: serial8250_handle_irq=
: iir c2 lsr 60
| sh-1042 [000] d.h. 444.394065: serial8250_tx_chars: =
empty

nothing more. less isn't sending data for some reason. Exactly the same
thing happens in a Debian environment except that it continues:
=E2=80=A6
| bash-2468 [000] d.h. 99.657899: serial8250_tx_chars: =
put 0a
| bash-2468 [000] d.h. 99.658089: serial8250_handle_irq=
: iir c2 lsr 60
| bash-2468 [000] d.h. 99.658095: serial8250_tx_chars: =
empty
=3D>
| less-2474 [000] d... 99.696038: serial8250_start_tx: =
empty?0
| less-2474 [000] d.h. 99.696069: serial8250_handle_irq=
: iir c2 lsr 60
| less-2474 [000] d.h. 99.696078: serial8250_tx_chars: =
put 1b
| less-2474 [000] d.h. 99.696082: serial8250_tx_chars: =
put 5b
| less-2474 [000] d.h. 99.696085: serial8250_tx_chars: =
put 3f
| less-2474 [000] d.h. 99.696087: serial8250_tx_chars: =
put 31

It has to be something about the environment. Booting Debian and chroot
into this RFS and less works perfectly. But since it behaves like that
with both drivers, I guess the problem is somewhere else=E2=80=A6
Post by Frans Klaver
vi runs normally, only occupies part of the total screen estate in
minicom. After quitting, a weird character shows up (typically I see
=C3=BF there), but minicom can use the rest of the screen estate aga=
in.
Post by Frans Klaver
If we disregard the odd character, this is much like the behavior we
have on the omap-serial driver.
- Custom board: Yocto poky, custom image
Login, "less file" locks up, showing only "=C3=BF" in the top left c=
orner
Post by Frans Klaver
of the screen. Can get out of there by having something dumped throu=
gh
Post by Frans Klaver
/dev/kmsg.
I managed to run into something like that with vi on dra7 and with
little more patience on am335x as well by "vi *" and then ":n".

This gets fixed indeed by writing. Hours of debugging and a lot of hair
less later: the yocto RFS calls set_termios quite a lot. This includes
changing the baudrate (not by yocto but the driver sets it to 0 and the=
n
to the requested one) and this seems to be responsible for the "bad
bytes". I haven't figured out yet I don't see this with omap-serial.
Even worse: If this (set_termios()) happens while the DMA is still
active then it might stall it. A write into the FIFO seems to fix it an=
d
this is where your "echo >/dev/kmsg" fixes things.
If I delay the restore_registers part of set_termios() until TX-DMA is
complete then it seems that the TX-DMA stall does not tall anymore.
Post by Frans Klaver
Having it summed up like this, I think we're back at ncurses and its
interaction with the serial driver.
Hope this helps. Thanks for your effort so far,
Frans
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Frans Klaver
2014-09-22 09:28:54 UTC
Permalink
On Sun, Sep 21, 2014 at 10:41:00PM +0200, Sebastian Andrzej Siewior wro=
Post by Sebastian Andrzej Siewior
=20
Post by Frans Klaver
- Bone Black: Yocto poky, core-image-minimal
Login, "less file" locks up, doesn't show anything. I can exit usi=
ng
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
Ctrl-C.
=20
So I have the same with my and the serial-omap driver. No difference
| <idle>-0 [000] d.h. 444.393585: serial8250_handle_i=
rq: iir cc lsr 61
Post by Sebastian Andrzej Siewior
| <idle>-0 [000] d.h. 444.393605: serial8250_rx_chars=
: get 0d
Post by Sebastian Andrzej Siewior
received the enter key
=20
| <idle>-0 [000] d.h. 444.393609: serial8250_rx_chars=
: insert d lsr 61
Post by Sebastian Andrzej Siewior
| <idle>-0 [000] d.h. 444.393614: uart_insert_char: 1
| <idle>-0 [000] d.h. 444.393617: uart_insert_char: 2
| <idle>-0 [000] dnh. 444.393636: serial8250_tx_chars=
: empty
Post by Sebastian Andrzej Siewior
| kworker/0:2-753 [000] d... 444.393686: serial8250_start_tx=
: empty?1
Post by Sebastian Andrzej Siewior
| kworker/0:2-753 [000] d.h. 444.393699: serial8250_handle_i=
rq: iir c2 lsr 60
Post by Sebastian Andrzej Siewior
| kworker/0:2-753 [000] d.h. 444.393705: serial8250_tx_chars=
: empty
Post by Sebastian Andrzej Siewior
| sh-1042 [000] d... 444.393822: serial8250_start_tx=
: empty?1
Post by Sebastian Andrzej Siewior
| sh-1042 [000] d.h. 444.393836: serial8250_handle_i=
rq: iir c2 lsr 60
Post by Sebastian Andrzej Siewior
| sh-1042 [000] d.h. 444.393842: serial8250_tx_chars=
: empty
Post by Sebastian Andrzej Siewior
| sh-1042 [000] d... 444.393855: serial8250_start_tx=
: empty?0
Post by Sebastian Andrzej Siewior
| sh-1042 [000] d.h. 444.393863: serial8250_handle_i=
rq: iir c2 lsr 60
Post by Sebastian Andrzej Siewior
| sh-1042 [000] d.h. 444.393867: serial8250_tx_chars=
: put 0d
Post by Sebastian Andrzej Siewior
| sh-1042 [000] d.h. 444.393871: serial8250_tx_chars=
: put 0a
Post by Sebastian Andrzej Siewior
=20
shell responded with "\r\n" which I see and then
=20
| sh-1042 [000] d.h. 444.394057: serial8250_handle_i=
rq: iir c2 lsr 60
Post by Sebastian Andrzej Siewior
| sh-1042 [000] d.h. 444.394065: serial8250_tx_chars=
: empty
Post by Sebastian Andrzej Siewior
=20
nothing more. less isn't sending data for some reason. Exactly the sa=
me
Post by Sebastian Andrzej Siewior
=E2=80=A6
| bash-2468 [000] d.h. 99.657899: serial8250_tx_chars=
: put 0a
Post by Sebastian Andrzej Siewior
| bash-2468 [000] d.h. 99.658089: serial8250_handle_i=
rq: iir c2 lsr 60
Post by Sebastian Andrzej Siewior
| bash-2468 [000] d.h. 99.658095: serial8250_tx_chars=
: empty
Post by Sebastian Andrzej Siewior
=3D>
| less-2474 [000] d... 99.696038: serial8250_start_tx=
: empty?0
Post by Sebastian Andrzej Siewior
| less-2474 [000] d.h. 99.696069: serial8250_handle_i=
rq: iir c2 lsr 60
Post by Sebastian Andrzej Siewior
| less-2474 [000] d.h. 99.696078: serial8250_tx_chars=
: put 1b
Post by Sebastian Andrzej Siewior
| less-2474 [000] d.h. 99.696082: serial8250_tx_chars=
: put 5b
Post by Sebastian Andrzej Siewior
| less-2474 [000] d.h. 99.696085: serial8250_tx_chars=
: put 3f
Post by Sebastian Andrzej Siewior
| less-2474 [000] d.h. 99.696087: serial8250_tx_chars=
: put 31
Post by Sebastian Andrzej Siewior
=20
It has to be something about the environment. Booting Debian and chro=
ot
Post by Sebastian Andrzej Siewior
into this RFS and less works perfectly. But since it behaves like tha=
t
Post by Sebastian Andrzej Siewior
with both drivers, I guess the problem is somewhere else=E2=80=A6
=20
Post by Frans Klaver
vi runs normally, only occupies part of the total screen estate in
minicom. After quitting, a weird character shows up (typically I s=
ee
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
=C3=BF there), but minicom can use the rest of the screen estate a=
gain.
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
If we disregard the odd character, this is much like the behavior =
we
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
have on the omap-serial driver.
- Custom board: Yocto poky, custom image
Login, "less file" locks up, showing only "=C3=BF" in the top left=
corner
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
of the screen. Can get out of there by having something dumped thr=
ough
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
/dev/kmsg.
=20
I managed to run into something like that with vi on dra7 and with
little more patience on am335x as well by "vi *" and then ":n".
=20
This gets fixed indeed by writing. Hours of debugging and a lot of ha=
ir
Post by Sebastian Andrzej Siewior
less later: the yocto RFS calls set_termios quite a lot. This include=
s
Post by Sebastian Andrzej Siewior
changing the baudrate (not by yocto but the driver sets it to 0 and t=
hen
Post by Sebastian Andrzej Siewior
to the requested one) and this seems to be responsible for the "bad
bytes". I haven't figured out yet I don't see this with omap-serial.
Even worse: If this (set_termios()) happens while the DMA is still
active then it might stall it. A write into the FIFO seems to fix it =
and
Post by Sebastian Andrzej Siewior
this is where your "echo >/dev/kmsg" fixes things.
If I delay the restore_registers part of set_termios() until TX-DMA i=
s
Post by Sebastian Andrzej Siewior
complete then it seems that the TX-DMA stall does not tall anymore.
Wow, thanks for your work here. This does indeed sound hard to trap.

I guess then we'd still have to answer the question why the yocto build
calls set_termios() so often, but that's not on you then. Did you notic=
e
it even changing settings? We might want to fix this even if the kernel
should be able to cope.

Thanks again,
=46rans
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-24 07:56:51 UTC
Permalink
Post by Frans Klaver
Wow, thanks for your work here. This does indeed sound hard to trap.
I guess then we'd still have to answer the question why the yocto build
calls set_termios() so often, but that's not on you then. Did you notice
it even changing settings? We might want to fix this even if the kernel
should be able to cope.
Yeah. I don't care if this is yocto's fault or not. If it is possible to
stop the DMA transfer from userland with whatever method then it needs to
be fixed in kernel so that this does happen.
Post by Frans Klaver
Thanks again,
Frans
Sebastian
Sebastian Andrzej Siewior
2014-09-25 15:14:03 UTC
Permalink
Post by Frans Klaver
I guess then we'd still have to answer the question why the yocto build
calls set_termios() so often, but that's not on you then. Did you notice
it even changing settings? We might want to fix this even if the kernel
should be able to cope.
could you please test uart_v10_pre3? I dropped a few register sets and
delayed the register update until TX-DMA is complete. I am traveling
currently and have only my boneblack and it passes my limited testing.
If it solves your problems, too then I would address Heikki's latest
comments and prepare the final v10 (and I hope I didn't break beagle
board in between).
Post by Frans Klaver
Thanks again,
Frans
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Frans Klaver
2014-09-25 15:18:15 UTC
Permalink
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
I guess then we'd still have to answer the question why the yocto
build
Post by Frans Klaver
calls set_termios() so often, but that's not on you then. Did you
notice
Post by Frans Klaver
it even changing settings? We might want to fix this even if the
kernel
Post by Frans Klaver
should be able to cope.
could you please test uart_v10_pre3? I dropped a few register sets and
delayed the register update until TX-DMA is complete. I am traveling
currently and have only my boneblack and it passes my limited testing.
If it solves your problems, too then I would address Heikki's latest
comments and prepare the final v10 (and I hope I didn't break beagle
board in between).
That'll be Monday earliest for me.
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
Thanks again,
Frans
Sebastian
_______________________________________________
linux-arm-kernel mailing list
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Frans Klaver
2014-09-29 08:50:42 UTC
Permalink
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
I guess then we'd still have to answer the question why the yocto build
calls set_termios() so often, but that's not on you then. Did you notice
it even changing settings? We might want to fix this even if the kernel
should be able to cope.
could you please test uart_v10_pre3? I dropped a few register sets and
delayed the register update until TX-DMA is complete. I am traveling
currently and have only my boneblack and it passes my limited testing.
If it solves your problems, too then I would address Heikki's latest
comments and prepare the final v10 (and I hope I didn't break beagle
board in between).
This version fixes the console things for us. It also increases the
amount of data we can push over the serial port. If I push the data
towards our requirements, we're not there yet. I get the "too much work
for irq" notice still. However, I don't think you'd need to be fixing
that in this series (or at all). We had similar issues there with
omap-serial as well.

As far as I'm concerned, this is

Tested-by: Frans Klaver <***@xsens.com>

Thanks,
Frans
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-29 09:54:40 UTC
Permalink
Post by Frans Klaver
This version fixes the console things for us. It also increases the
amount of data we can push over the serial port. If I push the data
towards our requirements, we're not there yet. I get the "too much work
for irq" notice still. However, I don't think you'd need to be fixing
that in this series (or at all). We had similar issues there with
omap-serial as well.
As far as I'm concerned, this is
Thanks a lot.
There is a patch named "ARM: edma: unconditionally ack the error
interrupt". I have the feeling that this is not really required once we
delay set_termios. I couldn't reproduce the bug with beagleblack with my
usual test case.

For your "too much work for irq" problem: Could you add trace_printk()
in tx/rx dma start/complete, and irq routine? The interresting part is
what is the irq routine doing once entered. It might be a condition that
is ignored at first and "acked" later while serving another event. Or it
is really doing something and this is more or less "legal".
Post by Frans Klaver
Thanks,
Frans
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Frans Klaver
2014-09-29 10:30:43 UTC
Permalink
Post by Sebastian Andrzej Siewior
There is a patch named "ARM: edma: unconditionally ack the error
interrupt". I have the feeling that this is not really required once we
delay set_termios. I couldn't reproduce the bug with beagleblack with my
usual test case.
I think so too. I didn't need it either.
Post by Sebastian Andrzej Siewior
For your "too much work for irq" problem: Could you add trace_printk()
in tx/rx dma start/complete, and irq routine? The interresting part is
what is the irq routine doing once entered. It might be a condition that
is ignored at first and "acked" later while serving another event. Or it
is really doing something and this is more or less "legal".
I'll have a look at that.
Post by Sebastian Andrzej Siewior
Post by Heikki Krogerus
Thanks,
Frans
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Frans Klaver
2014-09-30 08:44:16 UTC
Permalink
Post by Sebastian Andrzej Siewior
For your "too much work for irq" problem: Could you add trace_printk()
in tx/rx dma start/complete, and irq routine? The interresting part is
what is the irq routine doing once entered. It might be a condition that
is ignored at first and "acked" later while serving another event. Or it
is really doing something and this is more or less "legal".
Here's some trace output I get. I hope branches become clear by the
calls they do.

uart_test-482 [000] .ns. 17.860139: __dma_rx_do_complete: error: 0, uart_bug_dma: 32
uart_test-482 [000] .ns. 17.860141: __dma_rx_do_complete: rx_dma(p, 0)
uart_test-480 [000] ..s. 17.860260: __dma_rx_do_complete: error: 0, uart_bug_dma: 32
uart_test-480 [000] ..s. 17.860263: __dma_rx_do_complete: rx_dma(p, 0)
uart_test-478 [000] ..s. 17.860369: __dma_rx_do_complete: error: 0, uart_bug_dma: 32
uart_test-478 [000] ..s. 17.860372: __dma_rx_do_complete: rx_dma(p, 0)
kworker/0:1-10 [000] ..s. 17.860508: __dma_rx_do_complete: error: 0, uart_bug_dma: 32
kworker/0:1-10 [000] ..s. 17.860512: __dma_rx_do_complete: rx_dma(p, 0)
uart_test-477 [000] ..s. 17.860634: __dma_rx_do_complete: error: 0, uart_bug_dma: 32
uart_test-477 [000] ..s. 17.860642: __dma_rx_do_complete: rx_dma(p, 0)
uart_test-477 [000] ..s. 17.860768: __dma_rx_do_complete: error: 0, uart_bug_dma: 32
uart_test-477 [000] ..s. 17.860772: __dma_rx_do_complete: rx_dma(p, 0)
kworker/0:1-10 [000] ..s. 17.860900: __dma_rx_do_complete: error: 0, uart_bug_dma: 32
kworker/0:1-10 [000] ..s. 17.860904: __dma_rx_do_complete: rx_dma(p, 0)
uart_test-483 [000] dnh. 17.861018: serial8250_interrupt: irq 89
uart_test-483 [000] dnh. 17.861026: serial8250_interrupt: 89 e
uart_test-483 [000] .ns. 17.861045: __dma_rx_do_complete: error: 0, uart_bug_dma: 32
uart_test-483 [000] .ns. 17.861047: __dma_rx_do_complete: rx_dma(p, 0)
uart_test-479 [000] d.H. 17.861124: serial8250_interrupt: irq 89
uart_test-479 [000] d.H. 17.861133: serial8250_handle_irq: l1 IIR cc LSR 61
uart_test-479 [000] d.H. 17.861135: serial8250_handle_irq: rx_dma(up, iir)
uart_test-479 [000] d.H. 17.861139: serial8250_rx_dma: l1, UART_IIR_RX_TIMEOUT
uart_test-479 [000] d.H. 17.861147: __dma_rx_do_complete: error: 1, uart_bug_dma: 32
uart_test-479 [000] d.H. 17.861150: serial8250_handle_irq: rx_chars(up, status)
uart_test-479 [000] d.H. 17.861190: serial8250_handle_irq: rx_dma(up, 0)
uart_test-479 [000] d.H. 17.861205: serial8250_interrupt: 89 e
kworker/0:1-10 [000] dnH. 17.864949: serial8250_interrupt: irq 89

So far so good. We're just entered interrupt where stuff goes awry. The
following pattern repeats over 600 times:

kworker/0:1-10 [000] dnH. 17.865198: serial8250_handle_irq: l1 IIR cc LSR 61
kworker/0:1-10 [000] dnH. 17.865254: serial8250_handle_irq: rx_dma(up, iir)
kworker/0:1-10 [000] dnH. 17.865333: serial8250_rx_dma: l1, UART_IIR_RX_TIMEOUT
kworker/0:1-10 [000] dnH. 17.865626: __dma_rx_do_complete: error: 1, uart_bug_dma: 32
kworker/0:1-10 [000] dnH. 17.865747: serial8250_handle_irq: rx_chars(up, status)
kworker/0:1-10 [000] dnH. 17.868797: serial8250_handle_irq: rx_dma(up, 0)

ending with:

kworker/0:1-10 [000] dnH. 20.027093: serial8250_interrupt: serial8250: too much work for irq89
kworker/0:1-10 [000] dnH. 20.027181: serial8250_interrupt: 89 e

This clogs the entire system until I disconnect the communication lines.

So we get an RX timeout. The receiver fifo trigger level was not reached
and 8250_core is left to copy the remaining data. I would expect that if
the trigger level wasn't reached, we wouldn't need to be doing this that
often. On the other hand, could we be trapped reading data from rx
without dma helping us? And how could we resolve this?

Frans
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-10-02 10:27:23 UTC
Permalink
Post by Frans Klaver
On Mon, Sep 29, 2014 at 11:54:40AM +0200, Sebastian Andrzej Siewior =
For your "too much work for irq" problem: Could you add trace_prin=
tk()
Post by Frans Klaver
in tx/rx dma start/complete, and irq routine? The interresting par=
t is
Post by Frans Klaver
what is the irq routine doing once entered. It might be a conditio=
n that
Post by Frans Klaver
is ignored at first and "acked" later while serving another event.=
Or it
Post by Frans Klaver
is really doing something and this is more or less "legal".
=20
Here's some trace output I get. I hope branches become clear by the
calls they do.
uart_test-482 [000] .ns. 17.860139: __dma_rx_do_complete: =
error: 0, uart_bug_dma: 32
Post by Frans Klaver
uart_test-482 [000] .ns. 17.860141: __dma_rx_do_complete: =
rx_dma(p, 0)

these two happen outside the IRQ routine for every 48 bytes.
=E2=80=A6
Post by Frans Klaver
uart_test-483 [000] dnh. 17.861018: serial8250_interrupt: =
irq 89
Post by Frans Klaver
uart_test-483 [000] dnh. 17.861026: serial8250_interrupt: =
89 e
e? Did was the routine invoked 0xe times or this a register?
Post by Frans Klaver
uart_test-483 [000] .ns. 17.861045: __dma_rx_do_complete: =
error: 0, uart_bug_dma: 32
Post by Frans Klaver
uart_test-483 [000] .ns. 17.861047: __dma_rx_do_complete: =
rx_dma(p, 0)
another 48bytes
Post by Frans Klaver
uart_test-479 [000] d.H. 17.861124: serial8250_interrupt: =
irq 89
Post by Frans Klaver
uart_test-479 [000] d.H. 17.861133: serial8250_handle_irq:=
l1 IIR cc LSR 61
Post by Frans Klaver
uart_test-479 [000] d.H. 17.861135: serial8250_handle_irq:=
rx_dma(up, iir)
Post by Frans Klaver
uart_test-479 [000] d.H. 17.861139: serial8250_rx_dma: l1,=
UART_IIR_RX_TIMEOUT
Post by Frans Klaver
uart_test-479 [000] d.H. 17.861147: __dma_rx_do_complete: =
error: 1, uart_bug_dma: 32
Post by Frans Klaver
uart_test-479 [000] d.H. 17.861150: serial8250_handle_irq:=
rx_chars(up, status)
Post by Frans Klaver
uart_test-479 [000] d.H. 17.861190: serial8250_handle_irq:=
rx_dma(up, 0)
timeout, manual purge. Do you have an idea how many bytes were manually
received?
Post by Frans Klaver
uart_test-479 [000] d.H. 17.861205: serial8250_interrupt: =
89 e
Post by Frans Klaver
kworker/0:1-10 [000] dnH. 17.864949: serial8250_interrupt: =
irq 89
Post by Frans Klaver
So far so good. We're just entered interrupt where stuff goes awry. Th=
e
Post by Frans Klaver
kworker/0:1-10 [000] dnH. 17.865198: serial8250_handle_irq:=
l1 IIR cc LSR 61
Post by Frans Klaver
kworker/0:1-10 [000] dnH. 17.865254: serial8250_handle_irq:=
rx_dma(up, iir)
Post by Frans Klaver
kworker/0:1-10 [000] dnH. 17.865333: serial8250_rx_dma: l1,=
UART_IIR_RX_TIMEOUT
Post by Frans Klaver
kworker/0:1-10 [000] dnH. 17.865626: __dma_rx_do_complete: =
error: 1, uart_bug_dma: 32
Post by Frans Klaver
kworker/0:1-10 [000] dnH. 17.865747: serial8250_handle_irq:=
rx_chars(up, status)
Post by Frans Klaver
kworker/0:1-10 [000] dnH. 17.868797: serial8250_handle_irq:=
rx_dma(up, 0)
Post by Frans Klaver
kworker/0:1-10 [000] dnH. 20.027093: serial8250_interrupt: =
serial8250: too much work for irq89
Post by Frans Klaver
kworker/0:1-10 [000] dnH. 20.027181: serial8250_interrupt: =
89 e
Post by Frans Klaver
This clogs the entire system until I disconnect the communication line=
s.
Post by Frans Klaver
So we get an RX timeout. The receiver fifo trigger level was not reach=
ed
Post by Frans Klaver
and 8250_core is left to copy the remaining data. I would expect that =
if
Post by Frans Klaver
the trigger level wasn't reached, we wouldn't need to be doing this th=
at
Post by Frans Klaver
often. On the other hand, could we be trapped reading data from rx
without dma helping us? And how could we resolve this?
So if I understand you correct, then you enter serial8250_interrupt()
and then you enter multiple times omap_8250_dma_handle_irq() and
you always get a TIMEOUT event and fetch byte(s) manualy via
serial8250_rx_chars(). And after one iteration UART_IIR_NO_INT is not
set and you do it again, right?

I have no idea when exactly the timeout-interrupt fires. The manual
says: "=E2=80=A6 the count is reset when there is activity on uarti_rx =
=E2=80=A6" but it
does not say how often it increments before the level is reached.
It also might be that you have a small gap between two bytes and this
high baud rate the gap is considered as a timeout event.

Another thing could be that if the we enqueue a RX transfer from the
dma_completion callback then we have too many bytes in the FIFO already
becahse the callback is invoked from softirq. What happens if we cut th=
e
middle man via


diff --git a/drivers/dma/virt-dma.c b/drivers/dma/virt-dma.c
index 6f80432..21b04bd 100644
--- a/drivers/dma/virt-dma.c
+++ b/drivers/dma/virt-dma.c
@@ -63,8 +63,9 @@ static void vchan_complete(unsigned long arg)
dma_async_tx_callback cb =3D NULL;
void *cb_data =3D NULL;
LIST_HEAD(head);
+ unsigned long flags;
=20
- spin_lock_irq(&vc->lock);
+ spin_lock_irqsave(&vc->lock, flags);
list_splice_tail_init(&vc->desc_completed, &head);
vd =3D vc->cyclic;
if (vd) {
@@ -72,7 +73,7 @@ static void vchan_complete(unsigned long arg)
cb =3D vd->tx.callback;
cb_data =3D vd->tx.callback_param;
}
- spin_unlock_irq(&vc->lock);
+ spin_unlock_irqrestore(&vc->lock, flags);
=20
if (cb)
cb(cb_data);
diff --git a/drivers/dma/virt-dma.h b/drivers/dma/virt-dma.h
index 181b952..7632338 100644
--- a/drivers/dma/virt-dma.h
+++ b/drivers/dma/virt-dma.h
@@ -92,7 +92,10 @@ static inline void vchan_cookie_complete(struct virt=
_dma_desc *vd)
vd, cookie);
list_add_tail(&vd->node, &vc->desc_completed);
=20
- tasklet_schedule(&vc->task);
+ if (vd->tx.my_uart)
+ vc->task.func(vc);
+ else
+ tasklet_schedule(&vc->task);
}
=20
/**
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8=
250/8250_omap.c
index 57a8b12..5d7ee92 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -728,6 +728,7 @@ static int omap_8250_rx_dma(struct uart_8250_port *=
p, unsigned int iir)
dma->rx_running =3D 1;
desc->callback =3D __dma_rx_complete;
desc->callback_param =3D p;
+ desc->my_uart =3D 1;
=20
dma->rx_cookie =3D dmaengine_submit(desc);
=20
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 1f9e642..0f5fbe1 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -459,6 +459,7 @@ struct dmaengine_unmap_data {
struct dma_async_tx_descriptor {
dma_cookie_t cookie;
enum dma_ctrl_flags flags; /* not a 'long' to pack with cookie */
+ u32 my_uart;
dma_addr_t phys;
struct dma_chan *chan;
dma_cookie_t (*tx_submit)(struct dma_async_tx_descriptor *tx);
Post by Frans Klaver
Frans
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Frans Klaver
2014-10-13 14:55:34 UTC
Permalink
On Thu, Oct 02, 2014 at 12:27:23PM +0200, Sebastian Andrzej Siewior wro=
Post by Sebastian Andrzej Siewior
=20
uart_test-483 [000] dnh. 17.861018: serial8250_interrupt=
: irq 89
Post by Sebastian Andrzej Siewior
uart_test-483 [000] dnh. 17.861026: serial8250_interrupt=
: 89 e
Post by Sebastian Andrzej Siewior
e? Did was the routine invoked 0xe times or this a register?
No, this marks the end of serial8250_interrupt().
Post by Sebastian Andrzej Siewior
uart_test-479 [000] d.H. 17.861124: serial8250_interrupt=
: irq 89
Post by Sebastian Andrzej Siewior
uart_test-479 [000] d.H. 17.861133: serial8250_handle_ir=
q: l1 IIR cc LSR 61
Post by Sebastian Andrzej Siewior
uart_test-479 [000] d.H. 17.861135: serial8250_handle_ir=
q: rx_dma(up, iir)
Post by Sebastian Andrzej Siewior
uart_test-479 [000] d.H. 17.861139: serial8250_rx_dma: l=
1, UART_IIR_RX_TIMEOUT
Post by Sebastian Andrzej Siewior
uart_test-479 [000] d.H. 17.861147: __dma_rx_do_complete=
: error: 1, uart_bug_dma: 32
Post by Sebastian Andrzej Siewior
uart_test-479 [000] d.H. 17.861150: serial8250_handle_ir=
q: rx_chars(up, status)
Post by Sebastian Andrzej Siewior
uart_test-479 [000] d.H. 17.861190: serial8250_handle_ir=
q: rx_dma(up, 0)
Post by Sebastian Andrzej Siewior
timeout, manual purge. Do you have an idea how many bytes were manual=
ly
Post by Sebastian Andrzej Siewior
received?
Here's an excerpt from a new trial using v10
(previous was v10_pre3). We manually read 10 bytes before we start
trying to get all bytes without dma.=20

uart_test-483 [000] dnH. 18.182647: serial8250_interrupt: i=
rq 89
uart_test-483 [000] dnH. 18.182653: omap_8250_dma_handle_ir=
q: rx_dma(up, iir)
uart_test-483 [000] dnH. 18.182655: omap_8250_rx_dma: UART_=
IIR_RX_TIMEOUT
uart_test-483 [000] dnH. 18.182657: omap_8250_rx_dma: rx wa=
s running
uart_test-483 [000] dnH. 18.182662: __dma_rx_do_complete: c=
ount =3D 0
uart_test-483 [000] dnH. 18.182666: omap_8250_dma_handle_ir=
q: rx_chars(up, status)
uart_test-483 [000] dnH. 18.182683: serial8250_rx_chars: by=
tes read =3D 10
uart_test-483 [000] dnH. 18.182685: omap_8250_dma_handle_ir=
q: rx_dma(up, 0)
uart_test-483 [000] dnH. 18.182698: omap_8250_dma_handle_ir=
q: rpm_put(up)
uart_test-483 [000] dnH. 18.182701: serial8250_interrupt: 8=
9 end
uart_test-488 [000] ..s. 18.185353: __dma_rx_do_complete: c=
ount =3D 48
uart_test-488 [000] ..s. 18.185366: __dma_rx_do_complete: r=
x_dma(p, 0)
kworker/0:1-10 [000] d.H. 18.185486: serial8250_interrupt: i=
rq 89
kworker/0:1-10 [000] d.H. 18.185496: omap_8250_dma_handle_ir=
q: rpm_put(up)
kworker/0:1-10 [000] d.H. 18.185501: serial8250_interrupt: 8=
9 end
kworker/0:1-10 [000] ..s. 18.185520: __dma_rx_do_complete: c=
ount =3D 48
kworker/0:1-10 [000] ..s. 18.185526: __dma_rx_do_complete: r=
x_dma(p, 0)

Below things go downhill again.

kworker/0:1-10 [000] ..s. 18.227273: __dma_rx_do_complete: r=
x_dma(p, 0)
uart_test-483 [000] ..s. 18.227396: __dma_rx_do_complete: c=
ount =3D 48
uart_test-483 [000] ..s. 18.227404: __dma_rx_do_complete: r=
x_dma(p, 0)
uart_test-485 [000] .ns. 18.227540: __dma_rx_do_complete: c=
ount =3D 48
uart_test-485 [000] .ns. 18.227547: __dma_rx_do_complete: r=
x_dma(p, 0)
kworker/0:2-65 [000] ..s. 18.227660: __dma_rx_do_complete: c=
ount =3D 48
kworker/0:2-65 [000] ..s. 18.227667: __dma_rx_do_complete: r=
x_dma(p, 0)
kworker/0:1-10 [000] .ns. 18.227786: __dma_rx_do_complete: c=
ount =3D 48
kworker/0:1-10 [000] .ns. 18.227793: __dma_rx_do_complete: r=
x_dma(p, 0)
uart_test-477 [000] ..s. 18.227921: __dma_rx_do_complete: c=
ount =3D 48
uart_test-477 [000] ..s. 18.227928: __dma_rx_do_complete: r=
x_dma(p, 0)
uart_test-481 [000] ..s. 18.228051: __dma_rx_do_complete: c=
ount =3D 48
uart_test-481 [000] ..s. 18.228057: __dma_rx_do_complete: r=
x_dma(p, 0)
kworker/0:1-10 [000] ..s. 18.228185: __dma_rx_do_complete: c=
ount =3D 48
kworker/0:1-10 [000] ..s. 18.228191: __dma_rx_do_complete: r=
x_dma(p, 0)
uart_test-485 [000] ..s. 18.228318: __dma_rx_do_complete: c=
ount =3D 48
uart_test-485 [000] ..s. 18.228324: __dma_rx_do_complete: r=
x_dma(p, 0)
uart_test-488 [000] d.h. 18.228445: serial8250_interrupt: i=
rq 89
uart_test-488 [000] d.h. 18.228454: omap_8250_dma_handle_ir=
q: rpm_put(up)
uart_test-488 [000] d.h. 18.228459: serial8250_interrupt: 8=
9 end
uart_test-488 [000] d.H. 18.228472: serial8250_interrupt: i=
rq 89
uart_test-488 [000] d.H. 18.228477: omap_8250_dma_handle_ir=
q: rx_dma(up, iir)
uart_test-488 [000] d.H. 18.228479: omap_8250_rx_dma: UART_=
IIR_RX_TIMEOUT
uart_test-488 [000] d.H. 18.228480: omap_8250_rx_dma: rx wa=
s running
uart_test-488 [000] d.H. 18.228487: __dma_rx_do_complete: c=
ount =3D 48
uart_test-488 [000] d.H. 18.228500: omap_8250_dma_handle_ir=
q: rx_chars(up, status)
uart_test-488 [000] d.H. 18.228517: serial8250_rx_chars: by=
tes read =3D 10
uart_test-488 [000] d.H. 18.228519: omap_8250_dma_handle_ir=
q: rx_dma(up, 0)
uart_test-488 [000] d.H. 18.228534: omap_8250_dma_handle_ir=
q: rpm_put(up)
uart_test-488 [000] d.H. 18.228540: serial8250_interrupt: 8=
9 end
kworker/0:1-10 [000] dnh. 18.234930: serial8250_interrupt: i=
rq 89
kworker/0:1-10 [000] dnh. 18.235074: omap_8250_dma_handle_ir=
q: rx_dma(up, iir)
kworker/0:1-10 [000] dnh. 18.235123: omap_8250_rx_dma: UART_=
IIR_RX_TIMEOUT
kworker/0:1-10 [000] dnh. 18.235159: omap_8250_rx_dma: rx wa=
s running
kworker/0:1-10 [000] dnh. 18.235334: __dma_rx_do_complete: c=
ount =3D 48
kworker/0:1-10 [000] dnh. 18.235455: omap_8250_dma_handle_ir=
q: rx_chars(up, status)
kworker/0:1-10 [000] dnh. 18.238523: serial8250_rx_chars: by=
tes read =3D 256
kworker/0:1-10 [000] dnh. 18.238566: omap_8250_dma_handle_ir=
q: rx_dma(up, 0)
kworker/0:1-10 [000] dnh. 18.239169: omap_8250_dma_handle_ir=
q: rx_dma(up, iir)
kworker/0:1-10 [000] dnh. 18.239223: omap_8250_rx_dma: UART_=
IIR_RX_TIMEOUT
kworker/0:1-10 [000] dnh. 18.239256: omap_8250_rx_dma: rx wa=
s running
kworker/0:1-10 [000] dnh. 18.239377: __dma_rx_do_complete: c=
ount =3D 0
kworker/0:1-10 [000] dnh. 18.239449: omap_8250_dma_handle_ir=
q: rx_chars(up, status)
kworker/0:1-10 [000] dnh. 18.242576: serial8250_rx_chars: by=
tes read =3D 256
kworker/0:1-10 [000] dnh. 18.242620: omap_8250_dma_handle_ir=
q: rx_dma(up, 0)
kworker/0:1-10 [000] dnh. 18.242878: omap_8250_dma_handle_ir=
q: rx_dma(up, iir)
kworker/0:1-10 [000] dnh. 18.242921: omap_8250_rx_dma: UART_=
IIR_RX_TIMEOUT
kworker/0:1-10 [000] dnh. 18.242954: omap_8250_rx_dma: rx wa=
s running
kworker/0:1-10 [000] dnh. 18.243064: __dma_rx_do_complete: c=
ount =3D 0
kworker/0:1-10 [000] dnh. 18.243127: omap_8250_dma_handle_ir=
q: rx_chars(up, status)
kworker/0:1-10 [000] dnh. 18.246238: serial8250_rx_chars: by=
tes read =3D 256
kworker/0:1-10 [000] dnh. 18.246283: omap_8250_dma_handle_ir=
q: rx_dma(up, 0)
kworker/0:1-10 [000] dnh. 18.246524: omap_8250_dma_handle_ir=
q: rx_dma(up, iir)
kworker/0:1-10 [000] dnh. 18.246563: omap_8250_rx_dma: UART_=
IIR_RX_TIMEOUT
kworker/0:1-10 [000] dnh. 18.246599: omap_8250_rx_dma: rx wa=
s running
kworker/0:1-10 [000] dnh. 18.246702: __dma_rx_do_complete: c=
ount =3D 0
Post by Sebastian Andrzej Siewior
kworker/0:1-10 [000] dnH. 20.027093: serial8250_interrupt=
: serial8250: too much work for irq89
Post by Sebastian Andrzej Siewior
kworker/0:1-10 [000] dnH. 20.027181: serial8250_interrupt=
: 89 e
Post by Sebastian Andrzej Siewior
This clogs the entire system until I disconnect the communication li=
nes.
Post by Sebastian Andrzej Siewior
So we get an RX timeout. The receiver fifo trigger level was not rea=
ched
Post by Sebastian Andrzej Siewior
and 8250_core is left to copy the remaining data. I would expect tha=
t if
Post by Sebastian Andrzej Siewior
the trigger level wasn't reached, we wouldn't need to be doing this =
that
Post by Sebastian Andrzej Siewior
often. On the other hand, could we be trapped reading data from rx
without dma helping us? And how could we resolve this?
=20
So if I understand you correct, then you enter serial8250_interrupt()
and then you enter multiple times omap_8250_dma_handle_irq() and
you always get a TIMEOUT event and fetch byte(s) manualy via
serial8250_rx_chars(). And after one iteration UART_IIR_NO_INT is not
set and you do it again, right?
That's what it looks like, yes.
Post by Sebastian Andrzej Siewior
I have no idea when exactly the timeout-interrupt fires. The manual
says: "=E2=80=A6 the count is reset when there is activity on uarti_r=
x =E2=80=A6" but it
Post by Sebastian Andrzej Siewior
does not say how often it increments before the level is reached.
It also might be that you have a small gap between two bytes and this
high baud rate the gap is considered as a timeout event.
=20
Another thing could be that if the we enqueue a RX transfer from the
dma_completion callback then we have too many bytes in the FIFO alrea=
dy
Post by Sebastian Andrzej Siewior
becahse the callback is invoked from softirq. What happens if we cut =
the
Post by Sebastian Andrzej Siewior
middle man via
=20
=20
diff --git a/drivers/dma/virt-dma.c b/drivers/dma/virt-dma.c
index 6f80432..21b04bd 100644
--- a/drivers/dma/virt-dma.c
+++ b/drivers/dma/virt-dma.c
@@ -63,8 +63,9 @@ static void vchan_complete(unsigned long arg)
dma_async_tx_callback cb =3D NULL;
void *cb_data =3D NULL;
LIST_HEAD(head);
+ unsigned long flags;
=20
- spin_lock_irq(&vc->lock);
+ spin_lock_irqsave(&vc->lock, flags);
list_splice_tail_init(&vc->desc_completed, &head);
vd =3D vc->cyclic;
if (vd) {
@@ -72,7 +73,7 @@ static void vchan_complete(unsigned long arg)
cb =3D vd->tx.callback;
cb_data =3D vd->tx.callback_param;
}
- spin_unlock_irq(&vc->lock);
+ spin_unlock_irqrestore(&vc->lock, flags);
=20
if (cb)
cb(cb_data);
diff --git a/drivers/dma/virt-dma.h b/drivers/dma/virt-dma.h
index 181b952..7632338 100644
--- a/drivers/dma/virt-dma.h
+++ b/drivers/dma/virt-dma.h
@@ -92,7 +92,10 @@ static inline void vchan_cookie_complete(struct vi=
rt_dma_desc *vd)
Post by Sebastian Andrzej Siewior
vd, cookie);
list_add_tail(&vd->node, &vc->desc_completed);
=20
- tasklet_schedule(&vc->task);
+ if (vd->tx.my_uart)
+ vc->task.func(vc);
+ else
+ tasklet_schedule(&vc->task);
}
=20
/**
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial=
/8250/8250_omap.c
Post by Sebastian Andrzej Siewior
index 57a8b12..5d7ee92 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -728,6 +728,7 @@ static int omap_8250_rx_dma(struct uart_8250_port=
*p, unsigned int iir)
Post by Sebastian Andrzej Siewior
dma->rx_running =3D 1;
desc->callback =3D __dma_rx_complete;
desc->callback_param =3D p;
+ desc->my_uart =3D 1;
=20
dma->rx_cookie =3D dmaengine_submit(desc);
=20
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 1f9e642..0f5fbe1 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -459,6 +459,7 @@ struct dmaengine_unmap_data {
struct dma_async_tx_descriptor {
dma_cookie_t cookie;
enum dma_ctrl_flags flags; /* not a 'long' to pack with cookie */
+ u32 my_uart;
dma_addr_t phys;
struct dma_chan *chan;
dma_cookie_t (*tx_submit)(struct dma_async_tx_descriptor *tx);
=20
[ 251.363111] BUG: spinlock recursion on CPU#0, swapper/0
[ 251.368394] lock: 0xcf151dc4, .magic: dead4ead, .owner: swapper/0, =
=2Eowner_cpu: 0
[ 251.375920] CPU: 0 PID: 0 Comm: swapper Not tainted 3.17.0-rc5-00141=
-geccc467-dirty #35
[ 251.384012] [<c0013454>] (unwind_backtrace) from [<c00113b0>] (show_=
stack+0x10/0x14)
[ 251.391811] [<c00113b0>] (show_stack) from [<c0055cdc>] (do_raw_spin=
_lock+0x48/0x110)
[ 251.399703] [<c0055cdc>] (do_raw_spin_lock) from [<c04dbef0>] (_raw_=
spin_lock_irqsave+0x10/0x18)
[ 251.408544] [<c04dbef0>] (_raw_spin_lock_irqsave) from [<c0221f74>] =
(vchan_complete+0x1c/0xe8)
[ 251.417207] [<c0221f74>] (vchan_complete) from [<c0222f54>] (edma_ca=
llback+0x138/0x274)
[ 251.425259] [<c0222f54>] (edma_callback) from [<c001a844>] (dma_irq_=
handler+0x154/0x190)
[ 251.433406] [<c001a844>] (dma_irq_handler) from [<c005b2dc>] (handle=
_irq_event_percpu+0x2c/0x130)
[ 251.442331] [<c005b2dc>] (handle_irq_event_percpu) from [<c005b41c>]=
(handle_irq_event+0x3c/0x5c)
[ 251.451254] [<c005b41c>] (handle_irq_event) from [<c005d940>] (handl=
e_level_irq+0xd0/0x108)
[ 251.459655] [<c005d940>] (handle_level_irq) from [<c005acc0>] (gener=
ic_handle_irq+0x20/0x30)
[ 251.468144] [<c005acc0>] (generic_handle_irq) from [<c000ee2c>] (han=
dle_IRQ+0x60/0x80)
[ 251.476109] [<c000ee2c>] (handle_IRQ) from [<c0008654>] (omap3_intc_=
handle_irq+0x64/0x8c)
[ 251.484337] [<c0008654>] (omap3_intc_handle_irq) from [<c0011dc0>] (=
__irq_svc+0x40/0x54)
[ 251.492466] Exception stack(0xc0725f68 to 0xc0725fb0)
[ 251.497549] 5f60: 00000000 c0757160 00000000 c0018=
fc0 c0724000 c0724038
[ 251.505772] 5f80: c078de40 c072c040 c0714124 cfefc4c0 00000000 00000=
000 c0757160 c0725fb0
[ 251.513989] 5fa0: c000ef40 c000ef44 60000013 ffffffff
[ 251.519077] [<c0011dc0>] (__irq_svc) from [<c000ef44>] (arch_cpu_idl=
e+0x2c/0x38)
[ 251.526517] [<c000ef44>] (arch_cpu_idle) from [<c005255c>] (cpu_star=
tup_entry+0x9c/0xf0)
[ 251.534664] [<c005255c>] (cpu_startup_entry) from [<c06e5b34>] (star=
t_kernel+0x308/0x360)

I should probably take out the spin_lock calls if vd->tx.my_uart is 1?

Thanks,
=46rans
--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Peter Hurley
2014-09-23 17:03:51 UTC
Permalink
Post by Sebastian Andrzej Siewior
=20
Post by Frans Klaver
- Bone Black: Yocto poky, core-image-minimal
Login, "less file" locks up, doesn't show anything. I can exit usin=
g
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
Ctrl-C.
=20
So I have the same with my and the serial-omap driver. No difference
| <idle>-0 [000] d.h. 444.393585: serial8250_handle_i=
rq: iir cc lsr 61
Post by Sebastian Andrzej Siewior
| <idle>-0 [000] d.h. 444.393605: serial8250_rx_chars=
: get 0d
Post by Sebastian Andrzej Siewior
received the enter key
=20
| <idle>-0 [000] d.h. 444.393609: serial8250_rx_chars=
: insert d lsr 61
Post by Sebastian Andrzej Siewior
| <idle>-0 [000] d.h. 444.393614: uart_insert_char: 1
| <idle>-0 [000] d.h. 444.393617: uart_insert_char: 2
| <idle>-0 [000] dnh. 444.393636: serial8250_tx_chars=
: empty
Post by Sebastian Andrzej Siewior
| kworker/0:2-753 [000] d... 444.393686: serial8250_start_tx=
: empty?1
Post by Sebastian Andrzej Siewior
| kworker/0:2-753 [000] d.h. 444.393699: serial8250_handle_i=
rq: iir c2 lsr 60
Post by Sebastian Andrzej Siewior
| kworker/0:2-753 [000] d.h. 444.393705: serial8250_tx_chars=
: empty
Post by Sebastian Andrzej Siewior
| sh-1042 [000] d... 444.393822: serial8250_start_tx=
: empty?1
Post by Sebastian Andrzej Siewior
| sh-1042 [000] d.h. 444.393836: serial8250_handle_i=
rq: iir c2 lsr 60
Post by Sebastian Andrzej Siewior
| sh-1042 [000] d.h. 444.393842: serial8250_tx_chars=
: empty
Post by Sebastian Andrzej Siewior
| sh-1042 [000] d... 444.393855: serial8250_start_tx=
: empty?0
Post by Sebastian Andrzej Siewior
| sh-1042 [000] d.h. 444.393863: serial8250_handle_i=
rq: iir c2 lsr 60
Post by Sebastian Andrzej Siewior
| sh-1042 [000] d.h. 444.393867: serial8250_tx_chars=
: put 0d
Post by Sebastian Andrzej Siewior
| sh-1042 [000] d.h. 444.393871: serial8250_tx_chars=
: put 0a
Post by Sebastian Andrzej Siewior
=20
shell responded with "\r\n" which I see and then
=20
| sh-1042 [000] d.h. 444.394057: serial8250_handle_i=
rq: iir c2 lsr 60
Post by Sebastian Andrzej Siewior
| sh-1042 [000] d.h. 444.394065: serial8250_tx_chars=
: empty
Post by Sebastian Andrzej Siewior
=20
nothing more. less isn't sending data for some reason. Exactly the sa=
me
Post by Sebastian Andrzej Siewior
=E2=80=A6
| bash-2468 [000] d.h. 99.657899: serial8250_tx_chars=
: put 0a
Post by Sebastian Andrzej Siewior
| bash-2468 [000] d.h. 99.658089: serial8250_handle_i=
rq: iir c2 lsr 60
Post by Sebastian Andrzej Siewior
| bash-2468 [000] d.h. 99.658095: serial8250_tx_chars=
: empty
Post by Sebastian Andrzej Siewior
=3D>
| less-2474 [000] d... 99.696038: serial8250_start_tx=
: empty?0
Post by Sebastian Andrzej Siewior
| less-2474 [000] d.h. 99.696069: serial8250_handle_i=
rq: iir c2 lsr 60
Post by Sebastian Andrzej Siewior
| less-2474 [000] d.h. 99.696078: serial8250_tx_chars=
: put 1b
Post by Sebastian Andrzej Siewior
| less-2474 [000] d.h. 99.696082: serial8250_tx_chars=
: put 5b
Post by Sebastian Andrzej Siewior
| less-2474 [000] d.h. 99.696085: serial8250_tx_chars=
: put 3f
Post by Sebastian Andrzej Siewior
| less-2474 [000] d.h. 99.696087: serial8250_tx_chars=
: put 31
Post by Sebastian Andrzej Siewior
=20
It has to be something about the environment. Booting Debian and chro=
ot
Post by Sebastian Andrzej Siewior
into this RFS and less works perfectly. But since it behaves like tha=
t
Post by Sebastian Andrzej Siewior
with both drivers, I guess the problem is somewhere else=E2=80=A6
=20
Post by Frans Klaver
vi runs normally, only occupies part of the total screen estate in
minicom. After quitting, a weird character shows up (typically I se=
e
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
=C3=BF there), but minicom can use the rest of the screen estate ag=
ain.
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
If we disregard the odd character, this is much like the behavior w=
e
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
have on the omap-serial driver.
- Custom board: Yocto poky, custom image
Login, "less file" locks up, showing only "=C3=BF" in the top left =
corner
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
of the screen. Can get out of there by having something dumped thro=
ugh
Post by Sebastian Andrzej Siewior
Post by Frans Klaver
/dev/kmsg.
=20
I managed to run into something like that with vi on dra7 and with
little more patience on am335x as well by "vi *" and then ":n".
=20
This gets fixed indeed by writing. Hours of debugging and a lot of ha=
ir
Post by Sebastian Andrzej Siewior
less later: the yocto RFS calls set_termios quite a lot.
readline() does this; it 'saves' the caller's termios, sets termios
for non-canonical reads, reads one char, and 'restores' the caller's
termios.
Post by Sebastian Andrzej Siewior
This includes
changing the baudrate (not by yocto but the driver sets it to 0 and t=
hen
Post by Sebastian Andrzej Siewior
to the requested one) and this seems to be responsible for the "bad
bytes". I haven't figured out yet I don't see this with omap-serial.
Even worse: If this (set_termios()) happens while the DMA is still
active then it might stall it. A write into the FIFO seems to fix it =
and
Post by Sebastian Andrzej Siewior
this is where your "echo >/dev/kmsg" fixes things.
If I delay the restore_registers part of set_termios() until TX-DMA i=
s
Post by Sebastian Andrzej Siewior
complete then it seems that the TX-DMA stall does not tall anymore.
The tty core calls the driver's wait_until_sent() method before changin=
g
the termios (if TCSADRAIN is used for tcsetattr(), which I think for re=
adline()
it does).

But DMA is cheating if the UART driver's tx_empty() method is saying th=
e
transmitter is empty while TX DMA is still running.

Regards,
Peter Hurley
=20
Sebastian Andrzej Siewior
2014-09-24 07:53:46 UTC
Permalink
Post by Peter Hurley
readline() does this; it 'saves' the caller's termios, sets termios
for non-canonical reads, reads one char, and 'restores' the caller's
termios.
interresting, thanks. I guess I would need to opimize this a little so
the baudrate isn't going to 0 and back to the requested baudrate.
Post by Peter Hurley
The tty core calls the driver's wait_until_sent() method before changi=
ng
Post by Peter Hurley
the termios (if TCSADRAIN is used for tcsetattr(), which I think for r=
eadline()
Post by Peter Hurley
it does).
The interresting difference is that when I take the yocto RFS and chroo=
t
into from Debian then I don't this problem. Not sure if this is really
readline or something else=E2=80=A6
Post by Peter Hurley
But DMA is cheating if the UART driver's tx_empty() method is saying t=
he
Post by Peter Hurley
transmitter is empty while TX DMA is still running.
This shouldn't be the case. But I will check this once I able to.
After TX-DMA is done, "xmit->tail" is updated and port.icount.tx is
incremented. At this time the TX FIFO is still full (up to 64 bytes) an=
d
I set UART_IER_THRI to wait until TX FIFO is empty so I can disable
runtime-pm. Therefore I would assume LSR does not say BOTH_EMPTY until
the FIFO is empty.
Post by Peter Hurley
Regards,
Peter Hurley
Sebastian
Sebastian Andrzej Siewior
2014-09-25 10:42:08 UTC
Permalink
Post by Sebastian Andrzej Siewior
But DMA is cheating if the UART driver's tx_empty() method is saying the
transmitter is empty while TX DMA is still running.
This shouldn't be the case. But I will check this once I able to.
I added

|#define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)
| trace_printk("delay <%d>\n", (lsr & BOTH_EMPTY) == BOTH_EMPTY ? 1 : 0);

in my set_termios() and the trace shows
| vi-949 [000] d... 70.477002: omap8250_restore_regs: delay <0>

so no, it does not wait until TX FIFO is empty. It looks like it uses
TCSANOW instead of TCSADRAIN. And since this looks "legal" I will delay
it until TX-DMA is complete because it is known to stall the DMA operation.
Post by Sebastian Andrzej Siewior
Regards,
Peter Hurley
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Peter Hurley
2014-09-25 11:31:32 UTC
Permalink
Post by Sebastian Andrzej Siewior
Post by Sebastian Andrzej Siewior
But DMA is cheating if the UART driver's tx_empty() method is saying the
transmitter is empty while TX DMA is still running.
This shouldn't be the case. But I will check this once I able to.
I added
|#define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)
| trace_printk("delay <%d>\n", (lsr & BOTH_EMPTY) == BOTH_EMPTY ? 1 : 0);
in my set_termios() and the trace shows
| vi-949 [000] d... 70.477002: omap8250_restore_regs: delay <0>
so no, it does not wait until TX FIFO is empty. It looks like it uses
TCSANOW instead of TCSADRAIN. And since this looks "legal" I will delay
it until TX-DMA is complete because it is known to stall the DMA operation.
I just verified that GNU readline6 uses ioctl(TCSETSW, ...) to do the set_termios
(which is the ioctl that libc should use for tcsetattr(TCSADRAIN)).

Maybe this userspace is using a readline()-alike that has a bug by not using the
correct tcsetattr() action?
Or maybe the glibc-equivalent has the bug, and tcsetattr(TCSADRAIN) is not using
ioctl(TCSETSW)?

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-25 13:11:37 UTC
Permalink
I just verified that GNU readline6 uses ioctl(TCSETSW, ...) to do the =
set_termios
(which is the ioctl that libc should use for tcsetattr(TCSADRAIN)).
Maybe this userspace is using a readline()-alike that has a bug by not=
using the
correct tcsetattr() action?
set_termios() has an opt argument. While doing ":n" in vi I see two inv=
ocations
with "opt =3D=3D 8" which stands for TCSETS.
browsing through vi's code I stumbled upon=20

|static void rawmode(void)
|{
=E2=80=A6
| tcsetattr_stdin_TCSANOW(&term_vi);
|}
|int FAST_FUNC tcsetattr_stdin_TCSANOW(const struct termios *tp)
|{
| return tcsetattr(STDIN_FILENO, TCSANOW, tp);
|}

and this is probably what you meant. There is also
| static void cookmode(void)
| {
| fflush_all();
| tcsetattr_stdin_TCSANOW(&term_orig);
| }

However I don't see __tty_perform_flush() in kernel invoked.
Or maybe the glibc-equivalent has the bug, and tcsetattr(TCSADRAIN) is=
not using
ioctl(TCSETSW)?
libc is "GNU C Library 2.20".=20
Regards,
Peter Hurley
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-17 16:34:45 UTC
Permalink
Post by Sebastian Andrzej Siewior
We should add hooks like tx_dma and rx_dma to struct uart_8250_dma s=
o
Post by Sebastian Andrzej Siewior
that the probe drivers can replace serial8250_tx_dma and
seria8250_rx_dma, like I think Alan already suggested.
=20
Okay. Wasn't aware that Alan already suggested that.
I also need a watchdog timer for TX since it seems that on omap3 the
DMA engine suddenly forgets to continue with DMA=E2=80=A6
=20
If this is really what we want, I would need to refactor a few things=
=E2=80=A6
Post by Sebastian Andrzej Siewior
=20
Let's keep serial8250_tx_dma/rx_dma as the default, and not add any
quirks to them. Only if there is a very common case should it be
handled in those. The case of RX req needing to be sent before data =
in
Post by Sebastian Andrzej Siewior
FIFO maybe one of those, but I'm no sure.
=20
keep in mind that both (RX & TX bugs/hacks) need also a bit of handli=
ng
Post by Sebastian Andrzej Siewior
in the 8250-core so it works together (like the tx_err member so we
fall back to manual xmit)
Done. I've kept the RX workarounds in the 8250_dma and moved the TX
part into the omap driver.
I needed to add the 8250_core pieces of patch #10 [0]. Now If you say,
couldn't this done in an other way then I could move the RX workarounds
pieces to the omap driver as well as the interrupt routine. Any
preferences?

[0] [PATCH 10/16] tty: serial: 8250_dma: optimize the xmit path due to
UART_BUG_DMA_TX

Sebastian

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Heikki Krogerus
2014-09-19 10:22:12 UTC
Permalink
On Wed, Sep 17, 2014 at 06:34:45PM +0200, Sebastian Andrzej Siewior wro=
Post by Sebastian Andrzej Siewior
Post by Sebastian Andrzej Siewior
We should add hooks like tx_dma and rx_dma to struct uart_8250_dma=
so
Post by Sebastian Andrzej Siewior
Post by Sebastian Andrzej Siewior
that the probe drivers can replace serial8250_tx_dma and
seria8250_rx_dma, like I think Alan already suggested.
=20
Okay. Wasn't aware that Alan already suggested that.
I also need a watchdog timer for TX since it seems that on omap3 th=
e
Post by Sebastian Andrzej Siewior
Post by Sebastian Andrzej Siewior
DMA engine suddenly forgets to continue with DMA=E2=80=A6
=20
If this is really what we want, I would need to refactor a few thin=
gs=E2=80=A6
Post by Sebastian Andrzej Siewior
Post by Sebastian Andrzej Siewior
=20
Let's keep serial8250_tx_dma/rx_dma as the default, and not add an=
y
Post by Sebastian Andrzej Siewior
Post by Sebastian Andrzej Siewior
quirks to them. Only if there is a very common case should it be
handled in those. The case of RX req needing to be sent before dat=
a in
Post by Sebastian Andrzej Siewior
Post by Sebastian Andrzej Siewior
FIFO maybe one of those, but I'm no sure.
=20
keep in mind that both (RX & TX bugs/hacks) need also a bit of hand=
ling
Post by Sebastian Andrzej Siewior
Post by Sebastian Andrzej Siewior
in the 8250-core so it works together (like the tx_err member so we
fall back to manual xmit)
=20
Done. I've kept the RX workarounds in the 8250_dma and moved the TX
part into the omap driver.
I needed to add the 8250_core pieces of patch #10 [0]. Now If you say=
,
Post by Sebastian Andrzej Siewior
couldn't this done in an other way then I could move the RX workaroun=
ds
Post by Sebastian Andrzej Siewior
pieces to the omap driver as well as the interrupt routine. Any
preferences?
=20
[0] [PATCH 10/16] tty: serial: 8250_dma: optimize the xmit path due t=
o
Post by Sebastian Andrzej Siewior
UART_BUG_DMA_TX
Couldn't you just replace the handle_irq with a custom irq routine in
the omap driver like you did with set_termios? Where you would do
those tricks and/or call serial8250_handle_irq()?

The 8250_core changes in that patch #10 only modify
serial8250_handle_irg right?


Cheers,

--=20
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-19 10:58:44 UTC
Permalink
Post by Heikki Krogerus
Couldn't you just replace the handle_irq with a custom irq routine in
the omap driver like you did with set_termios? Where you would do
those tricks and/or call serial8250_handle_irq()?
Tricks within serial8250_handle_irq(), see [0]. It is not a lot but
still. I could provide my own handle irq, just asking what you would
prefer.

[0]
http://git.breakpoint.cc/cgit/bigeasy/linux.git/commit/?h=uart_v10_pre2&id=f26f161d998ee4a84a0aa6ddff69a435c25f204d
Post by Heikki Krogerus
The 8250_core changes in that patch #10 only modify
serial8250_handle_irg right?
Correct. However there is another change due to the RX_DMA_BUG. A while
ago you said that this RX_DMA_BUG thing might be something that other
SoC could use, too.
If you ask me now for my own irq routine it would make sense to move RX
bug handling into omap specific code as well.
Post by Heikki Krogerus
Cheers,
Sebastian
Peter Hurley
2014-09-19 11:25:24 UTC
Permalink
Post by Sebastian Andrzej Siewior
Post by Heikki Krogerus
Couldn't you just replace the handle_irq with a custom irq routine in
the omap driver like you did with set_termios? Where you would do
those tricks and/or call serial8250_handle_irq()?
Tricks within serial8250_handle_irq(), see [0]. It is not a lot but
still. I could provide my own handle irq, just asking what you would
prefer.
[0]
http://git.breakpoint.cc/cgit/bigeasy/linux.git/commit/?h=uart_v10_pre2&id=f26f161d998ee4a84a0aa6ddff69a435c25f204d
Post by Heikki Krogerus
The 8250_core changes in that patch #10 only modify
serial8250_handle_irg right?
Correct. However there is another change due to the RX_DMA_BUG. A while
ago you said that this RX_DMA_BUG thing might be something that other
SoC could use, too.
If you ask me now for my own irq routine it would make sense to move RX
bug handling into omap specific code as well.
I'm not excited at the prospect of an omap-specific irq handler, especially
if this is just a silicon bug. I think it will create and encourage
unnecessary code variation in the 8250 driver. The inertia of an
omap-specific irq handler will mean it will probable stay forever. Suppose
this tx dma bug is later discovered to be fixable inline (rather than by
state), then we'll be stuck with an irq handler that no one will want to
integrate.

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Heikki Krogerus
2014-09-22 07:46:05 UTC
Permalink
Post by Sebastian Andrzej Siewior
Post by Heikki Krogerus
Couldn't you just replace the handle_irq with a custom irq routine in
the omap driver like you did with set_termios? Where you would do
those tricks and/or call serial8250_handle_irq()?
Tricks within serial8250_handle_irq(), see [0]. It is not a lot but
still. I could provide my own handle irq, just asking what you would
prefer.
[0]
http://git.breakpoint.cc/cgit/bigeasy/linux.git/commit/?h=uart_v10_pre2&id=f26f161d998ee4a84a0aa6ddff69a435c25f204d
Post by Heikki Krogerus
The 8250_core changes in that patch #10 only modify
serial8250_handle_irg right?
Correct. However there is another change due to the RX_DMA_BUG. A while
ago you said that this RX_DMA_BUG thing might be something that other
SoC could use, too.
If you ask me now for my own irq routine it would make sense to move RX
bug handling into omap specific code as well.
Well, there are no other SoCs at the moment that would need it, and
it's actually possible that there never will be. So yes, just handle
that also in the omap specific code.


Thanks,
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-25 09:24:46 UTC
Permalink
Post by Heikki Krogerus
Well, there are no other SoCs at the moment that would need it, and
it's actually possible that there never will be. So yes, just handle
that also in the omap specific code.
Okay. So let me move the irq routine and the workarounds into omap then.
Post by Heikki Krogerus
Thanks,
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-10 19:29:57 UTC
Permalink
While comparing the OMAP-serial and the 8250 part of this I noticed that
the latter does not use run time-pm. Here are the pieces. It is
basically a get before first register access and a last_busy + put after
last access. This has to be enabled from userland _and_ UART_CAP_RPM is
required for this.
The runtime PM can usually work transparently in the background however
there is one exception to this: After serial8250_tx_chars() completes
there still may be unsent bytes in the FIFO (depending on CPU speed vs
baud rate + flow control). Even if the TTY-buffer is empty we do not
want RPM to disable the device because it won't send the remaining
bytes. Instead we leave serial8250_tx_chars() with RPM enabled and wait
for the FIFO empty interrupt. Once we enter serial8250_tx_chars() with
an empty buffer we know that the FIFO is empty and since we are not going
to send anything, we can disable the device.
That xchg() is to ensure that serial8250_tx_chars() can be called
multiple times and only the first invocation will actually invoke the
runtime PM function. So that the last invocation of __stop_tx() will
disable runtime pm.

NOTE: do not enable RPM on the device unless you know what you do! If
the device goes idle, it won't be woken up by incomming RX data _unless_
there is a wakeup irq configured which is usually the RX pin configure
for wakeup via the reset module. The RX activity will then wake up the
device from idle. However the first character is garbage and lost. The
following bytes will be received once the device is up in time. On the
beagle board xm (omap3) it takes approx 13ms from the first wakeup byte
until the first byte that is received properly if the device was in
core-off.

v5…v8:
- drop RPM from serial8250_set_mctrl() it will be used in
restore path which already has RPM active and holds
dev->power.lock
v4…v5:
- add a wrapper around rpm function and introduce UART_CAP_RPM
to ensure RPM put is invoked after the TX FIFO is empty.
v3…v4:
- added runtime to the console code
- removed device_may_wakeup() from serial8250_set_sleep()

Cc: ***@linux.intel.com
Reviewed-by: Tony Lindgren <***@atomide.com>
Tested-by: Tony Lindgren <***@atomide.com>
Signed-off-by: Sebastian Andrzej Siewior <***@linutronix.de>
---
drivers/tty/serial/8250/8250.h | 1 +
drivers/tty/serial/8250/8250_core.c | 133 ++++++++++++++++++++++++++++++++----
include/linux/serial_8250.h | 1 +
3 files changed, 122 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 85bfec58d77c..1bcb4b2141a6 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -72,6 +72,7 @@ struct serial8250_config {
#define UART_CAP_UUE (1 << 12) /* UART needs IER bit 6 set (Xscale) */
#define UART_CAP_RTOIE (1 << 13) /* UART needs IER bit 4 set (Xscale, Tegra) */
#define UART_CAP_HFIFO (1 << 14) /* UART has a "hidden" FIFO */
+#define UART_CAP_RPM (1 << 15) /* Runtime PM is active while idle */

#define UART_BUG_QUOT (1 << 0) /* UART has buggy quot LSB */
#define UART_BUG_TXEN (1 << 1) /* UART has buggy TX IIR status */
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 68c44d97091b..3cf5c98013e4 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -38,6 +38,7 @@
#include <linux/mutex.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
+#include <linux/pm_runtime.h>
#ifdef CONFIG_SPARC
#include <linux/sunserialcore.h>
#endif
@@ -540,6 +541,53 @@ void serial8250_clear_and_reinit_fifos(struct uart_8250_port *p)
}
EXPORT_SYMBOL_GPL(serial8250_clear_and_reinit_fifos);

+static void serial8250_rpm_get(struct uart_8250_port *p)
+{
+ if (!(p->capabilities & UART_CAP_RPM))
+ return;
+ pm_runtime_get_sync(p->port.dev);
+}
+
+static void serial8250_rpm_put(struct uart_8250_port *p)
+{
+ if (!(p->capabilities & UART_CAP_RPM))
+ return;
+ pm_runtime_mark_last_busy(p->port.dev);
+ pm_runtime_put_autosuspend(p->port.dev);
+}
+
+/*
+ * This two wrapper ensure, that enable_runtime_pm_tx() can be called more than
+ * once and disable_runtime_pm_tx() will still disable RPM because the fifo is
+ * empty and the HW can idle again.
+ */
+static void serial8250_rpm_get_tx(struct uart_8250_port *p)
+{
+ unsigned char rpm_active;
+
+ if (!(p->capabilities & UART_CAP_RPM))
+ return;
+
+ rpm_active = xchg(&p->rpm_tx_active, 1);
+ if (rpm_active)
+ return;
+ pm_runtime_get_sync(p->port.dev);
+}
+
+static void serial8250_rpm_put_tx(struct uart_8250_port *p)
+{
+ unsigned char rpm_active;
+
+ if (!(p->capabilities & UART_CAP_RPM))
+ return;
+
+ rpm_active = xchg(&p->rpm_tx_active, 0);
+ if (!rpm_active)
+ return;
+ pm_runtime_mark_last_busy(p->port.dev);
+ pm_runtime_put_autosuspend(p->port.dev);
+}
+
/*
* IER sleep support. UARTs which have EFRs need the "extended
* capability" bit enabled. Note that on XR16C850s, we need to
@@ -554,10 +602,11 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep)
* offset but the UART channel may only write to the corresponding
* bit.
*/
+ serial8250_rpm_get(p);
if ((p->port.type == PORT_XR17V35X) ||
(p->port.type == PORT_XR17D15X)) {
serial_out(p, UART_EXAR_SLEEP, sleep ? 0xff : 0);
- return;
+ goto out;
}

if (p->capabilities & UART_CAP_SLEEP) {
@@ -573,6 +622,8 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep)
serial_out(p, UART_LCR, 0);
}
}
+out:
+ serial8250_rpm_put(p);
}

#ifdef CONFIG_SERIAL_8250_RSA
@@ -1273,6 +1324,7 @@ static inline void __stop_tx(struct uart_8250_port *p)
if (p->ier & UART_IER_THRI) {
p->ier &= ~UART_IER_THRI;
serial_out(p, UART_IER, p->ier);
+ serial8250_rpm_put_tx(p);
}
}

@@ -1280,6 +1332,7 @@ static void serial8250_stop_tx(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);

+ serial8250_rpm_get(up);
__stop_tx(up);

/*
@@ -1289,12 +1342,14 @@ static void serial8250_stop_tx(struct uart_port *port)
up->acr |= UART_ACR_TXDIS;
serial_icr_write(up, UART_ACR, up->acr);
}
+ serial8250_rpm_put(up);
}

static void serial8250_start_tx(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);

+ serial8250_rpm_get_tx(up);
if (up->dma && !serial8250_tx_dma(up)) {
return;
} else if (!(up->ier & UART_IER_THRI)) {
@@ -1333,9 +1388,13 @@ static void serial8250_stop_rx(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);

+ serial8250_rpm_get(up);
+
up->ier &= ~UART_IER_RLSI;
up->port.read_status_mask &= ~UART_LSR_DR;
serial_port_out(port, UART_IER, up->ier);
+
+ serial8250_rpm_put(up);
}

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

up->ier |= UART_IER_MSI;
+
+ serial8250_rpm_get(up);
serial_port_out(port, UART_IER, up->ier);
+ serial8250_rpm_put(up);
}

/*
@@ -1469,7 +1531,12 @@ void serial8250_tx_chars(struct uart_8250_port *up)

DEBUG_INTR("THRE...");

- if (uart_circ_empty(xmit))
+ /*
+ * With RPM enabled, we have to wait once the FIFO is empty before the
+ * HW can go idle. So we get here once again with empty FIFO and disable
+ * the interrupt and RPM in __stop_tx()
+ */
+ if (uart_circ_empty(xmit) && !(up->capabilities & UART_CAP_RPM))
__stop_tx(up);
}
EXPORT_SYMBOL_GPL(serial8250_tx_chars);
@@ -1537,9 +1604,17 @@ EXPORT_SYMBOL_GPL(serial8250_handle_irq);

static int serial8250_default_handle_irq(struct uart_port *port)
{
- unsigned int iir = serial_port_in(port, UART_IIR);
+ struct uart_8250_port *up = up_to_u8250p(port);
+ unsigned int iir;
+ int ret;
+
+ serial8250_rpm_get(up);

- return serial8250_handle_irq(port, iir);
+ iir = serial_port_in(port, UART_IIR);
+ ret = serial8250_handle_irq(port, iir);
+
+ serial8250_rpm_put(up);
+ return ret;
}

/*
@@ -1796,11 +1871,15 @@ static unsigned int serial8250_tx_empty(struct uart_port *port)
unsigned long flags;
unsigned int lsr;

+ serial8250_rpm_get(up);
+
spin_lock_irqsave(&port->lock, flags);
lsr = serial_port_in(port, UART_LSR);
up->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
spin_unlock_irqrestore(&port->lock, flags);

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

@@ -1810,7 +1889,9 @@ static unsigned int serial8250_get_mctrl(struct uart_port *port)
unsigned int status;
unsigned int ret;

+ serial8250_rpm_get(up);
status = serial8250_modem_status(up);
+ serial8250_rpm_put(up);

ret = 0;
if (status & UART_MSR_DCD)
@@ -1850,6 +1931,7 @@ static void serial8250_break_ctl(struct uart_port *port, int break_state)
struct uart_8250_port *up = up_to_u8250p(port);
unsigned long flags;

+ serial8250_rpm_get(up);
spin_lock_irqsave(&port->lock, flags);
if (break_state == -1)
up->lcr |= UART_LCR_SBC;
@@ -1857,6 +1939,7 @@ static void serial8250_break_ctl(struct uart_port *port, int break_state)
up->lcr &= ~UART_LCR_SBC;
serial_port_out(port, UART_LCR, up->lcr);
spin_unlock_irqrestore(&port->lock, flags);
+ serial8250_rpm_put(up);
}

/*
@@ -1901,12 +1984,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);
+ struct uart_8250_port *up = up_to_u8250p(port);
+ unsigned char lsr;
+ int status;
+
+ serial8250_rpm_get(up);

- if (!(lsr & UART_LSR_DR))
- return NO_POLL_CHAR;
+ lsr = serial_port_in(port, UART_LSR);

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


@@ -1916,6 +2010,7 @@ static void serial8250_put_poll_char(struct uart_port *port,
unsigned int ier;
struct uart_8250_port *up = up_to_u8250p(port);

+ serial8250_rpm_get(up);
/*
* First save the IER then disable the interrupts
*/
@@ -1937,6 +2032,7 @@ static void serial8250_put_poll_char(struct uart_port *port,
*/
wait_for_xmitr(up, BOTH_EMPTY);
serial_port_out(port, UART_IER, ier);
+ serial8250_rpm_put(up);
}

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

+ serial8250_rpm_get(up);
if (port->type == PORT_16C950) {
/* Wake up and initialize UART */
up->acr = 0;
@@ -1982,7 +2079,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())
@@ -2006,7 +2102,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;
}

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

/*
@@ -2189,8 +2286,10 @@ int serial8250_do_startup(struct uart_port *port)
outb_p(0x80, icp);
inb_p(icp);
}
-
- return 0;
+ retval = 0;
+out:
+ serial8250_rpm_put(up);
+ return retval;
}
EXPORT_SYMBOL_GPL(serial8250_do_startup);

@@ -2206,6 +2305,7 @@ void serial8250_do_shutdown(struct uart_port *port)
struct uart_8250_port *up = up_to_u8250p(port);
unsigned long flags;

+ serial8250_rpm_get(up);
/*
* Disable interrupts from this port
*/
@@ -2245,6 +2345,7 @@ void serial8250_do_shutdown(struct uart_port *port)
* the IRQ chain.
*/
serial_port_in(port, UART_RX);
+ serial8250_rpm_put(up);

del_timer_sync(&up->timer);
up->timer.function = serial8250_timeout;
@@ -2360,6 +2461,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
* Ok, we're now changing the port state. Do it with
* interrupts disabled.
*/
+ serial8250_rpm_get(up);
spin_lock_irqsave(&port->lock, flags);

/*
@@ -2481,6 +2583,8 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
}
serial8250_set_mctrl(port, port->mctrl);
spin_unlock_irqrestore(&port->lock, flags);
+ serial8250_rpm_put(up);
+
/* Don't rewrite B0 */
if (tty_termios_baud_rate(termios))
tty_termios_encode_baud_rate(termios, baud, baud);
@@ -3091,6 +3195,8 @@ serial8250_console_write(struct console *co, const char *s, unsigned int count)

touch_nmi_watchdog();

+ serial8250_rpm_get(up);
+
if (port->sysrq || oops_in_progress)
locked = spin_trylock_irqsave(&port->lock, flags);
else
@@ -3127,6 +3233,7 @@ serial8250_console_write(struct console *co, const char *s, unsigned int count)

if (locked)
spin_unlock_irqrestore(&port->lock, flags);
+ serial8250_rpm_put(up);
}

static int __init serial8250_console_setup(struct console *co, char *options)
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 6fc9d7bee05e..c267412a3ef4 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -84,6 +84,7 @@ struct uart_8250_port {
unsigned char mcr_mask; /* mask of user bits */
unsigned char mcr_force; /* mask of forced bits */
unsigned char cur_iotype; /* Running I/O type */
+ unsigned char rpm_tx_active;

/*
* Some bits in registers are cleared on a read, so they must
--
2.1.0
Frans Klaver
2014-09-29 09:46:20 UTC
Permalink
Post by Sebastian Andrzej Siewior
+/*
+ * This two wrapper ensure, that enable_runtime_pm_tx() can be called more than
These two wrappers ensure that enable_runtime_pm_tx() ...
Post by Sebastian Andrzej Siewior
+ * once and disable_runtime_pm_tx() will still disable RPM because the fifo is
+ * empty and the HW can idle again.
+ */
+static void serial8250_rpm_get_tx(struct uart_8250_port *p)
+{
+ unsigned char rpm_active;
+
+ if (!(p->capabilities & UART_CAP_RPM))
+ return;
+
+ rpm_active = xchg(&p->rpm_tx_active, 1);
+ if (rpm_active)
+ return;
+ pm_runtime_get_sync(p->port.dev);
+}
+
+static void serial8250_rpm_put_tx(struct uart_8250_port *p)
+{
+ unsigned char rpm_active;
+
+ if (!(p->capabilities & UART_CAP_RPM))
+ return;
+
+ rpm_active = xchg(&p->rpm_tx_active, 0);
+ if (!rpm_active)
+ return;
+ pm_runtime_mark_last_busy(p->port.dev);
+ pm_runtime_put_autosuspend(p->port.dev);
+}
+
@@ -1469,7 +1531,12 @@ void serial8250_tx_chars(struct uart_8250_port *up)
DEBUG_INTR("THRE...");
- if (uart_circ_empty(xmit))
+ /*
+ * With RPM enabled, we have to wait once the FIFO is empty before the
s,once,until,? Or do I not understand the sentence correctly?
Post by Sebastian Andrzej Siewior
+ * HW can go idle. So we get here once again with empty FIFO and disable
+ * the interrupt and RPM in __stop_tx()
+ */
+ if (uart_circ_empty(xmit) && !(up->capabilities & UART_CAP_RPM))
__stop_tx(up);
}
EXPORT_SYMBOL_GPL(serial8250_tx_chars);
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-29 13:39:18 UTC
Permalink
Post by Frans Klaver
Post by Sebastian Andrzej Siewior
+/*
+ * This two wrapper ensure, that enable_runtime_pm_tx() can be called more than
These two wrappers ensure that enable_runtime_pm_tx() ...
fixed
Post by Frans Klaver
Post by Sebastian Andrzej Siewior
@@ -1469,7 +1531,12 @@ void serial8250_tx_chars(struct uart_8250_port *up)
DEBUG_INTR("THRE...");
- if (uart_circ_empty(xmit))
+ /*
+ * With RPM enabled, we have to wait once the FIFO is empty before the
s,once,until,? Or do I not understand the sentence correctly?
No, you understand it correct. "until" is the better word here.

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-10 19:30:11 UTC
Permalink
This patch adds the required pieces to 8250-OMAP UART driver for DMA
support. The TX burst size is set to 1 so we can send an arbitrary
amount of bytes.

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

On AM375x there is "Usage Note 2.7: UART: Cannot Acknowledge Idle
Requests in Smartidle Mode When Configured for DMA Operations" in the
errata document. This problem persists even after disabling DMA in the
UART and will be addressed in the HWMOD.

Reviewed-by: Tony Lindgren <***@atomide.com>
Tested-by: Tony Lindgren <***@atomide.com>
Signed-off-by: Sebastian Andrzej Siewior <***@linutronix.de>
---
drivers/tty/serial/8250/8250_omap.c | 49 +++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 2a187b00ed0a..5312c9e7ab15 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -366,6 +366,10 @@ static void omap_8250_set_termios(struct uart_port *port,
priv->scr = OMAP_UART_SCR_RX_TRIG_GRANU1_MASK | OMAP_UART_SCR_TX_EMPTY |
OMAP_UART_SCR_TX_TRIG_GRANU1_MASK;

+ if (up->dma)
+ priv->scr |= OMAP_UART_SCR_DMAMODE_1 |
+ OMAP_UART_SCR_DMAMODE_CTL;
+
priv->xon = termios->c_cc[VSTART];
priv->xoff = termios->c_cc[VSTOP];

@@ -548,6 +552,11 @@ static int omap_8250_startup(struct uart_port *port)
priv->wer |= OMAP_UART_TX_WAKEUP_EN;
serial_out(up, UART_OMAP_WER, priv->wer);

+ if (up->dma) {
+ serial8250_rx_dma(up, 0);
+ priv->dma_active = true;
+ }
+
pm_runtime_mark_last_busy(port->dev);
pm_runtime_put_autosuspend(port->dev);
return 0;
@@ -566,6 +575,10 @@ static void omap_8250_shutdown(struct uart_port *port)
struct omap8250_priv *priv = port->private_data;

flush_work(&priv->qos_work);
+ if (up->dma) {
+ serial8250_rx_dma(up, UART_IIR_RX_TIMEOUT);
+ priv->dma_active = false;
+ }

pm_runtime_get_sync(port->dev);

@@ -613,6 +626,13 @@ static void omap_8250_unthrottle(struct uart_port *port)
pm_runtime_put_autosuspend(port->dev);
}

+#ifdef CONFIG_SERIAL_8250_DMA
+static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
+{
+ return false;
+}
+#endif
+
static int omap8250_probe(struct platform_device *pdev)
{
struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -712,6 +732,30 @@ static int omap8250_probe(struct platform_device *pdev)
pm_runtime_get_sync(&pdev->dev);

omap_serial_fill_features_erratas(&up, priv);
+#ifdef CONFIG_SERIAL_8250_DMA
+ if (pdev->dev.of_node) {
+ /*
+ * Oh DMA support. If there are no DMA properties in the DT then
+ * we will fall back to a generic DMA channel which does not
+ * really work here. To ensure that we do not get a generic DMA
+ * channel assigned, we have the the_no_dma_filter_fn() here.
+ * To avoid "failed to request DMA" messages we check for DMA
+ * properties in DT.
+ */
+ ret = of_property_count_strings(pdev->dev.of_node, "dma-names");
+ if (ret == 2) {
+ up.dma = &priv->omap8250_dma;
+ priv->omap8250_dma.fn = the_no_dma_filter_fn;
+ priv->omap8250_dma.rx_size = RX_TRIGGER;
+ priv->omap8250_dma.rxconf.src_maxburst = RX_TRIGGER;
+ priv->omap8250_dma.txconf.dst_maxburst = TX_TRIGGER;
+
+ if (of_machine_is_compatible("ti,am33xx"))
+ up.bugs |= UART_BUG_DMA_TX;
+ up.bugs |= UART_BUG_DMA_RX;
+ }
+ }
+#endif
ret = serial8250_register_8250_port(&up);
if (ret < 0) {
dev_err(&pdev->dev, "unable to register 8250 port\n");
@@ -848,6 +892,8 @@ static int omap8250_runtime_suspend(struct device *dev)
}

omap8250_enable_wakeup(priv, true);
+ if (priv->dma_active)
+ serial8250_rx_dma(up, UART_IIR_RX_TIMEOUT);

priv->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
schedule_work(&priv->qos_work);
@@ -872,6 +918,9 @@ static int omap8250_runtime_resume(struct device *dev)
if (loss_cntx)
omap8250_restore_regs(up);

+ if (priv->dma_active)
+ serial8250_rx_dma(up, 0);
+
priv->latency = priv->calc_latency;
schedule_work(&priv->qos_work);
return 0;
--
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-10 19:30:05 UTC
Permalink
Due to UART_BUG_DMA_TX the am335x has to put the first one byte into th=
e
TX FIFO to get things going. If we get to serial8250_tx_dma() via
__dma_tx_complete() then the DMA engine just finished and the FIFO is
full and we can't put the first byte into the TX FIFO as kick start so
we leave with THRI enabled.
The result is that we end up manually filling the FIFO. We could be a
little better at this and try DMA once again. If it works, it works and
if not we do it manually.

v8=E2=80=A6v9:
- fix receiving THRI interrupts after DMA operation completed and
the next one would fail (length < 4 bytes).
- while at it makes sure we fail with an error for zero lenght
transfers with the TX bug or runtime PM
- make sure THRI is always cleared after DMA is fired.

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

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8=
250/8250_core.c
index 39b1c7318f17..660d19771bbe 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1599,9 +1599,22 @@ int serial8250_handle_irq(struct uart_port *port=
, unsigned int iir)
serial8250_modem_status(up);
if ((!up->dma || (up->dma && up->dma->tx_err)) &&
(status & UART_LSR_THRE)) {
- serial8250_tx_chars(up);
- }
+ if (!up->dma) {
+ serial8250_tx_chars(up);
+ } else {
+ if (uart_tx_stopped(&up->port) ||
+ uart_circ_empty(&up->port.state->xmit)) {
+ up->dma->tx_err =3D 0;
+ serial8250_tx_chars(up);
=20
+ } else {
+ /* try again due to UART_BUG_DMA_TX+full fifo */
+ dma_err =3D serial8250_tx_dma(up);
+ if (dma_err)
+ serial8250_tx_chars(up);
+ }
+ }
+ }
spin_unlock_irqrestore(&port->lock, flags);
return 1;
}
diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/82=
50/8250_dma.c
index 48dc57aad0dd..e836d128e63a 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -86,12 +86,23 @@ int serial8250_tx_dma(struct uart_8250_port *p)
unsigned int skip_byte =3D 0;
int ret;
=20
- if (uart_tx_stopped(&p->port) || dma->tx_running ||
- uart_circ_empty(xmit))
+ if (dma->tx_running)
return 0;
+ if (uart_tx_stopped(&p->port) || uart_circ_empty(xmit)) {
=20
- dma->tx_size =3D CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SI=
ZE);
+ /*
+ * Even if no data, we need to return an error for the two cases
+ * below so serial8250_tx_chars() is invoked and properly clears
+ * THRI and/or runtime suspend.
+ */
+ if (dma->tx_err || p->capabilities & UART_CAP_RPM) {
+ ret =3D -EBUSY;
+ goto err;
+ }
+ return 0;
+ }
=20
+ dma->tx_size =3D CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SI=
ZE);
if (p->bugs & UART_BUG_DMA_TX) {
u8 tx_lvl;
=20
@@ -142,12 +153,12 @@ int serial8250_tx_dma(struct uart_8250_port *p)
UART_XMIT_SIZE, DMA_TO_DEVICE);
=20
dma_async_issue_pending(dma->txchan);
- if (dma->tx_err) {
+ if (dma->tx_err)
dma->tx_err =3D 0;
- if (p->ier & UART_IER_THRI) {
- p->ier &=3D ~UART_IER_THRI;
- serial_out(p, UART_IER, p->ier);
- }
+
+ if (p->ier & UART_IER_THRI) {
+ p->ier &=3D ~UART_IER_THRI;
+ serial_out(p, UART_IER, p->ier);
}
if (skip_byte)
serial_out(p, UART_TX, xmit->buf[xmit->tail]);
--=20
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-10 19:30:08 UTC
Permalink
There is nothing to do for RPM in the RX path. If the HW goes off then it
won't assert the DMA line and the transfer won't happen. So we hope that
the HW does not go off for RX to work (DMA or PIO makes no difference
here).

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

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

diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index 898a6781d0b3..e8850219b150 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -21,6 +21,7 @@ static void __dma_tx_complete(void *param)
struct uart_8250_dma *dma = p->dma;
struct circ_buf *xmit = &p->port.state->xmit;
unsigned long flags;
+ bool en_thri = false;

dma_sync_single_for_cpu(dma->txchan->device->dev, dma->tx_addr,
UART_XMIT_SIZE, DMA_TO_DEVICE);
@@ -40,11 +41,16 @@ static void __dma_tx_complete(void *param)
int ret;

ret = serial8250_tx_dma(p);
- if (ret) {
- dma->tx_err = 1;
- p->ier |= UART_IER_THRI;
- serial_port_out(&p->port, UART_IER, p->ier);
- }
+ if (ret)
+ en_thri = true;
+
+ } else if (p->capabilities & UART_CAP_RPM)
+ en_thri = true;
+
+ if (en_thri) {
+ dma->tx_err = 1;
+ p->ier |= UART_IER_THRI;
+ serial_port_out(&p->port, UART_IER, p->ier);
}

spin_unlock_irqrestore(&p->port.lock, flags);
--
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Frans Klaver
2014-09-29 09:26:06 UTC
Permalink
Post by Sebastian Andrzej Siewior
There is nothing to do for RPM in the RX path. If the HW goes off then it
won't assert the DMA line and the transfer won't happen. So we hope that
the HW does not go off for RX to work (DMA or PIO makes no difference
here).
For TX the situation is slightly different. RPM is enabled on
start_tx(). We can't disable RPM on DMA complete callback because there
is still data in the FIFO which is being sent. We have to wait until
the FIFO is empty before we disable it.
For this to happen we fake a TX sent error and enable THRI. Once the
FIFO is empty we receive an interrupt and since the TTY-buffer is still
empty we "put RPM" via __stop_tx(). Should it been filed then in the
start_tx() path we should program the DMA transfer and remove the error
flag and the THRI bit.
That last sentence starts out a bit messy.
Post by Sebastian Andrzej Siewior
---
drivers/tty/serial/8250/8250_dma.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index 898a6781d0b3..e8850219b150 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -21,6 +21,7 @@ static void __dma_tx_complete(void *param)
struct uart_8250_dma *dma = p->dma;
struct circ_buf *xmit = &p->port.state->xmit;
unsigned long flags;
+ bool en_thri = false;
dma_sync_single_for_cpu(dma->txchan->device->dev, dma->tx_addr,
UART_XMIT_SIZE, DMA_TO_DEVICE);
@@ -40,11 +41,16 @@ static void __dma_tx_complete(void *param)
int ret;
ret = serial8250_tx_dma(p);
- if (ret) {
- dma->tx_err = 1;
- p->ier |= UART_IER_THRI;
- serial_port_out(&p->port, UART_IER, p->ier);
- }
+ if (ret)
+ en_thri = true;
+
+ } else if (p->capabilities & UART_CAP_RPM)
+ en_thri = true;
+
+ if (en_thri) {
+ dma->tx_err = 1;
+ p->ier |= UART_IER_THRI;
+ serial_port_out(&p->port, UART_IER, p->ier);
}
spin_unlock_irqrestore(&p->port.lock, flags);
--
2.1.0
_______________________________________________
linux-arm-kernel mailing list
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Sebastian Andrzej Siewior
2014-09-29 09:56:45 UTC
Permalink
Post by Frans Klaver
Post by Sebastian Andrzej Siewior
There is nothing to do for RPM in the RX path. If the HW goes off then it
won't assert the DMA line and the transfer won't happen. So we hope that
the HW does not go off for RX to work (DMA or PIO makes no difference
here).
For TX the situation is slightly different. RPM is enabled on
start_tx(). We can't disable RPM on DMA complete callback because there
is still data in the FIFO which is being sent. We have to wait until
the FIFO is empty before we disable it.
For this to happen we fake a TX sent error and enable THRI. Once the
FIFO is empty we receive an interrupt and since the TTY-buffer is still
empty we "put RPM" via __stop_tx(). Should it been filed then in the
start_tx() path we should program the DMA transfer and remove the error
flag and the THRI bit.
That last sentence starts out a bit messy.
This got mered so there is nothing I can do about it anymore. But I will
try to fix comments in code where and in patches that are not yet merged
(what you report :))

Sebastian
Sebastian Andrzej Siewior
2014-09-10 19:30:10 UTC
Permalink
Cc: ***@vger.kernel.org
Reviewed-by: Tony Lindgren <***@atomide.com>
Tested-by: Tony Lindgren <***@atomide.com>
Signed-off-by: Sebastian Andrzej Siewior <***@linutronix.de>
---
arch/arm/boot/dts/dra7.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index d678152db4cb..f273e3811f75 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -332,6 +332,8 @@
ti,hwmods = "uart1";
clock-frequency = <48000000>;
status = "disabled";
+ dmas = <&sdma 49>, <&sdma 50>;
+ dma-names = "tx", "rx";
};

uart2: ***@4806c000 {
@@ -341,6 +343,8 @@
ti,hwmods = "uart2";
clock-frequency = <48000000>;
status = "disabled";
+ dmas = <&sdma 51>, <&sdma 52>;
+ dma-names = "tx", "rx";
};

uart3: ***@48020000 {
@@ -350,6 +354,8 @@
ti,hwmods = "uart3";
clock-frequency = <48000000>;
status = "disabled";
+ dmas = <&sdma 53>, <&sdma 54>;
+ dma-names = "tx", "rx";
};

uart4: ***@4806e000 {
@@ -359,6 +365,8 @@
ti,hwmods = "uart4";
clock-frequency = <48000000>;
status = "disabled";
+ dmas = <&sdma 55>, <&sdma 56>;
+ dma-names = "tx", "rx";
};

uart5: ***@48066000 {
@@ -368,6 +376,8 @@
ti,hwmods = "uart5";
clock-frequency = <48000000>;
status = "disabled";
+ dmas = <&sdma 63>, <&sdma 64>;
+ dma-names = "tx", "rx";
};

uart6: ***@48068000 {
@@ -377,6 +387,8 @@
ti,hwmods = "uart6";
clock-frequency = <48000000>;
status = "disabled";
+ dmas = <&sdma 79>, <&sdma 80>;
+ dma-names = "tx", "rx";
};

uart7: ***@48420000 {
--
2.1.0
Sebastian Andrzej Siewior
2014-09-10 19:30:07 UTC
Permalink
Sometimes the OMAP UART does not signal the DMA engine to unload the FIFO.
Usually this happens when we have >threshold bytes in the FIFO
and start the DMA transfer. It seems that in those cases the UART won't
trigger the transfer once the requested threshold is reached. In some
rare cases the UART does not trigger the DMA transfer even if programmed
while the FIFO was empty.
In those cases the UART drops an RDI event and we have to empty the FIFO
manually. If we ignore it because the DMA transfer is programmed then we
will enter the function a few times until we receive the RX_TIMEOUT
event. At that point the FIFO is usually full and we risk to overflow
the FIFO.

Reviewed-by: Tony Lindgren <***@atomide.com>
Tested-by: Tony Lindgren <***@atomide.com>
Signed-off-by: Sebastian Andrzej Siewior <***@linutronix.de>
---
drivers/tty/serial/8250/8250_dma.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index fa1dc966f394..898a6781d0b3 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -193,6 +193,24 @@ int serial8250_rx_dma(struct uart_8250_port *p, unsigned int iir)
__dma_rx_do_complete(p, true);
}
return -ETIMEDOUT;
+ case UART_IIR_RDI:
+ if (p->bugs & UART_BUG_DMA_RX)
+ break;
+ /*
+ * The OMAP UART is a special BEAST. If we receive RDI we _have_
+ * a DMA transfer programmed but it didn't worked. One reason is
+ * that we were too slow and there were too many bytes in the
+ * FIFO, the UART counted wrong and never kicked the DMA engine
+ * to do anything. That means once we receive RDI on OMAP than
+ * the DMA won't do anything soon so we have to cancel the DMA
+ * transfer and purge the FIFO manually.
+ */
+ if (dma->rx_running) {
+ dmaengine_pause(dma->rxchan);
+ __dma_rx_do_complete(p, true);
+ }
+ return -ETIMEDOUT;
+
default:
break;
}
--
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Frans Klaver
2014-09-29 09:23:40 UTC
Permalink
Post by Sebastian Andrzej Siewior
Sometimes the OMAP UART does not signal the DMA engine to unload the FIFO.
Usually this happens when we have >threshold bytes in the FIFO
and start the DMA transfer. It seems that in those cases the UART won't
trigger the transfer once the requested threshold is reached. In some
rare cases the UART does not trigger the DMA transfer even if programmed
while the FIFO was empty.
In those cases the UART drops an RDI event and we have to empty the FIFO
manually. If we ignore it because the DMA transfer is programmed then we
will enter the function a few times until we receive the RX_TIMEOUT
event. At that point the FIFO is usually full and we risk to overflow
the FIFO.
---
drivers/tty/serial/8250/8250_dma.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index fa1dc966f394..898a6781d0b3 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -193,6 +193,24 @@ int serial8250_rx_dma(struct uart_8250_port *p, unsigned int iir)
__dma_rx_do_complete(p, true);
}
return -ETIMEDOUT;
+ if (p->bugs & UART_BUG_DMA_RX)
+ break;
+ /*
+ * The OMAP UART is a special BEAST. If we receive RDI we _have_
+ * a DMA transfer programmed but it didn't worked. One reason is
didn't work
Post by Sebastian Andrzej Siewior
+ * that we were too slow and there were too many bytes in the
+ * FIFO, the UART counted wrong and never kicked the DMA engine
+ * to do anything. That means once we receive RDI on OMAP than
then
Post by Sebastian Andrzej Siewior
+ * the DMA won't do anything soon so we have to cancel the DMA
+ * transfer and purge the FIFO manually.
+ */
+ if (dma->rx_running) {
+ dmaengine_pause(dma->rxchan);
+ __dma_rx_do_complete(p, true);
+ }
+ return -ETIMEDOUT;
+
break;
}
--
2.1.0
_______________________________________________
linux-arm-kernel mailing list
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Sebastian Andrzej Siewior
2014-09-10 19:30:09 UTC
Permalink
Cc: devicetree-***@public.gmane.org
Reviewed-by: Tony Lindgren <tony-***@public.gmane.org>
Tested-by: Tony Lindgren <tony-***@public.gmane.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy-***@public.gmane.org>
---
arch/arm/boot/dts/am33xx.dtsi | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 3a0a161342ba..078a760a4a21 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -200,6 +200,8 @@
reg = <0x44e09000 0x2000>;
interrupts = <72>;
status = "disabled";
+ dmas = <&edma 26>, <&edma 27>;
+ dma-names = "tx", "rx";
};

uart1: ***@48022000 {
@@ -209,6 +211,8 @@
reg = <0x48022000 0x2000>;
interrupts = <73>;
status = "disabled";
+ dmas = <&edma 28>, <&edma 29>;
+ dma-names = "tx", "rx";
};

uart2: ***@48024000 {
@@ -218,6 +222,8 @@
reg = <0x48024000 0x2000>;
interrupts = <74>;
status = "disabled";
+ dmas = <&edma 30>, <&edma 31>;
+ dma-names = "tx", "rx";
};

uart3: ***@481a6000 {
--
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-10 19:30:06 UTC
Permalink
After dmaengine_terminate_all() has been invoked then both DMA drivers
(edma and omap-dma) do not invoke dma_cookie_complete() to mark the
transfer as complete. This dma_cookie_complete() is performed by the
Synopsys DesignWare driver which is probably the only one that is used
by omap8250-dma and hence don't see following problem=E2=80=A6
=E2=80=A6which is that once a RX transfer has been terminated then foll=
owing
query of channel status reports DMA_IN_PROGRESS (again: the actual
transfer has been canceled, there is nothing going on anymore).

This means that serial8250_rx_dma() never enqueues another DMA transfer
because it (wrongly) assumes that there is a transer already pending.

Vinod Koul refuses to accept a patch which adds this
dma_cookie_complete() to both drivers and so dmaengine_tx_status() woul=
d
report DMA_COMPLETE instead (and behave like the Synopsys DesignWare
driver already does). He argues that I am not allowed to use the cookie
to query the status and that the driver already cleaned everything up a=
fter
the invokation of dmaengine_terminate_all().

To end this I add a bookkeeping whether or not a RX-transfer has been
started to the 8250-dma code. It has already been done for the TX side.
*Now* we learn about the RX status based on our bookkeeping and don't
need dmaengine_tx_status() for this anymore.

Cc: ***@intel.com
Reviewed-by: Tony Lindgren <***@atomide.com>
Tested-by: Tony Lindgren <***@atomide.com>
Signed-off-by: Sebastian Andrzej Siewior <***@linutronix.de>
---
drivers/tty/serial/8250/8250.h | 1 +
drivers/tty/serial/8250/8250_dma.c | 14 ++++++++------
2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8=
250.h
index 09489b391568..a7581b37f264 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -42,6 +42,7 @@ struct uart_8250_dma {
=20
unsigned char tx_running:1;
unsigned char tx_err: 1;
+ unsigned char rx_running:1;
};
=20
struct old_serial_port {
diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/82=
50/8250_dma.c
index e836d128e63a..fa1dc966f394 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -60,6 +60,7 @@ static void __dma_rx_do_complete(struct uart_8250_por=
t *p, bool error)
dma_sync_single_for_cpu(dma->rxchan->device->dev, dma->rx_addr,
dma->rx_size, DMA_FROM_DEVICE);
=20
+ dma->rx_running =3D 0;
dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state);
dmaengine_terminate_all(dma->rxchan);
=20
@@ -173,21 +174,21 @@ int serial8250_rx_dma(struct uart_8250_port *p, u=
nsigned int iir)
{
struct uart_8250_dma *dma =3D p->dma;
struct dma_async_tx_descriptor *desc;
- struct dma_tx_state state;
- int dma_status;
-
- dma_status =3D dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &stat=
e);
=20
switch (iir & 0x3f) {
case UART_IIR_RLSI:
/* 8250_core handles errors and break interrupts */
+ if (dma->rx_running) {
+ dmaengine_pause(dma->rxchan);
+ __dma_rx_do_complete(p, true);
+ }
return -EIO;
case UART_IIR_RX_TIMEOUT:
/*
* If RCVR FIFO trigger level was not reached, complete the
* transfer and let 8250_core copy the remaining data.
*/
- if (dma_status =3D=3D DMA_IN_PROGRESS) {
+ if (dma->rx_running) {
dmaengine_pause(dma->rxchan);
__dma_rx_do_complete(p, true);
}
@@ -196,7 +197,7 @@ int serial8250_rx_dma(struct uart_8250_port *p, uns=
igned int iir)
break;
}
=20
- if (dma_status)
+ if (dma->rx_running)
return 0;
=20
desc =3D dmaengine_prep_slave_single(dma->rxchan, dma->rx_addr,
@@ -205,6 +206,7 @@ int serial8250_rx_dma(struct uart_8250_port *p, uns=
igned int iir)
if (!desc)
return -EBUSY;
=20
+ dma->rx_running =3D 1;
desc->callback =3D __dma_rx_complete;
desc->callback_param =3D p;
=20
--=20
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Tony Lindgren
2014-09-12 22:43:25 UTC
Permalink
Post by Sebastian Andrzej Siewior
- rebased on top's of Greg's tty-next branch
- fixed #10 where we might have THRE interrupt enabled for longer tha=
n
Post by Sebastian Andrzej Siewior
needed
- re-did register setup in #10. Before this "less file" could freeze =
the
Post by Sebastian Andrzej Siewior
am335x-evm.
=46YI, looks like merging in your uart_v9 branch for testing
with my hwmod changes now fails to produce RX characters on omap3.

The device wakes up just fine to the wake-up interrupt, and then
serial8250_handle_irq() gets called, but I'm not seeing anything
in the console getting printed out. It's like all the RX characters
are getting lost instead of just the first one. The RX characters
work fine when the system is running.

So it seems there's been some kind of regression since v8?

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-15 11:50:56 UTC
Permalink
Post by Sebastian Andrzej Siewior
- rebased on top's of Greg's tty-next branch
- fixed #10 where we might have THRE interrupt enabled for longer th=
an
Post by Sebastian Andrzej Siewior
needed
- re-did register setup in #10. Before this "less file" could freeze=
the
Post by Sebastian Andrzej Siewior
am335x-evm.
=20
FYI, looks like merging in your uart_v9 branch for testing
with my hwmod changes now fails to produce RX characters on omap3.
=20
The device wakes up just fine to the wake-up interrupt, and then
serial8250_handle_irq() gets called, but I'm not seeing anything
in the console getting printed out. It's like all the RX characters
are getting lost instead of just the first one. The RX characters
work fine when the system is running.
=20
So it seems there's been some kind of regression since v8?
I changed the restore function the fix the am335x-evm + less "freeze".
And now we have this. So let me search=E2=80=A6
Regards,
=20
Tony
=20
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-16 12:57:36 UTC
Permalink
I changed the restore function the fix the am335x-evm + less "freeze"=
=2E
And now we have this. So let me search=E2=80=A6
IER was 0 by accident. It fixed in TX path.
I pushed uart_v10_pre1 which should have it fixed aport from other
things=E2=80=A6
I am going to address the review comments, to split the DMA callbacks
as requested and if nobody comes up with something fancy we will have a
v10 :)
Post by Peter Hurley
Regards,
Tony
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Tony Lindgren
2014-09-16 16:48:59 UTC
Permalink
Post by Sebastian Andrzej Siewior
I changed the restore function the fix the am335x-evm + less "freez=
e".
Post by Sebastian Andrzej Siewior
And now we have this. So let me search=E2=80=A6
=20
IER was 0 by accident. It fixed in TX path.
I pushed uart_v10_pre1 which should have it fixed aport from other
things=E2=80=A6
Just verified that works for my off-idle test case thanks.
Post by Sebastian Andrzej Siewior
I am going to address the review comments, to split the DMA callbacks
as requested and if nobody comes up with something fancy we will have=
a
Post by Sebastian Andrzej Siewior
v10 :)
OK

Regards,

Tony
Tony Lindgren
2014-09-16 21:30:43 UTC
Permalink
Post by Tony Lindgren
Post by Sebastian Andrzej Siewior
I changed the restore function the fix the am335x-evm + less "fre=
eze".
Post by Tony Lindgren
Post by Sebastian Andrzej Siewior
And now we have this. So let me search=E2=80=A6
=20
IER was 0 by accident. It fixed in TX path.
I pushed uart_v10_pre1 which should have it fixed aport from other
things=E2=80=A6
=20
Just verified that works for my off-idle test case thanks.
=20
Post by Sebastian Andrzej Siewior
I am going to address the review comments, to split the DMA callbac=
ks
Post by Tony Lindgren
Post by Sebastian Andrzej Siewior
as requested and if nobody comes up with something fancy we will ha=
ve a
Post by Tony Lindgren
Post by Sebastian Andrzej Siewior
v10 :)
=20
OK
=46ound one more issue when booting on 2420 n8x0, maybe something to do
with runtime PM?

Regards,

Tony

[ 4.770507] Internal error: Oops - undefined instruction: 0 [#1] SMP=
ARM
[ 4.777343] Modules linked in:
[ 4.780487] CPU: 0 PID: 1 Comm: init Not tainted 3.17.0-rc5-00211-gc=
2182d0-dirty #1408
[ 4.788482] task: c5842b80 ti: c5850000 task.ti: c5850000
[ 4.793945] PC is at serial8250_start_tx+0x124/0x154
[ 4.798980] LR is at uart_start+0x4c/0x5c
[ 4.803039] pc : [<c03d7340>] lr : [<c03d1ee4>] psr: a0000093
[ 4.803039] sp : c5851e10 ip : c5851e28 fp : c5851e24
[ 4.814605] r10: 00000007 r9 : 00000000 r8 : c134d554
[ 4.819885] r7 : c5cb7800 r6 : 20000013 r5 : c134d554 r4 : c134d5=
54
[ 4.826477] r3 : c134d690 r2 : 00000000 r1 : 00000001 r0 : c134d5=
54
[ 4.833068] Flags: NzCv IRQs off FIQs on Mode SVC_32 ISA ARM Se=
gment user
[ 4.840332] Control: 00c5387d Table: 85cd8000 DAC: 00000015
[ 4.846130] Process init (pid: 1, stack limit =3D 0xc5850248)
[ 4.851776] Stack: (0xc5851e10 to 0xc5852000)
[ 4.856201] 1e00: c5cb7800 c134d=
554 c5851e44 c5851e28
[ 4.864440] 1e20: c03d1ee4 c03d7228 c5842b80 00000000 c5b00408 c5cf3=
c07 c5851e7c c5851e48
[ 4.872711] 1e40: c03d2da0 c03d1ea4 c5851e6c a0000013 c03d1fe0 c5cb7=
800 00000007 0000224c
[ 4.880981] 1e60: c88cc2a0 00002250 c5cf3c00 c88ca000 c5851edc c5851=
e80 c03b9474 c03d2cd4
[ 4.889251] 1e80: c5851e9c c5cb7ab4 c5cf3c00 c5cb793c c5a5ae00 c5850=
000 c0158e1c 00000000
[ 4.897491] 1ea0: c5842b80 c007152c c5cb7ad4 c5cb7ad4 c5cb7800 0000e=
0cc 00000007 c5cb7800
[ 4.905761] 1ec0: 00000000 c5850000 00000400 c5a5ae00 c5851f1c c5851=
ee0 c03b61c0 c03b92d0
[ 4.914031] 1ee0: 00000007 00000007 c5cb18c0 c03b92c4 c0b19f20 00000=
000 00000007 0000e0cc
[ 4.922271] 1f00: c5851f78 c5a5ae00 c5850000 0000e0cc c5851f44 c5851=
f20 c03b63c4 c03b60c0
[ 4.930541] 1f20: c5851f78 c5a5ae00 00000007 0000e0cc c5851f78 00000=
007 c5851f74 c5851f48
[ 4.938812] 1f40: c0168f38 c03b632c c0185718 c0185688 00000000 00000=
000 c5a5ae00 c5a5ae00
[ 4.947052] 1f60: 00000007 0000e0cc c5851fa4 c5851f78 c0169380 c0168=
e90 00000000 00000000
[ 4.955322] 1f80: 00000000 0000e0cc 00000001 00000004 c000f164 00000=
000 00000000 c5851fa8
[ 4.963592] 1fa0: c000eee0 c0169340 00000000 0000e0cc 00000000 0000e=
0cc 00000007 00000000
[ 4.971862] 1fc0: 00000000 0000e0cc 00000001 00000004 0000a24c 00017=
504 10000000 00000000
[ 4.980102] 1fe0: bef5aa40 bef5aa30 0000a474 b6e801ec 60000010 00000=
000 ffffffff ffffffff
[ 4.988372] [<c03d7340>] (serial8250_start_tx) from [<c03d1ee4>] (ua=
rt_start+0x4c/0x5c)
[ 4.996490] [<c03d1ee4>] (uart_start) from [<c03d2da0>] (uart_write+=
0xd8/0x100)
[ 5.003875] [<c03d2da0>] (uart_write) from [<c03b9474>] (n_tty_write=
+0x1b0/0x510)
[ 5.011474] [<c03b9474>] (n_tty_write) from [<c03b61c0>] (tty_write+=
0x10c/0x26c)
[ 5.018951] [<c03b61c0>] (tty_write) from [<c03b63c4>] (redirected_t=
ty_write+0xa4/0xb8)
[ 5.027038] [<c03b63c4>] (redirected_tty_write) from [<c0168f38>] (v=
fs_write+0xb4/0x1bc)
[ 5.035217] [<c0168f38>] (vfs_write) from [<c0169380>] (SyS_write+0x=
4c/0x98)
[ 5.042358] [<c0169380>] (SyS_write) from [<c000eee0>] (ret_fast_sys=
call+0x0/0x48)
[ 5.050018] Code: e3a02000 ee072fba e3a01001 f5d3f000 (e1d30f9f)=20

--
To unsubscribe from this list: send the line "unsubscribe linux-serial"=
in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-17 08:38:52 UTC
Permalink
Found one more issue when booting on 2420 n8x0, maybe something to do
with runtime PM?
To some degree, yes.
[ 4.770507] Internal error: Oops - undefined instruction: 0 [#1]
SMP ARM

e3a02000 mov r2, #0
ee072fba mcr 15, 0, r2, cr7, cr10, {5}
e3a01001 mov r1, #1
f5d3f000 pld [r3]
=>e1d30f9f ldrexb r0, [r3]

That ldrexb is part of the xchg() function in serial8250_rpm_get_tx().
So it looks like 2420 n8x0 does not understand ldrexb but the inline
assembly decided that it should.

This OMAP2420 should be ARM1136 / ARMv6. The ARM1136J(F)-S TRM says for
ldrexb: "This command is only available from the rev1 (r1p0) release of
the ARM1136JF-S processor."

So it looks like the CPU should know what to do when this opcode comes
around.
Regards,
Tony
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Sebastian Andrzej Siewior
2014-09-17 09:05:06 UTC
Permalink
Greg, do you mind taking patches from this series up to "[PATCH 05/16]"=
?
Nobody complained about those so far and it would keep v10 a little sma=
ller.
I have changes to #6 (the omap driver) and need to do some DMA related
changes in the following

Sebastian
Greg KH
2014-09-26 16:02:39 UTC
Permalink
On Wed, Sep 17, 2014 at 11:05:06AM +0200, Sebastian Andrzej Siewior wro=
=20
Greg, do you mind taking patches from this series up to "[PATCH 05/16=
]"?
Nobody complained about those so far and it would keep v10 a little s=
maller.
I have changes to #6 (the omap driver) and need to do some DMA relate=
d
changes in the following
Now applied, thanks.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...