All of lore.kernel.org
 help / color / mirror / Atom feed
From: Grygorii Strashko <grygorii.strashko@ti.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: <linux-serial@vger.kernel.org>, <linux-omap@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	Grygorii Strashko <grygorii.strashko@ti.com>,
	Tony Lindgren <tony@atomide.com>, Rajendra Nayak <rnayak@ti.com>,
	Felipe Balbi <balbi@ti.com>, Kevin Hilman <khilman@linaro.org>
Subject: [PATCH 2/2] serial: omap: fix wrong context restoration on init
Date: Fri, 12 Jul 2013 14:55:42 +0300	[thread overview]
Message-ID: <1373630142-21765-2-git-send-email-grygorii.strashko@ti.com> (raw)
In-Reply-To: <1373630142-21765-1-git-send-email-grygorii.strashko@ti.com>

Since commit a630fbf "serial: omap: Fix device tree based PM runtime"
the OMAP serial driver will always try to restore its context in
serial_omap_runtime_resume(). But the problem is that during driver
initialization the UART context is not ready yet and, as result, first
call to pm_runtime_get*() will cause UART register overwriting by all
zeros. This causes Kernel boot hang in case if "earlyprintk" feature is
enabled at least [1].

Unfortunately, there is no exact place in driver now where we can
determine that UART context is ready - most of registers configured in
serial_omap_set_termios(), but some of them in other places.
More over, even if PM runtime will be disabled (blocked) during OMAP
serial driver probe() execution [2],[3] it will fix only console UART,
but context of other UARTs will be overwriting by all zeros during first
access to the corresponding UART.

To fix this issue:
- introduce additional "initialized" flag and update PM runtime callback
to do nothing if its not set. Set "initialized" at the end of probe().
- read current UART registers configuration in probe and use it by
default.

[1] http://www.spinics.net/lists/arm-kernel/msg256828.html
[2] http://www.spinics.net/lists/arm-kernel/msg258062.html
[3] http://www.spinics.net/lists/arm-kernel/msg258040.html

CC: Tony Lindgren <tony@atomide.com>
CC: Rajendra Nayak <rnayak@ti.com>
CC: Felipe Balbi <balbi@ti.com>
CC: Kevin Hilman <khilman@linaro.org>

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
tested on OMAP4 SDP with and without earlyprintk enabled.
 drivers/tty/serial/omap-serial.c |   27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index f39bf0c..e1e9667 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -162,6 +162,7 @@ struct uart_omap_port {
 	struct work_struct	qos_work;
 	struct pinctrl		*pins;
 	bool			is_suspending;
+	bool			initialized;
 };
 
 #define to_uart_omap_port(p)	((container_of((p), struct uart_omap_port, port)))
@@ -1395,6 +1396,19 @@ static struct omap_uart_port_info *of_get_uart_port_info(struct device *dev)
 	return omap_up_info;
 }
 
+static void serial_omap_save_context_def(struct uart_omap_port *up)
+{
+	up->dll = serial_in(up, UART_DLL);
+	up->dlh = serial_in(up, UART_DLM);
+	up->ier = serial_in(up, UART_IER);
+	up->fcr = serial_in(up, UART_FCR);
+	up->mcr = serial_in(up, UART_MCR);
+	up->scr = serial_in(up, UART_OMAP_SCR);
+	up->efr = serial_in(up, UART_EFR);
+	up->lcr = serial_in(up, UART_LCR);
+	up->mdr1 = serial_in(up, UART_OMAP_MDR1);
+}
+
 static int serial_omap_probe(struct platform_device *pdev)
 {
 	struct uart_omap_port	*up;
@@ -1518,10 +1532,14 @@ static int serial_omap_probe(struct platform_device *pdev)
 	ui[up->port.line] = up;
 	serial_omap_add_console_port(up);
 
+	serial_omap_save_context_def(up);
+
 	ret = uart_add_one_port(&serial_omap_reg, &up->port);
 	if (ret != 0)
 		goto err_add_port;
 
+	up->initialized = true;
+
 	pm_runtime_mark_last_busy(up->dev);
 	pm_runtime_put_autosuspend(up->dev);
 	return 0;
@@ -1619,6 +1637,9 @@ static int serial_omap_runtime_suspend(struct device *dev)
 	if (!up)
 		return -EINVAL;
 
+	if (!up->initialized)
+		return 0;
+
 	/*
 	* When using 'no_console_suspend', the console UART must not be
 	* suspended. Since driver suspend is managed by runtime suspend,
@@ -1652,8 +1673,12 @@ static int serial_omap_runtime_suspend(struct device *dev)
 static int serial_omap_runtime_resume(struct device *dev)
 {
 	struct uart_omap_port *up = dev_get_drvdata(dev);
+	int loss_cnt;
+
+	if (!up->initialized)
+		return 0;
 
-	int loss_cnt = serial_omap_get_context_loss_count(up);
+	loss_cnt = serial_omap_get_context_loss_count(up);
 
 	if (loss_cnt < 0) {
 		dev_dbg(dev, "serial_omap_get_context_loss_count failed : %d\n",
-- 
1.7.9.5


WARNING: multiple messages have this Message-ID (diff)
From: Grygorii Strashko <grygorii.strashko@ti.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-serial@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Grygorii Strashko <grygorii.strashko@ti.com>,
	Tony Lindgren <tony@atomide.com>, Rajendra Nayak <rnayak@ti.com>,
	Felipe Balbi <balbi@ti.com>, Kevin Hilman <khilman@linaro.org>
Subject: [PATCH 2/2] serial: omap: fix wrong context restoration on init
Date: Fri, 12 Jul 2013 14:55:42 +0300	[thread overview]
Message-ID: <1373630142-21765-2-git-send-email-grygorii.strashko@ti.com> (raw)
In-Reply-To: <1373630142-21765-1-git-send-email-grygorii.strashko@ti.com>

Since commit a630fbf "serial: omap: Fix device tree based PM runtime"
the OMAP serial driver will always try to restore its context in
serial_omap_runtime_resume(). But the problem is that during driver
initialization the UART context is not ready yet and, as result, first
call to pm_runtime_get*() will cause UART register overwriting by all
zeros. This causes Kernel boot hang in case if "earlyprintk" feature is
enabled at least [1].

Unfortunately, there is no exact place in driver now where we can
determine that UART context is ready - most of registers configured in
serial_omap_set_termios(), but some of them in other places.
More over, even if PM runtime will be disabled (blocked) during OMAP
serial driver probe() execution [2],[3] it will fix only console UART,
but context of other UARTs will be overwriting by all zeros during first
access to the corresponding UART.

To fix this issue:
- introduce additional "initialized" flag and update PM runtime callback
to do nothing if its not set. Set "initialized" at the end of probe().
- read current UART registers configuration in probe and use it by
default.

[1] http://www.spinics.net/lists/arm-kernel/msg256828.html
[2] http://www.spinics.net/lists/arm-kernel/msg258062.html
[3] http://www.spinics.net/lists/arm-kernel/msg258040.html

CC: Tony Lindgren <tony@atomide.com>
CC: Rajendra Nayak <rnayak@ti.com>
CC: Felipe Balbi <balbi@ti.com>
CC: Kevin Hilman <khilman@linaro.org>

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
tested on OMAP4 SDP with and without earlyprintk enabled.
 drivers/tty/serial/omap-serial.c |   27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index f39bf0c..e1e9667 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -162,6 +162,7 @@ struct uart_omap_port {
 	struct work_struct	qos_work;
 	struct pinctrl		*pins;
 	bool			is_suspending;
+	bool			initialized;
 };
 
 #define to_uart_omap_port(p)	((container_of((p), struct uart_omap_port, port)))
@@ -1395,6 +1396,19 @@ static struct omap_uart_port_info *of_get_uart_port_info(struct device *dev)
 	return omap_up_info;
 }
 
+static void serial_omap_save_context_def(struct uart_omap_port *up)
+{
+	up->dll = serial_in(up, UART_DLL);
+	up->dlh = serial_in(up, UART_DLM);
+	up->ier = serial_in(up, UART_IER);
+	up->fcr = serial_in(up, UART_FCR);
+	up->mcr = serial_in(up, UART_MCR);
+	up->scr = serial_in(up, UART_OMAP_SCR);
+	up->efr = serial_in(up, UART_EFR);
+	up->lcr = serial_in(up, UART_LCR);
+	up->mdr1 = serial_in(up, UART_OMAP_MDR1);
+}
+
 static int serial_omap_probe(struct platform_device *pdev)
 {
 	struct uart_omap_port	*up;
@@ -1518,10 +1532,14 @@ static int serial_omap_probe(struct platform_device *pdev)
 	ui[up->port.line] = up;
 	serial_omap_add_console_port(up);
 
+	serial_omap_save_context_def(up);
+
 	ret = uart_add_one_port(&serial_omap_reg, &up->port);
 	if (ret != 0)
 		goto err_add_port;
 
+	up->initialized = true;
+
 	pm_runtime_mark_last_busy(up->dev);
 	pm_runtime_put_autosuspend(up->dev);
 	return 0;
@@ -1619,6 +1637,9 @@ static int serial_omap_runtime_suspend(struct device *dev)
 	if (!up)
 		return -EINVAL;
 
+	if (!up->initialized)
+		return 0;
+
 	/*
 	* When using 'no_console_suspend', the console UART must not be
 	* suspended. Since driver suspend is managed by runtime suspend,
@@ -1652,8 +1673,12 @@ static int serial_omap_runtime_suspend(struct device *dev)
 static int serial_omap_runtime_resume(struct device *dev)
 {
 	struct uart_omap_port *up = dev_get_drvdata(dev);
+	int loss_cnt;
+
+	if (!up->initialized)
+		return 0;
 
-	int loss_cnt = serial_omap_get_context_loss_count(up);
+	loss_cnt = serial_omap_get_context_loss_count(up);
 
 	if (loss_cnt < 0) {
 		dev_dbg(dev, "serial_omap_get_context_loss_count failed : %d\n",
-- 
1.7.9.5

  reply	other threads:[~2013-07-12 11:57 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-12 11:55 [PATCH 1/2] serial: omap: enable PM runtime only when its fully configured Grygorii Strashko
2013-07-12 11:55 ` Grygorii Strashko
2013-07-12 11:55 ` Grygorii Strashko [this message]
2013-07-12 11:55   ` [PATCH 2/2] serial: omap: fix wrong context restoration on init Grygorii Strashko
2013-07-12 12:11   ` Felipe Balbi
2013-07-12 12:11     ` Felipe Balbi
2013-07-26 23:00     ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1373630142-21765-2-git-send-email-grygorii.strashko@ti.com \
    --to=grygorii.strashko@ti.com \
    --cc=balbi@ti.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=khilman@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=rnayak@ti.com \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.