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
From: Rafael J. Wysocki <[email protected]>
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 <[email protected]>
---
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) {
From: Rafael J. Wysocki <[email protected]>
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 <[email protected]>
---
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;
From: Rafael J. Wysocki <[email protected]>
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 <[email protected]>
---
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;
}
On Fri, 3 Dec 2010, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
On Friday, December 03, 2010, Alan Stern wrote:
> On Fri, 3 Dec 2010, Rafael J. Wysocki wrote:
>
> > From: Rafael J. Wysocki <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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 <[email protected]>
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 <[email protected]>
---
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 ");
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 <[email protected]>
> 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 <[email protected]>
> ---
> 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