[...] > > 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 > [On a related note, I'm not sure why phy tries to intercept runtime PM > errors and "fix up" the reference counters. That doesn't look right > to me at all.] > > That said, the current phy code is not strictly invalid. While it > looks more complicated than necessary, it doesn't do things documented > as invalid in principle, so saying "The behaviour around the runtime > PM deployment cause some issues during system suspend" in the > changelog is describing the problem from a very specific angle. > Simply put, pm_runtime_force_suspend() and the current phy code cannot > work together and so using them together is a bug. None of them > individually is at fault, but combining them is incorrect. > > Fortunately enough, the phy code can be modified so that it can be > used with pm_runtime_force_suspend() without problems, but picturing > it as "problematic", because it cannot do that today is not entirely > fair IMO. Right, this makes sense. Let me clarify this in the changelog. Kind regards Uffe
