* [PATCH] PM / Sleep: Remove pm_runtime_suspended() from __pm_generic_call()
@ 2011-11-17 23:37 Rafael J. Wysocki
2011-11-27 12:41 ` [PATCH] PM / Sleep: Simplify generic system suspend callbacks Rafael J. Wysocki
0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2011-11-17 23:37 UTC (permalink / raw)
To: Linux PM list; +Cc: LKML
From: Rafael J. Wysocki <rjw@sisk.pl>
The pm_runtime_suspended() check in __pm_generic_call() doesn't
really help and may cause problems to happen, because in some cases
the system suspend callbacks need to be called even if the given
device has been suspended by runtime PM. For example, if the device
generally supports remote wakeup and is not enabled to wake up
the system from sleep, it should be prevented from generating wakeup
signals during system suspend and that has to be done by the
suspend callbacks that the pm_runtime_suspended() check prevents from
being executed. For this reason, remove the pm_runtime_suspended()
check from __pm_generic_call().
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/base/power/generic_ops.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux/drivers/base/power/generic_ops.c
===================================================================
--- linux.orig/drivers/base/power/generic_ops.c
+++ linux/drivers/base/power/generic_ops.c
@@ -106,7 +106,7 @@ static int __pm_generic_call(struct devi
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
int (*callback)(struct device *);
- if (!pm || pm_runtime_suspended(dev))
+ if (!pm)
return 0;
switch (event) {
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] PM / Sleep: Simplify generic system suspend callbacks 2011-11-17 23:37 [PATCH] PM / Sleep: Remove pm_runtime_suspended() from __pm_generic_call() Rafael J. Wysocki @ 2011-11-27 12:41 ` Rafael J. Wysocki 2011-12-09 23:05 ` Rafael J. Wysocki 0 siblings, 1 reply; 6+ messages in thread From: Rafael J. Wysocki @ 2011-11-27 12:41 UTC (permalink / raw) To: Linux PM list; +Cc: LKML From: Rafael J. Wysocki <rjw@sisk.pl> The pm_runtime_suspended() check in __pm_generic_call() doesn't really help and may cause problems to happen, because in some cases the system suspend callbacks need to be called even if the given device has been suspended by runtime PM. For example, if the device generally supports remote wakeup and is not enabled to wake up the system from sleep, it should be prevented from generating wakeup signals during system suspend and that has to be done by the suspend callbacks that the pm_runtime_suspended() check prevents from being executed. Similarly, it may not be a good idea to unconditionally change the runtime PM status of the device to 'active' in __pm_generic_resume(), because the driver may want to leave the device in the 'suspended' state, depending on what happened to it before the system suspend and whether or not it is enabled to wake up the system. For the above reasons, remove the pm_runtime_suspended() check from __pm_generic_call() and remove the code changing the device's runtime PM status from __pm_generic_resume(). Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- drivers/base/power/generic_ops.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) Index: linux/drivers/base/power/generic_ops.c =================================================================== --- linux.orig/drivers/base/power/generic_ops.c +++ linux/drivers/base/power/generic_ops.c @@ -106,7 +106,7 @@ static int __pm_generic_call(struct devi const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; int (*callback)(struct device *); - if (!pm || pm_runtime_suspended(dev)) + if (!pm) return 0; switch (event) { @@ -224,7 +224,6 @@ static int __pm_generic_resume(struct de { const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; int (*callback)(struct device *); - int ret; if (!pm) return 0; @@ -241,17 +240,7 @@ static int __pm_generic_resume(struct de break; } - if (!callback) - return 0; - - ret = callback(dev); - if (!ret && !noirq && pm_runtime_enabled(dev)) { - pm_runtime_disable(dev); - pm_runtime_set_active(dev); - pm_runtime_enable(dev); - } - - return ret; + return callback ? callback(dev) : 0; } /** ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PM / Sleep: Simplify generic system suspend callbacks 2011-11-27 12:41 ` [PATCH] PM / Sleep: Simplify generic system suspend callbacks Rafael J. Wysocki @ 2011-12-09 23:05 ` Rafael J. Wysocki 2011-12-14 23:19 ` [PATCH 0/2 v2] " Rafael J. Wysocki 0 siblings, 1 reply; 6+ messages in thread From: Rafael J. Wysocki @ 2011-12-09 23:05 UTC (permalink / raw) To: Linux PM list; +Cc: LKML On Sunday, November 27, 2011, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rjw@sisk.pl> > > The pm_runtime_suspended() check in __pm_generic_call() doesn't > really help and may cause problems to happen, because in some cases > the system suspend callbacks need to be called even if the given > device has been suspended by runtime PM. For example, if the device > generally supports remote wakeup and is not enabled to wake up > the system from sleep, it should be prevented from generating wakeup > signals during system suspend and that has to be done by the > suspend callbacks that the pm_runtime_suspended() check prevents from > being executed. > > Similarly, it may not be a good idea to unconditionally change > the runtime PM status of the device to 'active' in > __pm_generic_resume(), because the driver may want to leave the > device in the 'suspended' state, depending on what happened to it > before the system suspend and whether or not it is enabled to > wake up the system. > > For the above reasons, remove the pm_runtime_suspended() > check from __pm_generic_call() and remove the code changing the > device's runtime PM status from __pm_generic_resume(). > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> Well, I'm not seeing any objections, so I gather so this change is fine by everyone. I'm going to put it into linux-pm/linux-next, then. Thanks, Rafael > --- > drivers/base/power/generic_ops.c | 15 ++------------- > 1 file changed, 2 insertions(+), 13 deletions(-) > > Index: linux/drivers/base/power/generic_ops.c > =================================================================== > --- linux.orig/drivers/base/power/generic_ops.c > +++ linux/drivers/base/power/generic_ops.c > @@ -106,7 +106,7 @@ static int __pm_generic_call(struct devi > const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; > int (*callback)(struct device *); > > - if (!pm || pm_runtime_suspended(dev)) > + if (!pm) > return 0; > > switch (event) { > @@ -224,7 +224,6 @@ static int __pm_generic_resume(struct de > { > const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; > int (*callback)(struct device *); > - int ret; > > if (!pm) > return 0; > @@ -241,17 +240,7 @@ static int __pm_generic_resume(struct de > break; > } > > - if (!callback) > - return 0; > - > - ret = callback(dev); > - if (!ret && !noirq && pm_runtime_enabled(dev)) { > - pm_runtime_disable(dev); > - pm_runtime_set_active(dev); > - pm_runtime_enable(dev); > - } > - > - return ret; > + return callback ? callback(dev) : 0; > } > > /** > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 0/2 v2] PM / Sleep: Simplify generic system suspend callbacks 2011-12-09 23:05 ` Rafael J. Wysocki @ 2011-12-14 23:19 ` Rafael J. Wysocki 2011-12-14 23:20 ` [PATCH 1/2 " Rafael J. Wysocki 2011-12-14 23:21 ` [PATCH 2/2 v2] PM / Sleep: Merge internal functions in generic_ops.c Rafael J. Wysocki 0 siblings, 2 replies; 6+ messages in thread From: Rafael J. Wysocki @ 2011-12-14 23:19 UTC (permalink / raw) To: Linux PM list; +Cc: LKML Hi Everyone, One more udate here. I forgot about the comments in this patch: On Saturday, December 10, 2011, Rafael J. Wysocki wrote: > On Sunday, November 27, 2011, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rjw@sisk.pl> > > > > The pm_runtime_suspended() check in __pm_generic_call() doesn't > > really help and may cause problems to happen, because in some cases > > the system suspend callbacks need to be called even if the given > > device has been suspended by runtime PM. For example, if the device > > generally supports remote wakeup and is not enabled to wake up > > the system from sleep, it should be prevented from generating wakeup > > signals during system suspend and that has to be done by the > > suspend callbacks that the pm_runtime_suspended() check prevents from > > being executed. > > > > Similarly, it may not be a good idea to unconditionally change > > the runtime PM status of the device to 'active' in > > __pm_generic_resume(), because the driver may want to leave the > > device in the 'suspended' state, depending on what happened to it > > before the system suspend and whether or not it is enabled to > > wake up the system. > > > > For the above reasons, remove the pm_runtime_suspended() > > check from __pm_generic_call() and remove the code changing the > > device's runtime PM status from __pm_generic_resume(). > > > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> so the updated one becomes [1/2] in the new series. In addition to that I noticed that __pm_generic_call() and __pm_generic_resume() may be merged after [1/2], which is done by [2/2]. I regard these patches as 3.3 material, if there are no objections. Thanks, Rafael ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2 v2] PM / Sleep: Simplify generic system suspend callbacks 2011-12-14 23:19 ` [PATCH 0/2 v2] " Rafael J. Wysocki @ 2011-12-14 23:20 ` Rafael J. Wysocki 2011-12-14 23:21 ` [PATCH 2/2 v2] PM / Sleep: Merge internal functions in generic_ops.c Rafael J. Wysocki 1 sibling, 0 replies; 6+ messages in thread From: Rafael J. Wysocki @ 2011-12-14 23:20 UTC (permalink / raw) To: Linux PM list; +Cc: LKML From: Rafael J. Wysocki <rjw@sisk.pl> Subject: PM / Sleep: Simplify generic system suspend callbacks The pm_runtime_suspended() check in __pm_generic_call() doesn't really help and may cause problems to happen, because in some cases the system suspend callbacks need to be called even if the given device has been suspended by runtime PM. For example, if the device generally supports remote wakeup and is not enabled to wake up the system from sleep, it should be prevented from generating wakeup signals during system suspend and that has to be done by the suspend callbacks that the pm_runtime_suspended() check prevents from being executed. Similarly, it may not be a good idea to unconditionally change the runtime PM status of the device to 'active' in __pm_generic_resume(), because the driver may want to leave the device in the 'suspended' state, depending on what happened to it before the system suspend and whether or not it is enabled to wake up the system. For the above reasons, remove the pm_runtime_suspended() check from __pm_generic_call() and remove the code changing the device's runtime PM status from __pm_generic_resume(). Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- drivers/base/power/generic_ops.c | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) Index: linux/drivers/base/power/generic_ops.c =================================================================== --- linux.orig/drivers/base/power/generic_ops.c +++ linux/drivers/base/power/generic_ops.c @@ -97,16 +97,16 @@ int pm_generic_prepare(struct device *de * @event: PM transition of the system under way. * @bool: Whether or not this is the "noirq" stage. * - * If the device has not been suspended at run time, execute the - * suspend/freeze/poweroff/thaw callback provided by its driver, if defined, and - * return its error code. Otherwise, return zero. + * Execute the suspend/freeze/poweroff/thaw callback provided by the driver of + * @dev, if defined, and return its error code. Return 0 if the callback is + * not present. */ static int __pm_generic_call(struct device *dev, int event, bool noirq) { const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; int (*callback)(struct device *); - if (!pm || pm_runtime_suspended(dev)) + if (!pm) return 0; switch (event) { @@ -217,14 +217,12 @@ EXPORT_SYMBOL_GPL(pm_generic_thaw); * @bool: Whether or not this is the "noirq" stage. * * Execute the resume/resotre callback provided by the @dev's driver, if - * defined. If it returns 0, change the device's runtime PM status to 'active'. - * Return the callback's error code. + * defined, and return its error code. Return 0 if the callback is not present. */ static int __pm_generic_resume(struct device *dev, int event, bool noirq) { const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; int (*callback)(struct device *); - int ret; if (!pm) return 0; @@ -241,17 +239,7 @@ static int __pm_generic_resume(struct de break; } - if (!callback) - return 0; - - ret = callback(dev); - if (!ret && !noirq && pm_runtime_enabled(dev)) { - pm_runtime_disable(dev); - pm_runtime_set_active(dev); - pm_runtime_enable(dev); - } - - return ret; + return callback ? callback(dev) : 0; } /** ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2 v2] PM / Sleep: Merge internal functions in generic_ops.c 2011-12-14 23:19 ` [PATCH 0/2 v2] " Rafael J. Wysocki 2011-12-14 23:20 ` [PATCH 1/2 " Rafael J. Wysocki @ 2011-12-14 23:21 ` Rafael J. Wysocki 1 sibling, 0 replies; 6+ messages in thread From: Rafael J. Wysocki @ 2011-12-14 23:21 UTC (permalink / raw) To: Linux PM list; +Cc: LKML From: Rafael J. Wysocki <rjw@sisk.pl> After the change that removed the code related to runtime PM from __pm_generic_call() and __pm_generic_resume() these two functions need not be separate any more, so merge them. Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- drivers/base/power/generic_ops.c | 48 ++++++++------------------------------- 1 file changed, 11 insertions(+), 37 deletions(-) Index: linux/drivers/base/power/generic_ops.c =================================================================== --- linux.orig/drivers/base/power/generic_ops.c +++ linux/drivers/base/power/generic_ops.c @@ -97,7 +97,7 @@ int pm_generic_prepare(struct device *de * @event: PM transition of the system under way. * @bool: Whether or not this is the "noirq" stage. * - * Execute the suspend/freeze/poweroff/thaw callback provided by the driver of + * Execute the PM callback corresponding to @event provided by the driver of * @dev, if defined, and return its error code. Return 0 if the callback is * not present. */ @@ -119,9 +119,15 @@ static int __pm_generic_call(struct devi case PM_EVENT_HIBERNATE: callback = noirq ? pm->poweroff_noirq : pm->poweroff; break; + case PM_EVENT_RESUME: + callback = noirq ? pm->resume_noirq : pm->resume; + break; case PM_EVENT_THAW: callback = noirq ? pm->thaw_noirq : pm->thaw; break; + case PM_EVENT_RESTORE: + callback = noirq ? pm->restore_noirq : pm->restore; + break; default: callback = NULL; break; @@ -211,44 +217,12 @@ int pm_generic_thaw(struct device *dev) EXPORT_SYMBOL_GPL(pm_generic_thaw); /** - * __pm_generic_resume - Generic resume/restore callback for subsystems. - * @dev: Device to handle. - * @event: PM transition of the system under way. - * @bool: Whether or not this is the "noirq" stage. - * - * Execute the resume/resotre callback provided by the @dev's driver, if - * defined, and return its error code. Return 0 if the callback is not present. - */ -static int __pm_generic_resume(struct device *dev, int event, bool noirq) -{ - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; - int (*callback)(struct device *); - - if (!pm) - return 0; - - switch (event) { - case PM_EVENT_RESUME: - callback = noirq ? pm->resume_noirq : pm->resume; - break; - case PM_EVENT_RESTORE: - callback = noirq ? pm->restore_noirq : pm->restore; - break; - default: - callback = NULL; - break; - } - - return callback ? callback(dev) : 0; -} - -/** * pm_generic_resume_noirq - Generic resume_noirq callback for subsystems. * @dev: Device to resume. */ int pm_generic_resume_noirq(struct device *dev) { - return __pm_generic_resume(dev, PM_EVENT_RESUME, true); + return __pm_generic_call(dev, PM_EVENT_RESUME, true); } EXPORT_SYMBOL_GPL(pm_generic_resume_noirq); @@ -258,7 +232,7 @@ EXPORT_SYMBOL_GPL(pm_generic_resume_noir */ int pm_generic_resume(struct device *dev) { - return __pm_generic_resume(dev, PM_EVENT_RESUME, false); + return __pm_generic_call(dev, PM_EVENT_RESUME, false); } EXPORT_SYMBOL_GPL(pm_generic_resume); @@ -268,7 +242,7 @@ EXPORT_SYMBOL_GPL(pm_generic_resume); */ int pm_generic_restore_noirq(struct device *dev) { - return __pm_generic_resume(dev, PM_EVENT_RESTORE, true); + return __pm_generic_call(dev, PM_EVENT_RESTORE, true); } EXPORT_SYMBOL_GPL(pm_generic_restore_noirq); @@ -278,7 +252,7 @@ EXPORT_SYMBOL_GPL(pm_generic_restore_noi */ int pm_generic_restore(struct device *dev) { - return __pm_generic_resume(dev, PM_EVENT_RESTORE, false); + return __pm_generic_call(dev, PM_EVENT_RESTORE, false); } EXPORT_SYMBOL_GPL(pm_generic_restore); ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-12-14 23:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-11-17 23:37 [PATCH] PM / Sleep: Remove pm_runtime_suspended() from __pm_generic_call() Rafael J. Wysocki 2011-11-27 12:41 ` [PATCH] PM / Sleep: Simplify generic system suspend callbacks Rafael J. Wysocki 2011-12-09 23:05 ` Rafael J. Wysocki 2011-12-14 23:19 ` [PATCH 0/2 v2] " Rafael J. Wysocki 2011-12-14 23:20 ` [PATCH 1/2 " Rafael J. Wysocki 2011-12-14 23:21 ` [PATCH 2/2 v2] PM / Sleep: Merge internal functions in generic_ops.c Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®