diff mbox series

[V6,2/6] dt-bindings: mfd: pm8008: Add regulators node

Message ID 1644915231-7308-3-git-send-email-quic_c_skakit@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Add Qualcomm Technologies, Inc. PM8008 regulator driver | expand

Commit Message

Satya Priya Kakitapalli (Temp) Feb. 15, 2022, 8:53 a.m. UTC
Add regulators node and their supply nodes. Add separate compatible
"qcom,pm8008-regulators" to differentiate between pm8008 infra
and pm8008 regulators mfd devices.

Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changes in V2:
 - As per Rob's comments changed "pm8008[a-z]?-regulator" to
   "^pm8008[a-z]?-regulators".

Changes in V3:
 - Fixed bot errors.
 - As per stephen's comments, changed "^pm8008[a-z]?-regulators$" to
   "regulators".

Changes in V4:
 - Changed compatible string to "qcom,pm8008-regulators"

Changes in V5:
 - Remove compatible for regulators node.
 - Move supply nodes of the regulators to chip level.

Changes in V6:
 - No changes.

 .../devicetree/bindings/mfd/qcom,pm8008.yaml       | 49 +++++++++++++++++++---
 1 file changed, 44 insertions(+), 5 deletions(-)

Comments

Stephen Boyd Feb. 16, 2022, 2:04 a.m. UTC | #1
Quoting Satya Priya (2022-02-15 00:53:47)
> Add regulators node and their supply nodes. Add separate compatible
> "qcom,pm8008-regulators" to differentiate between pm8008 infra
> and pm8008 regulators mfd devices.
>
> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Please remove my tag.

> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> Changes in V2:
>  - As per Rob's comments changed "pm8008[a-z]?-regulator" to
>    "^pm8008[a-z]?-regulators".
>
> Changes in V3:
>  - Fixed bot errors.
>  - As per stephen's comments, changed "^pm8008[a-z]?-regulators$" to
>    "regulators".
>
> Changes in V4:
>  - Changed compatible string to "qcom,pm8008-regulators"
>
> Changes in V5:
>  - Remove compatible for regulators node.
>  - Move supply nodes of the regulators to chip level.
>
> Changes in V6:
>  - No changes.
>
>  .../devicetree/bindings/mfd/qcom,pm8008.yaml       | 49 +++++++++++++++++++---
>  1 file changed, 44 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
> index ec3138c..1ec43f7 100644
> --- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
> +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
> @@ -44,6 +46,25 @@ properties:
>    "#size-cells":
>      const: 0
>
> +  vdd_l1_l2-supply:
> +    description: Input supply phandle of ldo1 and ldo2 regulators.
> +
> +  vdd_l3_l4-supply:
> +    description: Input supply phandle of ldo3 and ldo4 regulators.
> +
> +  vdd_l5-supply:
> +    description: Input supply phandle of ldo5 regulator.
> +
> +  vdd_l6-supply:
> +    description: Input supply phandle of ldo6 regulator.
> +
> +  vdd_l7-supply:
> +    description: Input supply phandle of ldo7 regulator.
> +
> +  regulators:
> +    type: object
> +    $ref: "../regulator/qcom,pm8008-regulator.yaml#"
> +
>  patternProperties:
>    "^gpio@[0-9a-f]+$":
>      type: object
> @@ -88,10 +109,8 @@ patternProperties:
>  required:
>    - compatible
>    - reg
> -  - interrupts
>    - "#address-cells"
>    - "#size-cells"
> -  - "#interrupt-cells"

Shouldn't interrupts and interrupt-cells still be required for
qcom,pm8008? This binding seems like it is for two different nodes.

>
>  additionalProperties: false
>
> @@ -102,7 +121,7 @@ examples:
>      qupv3_se13_i2c {
>        #address-cells = <1>;
>        #size-cells = <0>;
> -      pm8008i@8 {
> +      pm8008_infra: pm8008@8 {
>          compatible = "qcom,pm8008";
>          reg = <0x8>;
>          #address-cells = <1>;
> @@ -123,6 +142,26 @@ examples:
>            #interrupt-cells = <2>;
>          };
>        };
> -    };
>
> +      pm8008_regulators: pm8008@9 {
> +        compatible = "qcom,pm8008";

Isn't this supposed to be "qcom,pm8008-regulators"?

> +        reg = <0x9>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        vdd_l1_l2-supply = <&vreg_s8b_1p2>;
> +        vdd_l3_l4-supply = <&vreg_s1b_1p8>;
> +        vdd_l5-supply = <&vreg_bob>;
> +        vdd_l6-supply = <&vreg_bob>;
> +        vdd_l7-supply = <&vreg_bob>;
> +
> +        regulators {

I still don't get the need for this extra container. Please remove it.

> +          pm8008_l1: ldo1 {
> +            regulator-name = "pm8008_l1";
> +            regulator-min-microvolt = <950000>;
> +            regulator-max-microvolt = <1300000>;
> +          };
> +        };
> +      };
> +    };
Subbaraman Narayanamurthy Feb. 16, 2022, 4:31 a.m. UTC | #2
On 2/15/22 12:53 AM, Satya Priya wrote:
> @@ -102,7 +121,7 @@ examples:
>      qupv3_se13_i2c {
>        #address-cells = <1>;
>        #size-cells = <0>;
> -      pm8008i@8 {
> +      pm8008_infra: pm8008@8 {
>          compatible = "qcom,pm8008";
>          reg = <0x8>;
>          #address-cells = <1>;
> @@ -123,6 +142,26 @@ examples:
>            #interrupt-cells = <2>;
>          };
>        };
> -    };
>  
> +      pm8008_regulators: pm8008@9 {
> +        compatible = "qcom,pm8008";

Should this be "qcom,pm8008-regulators" ?

> +        reg = <0x9>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        vdd_l1_l2-supply = <&vreg_s8b_1p2>;
> +        vdd_l3_l4-supply = <&vreg_s1b_1p8>;
> +        vdd_l5-supply = <&vreg_bob>;
> +        vdd_l6-supply = <&vreg_bob>;
> +        vdd_l7-supply = <&vreg_bob>;
> +
> +        regulators {
> +          pm8008_l1: ldo1 {
> +            regulator-name = "pm8008_l1";
> +            regulator-min-microvolt = <950000>;
> +            regulator-max-microvolt = <1300000>;
> +          };
> +        };
> +      };
> +    };
>  ...

-Subbaraman
Satya Priya Kakitapalli (Temp) Feb. 16, 2022, 9:49 a.m. UTC | #3
On 2/16/2022 7:34 AM, Stephen Boyd wrote:
> Quoting Satya Priya (2022-02-15 00:53:47)
>> Add regulators node and their supply nodes. Add separate compatible
>> "qcom,pm8008-regulators" to differentiate between pm8008 infra
>> and pm8008 regulators mfd devices.
>>
>> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
>> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> Please remove my tag.


Okay


>> Reviewed-by: Rob Herring <robh@kernel.org>
>> ---
>> Changes in V2:
>>   - As per Rob's comments changed "pm8008[a-z]?-regulator" to
>>     "^pm8008[a-z]?-regulators".
>>
>> Changes in V3:
>>   - Fixed bot errors.
>>   - As per stephen's comments, changed "^pm8008[a-z]?-regulators$" to
>>     "regulators".
>>
>> Changes in V4:
>>   - Changed compatible string to "qcom,pm8008-regulators"
>>
>> Changes in V5:
>>   - Remove compatible for regulators node.
>>   - Move supply nodes of the regulators to chip level.
>>
>> Changes in V6:
>>   - No changes.
>>
>>   .../devicetree/bindings/mfd/qcom,pm8008.yaml       | 49 +++++++++++++++++++---
>>   1 file changed, 44 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
>> index ec3138c..1ec43f7 100644
>> --- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
>> @@ -44,6 +46,25 @@ properties:
>>     "#size-cells":
>>       const: 0
>>
>> +  vdd_l1_l2-supply:
>> +    description: Input supply phandle of ldo1 and ldo2 regulators.
>> +
>> +  vdd_l3_l4-supply:
>> +    description: Input supply phandle of ldo3 and ldo4 regulators.
>> +
>> +  vdd_l5-supply:
>> +    description: Input supply phandle of ldo5 regulator.
>> +
>> +  vdd_l6-supply:
>> +    description: Input supply phandle of ldo6 regulator.
>> +
>> +  vdd_l7-supply:
>> +    description: Input supply phandle of ldo7 regulator.
>> +
>> +  regulators:
>> +    type: object
>> +    $ref: "../regulator/qcom,pm8008-regulator.yaml#"
>> +
>>   patternProperties:
>>     "^gpio@[0-9a-f]+$":
>>       type: object
>> @@ -88,10 +109,8 @@ patternProperties:
>>   required:
>>     - compatible
>>     - reg
>> -  - interrupts
>>     - "#address-cells"
>>     - "#size-cells"
>> -  - "#interrupt-cells"
> Shouldn't interrupts and interrupt-cells still be required for
> qcom,pm8008?


No, these are not strictly required.


> This binding seems like it is for two different nodes.


This binding is for the same node with different SIDs 8 and 9, having 
different set of peripherals.


>>   additionalProperties: false
>>
>> @@ -102,7 +121,7 @@ examples:
>>       qupv3_se13_i2c {
>>         #address-cells = <1>;
>>         #size-cells = <0>;
>> -      pm8008i@8 {
>> +      pm8008_infra: pm8008@8 {
>>           compatible = "qcom,pm8008";
>>           reg = <0x8>;
>>           #address-cells = <1>;
>> @@ -123,6 +142,26 @@ examples:
>>             #interrupt-cells = <2>;
>>           };
>>         };
>> -    };
>>
>> +      pm8008_regulators: pm8008@9 {
>> +        compatible = "qcom,pm8008";
> Isn't this supposed to be "qcom,pm8008-regulators"?


Yes, I'll correct this.


>> +        reg = <0x9>;
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        vdd_l1_l2-supply = <&vreg_s8b_1p2>;
>> +        vdd_l3_l4-supply = <&vreg_s1b_1p8>;
>> +        vdd_l5-supply = <&vreg_bob>;
>> +        vdd_l6-supply = <&vreg_bob>;
>> +        vdd_l7-supply = <&vreg_bob>;
>> +
>> +        regulators {
> I still don't get the need for this extra container. Please remove it.


Okay.


>> +          pm8008_l1: ldo1 {
>> +            regulator-name = "pm8008_l1";
>> +            regulator-min-microvolt = <950000>;
>> +            regulator-max-microvolt = <1300000>;
>> +          };
>> +        };
>> +      };
>> +    };
Satya Priya Kakitapalli (Temp) Feb. 16, 2022, 9:50 a.m. UTC | #4
On 2/16/2022 10:01 AM, Subbaraman Narayanamurthy wrote:
> On 2/15/22 12:53 AM, Satya Priya wrote:
>> @@ -102,7 +121,7 @@ examples:
>>       qupv3_se13_i2c {
>>         #address-cells = <1>;
>>         #size-cells = <0>;
>> -      pm8008i@8 {
>> +      pm8008_infra: pm8008@8 {
>>           compatible = "qcom,pm8008";
>>           reg = <0x8>;
>>           #address-cells = <1>;
>> @@ -123,6 +142,26 @@ examples:
>>             #interrupt-cells = <2>;
>>           };
>>         };
>> -    };
>>   
>> +      pm8008_regulators: pm8008@9 {
>> +        compatible = "qcom,pm8008";
> Should this be "qcom,pm8008-regulators" ?


Yes, I'll correct it.


Thanks.


>> +        reg = <0x9>;
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        vdd_l1_l2-supply = <&vreg_s8b_1p2>;
>> +        vdd_l3_l4-supply = <&vreg_s1b_1p8>;
>> +        vdd_l5-supply = <&vreg_bob>;
>> +        vdd_l6-supply = <&vreg_bob>;
>> +        vdd_l7-supply = <&vreg_bob>;
>> +
>> +        regulators {
>> +          pm8008_l1: ldo1 {
>> +            regulator-name = "pm8008_l1";
>> +            regulator-min-microvolt = <950000>;
>> +            regulator-max-microvolt = <1300000>;
>> +          };
>> +        };
>> +      };
>> +    };
>>   ...
> -Subbaraman
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
index ec3138c..1ec43f7 100644
--- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
+++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
@@ -16,7 +16,9 @@  description: |
 
 properties:
   compatible:
-    const: qcom,pm8008
+    enum:
+      - qcom,pm8008
+      - qcom,pm8008-regulators
 
   reg:
     description:
@@ -44,6 +46,25 @@  properties:
   "#size-cells":
     const: 0
 
+  vdd_l1_l2-supply:
+    description: Input supply phandle of ldo1 and ldo2 regulators.
+
+  vdd_l3_l4-supply:
+    description: Input supply phandle of ldo3 and ldo4 regulators.
+
+  vdd_l5-supply:
+    description: Input supply phandle of ldo5 regulator.
+
+  vdd_l6-supply:
+    description: Input supply phandle of ldo6 regulator.
+
+  vdd_l7-supply:
+    description: Input supply phandle of ldo7 regulator.
+
+  regulators:
+    type: object
+    $ref: "../regulator/qcom,pm8008-regulator.yaml#"
+
 patternProperties:
   "^gpio@[0-9a-f]+$":
     type: object
@@ -88,10 +109,8 @@  patternProperties:
 required:
   - compatible
   - reg
-  - interrupts
   - "#address-cells"
   - "#size-cells"
-  - "#interrupt-cells"
 
 additionalProperties: false
 
@@ -102,7 +121,7 @@  examples:
     qupv3_se13_i2c {
       #address-cells = <1>;
       #size-cells = <0>;
-      pm8008i@8 {
+      pm8008_infra: pm8008@8 {
         compatible = "qcom,pm8008";
         reg = <0x8>;
         #address-cells = <1>;
@@ -123,6 +142,26 @@  examples:
           #interrupt-cells = <2>;
         };
       };
-    };
 
+      pm8008_regulators: pm8008@9 {
+        compatible = "qcom,pm8008";
+        reg = <0x9>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        vdd_l1_l2-supply = <&vreg_s8b_1p2>;
+        vdd_l3_l4-supply = <&vreg_s1b_1p8>;
+        vdd_l5-supply = <&vreg_bob>;
+        vdd_l6-supply = <&vreg_bob>;
+        vdd_l7-supply = <&vreg_bob>;
+
+        regulators {
+          pm8008_l1: ldo1 {
+            regulator-name = "pm8008_l1";
+            regulator-min-microvolt = <950000>;
+            regulator-max-microvolt = <1300000>;
+          };
+        };
+      };
+    };
 ...