2020-01-09 12:37:16

by saravanan sekar

[permalink] [raw]
Subject: [PATCH v6 2/4] dt-bindings: regulator: add document bindings for mpq7920

Add device tree binding information for mpq7920 regulator driver.
Example bindings for mpq7920 are added.

Signed-off-by: Saravanan Sekar <[email protected]>
---
.../bindings/regulator/mps,mpq7920.yaml | 202 ++++++++++++++++++
1 file changed, 202 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/mps,mpq7920.yaml

diff --git a/Documentation/devicetree/bindings/regulator/mps,mpq7920.yaml b/Documentation/devicetree/bindings/regulator/mps,mpq7920.yaml
new file mode 100644
index 000000000000..598f3ea070c9
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/mps,mpq7920.yaml
@@ -0,0 +1,202 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/mps,mpq7920.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Monolithic Power System MPQ7920 PMIC
+
+maintainers:
+ - Saravanan Sekar <[email protected]>
+
+properties:
+ $nodename:
+ pattern: "pmic@[0-9a-f]{1,2}"
+ compatible:
+ enum:
+ - mps,mpq7920
+
+ reg:
+ maxItems: 1
+
+ regulators:
+ type: object
+ description: |
+ list of regulators provided by this controller, must be named
+ after their hardware counterparts BUCK[1-4], one LDORTC, and LDO[2-5]
+
+ mps,switch-freq:
+ description: |
+ switching frequency must be one of following corresponding value
+ 1.1MHz, 1.65MHz, 2.2MHz, 2.75MHz
+ $ref: "/schemas/types.yaml#/definitions/uint8"
+ enum: [ 0, 1, 2, 3 ]
+ default: 2
+
+ buck1:
+ type: object
+ $ref: "regulator.yaml#"
+ description: |
+ 4.5A DC-DC step down converter
+
+ mps,buck-softstart:
+ $ref: "/schemas/types.yaml#/definitions/uint8"
+ enum: [ 0, 1, 2, 3 ]
+ default: 1
+ description: |
+ defines the soft start time of this buck, must be one of the following
+ corresponding values 150us, 300us, 610us, 920us
+
+ mps,buck-phase-delay:
+ $ref: "/schemas/types.yaml#/definitions/uint8"
+ enum: [ 0, 1, 2, 3 ]
+ default: 0
+ description: |
+ defines the phase delay of this buck, must be one of the following
+ corresponding values 0deg, 90deg, 180deg, 270deg
+
+ mps,buck-ovp-disable:
+ type: boolean
+ description: |
+ disables over voltage protection of this buck
+
+ buck2:
+ type: object
+ $ref: "regulator.yaml#"
+ description: |
+ 2.5A DC-DC step down converter
+
+ mps,buck-softstart:
+ description: |
+ defines the soft start time of this buck, must be one of the following
+ corresponding values 150us, 300us, 610us, 920us
+ $ref: "/schemas/types.yaml#/definitions/uint8"
+ enum: [ 0, 1, 2, 3 ]
+ default: 1
+
+ mps,buck-phase-delay:
+ description: |
+ defines the phase delay of this buck, must be one of the following
+ corresponding values 0deg, 90deg, 180deg, 270deg
+ $ref: "/schemas/types.yaml#/definitions/uint8"
+ enum: [ 0, 1, 2, 3 ]
+ default: 0
+
+ mps,buck-ovp-disable:
+ description: |
+ disables over voltage protection of this buck
+ type: boolean
+
+ buck3:
+ type: object
+ $ref: "regulator.yaml#"
+ description: |
+ 4.5A DC-DC step down converter
+
+ mps,buck-softstart:
+ description: |
+ defines the soft start time of this buck, must be one of the following
+ corresponding values 150us, 300us, 610us, 920us
+ $ref: "/schemas/types.yaml#/definitions/uint8"
+ enum: [ 0, 1, 2, 3 ]
+ default: 1
+
+ mps,buck-phase-delay:
+ description: |
+ defines the phase delay of this buck, must be one of the following
+ corresponding values 0deg, 90deg, 180deg, 270deg
+ $ref: "/schemas/types.yaml#/definitions/uint8"
+ enum: [ 0, 1, 2, 3 ]
+ default: 1
+
+ mps,buck-ovp-disable:
+ description: |
+ disables over voltage protection of this buck
+ type: boolean
+
+ buck4:
+ type: object
+ $ref: "regulator.yaml#"
+ description: |
+ 2.5A DC-DC step down converter
+
+ mps,buck-softstart:
+ description: |
+ defines the soft start time of this buck, must be one of the following
+ corresponding values 150us, 300us, 610us, 920us
+ $ref: "/schemas/types.yaml#/definitions/uint8"
+ enum: [ 0, 1, 2, 3 ]
+ default: 1
+
+ mps,buck-phase-delay:
+ description: |
+ defines the phase delay of this buck, must be one of the following
+ corresponding values 0deg, 90deg, 180deg, 270deg
+ $ref: "/schemas/types.yaml#/definitions/uint8"
+ enum: [ 0, 1, 2, 3 ]
+ default: 1
+
+ mps,buck-ovp-disable:
+ description: |
+ disables over voltage protection of this buck
+ type: boolean
+
+ ldortc:
+ $ref: "regulator.yaml#"
+ description: |
+ regulator with 0.65V-3.5875V for RTC, always enabled
+
+ ldo2:
+ $ref: "regulator.yaml#"
+ description: |
+ regulator with 0.65V-3.5875V
+
+ ldo3:
+ $ref: "regulator.yaml#"
+ description: |
+ regulator with 0.65V-3.5875V
+
+ ldo4:
+ $ref: "regulator.yaml#"
+ description: |
+ regulator with 0.65V-3.5875V
+
+ ldo5:
+ $ref: "regulator.yaml#"
+ description: |
+ regulator with 0.65V-3.5875V
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pmic@69 {
+ compatible = "mps,mpq7920";
+ reg = <0x69>;
+
+ regulators {
+ mps,switch-freq = <1>;
+
+ buck1 {
+ regulator-name = "buck1";
+ regulator-min-microvolt = <400000>;
+ regulator-max-microvolt = <3587500>;
+ regulator-min-microamp = <460000>;
+ regulator-max-microamp = <7600000>;
+ regulator-boot-on;
+ mps,buck-ovp-disable;
+ mps,buck-phase-delay = <2>;
+ mps,buck-softstart = <1>;
+ };
+
+ ldo2 {
+ regulator-name = "ldo2";
+ regulator-min-microvolt = <650000>;
+ regulator-max-microvolt = <3587500>;
+ };
+ };
+ };
+ };
+...
--
2.17.1


2020-01-13 16:58:23

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v6 2/4] dt-bindings: regulator: add document bindings for mpq7920

On Thu, Jan 9, 2020 at 5:26 AM Saravanan Sekar <[email protected]> wrote:
>
> Add device tree binding information for mpq7920 regulator driver.
> Example bindings for mpq7920 are added.

Mark, Please revert this. Not even close to valid schema and my
questions on v4 are unanswered.

>
> Signed-off-by: Saravanan Sekar <[email protected]>
> ---
> .../bindings/regulator/mps,mpq7920.yaml | 202 ++++++++++++++++++
> 1 file changed, 202 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/mps,mpq7920.yaml
>
> diff --git a/Documentation/devicetree/bindings/regulator/mps,mpq7920.yaml b/Documentation/devicetree/bindings/regulator/mps,mpq7920.yaml
> new file mode 100644
> index 000000000000..598f3ea070c9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/mps,mpq7920.yaml
> @@ -0,0 +1,202 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/mps,mpq7920.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Monolithic Power System MPQ7920 PMIC
> +
> +maintainers:
> + - Saravanan Sekar <[email protected]>
> +
> +properties:
> + $nodename:
> + pattern: "pmic@[0-9a-f]{1,2}"
> + compatible:
> + enum:
> + - mps,mpq7920
> +
> + reg:
> + maxItems: 1
> +
> + regulators:
> + type: object
> + description: |
> + list of regulators provided by this controller, must be named
> + after their hardware counterparts BUCK[1-4], one LDORTC, and LDO[2-5]
> +
> + mps,switch-freq:

This needs to be under 'properties'. Otherwise, everything below here
is just part of 'description'.

As I asked on v4, shouldn't this be a common property? Switching
frequency is a common property for switching regulators, right?

> + description: |
> + switching frequency must be one of following corresponding value
> + 1.1MHz, 1.65MHz, 2.2MHz, 2.75MHz
> + $ref: "/schemas/types.yaml#/definitions/uint8"
> + enum: [ 0, 1, 2, 3 ]
> + default: 2
> +
> + buck1:

This should be under 'patternProperties' as '^buck[1-4]$'. Then you
aren't duplicating a bunch of property schemas.

> + type: object
> + $ref: "regulator.yaml#"

Should be:

allOf:
- $ref: regulator.yaml#

> + description: |
> + 4.5A DC-DC step down converter
> +
> + mps,buck-softstart:

Needs to be under 'properties'

> + $ref: "/schemas/types.yaml#/definitions/uint8"
> + enum: [ 0, 1, 2, 3 ]
> + default: 1
> + description: |
> + defines the soft start time of this buck, must be one of the following
> + corresponding values 150us, 300us, 610us, 920us
> +
> + mps,buck-phase-delay:
> + $ref: "/schemas/types.yaml#/definitions/uint8"
> + enum: [ 0, 1, 2, 3 ]
> + default: 0
> + description: |
> + defines the phase delay of this buck, must be one of the following
> + corresponding values 0deg, 90deg, 180deg, 270deg
> +
> + mps,buck-ovp-disable:
> + type: boolean
> + description: |
> + disables over voltage protection of this buck

Seems like configurable over voltage protection would be a common
regulator property?

> +
> + buck2:
> + type: object
> + $ref: "regulator.yaml#"
> + description: |
> + 2.5A DC-DC step down converter
> +
> + mps,buck-softstart:
> + description: |
> + defines the soft start time of this buck, must be one of the following
> + corresponding values 150us, 300us, 610us, 920us
> + $ref: "/schemas/types.yaml#/definitions/uint8"
> + enum: [ 0, 1, 2, 3 ]
> + default: 1
> +
> + mps,buck-phase-delay:
> + description: |
> + defines the phase delay of this buck, must be one of the following
> + corresponding values 0deg, 90deg, 180deg, 270deg
> + $ref: "/schemas/types.yaml#/definitions/uint8"
> + enum: [ 0, 1, 2, 3 ]
> + default: 0
> +
> + mps,buck-ovp-disable:
> + description: |
> + disables over voltage protection of this buck
> + type: boolean
> +
> + buck3:
> + type: object
> + $ref: "regulator.yaml#"
> + description: |
> + 4.5A DC-DC step down converter
> +
> + mps,buck-softstart:
> + description: |
> + defines the soft start time of this buck, must be one of the following
> + corresponding values 150us, 300us, 610us, 920us
> + $ref: "/schemas/types.yaml#/definitions/uint8"
> + enum: [ 0, 1, 2, 3 ]
> + default: 1
> +
> + mps,buck-phase-delay:
> + description: |
> + defines the phase delay of this buck, must be one of the following
> + corresponding values 0deg, 90deg, 180deg, 270deg
> + $ref: "/schemas/types.yaml#/definitions/uint8"
> + enum: [ 0, 1, 2, 3 ]
> + default: 1
> +
> + mps,buck-ovp-disable:
> + description: |
> + disables over voltage protection of this buck
> + type: boolean
> +
> + buck4:
> + type: object
> + $ref: "regulator.yaml#"
> + description: |
> + 2.5A DC-DC step down converter
> +
> + mps,buck-softstart:
> + description: |
> + defines the soft start time of this buck, must be one of the following
> + corresponding values 150us, 300us, 610us, 920us
> + $ref: "/schemas/types.yaml#/definitions/uint8"
> + enum: [ 0, 1, 2, 3 ]
> + default: 1
> +
> + mps,buck-phase-delay:
> + description: |
> + defines the phase delay of this buck, must be one of the following
> + corresponding values 0deg, 90deg, 180deg, 270deg
> + $ref: "/schemas/types.yaml#/definitions/uint8"
> + enum: [ 0, 1, 2, 3 ]
> + default: 1
> +
> + mps,buck-ovp-disable:
> + description: |
> + disables over voltage protection of this buck
> + type: boolean
> +
> + ldortc:

Again, make this a pattern.

> + $ref: "regulator.yaml#"
> + description: |
> + regulator with 0.65V-3.5875V for RTC, always enabled
> +
> + ldo2:
> + $ref: "regulator.yaml#"
> + description: |
> + regulator with 0.65V-3.5875V
> +
> + ldo3:
> + $ref: "regulator.yaml#"
> + description: |
> + regulator with 0.65V-3.5875V
> +
> + ldo4:
> + $ref: "regulator.yaml#"
> + description: |
> + regulator with 0.65V-3.5875V
> +
> + ldo5:
> + $ref: "regulator.yaml#"
> + description: |
> + regulator with 0.65V-3.5875V

You need 'required' here listing compatible, reg, and regulators.

And then 'additionalProperties: false'

> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pmic@69 {
> + compatible = "mps,mpq7920";
> + reg = <0x69>;
> +
> + regulators {
> + mps,switch-freq = <1>;
> +
> + buck1 {
> + regulator-name = "buck1";
> + regulator-min-microvolt = <400000>;
> + regulator-max-microvolt = <3587500>;
> + regulator-min-microamp = <460000>;
> + regulator-max-microamp = <7600000>;
> + regulator-boot-on;
> + mps,buck-ovp-disable;
> + mps,buck-phase-delay = <2>;
> + mps,buck-softstart = <1>;
> + };
> +
> + ldo2 {
> + regulator-name = "ldo2";
> + regulator-min-microvolt = <650000>;
> + regulator-max-microvolt = <3587500>;
> + };
> + };
> + };
> + };
> +...
> --
> 2.17.1
>

2020-01-14 12:34:41

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v6 2/4] dt-bindings: regulator: add document bindings for mpq7920

On Mon, Jan 13, 2020 at 10:56:55AM -0600, Rob Herring wrote:
> On Thu, Jan 9, 2020 at 5:26 AM Saravanan Sekar <[email protected]> wrote:

> > Add device tree binding information for mpq7920 regulator driver.
> > Example bindings for mpq7920 are added.

> Mark, Please revert this. Not even close to valid schema and my
> questions on v4 are unanswered.

OK... I didn't see any comments on V5.

> > + mps,switch-freq:

> As I asked on v4, shouldn't this be a common property? Switching
> frequency is a common property for switching regulators, right?

It's not very common and where it's offered the valid values are
generally highly constrained so I'm not sure it makes sense to do
that, any device that does have this feature will want to specify
the valid values for that particular device.


Attachments:
(No filename) (823.00 B)
signature.asc (499.00 B)
Download all attachments