[PATCH] Serial: Avoid unbalanced IRQ wake disable during resume

[PATCH] Serial: Avoid unbalanced IRQ wake disable during resume

[Govindraj.R]

To avoid unbalanced IRQ wake disable, ensure that wakeups are disabled
only when wakeups have been successfully enabled.
Tested on OMAP3630SDP/ZOOM3.

Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
Reported-by: Paul Walmsley <paul@pwsan.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Greg KH <greg@kroah.com>
---
This patch is in conclusion to issue discussed in below thread:
http://www.spinics.net/lists/linux-omap/msg41356.html

 drivers/serial/serial_core.c |    8 ++++++--
 include/linux/serial_core.h  |    1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 9ffa5be..7ec1e11 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1985,7 +1985,8 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)

 	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
 	if (device_may_wakeup(tty_dev)) {
-		enable_irq_wake(uport->irq);
+		if (!enable_irq_wake(uport->irq))
+			uport->irq_wake = 1;
 		put_device(tty_dev);
 		mutex_unlock(&port->mutex);
 		return 0;
@@ -2051,7 +2052,10 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)

 	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
 	if (!uport->suspended && device_may_wakeup(tty_dev)) {
-		disable_irq_wake(uport->irq);
+		if (uport->irq_wake) {
+			disable_irq_wake(uport->irq);
+			uport->irq_wake = 0;
+		}
 		mutex_unlock(&port->mutex);
 		return 0;
 	}
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 212eb4c..28b6698 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -361,6 +361,7 @@ struct uart_port {
 	struct device		*dev;			/* parent device */
 	unsigned char		hub6;			/* this should be in the 8250 driver */
 	unsigned char		suspended;
+	unsigned char		irq_wake;
 	unsigned char		unused[2];
 	void			*private_data;		/* generic platform data pointer */
 };
-- 
1.7.1

Re: [PATCH] Serial: Avoid unbalanced IRQ wake disable during resume

[Kevin Hilman]

"Govindraj.R" <govindraj.raja@ti.com> writes:

> To avoid unbalanced IRQ wake disable, ensure that wakeups are disabled
> only when wakeups have been successfully enabled.

The changelog could be a bit more descriptive here.  

You should summarize why this happens in the first place.  e.g., for
IRQs that do not have IRQ wake functionality, the default functions
return errors, so that a disable_irq_wake() following a failing
enable_irq_wake() will result in the unbalanced IRQ wake disable.

Otherwise,

Acked-by: Kevin Hilman <khilman@deeprootsystems.com>

> Tested on OMAP3630SDP/ZOOM3.
>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
> Reported-by: Paul Walmsley <paul@pwsan.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> Cc: Greg KH <greg@kroah.com>
> ---
> This patch is in conclusion to issue discussed in below thread:
> http://www.spinics.net/lists/linux-omap/msg41356.html
>
>  drivers/serial/serial_core.c |    8 ++++++--
>  include/linux/serial_core.h  |    1 +
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
> index 9ffa5be..7ec1e11 100644
> --- a/drivers/serial/serial_core.c
> +++ b/drivers/serial/serial_core.c
> @@ -1985,7 +1985,8 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
>
>  	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
>  	if (device_may_wakeup(tty_dev)) {
> -		enable_irq_wake(uport->irq);
> +		if (!enable_irq_wake(uport->irq))
> +			uport->irq_wake = 1;
>  		put_device(tty_dev);
>  		mutex_unlock(&port->mutex);
>  		return 0;
> @@ -2051,7 +2052,10 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
>
>  	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
>  	if (!uport->suspended && device_may_wakeup(tty_dev)) {
> -		disable_irq_wake(uport->irq);
> +		if (uport->irq_wake) {
> +			disable_irq_wake(uport->irq);
> +			uport->irq_wake = 0;
> +		}
>  		mutex_unlock(&port->mutex);
>  		return 0;
>  	}
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 212eb4c..28b6698 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -361,6 +361,7 @@ struct uart_port {
>  	struct device		*dev;			/* parent device */
>  	unsigned char		hub6;			/* this should be in the 8250 driver */
>  	unsigned char		suspended;
> +	unsigned char		irq_wake;
>  	unsigned char		unused[2];
>  	void			*private_data;		/* generic platform data pointer */
>  };

RE: [PATCH] Serial: Avoid unbalanced IRQ wake disable during resume

[Santosh Shilimkar]

> -----Original Message-----
> From: Govindraj.R [mailto:govindraj.raja@ti.com]
> Sent: Thursday, December 16, 2010 6:13 PM
> To: linux-serial@vger.kernel.org; Greg KH
> Cc: Paul Walmsley; Santosh Shilimkar; Kevin Hilman; Greg KH
> Subject: [PATCH] Serial: Avoid unbalanced IRQ wake disable during resume
>
> To avoid unbalanced IRQ wake disable, ensure that wakeups are disabled
> only when wakeups have been successfully enabled.
> Tested on OMAP3630SDP/ZOOM3.
>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
> Reported-by: Paul Walmsley <paul@pwsan.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> Cc: Greg KH <greg@kroah.com>
As suggested by Kevin, please update the change log. You could
also use the change log of intermediate version I created.

Also add a note the fix is suggested by Paul.

o.w
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
> This patch is in conclusion to issue discussed in below thread:
> http://www.spinics.net/lists/linux-omap/msg41356.html
>
>  drivers/serial/serial_core.c |    8 ++++++--
>  include/linux/serial_core.h  |    1 +
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
> index 9ffa5be..7ec1e11 100644
> --- a/drivers/serial/serial_core.c
> +++ b/drivers/serial/serial_core.c
> @@ -1985,7 +1985,8 @@ int uart_suspend_port(struct uart_driver *drv,
> struct uart_port *uport)
>
>  	tty_dev = device_find_child(uport->dev, &match,
serial_match_port);
>  	if (device_may_wakeup(tty_dev)) {
> -		enable_irq_wake(uport->irq);
> +		if (!enable_irq_wake(uport->irq))
> +			uport->irq_wake = 1;
>  		put_device(tty_dev);
>  		mutex_unlock(&port->mutex);
>  		return 0;
> @@ -2051,7 +2052,10 @@ int uart_resume_port(struct uart_driver *drv,
> struct uart_port *uport)
>
>  	tty_dev = device_find_child(uport->dev, &match,
serial_match_port);
>  	if (!uport->suspended && device_may_wakeup(tty_dev)) {
> -		disable_irq_wake(uport->irq);
> +		if (uport->irq_wake) {
> +			disable_irq_wake(uport->irq);
> +			uport->irq_wake = 0;
> +		}
>  		mutex_unlock(&port->mutex);
>  		return 0;
>  	}
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 212eb4c..28b6698 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -361,6 +361,7 @@ struct uart_port {
>  	struct device		*dev;			/* parent device
*/
>  	unsigned char		hub6;			/* this should be
in the
> 8250 driver */
>  	unsigned char		suspended;
> +	unsigned char		irq_wake;
>  	unsigned char		unused[2];
>  	void			*private_data;		/* generic
platform data
> pointer */
>  };
> --
> 1.7.1
>

Re: [PATCH] Serial: Avoid unbalanced IRQ wake disable during resume

[Greg KH]

On Fri, Dec 17, 2010 at 11:52:44AM +0530, Santosh Shilimkar wrote:
> > -----Original Message-----
> > From: Govindraj.R [mailto:govindraj.raja@ti.com]
> > Sent: Thursday, December 16, 2010 6:13 PM
> > To: linux-serial@vger.kernel.org; Greg KH
> > Cc: Paul Walmsley; Santosh Shilimkar; Kevin Hilman; Greg KH
> > Subject: [PATCH] Serial: Avoid unbalanced IRQ wake disable during resume
> >
> > To avoid unbalanced IRQ wake disable, ensure that wakeups are disabled
> > only when wakeups have been successfully enabled.
> > Tested on OMAP3630SDP/ZOOM3.
> >
> > Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
> > Reported-by: Paul Walmsley <paul@pwsan.com>
> > Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > Cc: Kevin Hilman <khilman@deeprootsystems.com>
> > Cc: Greg KH <greg@kroah.com>
> As suggested by Kevin, please update the change log. You could
> also use the change log of intermediate version I created.
> 
> Also add a note the fix is suggested by Paul.

Sorry, I already checked this into my git tree, so it will have to stay
as-is.

thanks,

greg k-h