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

["Satya Priya Kakitapalli (Temp)" <quic_c_skakit@xxxxxxxxxxx>]

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@xxxxxxxxxxx>
> > Reviewed-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> Please remove my tag.


Okay


> > Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
> > ---
> > 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>;
> > +          };
> > +        };
> > +      };
> > +    };