Hi Bjorn,

On Thu Sep 3, 2026 at 5:01 PM CEST, Bjorn Andersson wrote:
> On Tue, Sep 01, 2026 at 10:58:59AM +0200, Luca Weiss wrote:
>> Add the vdd-*-supply properties to the rpmh-regulators nodes, so that
>> the parent regulators can be enabled when needed.
>> 
>
> https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> starts with "Describe your problem.". Is there an actual problem observed?

No, this was noticed just in 'code', that other (also rpmh) devices have
the -supply properties, but not this device.

> I've been told that rpmh will have your back here, so you should not
> need to explicitly vote for these resources from Linux.

Then, this change still makes
/sys/kernel/debug/regulator/regulator_summary have a nice hierachy of
regulators and not all of them parented to regulator-dummy.

Shall I send a v2 with this justification in the commit message? Or is
it okay to pick it up as-is?

Regards
Luca

>
> Regards,
> Bjorn
>
>> The regulators s4a and s6a are a bit special. They feed back into
>> PM6350, but are not modelled as regulators (or anything else?), so they
>> also can't be added as vdd-*-supply. It would seem to be possible to add
>> them with their "Default on booting voltage" from the PM6350 Data Sheet
>> in the dts, but I'm not sure this is desired.
>> 
>> And document what the "missing" regulators are used for.
>> 
>> Signed-off-by: Luca Weiss <[email protected]>
>> ---
>>  arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts | 46 
>> +++++++++++++++++++++++
>>  1 file changed, 46 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts 
>> b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
>> index 5d20ad93b0c5..a1556d802a8b 100644
>> --- a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
>> +++ b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
>> @@ -78,6 +78,17 @@ lcdb_dummy: regulator-lcdb-dummy {
>>              regulator-max-microvolt = <5500000>;
>>      };
>>  
>> +    vph_pwr: regulator-vph-pwr {
>> +            compatible = "regulator-fixed";
>> +
>> +            regulator-name = "vph_pwr";
>> +            regulator-min-microvolt = <3700000>;
>> +            regulator-max-microvolt = <3700000>;
>> +
>> +            regulator-always-on;
>> +            regulator-boot-on;
>> +    };
>> +
>>      reserved-memory {
>>              /*
>>               * The rmtfs memory region in downstream is 'dynamically 
>> allocated'
>> @@ -296,6 +307,15 @@ &adsp {
>>  &apps_rsc {
>>      regulators-0 {
>>              compatible = "qcom,pm6350-rpmh-regulators";
>> +
>> +            vdd-l2-l5-l7-supply = <&vreg_s2a>;
>> +            vdd-l3-l6-l8-l9-l10-supply = <&vreg_bob>;
>> +            vdd-l11-supply = <&vreg_s2a>;
>> +            vdd-l12-l14-supply = <&vreg_s2a>;
>> +            vdd-l15-l19-l22-supply = <&vreg_s8e>;
>> +            vdd-s1-supply = <&vph_pwr>;
>> +            vdd-s2-supply = <&vph_pwr>;
>> +
>>              qcom,pmic-id = "a";
>>  
>>              vreg_s1a: smps1 {
>> @@ -310,6 +330,14 @@ vreg_s2a: smps2 {
>>                      regulator-max-microvolt = <2048000>;
>>              };
>>  
>> +            /*
>> +             * s3a (smps3) is gfx.lvl
>> +             * s4a (smps4) is supply for vdd-l4 & vdd-l13
>> +             * s5a (smps5) is mx.lvl
>> +             * s6a (smps6) is supply for vdd-l16 & vdd-l18-l20-l21
>> +             * l1a (ldo1) is lcx.lvl
>> +             */
>> +
>>              vreg_l2a: ldo2 {
>>                      regulator-name = "vreg_l2a";
>>                      regulator-min-microvolt = <1503000>;
>> @@ -408,6 +436,8 @@ vreg_l16a: ldo16 {
>>                      regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>>              };
>>  
>> +            /* l17a (ldo17) is lmx.lvl */
>> +
>>              vreg_l18a: ldo18 {
>>                      regulator-name = "vreg_l18a";
>>                      regulator-min-microvolt = <788000>;
>> @@ -446,8 +476,24 @@ vreg_l22a: ldo22 {
>>  
>>      regulators-1 {
>>              compatible = "qcom,pm6150l-rpmh-regulators";
>> +
>> +            vdd-bob-supply = <&vph_pwr>;
>> +            vdd-l1-l8-supply = <&vreg_s2a>;
>> +            vdd-l2-l3-supply = <&vreg_s8e>;
>> +            vdd-l4-l5-l6-supply = <&vreg_bob>;
>> +            vdd-l7-l11-supply = <&vreg_bob>;
>> +            vdd-l9-l10-supply = <&vreg_bob>;
>> +            vdd-s8-supply = <&vph_pwr>;
>> +
>>              qcom,pmic-id = "e";
>>  
>> +            /*
>> +             * s1e-s2e (smps1-smps2) is cx.lvl
>> +             * s3e-s4e (smps3-smps4) is VDD_APC1
>> +             * s5e (smps5) is VDD_APC0
>> +             * s6e-s7e (smps6-smps7) is mss.lvl
>> +             */
>> +
>>              vreg_s8e: smps8 {
>>                      regulator-name = "vreg_s8e";
>>                      regulator-min-microvolt = <313000>;
>> 
>> -- 
>> 2.55.0
>> 



Reply via email to