All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] serial: 8250: restart console after suspend
@ 2015-01-22 17:24 Peter Hurley
  2015-01-22 17:24 ` [PATCH 1/7] serial: core: Simplify console suspend logic in uart_suspend_port() Peter Hurley
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Peter Hurley @ 2015-01-22 17:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-serial, linux-kernel, Peter Hurley

Hi Greg,

This series fixes 8250 console resume from system sleep when the
console has been powered down, even though consoles aren't
suspending (eg., no_console_suspend on the kernel command line).

Patch 1 refactors the overly-complicated fall-through logic for
determining if the port should be shutdown.

Patches 2-5 refactor serial8250_do_set_termios() so that the
divisor programming can be used to reprogram the divisor when the
port has been powered-down during system sleep.

[ Patch 3, "serial: 8250: Refactor LCR computation" is, strictly
  speaking, not a required cleanup for this series. However, it
  will facilitate re-use by the omap_8250 driver.
]

Patch 6 implements the console restart by cribbing from an idea
first put forth by Doug Anderson <dianders@chromium.org> to
use the scratch register to discover the port has been powered-down.
This patch implements this restart for all 8250 drivers _except_
omap_8250 (which has a different divisor scheme) and 8250_dw
which unfortunately declares itself to be a PORT_8250 type (fixing
this in the 8250_dw driver will enable console restart for DW ports).

Patch 7 is a straggler fix which became obvious once all the
refactoring cleaned up serial8250_do_set_termios().

Regards,

Peter Hurley (7):
  serial: core: Simplify console suspend logic in uart_suspend_port()
  serial: 8250: Move UART_BUG_QUOT workaround
  serial: 8250: Refactor LCR computation
  serial: 8250: Refactor divisor programming
  serial: 8250: Refactor XR17V35X divisor calculation
  serial: 8250: Use canary to restart console after suspend
  serial: 8250: Prevent concurrent updates to shadow registers

 drivers/tty/serial/8250/8250_core.c | 187 ++++++++++++++++++++++++------------
 drivers/tty/serial/serial_core.c    |  35 ++++---
 include/linux/serial_8250.h         |   3 +
 3 files changed, 144 insertions(+), 81 deletions(-)

-- 
2.2.2


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

* [PATCH 1/7] serial: core: Simplify console suspend logic in uart_suspend_port()
  2015-01-22 17:24 [PATCH 0/7] serial: 8250: restart console after suspend Peter Hurley
@ 2015-01-22 17:24 ` Peter Hurley
  2015-01-22 17:24 ` [PATCH 2/7] serial: 8250: Move UART_BUG_QUOT workaround Peter Hurley
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Peter Hurley @ 2015-01-22 17:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-serial, linux-kernel, Peter Hurley

When the uart port being suspended is a console and consoles are
not suspending (kernel command line contains no_console_suspend),
then no action is performed for that port, and the function can
return early.

If the function has not returned early, then one of the conditions
is not true, so the expression
   (console_suspend_enabled || !uart_console(uport))
must be true and can be eliminated.

Similarly, the expression
   (console_suspend_enabled && uart_console(uport))
simplifies to just uart_console(uport).

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

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 57ca61b..37dc808 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2008,23 +2008,24 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
 	}
 	put_device(tty_dev);
 
-	if (console_suspend_enabled || !uart_console(uport))
-		uport->suspended = 1;
+	/* Nothing to do if the console is not suspending */
+	if (!console_suspend_enabled && uart_console(uport))
+		goto unlock;
+
+	uport->suspended = 1;
 
 	if (port->flags & ASYNC_INITIALIZED) {
 		const struct uart_ops *ops = uport->ops;
 		int tries;
 
-		if (console_suspend_enabled || !uart_console(uport)) {
-			set_bit(ASYNCB_SUSPENDED, &port->flags);
-			clear_bit(ASYNCB_INITIALIZED, &port->flags);
-
-			spin_lock_irq(&uport->lock);
-			ops->stop_tx(uport);
-			ops->set_mctrl(uport, 0);
-			ops->stop_rx(uport);
-			spin_unlock_irq(&uport->lock);
-		}
+		set_bit(ASYNCB_SUSPENDED, &port->flags);
+		clear_bit(ASYNCB_INITIALIZED, &port->flags);
+
+		spin_lock_irq(&uport->lock);
+		ops->stop_tx(uport);
+		ops->set_mctrl(uport, 0);
+		ops->stop_rx(uport);
+		spin_unlock_irq(&uport->lock);
 
 		/*
 		 * Wait for the transmitter to empty.
@@ -2036,19 +2037,17 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
 				drv->dev_name,
 				drv->tty_driver->name_base + uport->line);
 
-		if (console_suspend_enabled || !uart_console(uport))
-			ops->shutdown(uport);
+		ops->shutdown(uport);
 	}
 
 	/*
 	 * Disable the console device before suspending.
 	 */
-	if (console_suspend_enabled && uart_console(uport))
+	if (uart_console(uport))
 		console_stop(uport->cons);
 
-	if (console_suspend_enabled || !uart_console(uport))
-		uart_change_pm(state, UART_PM_STATE_OFF);
-
+	uart_change_pm(state, UART_PM_STATE_OFF);
+unlock:
 	mutex_unlock(&port->mutex);
 
 	return 0;
-- 
2.2.2


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

* [PATCH 2/7] serial: 8250: Move UART_BUG_QUOT workaround
  2015-01-22 17:24 [PATCH 0/7] serial: 8250: restart console after suspend Peter Hurley
  2015-01-22 17:24 ` [PATCH 1/7] serial: core: Simplify console suspend logic in uart_suspend_port() Peter Hurley
@ 2015-01-22 17:24 ` Peter Hurley
  2015-01-22 17:24 ` [PATCH 3/7] serial: 8250: Refactor LCR computation Peter Hurley
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Peter Hurley @ 2015-01-22 17:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-serial, linux-kernel, Peter Hurley

The UART_BUG_QUOT workaround adjusts the divisor computed from the
baud rate by serial8250_get_divisor(). Move the workaround into
serial8250_get_divisor(), so that divisor-from-baud computation
is encapsulated.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/8250/8250_core.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 3bfcfdb..fa40982 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2390,8 +2390,9 @@ static void serial8250_shutdown(struct uart_port *port)
 		serial8250_do_shutdown(port);
 }
 
-static unsigned int serial8250_get_divisor(struct uart_port *port, unsigned int baud)
+static unsigned int serial8250_get_divisor(struct uart_8250_port *up, unsigned int baud)
 {
+	struct uart_port *port = &up->port;
 	unsigned int quot;
 
 	/*
@@ -2407,6 +2408,12 @@ static unsigned int serial8250_get_divisor(struct uart_port *port, unsigned int
 	else
 		quot = uart_get_divisor(port, baud);
 
+	/*
+	 * Oxford Semi 952 rev B workaround
+	 */
+	if (up->bugs & UART_BUG_QUOT && (quot & 0xff) == 0)
+		quot++;
+
 	return quot;
 }
 
@@ -2455,13 +2462,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 	baud = uart_get_baud_rate(port, termios, old,
 				  port->uartclk / 16 / 0xffff,
 				  port->uartclk / 16);
-	quot = serial8250_get_divisor(port, baud);
-
-	/*
-	 * Oxford Semi 952 rev B workaround
-	 */
-	if (up->bugs & UART_BUG_QUOT && (quot & 0xff) == 0)
-		quot++;
+	quot = serial8250_get_divisor(up, baud);
 
 	if (up->capabilities & UART_CAP_FIFO && port->fifosize > 1) {
 		/* NOTE: If fifo_bug is not set, a user can set RX_trigger. */
-- 
2.2.2


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

* [PATCH 3/7] serial: 8250: Refactor LCR computation
  2015-01-22 17:24 [PATCH 0/7] serial: 8250: restart console after suspend Peter Hurley
  2015-01-22 17:24 ` [PATCH 1/7] serial: core: Simplify console suspend logic in uart_suspend_port() Peter Hurley
  2015-01-22 17:24 ` [PATCH 2/7] serial: 8250: Move UART_BUG_QUOT workaround Peter Hurley
@ 2015-01-22 17:24 ` Peter Hurley
  2015-01-22 17:24 ` [PATCH 4/7] serial: 8250: Refactor divisor programming Peter Hurley
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Peter Hurley @ 2015-01-22 17:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-serial, linux-kernel, Peter Hurley

Refactor the computation of the LCR register value from termios c_cflag
into a new local function, serial8250_compute_lcr().

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/8250/8250_core.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index fa40982..77afa5f 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2417,16 +2417,12 @@ static unsigned int serial8250_get_divisor(struct uart_8250_port *up, unsigned i
 	return quot;
 }
 
-void
-serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
-		          struct ktermios *old)
+static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
+					    tcflag_t c_cflag)
 {
-	struct uart_8250_port *up = up_to_u8250p(port);
 	unsigned char cval;
-	unsigned long flags;
-	unsigned int baud, quot;
 
-	switch (termios->c_cflag & CSIZE) {
+	switch (c_cflag & CSIZE) {
 	case CS5:
 		cval = UART_LCR_WLEN5;
 		break;
@@ -2442,20 +2438,34 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 		break;
 	}
 
-	if (termios->c_cflag & CSTOPB)
+	if (c_cflag & CSTOPB)
 		cval |= UART_LCR_STOP;
-	if (termios->c_cflag & PARENB) {
+	if (c_cflag & PARENB) {
 		cval |= UART_LCR_PARITY;
 		if (up->bugs & UART_BUG_PARITY)
 			up->fifo_bug = true;
 	}
-	if (!(termios->c_cflag & PARODD))
+	if (!(c_cflag & PARODD))
 		cval |= UART_LCR_EPAR;
 #ifdef CMSPAR
-	if (termios->c_cflag & CMSPAR)
+	if (c_cflag & CMSPAR)
 		cval |= UART_LCR_SPAR;
 #endif
 
+	return cval;
+}
+
+void
+serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
+		          struct ktermios *old)
+{
+	struct uart_8250_port *up = up_to_u8250p(port);
+	unsigned char cval;
+	unsigned long flags;
+	unsigned int baud, quot;
+
+	cval = serial8250_compute_lcr(up, termios->c_cflag);
+
 	/*
 	 * Ask the core to calculate the divisor for us.
 	 */
-- 
2.2.2


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

* [PATCH 4/7] serial: 8250: Refactor divisor programming
  2015-01-22 17:24 [PATCH 0/7] serial: 8250: restart console after suspend Peter Hurley
                   ` (2 preceding siblings ...)
  2015-01-22 17:24 ` [PATCH 3/7] serial: 8250: Refactor LCR computation Peter Hurley
@ 2015-01-22 17:24 ` Peter Hurley
  2015-01-22 17:24 ` [PATCH 5/7] serial: 8250: Refactor XR17V35X divisor calculation Peter Hurley
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Peter Hurley @ 2015-01-22 17:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-serial, linux-kernel, Peter Hurley

Refactor divisor register programming into a new function,
serial8250_set_divisor; this allows serial console to reinitialize
early after resume from suspend.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/8250/8250_core.c | 87 ++++++++++++++++++++-----------------
 1 file changed, 48 insertions(+), 39 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 77afa5f..b8aa6ef 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2455,6 +2455,50 @@ static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
 	return cval;
 }
 
+void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
+			    unsigned int quot)
+{
+	struct uart_8250_port *up = up_to_u8250p(port);
+
+	/* Workaround to enable 115200 baud on OMAP1510 internal ports */
+	if (is_omap1510_8250(up)) {
+		if (baud == 115200) {
+			quot = 1;
+			serial_port_out(port, UART_OMAP_OSC_12M_SEL, 1);
+		} else
+			serial_port_out(port, UART_OMAP_OSC_12M_SEL, 0);
+	}
+
+	/*
+	 * For NatSemi, switch to bank 2 not bank 1, to avoid resetting EXCR2,
+	 * otherwise just set DLAB
+	 */
+	if (up->capabilities & UART_NATSEMI)
+		serial_port_out(port, UART_LCR, 0xe0);
+	else
+		serial_port_out(port, UART_LCR, up->lcr | UART_LCR_DLAB);
+
+	serial_dl_write(up, quot);
+
+	/*
+	 * XR17V35x UARTs have an extra fractional divisor register (DLD)
+	 *
+	 * We need to recalculate all of the registers, because DLM and DLL
+	 * are already rounded to a whole integer.
+	 *
+	 * When recalculating we use a 32x clock instead of a 16x clock to
+	 * allow 1-bit for rounding in the fractional part.
+	 */
+	if (up->port.type == PORT_XR17V35X) {
+		unsigned int baud_x32 = (port->uartclk * 2) / baud;
+		u16 quot = baud_x32 / 32;
+		u8 quot_frac = DIV_ROUND_CLOSEST(baud_x32 % 32, 2);
+
+		serial_dl_write(up, quot);
+		serial_port_out(port, 0x2, quot_frac & 0xf);
+	}
+}
+
 void
 serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 		          struct ktermios *old)
@@ -2503,6 +2547,8 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 	serial8250_rpm_get(up);
 	spin_lock_irqsave(&port->lock, flags);
 
+	up->lcr = cval;					/* Save computed LCR */
+
 	/*
 	 * Update the per-port timeout.
 	 */
@@ -2567,43 +2613,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 			serial_port_out(port, UART_EFR, efr);
 	}
 
-	/* Workaround to enable 115200 baud on OMAP1510 internal ports */
-	if (is_omap1510_8250(up)) {
-		if (baud == 115200) {
-			quot = 1;
-			serial_port_out(port, UART_OMAP_OSC_12M_SEL, 1);
-		} else
-			serial_port_out(port, UART_OMAP_OSC_12M_SEL, 0);
-	}
-
-	/*
-	 * For NatSemi, switch to bank 2 not bank 1, to avoid resetting EXCR2,
-	 * otherwise just set DLAB
-	 */
-	if (up->capabilities & UART_NATSEMI)
-		serial_port_out(port, UART_LCR, 0xe0);
-	else
-		serial_port_out(port, UART_LCR, cval | UART_LCR_DLAB);
-
-	serial_dl_write(up, quot);
-
-	/*
-	 * XR17V35x UARTs have an extra fractional divisor register (DLD)
-	 *
-	 * We need to recalculate all of the registers, because DLM and DLL
-	 * are already rounded to a whole integer.
-	 *
-	 * When recalculating we use a 32x clock instead of a 16x clock to
-	 * allow 1-bit for rounding in the fractional part.
-	 */
-	if (up->port.type == PORT_XR17V35X) {
-		unsigned int baud_x32 = (port->uartclk * 2) / baud;
-		u16 quot = baud_x32 / 32;
-		u8 quot_frac = DIV_ROUND_CLOSEST(baud_x32 % 32, 2);
-
-		serial_dl_write(up, quot);
-		serial_port_out(port, 0x2, quot_frac & 0xf);
-	}
+	serial8250_set_divisor(port, baud, quot);
 
 	/*
 	 * LCR DLAB must be set to enable 64-byte FIFO mode. If the FCR
@@ -2612,8 +2622,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 	if (port->type == PORT_16750)
 		serial_port_out(port, UART_FCR, up->fcr);
 
-	serial_port_out(port, UART_LCR, cval);		/* reset DLAB */
-	up->lcr = cval;					/* Save LCR */
+	serial_port_out(port, UART_LCR, up->lcr);	/* reset DLAB */
 	if (port->type != PORT_16750) {
 		/* emulated UARTs (Lucent Venus 167x) need two steps */
 		if (up->fcr & UART_FCR_ENABLE_FIFO)
-- 
2.2.2


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

* [PATCH 5/7] serial: 8250: Refactor XR17V35X divisor calculation
  2015-01-22 17:24 [PATCH 0/7] serial: 8250: restart console after suspend Peter Hurley
                   ` (3 preceding siblings ...)
  2015-01-22 17:24 ` [PATCH 4/7] serial: 8250: Refactor divisor programming Peter Hurley
@ 2015-01-22 17:24 ` Peter Hurley
  2015-01-22 17:46   ` Peter Hurley
  2015-01-22 17:24 ` [PATCH 6/7] serial: 8250: Use canary to restart console after suspend Peter Hurley
  2015-01-22 17:24 ` [PATCH 7/7] serial: 8250: Prevent concurrent updates to shadow registers Peter Hurley
  6 siblings, 1 reply; 9+ messages in thread
From: Peter Hurley @ 2015-01-22 17:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, linux-serial, linux-kernel, Peter Hurley,
	Joe Schultz, Aaron Sierra

Exar XR17V35X PCIe uarts support a 4-bit fractional divisor register.
Refactor the divisor calculation from the divisor programming.

Allow a fractional result from serial8250_get_divisor() and pass this
result to serial8250_dl_write().

Simplify the calculation for quot and quot_frac. This was verified
to be identical to the results of the original calculation with a test
jig.

NB: The results were also compared with the divisor value chart
on pg 33 of the Exar XR17V352 datasheet, rev 1.0.3, here:
http://www.exar.com/common/content/document.ashx?id=1585
which differs from the calculated values by 1 in the fractional result.
This is because the calculated values are still rounded in the
fractional result, whereas the table values are truncated. Note
that the data error rate % values in the datasheet are for
rounded fractional results, as the truncated fractional results
have more error.

Cc: Joe Schultz <jschultz@xes-inc.com>
Cc: Aaron Sierra <asierra@xes-inc.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/8250/8250_core.c | 52 +++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index b8aa6ef..5240c1a 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2390,7 +2390,26 @@ static void serial8250_shutdown(struct uart_port *port)
 		serial8250_do_shutdown(port);
 }
 
-static unsigned int serial8250_get_divisor(struct uart_8250_port *up, unsigned int baud)
+/*
+ * XR17V35x UARTs have an extra fractional divisor register (DLD)
+ * Calculate divisor with extra 4-bit fractional portion
+ */
+static unsigned int xr17v35x_get_divisor(struct uart_8250_port *up,
+					 unsigned int baud,
+					 unsigned int *frac)
+{
+	struct uart_port *port = &up->port;
+	unsigned int quot_16;
+
+	quot_16 = DIV_ROUND_CLOSEST(port->uartclk, baud);
+	*frac = quot_16 & 0x0f;
+
+	return quot_16 >> 4;
+}
+
+static unsigned int serial8250_get_divisor(struct uart_8250_port *up,
+					   unsigned int baud,
+					   unsigned int *frac)
 {
 	struct uart_port *port = &up->port;
 	unsigned int quot;
@@ -2398,6 +2417,7 @@ static unsigned int serial8250_get_divisor(struct uart_8250_port *up, unsigned i
 	/*
 	 * Handle magic divisors for baud rates above baud_base on
 	 * SMSC SuperIO chips.
+	 *
 	 */
 	if ((port->flags & UPF_MAGIC_MULTIPLIER) &&
 	    baud == (port->uartclk/4))
@@ -2405,6 +2425,8 @@ static unsigned int serial8250_get_divisor(struct uart_8250_port *up, unsigned i
 	else if ((port->flags & UPF_MAGIC_MULTIPLIER) &&
 		 baud == (port->uartclk/8))
 		quot = 0x8002;
+	else if (up->port.type == PORT_XR17V35X)
+		quot = xr17v35x_get_divisor(up, baud, frac);
 	else
 		quot = uart_get_divisor(port, baud);
 
@@ -2456,7 +2478,7 @@ static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
 }
 
 void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
-			    unsigned int quot)
+			    unsigned int quot, unsigned int quot_frac)
 {
 	struct uart_8250_port *up = up_to_u8250p(port);
 
@@ -2480,23 +2502,9 @@ void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
 
 	serial_dl_write(up, quot);
 
-	/*
-	 * XR17V35x UARTs have an extra fractional divisor register (DLD)
-	 *
-	 * We need to recalculate all of the registers, because DLM and DLL
-	 * are already rounded to a whole integer.
-	 *
-	 * When recalculating we use a 32x clock instead of a 16x clock to
-	 * allow 1-bit for rounding in the fractional part.
-	 */
-	if (up->port.type == PORT_XR17V35X) {
-		unsigned int baud_x32 = (port->uartclk * 2) / baud;
-		u16 quot = baud_x32 / 32;
-		u8 quot_frac = DIV_ROUND_CLOSEST(baud_x32 % 32, 2);
-
-		serial_dl_write(up, quot);
-		serial_port_out(port, 0x2, quot_frac & 0xf);
-	}
+	/* XR17V35x UARTs have an extra fractional divisor register (DLD) */
+	if (up->port.type == PORT_XR17V35X)
+		serial_port_out(port, 0x2, quot_frac);
 }
 
 void
@@ -2506,7 +2514,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 	struct uart_8250_port *up = up_to_u8250p(port);
 	unsigned char cval;
 	unsigned long flags;
-	unsigned int baud, quot;
+	unsigned int baud, quot, frac = 0;
 
 	cval = serial8250_compute_lcr(up, termios->c_cflag);
 
@@ -2516,7 +2524,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 	baud = uart_get_baud_rate(port, termios, old,
 				  port->uartclk / 16 / 0xffff,
 				  port->uartclk / 16);
-	quot = serial8250_get_divisor(up, baud);
+	quot = serial8250_get_divisor(up, baud, &frac);
 
 	if (up->capabilities & UART_CAP_FIFO && port->fifosize > 1) {
 		/* NOTE: If fifo_bug is not set, a user can set RX_trigger. */
@@ -2613,7 +2621,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 			serial_port_out(port, UART_EFR, efr);
 	}
 
-	serial8250_set_divisor(port, baud, quot);
+	serial8250_set_divisor(port, baud, quot, frac);
 
 	/*
 	 * LCR DLAB must be set to enable 64-byte FIFO mode. If the FCR
-- 
2.2.2


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

* [PATCH 6/7] serial: 8250: Use canary to restart console after suspend
  2015-01-22 17:24 [PATCH 0/7] serial: 8250: restart console after suspend Peter Hurley
                   ` (4 preceding siblings ...)
  2015-01-22 17:24 ` [PATCH 5/7] serial: 8250: Refactor XR17V35X divisor calculation Peter Hurley
@ 2015-01-22 17:24 ` Peter Hurley
  2015-01-22 17:24 ` [PATCH 7/7] serial: 8250: Prevent concurrent updates to shadow registers Peter Hurley
  6 siblings, 0 replies; 9+ messages in thread
From: Peter Hurley @ 2015-01-22 17:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, linux-serial, linux-kernel, Peter Hurley, Doug Anderson

When using no_console_suspend, the serial console may be powered off
anyway during system sleep. Upon resume, the port may be in its default
power-on state, but is expected to continue console i/o before the device
has received its pm callback. The resultant garbage i/o can cause all
kinds of havoc on the remote end.

Use the scratch register as a canary to discover if the console
has been powered-off. Write a non-zero value to the scratch register
at port suspend and reprogram the port before any console i/o if the
scratch register != canary before port resume.

This workaround is disabled for omap_8250 (which uses different divisor
programming).

Credit to Doug Anderson <dianders@chromium.org> for the idea of using
the scratch register canary to discover port power-down.

Cc: Doug Anderson <dianders@chromium.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/8250/8250_core.c | 35 ++++++++++++++++++++++++++++++++++-
 include/linux/serial_8250.h         |  3 +++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 5240c1a..8fd4b57 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3244,6 +3244,27 @@ serial8250_console_write(struct console *co, const char *s, unsigned int count)
 	else
 		serial_port_out(port, UART_IER, 0);
 
+	/* check scratch reg to see if port powered off during system sleep */
+	if (up->canary && (up->canary != serial_port_in(port, UART_SCR))) {
+		struct ktermios termios;
+		unsigned int baud, quot, frac = 0;
+
+		termios.c_cflag = port->cons->cflag;
+		if (port->state->port.tty && termios.c_cflag == 0)
+			termios.c_cflag = port->state->port.tty->termios.c_cflag;
+
+		baud = uart_get_baud_rate(port, &termios, NULL,
+					  port->uartclk / 16 / 0xffff,
+					  port->uartclk / 16);
+		quot = serial8250_get_divisor(up, baud, &frac);
+
+		serial8250_set_divisor(port, baud, quot, frac);
+		serial_port_out(port, UART_LCR, up->lcr);
+		serial_port_out(port, UART_MCR, UART_MCR_DTR | UART_MCR_RTS);
+
+		up->canary = 0;
+	}
+
 	uart_console_write(port, s, count, serial8250_console_putchar);
 
 	/*
@@ -3394,7 +3415,17 @@ int __init early_serial_setup(struct uart_port *port)
  */
 void serial8250_suspend_port(int line)
 {
-	uart_suspend_port(&serial8250_reg, &serial8250_ports[line].port);
+	struct uart_8250_port *up = &serial8250_ports[line];
+	struct uart_port *port = &up->port;
+
+	if (!console_suspend_enabled && uart_console(port) &&
+	    port->type != PORT_8250) {
+		unsigned char canary = 0xa5;
+		serial_out(up, UART_SCR, canary);
+		up->canary = canary;
+	}
+
+	uart_suspend_port(&serial8250_reg, port);
 }
 
 /**
@@ -3408,6 +3439,8 @@ void serial8250_resume_port(int line)
 	struct uart_8250_port *up = &serial8250_ports[line];
 	struct uart_port *port = &up->port;
 
+	up->canary = 0;
+
 	if (up->capabilities & UART_NATSEMI) {
 		/* Ensure it's still in high speed mode */
 		serial_port_out(port, UART_LCR, 0xE0);
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 245b959..a8efa23 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -85,6 +85,9 @@ struct uart_8250_port {
 	unsigned char		mcr_force;	/* mask of forced bits */
 	unsigned char		cur_iotype;	/* Running I/O type */
 	unsigned int		rpm_tx_active;
+	unsigned char		canary;		/* non-zero during system sleep
+						 *   if no_console_suspend
+						 */
 
 	/*
 	 * Some bits in registers are cleared on a read, so they must
-- 
2.2.2


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

* [PATCH 7/7] serial: 8250: Prevent concurrent updates to shadow registers
  2015-01-22 17:24 [PATCH 0/7] serial: 8250: restart console after suspend Peter Hurley
                   ` (5 preceding siblings ...)
  2015-01-22 17:24 ` [PATCH 6/7] serial: 8250: Use canary to restart console after suspend Peter Hurley
@ 2015-01-22 17:24 ` Peter Hurley
  6 siblings, 0 replies; 9+ messages in thread
From: Peter Hurley @ 2015-01-22 17:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-serial, linux-kernel, Peter Hurley

The port shadow registers, ->fcr and ->mcr, must be protected from
concurrent updates. Relocate the shadow register updates in
serial8250_do_set_termios() to the port lock critical section.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/8250/8250_core.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 8fd4b57..ed15341 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2526,6 +2526,15 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 				  port->uartclk / 16);
 	quot = serial8250_get_divisor(up, baud, &frac);
 
+	/*
+	 * Ok, we're now changing the port state.  Do it with
+	 * interrupts disabled.
+	 */
+	serial8250_rpm_get(up);
+	spin_lock_irqsave(&port->lock, flags);
+
+	up->lcr = cval;					/* Save computed LCR */
+
 	if (up->capabilities & UART_CAP_FIFO && port->fifosize > 1) {
 		/* NOTE: If fifo_bug is not set, a user can set RX_trigger. */
 		if ((baud < 2400 && !up->dma) || up->fifo_bug) {
@@ -2549,15 +2558,6 @@ 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);
-
-	up->lcr = cval;					/* Save computed LCR */
-
-	/*
 	 * Update the per-port timeout.
 	 */
 	uart_update_timeout(port, termios->c_cflag, baud);
-- 
2.2.2


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

* Re: [PATCH 5/7] serial: 8250: Refactor XR17V35X divisor calculation
  2015-01-22 17:24 ` [PATCH 5/7] serial: 8250: Refactor XR17V35X divisor calculation Peter Hurley
@ 2015-01-22 17:46   ` Peter Hurley
  0 siblings, 0 replies; 9+ messages in thread
From: Peter Hurley @ 2015-01-22 17:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, linux-serial, linux-kernel, Joe Schultz, Aaron Sierra

On 01/22/2015 12:24 PM, Peter Hurley wrote:
> Exar XR17V35X PCIe uarts support a 4-bit fractional divisor register.
> Refactor the divisor calculation from the divisor programming.
> 
> Allow a fractional result from serial8250_get_divisor() and pass this
> result to serial8250_dl_write().
> 
> Simplify the calculation for quot and quot_frac. This was verified
> to be identical to the results of the original calculation with a test
> jig.
> 
> NB: The results were also compared with the divisor value chart
> on pg 33 of the Exar XR17V352 datasheet, rev 1.0.3, here:
> http://www.exar.com/common/content/document.ashx?id=1585
> which differs from the calculated values by 1 in the fractional result.
> This is because the calculated values are still rounded in the
> fractional result, whereas the table values are truncated. Note
> that the data error rate % values in the datasheet are for
> rounded fractional results, as the truncated fractional results
> have more error.
> 
> Cc: Joe Schultz <jschultz@xes-inc.com>
> Cc: Aaron Sierra <asierra@xes-inc.com>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>

FWIW, here's the test jig:

--- /dev/null	2015-01-22 11:30:11.407707482 -0500
+++ divisor.c	2015-01-22 12:42:34.722403359 -0500
@@ -0,0 +1,92 @@
+/*
+ * divisor.c - test jig for evaluating computation equivalence
+ *	       of divisors for XR17V35x UART
+ */
+
+#include <stdio.h>
+
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof(x[0]))
+
+/* from include/linux/kernel.h */
+
+#define DIV_ROUND_CLOSEST(x, divisor)(			\
+{							\
+	typeof(x) __x = x;				\
+	typeof(divisor) __d = divisor;			\
+	(((typeof(x))-1) > 0 ||				\
+	 ((typeof(divisor))-1) > 0 || (__x) > 0) ?	\
+		(((__x) + ((__d) / 2)) / (__d)) :	\
+		(((__x) - ((__d) / 2)) / (__d));	\
+}							\
+)
+
+/* 125MHz clock on XR17V35x */
+unsigned int clk = 125000000;
+
+
+void calc_quot(unsigned int baud, unsigned int *quot, unsigned int *frac) {
+
+	unsigned int baud_x32 = (2 * clk) / baud;
+
+	*quot = baud_x32 / 32;
+	*frac = DIV_ROUND_CLOSEST(baud_x32 % 32, 2);
+}
+
+void calc_quot2(unsigned int baud, unsigned int *quot, unsigned int *frac) {
+
+	/* compute quotient as a 16.4 fixed point value */
+	unsigned int quot_16 = DIV_ROUND_CLOSEST(clk, baud);
+
+	*quot = quot_16 >> 4;
+	*frac = quot_16 & 0x0f;
+}
+
+
+int main() {
+	int i;
+	unsigned int bauds[] = {   2400,
+				   4800,
+				   9600,
+				  10000,
+				  19200,
+				  25000,
+				  28800,
+				  38400,
+				  50000,
+				  57600,
+				  75000,
+				 100000,
+				 115200,
+				 153600,
+				 200000,
+				 225000,
+				 230400,
+				 250000,
+				 300000,
+				 400000,
+				 460800,
+				 500000,
+				 576000,
+				 750000,
+				 921600,
+				1000000,
+				1152000,
+			       };
+
+	for (i = 0; i < ARRAY_SIZE(bauds); i++) {
+		unsigned int quot, frac;
+		unsigned int quot2, frac2;
+
+		calc_quot(bauds[i], &quot, &frac);
+		calc_quot2(bauds[i], &quot2, &frac2);
+
+		if (quot != quot2 || frac != frac2) {
+			printf("diff: %04x.%01x != %04x.%01x\n", quot, frac,
+			       quot2, frac2);
+		} else
+			printf("%10d  %02X %02X %01X\n", bauds[i],
+			       quot2 >> 8, quot2 & 0xff, frac2);
+	}
+
+	return 0;
+}


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

end of thread, other threads:[~2015-01-22 17:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-22 17:24 [PATCH 0/7] serial: 8250: restart console after suspend Peter Hurley
2015-01-22 17:24 ` [PATCH 1/7] serial: core: Simplify console suspend logic in uart_suspend_port() Peter Hurley
2015-01-22 17:24 ` [PATCH 2/7] serial: 8250: Move UART_BUG_QUOT workaround Peter Hurley
2015-01-22 17:24 ` [PATCH 3/7] serial: 8250: Refactor LCR computation Peter Hurley
2015-01-22 17:24 ` [PATCH 4/7] serial: 8250: Refactor divisor programming Peter Hurley
2015-01-22 17:24 ` [PATCH 5/7] serial: 8250: Refactor XR17V35X divisor calculation Peter Hurley
2015-01-22 17:46   ` Peter Hurley
2015-01-22 17:24 ` [PATCH 6/7] serial: 8250: Use canary to restart console after suspend Peter Hurley
2015-01-22 17:24 ` [PATCH 7/7] serial: 8250: Prevent concurrent updates to shadow registers Peter Hurley

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.