Hi, Rob Ack is in below comments. Hi, Mark: Due to that already merged into your regulator for-next git, may I send the patch to fix Rob's comment? And I also found one line need to be added into rtmv20 probe phase. Please check below. /* * keep in shutdown mode to minimize the current consumption * and also mark regcache as dirty */ + regcache_cache_only(priv->regmap, true); regcache_mark_dirty(priv->regmap); gpiod_set_value(priv->enable_gpio, 0); Can I directly merge into one that includes Rob's comment and the above line to be added? Rob Herring <robh@xxxxxxxxxx> 於 2020年9月29日 週二 下午11:06寫道: > > On Mon, Sep 28, 2020 at 03:19:44PM +0800, cy_huang wrote: > > From: ChiYuan Huang <cy_huang@xxxxxxxxxxx> > > > > Add DT-binding document for Richtek RTMV20 > > > > Signed-off-by: ChiYuan Huang <cy_huang@xxxxxxxxxxx> > > --- > > .../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@xxxxxxxxxxx> > > + > > +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. Yes, you're righ. Sorry, I key in the yaml file, line by line. It's really a typo. > > > + 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. Ack, remove type ref for all properties that included unit suffix. > > This and all the following need a vendor prefix too. Is it okay to Add "richtek," prefix for all the properties except lsw regulator node? > > > + 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. > OK > > + 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. Yes, fix it right now. > > > + 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 > >