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

[Krzysztof Kozlowski <krzk@kernel.org>]

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.

> 
> 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.

> +  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.

> +        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.

> +
> +  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?

> +
> +required:
> +  - compatible
> +  - reg
> +  - regulators


Best regards,
Krzysztof