diff mbox series

[v2] regulator: dt-bindings: microchip,mcp16502: convert to YAML

Message ID 20240129135336.98197-1-andrei.simion@microchip.com (mailing list archive)
State New, archived
Headers show
Series [v2] regulator: dt-bindings: microchip,mcp16502: convert to YAML | expand

Commit Message

Andrei Simion Jan. 29, 2024, 1:53 p.m. UTC
Convert devicetree binding mcp16502-regulator.txt to YAML format.

Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
---
Changes v1 -> v2:
- reverse subject prefixes
- line break after file "description:"
- drop description for reg
- drop regulator-name
- add regulator-initial-mode
- unevaluatedProperties just after $ref: regulator.yaml#
- additionalProperties  just before "required"
- node names generic in example
- use 4 spaces for example indentation
---
 .../bindings/regulator/mcp16502-regulator.txt | 144 --------------
 .../regulator/microchip,mcp16502.yaml         | 182 ++++++++++++++++++
 2 files changed, 182 insertions(+), 144 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/regulator/mcp16502-regulator.txt
 create mode 100644 Documentation/devicetree/bindings/regulator/microchip,mcp16502.yaml

Comments

Krzysztof Kozlowski Jan. 30, 2024, 8:21 a.m. UTC | #1
On 29/01/2024 14:53, Andrei Simion wrote:
> Convert devicetree binding mcp16502-regulator.txt to YAML format.
> 
> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>

Thank you for your patch. There is something to discuss/improve.

> ---
> Changes v1 -> v2:
> - reverse subject prefixes
> - line break after file "description:"
> - drop description for reg
> - drop regulator-name
> - add regulator-initial-mode
> - unevaluatedProperties just after $ref: regulator.yaml#
> - additionalProperties  just before "required"
> - node names generic in example
> - use 4 spaces for example indentation


> +
> +  reg:
> +    maxItems: 1
> +
> +  regulators:
> +    type: object
> +    description: List of regulators and its properties.
> +      A node that houses a sub-node for each regulator within
> +      the device. Each sub-node is identified using the node's
> +      name. The content of each sub-node is defined by the
> +      standard binding for regulators;

This is a friendly reminder during the review process.

It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.

Also: you can keep just one line of description. Rest is redundant.


Best regards,
Krzysztof
Andrei Simion Jan. 30, 2024, 3:22 p.m. UTC | #2
On 30.01.2024 10:21, Krzysztof Kozlowski wrote:
> [You don't often get email from krzysztof.kozlowski@linaro.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 29/01/2024 14:53, Andrei Simion wrote:
>> Convert devicetree binding mcp16502-regulator.txt to YAML format.
>>
>> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
> 
> Thank you for your patch. There is something to discuss/improve.
> 
>> ---
>> Changes v1 -> v2:
>> - reverse subject prefixes
>> - line break after file "description:"
>> - drop description for reg
>> - drop regulator-name
>> - add regulator-initial-mode
>> - unevaluatedProperties just after $ref: regulator.yaml#
>> - additionalProperties  just before "required"
>> - node names generic in example
>> - use 4 spaces for example indentation
> 
> 
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  regulators:
>> +    type: object

	additionalProperties: false  -> forgot this line. That is my missing to hit all the points in the previous review. Let me know If that is all.

>> +    description: List of regulators and its properties.
>> +      A node that houses a sub-node for each regulator within
>> +      the device. Each sub-node is identified using the node's
>> +      name. The content of each sub-node is defined by the
>> +      standard binding for regulators;> 
> This is a friendly reminder during the review process.
> 
> It seems my or other reviewer's previous comments were not fully
> addressed. Maybe the feedback got lost between the quotes, maybe you
> just forgot to apply it. Please go back to the previous discussion and
> either implement all requested changes or keep discussing them.
> > Also: you can keep just one line of description. Rest is redundant.
> 
Yes, I will change in v3 .

>> Best regards,
> Krzysztof
> 

Thank you for review and best regards,
Andrei Simion
Krzysztof Kozlowski Jan. 30, 2024, 4:16 p.m. UTC | #3
On 30/01/2024 16:22, Andrei.Simion@microchip.com wrote:
> On 30.01.2024 10:21, Krzysztof Kozlowski wrote:
>>
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  regulators:
>>> +    type: object
> 
> 	additionalProperties: false  -> forgot this line. That is my missing to hit all the points in the previous review. Let me know If that is all.

I think this should be it, thanks.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/regulator/mcp16502-regulator.txt b/Documentation/devicetree/bindings/regulator/mcp16502-regulator.txt
deleted file mode 100644
index 451cc4e86b01..000000000000
--- a/Documentation/devicetree/bindings/regulator/mcp16502-regulator.txt
+++ /dev/null
@@ -1,144 +0,0 @@ 
-MCP16502 PMIC
-
-Required properties:
-- compatible: "microchip,mcp16502"
-- reg: I2C slave address
-- lpm-gpios: GPIO for LPM pin. Note that this GPIO *must* remain high during
-	     suspend-to-ram, keeping the PMIC into HIBERNATE mode; this
-	     property is optional;
-- regulators: A node that houses a sub-node for each regulator within
-              the device. Each sub-node is identified using the node's
-              name. The content of each sub-node is defined by the
-              standard binding for regulators; see regulator.txt.
-
-Regulators of MCP16502 PMIC:
-1) VDD_IO	- Buck (1.2 - 3.7 V)
-2) VDD_DDR	- Buck (0.6 - 1.85 V)
-3) VDD_CORE	- Buck (0.6 - 1.85 V)
-4) VDD_OTHER	- BUCK (0.6 - 1.85 V)
-5) LDO1		- LDO  (1.2 - 3.7 V)
-6) LDO2		- LDO  (1.2 - 3.7 V)
-
-Regulator modes:
-2 - FPWM: higher precision, higher consumption
-4 - AutoPFM: lower precision, lower consumption
-
-Each regulator is defined using the standard binding for regulators.
-
-Example:
-
-mcp16502@5b {
-	compatible = "microchip,mcp16502";
-	reg = <0x5b>;
-	status = "okay";
-	lpm-gpios = <&pioBU 7 GPIO_ACTIVE_HIGH>;
-
-	regulators {
-		VDD_IO {
-			regulator-name = "VDD_IO";
-			regulator-min-microvolt = <1200000>;
-			regulator-max-microvolt = <3700000>;
-			regulator-initial-mode = <2>;
-			regulator-allowed-modes = <2>, <4>;
-			regulator-always-on;
-
-			regulator-state-standby {
-				regulator-on-in-suspend;
-				regulator-mode = <4>;
-			};
-
-			regulator-state-mem {
-				regulator-off-in-suspend;
-				regulator-mode = <4>;
-			};
-		};
-
-		VDD_DDR {
-			regulator-name = "VDD_DDR";
-			regulator-min-microvolt = <600000>;
-			regulator-max-microvolt = <1850000>;
-			regulator-initial-mode = <2>;
-			regulator-allowed-modes = <2>, <4>;
-			regulator-always-on;
-
-			regulator-state-standby {
-				regulator-on-in-suspend;
-				regulator-mode = <4>;
-			};
-
-			regulator-state-mem {
-				regulator-on-in-suspend;
-				regulator-mode = <4>;
-			};
-		};
-
-		VDD_CORE {
-			regulator-name = "VDD_CORE";
-			regulator-min-microvolt = <600000>;
-			regulator-max-microvolt = <1850000>;
-			regulator-initial-mode = <2>;
-			regulator-allowed-modes = <2>, <4>;
-			regulator-always-on;
-
-			regulator-state-standby {
-				regulator-on-in-suspend;
-				regulator-mode = <4>;
-			};
-
-			regulator-state-mem {
-				regulator-off-in-suspend;
-				regulator-mode = <4>;
-			};
-		};
-
-		VDD_OTHER {
-			regulator-name = "VDD_OTHER";
-			regulator-min-microvolt = <600000>;
-			regulator-max-microvolt = <1850000>;
-			regulator-initial-mode = <2>;
-			regulator-allowed-modes = <2>, <4>;
-			regulator-always-on;
-
-			regulator-state-standby {
-				regulator-on-in-suspend;
-				regulator-mode = <4>;
-			};
-
-			regulator-state-mem {
-				regulator-off-in-suspend;
-				regulator-mode = <4>;
-			};
-		};
-
-		LDO1 {
-			regulator-name = "LDO1";
-			regulator-min-microvolt = <1200000>;
-			regulator-max-microvolt = <3700000>;
-			regulator-always-on;
-
-			regulator-state-standby {
-				regulator-on-in-suspend;
-			};
-
-			regulator-state-mem {
-				regulator-off-in-suspend;
-			};
-		};
-
-		LDO2 {
-			regulator-name = "LDO2";
-			regulator-min-microvolt = <1200000>;
-			regulator-max-microvolt = <3700000>;
-			regulator-always-on;
-
-			regulator-state-standby {
-				regulator-on-in-suspend;
-			};
-
-			regulator-state-mem {
-				regulator-off-in-suspend;
-			};
-		};
-
-	};
-};
diff --git a/Documentation/devicetree/bindings/regulator/microchip,mcp16502.yaml b/Documentation/devicetree/bindings/regulator/microchip,mcp16502.yaml
new file mode 100644
index 000000000000..708f922764fe
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/microchip,mcp16502.yaml
@@ -0,0 +1,182 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/microchip,mcp16502.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MCP16502 - High-Performance PMIC
+
+maintainers:
+  - Andrei Simion <andrei.simion@microchip.com>
+
+description:
+  The MCP16502 is an optimally integrated PMIC compatible
+  with Microchip's eMPUs(Embedded Microprocessor Units), requiring
+  Dynamic Voltage Scaling (DVS) with the use of High-Performance mode (HPM).
+
+properties:
+  compatible:
+    const: microchip,mcp16502
+
+  lpm-gpios:
+    description: GPIO for LPM pin.
+      Note that this GPIO must remain high during
+      suspend-to-ram, keeping the PMIC into HIBERNATE mode;
+    maxItems: 1
+
+  reg:
+    maxItems: 1
+
+  regulators:
+    type: object
+    description: List of regulators and its properties.
+      A node that houses a sub-node for each regulator within
+      the device. Each sub-node is identified using the node's
+      name. The content of each sub-node is defined by the
+      standard binding for regulators;
+
+    patternProperties:
+      "^(VDD_(IO|CORE|DDR|OTHER)|LDO[1-2])$":
+        type: object
+        $ref: regulator.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          regulator-initial-mode:
+            description: Initial operating mode.
+            enum: [2, 4]
+            default: 2
+
+          regulator-allowed-modes:
+            description: Supported modes
+              2 - FPWM higher precision, higher consumption
+              4 - AutoPFM lower precision, lower consumption
+            items:
+              enum: [2, 4]
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - regulators
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pmic@5b {
+            compatible = "microchip,mcp16502";
+            reg = <0x5b>;
+
+            regulators {
+                VDD_IO {
+                    regulator-name = "VDD_IO";
+                    regulator-min-microvolt = <3300000>;
+                    regulator-max-microvolt = <3300000>;
+                    regulator-initial-mode = <2>;
+                    regulator-allowed-modes = <2>, <4>;
+                    regulator-always-on;
+
+                    regulator-state-standby {
+                        regulator-on-in-suspend;
+                        regulator-mode = <4>;
+                    };
+
+                    regulator-state-mem {
+                        regulator-off-in-suspend;
+                        regulator-mode = <4>;
+                    };
+                };
+
+                VDD_DDR {
+                    regulator-name = "VDD_DDR";
+                    regulator-min-microvolt = <1350000>;
+                    regulator-max-microvolt = <1350000>;
+                    regulator-initial-mode = <2>;
+                    regulator-allowed-modes = <2>, <4>;
+                    regulator-always-on;
+
+                    regulator-state-standby {
+                        regulator-on-in-suspend;
+                        regulator-mode = <4>;
+                    };
+
+                    regulator-state-mem {
+                        regulator-on-in-suspend;
+                        regulator-mode = <4>;
+                    };
+                };
+
+                VDD_CORE {
+                    regulator-name = "VDD_CORE";
+                    regulator-min-microvolt = <1150000>;
+                    regulator-max-microvolt = <1150000>;
+                    regulator-initial-mode = <2>;
+                    regulator-allowed-modes = <2>, <4>;
+                    regulator-always-on;
+
+                    regulator-state-standby {
+                        regulator-on-in-suspend;
+                        regulator-mode = <4>;
+                    };
+
+                    regulator-state-mem {
+                        regulator-off-in-suspend;
+                        regulator-mode = <4>;
+                    };
+                };
+
+                VDD_OTHER {
+                    regulator-name = "VDD_OTHER";
+                    regulator-min-microvolt = <1050000>;
+                    regulator-max-microvolt = <1250000>;
+                    regulator-initial-mode = <2>;
+                    regulator-allowed-modes = <2>, <4>;
+                    regulator-always-on;
+
+                    regulator-state-standby {
+                        regulator-on-in-suspend;
+                        regulator-mode = <4>;
+                    };
+
+                    regulator-state-mem {
+                        regulator-off-in-suspend;
+                        regulator-mode = <4>;
+                    };
+                };
+
+                LDO1 {
+                    regulator-name = "LDO1";
+                    regulator-min-microvolt = <1800000>;
+                    regulator-max-microvolt = <1800000>;
+                    regulator-always-on;
+
+                    regulator-state-standby {
+                        regulator-on-in-suspend;
+                    };
+
+                    regulator-state-mem {
+                        regulator-off-in-suspend;
+                    };
+                };
+
+                LDO2 {
+                    regulator-name = "LDO2";
+                    regulator-min-microvolt = <1200000>;
+                    regulator-max-microvolt = <3700000>;
+                    regulator-always-on;
+
+                    regulator-state-standby {
+                        regulator-on-in-suspend;
+                    };
+
+                    regulator-state-mem {
+                        regulator-off-in-suspend;
+                    };
+                };
+            };
+        };
+    };