* Rafael J. Wysocki [off-list ref] [160201 15:11]:

On Mon, Feb 1, 2016 at 11:29 PM, Tony Lindgren [off-list ref] wrote:

quoted

* Rafael J. Wysocki [off-list ref] [160201 14:18]:

quoted

On Mon, Feb 1, 2016 at 11:06 PM, Tony Lindgren [off-list ref] wrote:

quoted

--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c

@@ -1419,17 +1419,25 @@ void pm_runtime_init(struct device *dev)
  */
 void pm_runtime_reinit(struct device *dev)
 {
-       if (!pm_runtime_enabled(dev)) {
-               if (dev->power.runtime_status == RPM_ACTIVE)
+       if (pm_runtime_enabled(dev))
+               return;
+
+       if (dev->power.runtime_status == RPM_ACTIVE) {
+               if (dev->power.use_autosuspend) {
+                       __pm_runtime_use_autosuspend(dev, false);
+                       pm_runtime_suspend(dev);

This won't work, because runtime PM is disabled at this point.

Hmm right OK. It does work from idling the hardware point
of view though..

pm_runtime_suspend() with runtime PM disabled is a NOP.  It will do
nothing and return -EACCES.

Hmm it  makes a difference here for sure :)

quoted

quoted

What about doing this instead:

               if (dev->power.use_autosuspend)
                       __pm_runtime_use_autosuspend(dev, false);

               pm_runtime_set_suspended(dev);

..while this does not work. The hardware is never idled
in this case.

I'm not sure what you mean.  pm_runtime_set_suspended() sets the
status to RPM_SUSPENDED for devices with runtime PM disabled.  It has
nothing to do with "idling" in principle.

Well looking at the update_autosuspend(), it seems we're now missing
rpm_idle() call that now never happens.

Does the patch below make more sense to you where we call rpm_idle?
That seems to make things behave here also.

quoted

What else does __pm_runtime_use_autosuspend() set initially
that changes things here?

The usage counter, if the delay is negative.

Yeah I don't see any difference with those.

I'll look at this in detail, but not right now, sorry.  I'm working on
something else ATM and I was hoping that Ulf would be able to figure
out what's going on here.

Yeah we need to understand what's going on here. Having the PM runtime
framework out of sync with the hardare is not good.. If we can't
figure this out we should probably revert the patch until we understand
it.

Regards,

Tony

8< ------------

--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c

@@ -1419,17 +1419,28 @@ void pm_runtime_init(struct device *dev)
  */
 void pm_runtime_reinit(struct device *dev)
 {
-	if (!pm_runtime_enabled(dev)) {
-		if (dev->power.runtime_status == RPM_ACTIVE)
+	int (*callback)(struct device *);
+	int err;
+
+	if (pm_runtime_enabled(dev))
+		return;
+
+	if (dev->power.runtime_status == RPM_ACTIVE) {
+		if (dev->power.use_autosuspend) {
+			__pm_runtime_use_autosuspend(dev, false);
+			rpm_idle(dev, RPM_AUTO);
+		} else {
 			pm_runtime_set_suspended(dev);
-		if (dev->power.irq_safe) {
-			spin_lock_irq(&dev->power.lock);
-			dev->power.irq_safe = 0;
-			spin_unlock_irq(&dev->power.lock);
-			if (dev->parent)
-				pm_runtime_put(dev->parent);
 		}
 	}
+
+	if (dev->power.irq_safe) {
+		spin_lock_irq(&dev->power.lock);
+		dev->power.irq_safe = 0;
+		spin_unlock_irq(&dev->power.lock);
+		if (dev->parent)
+			pm_runtime_put(dev->parent);
+	}
 }
 
 /**