2020-06-11 08:07:54

by Pi-Hsun Shih

[permalink] [raw]
Subject: [PATCH v4 1/2] dt-bindings: regulator: Add DT binding for cros-ec-regulator

Add DT binding documentation for cros-ec-regulator, a voltage regulator
controlled by ChromeOS EC.

Signed-off-by: Pi-Hsun Shih <[email protected]>
---
Changes from v3:
* Fix dt bindings file name.
* Add full example.

Changes from v2:
* No change

Changes from v1:
* Change compatible string to google,regulator-cros-ec.
* Use reg property in device tree.
* Change license for dt binding according to checkpatch.pl.
---
.../regulator/google,cros-ec-regulator.yaml | 51 +++++++++++++++++++
1 file changed, 51 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/google,cros-ec-regulator.yaml

diff --git a/Documentation/devicetree/bindings/regulator/google,cros-ec-regulator.yaml b/Documentation/devicetree/bindings/regulator/google,cros-ec-regulator.yaml
new file mode 100644
index 000000000000..7e341b65b2dd
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/google,cros-ec-regulator.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/google,cros-ec-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ChromeOS EC controlled voltage regulators
+
+maintainers:
+ - Pi-Hsun Shih <[email protected]>
+
+description:
+ Any property defined as part of the core regulator binding, defined in
+ regulator.yaml, can also be used.
+
+allOf:
+ - $ref: "regulator.yaml#"
+
+properties:
+ compatible:
+ const: google,regulator-cros-ec
+
+ reg:
+ maxItems: 1
+ description: Identifier for the voltage regulator to ChromeOS EC.
+
+required:
+ - compatible
+ - reg
+
+examples:
+ - |
+ spi0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cros_ec: ec@0 {
+ compatible = "google,cros-ec-spi";
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ regulator@0 {
+ compatible = "google,regulator-cros-ec";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ reg = <0>;
+ };
+ };
+ };
+...
--
2.27.0.278.ge193c7cf3a9-goog


2020-06-11 08:19:34

by Enric Balletbo i Serra

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: regulator: Add DT binding for cros-ec-regulator

Hi Pi-Hsun,

Thank you for the patch, one minor nitpick that in my opinion would be good to
address.

On 11/6/20 10:04, Pi-Hsun Shih wrote:
> Add DT binding documentation for cros-ec-regulator, a voltage regulator
> controlled by ChromeOS EC.
>
> Signed-off-by: Pi-Hsun Shih <[email protected]>
> ---
> Changes from v3:
> * Fix dt bindings file name.
> * Add full example.
>
> Changes from v2:
> * No change
>
> Changes from v1:
> * Change compatible string to google,regulator-cros-ec.
> * Use reg property in device tree.
> * Change license for dt binding according to checkpatch.pl.
> ---
> .../regulator/google,cros-ec-regulator.yaml | 51 +++++++++++++++++++
> 1 file changed, 51 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/google,cros-ec-regulator.yaml
>
> diff --git a/Documentation/devicetree/bindings/regulator/google,cros-ec-regulator.yaml b/Documentation/devicetree/bindings/regulator/google,cros-ec-regulator.yaml
> new file mode 100644
> index 000000000000..7e341b65b2dd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/google,cros-ec-regulator.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/google,cros-ec-regulator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ChromeOS EC controlled voltage regulators
> +
> +maintainers:
> + - Pi-Hsun Shih <[email protected]>
> +
> +description:
> + Any property defined as part of the core regulator binding, defined in
> + regulator.yaml, can also be used.
> +
> +allOf:
> + - $ref: "regulator.yaml#"
> +
> +properties:
> + compatible:
> + const: google,regulator-cros-ec


Could we use "google,cros-ec-regulator" as compatible, so we are aligned with
the driver name and other cros-ec drivers? If you're agree you need to do
changes here, in the example and in the of table id.

In any case the file binding name should match the compatible name and that's
not the case now, as the file is google,cros-ec-regulator and the compatible is
google,regulator-cros-ec. So either fix the file binding name or the compatible
(I'd prefer the second).

Thanks,
Enric

> +
> + reg:
> + maxItems: 1
> + description: Identifier for the voltage regulator to ChromeOS EC.
> +
> +required:
> + - compatible
> + - reg
> +
> +examples:
> + - |
> + spi0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cros_ec: ec@0 {
> + compatible = "google,cros-ec-spi";
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + regulator@0 {
> + compatible = "google,regulator-cros-ec";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <3300000>;
> + reg = <0>;
> + };
> + };
> + };
> +...
>