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] [day] [month] [year] [list]
Message-ID: <rh3qxu2rijpjswfash3rpmmh6sw47l3b6j5p5upti6zffknasz@cywwm3fypghd>
Date: Sat, 9 Aug 2025 16:37:16 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Nitin Rawat <quic_nitirawa@...cinc.com>
Cc: Krzysztof Kozlowski <krzk@...nel.org>, 
	Konrad Dybcio <konrad.dybcio@....qualcomm.com>, vkoul@...nel.org, kishon@...nel.org, conor+dt@...nel.org, 
	bvanassche@....org, andersson@...nel.org, neil.armstrong@...aro.org, 
	dmitry.baryshkov@....qualcomm.com, konradybcio@...nel.org, krzk+dt@...nel.org, 
	linux-arm-msm@...r.kernel.org, linux-phy@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org
Subject: Re: [PATCH V1 2/4] arm64: dts: qcom: sm8750: add max-microamp for
 UFS PHY and PLL supplies

On Fri, Aug 08, 2025 at 08:49:45PM GMT, Nitin Rawat wrote:
> 
> 
> On 8/8/2025 3:09 PM, Krzysztof Kozlowski wrote:
> > On 08/08/2025 10:58, Konrad Dybcio wrote:
> > > On 8/8/25 9:29 AM, Krzysztof Kozlowski wrote:
> > > > On Wed, Aug 06, 2025 at 09:13:38PM +0530, Nitin Rawat wrote:
> > > > > Add `vdda-phy-max-microamp` and `vdda-pll-max-microamp` properties to
> > > > > the UFS PHY node in the device tree.
> > > > > 
> > > > > These properties define the maximum current (in microamps) expected
> > > > > from the PHY and PLL regulators. This allows the PHY driver to
> > > > > configure regulator load accurately and ensure proper regulator
> > > > > mode based on load requirements.
> > > > 
> > > > That's not the property of phy, but regulator.
> > > > 
> > > > Also reasoning is here incomplete - you just post downstream code. :/
> > > 
> > > The reason for this change is good, but perhaps not explained clearly
> > > 
> > > All of these values refer to the maximum current draw that needs to be
> > > allocated on a shared voltage supply for this peripheral (because the
> > 
> > 
> > It sounds very different than how much it can be drawn. How much can be
> > drawn is the property of the regulator. The regulator knows how much
> > current it can support.
> 
> Consumers are aware of their dynamic load requirements, which can vary at
> runtime—this awareness is not reciprocal. The power grid is designed based
> on the collective load requirements of all clients sharing the same power
> rail.
> 
> Since regulators can operate in multiple modes for power optimization, each
> consumer is expected to vote for its runtime power needs. These votes help
> the regulator framework maintain the regulator in the appropriate mode,
> ensuring stable and efficient operation across all clients.
> 
> 
> Stability issues can arise if each consumer does not vote for its own load
> requirement.
> For example, consider a scenario where a single regulator is shared by two
> consumers.
> 
> If the first client requests low-power mode by voting for zero or a minimal
> load to regulator framework during its driver's LPM sequence, and the second
> client (e.g., UFS PHY) has not voted for its own load requirement through
> the regulator framework, the regulator may transition to low-power mode.
> This can lead to issues for the second client, which expects a higher power
> state to operate correctly.
> 

I think we all agree on consumers setting the load for shared regulators, but
the naming and description of the DT property is what causing confusion here.
There is no way the consumers can set the *max* current draw for a shared
regulator. They can only request load as per their requirement. But the max
current draw is a regulator constraint.

> 
> > 
> > 
> > > supply's capabilities change depending on the maximum potential load
> > > at any given time, which the regulator driver must be aware of)
> > > 
> > > This is a property of a regulator *consumer*, i.e. if we had a chain
> > > of LEDs hanging off of this supply, we'd need to specify NUM_LEDS *
> > > MAX_CURR under the "led chain" device, to make sure that if the
> > > aggregated current requirements go over a certain threshold (which is
> > > unknown to Linux and hidden in RPMh fw), the regulator can be
> > > reconfigured to allow for a higher current draw (likely at some
> > > downgrade to efficiency)
> > 
> > 
> > The problem is that rationale is downstream. Instead I want to see some
> > reason: e.g. datasheets, spec, type of UFS device (that was the argument
> > in the driver patch discussion).
> 
> 
> The PHY load requirements for consumers such as UFS, USB, PCIe are defined
> by Qualcomm’s PHY IP and are well-documented in Qualcomm’s datasheets and
> power grid documentation. These values can depending on the process or
> technology node, board design, and even the chip foundry used.
> 
> As a result, the load values can differ across SoCs or may be even
> board(unlikely though) due to variations in any of these parameters.
> 

Okay. This goes into the commit message and possibly some part of it to property
description also.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ