• R
    PM / genpd: Stop/start devices without pm_runtime_force_suspend/resume() · 17218e00
    Rafael J. Wysocki 提交于
    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: NRafael J. Wysocki <rafael.j.wysocki@intel.com>
    Acked-by: NUlf Hansson <ulf.hansson@linaro.org>
    17218e00
domain.c 66.6 KB