[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20191219111811.6e3m3qvuijfi6ew6@gilmour.lan>
Date: Thu, 19 Dec 2019 12:18:11 +0100
From: Maxime Ripard <mripard@...nel.org>
To: Saravanan Sekar <sravanhome@...il.com>
Cc: lgirdwood@...il.com, broonie@...nel.org, robh+dt@...nel.org,
mark.rutland@....com, heiko@...ech.de, shawnguo@...nel.org,
laurent.pinchart@...asonboard.com, icenowy@...c.io,
mchehab+samsung@...nel.org, davem@...emloft.net,
gregkh@...uxfoundation.org, Jonathan.Cameron@...wei.com,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 2/4] dt-bindings: regulator: add document bindings for
mpq7920
Hi,
On Thu, Dec 19, 2019 at 11:37:19AM +0100, Saravanan Sekar wrote:
> Add device tree binding information for mpq7920 regulator driver.
> Example bindings for mpq7920 are added.
>
> Signed-off-by: Saravanan Sekar <sravanhome@...il.com>
> ---
> .../bindings/regulator/mpq7920.yaml | 149 ++++++++++++++++++
> 1 file changed, 149 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/mpq7920.yaml
>
> diff --git a/Documentation/devicetree/bindings/regulator/mpq7920.yaml b/Documentation/devicetree/bindings/regulator/mpq7920.yaml
> new file mode 100644
> index 000000000000..79000b745cfd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/mpq7920.yaml
> @@ -0,0 +1,149 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/mpq7920.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Monolithic Power System MPQ7920 PMIC
> +
> +maintainers:
> + - Saravanan Sekar <sravanhome@...il.com>
> +
> +properties:
> + $nodename:
> + pattern: "mpq@[0-9a-f]{1,2}"
The node name is supposed to be the class of the device, so pmic or
regulator seems more suited here.
> + compatible:
> + enum:
> + - mps,mpq7920
> +
> + reg:
> + maxItems: 1
> +
> + regulators:
> + type: string
> + description: |
> + list of regulators provided by this controller, must be named
> + after their hardware counterparts BUCK[1-4], one LDORTC, and LDO[2-5]
> + The valid names for regulators are
> + buck1, buck2, buck3, buck4, ldortc, ldo2, ldo3, ldo4, ldo5
This should be an enum with the valid values
> +
> + properties:
> + mps,time-slot:
I'm not sure what this is supposed to be doing?
Is that another property in the regulator node, or regulators is
supposed to be a node?
> + - $ref: "/schemas/types.yaml#/definitions/uint8-array"
> + description: |
> + power on/off sequence time slot/interval must be one of following values
> + With:
> + * 0: 0.5ms
> + * 1: 2ms
> + * 2: 8ms
> + * 3: 16ms
> + Defaults to 0.5ms if not specified.
So it's not an array, but just a single value?
Either wai, the valid values should be an enum, and the default
specified using the default keyword.
> +
> + properties:
> + mps,fixed-on-time:
You don't need to set properties all the time.
> + - $ref: "/schemas/types.yaml#/definitions/boolean"
> + description: |
> + select power on sequence with fixed time interval mentioned in
> + time-slot reg for all the regulators.
Can't you just derive that from the fact that time-slot is present?
> + properties:
> + mps,fixed-off-time:
> + - $ref: "/schemas/types.yaml#/definitions/boolean"
> + description: |
> + select power off sequence with fixed time interval mentioned in
> + time-slot reg for all the regulators.
Same thing here
> + properties:
> + mps,inc-off-time:
> + - $ref: "/schemas/types.yaml#/definitions/uint8-array"
> + description: |
> + An array of 8, linearly increase power off sequence time
> + slot/interval for each regulator must be one of following values
> + * 0 to 15
This should be an enum, or a combination of minimum/maximum. And the
size of the array should be fixed too.
> + properties:
> + mps,inc-on-time:
> + - $ref: "/schemas/types.yaml#/definitions/uint8-array"
> + description: |
> + An array of 8, linearly increase power on sequence time
> + slot/interval for each regulator must be one of following values
> + * 0 to 15
Ditto,
> + properties:
> + mps,switch-freq:
> + - $ref: "/schemas/types.yaml#/definitions/uint8"
> + description: |
> + switching frequency must be one of following values
> + * 0 : 1.1MHz
> + * 1 : 1.65MHz
> + * 2 : 2.2MHz
> + * 3 : 2.75MHz
> + Defaults to 2.2 MHz if not specified.
enum and default
> + properties:
> + mps,buck-softstart:
> + - $ref: "/schemas/types.yaml#/definitions/uint8-array"
> + description: |
> + An array of 4 contains soft start time of each buck, must be one of
> + following values
> + * 0 : 150us
> + * 1 : 300us
> + * 2 : 610us
> + * 3 : 920us
> + Defaults to 300us if not specified.
Same story than mps,inc-off-time
> + properties:
> + mps,buck-ovp:
> + - $ref: "/schemas/types.yaml#/definitions/uint8-array"
> + description: |
> + An array of 4 contains over voltage protection of each buck, must be
> + one of following values
> + * 0 : disabled
> + * 1 : enabled
> + Defaults is enabled if not specified.
Ditto
> + properties:
> + mps,buck-phase-delay:
> + - $ref: "/schemas/types.yaml#/definitions/uint8-array"
> + description: |
> + An array of 4 contains each buck phase delay must be one of following
> + values
> + * 0: 0deg
> + * 1: 90deg
> + * 2: 180deg
> + * 3: 270deg
> + Defaults to 0deg for buck1 & buck2, 90deg for buck3 & buck4 if not
> + specified.
ditto
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + mpq7920@69 {
> + compatible = "mps,mpq7920";
> + reg = <0x69>;
> +
> + mps,switch-freq = <1>;
> + mps,buck-softstart = /bits/ 8 <1 2 1 3>;
> + mps,buck-ovp = /bits/ 8 <1 0 1 1>;
> +
> + regulators {
> + buck1 {
So regulators isn't a string after all?
If it's supposed to be a node, it should be type: object
and you can check that the node has a valid value using propertyNames
Maxime
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists