* [PATCH 1/2] PM/runtime: update document about callbacks
@ 2011-09-10 14:37 tom.leiming
2011-09-10 14:37 ` [PATCH 2/2] PM/runtime: handle ->runtime_suspend failure correctly tom.leiming
2011-09-22 1:33 ` tom.leiming
0 siblings, 2 replies; 6+ messages in thread
From: tom.leiming @ 2011-09-10 14:37 UTC (permalink / raw)
To: rjw, stern; +Cc: linux-pm, linux-kernel, Ming Lei
From: Ming Lei <tom.leiming@gmail.com>
Support for device power domains has been introduced in
commit 9659cc0678b954f187290c6e8b247a673c5d37e1 (PM: Make
system-wide PM and runtime PM treat subsystems consistently),
also power domain callbacks will take precedence over subsystem ones
from commit 4d27e9dcff00a6425d779b065ec8892e4f391661(PM: Make
power domain callbacks take precedence over subsystem ones).
So update part of "Device Runtime PM Callbacks" in
Documentation/power/runtime_pm.txt.
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
Documentation/power/runtime_pm.txt | 19 ++++++++++++-------
1 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/Documentation/power/runtime_pm.txt b/Documentation/power/runtime_pm.txt
index 1f05404..0e85608 100644
--- a/Documentation/power/runtime_pm.txt
+++ b/Documentation/power/runtime_pm.txt
@@ -43,13 +43,18 @@ struct dev_pm_ops {
...
};
-The ->runtime_suspend(), ->runtime_resume() and ->runtime_idle() callbacks are
-executed by the PM core for either the device type, or the class (if the device
-type's struct dev_pm_ops object does not exist), or the bus type (if the
-device type's and class' struct dev_pm_ops objects do not exist) of the given
-device (this allows device types to override callbacks provided by bus types or
-classes if necessary). The bus type, device type and class callbacks are
-referred to as subsystem-level callbacks in what follows.
+The ->runtime_suspend(), ->runtime_resume() and ->runtime_idle() callbacks
+are executed by the PM core for either the power domain, or the device type
+(if the device power domain's struct dev_pm_ops does not exist), or the class
+(if the device power domain's and type's struct dev_pm_ops object does not
+exist), or the bus type (if the device power domain's, type's and class'
+struct dev_pm_ops objects do not exist) of the given device, so the priority
+order of callbacks from high to low is that power domain callbacks, device
+type callbacks, class callbacks and bus type callbacks, and the high priority
+one will take precedence over low priority one. The bus type, device type and
+class callbacks are referred to as subsystem-level callbacks in what follows,
+and generally speaking, the power domain callbacks are used for representing
+power domains within a SoC.
By default, the callbacks are always invoked in process context with interrupts
enabled. However, subsystems can use the pm_runtime_irq_safe() helper function
--
1.7.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] PM/runtime: handle ->runtime_suspend failure correctly
2011-09-10 14:37 [PATCH 1/2] PM/runtime: update document about callbacks tom.leiming
@ 2011-09-10 14:37 ` tom.leiming
2011-09-11 20:44 ` Alan Stern
2011-09-22 1:33 ` tom.leiming
1 sibling, 1 reply; 6+ messages in thread
From: tom.leiming @ 2011-09-10 14:37 UTC (permalink / raw)
To: rjw, stern; +Cc: linux-pm, linux-kernel, Ming Lei
From: Ming Lei <tom.leiming@gmail.com>
If ->runtime_suspend returns -EAGAIN or -EBUSY, the device should
still be in ACTIVE state, so it is not needed to handle defered
resume and idle notification to its parent; if ->runtime_suspend
returns other fatal failure, it doesn't make sense to process defered
resume and send idle notification to its parent.
So skip these when failure is returned from ->runtime_suspend, also add
comments for this handling in rpm_suspend.
This patch also updates comments for rpm_suspend:
- 'Cancel a pending idle notification' should be put before, also
should be changed as 'Cancel a pending idle notification or
autosuspend/suspend'
- idle notification for suspend failure has been removed, so update
comments for it
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
drivers/base/power/runtime.c | 22 ++++++++++++++--------
1 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 441b5a3..6fa3241 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -284,14 +284,17 @@ static int rpm_callback(int (*cb)(struct device *), struct device *dev)
* @dev: Device to suspend.
* @rpmflags: Flag bits.
*
- * Check if the device's runtime PM status allows it to be suspended. If
- * another suspend has been started earlier, either return immediately or wait
- * for it to finish, depending on the RPM_NOWAIT and RPM_ASYNC flags. Cancel a
- * pending idle notification. If the RPM_ASYNC flag is set then queue a
- * suspend request; otherwise run the ->runtime_suspend() callback directly.
- * If a deferred resume was requested while the callback was running then carry
- * it out; otherwise send an idle notification for the device (if the suspend
- * failed) or for its parent (if the suspend succeeded).
+ * Check if the device's runtime PM status allows it to be suspended. Cancel
+ * a pending idle notification or autosuspend/suspend. If another suspend has
+ * been started earlier, either return immediately or wait for it to finish,
+ * depending on the RPM_NOWAIT and RPM_ASYNC flags. If the RPM_ASYNC flag is
+ * set then queue a suspend request; otherwise run the ->runtime_suspend()
+ * callback directly. If ->runtime_suspend returns failure, just cancel
+ * pending request and wake up waited tasks, then return immediatelly.
+ * After ->runtime_suspend succeeded, if a deferred resume was requested
+ * while the callback was running then carry it out; otherwise send an idle
+ * notification for its parent (if both ignore_children and irq_safe
+ * are not set).
*
* This function must be called under dev->power.lock with interrupts disabled.
*/
@@ -422,6 +425,9 @@ static int rpm_suspend(struct device *dev, int rpmflags)
}
wake_up_all(&dev->power.wait_queue);
+ if (retval)
+ goto out;
+
if (dev->power.deferred_resume) {
rpm_resume(dev, 0);
retval = -EAGAIN;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] PM/runtime: handle ->runtime_suspend failure correctly
2011-09-10 14:37 ` [PATCH 2/2] PM/runtime: handle ->runtime_suspend failure correctly tom.leiming
@ 2011-09-11 20:44 ` Alan Stern
2011-09-14 1:17 ` Ming Lei
0 siblings, 1 reply; 6+ messages in thread
From: Alan Stern @ 2011-09-11 20:44 UTC (permalink / raw)
To: Ming Lei; +Cc: rjw, linux-pm, linux-kernel
On Sat, 10 Sep 2011 tom.leiming@gmail.com wrote:
> From: Ming Lei <tom.leiming@gmail.com>
>
> If ->runtime_suspend returns -EAGAIN or -EBUSY, the device should
> still be in ACTIVE state, so it is not needed to handle defered
> resume and idle notification to its parent; if ->runtime_suspend
> returns other fatal failure, it doesn't make sense to process defered
> resume and send idle notification to its parent.
> @@ -422,6 +425,9 @@ static int rpm_suspend(struct device *dev, int rpmflags)
> }
> wake_up_all(&dev->power.wait_queue);
>
> + if (retval)
> + goto out;
> +
> if (dev->power.deferred_resume) {
> rpm_resume(dev, 0);
> retval = -EAGAIN;
If there's a suspend failure, the deferred_resume flag gets turned off
anyway. But skipping this test won't hurt, and skipping the parent
notification is a good idea.
In fact, it might be even better to put a copy of the wake_up_all() in
the "if (retval)" branch after the suspend callback and then go
directly to out. The "else" branch could then become part of the
straight-through code, not indented.
Alan Stern
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] PM/runtime: handle ->runtime_suspend failure correctly
2011-09-11 20:44 ` Alan Stern
@ 2011-09-14 1:17 ` Ming Lei
0 siblings, 0 replies; 6+ messages in thread
From: Ming Lei @ 2011-09-14 1:17 UTC (permalink / raw)
To: Alan Stern; +Cc: rjw, linux-pm, linux-kernel
Hi,
On Mon, Sep 12, 2011 at 4:44 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Sat, 10 Sep 2011 tom.leiming@gmail.com wrote:
>
>> From: Ming Lei <tom.leiming@gmail.com>
>>
>> If ->runtime_suspend returns -EAGAIN or -EBUSY, the device should
>> still be in ACTIVE state, so it is not needed to handle defered
>> resume and idle notification to its parent; if ->runtime_suspend
>> returns other fatal failure, it doesn't make sense to process defered
>> resume and send idle notification to its parent.
>
>> @@ -422,6 +425,9 @@ static int rpm_suspend(struct device *dev, int rpmflags)
>> }
>> wake_up_all(&dev->power.wait_queue);
>>
>> + if (retval)
>> + goto out;
>> +
>> if (dev->power.deferred_resume) {
>> rpm_resume(dev, 0);
>> retval = -EAGAIN;
>
> If there's a suspend failure, the deferred_resume flag gets turned off
> anyway. But skipping this test won't hurt, and skipping the parent
> notification is a good idea.
Yes, I will update the commit log.
>
> In fact, it might be even better to put a copy of the wake_up_all() in
> the "if (retval)" branch after the suspend callback and then go
> directly to out. The "else" branch could then become part of the
> straight-through code, not indented.
Good idea, will update this in -v1.
thanks,
--
Ming Lei
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] PM/runtime: handle ->runtime_suspend failure correctly
2011-09-10 14:37 [PATCH 1/2] PM/runtime: update document about callbacks tom.leiming
2011-09-10 14:37 ` [PATCH 2/2] PM/runtime: handle ->runtime_suspend failure correctly tom.leiming
@ 2011-09-22 1:33 ` tom.leiming
1 sibling, 0 replies; 6+ messages in thread
From: tom.leiming @ 2011-09-22 1:33 UTC (permalink / raw)
To: rjw, stern; +Cc: linux-pm, linux-kernel, Ming Lei
From: Ming Lei <tom.leiming@gmail.com>
If ->runtime_suspend returns -EAGAIN or -EBUSY, the device should
still be in ACTIVE state, so it is not needed to send idle notification
to its parent; if ->runtime_suspend returns other fatal failure, it
doesn't make sense to send idle notification to its parent.
So skip these when failure is returned from ->runtime_suspend, also add
comments for this handling in rpm_suspend.
This patch also updates comments for rpm_suspend:
- 'Cancel a pending idle notification' should be put before, also
should be changed as 'Cancel a pending idle notification or
autosuspend/suspend'
- idle notification for suspend failure has been removed, so update
comments for it
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
v1: some minor change on Alan's suggestion
---
drivers/base/power/runtime.c | 34 +++++++++++++++++++---------------
1 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 441b5a3..e3c6a8f 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -284,14 +284,17 @@ static int rpm_callback(int (*cb)(struct device *), struct device *dev)
* @dev: Device to suspend.
* @rpmflags: Flag bits.
*
- * Check if the device's runtime PM status allows it to be suspended. If
- * another suspend has been started earlier, either return immediately or wait
- * for it to finish, depending on the RPM_NOWAIT and RPM_ASYNC flags. Cancel a
- * pending idle notification. If the RPM_ASYNC flag is set then queue a
- * suspend request; otherwise run the ->runtime_suspend() callback directly.
- * If a deferred resume was requested while the callback was running then carry
- * it out; otherwise send an idle notification for the device (if the suspend
- * failed) or for its parent (if the suspend succeeded).
+ * Check if the device's runtime PM status allows it to be suspended. Cancel
+ * a pending idle notification or autosuspend/suspend. If another suspend has
+ * been started earlier, either return immediately or wait for it to finish,
+ * depending on the RPM_NOWAIT and RPM_ASYNC flags. If the RPM_ASYNC flag is
+ * set then queue a suspend request; otherwise run the ->runtime_suspend()
+ * callback directly. If ->runtime_suspend returns failure, just cancel
+ * pending request and wake up waited tasks, then return immediatelly.
+ * After ->runtime_suspend succeeded, if a deferred resume was requested
+ * while the callback was running then carry it out; otherwise send an idle
+ * notification for its parent (if both ignore_children and irq_safe
+ * are not set).
*
* This function must be called under dev->power.lock with interrupts disabled.
*/
@@ -410,15 +413,16 @@ static int rpm_suspend(struct device *dev, int rpmflags)
dev->power.runtime_error = 0;
else
pm_runtime_cancel_pending(dev);
- } else {
+ wake_up_all(&dev->power.wait_queue);
+ goto out;
+ }
no_callback:
- __update_runtime_status(dev, RPM_SUSPENDED);
- pm_runtime_deactivate_timer(dev);
+ __update_runtime_status(dev, RPM_SUSPENDED);
+ pm_runtime_deactivate_timer(dev);
- if (dev->parent) {
- parent = dev->parent;
- atomic_add_unless(&parent->power.child_count, -1, 0);
- }
+ if (dev->parent) {
+ parent = dev->parent;
+ atomic_add_unless(&parent->power.child_count, -1, 0);
}
wake_up_all(&dev->power.wait_queue);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] PM/runtime: handle ->runtime_suspend failure correctly
2011-10-12 3:53 [PATCH 1/2] PM/runtime: fix comment for rpm_suspend ming.lei
@ 2011-10-12 3:53 ` ming.lei
0 siblings, 0 replies; 6+ messages in thread
From: ming.lei @ 2011-10-12 3:53 UTC (permalink / raw)
To: rjw; +Cc: linux-pm, linux-kernel, Ming Lei
From: Ming Lei <ming.lei@canonical.com>
If ->runtime_suspend returns -EAGAIN or -EBUSY, the device should
still be in ACTIVE state, so it is not needed to send idle notification
to its parent; if ->runtime_suspend returns other fatal failure, it
doesn't make sense to send idle notification to its parent.
So skip these when failure is returned from ->runtime_suspend, also
update comments for this handling in rpm_suspend.
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/base/power/runtime.c | 25 +++++++++++++------------
1 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index aa23a64..6bb3aaf 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -291,11 +291,11 @@ static int rpm_callback(int (*cb)(struct device *), struct device *dev)
* another suspend has been started earlier, either return immediately
* or wait for it to finish, depending on the RPM_NOWAIT and RPM_ASYNC
* flags. If the RPM_ASYNC flag is set then queue a suspend request;
- * otherwise run the ->runtime_suspend() callback directly. If a deferred
- * resume was requested while the callback was running then carry it out;
- * otherwise send an idle notification for its parent (if the suspend
- * succeeded and both ignore_children of parent->power and irq_safe of
- * dev->power are not set).
+ * otherwise run the ->runtime_suspend() callback directly. When
+ * ->runtime_suspend succeeded, if a deferred resume was requested while
+ * the callback was running then carry it out, otherwise send an idle
+ * notification for its parent (if the suspend succeeded and both
+ * ignore_children of parent->power and irq_safe of dev->power are not set).
*
* This function must be called under dev->power.lock with interrupts disabled.
*/
@@ -420,15 +420,16 @@ static int rpm_suspend(struct device *dev, int rpmflags)
dev->power.runtime_error = 0;
else
pm_runtime_cancel_pending(dev);
- } else {
+ wake_up_all(&dev->power.wait_queue);
+ goto out;
+ }
no_callback:
- __update_runtime_status(dev, RPM_SUSPENDED);
- pm_runtime_deactivate_timer(dev);
+ __update_runtime_status(dev, RPM_SUSPENDED);
+ pm_runtime_deactivate_timer(dev);
- if (dev->parent) {
- parent = dev->parent;
- atomic_add_unless(&parent->power.child_count, -1, 0);
- }
+ if (dev->parent) {
+ parent = dev->parent;
+ atomic_add_unless(&parent->power.child_count, -1, 0);
}
wake_up_all(&dev->power.wait_queue);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-10-12 3:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-10 14:37 [PATCH 1/2] PM/runtime: update document about callbacks tom.leiming
2011-09-10 14:37 ` [PATCH 2/2] PM/runtime: handle ->runtime_suspend failure correctly tom.leiming
2011-09-11 20:44 ` Alan Stern
2011-09-14 1:17 ` Ming Lei
2011-09-22 1:33 ` tom.leiming
-- strict thread matches above, loose matches on Subject: below --
2011-10-12 3:53 [PATCH 1/2] PM/runtime: fix comment for rpm_suspend ming.lei
2011-10-12 3:53 ` [PATCH 2/2] PM/runtime: handle ->runtime_suspend failure correctly ming.lei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox