[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6ebf5fcc-011c-4984-ac5b-8139d0da8588@quicinc.com>
Date: Mon, 25 Nov 2024 18:02:40 +0530
From: Komal Bajaj <quic_kbajaj@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC: Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio
<konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski
<krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Melody Olvera
<quic_molvera@...cinc.com>,
<linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] arm64: dts: qcom: qdu/qru1000-idp: Fix the voltage
setting
On 11/20/2024 5:27 PM, Dmitry Baryshkov wrote:
> On Tue, Nov 19, 2024 at 12:38:11PM +0530, Komal Bajaj wrote:
>> While adding the USB support, it was found that the configuration
>> for regulator smps5 was incorrectly set. Upon cross verifying for
>> all the regulators, found that smps4, smps6 and smps8 are also
>> incorrectly configured. The patch corrects these configurations.
>>
>> In particular -
>> - smps4 is 1.574V min and 2.04V max
>> - smps5 is 1.2V min and 1.4V max
>> - smps6 is 0.382V min and 1.12V max
>> - smps8 is fixed at 0.752V
> Could you please comment whether your values represent the min/max
> supported by the regulators themselves or the shared min/max by all the
> devices powered by the corresponding regulator?
values represent the min/max supported by the regulators themselves
>> Fixes: d1f2cfe2f669 ("arm64: dts: qcom: Add base QDU1000/QRU1000 IDP DTs")
>> Signed-off-by: Komal Bajaj <quic_kbajaj@...cinc.com>
>> ---
>> Changes in v3 -
>> * Minor nit pick in commit message
>> * Link to v2: https://lore.kernel.org/all/20240524082236.24112-1-quic_kbajaj@quicinc.com/
>>
>> Changes in v2-
>> * Updated the commit message as suggested by Krzysztof
>> * Link to v1: https://lore.kernel.org/linux-arm-msm/20240514131038.28036-1-quic_kbajaj@quicinc.com/
>> ---
>> arch/arm64/boot/dts/qcom/qdu1000-idp.dts | 16 ++++++++--------
>> arch/arm64/boot/dts/qcom/qru1000-idp.dts | 16 ++++++++--------
>> 2 files changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/qdu1000-idp.dts b/arch/arm64/boot/dts/qcom/qdu1000-idp.dts
>> index e65305f8136c..6e8f9007068b 100644
>> --- a/arch/arm64/boot/dts/qcom/qdu1000-idp.dts
>> +++ b/arch/arm64/boot/dts/qcom/qdu1000-idp.dts
>> @@ -96,20 +96,20 @@ vreg_s3a_1p05: smps3 {
>>
>> vreg_s4a_1p8: smps4 {
>> regulator-name = "vreg_s4a_1p8";
>> - regulator-min-microvolt = <1800000>;
>> - regulator-max-microvolt = <1800000>;
>> + regulator-min-microvolt = <1574000>;
>> + regulator-max-microvolt = <2040000>;
>> };
>>
>> vreg_s5a_2p0: smps5 {
>> regulator-name = "vreg_s5a_2p0";
>> - regulator-min-microvolt = <1904000>;
>> - regulator-max-microvolt = <2000000>;
>> + regulator-min-microvolt = <1200000>;
>> + regulator-max-microvolt = <1400000>;
> Having 2.0 V regulator with the range of 1.2V - 1.4V is strange.
The configuration for this regulator was incorrectly set, as its nominal
range is 1.2 V.
There was no scenario utilizing this regulator, but we discovered this
misconfiguration while enabling USB.
>
>> };
>>
>> vreg_s6a_0p9: smps6 {
>> regulator-name = "vreg_s6a_0p9";
>> - regulator-min-microvolt = <920000>;
>> - regulator-max-microvolt = <1128000>;
>> + regulator-min-microvolt = <382000>;
>> + regulator-max-microvolt = <1120000>;
> The same applies to this regulator, 0.9V usually can not go to 0.382 V
> and still let the devices to continue working.
This regulator supports a minimum voltage, but it won't actually drop to
that level. The previous voltage values did not align with the Power Grid.
>
>> };
>>
>> vreg_s7a_1p2: smps7 {
>> @@ -120,8 +120,8 @@ vreg_s7a_1p2: smps7 {
>>
>> vreg_s8a_1p3: smps8 {
>> regulator-name = "vreg_s8a_1p3";
>> - regulator-min-microvolt = <1352000>;
>> - regulator-max-microvolt = <1352000>;
>> + regulator-min-microvolt = <752000>;
>> + regulator-max-microvolt = <752000>;
> 1.3V at 0.752V?
same applies here as well.
Thanks
Komal
>
>> };
>>
>> vreg_l1a_0p91: ldo1 {
>> diff --git a/arch/arm64/boot/dts/qcom/qru1000-idp.dts b/arch/arm64/boot/dts/qcom/qru1000-idp.dts
>> index 1c781d9e24cf..8b0ddc187ca0 100644
>> --- a/arch/arm64/boot/dts/qcom/qru1000-idp.dts
>> +++ b/arch/arm64/boot/dts/qcom/qru1000-idp.dts
>> @@ -96,20 +96,20 @@ vreg_s3a_1p05: smps3 {
>>
>> vreg_s4a_1p8: smps4 {
>> regulator-name = "vreg_s4a_1p8";
>> - regulator-min-microvolt = <1800000>;
>> - regulator-max-microvolt = <1800000>;
>> + regulator-min-microvolt = <1574000>;
>> + regulator-max-microvolt = <2040000>;
>> };
>>
>> vreg_s5a_2p0: smps5 {
>> regulator-name = "vreg_s5a_2p0";
>> - regulator-min-microvolt = <1904000>;
>> - regulator-max-microvolt = <2000000>;
>> + regulator-min-microvolt = <1200000>;
>> + regulator-max-microvolt = <1400000>;
>> };
>>
>> vreg_s6a_0p9: smps6 {
>> regulator-name = "vreg_s6a_0p9";
>> - regulator-min-microvolt = <920000>;
>> - regulator-max-microvolt = <1128000>;
>> + regulator-min-microvolt = <382000>;
>> + regulator-max-microvolt = <1120000>;
>> };
>>
>> vreg_s7a_1p2: smps7 {
>> @@ -120,8 +120,8 @@ vreg_s7a_1p2: smps7 {
>>
>> vreg_s8a_1p3: smps8 {
>> regulator-name = "vreg_s8a_1p3";
>> - regulator-min-microvolt = <1352000>;
>> - regulator-max-microvolt = <1352000>;
>> + regulator-min-microvolt = <752000>;
>> + regulator-max-microvolt = <752000>;
>> };
>>
>> vreg_l1a_0p91: ldo1 {
>> --
>> 2.46.0
>>
Powered by blists - more mailing lists