lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zu007YUYie7bEQvj@hovoldconsulting.com>
Date: Fri, 20 Sep 2024 10:40:13 +0200
From: Johan Hovold <johan@...nel.org>
To: Abel Vesa <abel.vesa@...aro.org>
Cc: Vinod Koul <vkoul@...nel.org>,
	Kishon Vijay Abraham I <kishon@...nel.org>,
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
	Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
	linux-arm-msm@...r.kernel.org, linux-phy@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] phy: qcom: edp: Add runtime PM support

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.

> 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.

> > > 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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ