| Message ID | Pine.LNX.4.44L0.1209191030130.1798-100000@iolanthe.rowland.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Wednesday, September 19, 2012, Alan Stern wrote: > This patch (as1591) moves the pm_runtime_get_noresume() and > pm_runtime_put_sync() calls from __device_suspend() and > device_resume() to device_prepare() and device_complete() in the PM > core. > > The reason for doing this is to make sure that parent devices remain > at full power (i.e., don't go into runtime suspend) while their > children are being resumed from a system sleep. > > The PCI core already contained equivalent code to serve the same > purpose. The patch removes the duplicated code, since it is no longer > needed. One of the comments from the PCI core gets moved into the PM > core, and a second comment is added to explain whe the _get_noresume > and _put_sync calls are present. > > Signed-off-by: Alan Stern <stern@rowland.harvard.edu> > > --- > > We discussed this some time ago, but I lost track of the patch. Here > it is, at last. Yes, I remember. OK, applied. Thanks, Rafael > drivers/base/power/main.c | 29 ++++++++++++++++++----------- > drivers/pci/pci-driver.c | 17 ----------------- > 2 files changed, 18 insertions(+), 28 deletions(-) > > Index: usb-3.6/drivers/base/power/main.c > =================================================================== > --- usb-3.6.orig/drivers/base/power/main.c > +++ usb-3.6/drivers/base/power/main.c > @@ -565,7 +565,6 @@ static int device_resume(struct device * > pm_callback_t callback = NULL; > char *info = NULL; > int error = 0; > - bool put = false; > > TRACE_DEVICE(dev); > TRACE_RESUME(0); > @@ -583,7 +582,6 @@ static int device_resume(struct device * > goto Unlock; > > pm_runtime_enable(dev); > - put = true; > > if (dev->pm_domain) { > info = "power domain "; > @@ -636,9 +634,6 @@ static int device_resume(struct device * > > TRACE_RESUME(error); > > - if (put) > - pm_runtime_put_sync(dev); > - > return error; > } > > @@ -749,6 +744,8 @@ static void device_complete(struct devic > } > > device_unlock(dev); > + > + pm_runtime_put_sync(dev); > } > > /** > @@ -1043,12 +1040,16 @@ static int __device_suspend(struct devic > if (async_error) > goto Complete; > > - pm_runtime_get_noresume(dev); > + /* > + * If a device configured to wake up the system from sleep states > + * has been suspended at run time and there's a resume request pending > + * for it, this is equivalent to the device signaling wakeup, so the > + * system suspend operation should be aborted. > + */ > if (pm_runtime_barrier(dev) && device_may_wakeup(dev)) > pm_wakeup_event(dev, 0); > > if (pm_wakeup_pending()) { > - pm_runtime_put_sync(dev); > async_error = -EBUSY; > goto Complete; > } > @@ -1111,12 +1112,10 @@ static int __device_suspend(struct devic > Complete: > complete_all(&dev->power.completion); > > - if (error) { > - pm_runtime_put_sync(dev); > + if (error) > async_error = error; > - } else if (dev->power.is_suspended) { > + else if (dev->power.is_suspended) > __pm_runtime_disable(dev, false); > - } > > return error; > } > @@ -1209,6 +1208,14 @@ static int device_prepare(struct device > char *info = NULL; > int error = 0; > > + /* > + * If a device's parent goes into runtime suspend at the wrong time, > + * it won't be possible to resume the device. To prevent this we > + * block runtime suspend here, during the prepare phase, and allow > + * it again during the complete phase. > + */ > + pm_runtime_get_noresume(dev); > + > device_lock(dev); > > dev->power.wakeup_path = device_may_wakeup(dev); > Index: usb-3.6/drivers/pci/pci-driver.c > =================================================================== > --- usb-3.6.orig/drivers/pci/pci-driver.c > +++ usb-3.6/drivers/pci/pci-driver.c > @@ -624,21 +624,6 @@ static int pci_pm_prepare(struct device > int error = 0; > > /* > - * If a PCI device configured to wake up the system from sleep states > - * has been suspended at run time and there's a resume request pending > - * for it, this is equivalent to the device signaling wakeup, so the > - * system suspend operation should be aborted. > - */ > - pm_runtime_get_noresume(dev); > - if (pm_runtime_barrier(dev) && device_may_wakeup(dev)) > - pm_wakeup_event(dev, 0); > - > - if (pm_wakeup_pending()) { > - pm_runtime_put_sync(dev); > - return -EBUSY; > - } > - > - /* > * PCI devices suspended at run time need to be resumed at this > * point, because in general it is necessary to reconfigure them for > * system suspend. Namely, if the device is supposed to wake up the > @@ -661,8 +646,6 @@ static void pci_pm_complete(struct devic > > if (drv && drv->pm && drv->pm->complete) > drv->pm->complete(dev); > - > - pm_runtime_put_sync(dev); > } > > #else /* !CONFIG_PM_SLEEP */ > > > -- 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
Index: usb-3.6/drivers/base/power/main.c =================================================================== --- usb-3.6.orig/drivers/base/power/main.c +++ usb-3.6/drivers/base/power/main.c @@ -565,7 +565,6 @@ static int device_resume(struct device * pm_callback_t callback = NULL; char *info = NULL; int error = 0; - bool put = false; TRACE_DEVICE(dev); TRACE_RESUME(0); @@ -583,7 +582,6 @@ static int device_resume(struct device * goto Unlock; pm_runtime_enable(dev); - put = true; if (dev->pm_domain) { info = "power domain "; @@ -636,9 +634,6 @@ static int device_resume(struct device * TRACE_RESUME(error); - if (put) - pm_runtime_put_sync(dev); - return error; } @@ -749,6 +744,8 @@ static void device_complete(struct devic } device_unlock(dev); + + pm_runtime_put_sync(dev); } /** @@ -1043,12 +1040,16 @@ static int __device_suspend(struct devic if (async_error) goto Complete; - pm_runtime_get_noresume(dev); + /* + * If a device configured to wake up the system from sleep states + * has been suspended at run time and there's a resume request pending + * for it, this is equivalent to the device signaling wakeup, so the + * system suspend operation should be aborted. + */ if (pm_runtime_barrier(dev) && device_may_wakeup(dev)) pm_wakeup_event(dev, 0); if (pm_wakeup_pending()) { - pm_runtime_put_sync(dev); async_error = -EBUSY; goto Complete; } @@ -1111,12 +1112,10 @@ static int __device_suspend(struct devic Complete: complete_all(&dev->power.completion); - if (error) { - pm_runtime_put_sync(dev); + if (error) async_error = error; - } else if (dev->power.is_suspended) { + else if (dev->power.is_suspended) __pm_runtime_disable(dev, false); - } return error; } @@ -1209,6 +1208,14 @@ static int device_prepare(struct device char *info = NULL; int error = 0; + /* + * If a device's parent goes into runtime suspend at the wrong time, + * it won't be possible to resume the device. To prevent this we + * block runtime suspend here, during the prepare phase, and allow + * it again during the complete phase. + */ + pm_runtime_get_noresume(dev); + device_lock(dev); dev->power.wakeup_path = device_may_wakeup(dev); Index: usb-3.6/drivers/pci/pci-driver.c =================================================================== --- usb-3.6.orig/drivers/pci/pci-driver.c +++ usb-3.6/drivers/pci/pci-driver.c @@ -624,21 +624,6 @@ static int pci_pm_prepare(struct device int error = 0; /* - * If a PCI device configured to wake up the system from sleep states - * has been suspended at run time and there's a resume request pending - * for it, this is equivalent to the device signaling wakeup, so the - * system suspend operation should be aborted. - */ - pm_runtime_get_noresume(dev); - if (pm_runtime_barrier(dev) && device_may_wakeup(dev)) - pm_wakeup_event(dev, 0); - - if (pm_wakeup_pending()) { - pm_runtime_put_sync(dev); - return -EBUSY; - } - - /* * PCI devices suspended at run time need to be resumed at this * point, because in general it is necessary to reconfigure them for * system suspend. Namely, if the device is supposed to wake up the @@ -661,8 +646,6 @@ static void pci_pm_complete(struct devic if (drv && drv->pm && drv->pm->complete) drv->pm->complete(dev); - - pm_runtime_put_sync(dev); } #else /* !CONFIG_PM_SLEEP */
This patch (as1591) moves the pm_runtime_get_noresume() and pm_runtime_put_sync() calls from __device_suspend() and device_resume() to device_prepare() and device_complete() in the PM core. The reason for doing this is to make sure that parent devices remain at full power (i.e., don't go into runtime suspend) while their children are being resumed from a system sleep. The PCI core already contained equivalent code to serve the same purpose. The patch removes the duplicated code, since it is no longer needed. One of the comments from the PCI core gets moved into the PM core, and a second comment is added to explain whe the _get_noresume and _put_sync calls are present. Signed-off-by: Alan Stern <stern@rowland.harvard.edu> --- We discussed this some time ago, but I lost track of the patch. Here it is, at last. drivers/base/power/main.c | 29 ++++++++++++++++++----------- drivers/pci/pci-driver.c | 17 ----------------- 2 files changed, 18 insertions(+), 28 deletions(-) -- 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