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. 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. [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.] So the issue with genpd really seems to be a messenger here. :-) Thanks, Rafael