PM / Runtime: rpm_suspend() and rpm_resume() fixes

For devices whose power.no_callbacks flag is set, rpm_resume() should return 1 if the device's parent is already active, so that the callers of pm_runtime_get() don't think that they have to wait for the device to resume (asynchronously) in that case (the core won't queue up an asynchronous resume in that case, so there's nothing to wait for anyway).

Modify the code accordingly (and make sure that an idle notification will be queued up on success, even if 1 is to be returned).

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

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

Index: linux/drivers/base/power/runtime.c
===================================================================

drivers/base/power/runtime.c +1 -0

@@ -584,6 +584,7 @@ static int rpm_resume(struct device *dev
 		    || dev->parent->power.runtime_status == RPM_ACTIVE) {
 			atomic_inc(&dev->parent->power.child_count);
 			spin_unlock(&dev->parent->power.lock);
+			retval = 1;
 			goto no_callback;	/* Assume success. */
 		}
 		spin_unlock(&dev->parent->power.lock);

The power.deferred_resume can only be set if the runtime PM status of device is RPM_SUSPENDING and it should be cleared after its status has been changed, regardless of whether or not the runtime suspend has been successful. However, it only is cleared on suspend failure, while it may remain set after successful suspend and is happily leaked to rpm_resume() executed in that case.

That shouldn't happen, so if power.deferred_resume is set in rpm_suspend() after the status has been changed to RPM_SUSPENDED, clear it before calling rpm_resume(). Then, it doesn't need to be cleared before changing the status to RPM_SUSPENDING any more, because it's always cleared after the status has been changed to either RPM_SUSPENDED (on success) or RPM_ACTIVE (on failure).

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

---
 drivers/base/power/runtime.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/base/power/runtime.c
===================================================================

drivers/base/power/runtime.c +0 -1

@@ -388,7 +388,6 @@ static int rpm_suspend(struct device *de
 		goto repeat;
 	}
 
-	dev->power.deferred_resume = false;
 	if (dev->power.no_callbacks)
 		goto no_callback;	/* Assume success. */
 

If __dev_pm_qos_read_value(dev) returns a negative value, rpm_suspend() should return -EPERM for dev even if its power.no_callbacks flag is set. For this to happen, the device's power.no_callbacks flag has to be checked after the PM QoS check, so modify the code accordingly.

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

---
 drivers/base/power/runtime.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Index: linux/drivers/base/power/runtime.c
===================================================================

drivers/base/power/runtime.c +6 -0

@@ -388,6 +388,12 @@ static int rpm_suspend(struct device *de
 		goto repeat;
 	}
 
+	if (__dev_pm_qos_read_value(dev) < 0) {
+		/* Negative PM QoS constraint means "never suspend". */
+		retval = -EPERM;
+		goto out;
+	}
+
 	if (dev->power.no_callbacks)
 		goto no_callback;	/* Assume success. */
 

@@ -402,12 +408,6 @@ static int rpm_suspend(struct device *de
 		goto out;
 	}

Quoted 41 lines · Rafael J. Wysocki

If __dev_pm_qos_read_value(dev) returns a negative value, rpm_suspend() should return -EPERM for dev even if its power.no_callbacks flag is set. For this to happen, the device's power.no_callbacks flag has to be checked after the PM QoS check, so modify the code accordingly.

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

---
 drivers/base/power/runtime.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Index: linux/drivers/base/power/runtime.c
===================================================================

drivers/base/power/runtime.c +6 -6

@@ -388,6 +388,12 @@ static int rpm_suspend(struct device *de
 		goto repeat;
 	}
 
+	if (__dev_pm_qos_read_value(dev) < 0) {
+		/* Negative PM QoS constraint means "never suspend". */
+		retval = -EPERM;
+		goto out;
+	}
+
 	if (dev->power.no_callbacks)
 		goto no_callback;	/* Assume success. */
 

@@ -402,12 +408,6 @@ static int rpm_suspend(struct device *de
 		goto out;
 	}
 
-	if (__dev_pm_qos_read_value(dev) < 0) {
-		/* Negative PM QoS constraint means "never suspend". */
-		retval = -EPERM;
-		goto out;
-	}
-
 	__update_runtime_status(dev, RPM_SUSPENDING);
 

drivers/base/power/runtime.c static int rpm_suspend(struct device *de · line 413

Quoted 1 line

drivers/base/power/runtime.c +0 -0

 	if (dev->pm_domain)

Wouldn't it be better to move the test into rpm_check_suspend_allowed()? Then it would apply to idle notifications and pm_schedule_suspend().

From: Rafael J. Wysocki <[email protected]>
Subject: PM / Runtime: Check device PM QoS setting before "no callbacks"

If __dev_pm_qos_read_value(dev) returns a negative value, rpm_suspend() should return -EPERM for dev even if its power.no_callbacks flag is set. For this to happen, the device's power.no_callbacks flag has to be checked after the PM QoS check, so move the PM QoS check to rpm_check_suspend_allowed() (this will make it cover idle notifications as well as runtime suspend too).

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

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

Index: linux/drivers/base/power/runtime.c
===================================================================

drivers/base/power/runtime.c +2 -0

@@ -147,6 +147,8 @@ static int rpm_check_suspend_allowed(str
 	    || (dev->power.request_pending
 			&& dev->power.request == RPM_REQ_RESUME))
 		retval = -EAGAIN;
+	else if (__dev_pm_qos_read_value(dev) < 0)
+		retval = -EPERM;
 	else if (dev->power.runtime_status == RPM_SUSPENDED)
 		retval = 1;
 

@@ -402,12 +404,6 @@ static int rpm_suspend(struct device *de
 		goto out;
 	}