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.
Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
arch/arm64/boot/dts/qcom/sm8750-mtp.dts | 2 ++
arch/arm64/boot/dts/qcom/sm8750-qrd.dts | 2 ++
2 files changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8750-mtp.dts b/arch/arm64/boot/dts/qcom/sm8750-mtp.dts
index 75cfbb510be5..2ae5915fe38d 100644
--- a/arch/arm64/boot/dts/qcom/sm8750-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sm8750-mtp.dts
@@ -1032,7 +1032,9 @@ wcd_default: wcd-reset-n-active-state {
&ufs_mem_phy {
vdda-phy-supply = <&vreg_l1j_0p91>;
+ vdda-phy-max-microamp = <213000>;
vdda-pll-supply = <&vreg_l3g_1p2>;
+ vdda-pll-max-microamp = <18300>;
status = "okay";
};
diff --git a/arch/arm64/boot/dts/qcom/sm8750-qrd.dts b/arch/arm64/boot/dts/qcom/sm8750-qrd.dts
index 13c7b9664c89..e9a41d34e2d6 100644
--- a/arch/arm64/boot/dts/qcom/sm8750-qrd.dts
+++ b/arch/arm64/boot/dts/qcom/sm8750-qrd.dts
@@ -1039,7 +1039,9 @@ &uart7 {
&ufs_mem_phy {
vdda-phy-supply = <&vreg_l1j_0p91>;
+ vdda-phy-max-microamp = <213000>;
vdda-pll-supply = <&vreg_l3g_1p2>;
+ vdda-pll-max-microamp = <18300>;
status = "okay";
};
--
2.48.1On 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.
>
But doesn't this imply that these values are fixed for a given SoC?
Perhaps even for a given generation of the PHY/process?
Is there any case where these values need to be tweaked on a per-board
basis?
If not, I think these should be constants in the driver.
Regards,
Bjorn
> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
> ---
> arch/arm64/boot/dts/qcom/sm8750-mtp.dts | 2 ++
> arch/arm64/boot/dts/qcom/sm8750-qrd.dts | 2 ++
> 2 files changed, 4 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8750-mtp.dts b/arch/arm64/boot/dts/qcom/sm8750-mtp.dts
> index 75cfbb510be5..2ae5915fe38d 100644
> --- a/arch/arm64/boot/dts/qcom/sm8750-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8750-mtp.dts
> @@ -1032,7 +1032,9 @@ wcd_default: wcd-reset-n-active-state {
>
> &ufs_mem_phy {
> vdda-phy-supply = <&vreg_l1j_0p91>;
> + vdda-phy-max-microamp = <213000>;
> vdda-pll-supply = <&vreg_l3g_1p2>;
> + vdda-pll-max-microamp = <18300>;
>
> status = "okay";
> };
> diff --git a/arch/arm64/boot/dts/qcom/sm8750-qrd.dts b/arch/arm64/boot/dts/qcom/sm8750-qrd.dts
> index 13c7b9664c89..e9a41d34e2d6 100644
> --- a/arch/arm64/boot/dts/qcom/sm8750-qrd.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8750-qrd.dts
> @@ -1039,7 +1039,9 @@ &uart7 {
>
> &ufs_mem_phy {
> vdda-phy-supply = <&vreg_l1j_0p91>;
> + vdda-phy-max-microamp = <213000>;
> vdda-pll-supply = <&vreg_l3g_1p2>;
> + vdda-pll-max-microamp = <18300>;
>
> status = "okay";
> };
> --
> 2.48.1
>
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. :/ Best regards, Krzysztof
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 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) Konrad
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. > 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 only argument here for given value is: downstream has it. Best regards, Krzysztof
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. > > >> 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. Regards, Nitin > > The only argument here for given value is: downstream has it. > > Best regards, > Krzysztof
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 -- மணிவண்ணன் சதாசிவம்
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 >
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
On 8/12/2025 4:22 PM, Dmitry Baryshkov wrote: > 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. Sure Dmitry. I’ll will take care of this in next patchset. Thanks, Nitin > >> >> >> 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 >>> >> >
-----邮件原件----- 发件人: 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
© 2016 - 2025 Red Hat, Inc.