PM regression with commit 5de85b9d57ab PM runtime re-init in v4.5-rc1

[Alan Stern]

On Tue, 2 Feb 2016, Tony Lindgren wrote:

> * Alan Stern <stern at rowland.harvard.edu> [160202 13:46]:
> > On Tue, 2 Feb 2016, Tony Lindgren wrote:
> > 
> > > > Also, what is autosuspend_delay set to for your device?  And is 
> > > > runtime_auto set?
> > > 
> > > It's 100 at that point, see the commented snippet below from
> > > omap_hsmmc_probe():
> > > 
> > > 	pm_runtime_enable(host->dev);
> > > 	pm_runtime_get_sync(host->dev);
> > > 	pm_runtime_set_autosuspend_delay(host->dev, MMC_AUTOSUSPEND_DELAY);
> > > 	/* NOTE: pm_runtime_dont_use_autosuspend(host->dev) needed here? */
> > > 	pm_runtime_use_autosuspend(host->dev);
> > > 	...
> > > 	/* gets -EPROBE_DEFER */
> > > err_irq:
> > > 	...
> > > 	pm_runtime_put_sync(host->dev);
> > 
> > You could try changing this to pm_runtime_put_sync_suspend().  But
> > putting pm_runtime_dont_use_autosuspend() before the put_sync seems
> > like a perfectly reasonable thing to do, especially if you feel you
> > should reverse all the changes you made at the start.
> 
> They both seem to fix the problem.

So you could use either one.  In my opinion, the 
pm_runtime_dont_use_autosuspend() solution is a little cleaner.

> > The reinit function gets called too late to do what you want -- namely, 
> > put the hardware in a low-power state.
> 
> Right, the problem is the lack of suspend on the first probe. But
> for having autosuspend timeout long enough for the next probe
> would mean that we can't reset the PM runtime state in between.

That's one way to look at it.  But in principle you don't need to
suspend the device after an unsuccessful probe.  You can just leave it
at high power.  If this causes problems for a second probe, it's the 
second probe's own fault for assuming the actual state matches the PM 
status.

> > pm_runtime_put_sync() is supposed to follow the driver's wishes.  It
> > invokes the driver's runtime_idle callback if there is one, and the
> > callback routine can start a suspend or an autosuspend.  If there is no
> > callback, it will use whatever autosuspend setting the driver has set
> > up.  If you want to override the autosuspend setting, use
> > pm_runtime_put_sync_suspend() instead.
> 
> Yes.. That works too. I guess the thing to consider is if we should
> make pm_runtime_put_sync() always sync along the lines of a patch
> I posted earlier today. That could avoid quite a bit of confusion
> as already seen in this thread :)

As Rafael pointed out, pm_runtime_put_sync() has well-documented 
behavior.  It shouldn't be changed.  I don't see how changing the 
behavior would reduce anybody's confusion.  At least, anybody who reads 
the documentation carefully.

Alan Stern