linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] serial-core: resume serial hardware with no_console_suspend
@ 2009-09-15 13:38 Stanislav Brabec
  2009-09-25  0:05 ` Andrew Morton
  0 siblings, 1 reply; 10+ messages in thread
From: Stanislav Brabec @ 2009-09-15 13:38 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, Russell King - ARM Linux

Hardware may need re-initialization to get serial port working after
resume. It does not happen with no_console_suspend. Attached patch
attempts to fix it.

The patch attempts to keep hardware running before suspend and run
hardware re-initialization after resume. Maybe simpler approach is
possible.

Signed-off-by: Stanislav Brabec <sbrabec@suse.cz>

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index b0bb29d..43cc1ab 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1990,12 +1990,6 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *port)
 
 	mutex_lock(&state->mutex);
 
-	if (!console_suspend_enabled && uart_console(port)) {
-		/* we're going to avoid suspending serial console */
-		mutex_unlock(&state->mutex);
-		return 0;
-	}
-
 	tty_dev = device_find_child(port->dev, &match, serial_match_port);
 	if (device_may_wakeup(tty_dev)) {
 		enable_irq_wake(port->irq);
@@ -2003,20 +1997,23 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *port)
 		mutex_unlock(&state->mutex);
 		return 0;
 	}
-	port->suspended = 1;
+	if (console_suspend_enabled || !uart_console(port))
+		port->suspended = 1;
 
 	if (state->info.flags & UIF_INITIALIZED) {
 		const struct uart_ops *ops = port->ops;
 		int tries;
 
-		state->info.flags = (state->info.flags & ~UIF_INITIALIZED)
-				     | UIF_SUSPENDED;
+		if (console_suspend_enabled || !uart_console(port)) {
+			state->info.flags = (state->info.flags & ~UIF_INITIALIZED)
+					     | UIF_SUSPENDED;
 
-		spin_lock_irq(&port->lock);
-		ops->stop_tx(port);
-		ops->set_mctrl(port, 0);
-		ops->stop_rx(port);
-		spin_unlock_irq(&port->lock);
+			spin_lock_irq(&port->lock);
+			ops->stop_tx(port);
+			ops->set_mctrl(port, 0);
+			ops->stop_rx(port);
+			spin_unlock_irq(&port->lock);
+		}
 
 		/*
 		 * Wait for the transmitter to empty.
@@ -2031,16 +2028,18 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *port)
 			       drv->dev_name,
 			       drv->tty_driver->name_base + port->line);
 
-		ops->shutdown(port);
+		if (console_suspend_enabled || !uart_console(port))
+			ops->shutdown(port);
 	}
 
 	/*
 	 * Disable the console device before suspending.
 	 */
-	if (uart_console(port))
+	if (console_suspend_enabled && uart_console(port))
 		console_stop(port->cons);
 
-	uart_change_pm(state, 3);
+	if (console_suspend_enabled || !uart_console(port))
+		uart_change_pm(state, 3);
 
 	mutex_unlock(&state->mutex);
 
@@ -2055,12 +2054,6 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *port)
 
 	mutex_lock(&state->mutex);
 
-	if (!console_suspend_enabled && uart_console(port)) {
-		/* no need to resume serial console, it wasn't suspended */
-		mutex_unlock(&state->mutex);
-		return 0;
-	}
-
 	tty_dev = device_find_child(port->dev, &match, serial_match_port);
 	if (!port->suspended && device_may_wakeup(tty_dev)) {
 		disable_irq_wake(port->irq);
@@ -2100,21 +2093,23 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *port)
 		spin_lock_irq(&port->lock);
 		ops->set_mctrl(port, 0);
 		spin_unlock_irq(&port->lock);
-		ret = ops->startup(port);
-		if (ret == 0) {
-			uart_change_speed(state, NULL);
-			spin_lock_irq(&port->lock);
-			ops->set_mctrl(port, port->mctrl);
-			ops->start_tx(port);
-			spin_unlock_irq(&port->lock);
-			state->info.flags |= UIF_INITIALIZED;
-		} else {
-			/*
-			 * Failed to resume - maybe hardware went away?
-			 * Clear the "initialized" flag so we won't try
-			 * to call the low level drivers shutdown method.
-			 */
-			uart_shutdown(state);
+		if (console_suspend_enabled || !uart_console(port)) {
+			ret = ops->startup(port);
+			if (ret == 0) {
+				uart_change_speed(state, NULL);
+				spin_lock_irq(&port->lock);
+				ops->set_mctrl(port, port->mctrl);
+				ops->start_tx(port);
+				spin_unlock_irq(&port->lock);
+				state->info.flags |= UIF_INITIALIZED;
+			} else {
+				/*
+				 * Failed to resume - maybe hardware went away?
+				 * Clear the "initialized" flag so we won't try
+				 * to call the low level drivers shutdown method.
+				 */
+				uart_shutdown(state);
+			}
 		}
 
 		state->info.flags &= ~UIF_SUSPENDED;


-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                          e-mail: sbrabec@suse.cz
Lihovarská 1060/12           tel: +420 284 028 966, +49 911 740538747
190 00 Praha 9                                  fax: +420 284 028 951
Czech Republic                                    http://www.suse.cz/


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial-core: resume serial hardware with no_console_suspend
  2009-09-15 13:38 [PATCH] serial-core: resume serial hardware with no_console_suspend Stanislav Brabec
@ 2009-09-25  0:05 ` Andrew Morton
  2009-09-25  7:36   ` Russell King - ARM Linux
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2009-09-25  0:05 UTC (permalink / raw)
  To: Stanislav Brabec; +Cc: linux-kernel, gregkh, linux, Alan Cox

On Tue, 15 Sep 2009 15:38:58 +0200
Stanislav Brabec <utx@penguin.cz> wrote:

> Hardware may need re-initialization to get serial port working after
> resume. It does not happen with no_console_suspend. Attached patch
> attempts to fix it.
> 
> The patch attempts to keep hardware running before suspend and run
> hardware re-initialization after resume. Maybe simpler approach is
> possible.

The patch doesn't apply any more and seems like a rather hacky thing to do.

It appears that you have specific serial hardware which doesn't resume
correctly?  If so, that's a bug, so how about we start with a bug
report/description?



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial-core: resume serial hardware with no_console_suspend
  2009-09-25  0:05 ` Andrew Morton
@ 2009-09-25  7:36   ` Russell King - ARM Linux
  2009-09-25  9:55     ` Stanislav Brabec
  0 siblings, 1 reply; 10+ messages in thread
From: Russell King - ARM Linux @ 2009-09-25  7:36 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Stanislav Brabec, linux-kernel, gregkh, Alan Cox

On Thu, Sep 24, 2009 at 05:05:03PM -0700, Andrew Morton wrote:
> On Tue, 15 Sep 2009 15:38:58 +0200
> Stanislav Brabec <utx@penguin.cz> wrote:
> 
> > Hardware may need re-initialization to get serial port working after
> > resume. It does not happen with no_console_suspend. Attached patch
> > attempts to fix it.
> > 
> > The patch attempts to keep hardware running before suspend and run
> > hardware re-initialization after resume. Maybe simpler approach is
> > possible.
> 
> The patch doesn't apply any more and seems like a rather hacky thing to do.
> 
> It appears that you have specific serial hardware which doesn't resume
> correctly?  If so, that's a bug, so how about we start with a bug
> report/description?

It's something that I did point out when the no_console_suspend patch
appeared, but I was overruled/ignored.

The problem is that on ARM hardware, there is no BIOS to re-initialize
hardware.  The kernel has to do restore the entire system hardware
state upon resume.  Unfortunately, no_console_suspend not only prevents
the console from being suspended but _also_ resumed.

The result of that is the console UART is left in a totally uninitialized
state.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial-core: resume serial hardware with no_console_suspend
  2009-09-25  7:36   ` Russell King - ARM Linux
@ 2009-09-25  9:55     ` Stanislav Brabec
  2009-09-30 21:12       ` Andrew Morton
  2009-10-05 23:27       ` Andrew Morton
  0 siblings, 2 replies; 10+ messages in thread
From: Stanislav Brabec @ 2009-09-25  9:55 UTC (permalink / raw)
  To: Russell King - ARM Linux; +Cc: Andrew Morton, linux-kernel, gregkh, Alan Cox

Russell King - ARM Linux wrote:
> On Thu, Sep 24, 2009 at 05:05:03PM -0700, Andrew Morton wrote:
> > On Tue, 15 Sep 2009 15:38:58 +0200
> > Stanislav Brabec <utx@penguin.cz> wrote:
> > 
> > > Hardware may need re-initialization to get serial port working after
> > > resume. It does not happen with no_console_suspend. Attached patch
> > > attempts to fix it.
> > > 
> > > The patch attempts to keep hardware running before suspend and run
> > > hardware re-initialization after resume. Maybe simpler approach is
> > > possible.
> > 
> > The patch doesn't apply any more and seems like a rather hacky thing to do.

Yes, but no_console_suspend itself is a bit hacky. You need to save
console state but keep it running as long as possible afterwards and
resume it after return from the sleep.

I was not sure, what exactly should be skipped and what must be run, so
I experimented a bit.

Not calling hardware suspend/resume (current implementation) breaks
serial port in hardware that need resume.

Calling of resume without suspend seems to be dangerous. Not calling
suspend at all and standard initialization on resume would probably
reset the port setting and maybe cause memory leak.

> > It appears that you have specific serial hardware which doesn't resume
> > correctly?  If so, that's a bug, so how about we start with a bug
> > report/description?

The hardware resumes correctly. But no_console_suspend breaks its
resume.

> It's something that I did point out when the no_console_suspend patch
> appeared, but I was overruled/ignored.
> 
> The problem is that on ARM hardware, there is no BIOS to re-initialize
> hardware.  The kernel has to do restore the entire system hardware
> state upon resume.  Unfortunately, no_console_suspend not only prevents
> the console from being suspended but _also_ resumed.
> 
> The result of that is the console UART is left in a totally uninitialized
> state.

Exactly, my ARM hardware is PXA270 on Zaurus SL-C3200.

-- 
Stanislav Brabec
http://www.penguin.cz/~utx


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial-core: resume serial hardware with no_console_suspend
  2009-09-25  9:55     ` Stanislav Brabec
@ 2009-09-30 21:12       ` Andrew Morton
  2009-10-05 23:27       ` Andrew Morton
  1 sibling, 0 replies; 10+ messages in thread
From: Andrew Morton @ 2009-09-30 21:12 UTC (permalink / raw)
  To: Stanislav Brabec; +Cc: linux, linux-kernel, gregkh, alan

On Fri, 25 Sep 2009 11:55:47 +0200
Stanislav Brabec <utx@penguin.cz> wrote:

> Russell King - ARM Linux wrote:
> > On Thu, Sep 24, 2009 at 05:05:03PM -0700, Andrew Morton wrote:
> > > On Tue, 15 Sep 2009 15:38:58 +0200
> > > Stanislav Brabec <utx@penguin.cz> wrote:
> > > 
> > > > Hardware may need re-initialization to get serial port working after
> > > > resume. It does not happen with no_console_suspend. Attached patch
> > > > attempts to fix it.
> > > > 
> > > > The patch attempts to keep hardware running before suspend and run
> > > > hardware re-initialization after resume. Maybe simpler approach is
> > > > possible.
> > > 
> > > The patch doesn't apply any more and seems like a rather hacky thing to do.
> 
> Yes, but no_console_suspend itself is a bit hacky. You need to save
> console state but keep it running as long as possible afterwards and
> resume it after return from the sleep.
> 
> I was not sure, what exactly should be skipped and what must be run, so
> I experimented a bit.
> 
> Not calling hardware suspend/resume (current implementation) breaks
> serial port in hardware that need resume.
> 
> Calling of resume without suspend seems to be dangerous. Not calling
> suspend at all and standard initialization on resume would probably
> reset the port setting and maybe cause memory leak.
> 
> > > It appears that you have specific serial hardware which doesn't resume
> > > correctly?  If so, that's a bug, so how about we start with a bug
> > > report/description?
> 
> The hardware resumes correctly. But no_console_suspend breaks its
> resume.
> 
> > It's something that I did point out when the no_console_suspend patch
> > appeared, but I was overruled/ignored.
> > 
> > The problem is that on ARM hardware, there is no BIOS to re-initialize
> > hardware.  The kernel has to do restore the entire system hardware
> > state upon resume.  Unfortunately, no_console_suspend not only prevents
> > the console from being suspended but _also_ resumed.
> > 
> > The result of that is the console UART is left in a totally uninitialized
> > state.
> 
> Exactly, my ARM hardware is PXA270 on Zaurus SL-C3200.

hm, ho hum.

Could you please redo/retest/resend the patch against the current
kernel?  Someone went and renamed everything and it doesn't apply at
all any more.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial-core: resume serial hardware with no_console_suspend
  2009-09-25  9:55     ` Stanislav Brabec
  2009-09-30 21:12       ` Andrew Morton
@ 2009-10-05 23:27       ` Andrew Morton
  2009-10-18 15:05         ` Stanislav Brabec
  1 sibling, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2009-10-05 23:27 UTC (permalink / raw)
  To: Stanislav Brabec
  Cc: linux, linux-kernel, gregkh, alan, s.hauer, saxena, gregkh

On Fri, 25 Sep 2009 11:55:47 +0200
Stanislav Brabec <utx@penguin.cz> wrote:

> Russell King - ARM Linux wrote:
> > On Thu, Sep 24, 2009 at 05:05:03PM -0700, Andrew Morton wrote:
> > > On Tue, 15 Sep 2009 15:38:58 +0200
> > > Stanislav Brabec <utx@penguin.cz> wrote:
> > > 
> > > > Hardware may need re-initialization to get serial port working after
> > > > resume. It does not happen with no_console_suspend. Attached patch
> > > > attempts to fix it.
> > > > 
> > > > The patch attempts to keep hardware running before suspend and run
> > > > hardware re-initialization after resume. Maybe simpler approach is
> > > > possible.
> > > 
> > > The patch doesn't apply any more and seems like a rather hacky thing to do.
> 
> Yes, but no_console_suspend itself is a bit hacky. You need to save
> console state but keep it running as long as possible afterwards and
> resume it after return from the sleep.
> 
> I was not sure, what exactly should be skipped and what must be run, so
> I experimented a bit.
> 
> Not calling hardware suspend/resume (current implementation) breaks
> serial port in hardware that need resume.
> 
> Calling of resume without suspend seems to be dangerous. Not calling
> suspend at all and standard initialization on resume would probably
> reset the port setting and maybe cause memory leak.
> 
> > > It appears that you have specific serial hardware which doesn't resume
> > > correctly?  If so, that's a bug, so how about we start with a bug
> > > report/description?
> 
> The hardware resumes correctly. But no_console_suspend breaks its
> resume.
> 
> > It's something that I did point out when the no_console_suspend patch
> > appeared, but I was overruled/ignored.
> > 
> > The problem is that on ARM hardware, there is no BIOS to re-initialize
> > hardware.  The kernel has to do restore the entire system hardware
> > state upon resume.  Unfortunately, no_console_suspend not only prevents
> > the console from being suspended but _also_ resumed.
> > 
> > The result of that is the console UART is left in a totally uninitialized
> > state.
> 
> Exactly, my ARM hardware is PXA270 on Zaurus SL-C3200.

hm, well.  We can't go much further without a patch and the one you
sent won't apply.  Can you please redo against current mainline and
resend?

Do you expect that my some miracle, your patch will also fix
http://lkml.org/lkml/2009/9/30/164 ?



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial-core: resume serial hardware with no_console_suspend
  2009-10-05 23:27       ` Andrew Morton
@ 2009-10-18 15:05         ` Stanislav Brabec
  2009-10-18 15:46           ` Stanislav Brabec
  2009-10-18 19:19           ` Pavel Machek
  0 siblings, 2 replies; 10+ messages in thread
From: Stanislav Brabec @ 2009-10-18 15:05 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux, linux-kernel, gregkh, alan, s.hauer, saxena, Pavel Machek

Andrew Morton wrote in Mon Oct 05, 2009 at 16:27 -0700:
> On Fri, 25 Sep 2009 11:55:47 +0200
> Stanislav Brabec <utx@penguin.cz> wrote:
> 
> > Russell King - ARM Linux wrote:
> > > On Thu, Sep 24, 2009 at 05:05:03PM -0700, Andrew Morton wrote:
> > > > On Tue, 15 Sep 2009 15:38:58 +0200
> > > > Stanislav Brabec <utx@penguin.cz> wrote:
> > > > 
> > > > > Hardware may need re-initialization to get serial port working after
> > > > > resume. It does not happen with no_console_suspend. Attached patch
> > > > > attempts to fix it.
> > > > > 
> > > > > The patch attempts to keep hardware running before suspend and run
> > > > > hardware re-initialization after resume. Maybe simpler approach is
> > > > > possible.
> > > > 
> > > > The patch doesn't apply any more and seems like a rather hacky thing to do.
> > 
> > Yes, but no_console_suspend itself is a bit hacky. You need to save
> > console state but keep it running as long as possible afterwards and
> > resume it after return from the sleep.
> > 
> > I was not sure, what exactly should be skipped and what must be run, so
> > I experimented a bit.
> > 
> > Not calling hardware suspend/resume (current implementation) breaks
> > serial port in hardware that need resume.
> > 
> > Calling of resume without suspend seems to be dangerous. Not calling
> > suspend at all and standard initialization on resume would probably
> > reset the port setting and maybe cause memory leak.
> > 
> > > > It appears that you have specific serial hardware which doesn't resume
> > > > correctly?  If so, that's a bug, so how about we start with a bug
> > > > report/description?
> > 
> > The hardware resumes correctly. But no_console_suspend breaks its
> > resume.
> > 
> > > It's something that I did point out when the no_console_suspend patch
> > > appeared, but I was overruled/ignored.
> > > 
> > > The problem is that on ARM hardware, there is no BIOS to re-initialize
> > > hardware.  The kernel has to do restore the entire system hardware
> > > state upon resume.  Unfortunately, no_console_suspend not only prevents
> > > the console from being suspended but _also_ resumed.
> > > 
> > > The result of that is the console UART is left in a totally uninitialized
> > > state.
> > 
> > Exactly, my ARM hardware is PXA270 on Zaurus SL-C3200.
> 
> hm, well.  We can't go much further without a patch and the one you
> sent won't apply.  Can you please redo against current mainline and
> resend?

I am just working on a rebase, but (besides other minor changes) it
conflicts with ba15ab0e8de0d4439a91342ad52d55ca9e313f3d (mentioned
below).

I would be glad, if somebody would say me, whether it is correct and
whether it fixes all affected devices. I cannot actually test any of
them on my Zaurus: There is an independent regression that makes
impossible to initiate resume.

Both patches are trying to do the same: working serial after resume with
no_console_suspend, but they do it in a different way:

- Deepak Saxena's patch skips suspend process and does reset on resume.

- My patch does tricky suspend: Some parts of suspend are called, state
is saved, but device is kept in a state that it thinks it is up.

It was based on experiments, just guessing what happens there. It was
done without deep knowledge of serial code. It attempts to:
- Save the hardware state
- Perform buffer flush in time of its suspend call
- Tell the driver that port is suspended
- But still accept new data
- And keep console hardware in state that allows to send them

Both patches have a the same problem:

There are no early resume messages. In difference to BIOS equipped
machines, serial port must be properly re-initialized to become working
again.

It can be fixed either by changing suspend/resume order to wake serial
console early in the order, or by a non-standard hook in the early
resume process.

Very late suspend messages may be lost as well. (It is a generic problem
of all devices, not dependent on any of these patches.) It can be
prevented by using blocking write of messages or adding an extra sleep
after each message. I would afraid of blocking write. There is no
guarantee, that serial is still functional messages can be sent.

> Do you expect that my some miracle, your patch will also fix
> http://lkml.org/lkml/2009/9/30/164 ?

My original version of patch removed the early return from the resume,
where Deepak Saxena's added chunk resides. With my patch, code always
continues in resume, and skips just a minor part of the code.


________________________________________________________________________
Stanislav Brabec
http://www.penguin.cz/~utx


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial-core: resume serial hardware with no_console_suspend
  2009-10-18 15:05         ` Stanislav Brabec
@ 2009-10-18 15:46           ` Stanislav Brabec
  2009-10-18 15:49             ` Stanislav Brabec
  2009-10-18 19:19           ` Pavel Machek
  1 sibling, 1 reply; 10+ messages in thread
From: Stanislav Brabec @ 2009-10-18 15:46 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux, linux-kernel, gregkh, alan, s.hauer, saxena, Pavel Machek

Attaching rebased patch.

Untested! (Explained in the previous mail.)

Also sending a "diff -w" on the same code in the next mail for easier
review.

-

BIOS-less devices may require hardware resume for proper work. It did
not happen with no_console_suspend (at least before ba15ab0e).

Perform a tricky suspend/resume with no_console_suspend.

Attempt to:
- Save the hardware state
- Perform buffer flush in time of its suspend call
- Tell the driver that port is suspended
- But still accept new data
- And keep console hardware in state that allows to send them

Signed-off-by: Stanislav Brabec <utx@penguin.cz>

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index dcc7244..f1e1ab2 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2008,12 +2008,6 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
 
 	mutex_lock(&port->mutex);
 
-	if (!console_suspend_enabled && uart_console(uport)) {
-		/* we're going to avoid suspending serial console */
-		mutex_unlock(&port->mutex);
-		return 0;
-	}
-
 	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
 	if (device_may_wakeup(tty_dev)) {
 		enable_irq_wake(uport->irq);
@@ -2021,20 +2015,23 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
 		mutex_unlock(&port->mutex);
 		return 0;
 	}
-	uport->suspended = 1;
+	if (console_suspend_enabled || !uart_console(uport))
+		uport->suspended = 1;
 
 	if (port->flags & ASYNC_INITIALIZED) {
 		const struct uart_ops *ops = uport->ops;
 		int tries;
 
-		set_bit(ASYNCB_SUSPENDED, &port->flags);
-		clear_bit(ASYNCB_INITIALIZED, &port->flags);
+		if (console_suspend_enabled || !uart_console(uport)) {
+			set_bit(ASYNCB_SUSPENDED, &port->flags);
+			clear_bit(ASYNCB_INITIALIZED, &port->flags);
 
-		spin_lock_irq(&uport->lock);
-		ops->stop_tx(uport);
-		ops->set_mctrl(uport, 0);
-		ops->stop_rx(uport);
-		spin_unlock_irq(&uport->lock);
+			spin_lock_irq(&uport->lock);
+			ops->stop_tx(uport);
+			ops->set_mctrl(uport, 0);
+			ops->stop_rx(uport);
+			spin_unlock_irq(&uport->lock);
+		}
 
 		/*
 		 * Wait for the transmitter to empty.
@@ -2049,16 +2046,18 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
 			       drv->dev_name,
 			       drv->tty_driver->name_base + uport->line);
 
-		ops->shutdown(uport);
+		if (console_suspend_enabled || !uart_console(uport))
+			ops->shutdown(uport);
 	}
 
 	/*
 	 * Disable the console device before suspending.
 	 */
-	if (uart_console(uport))
+	if (console_suspend_enabled && uart_console(uport))
 		console_stop(uport->cons);
 
-	uart_change_pm(state, 3);
+	if (console_suspend_enabled || !uart_console(uport))
+		uart_change_pm(state, 3);
 
 	mutex_unlock(&port->mutex);
 
@@ -2075,29 +2074,6 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
 
 	mutex_lock(&port->mutex);
 
-	if (!console_suspend_enabled && uart_console(uport)) {
-		/* no need to resume serial console, it wasn't suspended */
-		/*
-		 * First try to use the console cflag setting.
-		 */
-		memset(&termios, 0, sizeof(struct ktermios));
-		termios.c_cflag = uport->cons->cflag;
-		/*
-		 * If that's unset, use the tty termios setting.
-		 */
-		if (termios.c_cflag == 0)
-			termios = *state->port.tty->termios;
-		else {
-			termios.c_ispeed = termios.c_ospeed =
-				tty_termios_input_baud_rate(&termios);
-			termios.c_ispeed = termios.c_ospeed =
-				tty_termios_baud_rate(&termios);
-		}
-		uport->ops->set_termios(uport, &termios, NULL);
-		mutex_unlock(&port->mutex);
-		return 0;
-	}
-
 	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
 	if (!uport->suspended && device_may_wakeup(tty_dev)) {
 		disable_irq_wake(uport->irq);
@@ -2123,21 +2099,23 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
 		spin_lock_irq(&uport->lock);
 		ops->set_mctrl(uport, 0);
 		spin_unlock_irq(&uport->lock);
-		ret = ops->startup(uport);
-		if (ret == 0) {
-			uart_change_speed(state, NULL);
-			spin_lock_irq(&uport->lock);
-			ops->set_mctrl(uport, uport->mctrl);
-			ops->start_tx(uport);
-			spin_unlock_irq(&uport->lock);
-			set_bit(ASYNCB_INITIALIZED, &port->flags);
-		} else {
-			/*
-			 * Failed to resume - maybe hardware went away?
-			 * Clear the "initialized" flag so we won't try
-			 * to call the low level drivers shutdown method.
-			 */
-			uart_shutdown(state);
+		if (console_suspend_enabled || !uart_console(uport)) {
+			ret = ops->startup(uport);
+			if (ret == 0) {
+				uart_change_speed(state, NULL);
+				spin_lock_irq(&uport->lock);
+				ops->set_mctrl(uport, uport->mctrl);
+				ops->start_tx(uport);
+				spin_unlock_irq(&uport->lock);
+				set_bit(ASYNCB_INITIALIZED, &port->flags);
+			} else {
+				/*
+				 * Failed to resume - maybe hardware went away?
+				 * Clear the "initialized" flag so we won't try
+				 * to call the low level drivers shutdown method.
+				 */
+				uart_shutdown(state);
+			}
 		}
 
 		clear_bit(ASYNCB_SUSPENDED, &port->flags);


________________________________________________________________________
Stanislav Brabec
http://www.penguin.cz/~utx


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial-core: resume serial hardware with no_console_suspend
  2009-10-18 15:46           ` Stanislav Brabec
@ 2009-10-18 15:49             ` Stanislav Brabec
  0 siblings, 0 replies; 10+ messages in thread
From: Stanislav Brabec @ 2009-10-18 15:49 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux, linux-kernel, gregkh, alan, s.hauer, saxena, Pavel Machek

Stanislav Brabec wrote:
> Also sending a "diff -w" on the same code in the next mail for easier
> review.

Here is a "diff -w" patch without indentation changes.

Do not apply!

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index dcc7244..f1e1ab2 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2008,12 +2008,6 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
 
 	mutex_lock(&port->mutex);
 
-	if (!console_suspend_enabled && uart_console(uport)) {
-		/* we're going to avoid suspending serial console */
-		mutex_unlock(&port->mutex);
-		return 0;
-	}
-
 	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
 	if (device_may_wakeup(tty_dev)) {
 		enable_irq_wake(uport->irq);
@@ -2021,12 +2015,14 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
 		mutex_unlock(&port->mutex);
 		return 0;
 	}
+	if (console_suspend_enabled || !uart_console(uport))
 	uport->suspended = 1;
 
 	if (port->flags & ASYNC_INITIALIZED) {
 		const struct uart_ops *ops = uport->ops;
 		int tries;
 
+		if (console_suspend_enabled || !uart_console(uport)) {
 		set_bit(ASYNCB_SUSPENDED, &port->flags);
 		clear_bit(ASYNCB_INITIALIZED, &port->flags);
 
@@ -2035,6 +2031,7 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
 		ops->set_mctrl(uport, 0);
 		ops->stop_rx(uport);
 		spin_unlock_irq(&uport->lock);
+		}
 
 		/*
 		 * Wait for the transmitter to empty.
@@ -2049,15 +2046,17 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
 			       drv->dev_name,
 			       drv->tty_driver->name_base + uport->line);
 
+		if (console_suspend_enabled || !uart_console(uport))
 		ops->shutdown(uport);
 	}
 
 	/*
 	 * Disable the console device before suspending.
 	 */
-	if (uart_console(uport))
+	if (console_suspend_enabled && uart_console(uport))
 		console_stop(uport->cons);
 
+	if (console_suspend_enabled || !uart_console(uport))
 	uart_change_pm(state, 3);
 
 	mutex_unlock(&port->mutex);
@@ -2075,29 +2074,6 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
 
 	mutex_lock(&port->mutex);
 
-	if (!console_suspend_enabled && uart_console(uport)) {
-		/* no need to resume serial console, it wasn't suspended */
-		/*
-		 * First try to use the console cflag setting.
-		 */
-		memset(&termios, 0, sizeof(struct ktermios));
-		termios.c_cflag = uport->cons->cflag;
-		/*
-		 * If that's unset, use the tty termios setting.
-		 */
-		if (termios.c_cflag == 0)
-			termios = *state->port.tty->termios;
-		else {
-			termios.c_ispeed = termios.c_ospeed =
-				tty_termios_input_baud_rate(&termios);
-			termios.c_ispeed = termios.c_ospeed =
-				tty_termios_baud_rate(&termios);
-		}
-		uport->ops->set_termios(uport, &termios, NULL);
-		mutex_unlock(&port->mutex);
-		return 0;
-	}
-
 	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
 	if (!uport->suspended && device_may_wakeup(tty_dev)) {
 		disable_irq_wake(uport->irq);
@@ -2123,6 +2099,7 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
 		spin_lock_irq(&uport->lock);
 		ops->set_mctrl(uport, 0);
 		spin_unlock_irq(&uport->lock);
+		if (console_suspend_enabled || !uart_console(uport)) {
 		ret = ops->startup(uport);
 		if (ret == 0) {
 			uart_change_speed(state, NULL);
@@ -2139,6 +2116,7 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
 			 */
 			uart_shutdown(state);
 		}
+		}
 
 		clear_bit(ASYNCB_SUSPENDED, &port->flags);
 	}


________________________________________________________________________
Stanislav Brabec
http://www.penguin.cz/~utx


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] serial-core: resume serial hardware with no_console_suspend
  2009-10-18 15:05         ` Stanislav Brabec
  2009-10-18 15:46           ` Stanislav Brabec
@ 2009-10-18 19:19           ` Pavel Machek
  1 sibling, 0 replies; 10+ messages in thread
From: Pavel Machek @ 2009-10-18 19:19 UTC (permalink / raw)
  To: Stanislav Brabec
  Cc: Andrew Morton, linux, linux-kernel, gregkh, alan, s.hauer, saxena

Hi!

> ???It can be fixed either by changing suspend/resume order to wake serial
> console early in the order, or by a non-standard hook in the early
> resume process.

Well, moving serial to sysdev state is probably bad idea, but having
non-standard hook that is easy to patch in would be very cool.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2009-10-18 19:19 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-15 13:38 [PATCH] serial-core: resume serial hardware with no_console_suspend Stanislav Brabec
2009-09-25  0:05 ` Andrew Morton
2009-09-25  7:36   ` Russell King - ARM Linux
2009-09-25  9:55     ` Stanislav Brabec
2009-09-30 21:12       ` Andrew Morton
2009-10-05 23:27       ` Andrew Morton
2009-10-18 15:05         ` Stanislav Brabec
2009-10-18 15:46           ` Stanislav Brabec
2009-10-18 15:49             ` Stanislav Brabec
2009-10-18 19:19           ` Pavel Machek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).