public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH]serial-core: resume serial hardware with no_console_suspend (resend)
@ 2009-12-02 15:20 Stanislav Brabec
  2009-12-03 13:51 ` Daniel Mack
  0 siblings, 1 reply; 2+ messages in thread
From: Stanislav Brabec @ 2009-12-02 15:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux, linux-kernel, alan, gregkh, Pavel Machek, Daniel Mack,
	saxena, Eric Miao, s.hauer, Haojian Zhuang

Perform a tricky suspend/resume even with no_console_suspend.

With no_console_suspend, kernel skips serial port suspend/resume and the
serial hardware may remain in undefined state after resume. It actually
happens on devices that don't have BIOS that handle serial
initialization. It makes impossible to use serial console after resume. 

Devices affected by this problem include:
Sharp Zaurus devices
Several PXA based ARM embedded boards

The patch does:
- 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

It allows to capture late console messages without breaking console
after resume.

This is just a resend of a patch discussed in these threads, as the
patch was not yet applied.

"Possible suspend/resume regression in .32-rc?" (Nov 1-5, 2009, ARM
list, later LKML)

"serial-core: resume serial hardware with no_console_suspend" (Sep
15-Oct 18, 2009, LKML & ARM lists)

Signed-off-by: Stanislav Brabec <sbrabec@suse.cz>
Tested-by: Haojian Zhuang <haojian.zhuang@gmail.com>

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);


-- 
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] 2+ messages in thread

* Re: [PATCH]serial-core: resume serial hardware with no_console_suspend (resend)
  2009-12-02 15:20 [PATCH]serial-core: resume serial hardware with no_console_suspend (resend) Stanislav Brabec
@ 2009-12-03 13:51 ` Daniel Mack
  0 siblings, 0 replies; 2+ messages in thread
From: Daniel Mack @ 2009-12-03 13:51 UTC (permalink / raw)
  To: Stanislav Brabec
  Cc: Andrew Morton, linux, linux-kernel, alan, gregkh, Pavel Machek,
	saxena, Eric Miao, s.hauer, Haojian Zhuang

On Wed, Dec 02, 2009 at 04:20:56PM +0100, Stanislav Brabec wrote:
> Perform a tricky suspend/resume even with no_console_suspend.
> 
> With no_console_suspend, kernel skips serial port suspend/resume and the
> serial hardware may remain in undefined state after resume. It actually
> happens on devices that don't have BIOS that handle serial
> initialization. It makes impossible to use serial console after resume. 
> 
> Devices affected by this problem include:
> Sharp Zaurus devices
> Several PXA based ARM embedded boards

True, I had occasions where the serial console did not get back to life
after suspend on a PXA300 based device. It didn't always happen though.

> The patch does:
> - 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
> 
> It allows to capture late console messages without breaking console
> after resume.
> 
> This is just a resend of a patch discussed in these threads, as the
> patch was not yet applied.
> 
> "Possible suspend/resume regression in .32-rc?" (Nov 1-5, 2009, ARM
> list, later LKML)
> 
> "serial-core: resume serial hardware with no_console_suspend" (Sep
> 15-Oct 18, 2009, LKML & ARM lists)
> 
> Signed-off-by: Stanislav Brabec <sbrabec@suse.cz>
> Tested-by: Haojian Zhuang <haojian.zhuang@gmail.com>

And I didn't see that problem anymore since your patch is applied,
great!

Tested-by: Daniel Mack <daniel@caiaq.de>

Thanks,
Daniel



> 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);
> 
> 
> -- 
> 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	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2009-12-03 13:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-02 15:20 [PATCH]serial-core: resume serial hardware with no_console_suspend (resend) Stanislav Brabec
2009-12-03 13:51 ` Daniel Mack

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox