On 3 February 2016 at 00:41, Tony Lindgren <tony@xxxxxxxxxxx> wrote: > * Ulf Hansson <ulf.hansson@xxxxxxxxxx> [160202 12:48]: >> On 2 February 2016 at 17:35, Tony Lindgren <tony@xxxxxxxxxxx> wrote: >> > 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.. > >> 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? > >> 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? > >> > --- 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. > >> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
