[PATCH] serial: do not call ->pm() on initialization for console port

[PATCH] serial: do not call ->pm() on initialization for console port

[Atsushi Nemoto]

This commit in current git tree broke serial console for serial_txx9 driver.

> commit 97d97224ff361e08777fb33e0fd193ca877dac28
> Author: Russell King <rmk@dyn-67.arm.linux.org.uk>
> 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 <anemo@mba.ocn.ne.jp>

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 <anemo@mba.ocn.ne.jp>
---
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);
 	}

Re: [PATCH] serial: do not call ->pm() on initialization for console port

[Russell King]

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 <rmk@dyn-67.arm.linux.org.uk>
> > 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:

Re: [PATCH] serial: do not call ->pm() on initialization for console port

[Atsushi Nemoto]

On Thu, 18 Oct 2007 17:18:27 +0100, Russell King <rmk+lkml@arm.linux.org.uk> 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

Re: [PATCH] serial: do not call ->pm() on initialization for console port

[Atsushi Nemoto]

On Thu, 18 Oct 2007 17:18:27 +0100, Russell King <rmk+lkml@arm.linux.org.uk> 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

Re: [PATCH] serial: do not call ->pm() on initialization for console port

[Andrew Morton]

On Fri, 26 Oct 2007 01:05:10 +0900 (JST)
Atsushi Nemoto <anemo@mba.ocn.ne.jp> wrote:

> On Thu, 18 Oct 2007 17:18:27 +0100, Russell King <rmk+lkml@arm.linux.org.uk> 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?

Re: [PATCH] serial: do not call ->pm() on initialization for console port

[Russell King]

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 <anemo@mba.ocn.ne.jp> wrote:
> 
> > On Thu, 18 Oct 2007 17:18:27 +0100, Russell King <rmk+lkml@arm.linux.org.uk> 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:

Re: [PATCH] serial: do not call ->pm() on initialization for console port

[Atsushi Nemoto]

On Thu, 25 Oct 2007 13:40:44 -0700, Andrew Morton <akpm@linux-foundation.org> 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 <anemo@mba.ocn.ne.jp>

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 <anemo@mba.ocn.ne.jp>
---
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 <rmk+lkml@arm.linux.org.uk> 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 <anemo@mba.ocn.ne.jp>

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 <anemo@mba.ocn.ne.jp>
---
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);
 }