[PATCH 4/5] PM: runtime: Synchronous suspend when updating autosuspend

[Brian Norris]

When updating autosuspend settings, we request synchronous autosuspend.
This is sometimes a best-effort action, because we don't know if there
are open reference counts for the device. In other cases, we expect this
to be reliable -- specifically, on the teardown side of
devm_pm_runtime_enable(), which calls pm_runtime_dont_use_autosuspend()
to (among other reasons) ensure the device will immediately suspend if
possible. (See commit b4060db9251f ("PM: runtime: Have
devm_pm_runtime_enable() handle pm_runtime_dont_use_autosuspend()").)

However, synchronous rpm_idle() does not flush pending suspend requests
-- it simply returns -EAGAIN. This is noticeable as a small race
condition in the following sort of scenario:

	/* Last user drops its references, and queues RPM_REQ_AUTOSUSPEND */
	pm_runtime_put_autosuspend(dev);

	/*
	 * Next, driver is unbound:
	 * --> driver remove()
	 *  --> devres teardown
	 *   --> pm_runtime_disable_action()
	 */
	 pm_runtime_dont_use_autosuspend(dev);
	/*
	 * --> rpm_idle(RPM_AUTO) may return -EAGAIN, and the SUSPEND
	 * request may still be pending...
	 */

	// Indeterminate device status: may be left SUSPENDED or ACTIVE,
	// depending on a race condition.
	pm_runtime_disable(dev);

If we use synchronous rpm_suspend() instead, we will properly wait for
an outstanding RPM_REQ_AUTOSUSPEND to quiesce (or else, perform our
own).

This is a safe change, because:

 * drivers that use autosuspend shouldn't care about .runtime_idle()
   (the difference between rpm_idle() and rpm_suspend()) [1]

 * drivers that don't use autosuspend will not reach this code, except
   on the teardown path of devm_pm_runtime_enable()

[1] This isn't actually mentioned in any documentation, but I infer this
    because its dedicated helpers (e.g., pm_runtime_put_autosuspend())
    skip any idle checks. And surveying existing drivers shows that
    those that implement .runtime_idle() behave similarly in their
    .runtime_suspend().

Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx>
---
See also a regression test in the next patch.

This resolves one racy pitfall when using devm_pm_runtime_enable(), but
it doesn't resolve another pitfall: what if user space forbade runtime
PM (`echo on > /sys/devices/.../power/control`)? In that case, the
device will still remain RPM_ACTIVE on teardown.

Surveying many devm_pm_runtime_enable() users, it's common to fall into
these pitfalls. Commit 2d90ecdfa326 ("ASoC: rockchip: i2s: Use managed
hclk and runtime PM cleanup") shows a rare example that got all the
details right.

I may propose other changes to devm_pm_runtime_enable() later to try to
help those, but I expect them to be a bit more controversial.

 drivers/base/power/runtime.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 2a99b7509744..b0262f187041 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1788,7 +1788,8 @@ EXPORT_SYMBOL_GPL(pm_runtime_irq_safe);
  * @old_use: The former use_autosuspend value.
  *
  * Prevent runtime suspend if the new delay is negative and use_autosuspend is
- * set; otherwise allow it.  Send an idle notification if suspends are allowed.
+ * set; otherwise allow it.  Attempt to suspend the device if suspends are
+ * allowed.
  *
  * This function must be called under dev->power.lock with interrupts disabled.
  */
@@ -1816,7 +1817,7 @@ static void update_autosuspend(struct device *dev, int old_delay, int old_use)
 			atomic_dec(&dev->power.usage_count);
 
 		/* Maybe we can autosuspend now. */
-		rpm_idle(dev, RPM_AUTO);
+		rpm_suspend(dev, RPM_AUTO);
 	}
 }
 
-- 
2.56.0.rc1.315.gc6ed9934b7-goog