Re: [PATCH 1/2] dt-bindings: regulator: add max20339 binding

["André Draszik" <andre.draszik@linaro.org>]

Hi Krzysztof,

On Mon, 2024-09-16 at 21:56 +0200, Krzysztof Kozlowski wrote:
> On 16/09/2024 18:48, André Draszik wrote:
> > Add device tree binding for Maxim MAX20339 overvoltage protector with
> > load switches.
> 
> Subject:
> regulator: dt-bindings:
> 
> See submitting patches.

OK

> > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> > ---
> >  .../bindings/regulator/maxim,max20339.yaml         | 171 +++++++++++++++++++++
> >  MAINTAINERS                                        |   6 +
> >  2 files changed, 177 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/regulator/maxim,max20339.yaml
> > b/Documentation/devicetree/bindings/regulator/maxim,max20339.yaml
> > new file mode 100644
> > index 000000000000..ef6490cf5a6b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/regulator/maxim,max20339.yaml
> > @@ -0,0 +1,171 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/regulator/maxim,max20339.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Maxim Integrated MAX20339 overvoltage protector with load switches
> > +
> > +maintainers:
> > +  - André Draszik <andre.draszik@linaro.org>
> > +
> > +description: |
> 
> Do not need '|' unless you need to preserve formatting.

Will drop.

> > +  The MAX20339 is an overvoltage protection (OVP) device with integrated load
> > +  switches and ESD Protection for USB Type-C applications. It protects low
> > +  voltage systems against voltage faults and surges. It also integrates two
> > +  load switches with current limiting, configurable by hard- and software.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - maxim,max20339
> > +
> > +  dig-supply:
> > +    description:
> > +      Input supply for the device itself (DIG pin, 1.7V to 1.9V). In absence of
> > +      Vdig, the IN pin will power the device.
> > +
> > +  gpio:
> > +    type: object
> > +
> > +    properties:
> > +      "#gpio-cells":
> > +        const: 2
> > +
> > +      gpio-controller: true
> > +
> > +      gpio-line-names:
> > +        description: Strings describing the names of each gpio line.
> 
> Drop description.

OK.

> > +        maxItems: 1
> > +
> > +    required:
> > +      - "#gpio-cells"
> > +      - gpio-controller
> > +
> > +    additionalProperties: false
> 
> Does any existing driver require this to be separate node? I don't see
> here any resources, so these properties should be in main device node.

OK. There was no specific reason, other than visual separation, and easier
'required' handling; will update.

> > +
> > +  insw-supply:
> > +    description:
> > +      Input supply for the outputs (IN pin, 3.9V to 28.0V). Also used as supply
> > +      for the device itself in case dig-supply is not wired up.
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  lsw1-supply:
> > +    description: Input supply for the outputs (LSW1IN pin, 4.0V to 10.0V)
> > +
> > +  lsw2-supply:
> > +    description: Input supply for the outputs (LSW2IN pin, 4.0V to 10.0V)
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  regulators:
> > +    type: object
> > +
> > +    properties:
> > +      insw:
> > +        type: object
> > +        $ref: regulator.yaml#
> > +        description: Represents the switch connecting IN to OUT, INSW.
> > +
> > +        properties:
> > +          regulator-ov-protection-microvolt:
> > +            enum: [5850000, 9700000, 14500000]
> > +
> > +        unevaluatedProperties: false
> > +
> > +    patternProperties:
> > +      "^lsw[12]$":
> > +        type: object
> > +        $ref: regulator.yaml#
> > +        description: Represents the load switches LSW1 and LSW2.
> > +
> > +        properties:
> > +          regulator-ov-protection-microvolt:
> > +            enum: [0, 1]
> > +
> > +          shunt-resistor-micro-ohms:
> > +            description:
> > +              Value in micro Ohm of the resistor connected between the SETx
> > +              pin and GND. This value is required to be able to calculate and
> > +              program the current limiting threshold register.
> > +
> > +        required:
> > +          - shunt-resistor-micro-ohms
> > +
> > +        unevaluatedProperties: false
> > +
> > +    required:
> > +      - lsw1
> > +      - lsw2
> > +
> > +    additionalProperties: false
> > +
> > +anyOf:
> > +  - oneOf:
> > +      - required:
> > +          - dig-supply
> > +      - required:
> > +          - insw-supply
> > +  - required:
> > +      - dig-supply
> > +      - insw-supply
> 
> oneOf looks redundant here. you allow any combination, right?

You're right, and yes, I allow either dig-supply, or insw-supply, or both,
and at least one of them must be specified.

Thanks,
Andre'

> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - regulators
> 
> 
> Best regards,
> Krzysztof
>