Re: [PATCH 2/2] serial: omap: fix wrong context restoration on init
On Fri, Jul 12, 2013 at 03:11:46PM +0300, Felipe Balbi wrote:
hi,
On Fri, Jul 12, 2013 at 02:55:42PM +0300, Grygorii Strashko wrote:
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 t...@atomide.com
CC: Rajendra Nayak rna...@ti.com
CC: Felipe Balbi ba...@ti.com
CC: Kevin Hilman khil...@linaro.org
Signed-off-by: Grygorii Strashko grygorii.stras...@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;
boolis_suspending;
+ boolinitialized;
you really think adding this sort of bool flag is the best thing we can
do ? Something which will, quite likely, spread through every single
driver ?
I agree, that's not ok, please fix this up properly somehow.
thanks,
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] serial: omap: fix wrong context restoration on init
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 t...@atomide.com
CC: Rajendra Nayak rna...@ti.com
CC: Felipe Balbi ba...@ti.com
CC: Kevin Hilman khil...@linaro.org
Signed-off-by: Grygorii Strashko grygorii.stras...@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;
boolis_suspending;
+ boolinitialized;
};
#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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] serial: omap: fix wrong context restoration on init
hi,
On Fri, Jul 12, 2013 at 02:55:42PM +0300, Grygorii Strashko wrote:
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 t...@atomide.com
CC: Rajendra Nayak rna...@ti.com
CC: Felipe Balbi ba...@ti.com
CC: Kevin Hilman khil...@linaro.org
Signed-off-by: Grygorii Strashko grygorii.stras...@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;
boolis_suspending;
+ boolinitialized;
you really think adding this sort of bool flag is the best thing we can
do ? Something which will, quite likely, spread through every single
driver ?
oh well...
--
balbi
signature.asc
Description: Digital signature
