Skip to contentSkip to conversation

PM: runtime: Capture device status before disabling runtime PM

PATCH New Rafael J. Wysocki 1 participant 1 message Dec 7, 2021

Subsystems (4)
Driver core Suspend to RAM Hibernation Power management core
lore
Share & tools
lore

Rafael J. Wysocki

author
#0 lore raw

PATCH v2 1/1 PM: runtime: Capture device status before disabling runtime PM

From: Rafael J. Wysocki <[email protected]>

In some cases (for example, during system-wide suspend and resume of devices) it is useful to know whether or not runtime PM has ever been enabled for a given device and, if so, what the runtime PM status of it had been right before runtime PM was disabled for it last time.

For this reason, introduce a new struct dev_pm_info field called last_status that will be used for capturing the runtime PM status of the device when its power.disable_depth counter changes from 0 to 1.

The new field will be set to RPM_INVALID to start with and whenever power.disable_depth changes from 1 to 0, so it will be valid only when runtime PM of the device is currently disabled, but it has been enabled at least once.

Immediately use power.last_status in rpm_resume() to make it handle the case when PM runtime is disabled for the device, but its runtime PM status is RPM_ACTIVE more consistently. Namely, make it return 1 if power.last_status is also equal to RPM_ACTIVE in that case (the idea being that if the status was RPM_ACTIVE last time when power.disable_depth was changing from 0 to 1 and it is still RPM_ACTIVE, it can be assumed to reflect what happened to the device last time when it was using runtime PM) and -EACCES otherwise.

Update the documentation to provide a description of last_status and change the description of pm_runtime_resume() in it to reflect the new behavior of rpm_active().

While at it, rearrange the code in pm_runtime_enable() to be more straightforward and replace the WARN() macro in it with a pr_warn() invocation which is less disruptive.

Link https://lore.kernel.org/linux-pm/[email protected]/t/#u open here
Reviewed-by Ulf Hansson <[email protected]>
Signed-off-by Rafael J. Wysocki <[email protected]>

---

-> v2: Avoid using the ternary operator in rpm_resume() (Alan).

---
 Documentation/power/runtime_pm.rst |   14 ++++++++---
 drivers/base/power/runtime.c       |   45 ++++++++++++++++++++-----------------
 include/linux/pm.h                 |    2 +
 3 files changed, 37 insertions(+), 24 deletions(-)
drivers/base/power/runtime.c +25 -20
Index: linux-pm/drivers/base/power/runtime.c
===================================================================
@@ -742,13 +742,15 @@ static int rpm_resume(struct device *dev
 	trace_rpm_resume_rcuidle(dev, rpmflags);
 
  repeat:
-	if (dev->power.runtime_error)
+	if (dev->power.runtime_error) {
 		retval = -EINVAL;
-	else if (dev->power.disable_depth == 1 && dev->power.is_suspended
-	    && dev->power.runtime_status == RPM_ACTIVE)
-		retval = 1;
-	else if (dev->power.disable_depth > 0)
-		retval = -EACCES;
+	} else if (dev->power.disable_depth > 0) {
+		if (dev->power.runtime_status == RPM_ACTIVE &&
+		    dev->power.last_status == RPM_ACTIVE)
+			retval = 1;
+		else
+			retval = -EACCES;
+	}
 	if (retval)
 		goto out;
 

@@ -1410,8 +1412,10 @@ void __pm_runtime_disable(struct device
 	/* Update time accounting before disabling PM-runtime. */
 	update_pm_runtime_accounting(dev);
 
-	if (!dev->power.disable_depth++)
+	if (!dev->power.disable_depth++) {
 		__pm_runtime_barrier(dev);
+		dev->power.last_status = dev->power.runtime_status;
+	}
 
  out:
 	spin_unlock_irq(&dev->power.lock);

@@ -1428,23 +1432,23 @@ void pm_runtime_enable(struct device *de
 
 	spin_lock_irqsave(&dev->power.lock, flags);
 
-	if (dev->power.disable_depth > 0) {
-		dev->power.disable_depth--;
-
-		/* About to enable runtime pm, set accounting_timestamp to now */
-		if (!dev->power.disable_depth)
-			dev->power.accounting_timestamp = ktime_get_mono_fast_ns();
-	} else {
+	if (!dev->power.disable_depth) {
 		dev_warn(dev, "Unbalanced %s!\n", __func__);
+		goto out;
 	}
 
-	WARN(!dev->power.disable_depth &&
-	     dev->power.runtime_status == RPM_SUSPENDED &&
-	     !dev->power.ignore_children &&
-	     atomic_read(&dev->power.child_count) > 0,
-	     "Enabling runtime PM for inactive device (%s) with active children\n",
-	     dev_name(dev));
+	if (--dev->power.disable_depth > 0)
+		goto out;
+
+	dev->power.last_status = RPM_INVALID;
+	dev->power.accounting_timestamp = ktime_get_mono_fast_ns();
+
+	if (dev->power.runtime_status == RPM_SUSPENDED &&
+	    !dev->power.ignore_children &&
+	    atomic_read(&dev->power.child_count) > 0)
+		dev_warn(dev, "Enabling runtime PM for inactive device with active children\n");
 
+out:
 	spin_unlock_irqrestore(&dev->power.lock, flags);
 }
 EXPORT_SYMBOL_GPL(pm_runtime_enable);

@@ -1640,6 +1644,7 @@ EXPORT_SYMBOL_GPL(__pm_runtime_use_autos
 void pm_runtime_init(struct device *dev)
 {
 	dev->power.runtime_status = RPM_SUSPENDED;
+	dev->power.last_status = RPM_INVALID;
 	dev->power.idle_notification = false;
 
 	dev->power.disable_depth = 1;
include/linux/pm.h +2 -0
Index: linux-pm/include/linux/pm.h
===================================================================
@@ -499,6 +499,7 @@ const struct dev_pm_ops __maybe_unused n
  */
 
 enum rpm_status {
+	RPM_INVALID = -1,
 	RPM_ACTIVE = 0,
 	RPM_RESUMING,
 	RPM_SUSPENDED,

@@ -612,6 +613,7 @@ struct dev_pm_info {
 	unsigned int		links_count;
 	enum rpm_request	request;
 	enum rpm_status		runtime_status;
+	enum rpm_status		last_status;
 	int			runtime_error;
 	int			autosuspend_delay;
 	u64			last_busy;
Documentation/power/runtime_pm.rst +10 -4
Index: linux-pm/Documentation/power/runtime_pm.rst
===================================================================
@@ -265,6 +265,10 @@ defined in include/linux/pm.h:
       RPM_SUSPENDED, which means that each device is initially regarded by the
       PM core as 'suspended', regardless of its real hardware status
 
+  `enum rpm_status last_status;`
+    - the last runtime PM status of the device captured before disabling runtime
+      PM for it (invalid initially and when disable_depth is 0)
+
   `unsigned int runtime_auto;`
     - if set, indicates that the user space has allowed the device driver to
       power manage the device at run time via the /sys/devices/.../power/control

@@ -333,10 +337,12 @@ drivers/base/power/runtime.c and include
 
   `int pm_runtime_resume(struct device *dev);`
     - execute the subsystem-level resume callback for the device; returns 0 on
-      success, 1 if the device's runtime PM status was already 'active' or
-      error code on failure, where -EAGAIN means it may be safe to attempt to
-      resume the device again in future, but 'power.runtime_error' should be
-      checked additionally, and -EACCES means that 'power.disable_depth' is
+      success, 1 if the device's runtime PM status is already 'active' (also if
+      'power.disable_depth' is nonzero, but the status was 'active' when it was
+      changing from 0 to 1) or error code on failure, where -EAGAIN means it may
+      be safe to attempt to resume the device again in future, but
+      'power.runtime_error' should be checked additionally, and -EACCES means
+      that the callback could not be run, because 'power.disable_depth' was
       different from 0
 
   `int pm_runtime_resume_and_get(struct device *dev);`