回复: [PATCH V1 2/4] arm64: dts: qcom: sm8750: add max-microamp for UFS PHY and PLL supplies

[<yizhijiao2025@163.com>]

-----邮件原件-----
发件人: linux-arm-msm+bounces-68747-yizhijiao2025=163.com@vger.kernel.org <linux-arm-msm+bounces-68747-yizhijiao2025=163.com@vger.kernel.org> 代表 Dmitry Baryshkov
发送时间: 2025年8月12日 18:52
收件人: Nitin Rawat <quic_nitirawa@quicinc.com>
抄送: Manivannan Sadhasivam <mani@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>; Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>; vkoul@kernel.org; kishon@kernel.org; conor+dt@kernel.org; bvanassche@acm.org; andersson@kernel.org; neil.armstrong@linaro.org; konradybcio@kernel.org; krzk+dt@kernel.org; linux-arm-msm@vger.kernel.org; linux-phy@lists.infradead.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
主题: Re: [PATCH V1 2/4] arm64: dts: qcom: sm8750: add max-microamp for UFS PHY and PLL supplies

On Tue, Aug 12, 2025 at 01:25:02AM +0530, Nitin Rawat wrote:
> 
> 
> On 8/9/2025 4:37 PM, Manivannan Sadhasivam wrote:
> > 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.
> 
> To avoid confusion with regulator-level constraints, I'm open to 
> renaming the property vdda-phy-max-microamp to something more 
> descriptive, such as vdda-phy-client-peak-load-microamp or 
> vdda-phy-peak-load-microamp. Along with updating the description, this 
> would better reflect the property's actual intent: to specify the 
> maximum current a client may draw during peak operation, rather than 
> implying it defines the regulator’s maximum capability.

Move them into the driver please.

> 
> 
> Having said that, I had a follow-up discussion with the PHY designer 
> to confirm whether this value could vary at the board level. Based on 
> their response, it's a fixed value for the SoC and does not change 
> across different boards(atleast for now). Therefore, I can remove from 
> device tree and replaced with hardcoded, per-compatible data in the driver.
> 
> > 
> > > 
> > > > 
> > > > 
> > > > > 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
> > 
> 

--
With best wishes
Dmitry