Re: [PATCH v2 2/2] regulator: rtmv20: Add DT-binding document for Richtek RTMV20

[Rob Herring <robh@kernel.org>]

On Mon, Sep 28, 2020 at 03:19:44PM +0800, cy_huang wrote:
> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> Add DT-binding document for Richtek RTMV20
> 
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> ---
>  .../regulator/richtek,rtmv20-regulator.yaml        | 168 +++++++++++++++++++++
>  1 file changed, 168 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml b/Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml
> new file mode 100644
> index 00000000..4cb4b68
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/richtek,rtmv20-regulator.yaml
> @@ -0,0 +1,168 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/richtek,rtmv20-regulator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Richtek RTMV20 laser diode regulator
> +
> +maintainers:
> +  - ChiYuan Huang <cy_huang@richtek.com>
> +
> +description: |
> +  Richtek RTMV20 is a load switch current regulator that can supply up to 6A.
> +  It is used to drive laser diode. There're two signals for chip controls
> +  (Enable/Fail), Enable pin to turn chip on, and Fail pin as fault indication.
> +  There're still four pins for camera control, two inputs (strobe and vsync),
> +  the others for outputs (fsin1 and fsin2). Strobe input to start the current
> +  supply, vsync input from IR camera, and fsin1/fsin2 output for the optional.
> +
> +properties:
> +  compatible:
> +    const: richtek,rtmv20
> +
> +  reg:
> +    maxItems: 1
> +
> +  wakeup-source: true
> +
> +  interrupts-extend:

You mean interrupts-extended?

In any case, use 'interrupts' here and the tooling allows for either.

> +    maxItems: 1
> +
> +  enable-gpios:
> +    description: A connection of the 'enable' gpio line.
> +    maxItems: 1
> +
> +  ld-pulse-delay-us:
> +    description: |
> +      load current pulse delay in microsecond after strobe pin pulse high.
> +    $ref: "/schemas/types.yaml#/definitions/uint32"

Don't need a type ref when you have a standard property unit suffix, so 
drop.

This and all the following need a vendor prefix too.

> +    minimum: 0
> +    maximum: 100000
> +    default: 0
> +
> +  ld-pulse-width-us:
> +    description: |
> +      Load current pulse width in microsecond after strobe pin pulse high.
> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +    minimum: 0
> +    maximum: 10000
> +    default: 1200
> +
> +  fsin1-delay-us:
> +    description: |
> +      Fsin1 pulse high delay in microsecond after vsync signal pulse high.
> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +    minimum: 0
> +    maximum: 100000
> +    default: 23000
> +
> +  fsin1-width-us:
> +    description: |
> +      Fsin1 pulse high width in microsecond after vsync signal pulse high.
> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +    minimum: 40
> +    maximum: 10000
> +    default: 160
> +
> +  fsin2-delay-us:
> +    description: |
> +      Fsin2 pulse high delay in microsecond after vsync signal pulse high.
> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +    minimum: 0
> +    maximum: 100000
> +    default: 23000
> +
> +  fsin2-width-us:
> +    description: |
> +      Fsin2 pulse high width in microsecond after vsync signal pulse high.
> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +    minimum: 40
> +    maximum: 10000
> +    default: 160
> +
> +  es-pulse-width-us:
> +    description: Eye safety function pulse width limit in microsecond.
> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +    minimum: 0
> +    maximum: 10000
> +    default: 1200
> +
> +  es-ld-current-microamp:
> +    description: Eye safety function load current limit in microamp.
> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +    minimum: 0
> +    maximum: 6000000
> +    default: 3000000
> +
> +  lbp-level-microvolt:
> +    description: Low battery protection level in microvolt.
> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +    minimum: 2400000
> +    maximum: 3700000
> +    default: 2700000
> +
> +  lbp-enable:
> +    description: Low battery protection function enable control.
> +    type: boolean
> +
> +  strobe-polarity-high:
> +    description: Strobe pin active polarity control.
> +    type: boolean
> +
> +  vsync-polarity-high:
> +    description: Vsync pin active polarity control.
> +    type: boolean
> +
> +  fsin-enable:
> +    description: Fsin function enable control.
> +    type: boolean
> +
> +  fsin-output:
> +    description: Fsin function output control.
> +    type: boolean
> +
> +  es-enable:
> +    description: Eye safety function enable control.
> +    type: boolean
> +
> +patternProperties:
> +  "lsw":

This matches ".*lsw.*". What you wanted? If just 'lsw', then it's not a 
pattern.

> +    type: object
> +    $ref: "regulator.yaml#"
> +
> +required:
> +  - compatible
> +  - reg
> +  - wakeup-source
> +  - interrupts-extend
> +  - enable-gpios
> +  - lsw
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      rtmv20@34 {
> +        compatible = "richtek,rtmv20";
> +        reg = <0x34>;
> +        wakeup-source;
> +        interrupts-extend = <&gpio26 2 IRQ_TYPE_LEVEL_LOW>;

2 wrongs make a right... But your driver interrupt probably doesn't work 
too well.

> +        enable-gpios = <&gpio26 3 0>;
> +
> +        strobe-polarity-high;
> +        vsync-polarity-high;
> +
> +        lsw {
> +                regulator-name = "rtmv20,lsw";
> +                regulator-min-microamp = <0>;
> +                regulator-max-microamp = <6000000>;
> +        };
> +      };
> +    };
> +...
> -- 
> 2.7.4
>