On 3 February 2016 at 00:41, Tony Lindgren [off-list ref] wrote:

* Ulf Hansson [off-list ref] [160202 12:48]:

quoted

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

quoted

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.

I'd rather not get the hardware state out of sync with PM runtime..

quoted

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.

Well we actually pretty much have devices in that state to start
with.

That's not true, not even for omap.

There are definitely devices which HW state is reflected by RPM_ACTIVE
at boot. It's the responsible of the subsystem/driver (including PM
domains) to make sure the runtime PM status is updated accordingly, to
reflect the real HW state.

For omap hwmod, at device registration in omap_device_build_from_dt()
it may actually invoke pm_runtime_set_active() if the device is
enabled. To my understanding, that's done to synchronize the real HW
state with the runtime PM status, right?

quoted

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

The problem is that the hardware state gets out of sync with
PM runtime. And that's going to be a pain to debug later on.

I don't see the problem, but of course you know omap for better than I do.

So if you are concerned about this, perhaps adding a dev_dbg print
when the omap hwmod's ->runtime_suspend() callback returns zero could
be a way forward?

quoted

quoted

--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c

@@ -2232,6 +2232,7 @@ err_irq:
                dma_release_channel(host->tx_chan);
        if (host->rx_chan)
                dma_release_channel(host->rx_chan);
+       pm_runtime_dont_use_autosuspend(host->dev);

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.

Yeah not sure what the right fix is. But I'd rather patch the
few drivers using autosuspend if we come to the conclusion
that there is no bug in PM runtime.

quoted

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. :-)

And I don't like it because of the reasons above :) But yeah
gave it a quick try and that too works as expected.

Okay, thanks for testing!

Kind regards
Uffe