Re: [PATCH 1/3] dt-bindings: phy: qcom,qmp-pcie: add optional current load properties

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Ziyue Zhang <quic_ziyuzhan@quicinc.com>,
	vkoul@kernel.org, kishon@kernel.org, dmitry.baryshkov@linaro.org,
	abel.vesa@linaro.org, neil.armstrong@linaro.org,
	andersson@kernel.org, konradybcio@kernel.org, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org,
	linux-arm-msm@vger.kernel.org, linux-phy@lists.infradead.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 1/3] dt-bindings: phy: qcom,qmp-pcie: add optional current load properties
Date: Wed, 11 Dec 2024 13:54:04 +0530	[thread overview]
Message-ID: <20241211082404.p7fbmhooikmipxvm@thinkpad> (raw)
In-Reply-To: <33697bd9-02f4-4a9a-b8c0-4930d7fdaee2@kernel.org>

On Wed, Dec 11, 2024 at 09:09:18AM +0100, Krzysztof Kozlowski wrote:
> On 11/12/2024 07:20, Manivannan Sadhasivam wrote:
> > On Thu, Dec 05, 2024 at 11:23:11AM +0100, Krzysztof Kozlowski wrote:
> >> On Wed, Dec 04, 2024 at 06:52:47PM +0800, Ziyue Zhang wrote:
> >>> On some platforms, the power supply for PCIe PHY is not able to provide
> >>> enough current when it works in LPM mode. Hence, PCIe PHY driver needs to
> >>> set current load to vote the regulator to HPM mode.
> >>>
> >>> Document the current load as properties for each power supply PCIe PHY
> >>> required, namely vdda-phy-max-microamp, vdda-pll-max-microamp and
> >>> vdda-qref-max-microamp, respectively.PCIe PHY driver should parse them to
> >>> set appropriate current load during PHY power on.
> >>>
> >>> This three properties are optional and not mandatory for those platforms
> >>> that PCIe PHY can still work with power supply.
> >>
> >>
> >> Uh uh, so the downstream comes finally!
> >>
> >> No sorry guys, use existing regulator bindings for this.
> >>
> > 
> > Maybe they got inspired by upstream UFS bindings?
> > Documentation/devicetree/bindings/ufs/ufs-common.yaml:
> > 
> > vcc-max-microamp
> > vccq-max-microamp
> > vccq2-max-microamp
> 
> And it is already an ABI, so we cannot do anything about it.
> 
> > 
> > Regulator binding only describes the min/max load for the regulators and not
> 
> No, it exactly describes min/max consumers can use. Let's quote:
> "largest current consumers may set"
> It is all about consumers.
> 
> > consumers. What if the consumers need to set variable load per platform? Should
> 
> Then each platform uses regulator API or regulator bindings to set it? I
> don't see the problem here.
> 
> > they hardcode the load in driver? (even so, the load should not vary for each
> > board).
> 
> The load must vary per board, because regulators vary per board. Of
> course in practice most designs could be the same, but regulators and
> their limits are always properties of the board, not the SoC.
> 

How the consumer drivers are supposed to know the optimum load?

I don't see how the consumer drivers can set the load without hardcoding the
values. And I could see from UFS properties that each board has different
values.

- Mani

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy