Message-ID: <89ab7e51-f82e-465a-aa22-1ccb8e7a0f6d@quicinc.com>
Date: Tue, 12 Aug 2025 01:25:02 +0530
From: Nitin Rawat <quic_nitirawa@...cinc.com>
To: Manivannan Sadhasivam <mani@...nel.org>
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 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.


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
> 

Powered by blists - more mailing lists