[PATCH 00/10] serial: sh-sci fixes - console PM, SCIFB, SMP lockup, etc.

[PATCH 00/10] serial: sh-sci fixes - console PM, SCIFB, SMP lockup, etc.

[Shinya Kuribayashi]

Hi Paul and serial forks,

Here are sh-sci serial driver fixes that have been cooking for months
privately on my end:
* console PM rework with the help of serial-core's uart_change_pm()
* provide missing piece in SCIFB support (from Yoshii-san)
* address console write function vs. SMP lockup issue

and so on.  Comments and suggestions are welcome.

Build tested with vanilla v3.6, but not boot- nor PM-tested with
newer kernels after v3.4.  Anyone with can-sleep shmobile systems,
please give it try.

Thanks in advance,

----------------------------------------------------------------
Shinya Kuribayashi (5):
      Revert "sh-sci / PM: Avoid deadlocking runtime PM"
      Revert "sh-sci / PM: Use power.irq_safe"
      Partially revert "serial: sh-sci: console Runtime PM support"
      serial: sh-sci: add locking to console write function to avoid SMP lockup
      serial: sh-sci: fix possible race cases on SCSCR register accesses

Takashi YOSHII (4):
      serial: sh-sci: fix condition test to set SCBRR
      serial: sh-sci: support lower baud rate
      serial: sh-sci: mask SCTFDR/RFDR according to fifosize
      serial: sh-sci: fix common SCIFB regmap definition

Teppei Kamijou (1):
      serial: sh-sci: console runtime PM support (revisit)

 drivers/tty/serial/sh-sci.c | 140 ++++++++++++++++++--------------------------
 1 file changed, 56 insertions(+), 84 deletions(-)

-- 
Shinya Kuribayashi
Renesas Electronics

[PATCH 01/10] Revert "sh-sci / PM: Avoid deadlocking runtime PM"

[Shinya Kuribayashi]

This reverts commit 048be431e4 (sh-sci / PM: Avoid deadlocking runtime
PM, from Rafael J. Wysocki <rjw@sisk.pl>, 2012-03-09).

In order to get console PM work properly, we should implement uart_ops
->pm() operation, rather than sprinkle band-ading runtime PM calls in
the driver.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 9be296c..526ad04 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1752,8 +1752,6 @@ static int sci_startup(struct uart_port *port)
 
 	dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);
 
-	pm_runtime_put_noidle(port->dev);
-
 	sci_port_enable(s);
 
 	ret = sci_request_irq(s);
@@ -1781,8 +1779,6 @@ static void sci_shutdown(struct uart_port *port)
 	sci_free_irq(s);
 
 	sci_port_disable(s);
-
-	pm_runtime_get_noresume(port->dev);
 }
 
 static unsigned int sci_scbrr_calc(unsigned int algo_id, unsigned int bps,
@@ -2121,7 +2117,6 @@ static int __devinit sci_init_single(struct platform_device *dev,
 		sci_init_gpios(sci_port);
 
 		pm_runtime_irq_safe(&dev->dev);
-		pm_runtime_get_noresume(&dev->dev);
 		pm_runtime_enable(&dev->dev);
 	}
 
-- 
1.7.12.4

[PATCH 02/10] Revert "sh-sci / PM: Use power.irq_safe"

[Shinya Kuribayashi]

This reverts commit 5a50a01bf0 (sh-sci / PM: Use power.irq_safe, from
Rafael J. Wysocki <rjw@sisk.pl>, 2011-08-24).

In order to get console PM work properly, we should implement uart_ops
->pm() operation, rather than sprinkle band-ading runtime PM calls in
the driver.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 526ad04..9abe977 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2116,7 +2116,6 @@ static int __devinit sci_init_single(struct platform_device *dev,
 
 		sci_init_gpios(sci_port);
 
-		pm_runtime_irq_safe(&dev->dev);
 		pm_runtime_enable(&dev->dev);
 	}
 
-- 
1.7.12.4

[PATCH 03/10] Partially revert "serial: sh-sci: console Runtime PM support"

[Shinya Kuribayashi]

This partially reverts commit 1ba7622094 (serial: sh-sci: console
Runtime PM support, from Magnus Damm <damm@opensource.se>, 2011-08-03).

The generic 'serial_core' can take care of console PM maintenance,
so all (or at least the first thing) we have to do to get console PM
work properly, is to implement uart_ops ->pm() operation in the sh-sci
serial client driver.

This patch partially reverts the commit above, but leaving sci_reset()
change in place, because sci_reset() is already part of another commit
(73c3d53f38 serial: sh-sci: Avoid FIFO clear for MCE toggle.).

A revised version of console PM support follows next.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 53 +--------------------------------------------
 1 file changed, 1 insertion(+), 52 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 9abe977..e9e8883 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -99,12 +99,6 @@ struct sci_port {
 #endif
 
 	struct notifier_block		freq_transition;
-
-#ifdef CONFIG_SERIAL_SH_SCI_CONSOLE
-	unsigned short saved_smr;
-	unsigned short saved_fcr;
-	unsigned char saved_brr;
-#endif
 };
 
 /* Function prototypes */
@@ -2247,8 +2241,7 @@ static int __devinit serial_console_setup(struct console *co, char *options)
 	if (options)
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
 
-	sci_port_disable(sci_port);
-
+	/* TODO: disable clock */
 	return uart_set_options(port, co, baud, parity, bits, flow);
 }
 
@@ -2291,46 +2284,6 @@ static int __devinit sci_probe_earlyprintk(struct platform_device *pdev)
 	return 0;
 }
 
-#define uart_console(port)	((port)->cons->index == (port)->line)
-
-static int sci_runtime_suspend(struct device *dev)
-{
-	struct sci_port *sci_port = dev_get_drvdata(dev);
-	struct uart_port *port = &sci_port->port;
-
-	if (uart_console(port)) {
-		struct plat_sci_reg *reg;
-
-		sci_port->saved_smr = serial_port_in(port, SCSMR);
-		sci_port->saved_brr = serial_port_in(port, SCBRR);
-
-		reg = sci_getreg(port, SCFCR);
-		if (reg->size)
-			sci_port->saved_fcr = serial_port_in(port, SCFCR);
-		else
-			sci_port->saved_fcr = 0;
-	}
-	return 0;
-}
-
-static int sci_runtime_resume(struct device *dev)
-{
-	struct sci_port *sci_port = dev_get_drvdata(dev);
-	struct uart_port *port = &sci_port->port;
-
-	if (uart_console(port)) {
-		sci_reset(port);
-		serial_port_out(port, SCSMR, sci_port->saved_smr);
-		serial_port_out(port, SCBRR, sci_port->saved_brr);
-
-		if (sci_port->saved_fcr)
-			serial_port_out(port, SCFCR, sci_port->saved_fcr);
-
-		serial_port_out(port, SCSCR, sci_port->cfg->scscr);
-	}
-	return 0;
-}
-
 #define SCI_CONSOLE	(&serial_console)
 
 #else
@@ -2340,8 +2293,6 @@ static inline int __devinit sci_probe_earlyprintk(struct platform_device *pdev)
 }
 
 #define SCI_CONSOLE	NULL
-#define sci_runtime_suspend	NULL
-#define sci_runtime_resume	NULL
 
 #endif /* CONFIG_SERIAL_SH_SCI_CONSOLE */
 
@@ -2459,8 +2410,6 @@ static int sci_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops sci_dev_pm_ops = {
-	.runtime_suspend = sci_runtime_suspend,
-	.runtime_resume = sci_runtime_resume,
 	.suspend	= sci_suspend,
 	.resume		= sci_resume,
 };
-- 
1.7.12.4

[PATCH 04/10] serial: sh-sci: console runtime PM support (revisit)

[Shinya Kuribayashi]

The commit 1ba7622094 (serial: sh-sci: console Runtime PM support,
from Magnus Damm <damm@opensource.se>, 2011-08-03), tried to support
console runtime PM, but unfortunately it didn't work for us for some
reason.  We did not investigated further at that time, instead would
like to propose a different approach.

In Linux tty/serial world, to get console PM work properly, a serial
client driver does not have to maintain .runtime_suspend()/..resume()
calls itself, but can leave console power power management handling to
the serial core driver.

This patch moves the sh-sci driver in that direction.

Notes:

* There is room to optimize console runtime PM more aggressively by
  maintaining additional local runtime PM calls, but as a first step
  having .pm() operation would suffice.

* We still have a couple of direct calls to sci_port_enable/..disable
  left in the driver.  We have to live with them, because they're out
  of serial core's help.

Signed-off-by: Teppei Kamijou <teppei.kamijou.yb@renesas.com>
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index e9e8883..e65e546 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1746,8 +1746,6 @@ static int sci_startup(struct uart_port *port)
 
 	dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);
 
-	sci_port_enable(s);
-
 	ret = sci_request_irq(s);
 	if (unlikely(ret < 0))
 		return ret;
@@ -1771,8 +1769,6 @@ static void sci_shutdown(struct uart_port *port)
 
 	sci_free_dma(port);
 	sci_free_irq(s);
-
-	sci_port_disable(s);
 }
 
 static unsigned int sci_scbrr_calc(unsigned int algo_id, unsigned int bps,
@@ -1921,6 +1917,21 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 	sci_port_disable(s);
 }
 
+static void sci_pm(struct uart_port *port, unsigned int state,
+		   unsigned int oldstate)
+{
+	struct sci_port *sci_port = to_sci_port(port);
+
+	switch (state) {
+	case 3:
+		sci_port_disable(sci_port);
+		break;
+	default:
+		sci_port_enable(sci_port);
+		break;
+	}
+}
+
 static const char *sci_type(struct uart_port *port)
 {
 	switch (port->type) {
@@ -2042,6 +2053,7 @@ static struct uart_ops sci_uart_ops = {
 	.startup	= sci_startup,
 	.shutdown	= sci_shutdown,
 	.set_termios	= sci_set_termios,
+	.pm		= sci_pm,
 	.type		= sci_type,
 	.release_port	= sci_release_port,
 	.request_port	= sci_request_port,
@@ -2195,16 +2207,12 @@ static void serial_console_write(struct console *co, const char *s,
 	struct uart_port *port = &sci_port->port;
 	unsigned short bits;
 
-	sci_port_enable(sci_port);
-
 	uart_console_write(port, s, count, serial_console_putchar);
 
 	/* wait until fifo is empty and last bit has been transmitted */
 	bits = SCxSR_TDxE(port) | SCxSR_TEND(port);
 	while ((serial_port_in(port, SCxSR) & bits) != bits)
 		cpu_relax();
-
-	sci_port_disable(sci_port);
 }
 
 static int __devinit serial_console_setup(struct console *co, char *options)
@@ -2236,12 +2244,9 @@ static int __devinit serial_console_setup(struct console *co, char *options)
 	if (unlikely(ret != 0))
 		return ret;
 
-	sci_port_enable(sci_port);
-
 	if (options)
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
 
-	/* TODO: disable clock */
 	return uart_set_options(port, co, baud, parity, bits, flow);
 }
 
-- 
1.7.12.4

[PATCH 05/10] serial: sh-sci: fix condition test to set SCBRR

[Shinya Kuribayashi]

SCBRR == 0 is valid value (divide by 1).

Signed-off-by: Takashi YOSHII <takashi.yoshii.zj@renesas.com>
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index e65e546..ffeca65 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1853,7 +1853,7 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 	dev_dbg(port->dev, "%s: SMR %x, t %x, SCSCR %x\n", __func__, smr_val, t,
 		s->cfg->scscr);
 
-	if (t > 0) {
+	if (t >= 0) {
 		if (t >= 256) {
 			serial_port_out(port, SCSMR, (serial_port_in(port, SCSMR) & ~3) | 1);
 			t >>= 2;
-- 
1.7.12.4

[PATCH 06/10] serial: sh-sci: support lower baud rate

[Shinya Kuribayashi]

Support prescaler 1/16 and 1/64, in addition to current 1 and 1/4.

Supporting below 2400bps was dropped long time ago in mainline.
Since then, setting lower rate has been resulting in erroneous
register value, without indicating any errors through API.

This patch adds more prescaler to support lower rates again.
This still doesn't check range, but we won't hit the case because
even 50bps at 48MHz clock is now supported.

Signed-off-by: Takashi Yoshii <takashi.yoshii.zj@renesas.com>
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index ffeca65..3021912 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1814,7 +1814,7 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 {
 	struct sci_port *s = to_sci_port(port);
 	struct plat_sci_reg *reg;
-	unsigned int baud, smr_val, max_baud;
+	unsigned int baud, smr_val, max_baud, cks;
 	int t = -1;
 
 	/*
@@ -1848,21 +1848,18 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	uart_update_timeout(port, termios->c_cflag, baud);
 
-	serial_port_out(port, SCSMR, smr_val);
+	for (cks = 0; t >= 256 && cks <= 3; cks++)
+		t >>= 2;
 
-	dev_dbg(port->dev, "%s: SMR %x, t %x, SCSCR %x\n", __func__, smr_val, t,
-		s->cfg->scscr);
+	dev_dbg(port->dev, "%s: SMR %x, cks %x, t %x, SCSCR %x\n",
+		__func__, smr_val, cks, t, s->cfg->scscr);
 
 	if (t >= 0) {
-		if (t >= 256) {
-			serial_port_out(port, SCSMR, (serial_port_in(port, SCSMR) & ~3) | 1);
-			t >>= 2;
-		} else
-			serial_port_out(port, SCSMR, serial_port_in(port, SCSMR) & ~3);
-
+		serial_port_out(port, SCSMR, (smr_val & ~3) | cks);
 		serial_port_out(port, SCBRR, t);
 		udelay((1000000+(baud-1)) / baud); /* Wait one bit interval */
-	}
+	} else
+		serial_port_out(port, SCSMR, smr_val);
 
 	sci_init_pins(port, termios->c_cflag);
 
-- 
1.7.12.4

[PATCH 07/10] serial: sh-sci: mask SCTFDR/RFDR according to fifosize

[Shinya Kuribayashi]

Current mask 0xff to SCTFDR/RFDR damages SCIFB, because the
registers on SCIFB have 9-bit data (0 to 256).

This patch changes the mask according to port->fifosize.
Though I'm not sure if the mask is really needed (I don't know if
there are variants which have non-zero upper bits), it is safer.

Signed-off-by: Takashi Yoshii <takashi.yoshii.zj@renesas.com>
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 3021912..63a3bd0 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -485,7 +485,7 @@ static int sci_txfill(struct uart_port *port)
 
 	reg = sci_getreg(port, SCTFDR);
 	if (reg->size)
-		return serial_port_in(port, SCTFDR) & 0xff;
+		return serial_port_in(port, SCTFDR) & ((port->fifosize << 1) - 1);
 
 	reg = sci_getreg(port, SCFDR);
 	if (reg->size)
@@ -505,7 +505,7 @@ static int sci_rxfill(struct uart_port *port)
 
 	reg = sci_getreg(port, SCRFDR);
 	if (reg->size)
-		return serial_port_in(port, SCRFDR) & 0xff;
+		return serial_port_in(port, SCRFDR) & ((port->fifosize << 1) - 1);
 
 	reg = sci_getreg(port, SCFDR);
 	if (reg->size)
-- 
1.7.12.4

[PATCH 08/10] serial: sh-sci: fix common SCIFB regmap definition

[Shinya Kuribayashi]

About FIFO count, there are two variants of SCIFs which show
a) TX count in upper, RX count in lower byte of FDR register
b) TX count in TFDR register, RX count in RFDR register

Common SCIFB regmap in current source code is defined as "a".
At least 7372 and 73a0 HW manual say their SICFB are "b".

This patch alters the definition to "b", considering the current
one has come from a mistake. The reason is as follows.

The flag SCIFB sh-sci driver means it has 256 byte FIFO.
The count is from 0(empty) to 256(full), that makes 9-bit.
Because FDR is 16-bit register, it can not hold two 9-bits.
That's why, SCIFB can not be "a".

Signed-off-by: Takashi Yoshii <takashi.yoshii.zj@renesas.com>
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 63a3bd0..502fa47 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -196,9 +196,9 @@ static struct plat_sci_reg sci_regmap[SCIx_NR_REGTYPES][SCIx_NR_REGS] = {
 		[SCxSR]		= { 0x14, 16 },
 		[SCxRDR]	= { 0x60,  8 },
 		[SCFCR]		= { 0x18, 16 },
-		[SCFDR]		= { 0x1c, 16 },
-		[SCTFDR]	= sci_reg_invalid,
-		[SCRFDR]	= sci_reg_invalid,
+		[SCFDR]		= sci_reg_invalid,
+		[SCTFDR]	= { 0x38, 16 },
+		[SCRFDR]	= { 0x3c, 16 },
 		[SCSPTR]	= sci_reg_invalid,
 		[SCLSR]		= sci_reg_invalid,
 	},
-- 
1.7.12.4

[PATCH 09/10] serial: sh-sci: add locking to console write function to avoid SMP lockup

[Shinya Kuribayashi]

Symptom:

When entering the suspend with Android logcat running, printk() call
gets stuck and never returns.  The issue can be observed at printk()s
on nonboot CPUs when going to offline with their interrupts disabled,
and never seen at boot CPU (core0 in our case).

Details:

serial_console_write() lacks of appropriate spinlock handling.

In SMP systems, as long as sci_transmit_chars() is being processed
at one CPU core, serial_console_write() can stuck at the other CPU
core(s), when it tries to access to the same serial port _without_
a proper locking.  serial_console_write() waits for the transmit FIFO
getting empty, while sci_transmit_chars() writes data to the FIFO.

In general, peripheral interrupts are routed to boot CPU (core0) by
Linux ARM standard affinity settings.  SCI(F) interrupts are handled
by core0, so sci_transmit_chars() is processed on core0 as well.

When logcat is running, it writes enormous log data to the kernel at
every moment, forever.  So core0 can repeatedly continue to process
sci_transmit_chars() in its interrupt handler, which eventually makes
the other CPU core(s) stuck at serial_console_write().

Looking at serial/8250.c, this is a known console write lockup issue
with SMP kernels.  Fix the sh-sci driver in the same way 8250.c does.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>

|commit ef605fdb33883d687cff5ba75095a91b313b4966
|Author: Rabin Vincent <rabin.vincent@stericsson.com>
|Date:   Tue Jan 17 11:52:28 2012 +0100
|
|    serial: amba-pl011: lock console writes against interrupts

|commit 9ec1882df244c4ee1baa692676fef5e8b0f5487d
|Author: Xinyu Chen <xinyu.chen@freescale.com>
|Date:   Mon Aug 27 09:36:51 2012 +0200
|
|    tty: serial: imx: console write routing is unsafe on SMP
---
 drivers/tty/serial/sh-sci.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 502fa47..0b8d029 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2202,7 +2202,21 @@ static void serial_console_write(struct console *co, const char *s,
 {
 	struct sci_port *sci_port = &sci_ports[co->index];
 	struct uart_port *port = &sci_port->port;
-	unsigned short bits;
+	unsigned short bits, ctrl;
+	unsigned long flags;
+	int locked = 1;
+
+	local_irq_save(flags);
+	if (port->sysrq)
+		locked = 0;
+	else if (oops_in_progress)
+		locked = spin_trylock(&port->lock);
+	else
+		spin_lock(&port->lock);
+
+	/* first save the SCSCR then disable the interrupts */
+	ctrl = serial_port_in(port, SCSCR);
+	serial_port_out(port, SCSCR, sci_port->cfg->scscr);
 
 	uart_console_write(port, s, count, serial_console_putchar);
 
@@ -2210,6 +2224,13 @@ static void serial_console_write(struct console *co, const char *s,
 	bits = SCxSR_TDxE(port) | SCxSR_TEND(port);
 	while ((serial_port_in(port, SCxSR) & bits) != bits)
 		cpu_relax();
+
+	/* restore the SCSCR */
+	serial_port_out(port, SCSCR, ctrl);
+
+	if (locked)
+		spin_unlock(&port->lock);
+	local_irq_restore(flags);
 }
 
 static int __devinit serial_console_setup(struct console *co, char *options)
-- 
1.7.12.4

[PATCH 10/10] serial: sh-sci: fix possible race cases on SCSCR register accesses

[Shinya Kuribayashi]

In the previous commit, console write function (serial_console_write)
is changed to disable SCI interrupts while printing console strings.
This introduces possible race cases in the serial startup / shutdown
functions on SMP systems.

This patch fixes the sh-sci in the same way as commit 9ec1882df2
(tty: serial: imx: console write routing is unsafe on SMP, from
Xinyu Chen <xinyu.chen@freescale.com>, 2012-08-27) did.

There could be several consumers of the console,
* the kernel printk
* the init process using /dev/kmsg to call printk to show log
* shell, which opens /dev/console and writes with sys_write()

The shell goes into the normal UART open() and write() system calls,
while the other two go into the console operations.  The open() call
invokes serial startup function (sci_startup), which will write to
the SCSCR register (to enable or disable SCI interrupts) without any
locking.  This will conflict with the console serial function.

Add spinlock protections in sci_startup() and sci_shutdown() properly.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 0b8d029..8e3c7f7 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1742,6 +1742,7 @@ static inline void sci_free_dma(struct uart_port *port)
 static int sci_startup(struct uart_port *port)
 {
 	struct sci_port *s = to_sci_port(port);
+	unsigned long flags;
 	int ret;
 
 	dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);
@@ -1752,8 +1753,10 @@ static int sci_startup(struct uart_port *port)
 
 	sci_request_dma(port);
 
+	spin_lock_irqsave(&port->lock, flags);
 	sci_start_tx(port);
 	sci_start_rx(port);
+	spin_unlock_irqrestore(&port->lock, flags);
 
 	return 0;
 }
@@ -1761,11 +1764,14 @@ static int sci_startup(struct uart_port *port)
 static void sci_shutdown(struct uart_port *port)
 {
 	struct sci_port *s = to_sci_port(port);
+	unsigned long flags;
 
 	dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);
 
+	spin_lock_irqsave(&port->lock, flags);
 	sci_stop_rx(port);
 	sci_stop_tx(port);
+	spin_unlock_irqrestore(&port->lock, flags);
 
 	sci_free_dma(port);
 	sci_free_irq(s);
-- 
1.7.12.4

Re: [PATCH 00/10] serial: sh-sci fixes - console PM, SCIFB, SMP lockup, etc.

[Shinya Kuribayashi]

On 11/7/2012 3:57 PM, Shinya Kuribayashi wrote:
> Hi Paul and serial forks,
[...]
> Takashi YOSHII (4):
>       serial: sh-sci: fix condition test to set SCBRR
>       serial: sh-sci: support lower baud rate
>       serial: sh-sci: mask SCTFDR/RFDR according to fifosize
>       serial: sh-sci: fix common SCIFB regmap definition
> 
> Teppei Kamijou (1):
>       serial: sh-sci: console runtime PM support (revisit)
> 
>  drivers/tty/serial/sh-sci.c | 140 ++++++++++++++++++--------------------------
>  1 file changed, 56 insertions(+), 84 deletions(-)

Oops, I forgot to append proper From: author lines in five commits
(patch 04/10 to 09/10).  Didn't mean to supersede your authoship.

I'd respin the patchset later, waiting for some feedback.
--
Shinya Kuribayashi
Renesas Electronics

Re: [PATCH 00/10] serial: sh-sci fixes - console PM, SCIFB, SMP lockup, etc.

[Greg KH]

On Wed, Nov 07, 2012 at 04:18:38PM +0900, Shinya Kuribayashi wrote:
> On 11/7/2012 3:57 PM, Shinya Kuribayashi wrote:
> > Hi Paul and serial forks,
> [...]
> > Takashi YOSHII (4):
> >       serial: sh-sci: fix condition test to set SCBRR
> >       serial: sh-sci: support lower baud rate
> >       serial: sh-sci: mask SCTFDR/RFDR according to fifosize
> >       serial: sh-sci: fix common SCIFB regmap definition
> > 
> > Teppei Kamijou (1):
> >       serial: sh-sci: console runtime PM support (revisit)
> > 
> >  drivers/tty/serial/sh-sci.c | 140 ++++++++++++++++++--------------------------
> >  1 file changed, 56 insertions(+), 84 deletions(-)
> 
> Oops, I forgot to append proper From: author lines in five commits
> (patch 04/10 to 09/10).  Didn't mean to supersede your authoship.
> 
> I'd respin the patchset later, waiting for some feedback.

Can you please resend this with this fixed up if you want to get this
into 3.8?

thanks,

greg k-h

[PATCH v2 00/10] serial: sh-sci fixes - console PM, SCIFB, SMP lockup, etc.

[Shinya Kuribayashi]

Hi Greg,

On 11/16/2012 10:01 AM, Greg KH wrote:> On Wed, Nov 07, 2012 at 04:18:38PM +0900, Shinya Kuribayashi wrote:
>> On 11/7/2012 3:57 PM, Shinya Kuribayashi wrote:
>>> Hi Paul and serial forks,
>> [...]
>>> Takashi YOSHII (4):
>>>       serial: sh-sci: fix condition test to set SCBRR
>>>       serial: sh-sci: support lower baud rate
>>>       serial: sh-sci: mask SCTFDR/RFDR according to fifosize
>>>       serial: sh-sci: fix common SCIFB regmap definition
>>>
>>> Teppei Kamijou (1):
>>>       serial: sh-sci: console runtime PM support (revisit)
>>>
>>>  drivers/tty/serial/sh-sci.c | 140 ++++++++++++++++++--------------------------
>>>  1 file changed, 56 insertions(+), 84 deletions(-)
>>
>> Oops, I forgot to append proper From: author lines in five commits
>> (patch 04/10 to 09/10).  Didn't mean to supersede your authoship.
>>
>> I'd respin the patchset later, waiting for some feedback.
> 
> Can you please resend this with this fixed up if you want to get this
> into 3.8?

Thanks for your kind reminder.  Here's v2, providing From: authorship
lines in five patches (04 to 08).  No logical change in the patches.

--- 8< ---

V2: provide missing authorship From: lines in five patches (04 to 08)

Here are sh-sci serial driver fixes that have been cooking for months
privately on my end:
* rework console PM with the help of serial-core's uart_change_pm()
* provide missing piece in SCIFB support (from Yoshii-san)
* address console write function vs. SMP lockup issue

and so on.  Comments and suggestions are welcome.

Build tested with vanilla v3.6, but not boot- nor PM-tested with
newer kernels after v3.4.  Anyone with can-sleep shmobile systems,
please give it try.

Thanks in advance,

----------------------------------------------------------------
Shinya Kuribayashi (5):
      Revert "sh-sci / PM: Avoid deadlocking runtime PM"
      Revert "sh-sci / PM: Use power.irq_safe"
      Partially revert "serial: sh-sci: console Runtime PM support"
      serial: sh-sci: add locking to console write function to avoid SMP lockup
      serial: sh-sci: fix possible race cases on SCSCR register accesses

Takashi YOSHII (4):
      serial: sh-sci: fix condition test to set SCBRR
      serial: sh-sci: support lower baud rate
      serial: sh-sci: mask SCTFDR/RFDR according to fifosize
      serial: sh-sci: fix common SCIFB regmap definition

Teppei Kamijou (1):
      serial: sh-sci: console runtime PM support (revisit)

 drivers/tty/serial/sh-sci.c | 140 ++++++++++++++++++--------------------------
 1 file changed, 56 insertions(+), 84 deletions(-)

-- 
Shinya Kuribayashi
Renesas Electronics

[PATCH v2 01/10] Revert "sh-sci / PM: Avoid deadlocking runtime PM"

[Shinya Kuribayashi]

This reverts commit 048be431e4 (sh-sci / PM: Avoid deadlocking runtime
PM, from Rafael J. Wysocki <rjw@sisk.pl>, 2012-03-09).

In order to get console PM work properly, we should implement uart_ops
->pm() operation, rather than sprinkle band-ading runtime PM calls in
the driver.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 9be296c..526ad04 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1752,8 +1752,6 @@ static int sci_startup(struct uart_port *port)
 
 	dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);
 
-	pm_runtime_put_noidle(port->dev);
-
 	sci_port_enable(s);
 
 	ret = sci_request_irq(s);
@@ -1781,8 +1779,6 @@ static void sci_shutdown(struct uart_port *port)
 	sci_free_irq(s);
 
 	sci_port_disable(s);
-
-	pm_runtime_get_noresume(port->dev);
 }
 
 static unsigned int sci_scbrr_calc(unsigned int algo_id, unsigned int bps,
@@ -2121,7 +2117,6 @@ static int __devinit sci_init_single(struct platform_device *dev,
 		sci_init_gpios(sci_port);
 
 		pm_runtime_irq_safe(&dev->dev);
-		pm_runtime_get_noresume(&dev->dev);
 		pm_runtime_enable(&dev->dev);
 	}
 
-- 
1.7.12.4

[PATCH v2 02/10] Revert "sh-sci / PM: Use power.irq_safe"

[Shinya Kuribayashi]

This reverts commit 5a50a01bf0 (sh-sci / PM: Use power.irq_safe, from
Rafael J. Wysocki <rjw@sisk.pl>, 2011-08-24).

In order to get console PM work properly, we should implement uart_ops
->pm() operation, rather than sprinkle band-ading runtime PM calls in
the driver.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 526ad04..9abe977 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2116,7 +2116,6 @@ static int __devinit sci_init_single(struct platform_device *dev,
 
 		sci_init_gpios(sci_port);
 
-		pm_runtime_irq_safe(&dev->dev);
 		pm_runtime_enable(&dev->dev);
 	}
 
-- 
1.7.12.4

[PATCH v2 03/10] Partially revert "serial: sh-sci: console Runtime PM support"

[Shinya Kuribayashi]

This partially reverts commit 1ba7622094 (serial: sh-sci: console
Runtime PM support, from Magnus Damm <damm@opensource.se>, 2011-08-03).

The generic 'serial_core' can take care of console PM maintenance,
so all (or at least the first thing) we have to do to get console PM
work properly, is to implement uart_ops ->pm() operation in the sh-sci
serial client driver.

This patch partially reverts the commit above, but leaving sci_reset()
change in place, because sci_reset() is already part of another commit
(73c3d53f38 serial: sh-sci: Avoid FIFO clear for MCE toggle.).

A revised version of console PM support follows next.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 53 +--------------------------------------------
 1 file changed, 1 insertion(+), 52 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 9abe977..e9e8883 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -99,12 +99,6 @@ struct sci_port {
 #endif
 
 	struct notifier_block		freq_transition;
-
-#ifdef CONFIG_SERIAL_SH_SCI_CONSOLE
-	unsigned short saved_smr;
-	unsigned short saved_fcr;
-	unsigned char saved_brr;
-#endif
 };
 
 /* Function prototypes */
@@ -2247,8 +2241,7 @@ static int __devinit serial_console_setup(struct console *co, char *options)
 	if (options)
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
 
-	sci_port_disable(sci_port);
-
+	/* TODO: disable clock */
 	return uart_set_options(port, co, baud, parity, bits, flow);
 }
 
@@ -2291,46 +2284,6 @@ static int __devinit sci_probe_earlyprintk(struct platform_device *pdev)
 	return 0;
 }
 
-#define uart_console(port)	((port)->cons->index == (port)->line)
-
-static int sci_runtime_suspend(struct device *dev)
-{
-	struct sci_port *sci_port = dev_get_drvdata(dev);
-	struct uart_port *port = &sci_port->port;
-
-	if (uart_console(port)) {
-		struct plat_sci_reg *reg;
-
-		sci_port->saved_smr = serial_port_in(port, SCSMR);
-		sci_port->saved_brr = serial_port_in(port, SCBRR);
-
-		reg = sci_getreg(port, SCFCR);
-		if (reg->size)
-			sci_port->saved_fcr = serial_port_in(port, SCFCR);
-		else
-			sci_port->saved_fcr = 0;
-	}
-	return 0;
-}
-
-static int sci_runtime_resume(struct device *dev)
-{
-	struct sci_port *sci_port = dev_get_drvdata(dev);
-	struct uart_port *port = &sci_port->port;
-
-	if (uart_console(port)) {
-		sci_reset(port);
-		serial_port_out(port, SCSMR, sci_port->saved_smr);
-		serial_port_out(port, SCBRR, sci_port->saved_brr);
-
-		if (sci_port->saved_fcr)
-			serial_port_out(port, SCFCR, sci_port->saved_fcr);
-
-		serial_port_out(port, SCSCR, sci_port->cfg->scscr);
-	}
-	return 0;
-}
-
 #define SCI_CONSOLE	(&serial_console)
 
 #else
@@ -2340,8 +2293,6 @@ static inline int __devinit sci_probe_earlyprintk(struct platform_device *pdev)
 }
 
 #define SCI_CONSOLE	NULL
-#define sci_runtime_suspend	NULL
-#define sci_runtime_resume	NULL
 
 #endif /* CONFIG_SERIAL_SH_SCI_CONSOLE */
 
@@ -2459,8 +2410,6 @@ static int sci_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops sci_dev_pm_ops = {
-	.runtime_suspend = sci_runtime_suspend,
-	.runtime_resume = sci_runtime_resume,
 	.suspend	= sci_suspend,
 	.resume		= sci_resume,
 };
-- 
1.7.12.4

[PATCH v2 04/10] serial: sh-sci: console runtime PM support (revisit)

[Shinya Kuribayashi]

From: Teppei Kamijou <teppei.kamijou.yb@renesas.com>

The commit 1ba7622094 (serial: sh-sci: console Runtime PM support,
from Magnus Damm <damm@opensource.se>, 2011-08-03), tried to support
console runtime PM, but unfortunately it didn't work for us for some
reason.  We did not investigated further at that time, instead would
like to propose a different approach.

In Linux tty/serial world, to get console PM work properly, a serial
client driver does not have to maintain .runtime_suspend()/..resume()
calls itself, but can leave console power power management handling to
the serial core driver.

This patch moves the sh-sci driver in that direction.

Notes:

* There is room to optimize console runtime PM more aggressively by
  maintaining additional local runtime PM calls, but as a first step
  having .pm() operation would suffice.

* We still have a couple of direct calls to sci_port_enable/..disable
  left in the driver.  We have to live with them, because they're out
  of serial core's help.

Signed-off-by: Teppei Kamijou <teppei.kamijou.yb@renesas.com>
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index e9e8883..e65e546 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1746,8 +1746,6 @@ static int sci_startup(struct uart_port *port)
 
 	dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);
 
-	sci_port_enable(s);
-
 	ret = sci_request_irq(s);
 	if (unlikely(ret < 0))
 		return ret;
@@ -1771,8 +1769,6 @@ static void sci_shutdown(struct uart_port *port)
 
 	sci_free_dma(port);
 	sci_free_irq(s);
-
-	sci_port_disable(s);
 }
 
 static unsigned int sci_scbrr_calc(unsigned int algo_id, unsigned int bps,
@@ -1921,6 +1917,21 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 	sci_port_disable(s);
 }
 
+static void sci_pm(struct uart_port *port, unsigned int state,
+		   unsigned int oldstate)
+{
+	struct sci_port *sci_port = to_sci_port(port);
+
+	switch (state) {
+	case 3:
+		sci_port_disable(sci_port);
+		break;
+	default:
+		sci_port_enable(sci_port);
+		break;
+	}
+}
+
 static const char *sci_type(struct uart_port *port)
 {
 	switch (port->type) {
@@ -2042,6 +2053,7 @@ static struct uart_ops sci_uart_ops = {
 	.startup	= sci_startup,
 	.shutdown	= sci_shutdown,
 	.set_termios	= sci_set_termios,
+	.pm		= sci_pm,
 	.type		= sci_type,
 	.release_port	= sci_release_port,
 	.request_port	= sci_request_port,
@@ -2195,16 +2207,12 @@ static void serial_console_write(struct console *co, const char *s,
 	struct uart_port *port = &sci_port->port;
 	unsigned short bits;
 
-	sci_port_enable(sci_port);
-
 	uart_console_write(port, s, count, serial_console_putchar);
 
 	/* wait until fifo is empty and last bit has been transmitted */
 	bits = SCxSR_TDxE(port) | SCxSR_TEND(port);
 	while ((serial_port_in(port, SCxSR) & bits) != bits)
 		cpu_relax();
-
-	sci_port_disable(sci_port);
 }
 
 static int __devinit serial_console_setup(struct console *co, char *options)
@@ -2236,12 +2244,9 @@ static int __devinit serial_console_setup(struct console *co, char *options)
 	if (unlikely(ret != 0))
 		return ret;
 
-	sci_port_enable(sci_port);
-
 	if (options)
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
 
-	/* TODO: disable clock */
 	return uart_set_options(port, co, baud, parity, bits, flow);
 }
 
-- 
1.7.12.4

[PATCH v2 05/10] serial: sh-sci: fix condition test to set SCBRR

[Shinya Kuribayashi]

From: Takashi Yoshii <takashi.yoshii.zj@renesas.com>

SCBRR == 0 is valid value (divide by 1).

Signed-off-by: Takashi Yoshii <takashi.yoshii.zj@renesas.com>
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index e65e546..ffeca65 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1853,7 +1853,7 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 	dev_dbg(port->dev, "%s: SMR %x, t %x, SCSCR %x\n", __func__, smr_val, t,
 		s->cfg->scscr);
 
-	if (t > 0) {
+	if (t >= 0) {
 		if (t >= 256) {
 			serial_port_out(port, SCSMR, (serial_port_in(port, SCSMR) & ~3) | 1);
 			t >>= 2;
-- 
1.7.12.4

[PATCH v2 06/10] serial: sh-sci: support lower baud rate

[Shinya Kuribayashi]

From: Takashi Yoshii <takashi.yoshii.zj@renesas.com>

Support prescaler 1/16 and 1/64, in addition to current 1 and 1/4.

Supporting below 2400bps was dropped long time ago in mainline.
Since then, setting lower rate has been resulting in erroneous
register value, without indicating any errors through API.

This patch adds more prescaler to support lower rates again.
This still doesn't check range, but we won't hit the case because
even 50bps at 48MHz clock is now supported.

Signed-off-by: Takashi Yoshii <takashi.yoshii.zj@renesas.com>
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index ffeca65..3021912 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1814,7 +1814,7 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 {
 	struct sci_port *s = to_sci_port(port);
 	struct plat_sci_reg *reg;
-	unsigned int baud, smr_val, max_baud;
+	unsigned int baud, smr_val, max_baud, cks;
 	int t = -1;
 
 	/*
@@ -1848,21 +1848,18 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	uart_update_timeout(port, termios->c_cflag, baud);
 
-	serial_port_out(port, SCSMR, smr_val);
+	for (cks = 0; t >= 256 && cks <= 3; cks++)
+		t >>= 2;
 
-	dev_dbg(port->dev, "%s: SMR %x, t %x, SCSCR %x\n", __func__, smr_val, t,
-		s->cfg->scscr);
+	dev_dbg(port->dev, "%s: SMR %x, cks %x, t %x, SCSCR %x\n",
+		__func__, smr_val, cks, t, s->cfg->scscr);
 
 	if (t >= 0) {
-		if (t >= 256) {
-			serial_port_out(port, SCSMR, (serial_port_in(port, SCSMR) & ~3) | 1);
-			t >>= 2;
-		} else
-			serial_port_out(port, SCSMR, serial_port_in(port, SCSMR) & ~3);
-
+		serial_port_out(port, SCSMR, (smr_val & ~3) | cks);
 		serial_port_out(port, SCBRR, t);
 		udelay((1000000+(baud-1)) / baud); /* Wait one bit interval */
-	}
+	} else
+		serial_port_out(port, SCSMR, smr_val);
 
 	sci_init_pins(port, termios->c_cflag);
 
-- 
1.7.12.4

[PATCH v2 07/10] serial: sh-sci: mask SCTFDR/RFDR according to fifosize

[Shinya Kuribayashi]

From: Takashi Yoshii <takashi.yoshii.zj@renesas.com>

Current mask 0xff to SCTFDR/RFDR damages SCIFB, because the
registers on SCIFB have 9-bit data (0 to 256).

This patch changes the mask according to port->fifosize.
Though I'm not sure if the mask is really needed (I don't know if
there are variants which have non-zero upper bits), it is safer.

Signed-off-by: Takashi Yoshii <takashi.yoshii.zj@renesas.com>
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 3021912..63a3bd0 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -485,7 +485,7 @@ static int sci_txfill(struct uart_port *port)
 
 	reg = sci_getreg(port, SCTFDR);
 	if (reg->size)
-		return serial_port_in(port, SCTFDR) & 0xff;
+		return serial_port_in(port, SCTFDR) & ((port->fifosize << 1) - 1);
 
 	reg = sci_getreg(port, SCFDR);
 	if (reg->size)
@@ -505,7 +505,7 @@ static int sci_rxfill(struct uart_port *port)
 
 	reg = sci_getreg(port, SCRFDR);
 	if (reg->size)
-		return serial_port_in(port, SCRFDR) & 0xff;
+		return serial_port_in(port, SCRFDR) & ((port->fifosize << 1) - 1);
 
 	reg = sci_getreg(port, SCFDR);
 	if (reg->size)
-- 
1.7.12.4

[PATCH v2 08/10] serial: sh-sci: fix common SCIFB regmap definition

[Shinya Kuribayashi]

From: Takashi Yoshii <takashi.yoshii.zj@renesas.com>

About FIFO count, there are two variants of SCIFs which show
a) TX count in upper, RX count in lower byte of FDR register
b) TX count in TFDR register, RX count in RFDR register

Common SCIFB regmap in current source code is defined as "a".
At least 7372 and 73a0 HW manual say their SICFB are "b".

This patch alters the definition to "b", considering the current
one has come from a mistake. The reason is as follows.

The flag SCIFB sh-sci driver means it has 256 byte FIFO.
The count is from 0(empty) to 256(full), that makes 9-bit.
Because FDR is 16-bit register, it can not hold two 9-bits.
That's why, SCIFB can not be "a".

Signed-off-by: Takashi Yoshii <takashi.yoshii.zj@renesas.com>
Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 63a3bd0..502fa47 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -196,9 +196,9 @@ static struct plat_sci_reg sci_regmap[SCIx_NR_REGTYPES][SCIx_NR_REGS] = {
 		[SCxSR]		= { 0x14, 16 },
 		[SCxRDR]	= { 0x60,  8 },
 		[SCFCR]		= { 0x18, 16 },
-		[SCFDR]		= { 0x1c, 16 },
-		[SCTFDR]	= sci_reg_invalid,
-		[SCRFDR]	= sci_reg_invalid,
+		[SCFDR]		= sci_reg_invalid,
+		[SCTFDR]	= { 0x38, 16 },
+		[SCRFDR]	= { 0x3c, 16 },
 		[SCSPTR]	= sci_reg_invalid,
 		[SCLSR]		= sci_reg_invalid,
 	},
-- 
1.7.12.4

[PATCH v2 09/10] serial: sh-sci: add locking to console write function to avoid SMP lockup

[Shinya Kuribayashi]

Symptom:

When entering the suspend with Android logcat running, printk() call
gets stuck and never returns.  The issue can be observed at printk()s
on nonboot CPUs when going to offline with their interrupts disabled,
and never seen at boot CPU (core0 in our case).

Details:

serial_console_write() lacks of appropriate spinlock handling.

In SMP systems, as long as sci_transmit_chars() is being processed
at one CPU core, serial_console_write() can stuck at the other CPU
core(s), when it tries to access to the same serial port _without_
a proper locking.  serial_console_write() waits for the transmit FIFO
getting empty, while sci_transmit_chars() writes data to the FIFO.

In general, peripheral interrupts are routed to boot CPU (core0) by
Linux ARM standard affinity settings.  SCI(F) interrupts are handled
by core0, so sci_transmit_chars() is processed on core0 as well.

When logcat is running, it writes enormous log data to the kernel at
every moment, forever.  So core0 can repeatedly continue to process
sci_transmit_chars() in its interrupt handler, which eventually makes
the other CPU core(s) stuck at serial_console_write().

Looking at serial/8250.c, this is a known console write lockup issue
with SMP kernels.  Fix the sh-sci driver in the same way 8250.c does.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---

Similar fixes can be found at:

|commit ef605fdb33883d687cff5ba75095a91b313b4966
|Author: Rabin Vincent <rabin.vincent@stericsson.com>
|Date:   Tue Jan 17 11:52:28 2012 +0100
|
|    serial: amba-pl011: lock console writes against interrupts

|commit 9ec1882df244c4ee1baa692676fef5e8b0f5487d
|Author: Xinyu Chen <xinyu.chen@freescale.com>
|Date:   Mon Aug 27 09:36:51 2012 +0200
|
|    tty: serial: imx: console write routing is unsafe on SMP

 drivers/tty/serial/sh-sci.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 502fa47..0b8d029 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2202,7 +2202,21 @@ static void serial_console_write(struct console *co, const char *s,
 {
 	struct sci_port *sci_port = &sci_ports[co->index];
 	struct uart_port *port = &sci_port->port;
-	unsigned short bits;
+	unsigned short bits, ctrl;
+	unsigned long flags;
+	int locked = 1;
+
+	local_irq_save(flags);
+	if (port->sysrq)
+		locked = 0;
+	else if (oops_in_progress)
+		locked = spin_trylock(&port->lock);
+	else
+		spin_lock(&port->lock);
+
+	/* first save the SCSCR then disable the interrupts */
+	ctrl = serial_port_in(port, SCSCR);
+	serial_port_out(port, SCSCR, sci_port->cfg->scscr);
 
 	uart_console_write(port, s, count, serial_console_putchar);
 
@@ -2210,6 +2224,13 @@ static void serial_console_write(struct console *co, const char *s,
 	bits = SCxSR_TDxE(port) | SCxSR_TEND(port);
 	while ((serial_port_in(port, SCxSR) & bits) != bits)
 		cpu_relax();
+
+	/* restore the SCSCR */
+	serial_port_out(port, SCSCR, ctrl);
+
+	if (locked)
+		spin_unlock(&port->lock);
+	local_irq_restore(flags);
 }
 
 static int __devinit serial_console_setup(struct console *co, char *options)
-- 
1.7.12.4

[PATCH v2 10/10] serial: sh-sci: fix possible race cases on SCSCR register accesses

[Shinya Kuribayashi]

In the previous commit, console write function (serial_console_write)
is changed to disable SCI interrupts while printing console strings.
This introduces possible race cases in the serial startup / shutdown
functions on SMP systems.

This patch fixes the sh-sci in the same way as commit 9ec1882df2
(tty: serial: imx: console write routing is unsafe on SMP, from
Xinyu Chen <xinyu.chen@freescale.com>, 2012-08-27) did.

There could be several consumers of the console,
* the kernel printk
* the init process using /dev/kmsg to call printk to show log
* shell, which opens /dev/console and writes with sys_write()

The shell goes into the normal UART open() and write() system calls,
while the other two go into the console operations.  The open() call
invokes serial startup function (sci_startup), which will write to
the SCSCR register (to enable or disable SCI interrupts) without any
locking.  This will conflict with the console serial function.

Add spinlock protections in sci_startup() and sci_shutdown() properly.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi.px@renesas.com>
---
 drivers/tty/serial/sh-sci.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 0b8d029..8e3c7f7 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1742,6 +1742,7 @@ static inline void sci_free_dma(struct uart_port *port)
 static int sci_startup(struct uart_port *port)
 {
 	struct sci_port *s = to_sci_port(port);
+	unsigned long flags;
 	int ret;
 
 	dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);
@@ -1752,8 +1753,10 @@ static int sci_startup(struct uart_port *port)
 
 	sci_request_dma(port);
 
+	spin_lock_irqsave(&port->lock, flags);
 	sci_start_tx(port);
 	sci_start_rx(port);
+	spin_unlock_irqrestore(&port->lock, flags);
 
 	return 0;
 }
@@ -1761,11 +1764,14 @@ static int sci_startup(struct uart_port *port)
 static void sci_shutdown(struct uart_port *port)
 {
 	struct sci_port *s = to_sci_port(port);
+	unsigned long flags;
 
 	dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);
 
+	spin_lock_irqsave(&port->lock, flags);
 	sci_stop_rx(port);
 	sci_stop_tx(port);
+	spin_unlock_irqrestore(&port->lock, flags);
 
 	sci_free_dma(port);
 	sci_free_irq(s);
-- 
1.7.12.4

Re: [PATCH v2 00/10] serial: sh-sci fixes - console PM, SCIFB, SMP lockup, etc.

[Greg KH]

On Fri, Nov 16, 2012 at 10:49:15AM +0900, Shinya Kuribayashi wrote:
> Hi Greg,
> 
> On 11/16/2012 10:01 AM, Greg KH wrote:> On Wed, Nov 07, 2012 at 04:18:38PM +0900, Shinya Kuribayashi wrote:
> >> On 11/7/2012 3:57 PM, Shinya Kuribayashi wrote:
> >>> Hi Paul and serial forks,
> >> [...]
> >>> Takashi YOSHII (4):
> >>>       serial: sh-sci: fix condition test to set SCBRR
> >>>       serial: sh-sci: support lower baud rate
> >>>       serial: sh-sci: mask SCTFDR/RFDR according to fifosize
> >>>       serial: sh-sci: fix common SCIFB regmap definition
> >>>
> >>> Teppei Kamijou (1):
> >>>       serial: sh-sci: console runtime PM support (revisit)
> >>>
> >>>  drivers/tty/serial/sh-sci.c | 140 ++++++++++++++++++--------------------------
> >>>  1 file changed, 56 insertions(+), 84 deletions(-)
> >>
> >> Oops, I forgot to append proper From: author lines in five commits
> >> (patch 04/10 to 09/10).  Didn't mean to supersede your authoship.
> >>
> >> I'd respin the patchset later, waiting for some feedback.
> > 
> > Can you please resend this with this fixed up if you want to get this
> > into 3.8?
> 
> Thanks for your kind reminder.  Here's v2, providing From: authorship
> lines in five patches (04 to 08).  No logical change in the patches.

All now applied, thanks.

greg k-h