On 24-09-20 10:40:13, Johan Hovold wrote: > On Wed, Sep 18, 2024 at 04:10:05PM +0300, Abel Vesa wrote: > > On 24-09-18 12:05:59, Johan Hovold wrote: > > > On Sat, Sep 07, 2024 at 06:25:21PM +0300, Abel Vesa wrote: > > > > Enable runtime PM support by adding proper ops which will handle the > > > > > clocks and regulators. These resources will now be handled on power_on and > > > > power_off instead of init and exit PHY ops. > > > > > > No, this is simply a false claim and indicates that you haven't reviewed > > > how PHY runtime PM works. Core will increment the usage count on init() > > > and decrement it on exit(). > > > > Yeah, I guess the better argument here would be that the PHY needs > > regulators and clocks enabled > > No, that's already handled today so is clearly not a valid argument. I think we're saying the same thing here. I was trying to say that, as it is currently done and it's correct, the init() needs those resources enabled. > > > Anyway, ignore this version as it was already NACKed by Dmitry. > > No, my feedback is still valid, and you're bound to repeat the same > mistakes over and over again unless you try to understand what I've been > saying here. Duly noted. My reply wasn't trying to dismiss your feedback. Thanks for your feedback. > > > > > Also enable these resources on > > > > probe in order to balance out the disabling that is happening right after. > > > > Prevent runtime PM from being ON by default as well. > > > > > > And here you just regressed all current systems that do not have udev > > > rules to enable runtime PM, and which will now be stuck with these > > > resources always-on (e.g. during DPMS off and system suspend). > > > > > > In fact, you are even regressing systems that would enable runtime PM, > > > as the runtime suspend callback would not currently be called when you > > > enter system suspend so the regulators and clocks will be left on. > > > > > > This clearly hasn't been tested and analysed properly. > > Johan
