Define positive return value to RPM_SUSPEND for runtime-suspended devices

Currently if the ->prepare() callback of a device returns a positive number, the PM core will regard that as an indication that it may leave the device runtime-suspended. However it would be more convenient to define this positive number then makes it more maintainable. Consider there might be already some device drivers using different positive values, this patch prints a warning if the positive value is other than RPM_SUSPENDED, and hoping driver developers would adjust their return values to RPM_SUSPENDED, then at last we can modify the code to only enable this feature for values return of RPM_SUSPENDED rather than arbitrary positive value.

Suggested-by Lee Jones <[email protected]>

Suggested-by Rafael J. Wysocki <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 Documentation/power/devices.txt    | 8 ++++----
 Documentation/power/runtime_pm.txt | 2 +-
 drivers/base/power/main.c          | 8 ++++++--
 3 files changed, 11 insertions(+), 7 deletions(-)

Documentation/power/devices.txt +4 -4

index 8ba6625..fc585a5 100644
@@ -331,8 +331,8 @@ the phases are:
        prepare callback can be used to indicate to the PM core that it may
        safely leave the device in runtime suspend (if runtime-suspended
        already), provided that all of the device's descendants are also left in
-       runtime suspend.  Namely, if the prepare callback returns a positive
-       number and that happens for all of the descendants of the device too,
+       runtime suspend.  Namely, if the prepare callback returns RPM_SUSPENDED
+       and that happens for all of the descendants of the device too,
        and all of them (including the device itself) are runtime-suspended, the
        PM core will skip the suspend, suspend_late and suspend_noirq suspend
        phases as well as the resume_noirq, resume_early and resume phases of

@@ -344,7 +344,7 @@ the phases are:
        Note that this direct-complete procedure applies even if the device is
        disabled for runtime PM; only the runtime-PM status matters.  It follows
        that if a device has system-sleep callbacks but does not support runtime
-       PM, then its prepare callback must never return a positive value.  This
+       PM, then its prepare callback must never return RPM_SUSPENDED.  This
        is because all devices are initially set to runtime-suspended with
        runtime PM disabled.
 

@@ -422,7 +422,7 @@ When resuming from freeze, standby or memory sleep, the phases are:
        the resume callbacks occur; it's not necessary to wait until the
        complete phase.
 
-       Moreover, if the preceding prepare callback returned a positive number,
+       Moreover, if the preceding prepare callback returned RPM_SUSPENDED,
        the device may have been left in runtime suspend throughout the whole
        system suspend and resume (the suspend, suspend_late, suspend_noirq
        phases of system suspend and the resume_noirq, resume_early, resume

Documentation/power/runtime_pm.txt +1 -1

index 1fd1fbe..5316daf 100644
@@ -667,7 +667,7 @@ suspend began in the suspended state.
 
 To this end, the PM core provides a mechanism allowing some coordination between
 different levels of device hierarchy.  Namely, if a system suspend .prepare()
-callback returns a positive number for a device, that indicates to the PM core
+callback returns RPM_SUSPENDED for a device, that indicates to the PM core
 that the device appears to be runtime-suspended and its state is fine, so it
 may be left in runtime suspend provided that all of its descendants are also
 left in runtime suspend.  If that happens, the PM core will not execute any

drivers/base/power/main.c +1 -1

index e44944f..03a047e 100644
@@ -1603,14 +1603,18 @@ unlock:
                return ret;
        }
        /*
-        * A positive return value from ->prepare() means "this device appears
+        * A return value of RPM_SUSPENDED from ->prepare() means "this device appears
         * to be runtime-suspended and its state is fine, so if it really is
         * runtime-suspended, you can leave it in that state provided that you
         * will do the same thing with all of its descendants".  This only
         * applies to suspend transitions, however.
         */

Quoted 10 lines · Chen Yu

Currently if the ->prepare() callback of a device returns a positive number, the PM core will regard that as an indication that it may leave the device runtime-suspended. However it would be more convenient to define this positive number then makes it more maintainable. Consider there might be already some device drivers using different positive values, this patch prints a warning if the positive value is other than RPM_SUSPENDED, and hoping driver developers would adjust their return values to RPM_SUSPENDED, then at last we can modify the code to only enable this feature for values return of RPM_SUSPENDED rather than arbitrary positive value.

Quoted 5 lines

Suggested-by Lee Jones <[email protected]>

Suggested-by Rafael J. Wysocki <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
}

No.

(1) RPM_SUSPENDED has a specific meaning to the runtime PM framework, so please don't overload it.

(2) Define a new symbol (e.g. DPM_DIRECT_COMPLETE), but allow drivers to return positive numbers different from that.

That may be useful if someone wants to do "return a > b" or "return count", where "direct complete" is to be used for all values of count different from 0.

The issue here is that "1" in your previous patch looked arbitrary, so you're addressing this by defining a symbol to use instead.

And BTW, there are places that return "1" already from their ->prepare(), so this patch would have generated a bunch of false-positives.

Thanks,
Rafael

So first try is to use pm_request_resume() instead, to make the runtime
resume process asynchronously. Unfortunately the asynchronous runtime resume relies on pm_wq, which is freezed at early stage. So we choose another method, that is to avoid resuming runtime-suspended devices, if they are already runtime suspended. This is safe because for LPSS driver, the runtime suspend and system suspend are of the same hook - i.e., intel_lpss_suspend(). And moreover, this device is neither runtime wakeup source nor system wakeup source.

Quoted 9 lines · Chen Yu

So first try is to use pm_request_resume() instead, to make the runtime
resume process asynchronously. Unfortunately the asynchronous runtime resume relies on pm_wq, which is freezed at early stage. So we choose another method, that is to avoid resuming runtime-suspended devices, if they are already runtime suspended. This is safe because for LPSS driver, the runtime suspend and system suspend are of the same hook - i.e., intel_lpss_suspend(). And moreover, this device is neither runtime wakeup source nor system wakeup source.

I agree with the reasoning but I don't see the specificity to LPSS. Shouldn't this go into the core?

Thanks for your reply. Do you mean we should add the logic to pm core? There is already one if the driver's
.prepare() returns a positive number(aka, RPM_SUSPENDED), then pm core will keep