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

Rob Herring
2020-09-29T15:06:31Z

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
>