Re: [PATCH 1/7] dt-bindings: regulator: Convert regulator bindings to YAML format

Hi Rob,

Thanks for reviewing the patch!

quoted

+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: 
http://devicetree.org/schemas/regulator/qcom,rpmh-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies, Inc. RPMh Regulators
+
+maintainers:
+  - David Collins [off-list ref]
+
+description:

I assume you want the formatting here maintained, so you need a '|' at 
the end.

Ok.

quoted

+    rpmh-regulator devices support PMIC regulator management via the 
Voltage
+    Regulator Manager (VRM) and Oscillator Buffer (XOB) RPMh 
accelerators.  The APPS
+    processor communicates with these hardware blocks via a Resource 
State
+    Coordinator (RSC) using command packets.  The VRM allows changing 
three
+    parameters for a given regulator, enable state, output voltage, 
and operating
+    mode.  The XOB allows changing only a single parameter for a 
given regulator,
+    its enable state.  Despite its name, the XOB is capable of 
controlling the
+    enable state of any PMIC peripheral.  It is used for clock 
buffers, low-voltage
+    switches, and LDO/SMPS regulators which have a fixed voltage and 
mode.
+
+    =======================
+    Required Node Structure
+    =======================
+
+    RPMh regulators must be described in two levels of device nodes.  
The first
+    level describes the PMIC containing the regulators and must 
reside within an
+    RPMh device node.  The second level describes each regulator 
within the PMIC
+    which is to be used on the board.  Each of these regulators maps 
to a single
+    RPMh resource.
+
+    The names used for regulator nodes must match those supported by 
a given PMIC.
+    Supported regulator node names are
+      For PM8005, smps1 - smps4
+      For PM8009, smps1 - smps2, ldo1 - ldo7
+      For PM8150, smps1 - smps10, ldo1 - ldo18
+      For PM8150L, smps1 - smps8, ldo1 - ldo11, bob, flash, rgb

flash and rgb aren't documented.

Ok will add them.

quoted

+      For PM8350, smps1 - smps12, ldo1 - ldo10
+      For PM8350C, smps1 - smps10, ldo1 - ldo13, bob
+      For PM8998, smps1 - smps13, ldo1 - ldo28, lvs1 - lvs2
+      For PMI8998, bob
+      For PM6150, smps1 - smps5, ldo1 - ldo19
+      For PM6150L, smps1 - smps8, ldo1 - ldo11, bob
+      For PMX55, smps1 - smps7, ldo1 - ldo16
+
+properties:
+    compatible:
+        enum:
+            - qcom,pm8005-rpmh-regulators
+            - qcom,pm8009-rpmh-regulators
+            - qcom,pm8009-1-rpmh-regulators
+            - qcom,pm8150-rpmh-regulators
+            - qcom,pm8150l-rpmh-regulators
+            - qcom,pm8350-rpmh-regulators
+            - qcom,pm8350c-rpmh-regulators
+            - qcom,pm8998-rpmh-regulators
+            - qcom,pmi8998-rpmh-regulators
+            - qcom,pm6150-rpmh-regulators
+            - qcom,pm6150l-rpmh-regulators
+            - qcom,pmx55-rpmh-regulators
+
+    qcom,pmic-id:
+        description: RPMh resource name suffix used for the 
regulators found on
+                     this PMIC.  Typical values are "a", "b", "c", 
"d", "e", "f".

Sounds like constraints. Make the values a schema.

Ok

quoted

+        $ref: /schemas/types.yaml#/definitions/string
+
+    qcom,always-wait-for-ack:
+        description: Boolean flag which indicates that the 
application processor
+                     must wait for an ACK or a NACK from RPMh for 
every request
+                     sent for this regulator including those which 
are for a
+                     strictly lower power state.
+        $ref: /schemas/types.yaml#/definitions/string

Boolean or string?

Ok, will change it to /schemas/types.yaml#/definitions/flag

quoted

+
+patternProperties:
+  ".*-supply$":

You can drop '.*'. That's already the case without '^'.

Ok.

The supply names need to be defined.

you mean I should define like this "^vdd-s|l([0-9]+)-supply$": ?

quoted

+    description: phandle of the parent supply regulator of one or 
more of the
+                 regulators for this PMIC.
+
+  "^((smps|ldo|lvs)[0-9]*)$":

s/*/+/ as 1 digit is always required, right?

ok

quoted

+    type: object
+    allOf:

Don't need allOf.

ok, will drop this.

quoted

+     - $ref: "regulator.yaml#"
+    description: List of regulator parent supply phandles

This is a node, not a list of phandles.

Okay.

quoted

+
+  "bob$":

'foobob' is okay as that would be allowed? If a fixed string, put
under 'properties'.

It is fixed string, will move it to properties.

quoted

+    type: object
+    allOf:
+     - $ref: "regulator.yaml#"
+    description: BOB regulator parent supply phandle
+
+additionalProperties: false
+
+required:
+ - compatible
+ - qcom,pmic-id
+
+examples:
+  - |
+    #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+
+    pm8998-rpmh-regulators {
+        compatible = "qcom,pm8998-rpmh-regulators";
+        qcom,pmic-id = "a";
+
+        vdd-l7-l12-l14-l15-supply = <&pm8998_s5>;
+
+        smps2 {
+            regulator-min-microvolt = <1100000>;
+            regulator-max-microvolt = <1100000>;
+        };
+
+        pm8998_s5: smps5 {

Drop unused labels.

Okay.

quoted

+            regulator-min-microvolt = <1904000>;
+            regulator-max-microvolt = <2040000>;
+        };
+
+        ldo7 {
+            regulator-min-microvolt = <1800000>;
+            regulator-max-microvolt = <1800000>;
+            regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+            regulator-allowed-modes =
+                <RPMH_REGULATOR_MODE_LPM
+                 RPMH_REGULATOR_MODE_HPM>;
+            regulator-allow-set-load;
+        };
+
+        lvs1 {
+            regulator-min-microvolt = <1800000>;
+            regulator-max-microvolt = <1800000>;
+        };
+    };
+
+    pmi8998-rpmh-regulators {
+        compatible = "qcom,pmi8998-rpmh-regulators";
+        qcom,pmic-id = "b";
+
+        bob {
+            regulator-min-microvolt = <3312000>;
+            regulator-max-microvolt = <3600000>;
+            regulator-allowed-modes =
+                <RPMH_REGULATOR_MODE_AUTO
+                 RPMH_REGULATOR_MODE_HPM>;
+            regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
+        };
+    };
+...
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

Thanks,
Satya Priya