lkml.org 
[lkml]   [2019]   [Dec]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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@gmail.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@gmail.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
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2019-12-19 12:18    [W:0.054 / U:1.900 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site