This commit in current git tree broke serial console for serial_txx9 driver.
> commit 97d97224ff361e08777fb33e0fd193ca877dac28
> Author: Russell King <[email protected]>
> Date: Sat Sep 1 21:25:09 2007 +0100
>
> [SERIAL] Fix console initialisation ordering
The serial_txx9 driver initialize its port entirely on its ->pm()
method if new state was 0. With the commit, serial_core calls ->pm()
even if the port was used for console.
Attached patch fixes this problem. Is this a right way?
If not, I will fix the serial_txx9 driver to skip the initialization in
->pm() if oldstate was -1.
------------------------------------------------------
Subject: serial: do not call ->pm() on initialization for console port
From: Atsushi Nemoto <[email protected]>
If the port was already enabled as a serial console, there is no need
to call pm callback to power it up. This patch fixes serial console
initialization on serial_txx9 driver.
Signed-off-by: Atsushi Nemoto <[email protected]>
---
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 68aa4da..5a38076 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2326,7 +2326,6 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *port)
}
state->port = port;
- state->pm_state = -1;
port->cons = drv->cons;
port->info = state->info;
@@ -2336,6 +2335,7 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *port)
* initialised.
*/
if (!(uart_console(port) && (port->cons->flags & CON_ENABLED))) {
+ state->pm_state = -1;
spin_lock_init(&port->lock);
lockdep_set_class(&port->lock, &port_lock_key);
}
On Fri, Oct 19, 2007 at 01:13:55AM +0900, Atsushi Nemoto wrote:
> This commit in current git tree broke serial console for serial_txx9 driver.
>
> > commit 97d97224ff361e08777fb33e0fd193ca877dac28
> > Author: Russell King <[email protected]>
> > Date: Sat Sep 1 21:25:09 2007 +0100
> >
> > [SERIAL] Fix console initialisation ordering
>
> The serial_txx9 driver initialize its port entirely on its ->pm()
> method if new state was 0. With the commit, serial_core calls ->pm()
> even if the port was used for console.
This'll break some of the ARM drivers, and although I investigated
this a month or so ago, I've completely forgotten the details.
> Attached patch fixes this problem. Is this a right way?
> If not, I will fix the serial_txx9 driver to skip the initialization in
> ->pm() if oldstate was -1.
I'll have to investigate further to refresh myself with what's going
on here, but your patch looks wrong.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
On Thu, 18 Oct 2007 17:18:27 +0100, Russell King <[email protected]> wrote:
> > Attached patch fixes this problem. Is this a right way?
> > If not, I will fix the serial_txx9 driver to skip the initialization in
> > ->pm() if oldstate was -1.
>
> I'll have to investigate further to refresh myself with what's going
> on here, but your patch looks wrong.
Thanks. JFYI, here is some patches I tried before changing
serial_core. Both fixes the serial console problem.
Patch 1:
diff --git a/drivers/serial/serial_txx9.c b/drivers/serial/serial_txx9.c
index 6846a6c..7ad2192 100644
--- a/drivers/serial/serial_txx9.c
+++ b/drivers/serial/serial_txx9.c
@@ -657,7 +657,15 @@ static void
serial_txx9_pm(struct uart_port *port, unsigned int state,
unsigned int oldstate)
{
- if (state == 0)
+ /*
+ * If oldstate was -1 this is called from
+ * uart_configure_port(). In this case do not initialize the
+ * port now, because the port was already initialized (for
+ * non-console port) or should not be initialized here (for
+ * console port). If we initialized the port here we lose
+ * serial console settings.
+ */
+ if (state == 0 && oldstate != -1)
serial_txx9_initialize(port);
}
Patch 2:
diff --git a/drivers/serial/serial_txx9.c b/drivers/serial/serial_txx9.c
index 6846a6c..167c3a7 100644
--- a/drivers/serial/serial_txx9.c
+++ b/drivers/serial/serial_txx9.c
@@ -657,6 +657,15 @@ static void
serial_txx9_pm(struct uart_port *port, unsigned int state,
unsigned int oldstate)
{
+ /*
+ * If oldstate was -1 this is called from
+ * uart_configure_port(). If we initialized the port here we
+ * lose serial console settings.
+ */
+#ifdef CONFIG_SERIAL_TXX9_CONSOLE
+ if (port->line == port->cons->index && oldstate == -1)
+ return;
+#endif
if (state == 0)
serial_txx9_initialize(port);
}
@@ -742,12 +751,7 @@ static void serial_txx9_config_port(struct uart_port *port, int uflags)
return;
port->type = PORT_TXX9;
up->port.fifosize = TXX9_SIO_TX_FIFO;
-
-#ifdef CONFIG_SERIAL_TXX9_CONSOLE
- if (up->port.line == up->port.cons->index)
- return;
-#endif
- serial_txx9_initialize(port);
+ /* This port will be initialized in subsequent serial_txx9_pm() */
}
static const char *
Hope these patches help your refreshing.
---
Atsushi Nemoto
On Thu, 18 Oct 2007 17:18:27 +0100, Russell King <[email protected]> wrote:
> > Attached patch fixes this problem. Is this a right way?
> > If not, I will fix the serial_txx9 driver to skip the initialization in
> > ->pm() if oldstate was -1.
>
> I'll have to investigate further to refresh myself with what's going
> on here, but your patch looks wrong.
Now rc1 is released. Any chances for this patch?
If none, I should push other workaround to fix this issue on 2.6.24.
---
Atsushi Nemoto
On Fri, 26 Oct 2007 01:05:10 +0900 (JST)
Atsushi Nemoto <[email protected]> wrote:
> On Thu, 18 Oct 2007 17:18:27 +0100, Russell King <[email protected]> wrote:
> > > Attached patch fixes this problem. Is this a right way?
> > > If not, I will fix the serial_txx9 driver to skip the initialization in
> > > ->pm() if oldstate was -1.
> >
> > I'll have to investigate further to refresh myself with what's going
> > on here, but your patch looks wrong.
>
> Now rc1 is released. Any chances for this patch?
> If none, I should push other workaround to fix this issue on 2.6.24.
I don't know what patch you're referring to. I don't appear to have
whatever-patch-this-is queued up anywhere?
On Thu, Oct 25, 2007 at 01:40:44PM -0700, Andrew Morton wrote:
> On Fri, 26 Oct 2007 01:05:10 +0900 (JST)
> Atsushi Nemoto <[email protected]> wrote:
>
> > On Thu, 18 Oct 2007 17:18:27 +0100, Russell King <[email protected]> wrote:
> > > > Attached patch fixes this problem. Is this a right way?
> > > > If not, I will fix the serial_txx9 driver to skip the initialization in
> > > > ->pm() if oldstate was -1.
> > >
> > > I'll have to investigate further to refresh myself with what's going
> > > on here, but your patch looks wrong.
> >
> > Now rc1 is released. Any chances for this patch?
> > If none, I should push other workaround to fix this issue on 2.6.24.
>
> I don't know what patch you're referring to. I don't appear to have
> whatever-patch-this-is queued up anywhere?
"Further investigation" is quite a good indication that a patch needs to
be worked on, no? In any case, I'm ill at the moment, so things are very
much on hold.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
On Thu, 25 Oct 2007 13:40:44 -0700, Andrew Morton <[email protected]> wrote:
> > Now rc1 is released. Any chances for this patch?
> > If none, I should push other workaround to fix this issue on 2.6.24.
>
> I don't know what patch you're referring to. I don't appear to have
> whatever-patch-this-is queued up anywhere?
Here is an original patch I proposed (http://lkml.org/lkml/2007/10/18/292)
------------------------------------------------------
Subject: serial: do not call ->pm() on initialization for console port
From: Atsushi Nemoto <[email protected]>
If the port was already enabled as a serial console, there is no need
to call pm callback to power it up. This patch fixes serial console
initialization on serial_txx9 driver.
Signed-off-by: Atsushi Nemoto <[email protected]>
---
diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 68aa4da..5a38076 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2326,7 +2326,6 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *port)
}
state->port = port;
- state->pm_state = -1;
port->cons = drv->cons;
port->info = state->info;
@@ -2336,6 +2335,7 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *port)
* initialised.
*/
if (!(uart_console(port) && (port->cons->flags & CON_ENABLED))) {
+ state->pm_state = -1;
spin_lock_init(&port->lock);
lockdep_set_class(&port->lock, &port_lock_key);
}
On Thu, 25 Oct 2007 22:36:44 +0100, Russell King <[email protected]> wrote:
> "Further investigation" is quite a good indication that a patch needs to
> be worked on, no? In any case, I'm ill at the moment, so things are very
> much on hold.
OK, then I'd like to propose this alternative
(http://lkml.org/lkml/2007/10/18/301). This should be work regardless
of the patch above.
------------------------------------------------------
Subject: serial: Fix serial_txx9 console initialization
From: Atsushi Nemoto <[email protected]>
Now serial_core calls ->pm() on initialization even if the port was
used for console. This behaviour breaks serial_txx9 console since The
serial_txx9 driver initialize its port entirely on its ->pm() method
if new state was 0. This patch adds checking for oldstate value to
fix this probelm.
Signed-off-by: Atsushi Nemoto <[email protected]>
---
diff --git a/drivers/serial/serial_txx9.c b/drivers/serial/serial_txx9.c
index 6846a6c..7ad2192 100644
--- a/drivers/serial/serial_txx9.c
+++ b/drivers/serial/serial_txx9.c
@@ -657,7 +657,15 @@ static void
serial_txx9_pm(struct uart_port *port, unsigned int state,
unsigned int oldstate)
{
- if (state == 0)
+ /*
+ * If oldstate was -1 this is called from
+ * uart_configure_port(). In this case do not initialize the
+ * port now, because the port was already initialized (for
+ * non-console port) or should not be initialized here (for
+ * console port). If we initialized the port here we lose
+ * serial console settings.
+ */
+ if (state == 0 && oldstate != -1)
serial_txx9_initialize(port);
}