On 24 December 2017 at 13:00, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote: > On Saturday, December 23, 2017 4:09:33 PM CET Ulf Hansson wrote: >> [...] >> >> > >> > So IMO the changes you are proposing make sense regardless of the >> > genpd issue, because they generally simplify the phy code, but the >> > additional use_runtime_pm field in struct phy represents redundant >> > information (manipulating reference counters shouldn't matter if >> > runtime PM is disabled), so it doesn't appear to be necessary. >> > >> >> Actually, the first version I posted treated the return codes from >> pm_runtime_get_sync() according to your suggestion above. >> >> However, Kishon pointed out that it didn't work. That's because, there >> are phy provider drivers that enables runtime PM *after* calling >> phy_create(). And in those cases, that is because they want to treat >> runtime PM themselves. >> >> I think that's probably something we should look into to change, but I >> find it being a separate issue, that I didn't want to investigate as >> part of this series. >> >> See more about the thread here: >> https://www.spinics.net/lists/linux-renesas-soc/msg21711.html > > Even so, it shouldn't matter when the provider enables runtime PM. > > Calling pm_runtime_get_*()/pm_runtime_put_*() should always work as > long as they are balanced properly regardless of whether or not > runtime PM is enabled for the device or it is enabled in the meantime. > Of course, the initial runtime PM status of the device must reflect > the values of the usage counters, but that should not be too hard to > ensure. Yes, this is probably the main reason. However, as stated, I think we should look into addressing this problem more carefully, in a separate next step. > > The reason why it matters here is because the phy core tries to be smart > about manipulating reference counters by itself and that's a mistake. > > So it looks to me like the whole thing needs to be reworked and yes, > that should be done in the first place IMO, because it will make the > issue with genpd go away automatically. Sorry, but I am not fully understanding what you suggest here. Perhaps you didn't check patch2? > > [Why is phy_pm_runtime_get() there at all, for instance? It seems > to have no users and I kind of don't see use cases for it. Also > phy_pm_runtime_get_sync() is only used by the phy core in three > places - shouldn't be too hard to fix that.] Removing these APIs and functions is done in patch2, so I guess I am already doing what you suggests above? No? [...] Kind regards Uffe
