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

On 2 February 2016 at 17:35, Tony Lindgren [off-list ref] wrote:

Hi,

* Ulf Hansson [off-list ref] [160202 02:43]:

quoted

For the omap_hsmmc and likely also other omap drivers, which needs more
than one attempt to ->probe() (returning -EPROBE_DEFER), this commit
causes a regression at the PM domain level (omap hwmod).

The reason is that the drivers don't put back the device into low power
state while bailing out in ->probe to return -EPROBE_DEFER. This leads to
that pm_runtime_reinit() in driver core, is re-initializing the runtime PM
status from RPM_ACTIVE to RPM_SUSPENDED.

Yup, that's the bug here. It seems that we never call the runtime_suspend
callback at the end of a first failed device driver probe if the driver
has set pm_runtime_use_autosuspend. Only rpm_idle runtime_idle callback
gets called. So the device stays on.

This does not happen if pm_runtime_dont_use_autosuspend() is added to
the end of the device driver probe before pm_runtime_put_sync().

Thanks! It then confirms the second option I proposed.

quoted

The next ->probe() attempt then triggers the ->runtime_resume() callback
to be invoked, which means this happens two times in a row. At the PM
domain level (omap hwmod) this is being treated as an error and thus the
runtime PM status of the device isn't correctly synchronized with the
runtime PM core.

That's a valid error though, let's not remove it. The reason why we
call runtime_resume() twice is because runtime_suspend callback never
gets called like I explain above.

This isn't an error, it's just a hickup in the synchronization of the
runtime PM status.

Very similar what happens at the first probe, when the driver core has
initialized the runtime PM status to RPM_SUSPENDED at the device
registration.

To me, it's the responsible of the PM domain to *help* with the
synchronization, not prevent it as it currently does.

It's good know this works, although do you intend to fix this sequence
for all omap drivers/devices that's part of the hwmod PM domain?

I haven't checked the number of drivers this would affect, but I
imagine there could be quite many with similar behaviour and thus may
suffer from the same issue.

Of course the regression is only noticed for those returning
-EPROBE_DEFER, which might not be that many, but it seems fragile to
rely on this when going forward. All related drivers then needs to be
fixed.

        pm_runtime_put_sync(host->dev);
        pm_runtime_disable(host->dev);
        if (host->dbclk)

Could you please test my version 2 of the patch I attached earlier. I
still believe it's the best way to solve the regression, if it works
of course. :-)

Kind regards
Uffe