[PATCH 0/3] PM: Changes related to wakeup events

[PATCH 0/3] PM: Changes related to wakeup events

[Rafael J. Wysocki]

Hi,

The following three patches change the way in which wakeup events are taken
into account by dpm_prepare() and dpm_suspend().

[1/3] - Prevent dpm_prepare() from aborting suspend if events_check_enabled is
        unset.

[2/3] - Replace pm_check_wakeup_events() with pm_wakeup_pending() to avoid
        confision with the return value.

[3/3] - Check pm_wakeup_pending() in device_suspend().

If there are no objections, I'm going to queue them up for 2.6.38.

Thanks,
Rafael

[PATCH 1/3] PM: Changes related to wakeup events

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Currently dpm_prepare() returns error code if it finds that a device
being suspended has a pending runtime resume request.  However, it
should not do that if the checking for wakeup events is not enabled.
On the other hand, if the checking for wakeup events is enabled, it
can return error when a wakeup event is detected, regardless of its
source.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/main.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -26,6 +26,7 @@
 #include <linux/interrupt.h>
 #include <linux/sched.h>
 #include <linux/async.h>
+#include <linux/suspend.h>
 
 #include "../base.h"
 #include "power.h"
@@ -1052,8 +1053,10 @@ static int dpm_prepare(pm_message_t stat
 		mutex_unlock(&dpm_list_mtx);
 
 		pm_runtime_get_noresume(dev);
-		if (pm_runtime_barrier(dev) && device_may_wakeup(dev)) {
-			/* Wake-up requested during system sleep transition. */
+		if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
+			pm_wakeup_event(dev, 0);
+
+		if (!pm_check_wakeup_events()) {
 			pm_runtime_put_sync(dev);
 			error = -EBUSY;
 		} else {
@@ -1068,8 +1071,8 @@ static int dpm_prepare(pm_message_t stat
 				error = 0;
 				continue;
 			}
-			printk(KERN_ERR "PM: Failed to prepare device %s "
-				"for power transition: error %d\n",
+			printk(KERN_INFO "PM: Device %s not prepared "
+				"for power transition: code %d\n",
 				kobject_name(&dev->kobj), error);
 			put_device(dev);
 			break;

[PATCH 2/3] PM / Wakeup: Replace pm_check_wakeup_events() with pm_wakeup_pending()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

To avoid confusion with the meaning and return value of
pm_check_wakeup_events() replace it with pm_wakeup_pending() that
will work the other way around (ie. return true when system-wide
power transition should be aborted).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/main.c   |    2 +-
 drivers/base/power/wakeup.c |   20 ++++++++++----------
 include/linux/suspend.h     |    4 ++--
 kernel/power/hibernate.c    |    4 ++--
 kernel/power/process.c      |    2 +-
 kernel/power/suspend.c      |    2 +-
 6 files changed, 17 insertions(+), 17 deletions(-)

Index: linux-2.6/drivers/base/power/wakeup.c
===================================================================
--- linux-2.6.orig/drivers/base/power/wakeup.c
+++ linux-2.6/drivers/base/power/wakeup.c
@@ -542,26 +542,26 @@ static void pm_wakeup_update_hit_counts(
 }
 
 /**
- * pm_check_wakeup_events - Check for new wakeup events.
+ * pm_wakeup_pending - Check if power transition in progress should be aborted.
  *
  * Compare the current number of registered wakeup events with its preserved
- * value from the past to check if new wakeup events have been registered since
- * the old value was stored.  Check if the current number of wakeup events being
- * processed is zero.
+ * value from the past and return true if new wakeup events have been registered
+ * since the old value was stored.  Also return true if the current number of
+ * wakeup events being processed is different from zero.
  */
-bool pm_check_wakeup_events(void)
+bool pm_wakeup_pending(void)
 {
 	unsigned long flags;
-	bool ret = true;
+	bool ret = false;
 
 	spin_lock_irqsave(&events_lock, flags);
 	if (events_check_enabled) {
-		ret = ((unsigned int)atomic_read(&event_count) == saved_count)
-			&& !atomic_read(&events_in_progress);
-		events_check_enabled = ret;
+		ret = ((unsigned int)atomic_read(&event_count) != saved_count)
+			|| atomic_read(&events_in_progress);
+		events_check_enabled = !ret;
 	}
 	spin_unlock_irqrestore(&events_lock, flags);
-	if (!ret)
+	if (ret)
 		pm_wakeup_update_hit_counts();
 	return ret;
 }
Index: linux-2.6/include/linux/suspend.h
===================================================================
--- linux-2.6.orig/include/linux/suspend.h
+++ linux-2.6/include/linux/suspend.h
@@ -301,7 +301,7 @@ extern int unregister_pm_notifier(struct
 /* drivers/base/power/wakeup.c */
 extern bool events_check_enabled;
 
-extern bool pm_check_wakeup_events(void);
+extern bool pm_wakeup_pending(void);
 extern bool pm_get_wakeup_count(unsigned int *count);
 extern bool pm_save_wakeup_count(unsigned int count);
 #else /* !CONFIG_PM_SLEEP */
@@ -318,7 +318,7 @@ static inline int unregister_pm_notifier
 
 #define pm_notifier(fn, pri)	do { (void)(fn); } while (0)
 
-static inline bool pm_check_wakeup_events(void) { return true; }
+static inline bool pm_wakeup_pending(void) { return false; }
 #endif /* !CONFIG_PM_SLEEP */
 
 extern struct mutex pm_mutex;
Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -1056,7 +1056,7 @@ static int dpm_prepare(pm_message_t stat
 		if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
 			pm_wakeup_event(dev, 0);
 
-		if (!pm_check_wakeup_events()) {
+		if (pm_wakeup_pending()) {
 			pm_runtime_put_sync(dev);
 			error = -EBUSY;
 		} else {
Index: linux-2.6/kernel/power/hibernate.c
===================================================================
--- linux-2.6.orig/kernel/power/hibernate.c
+++ linux-2.6/kernel/power/hibernate.c
@@ -278,7 +278,7 @@ static int create_image(int platform_mod
 		goto Enable_irqs;
 	}
 
-	if (hibernation_test(TEST_CORE) || !pm_check_wakeup_events())
+	if (hibernation_test(TEST_CORE) || pm_wakeup_pending())
 		goto Power_up;
 
 	in_suspend = 1;
@@ -516,7 +516,7 @@ int hibernation_platform_enter(void)
 
 	local_irq_disable();
 	sysdev_suspend(PMSG_HIBERNATE);
-	if (!pm_check_wakeup_events()) {
+	if (pm_wakeup_pending()) {
 		error = -EAGAIN;
 		goto Power_up;
 	}
Index: linux-2.6/kernel/power/process.c
===================================================================
--- linux-2.6.orig/kernel/power/process.c
+++ linux-2.6/kernel/power/process.c
@@ -85,7 +85,7 @@ static int try_to_freeze_tasks(bool sig_
 		if (!todo || time_after(jiffies, end_time))
 			break;
 
-		if (!pm_check_wakeup_events()) {
+		if (pm_wakeup_pending()) {
 			wakeup = true;
 			break;
 		}
Index: linux-2.6/kernel/power/suspend.c
===================================================================
--- linux-2.6.orig/kernel/power/suspend.c
+++ linux-2.6/kernel/power/suspend.c
@@ -172,7 +172,7 @@ static int suspend_enter(suspend_state_t
 
 	error = sysdev_suspend(PMSG_SUSPEND);
 	if (!error) {
-		if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
+		if (!(suspend_test(TEST_CORE) || pm_wakeup_pending())) {
 			error = suspend_ops->enter(state);
 			events_check_enabled = false;
 		}

[PATCH 3/3] PM: Use pm_wakeup_pending() in device_suspend()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Before starting to suspend a device in device_suspend() check if
there's a request to abort the power transition and return -EBUSY
in that case.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/main.c |    5 +++++
 1 file changed, 5 insertions(+)

Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -935,6 +935,11 @@ static void async_suspend(void *data, as
 
 static int device_suspend(struct device *dev)
 {
+	if (pm_wakeup_pending()) {
+		async_error = -EBUSY;
+		return async_error;
+	}
+
 	INIT_COMPLETION(dev->power.completion);
 
 	if (pm_async_enabled && dev->power.async_suspend) {

Re: [PATCH 3/3] PM: Use pm_wakeup_pending() in device_suspend()

[Alan Stern]

On Fri, 3 Dec 2010, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Before starting to suspend a device in device_suspend() check if
> there's a request to abort the power transition and return -EBUSY
> in that case.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/base/power/main.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> Index: linux-2.6/drivers/base/power/main.c
> ===================================================================
> --- linux-2.6.orig/drivers/base/power/main.c
> +++ linux-2.6/drivers/base/power/main.c
> @@ -935,6 +935,11 @@ static void async_suspend(void *data, as
>  
>  static int device_suspend(struct device *dev)
>  {
> +	if (pm_wakeup_pending()) {
> +		async_error = -EBUSY;
> +		return async_error;
> +	}
> +
>  	INIT_COMPLETION(dev->power.completion);
>  
>  	if (pm_async_enabled && dev->power.async_suspend) {

Is a similar test needed in async_suspend()?  What happens if 
dpm_suspend() runs through the entire device list okay, but a wakeup 
event occurs in the middle of the async_synchronize_full() delay?

Alan Stern

Re: [PATCH 3/3] PM: Use pm_wakeup_pending() in device_suspend()

[Rafael J. Wysocki]

On Friday, December 03, 2010, Alan Stern wrote:
> On Fri, 3 Dec 2010, Rafael J. Wysocki wrote:
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Before starting to suspend a device in device_suspend() check if
> > there's a request to abort the power transition and return -EBUSY
> > in that case.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> >  drivers/base/power/main.c |    5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > Index: linux-2.6/drivers/base/power/main.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/base/power/main.c
> > +++ linux-2.6/drivers/base/power/main.c
> > @@ -935,6 +935,11 @@ static void async_suspend(void *data, as
> >  
> >  static int device_suspend(struct device *dev)
> >  {
> > +	if (pm_wakeup_pending()) {
> > +		async_error = -EBUSY;
> > +		return async_error;
> > +	}
> > +
> >  	INIT_COMPLETION(dev->power.completion);
> >  
> >  	if (pm_async_enabled && dev->power.async_suspend) {
> 
> Is a similar test needed in async_suspend()?  What happens if 
> dpm_suspend() runs through the entire device list okay, but a wakeup 
> event occurs in the middle of the async_synchronize_full() delay?

The devices will be suspended and the suspend will progress until it's
finally aborted before putting the system into the sleep state.  But I agree
it's better to check pm_wakeup_pending() in async_suspend() too,
so I think we can simply put it into __device_suspend() instead, like in the
patch below.

Thanks,
Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PM: Use pm_wakeup_pending() in __device_suspend()

Before starting to suspend a device in __device_suspend() check if
there's a request to abort the power transition and return -EBUSY
in that case.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/main.c |    5 +++++
 1 file changed, 5 insertions(+)

Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -877,6 +877,11 @@ static int __device_suspend(struct devic
 	if (async_error)
 		goto End;
 
+	if (pm_wakeup_pending()) {
+		async_error = -EBUSY;
+		goto End;
+	}
+
 	if (dev->class) {
 		if (dev->class->pm) {
 			pm_dev_dbg(dev, state, "class ");

Re: [PATCH 3/3] PM: Use pm_wakeup_pending() in device_suspend()

[Alan Stern]

On Fri, 3 Dec 2010, Rafael J. Wysocki wrote:

> The devices will be suspended and the suspend will progress until it's
> finally aborted before putting the system into the sleep state.  But I agree
> it's better to check pm_wakeup_pending() in async_suspend() too,
> so I think we can simply put it into __device_suspend() instead, like in the
> patch below.
> 
> Thanks,
> Rafael
> 
> ---
> From: Rafael J. Wysocki <rjw@sisk.pl>
> Subject: PM: Use pm_wakeup_pending() in __device_suspend()
> 
> Before starting to suspend a device in __device_suspend() check if
> there's a request to abort the power transition and return -EBUSY
> in that case.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/base/power/main.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> Index: linux-2.6/drivers/base/power/main.c
> ===================================================================
> --- linux-2.6.orig/drivers/base/power/main.c
> +++ linux-2.6/drivers/base/power/main.c
> @@ -877,6 +877,11 @@ static int __device_suspend(struct devic
>  	if (async_error)
>  		goto End;
>  
> +	if (pm_wakeup_pending()) {
> +		async_error = -EBUSY;
> +		goto End;
> +	}
> +
>  	if (dev->class) {
>  		if (dev->class->pm) {
>  			pm_dev_dbg(dev, state, "class ");
> 

Looks good.

Alan Stern