PM / genpd: Stop/start devices without pm_runtime_force_suspend/resume()
There are problems with calling pm_runtime_force_suspend/resume() to "stop" and "start" devices in genpd_finish_suspend() and genpd_resume_noirq() (and in analogous hibernation-specific genpd callbacks) after commit 122a2237 (PM / Domains: Stop/start devices during system PM suspend/resume in genpd) as those routines do much more than just "stopping" and "starting" devices (which was the stated purpose of that commit) unnecessarily and may not play well with system-wide PM driver callbacks. First, consider the pm_runtime_force_suspend() in genpd_finish_suspend(). If the current runtime PM status of the device is "suspended", that function most likely does the right thing by ignoring the device, because it should have been "stopped" already and whatever needed to be done to deactivate it shoud have been done. In turn, if the runtime PM status of the device is "active", genpd_runtime_suspend() is called for it (indirectly) and (1) runs the ->runtime_suspend callback provided by the device's driver (assuming no bus type with ->runtime_suspend of its own), (2) "stops" the device and (3) checks if the domain can be powered down, and then (4) the device's runtime PM status is changed to "suspended". Out of the four actions above (1) is not necessary and it may be outright harmful, (3) is pointless and (4) is questionable. The only operation that needs to be carried out here is (2). The reason why (1) is not necessary is because the system-wide PM callbacks provided by the device driver for the transition in question have been run and they should have taken care of the driver's part of device suspend already. Moreover, it may be harmful, because the ->runtime_suspend callback may want to access the device which is partially suspended at that point and may not be responsive. Also, system-wide PM callbacks may have been run already (in the previous phases of the system transition under way) for the device's parent or for its supplier devices (if any) and the device may not be accessible because of that. There also is no reason to do (3), because genpd_finish_suspend() will repeat it anyway, and (4) potentially causes confusion to ensue during the subsequent system transition to the working state. Consider pm_runtime_force_resume() in genpd_resume_noirq() now. It runs genpd_runtime_resume() for all devices with runtime PM status set to "suspended", which includes all of the devices whose runtime PM status was changed by pm_runtime_force_suspend() before and may include some devices already suspended when the pm_runtime_force_suspend() was running, which may be confusing. The genpd_runtime_resume() first tries to power up the domain, which (again) is pointless, because genpd_resume_noirq() has done that already. Then, it "starts" the device and runs the ->runtime_resume callback (from the driver, say) for it. If all is well, the device is left with the runtime PM status set to "active". Unfortunately, running the driver's ->runtime_resume callback before its system-wide PM callbacks and possibly before some system-wide PM callbacks of the parent device's driver (let alone supplier drivers) is asking for trouble, especially if the device had been suspended before pm_runtime_force_suspend() ran previously or if the callbacks in question expect to be run back-to-back with their suspend-side counterparts. It also should not be necessary, because the system-wide PM driver callbacks that will be invoked for the device subsequently should take care of resuming it just fine. [Running the driver's ->runtime_resume callback in the "noirq" phase of the transition to the working state may be problematic even for devices whose drivers do use pm_runtime_force_resume() in (or as) their system-wide PM callbacks if they have suppliers other than their parents, because it may cause the supplier to be resumed after the consumer in some cases.] Because of the above, modify genpd as follows: 1. Change genpd_finish_suspend() to only "stop" devices with runtime PM status set to "active" (without invoking runtime PM callbacks for them, changing their runtime PM status and so on). That doesn't change the handling of devices whose drivers use pm_runtime_force_suspend/resume() in (or as) their system-wide PM callbacks and addresses the issues described above for the other devices. 2. Change genpd_resume_noirq() to only "start" devices with runtime PM status set to "active" (without invoking runtime PM callbacks for them, changing their runtime PM status and so on). Again, that doesn't change the handling of devices whose drivers use pm_runtime_force_suspend/resume() in (or as) their system-wide PM callbacks and addresses the described issues for the other devices. Devices with runtime PM status set to "suspended" are not started with the assumption that they will be resumed later, either by pm_runtime_force_resume() or via runtime PM. 3. Change genpd_restore_noirq() to follow genpd_resume_noirq(). That causes devices already suspended before hibernation to be left alone (which also is the case without the change) and avoids running the ->runtime_resume driver callback too early for the other devices. 4. Change genpd_freeze_noirq() and genpd_thaw_noirq() in accordance with the above modifications. Fixes: 122a2237 (PM / Domains: Stop/start devices during system PM suspend/resume in genpd) Signed-off-by:Rafael J. Wysocki <rafael.j.wysocki@intel.com> Acked-by:
Ulf Hansson <ulf.hansson@linaro.org>
Showing
Please register or sign in to comment