| Message ID | 1476370734-23168-1-git-send-email-ulf.hansson@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Ulf, On 2016-10-13 16:58, Ulf Hansson wrote: > When the pm_runtime_force_suspend|resume() helpers were invented, we still > had CONFIG_PM_RUNTIME and CONFIG_PM_SLEEP as separate Kconfig options. > > To make sure these helpers worked for all combinations and without > introducing too much of complexity, the device was always resumed in > pm_runtime_force_resume(). > > More precisely, when CONFIG_PM_SLEEP was set and CONFIG_PM_RUNTIME was > unset, we needed to resume the device as the subsystem/driver couldn't > rely on using runtime PM to do it. > > As the CONFIG_PM_RUNTIME option was merged into CONFIG_PM a while ago, it > removed this combination, of using CONFIG_PM_SLEEP without the earlier > CONFIG_PM_RUNTIME. > > For this reason we can now rely on the subsystem/driver to use runtime PM > to resume the device, instead of forcing that to be done in all cases. In > other words, let's defer the runtime resume to a later point when it's > actually needed. > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> I wasn't aware that You are working on this feature. I had it on my todo list, because it was a waste of power to always wake up all devices in resume from sleep. Your change solves this issue and it works fine with my use cases. :) You can add my: Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> I've tested it together with my Exynos IOMMU runtime pm rework and Rafael's device dependency patchset (needed for my rework). > --- > > Changes in v3: > - Updated to take care of parent-child relations. > - Improved comment in the code and updated text in a function header > to better describe the changes. > > This patch has earlier been sent standalone, but also as a part of series. In > the end it turned out the solution needed some improvement to take care of > parent-child relations, as reported by Geert [1]. > > Geert, I would really appreciate if you could help out testing to make sure the > reported issue is fixed. > > [1] > https://patches.linaro.org/patch/67940/ > > --- > > drivers/base/power/runtime.c | 35 ++++++++++++++++++++++++++++++----- > 1 file changed, 30 insertions(+), 5 deletions(-) > > diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c > index e097d35..f662267 100644 > --- a/drivers/base/power/runtime.c > +++ b/drivers/base/power/runtime.c > @@ -1478,6 +1478,16 @@ int pm_runtime_force_suspend(struct device *dev) > if (ret) > goto err; > > + /* > + * Increase the runtime PM usage count for the device's parent, in case > + * when we find the device being used when system suspend was invoked. > + * This informs pm_runtime_force_resume() to resume the parent > + * immediately, which is needed to be able to resume its children, > + * when not deferring the resume to be managed via runtime PM. > + */ > + if (dev->parent && atomic_read(&dev->power.usage_count) > 1) > + pm_runtime_get_noresume(dev->parent); > + > pm_runtime_set_suspended(dev); > return 0; > err: > @@ -1487,16 +1497,20 @@ err: > EXPORT_SYMBOL_GPL(pm_runtime_force_suspend); > > /** > - * pm_runtime_force_resume - Force a device into resume state. > + * pm_runtime_force_resume - Force a device into resume state if needed. > * @dev: Device to resume. > * > * Prior invoking this function we expect the user to have brought the device > * into low power state by a call to pm_runtime_force_suspend(). Here we reverse > - * those actions and brings the device into full power. We update the runtime PM > - * status and re-enables runtime PM. > + * those actions and brings the device into full power, if it is expected to be > + * used on system resume. To distinguish that, we check whether the runtime PM > + * usage count is greater than 1 (the PM core increases the usage count in the > + * system PM prepare phase), as that indicates a real user (such as a subsystem, > + * driver, userspace, etc.) is using it. If that is the case, the device is > + * expected to be used on system resume as well, so then we resume it. In the > + * other case, we defer the resume to be managed via runtime PM. > * > - * Typically this function may be invoked from a system resume callback to make > - * sure the device is put into full power state. > + * Typically this function may be invoked from a system resume callback. > */ > int pm_runtime_force_resume(struct device *dev) > { > @@ -1513,6 +1527,17 @@ int pm_runtime_force_resume(struct device *dev) > if (!pm_runtime_status_suspended(dev)) > goto out; > > + /* > + * Decrease the parent's runtime PM usage count, if we increased it > + * during system suspend in pm_runtime_force_suspend(). > + */ > + if (atomic_read(&dev->power.usage_count) > 1) { > + if (dev->parent) > + pm_runtime_put_noidle(dev->parent); > + } else { > + goto out; > + } > + > ret = pm_runtime_set_active(dev); > if (ret) > goto out; Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland -- 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
Hi Ulf, On Thu, Oct 13, 2016 at 4:58 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > When the pm_runtime_force_suspend|resume() helpers were invented, we still > had CONFIG_PM_RUNTIME and CONFIG_PM_SLEEP as separate Kconfig options. > > To make sure these helpers worked for all combinations and without > introducing too much of complexity, the device was always resumed in > pm_runtime_force_resume(). > > More precisely, when CONFIG_PM_SLEEP was set and CONFIG_PM_RUNTIME was > unset, we needed to resume the device as the subsystem/driver couldn't > rely on using runtime PM to do it. > > As the CONFIG_PM_RUNTIME option was merged into CONFIG_PM a while ago, it > removed this combination, of using CONFIG_PM_SLEEP without the earlier > CONFIG_PM_RUNTIME. > > For this reason we can now rely on the subsystem/driver to use runtime PM > to resume the device, instead of forcing that to be done in all cases. In > other words, let's defer the runtime resume to a later point when it's > actually needed. > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > --- > > Changes in v3: > - Updated to take care of parent-child relations. > - Improved comment in the code and updated text in a function header > to better describe the changes. Thanks for the update! > This patch has earlier been sent standalone, but also as a part of series. In > the end it turned out the solution needed some improvement to take care of > parent-child relations, as reported by Geert [1]. > > Geert, I would really appreciate if you could help out testing to make sure the > reported issue is fixed. Unfortunately it doesn't help. Still fails on both r8a73a4/ape6evm and sh73a0/kzm9g. First log (with some debugging) is: [ 455.004744] PM: Syncing filesystems ... [ 455.011339] done. [ 455.013278] PM: Preparing system for sleep (mem) [ 455.029505] Freezing user space processes ... (elapsed 0.002 seconds) done. [ 455.039014] Freezing remaining freezable tasks ... (elapsed 0.002 seconds) done. [ 455.048691] PM: Suspending system (mem) [ 455.074914] PM: suspend of devices complete after 14.545 msecs [ 455.088095] PM: late suspend of devices complete after 7.338 msecs [ 455.100999] rmobile_pd_power_down: a3sp [ 455.105079] PM: noirq suspend of devices complete after 10.802 msecs [ 455.111515] Disabling non-boot CPUs ... [ 455.115389] Suspended for 5.486 seconds [ 455.116189] rmobile_pd_power_up: a3sp [ 455.181331] PM: noirq resume of devices complete after 65.698 msecs [ 455.194384] PM: early resume of devices complete after 6.162 msecs [ 455.202701] Unhandled fault: imprecise external abort (0x1406) at 0x00000000 [ 455.209742] pgd = ee684000 [ 455.212446] [00000000] *pgd=7fc35835 [ 455.216042] Internal error: : 1406 [#1] SMP ARM [ 455.220577] CPU: 0 PID: 1128 Comm: s2ram Not tainted 4.9.0-rc1-ape6evm-00630-g98d02f8b47e9ad17-dirty #463 [ 455.230128] Hardware name: Generic R8A73A4 (Flattened Device Tree) [ 455.236300] task: ee44a740 task.stack: ee5d4000 [ 455.240842] PC is at smsc911x_resume+0x6c/0xbc [ 455.245280] LR is at smsc911x_resume+0x6c/0xbc [ 455.249720] pc : [<c02e2ce4>] lr : [<c02e2ce4>] psr: 200d0093 [ 455.249720] sp : ee5d5d70 ip : 4302710e fp : c0e35420 [ 455.261179] r10: 200d0013 r9 : c056dbe5 r8 : c05861f8 [ 455.266396] r7 : ee866600 r6 : ee866000 r5 : 00000064 r4 : ee866648 [ 455.272914] r3 : f08c5000 r2 : 00000000 r1 : f08c5084 r0 : 00000000 [ 455.279434] Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none [ 455.286645] Control: 10c5387d Table: 6e68406a DAC: 00000051 [ 455.292382] Process s2ram (pid: 1128, stack limit = 0xee5d4210) [ 455.298293] Stack: (0xee5d5d70 to 0xee5d6000) [ 455.302647] 5d60: ee86a810 c065b8ca c066a840 c06093ec [ 455.310814] 5d80: c066a82c c056dbe5 c02c6f48 c02d11a8 c02d208c ee86a810 00000000 00000000 [ 455.318981] 5da0: 00000000 00000000 ee44a740 00000001 ee86a810 00000010 ee86a844 00000000 [ 455.327148] 5dc0: ee86a810 c06093ec c0e35420 c02d21a0 ee86a8c0 c0e35420 c066a818 00000010 [ 455.335316] 5de0: c065573c c02d23a0 fc08817d 00000069 fb4c1b05 c066c224 00000000 c066c224 [ 455.343483] 5e00: fc08817d 00000069 fb4c1b05 00000010 00000003 c066a818 c0df99a8 c061cdc6 [ 455.351650] 5e20: c066a818 c066a818 c06093ec c02d269c 00000000 c0083824 c05497f9 c05497da [ 455.359817] 5e40: c061cdc6 c0df99b4 c05497c9 c008dbbc c0549792 ee5d5e74 ee5d5e74 c061cdc6 [ 455.367984] 5e60: c066a818 c06093ec 00000003 c05497da c061cdc6 c0df99b4 c05497c9 c00845a4 [ 455.376150] 5e80: 00000000 c01a3214 00000000 00000008 00000001 00000003 00000003 ee49f840 [ 455.384317] 5ea0: 00000004 c0543e99 c0df99b4 00000000 00000000 c0082058 00000004 00000004 [ 455.392484] 5ec0: eeb75240 ee49f840 ee5d5f88 eeb7524c 00000051 c01a325c 00000000 00000000 [ 455.400652] 5ee0: 00000004 c01a30d0 ee446a80 ee5d5f88 000ce408 c000fce4 ee5d4000 c01392d8 [ 455.408819] 5f00: eebb023c c007b588 00000001 c061cde4 eebb023c c0097968 c0097920 c0097cac [ 455.416985] 5f20: d0632d2c eebb00b4 00000000 c013c970 00000001 00000000 c0139520 00000000 [ 455.425152] 5f40: 00000000 00000004 ee446a80 00000004 ee446a80 ee5d5f88 000ce408 c0139534 [ 455.433320] 5f60: ee446a80 000ce408 00000004 ee446a80 ee446a80 00000004 000ce408 c000fce4 [ 455.441486] 5f80: ee5d4000 c0139680 00000000 00000000 00000004 00000004 000ce408 b6f25b50 [ 455.449653] 5fa0: 00000004 c000fb40 00000004 000ce408 00000001 000ce408 00000004 00000000 [ 455.457820] 5fc0: 00000004 000ce408 b6f25b50 00000004 00000004 00000000 000c5758 00000000 [ 455.465987] 5fe0: 00000000 bef2b754 b6e88c65 b6ec3e56 40010030 00000001 2b14070c 1a0d2022 [ 455.474169] [<c02e2ce4>] (smsc911x_resume) from [<c02d11a8>] (dpm_run_callback+0x118/0x33c) [ 455.482515] [<c02d11a8>] (dpm_run_callback) from [<c02d21a0>] (device_resume+0x164/0x1a8) [ 455.490685] [<c02d21a0>] (device_resume) from [<c02d23a0>] (dpm_resume+0x1bc/0x4ac) [ 455.498335] [<c02d23a0>] (dpm_resume) from [<c02d269c>] (dpm_resume_end+0xc/0x18) [ 455.505818] [<c02d269c>] (dpm_resume_end) from [<c0083824>] (suspend_devices_and_enter+0x734/0xc28) [ 455.514856] [<c0083824>] (suspend_devices_and_enter) from [<c00845a4>] (pm_suspend+0x88c/0xa24) [ 455.523546] [<c00845a4>] (pm_suspend) from [<c0082058>] (state_store+0xa0/0xbc) [ 455.530856] [<c0082058>] (state_store) from [<c01a325c>] (kernfs_fop_write+0x18c/0x1c8) [ 455.538857] [<c01a325c>] (kernfs_fop_write) from [<c01392d8>] (__vfs_write+0x20/0x108) [ 455.546765] [<c01392d8>] (__vfs_write) from [<c0139534>] (vfs_write+0xb8/0x144) [ 455.554068] [<c0139534>] (vfs_write) from [<c0139680>] (SyS_write+0x40/0x80) [ 455.561115] [<c0139680>] (SyS_write) from [<c000fb40>] (ret_fast_syscall+0x0/0x1c) [ 455.568676] Code: e5933000 e1a0a000 e1a00007 e12fff33 (e1a0100a) [ 455.574769] ---[ end trace b00ed34052fc8444 ]--- Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- 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
On 18 October 2016 at 12:32, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > Hi Ulf, > > > On 2016-10-13 16:58, Ulf Hansson wrote: >> >> When the pm_runtime_force_suspend|resume() helpers were invented, we still >> had CONFIG_PM_RUNTIME and CONFIG_PM_SLEEP as separate Kconfig options. >> >> To make sure these helpers worked for all combinations and without >> introducing too much of complexity, the device was always resumed in >> pm_runtime_force_resume(). >> >> More precisely, when CONFIG_PM_SLEEP was set and CONFIG_PM_RUNTIME was >> unset, we needed to resume the device as the subsystem/driver couldn't >> rely on using runtime PM to do it. >> >> As the CONFIG_PM_RUNTIME option was merged into CONFIG_PM a while ago, it >> removed this combination, of using CONFIG_PM_SLEEP without the earlier >> CONFIG_PM_RUNTIME. >> >> For this reason we can now rely on the subsystem/driver to use runtime PM >> to resume the device, instead of forcing that to be done in all cases. In >> other words, let's defer the runtime resume to a later point when it's >> actually needed. >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > > > I wasn't aware that You are working on this feature. I had it on my todo > list, > because it was a waste of power to always wake up all devices in resume from > sleep. Your change solves this issue and it works fine with my use cases. :) > You can add my: > > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > > I've tested it together with my Exynos IOMMU runtime pm rework and Rafael's > device dependency patchset (needed for my rework). Great news, thanks for helping out testing! Kind regards Uffe > > >> --- >> >> Changes in v3: >> - Updated to take care of parent-child relations. >> - Improved comment in the code and updated text in a function >> header >> to better describe the changes. >> >> This patch has earlier been sent standalone, but also as a part of series. >> In >> the end it turned out the solution needed some improvement to take care of >> parent-child relations, as reported by Geert [1]. >> >> Geert, I would really appreciate if you could help out testing to make >> sure the >> reported issue is fixed. >> >> [1] >> https://patches.linaro.org/patch/67940/ >> >> --- >> >> drivers/base/power/runtime.c | 35 ++++++++++++++++++++++++++++++----- >> 1 file changed, 30 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c >> index e097d35..f662267 100644 >> --- a/drivers/base/power/runtime.c >> +++ b/drivers/base/power/runtime.c >> @@ -1478,6 +1478,16 @@ int pm_runtime_force_suspend(struct device *dev) >> if (ret) >> goto err; >> + /* >> + * Increase the runtime PM usage count for the device's parent, in >> case >> + * when we find the device being used when system suspend was >> invoked. >> + * This informs pm_runtime_force_resume() to resume the parent >> + * immediately, which is needed to be able to resume its children, >> + * when not deferring the resume to be managed via runtime PM. >> + */ >> + if (dev->parent && atomic_read(&dev->power.usage_count) > 1) >> + pm_runtime_get_noresume(dev->parent); >> + >> pm_runtime_set_suspended(dev); >> return 0; >> err: >> @@ -1487,16 +1497,20 @@ err: >> EXPORT_SYMBOL_GPL(pm_runtime_force_suspend); >> /** >> - * pm_runtime_force_resume - Force a device into resume state. >> + * pm_runtime_force_resume - Force a device into resume state if needed. >> * @dev: Device to resume. >> * >> * Prior invoking this function we expect the user to have brought the >> device >> * into low power state by a call to pm_runtime_force_suspend(). Here we >> reverse >> - * those actions and brings the device into full power. We update the >> runtime PM >> - * status and re-enables runtime PM. >> + * those actions and brings the device into full power, if it is expected >> to be >> + * used on system resume. To distinguish that, we check whether the >> runtime PM >> + * usage count is greater than 1 (the PM core increases the usage count >> in the >> + * system PM prepare phase), as that indicates a real user (such as a >> subsystem, >> + * driver, userspace, etc.) is using it. If that is the case, the device >> is >> + * expected to be used on system resume as well, so then we resume it. In >> the >> + * other case, we defer the resume to be managed via runtime PM. >> * >> - * Typically this function may be invoked from a system resume callback >> to make >> - * sure the device is put into full power state. >> + * Typically this function may be invoked from a system resume callback. >> */ >> int pm_runtime_force_resume(struct device *dev) >> { >> @@ -1513,6 +1527,17 @@ int pm_runtime_force_resume(struct device *dev) >> if (!pm_runtime_status_suspended(dev)) >> goto out; >> + /* >> + * Decrease the parent's runtime PM usage count, if we increased >> it >> + * during system suspend in pm_runtime_force_suspend(). >> + */ >> + if (atomic_read(&dev->power.usage_count) > 1) { >> + if (dev->parent) >> + pm_runtime_put_noidle(dev->parent); >> + } else { >> + goto out; >> + } >> + >> ret = pm_runtime_set_active(dev); >> if (ret) >> goto out; > > > Best regards > -- > Marek Szyprowski, PhD > Samsung R&D Institute Poland > -- 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
On 18 October 2016 at 15:50, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Ulf, > > On Thu, Oct 13, 2016 at 4:58 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> When the pm_runtime_force_suspend|resume() helpers were invented, we still >> had CONFIG_PM_RUNTIME and CONFIG_PM_SLEEP as separate Kconfig options. >> >> To make sure these helpers worked for all combinations and without >> introducing too much of complexity, the device was always resumed in >> pm_runtime_force_resume(). >> >> More precisely, when CONFIG_PM_SLEEP was set and CONFIG_PM_RUNTIME was >> unset, we needed to resume the device as the subsystem/driver couldn't >> rely on using runtime PM to do it. >> >> As the CONFIG_PM_RUNTIME option was merged into CONFIG_PM a while ago, it >> removed this combination, of using CONFIG_PM_SLEEP without the earlier >> CONFIG_PM_RUNTIME. >> >> For this reason we can now rely on the subsystem/driver to use runtime PM >> to resume the device, instead of forcing that to be done in all cases. In >> other words, let's defer the runtime resume to a later point when it's >> actually needed. >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >> --- >> >> Changes in v3: >> - Updated to take care of parent-child relations. >> - Improved comment in the code and updated text in a function header >> to better describe the changes. > > Thanks for the update! > >> This patch has earlier been sent standalone, but also as a part of series. In >> the end it turned out the solution needed some improvement to take care of >> parent-child relations, as reported by Geert [1]. >> >> Geert, I would really appreciate if you could help out testing to make sure the >> reported issue is fixed. > > Unfortunately it doesn't help. Still fails on both r8a73a4/ape6evm and > sh73a0/kzm9g. Again, thanks for testing! Seems like we need to debug this a bit more. :-) I have more or less set up the similar environment as you have, using the simple PM bus, and having a child device below it. The child device is being operated by my runtime PM test-driver - and both devices are in a genpd. Anyway, I will continue to look into this via my test environment, but in the end we probably need to add some debug code in the PM callbacks of the real drivers (and HW) at your side. I hope you are willing to help a little with that. Allow me to send you a debug patch within a couple of days, hopefully that will give us some answers. Unless, you have other ideas for how to proceed? > > First log (with some debugging) is: > > [ 455.004744] PM: Syncing filesystems ... [ 455.011339] done. > [ 455.013278] PM: Preparing system for sleep (mem) > [ 455.029505] Freezing user space processes ... (elapsed 0.002 seconds) done. > [ 455.039014] Freezing remaining freezable tasks ... (elapsed 0.002 > seconds) done. > [ 455.048691] PM: Suspending system (mem) > [ 455.074914] PM: suspend of devices complete after 14.545 msecs > [ 455.088095] PM: late suspend of devices complete after 7.338 msecs > [ 455.100999] rmobile_pd_power_down: a3sp > [ 455.105079] PM: noirq suspend of devices complete after 10.802 msecs > [ 455.111515] Disabling non-boot CPUs ... > [ 455.115389] Suspended for 5.486 seconds > [ 455.116189] rmobile_pd_power_up: a3sp > [ 455.181331] PM: noirq resume of devices complete after 65.698 msecs > [ 455.194384] PM: early resume of devices complete after 6.162 msecs > [ 455.202701] Unhandled fault: imprecise external abort (0x1406) at 0x00000000 > [ 455.209742] pgd = ee684000 > [ 455.212446] [00000000] *pgd=7fc35835 > [ 455.216042] Internal error: : 1406 [#1] SMP ARM > [ 455.220577] CPU: 0 PID: 1128 Comm: s2ram Not tainted > 4.9.0-rc1-ape6evm-00630-g98d02f8b47e9ad17-dirty #463 > [ 455.230128] Hardware name: Generic R8A73A4 (Flattened Device Tree) > [ 455.236300] task: ee44a740 task.stack: ee5d4000 > [ 455.240842] PC is at smsc911x_resume+0x6c/0xbc > [ 455.245280] LR is at smsc911x_resume+0x6c/0xbc > [ 455.249720] pc : [<c02e2ce4>] lr : [<c02e2ce4>] psr: 200d0093 > [ 455.249720] sp : ee5d5d70 ip : 4302710e fp : c0e35420 > [ 455.261179] r10: 200d0013 r9 : c056dbe5 r8 : c05861f8 > [ 455.266396] r7 : ee866600 r6 : ee866000 r5 : 00000064 r4 : ee866648 > [ 455.272914] r3 : f08c5000 r2 : 00000000 r1 : f08c5084 r0 : 00000000 > [ 455.279434] Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM > Segment none > [ 455.286645] Control: 10c5387d Table: 6e68406a DAC: 00000051 > [ 455.292382] Process s2ram (pid: 1128, stack limit = 0xee5d4210) > [ 455.298293] Stack: (0xee5d5d70 to 0xee5d6000) > [ 455.302647] 5d60: ee86a810 > c065b8ca c066a840 c06093ec > [ 455.310814] 5d80: c066a82c c056dbe5 c02c6f48 c02d11a8 c02d208c > ee86a810 00000000 00000000 > [ 455.318981] 5da0: 00000000 00000000 ee44a740 00000001 ee86a810 > 00000010 ee86a844 00000000 > [ 455.327148] 5dc0: ee86a810 c06093ec c0e35420 c02d21a0 ee86a8c0 > c0e35420 c066a818 00000010 > [ 455.335316] 5de0: c065573c c02d23a0 fc08817d 00000069 fb4c1b05 > c066c224 00000000 c066c224 > [ 455.343483] 5e00: fc08817d 00000069 fb4c1b05 00000010 00000003 > c066a818 c0df99a8 c061cdc6 > [ 455.351650] 5e20: c066a818 c066a818 c06093ec c02d269c 00000000 > c0083824 c05497f9 c05497da > [ 455.359817] 5e40: c061cdc6 c0df99b4 c05497c9 c008dbbc c0549792 > ee5d5e74 ee5d5e74 c061cdc6 > [ 455.367984] 5e60: c066a818 c06093ec 00000003 c05497da c061cdc6 > c0df99b4 c05497c9 c00845a4 > [ 455.376150] 5e80: 00000000 c01a3214 00000000 00000008 00000001 > 00000003 00000003 ee49f840 > [ 455.384317] 5ea0: 00000004 c0543e99 c0df99b4 00000000 00000000 > c0082058 00000004 00000004 > [ 455.392484] 5ec0: eeb75240 ee49f840 ee5d5f88 eeb7524c 00000051 > c01a325c 00000000 00000000 > [ 455.400652] 5ee0: 00000004 c01a30d0 ee446a80 ee5d5f88 000ce408 > c000fce4 ee5d4000 c01392d8 > [ 455.408819] 5f00: eebb023c c007b588 00000001 c061cde4 eebb023c > c0097968 c0097920 c0097cac > [ 455.416985] 5f20: d0632d2c eebb00b4 00000000 c013c970 00000001 > 00000000 c0139520 00000000 > [ 455.425152] 5f40: 00000000 00000004 ee446a80 00000004 ee446a80 > ee5d5f88 000ce408 c0139534 > [ 455.433320] 5f60: ee446a80 000ce408 00000004 ee446a80 ee446a80 > 00000004 000ce408 c000fce4 > [ 455.441486] 5f80: ee5d4000 c0139680 00000000 00000000 00000004 > 00000004 000ce408 b6f25b50 > [ 455.449653] 5fa0: 00000004 c000fb40 00000004 000ce408 00000001 > 000ce408 00000004 00000000 > [ 455.457820] 5fc0: 00000004 000ce408 b6f25b50 00000004 00000004 > 00000000 000c5758 00000000 > [ 455.465987] 5fe0: 00000000 bef2b754 b6e88c65 b6ec3e56 40010030 > 00000001 2b14070c 1a0d2022 > [ 455.474169] [<c02e2ce4>] (smsc911x_resume) from [<c02d11a8>] > (dpm_run_callback+0x118/0x33c) > [ 455.482515] [<c02d11a8>] (dpm_run_callback) from [<c02d21a0>] > (device_resume+0x164/0x1a8) > [ 455.490685] [<c02d21a0>] (device_resume) from [<c02d23a0>] > (dpm_resume+0x1bc/0x4ac) > [ 455.498335] [<c02d23a0>] (dpm_resume) from [<c02d269c>] > (dpm_resume_end+0xc/0x18) > [ 455.505818] [<c02d269c>] (dpm_resume_end) from [<c0083824>] > (suspend_devices_and_enter+0x734/0xc28) > [ 455.514856] [<c0083824>] (suspend_devices_and_enter) from > [<c00845a4>] (pm_suspend+0x88c/0xa24) > [ 455.523546] [<c00845a4>] (pm_suspend) from [<c0082058>] > (state_store+0xa0/0xbc) > [ 455.530856] [<c0082058>] (state_store) from [<c01a325c>] > (kernfs_fop_write+0x18c/0x1c8) > [ 455.538857] [<c01a325c>] (kernfs_fop_write) from [<c01392d8>] > (__vfs_write+0x20/0x108) > [ 455.546765] [<c01392d8>] (__vfs_write) from [<c0139534>] > (vfs_write+0xb8/0x144) > [ 455.554068] [<c0139534>] (vfs_write) from [<c0139680>] (SyS_write+0x40/0x80) > [ 455.561115] [<c0139680>] (SyS_write) from [<c000fb40>] > (ret_fast_syscall+0x0/0x1c) > [ 455.568676] Code: e5933000 e1a0a000 e1a00007 e12fff33 (e1a0100a) > [ 455.574769] ---[ end trace b00ed34052fc8444 ]--- > The problem seems very similar as what we had before. Clearly the problem wasn't just about dealing with parent-childs, but also something else.... Kind regards Uffe -- 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
Hi Ulf, On Fri, Oct 21, 2016 at 10:42 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 18 October 2016 at 15:50, Geert Uytterhoeven <geert@linux-m68k.org> wrote: >> On Thu, Oct 13, 2016 at 4:58 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> When the pm_runtime_force_suspend|resume() helpers were invented, we still >>> had CONFIG_PM_RUNTIME and CONFIG_PM_SLEEP as separate Kconfig options. >>> >>> To make sure these helpers worked for all combinations and without >>> introducing too much of complexity, the device was always resumed in >>> pm_runtime_force_resume(). >>> >>> More precisely, when CONFIG_PM_SLEEP was set and CONFIG_PM_RUNTIME was >>> unset, we needed to resume the device as the subsystem/driver couldn't >>> rely on using runtime PM to do it. >>> >>> As the CONFIG_PM_RUNTIME option was merged into CONFIG_PM a while ago, it >>> removed this combination, of using CONFIG_PM_SLEEP without the earlier >>> CONFIG_PM_RUNTIME. >>> >>> For this reason we can now rely on the subsystem/driver to use runtime PM >>> to resume the device, instead of forcing that to be done in all cases. In >>> other words, let's defer the runtime resume to a later point when it's >>> actually needed. >>> >>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >>> --- >>> >>> Changes in v3: >>> - Updated to take care of parent-child relations. >>> - Improved comment in the code and updated text in a function header >>> to better describe the changes. >> >> Thanks for the update! >> >>> This patch has earlier been sent standalone, but also as a part of series. In >>> the end it turned out the solution needed some improvement to take care of >>> parent-child relations, as reported by Geert [1]. >>> >>> Geert, I would really appreciate if you could help out testing to make sure the >>> reported issue is fixed. >> >> Unfortunately it doesn't help. Still fails on both r8a73a4/ape6evm and >> sh73a0/kzm9g. > > Again, thanks for testing! Seems like we need to debug this a bit more. :-) > > I have more or less set up the similar environment as you have, using > the simple PM bus, and having a child device below it. The child > device is being operated by my runtime PM test-driver - and both > devices are in a genpd. > > Anyway, I will continue to look into this via my test environment, but > in the end we probably need to add some debug code in the PM callbacks > of the real drivers (and HW) at your side. I hope you are willing to > help a little with that. Allow me to send you a debug patch within a > couple of days, hopefully that will give us some answers. Unless, you > have other ideas for how to proceed? No problem. I can easily test a debug patch. Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- 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
On 13 October 2016 at 16:58, Ulf Hansson <ulf.hansson@linaro.org> wrote: > When the pm_runtime_force_suspend|resume() helpers were invented, we still > had CONFIG_PM_RUNTIME and CONFIG_PM_SLEEP as separate Kconfig options. > > To make sure these helpers worked for all combinations and without > introducing too much of complexity, the device was always resumed in > pm_runtime_force_resume(). > > More precisely, when CONFIG_PM_SLEEP was set and CONFIG_PM_RUNTIME was > unset, we needed to resume the device as the subsystem/driver couldn't > rely on using runtime PM to do it. > > As the CONFIG_PM_RUNTIME option was merged into CONFIG_PM a while ago, it > removed this combination, of using CONFIG_PM_SLEEP without the earlier > CONFIG_PM_RUNTIME. > > For this reason we can now rely on the subsystem/driver to use runtime PM > to resume the device, instead of forcing that to be done in all cases. In > other words, let's defer the runtime resume to a later point when it's > actually needed. > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> Rafael, this one is ready to be queued. Unless there are other objections. The problems that was reported by Geert for this change, has been taken care of. Those are fixed by the patch for the smsc911x ethernet driver [1], which you queued a while ago. I realized that wasn't obvious unless one digested the conversations in detail. Geert, perhaps you can confirm by providing a tested-by tag? Kind regards Uffe [1] https://www.spinics.net/lists/netdev/msg401339.html > --- > > Changes in v3: > - Updated to take care of parent-child relations. > - Improved comment in the code and updated text in a function header > to better describe the changes. > > This patch has earlier been sent standalone, but also as a part of series. In > the end it turned out the solution needed some improvement to take care of > parent-child relations, as reported by Geert [1]. > > Geert, I would really appreciate if you could help out testing to make sure the > reported issue is fixed. > > [1] > https://patches.linaro.org/patch/67940/ > > --- > > drivers/base/power/runtime.c | 35 ++++++++++++++++++++++++++++++----- > 1 file changed, 30 insertions(+), 5 deletions(-) > > diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c > index e097d35..f662267 100644 > --- a/drivers/base/power/runtime.c > +++ b/drivers/base/power/runtime.c > @@ -1478,6 +1478,16 @@ int pm_runtime_force_suspend(struct device *dev) > if (ret) > goto err; > > + /* > + * Increase the runtime PM usage count for the device's parent, in case > + * when we find the device being used when system suspend was invoked. > + * This informs pm_runtime_force_resume() to resume the parent > + * immediately, which is needed to be able to resume its children, > + * when not deferring the resume to be managed via runtime PM. > + */ > + if (dev->parent && atomic_read(&dev->power.usage_count) > 1) > + pm_runtime_get_noresume(dev->parent); > + > pm_runtime_set_suspended(dev); > return 0; > err: > @@ -1487,16 +1497,20 @@ err: > EXPORT_SYMBOL_GPL(pm_runtime_force_suspend); > > /** > - * pm_runtime_force_resume - Force a device into resume state. > + * pm_runtime_force_resume - Force a device into resume state if needed. > * @dev: Device to resume. > * > * Prior invoking this function we expect the user to have brought the device > * into low power state by a call to pm_runtime_force_suspend(). Here we reverse > - * those actions and brings the device into full power. We update the runtime PM > - * status and re-enables runtime PM. > + * those actions and brings the device into full power, if it is expected to be > + * used on system resume. To distinguish that, we check whether the runtime PM > + * usage count is greater than 1 (the PM core increases the usage count in the > + * system PM prepare phase), as that indicates a real user (such as a subsystem, > + * driver, userspace, etc.) is using it. If that is the case, the device is > + * expected to be used on system resume as well, so then we resume it. In the > + * other case, we defer the resume to be managed via runtime PM. > * > - * Typically this function may be invoked from a system resume callback to make > - * sure the device is put into full power state. > + * Typically this function may be invoked from a system resume callback. > */ > int pm_runtime_force_resume(struct device *dev) > { > @@ -1513,6 +1527,17 @@ int pm_runtime_force_resume(struct device *dev) > if (!pm_runtime_status_suspended(dev)) > goto out; > > + /* > + * Decrease the parent's runtime PM usage count, if we increased it > + * during system suspend in pm_runtime_force_suspend(). > + */ > + if (atomic_read(&dev->power.usage_count) > 1) { > + if (dev->parent) > + pm_runtime_put_noidle(dev->parent); > + } else { > + goto out; > + } > + > ret = pm_runtime_set_active(dev); > if (ret) > goto out; > -- > 1.9.1 > -- 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
On Thu, Nov 17, 2016 at 9:21 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 13 October 2016 at 16:58, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> When the pm_runtime_force_suspend|resume() helpers were invented, we still >> had CONFIG_PM_RUNTIME and CONFIG_PM_SLEEP as separate Kconfig options. >> >> To make sure these helpers worked for all combinations and without >> introducing too much of complexity, the device was always resumed in >> pm_runtime_force_resume(). >> >> More precisely, when CONFIG_PM_SLEEP was set and CONFIG_PM_RUNTIME was >> unset, we needed to resume the device as the subsystem/driver couldn't >> rely on using runtime PM to do it. >> >> As the CONFIG_PM_RUNTIME option was merged into CONFIG_PM a while ago, it >> removed this combination, of using CONFIG_PM_SLEEP without the earlier >> CONFIG_PM_RUNTIME. >> >> For this reason we can now rely on the subsystem/driver to use runtime PM >> to resume the device, instead of forcing that to be done in all cases. In >> other words, let's defer the runtime resume to a later point when it's >> actually needed. >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > > Rafael, this one is ready to be queued. Unless there are other objections. Not an objection, but I'd like to revisit this when the device dependencies work gets in, as something similar may need to be done for suppliers, in principle. Thanks, Rafael -- 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
Hi Ulf, On Thu, Nov 17, 2016 at 9:21 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 13 October 2016 at 16:58, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> When the pm_runtime_force_suspend|resume() helpers were invented, we still >> had CONFIG_PM_RUNTIME and CONFIG_PM_SLEEP as separate Kconfig options. >> >> To make sure these helpers worked for all combinations and without >> introducing too much of complexity, the device was always resumed in >> pm_runtime_force_resume(). >> >> More precisely, when CONFIG_PM_SLEEP was set and CONFIG_PM_RUNTIME was >> unset, we needed to resume the device as the subsystem/driver couldn't >> rely on using runtime PM to do it. >> >> As the CONFIG_PM_RUNTIME option was merged into CONFIG_PM a while ago, it >> removed this combination, of using CONFIG_PM_SLEEP without the earlier >> CONFIG_PM_RUNTIME. >> >> For this reason we can now rely on the subsystem/driver to use runtime PM >> to resume the device, instead of forcing that to be done in all cases. In >> other words, let's defer the runtime resume to a later point when it's >> actually needed. >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > > Rafael, this one is ready to be queued. Unless there are other objections. > > The problems that was reported by Geert for this change, has been > taken care of. Those are fixed by the patch for the smsc911x ethernet > driver [1], which you queued a while ago. > I realized that wasn't obvious unless one digested the conversations in detail. > [1] > https://www.spinics.net/lists/netdev/msg401339.html That's correct, thanks again for that fix! > Geert, perhaps you can confirm by providing a tested-by tag? Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- 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
On 17 November 2016 at 22:00, Rafael J. Wysocki <rafael@kernel.org> wrote: > On Thu, Nov 17, 2016 at 9:21 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> On 13 October 2016 at 16:58, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> When the pm_runtime_force_suspend|resume() helpers were invented, we still >>> had CONFIG_PM_RUNTIME and CONFIG_PM_SLEEP as separate Kconfig options. >>> >>> To make sure these helpers worked for all combinations and without >>> introducing too much of complexity, the device was always resumed in >>> pm_runtime_force_resume(). >>> >>> More precisely, when CONFIG_PM_SLEEP was set and CONFIG_PM_RUNTIME was >>> unset, we needed to resume the device as the subsystem/driver couldn't >>> rely on using runtime PM to do it. >>> >>> As the CONFIG_PM_RUNTIME option was merged into CONFIG_PM a while ago, it >>> removed this combination, of using CONFIG_PM_SLEEP without the earlier >>> CONFIG_PM_RUNTIME. >>> >>> For this reason we can now rely on the subsystem/driver to use runtime PM >>> to resume the device, instead of forcing that to be done in all cases. In >>> other words, let's defer the runtime resume to a later point when it's >>> actually needed. >>> >>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >> >> Rafael, this one is ready to be queued. Unless there are other objections. > > Not an objection, but I'd like to revisit this when the device > dependencies work gets in, as something similar may need to be done > for suppliers, in principle. I certainly think it would make sense to extend this approach to also respect device-dependencies. However, I suggest we deal with that as a second step, on top of $subject patch. It should not be a problem, as we don't have any users of device-dependencies that also uses pm_runtime_force_suspend|resume(), right? Queuing this now, also allow us to get this more widely tested, before we continue extending it for dependencies. Kind regards Uffe -- 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
Ulf Hansson <ulf.hansson@linaro.org> writes: > When the pm_runtime_force_suspend|resume() helpers were invented, we still > had CONFIG_PM_RUNTIME and CONFIG_PM_SLEEP as separate Kconfig options. > > To make sure these helpers worked for all combinations and without > introducing too much of complexity, the device was always resumed in > pm_runtime_force_resume(). > > More precisely, when CONFIG_PM_SLEEP was set and CONFIG_PM_RUNTIME was > unset, we needed to resume the device as the subsystem/driver couldn't > rely on using runtime PM to do it. > > As the CONFIG_PM_RUNTIME option was merged into CONFIG_PM a while ago, it > removed this combination, of using CONFIG_PM_SLEEP without the earlier > CONFIG_PM_RUNTIME. > > For this reason we can now rely on the subsystem/driver to use runtime PM > to resume the device, instead of forcing that to be done in all cases. In > other words, let's defer the runtime resume to a later point when it's > actually needed. > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> Acked-by: Kevin Hilman <khilman@baylibre.com> -- 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
On Fri, Nov 18, 2016 at 11:50 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 17 November 2016 at 22:00, Rafael J. Wysocki <rafael@kernel.org> wrote: >> On Thu, Nov 17, 2016 at 9:21 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> On 13 October 2016 at 16:58, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>> When the pm_runtime_force_suspend|resume() helpers were invented, we still >>>> had CONFIG_PM_RUNTIME and CONFIG_PM_SLEEP as separate Kconfig options. >>>> >>>> To make sure these helpers worked for all combinations and without >>>> introducing too much of complexity, the device was always resumed in >>>> pm_runtime_force_resume(). >>>> >>>> More precisely, when CONFIG_PM_SLEEP was set and CONFIG_PM_RUNTIME was >>>> unset, we needed to resume the device as the subsystem/driver couldn't >>>> rely on using runtime PM to do it. >>>> >>>> As the CONFIG_PM_RUNTIME option was merged into CONFIG_PM a while ago, it >>>> removed this combination, of using CONFIG_PM_SLEEP without the earlier >>>> CONFIG_PM_RUNTIME. >>>> >>>> For this reason we can now rely on the subsystem/driver to use runtime PM >>>> to resume the device, instead of forcing that to be done in all cases. In >>>> other words, let's defer the runtime resume to a later point when it's >>>> actually needed. >>>> >>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >>> >>> Rafael, this one is ready to be queued. Unless there are other objections. >> >> Not an objection, but I'd like to revisit this when the device >> dependencies work gets in, as something similar may need to be done >> for suppliers, in principle. > > I certainly think it would make sense to extend this approach to also > respect device-dependencies. > > However, I suggest we deal with that as a second step, on top of > $subject patch. It should not be a problem, as we don't have any users > of device-dependencies that also uses > pm_runtime_force_suspend|resume(), right? > > Queuing this now, also allow us to get this more widely tested, before > we continue extending it for dependencies. Fair enough. Thanks, Rafael -- 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
On Saturday, November 19, 2016 12:13:03 AM Rafael J. Wysocki wrote: > On Fri, Nov 18, 2016 at 11:50 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On 17 November 2016 at 22:00, Rafael J. Wysocki <rafael@kernel.org> wrote: > >> On Thu, Nov 17, 2016 at 9:21 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > >>> On 13 October 2016 at 16:58, Ulf Hansson <ulf.hansson@linaro.org> wrote: > >>>> When the pm_runtime_force_suspend|resume() helpers were invented, we still > >>>> had CONFIG_PM_RUNTIME and CONFIG_PM_SLEEP as separate Kconfig options. > >>>> > >>>> To make sure these helpers worked for all combinations and without > >>>> introducing too much of complexity, the device was always resumed in > >>>> pm_runtime_force_resume(). > >>>> > >>>> More precisely, when CONFIG_PM_SLEEP was set and CONFIG_PM_RUNTIME was > >>>> unset, we needed to resume the device as the subsystem/driver couldn't > >>>> rely on using runtime PM to do it. > >>>> > >>>> As the CONFIG_PM_RUNTIME option was merged into CONFIG_PM a while ago, it > >>>> removed this combination, of using CONFIG_PM_SLEEP without the earlier > >>>> CONFIG_PM_RUNTIME. > >>>> > >>>> For this reason we can now rely on the subsystem/driver to use runtime PM > >>>> to resume the device, instead of forcing that to be done in all cases. In > >>>> other words, let's defer the runtime resume to a later point when it's > >>>> actually needed. > >>>> > >>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > >>> > >>> Rafael, this one is ready to be queued. Unless there are other objections. > >> > >> Not an objection, but I'd like to revisit this when the device > >> dependencies work gets in, as something similar may need to be done > >> for suppliers, in principle. > > > > I certainly think it would make sense to extend this approach to also > > respect device-dependencies. > > > > However, I suggest we deal with that as a second step, on top of > > $subject patch. It should not be a problem, as we don't have any users > > of device-dependencies that also uses > > pm_runtime_force_suspend|resume(), right? > > > > Queuing this now, also allow us to get this more widely tested, before > > we continue extending it for dependencies. > > Fair enough. The patch has been queued up for 4.10. Thanks, Rafael -- 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
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index e097d35..f662267 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1478,6 +1478,16 @@ int pm_runtime_force_suspend(struct device *dev) if (ret) goto err; + /* + * Increase the runtime PM usage count for the device's parent, in case + * when we find the device being used when system suspend was invoked. + * This informs pm_runtime_force_resume() to resume the parent + * immediately, which is needed to be able to resume its children, + * when not deferring the resume to be managed via runtime PM. + */ + if (dev->parent && atomic_read(&dev->power.usage_count) > 1) + pm_runtime_get_noresume(dev->parent); + pm_runtime_set_suspended(dev); return 0; err: @@ -1487,16 +1497,20 @@ err: EXPORT_SYMBOL_GPL(pm_runtime_force_suspend); /** - * pm_runtime_force_resume - Force a device into resume state. + * pm_runtime_force_resume - Force a device into resume state if needed. * @dev: Device to resume. * * Prior invoking this function we expect the user to have brought the device * into low power state by a call to pm_runtime_force_suspend(). Here we reverse - * those actions and brings the device into full power. We update the runtime PM - * status and re-enables runtime PM. + * those actions and brings the device into full power, if it is expected to be + * used on system resume. To distinguish that, we check whether the runtime PM + * usage count is greater than 1 (the PM core increases the usage count in the + * system PM prepare phase), as that indicates a real user (such as a subsystem, + * driver, userspace, etc.) is using it. If that is the case, the device is + * expected to be used on system resume as well, so then we resume it. In the + * other case, we defer the resume to be managed via runtime PM. * - * Typically this function may be invoked from a system resume callback to make - * sure the device is put into full power state. + * Typically this function may be invoked from a system resume callback. */ int pm_runtime_force_resume(struct device *dev) { @@ -1513,6 +1527,17 @@ int pm_runtime_force_resume(struct device *dev) if (!pm_runtime_status_suspended(dev)) goto out; + /* + * Decrease the parent's runtime PM usage count, if we increased it + * during system suspend in pm_runtime_force_suspend(). + */ + if (atomic_read(&dev->power.usage_count) > 1) { + if (dev->parent) + pm_runtime_put_noidle(dev->parent); + } else { + goto out; + } + ret = pm_runtime_set_active(dev); if (ret) goto out;
When the pm_runtime_force_suspend|resume() helpers were invented, we still had CONFIG_PM_RUNTIME and CONFIG_PM_SLEEP as separate Kconfig options. To make sure these helpers worked for all combinations and without introducing too much of complexity, the device was always resumed in pm_runtime_force_resume(). More precisely, when CONFIG_PM_SLEEP was set and CONFIG_PM_RUNTIME was unset, we needed to resume the device as the subsystem/driver couldn't rely on using runtime PM to do it. As the CONFIG_PM_RUNTIME option was merged into CONFIG_PM a while ago, it removed this combination, of using CONFIG_PM_SLEEP without the earlier CONFIG_PM_RUNTIME. For this reason we can now rely on the subsystem/driver to use runtime PM to resume the device, instead of forcing that to be done in all cases. In other words, let's defer the runtime resume to a later point when it's actually needed. Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- Changes in v3: - Updated to take care of parent-child relations. - Improved comment in the code and updated text in a function header to better describe the changes. This patch has earlier been sent standalone, but also as a part of series. In the end it turned out the solution needed some improvement to take care of parent-child relations, as reported by Geert [1]. Geert, I would really appreciate if you could help out testing to make sure the reported issue is fixed. [1] https://patches.linaro.org/patch/67940/ --- drivers/base/power/runtime.c | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) -- 1.9.1