Re: [PATCH v2 05/11] regulator: dt-bindings: Add TI TPS65219 PMIC bindings

[Markus Schneider-Pargmann]

Hi Jerome,

On Tue, Jul 26, 2022 at 12:33:49PM +0200, Jerome Neanne wrote:
> Add TPS65219 PMIC bindings using json-schema.
> 
> Describe required properties and regname-supply.
> regname-supply is required when bypass mode is used for a regulator.
> Describes regulator topology.
> Interrupts support.
> Add a power-button property to configure the EN/PB/VSENSE pin as a
> powerbutton:
> 
> TPS65219 has a multipurpose pin called EN/PB/VSENSE that can be either:
> - EN in which case it functions as an enable pin.
> - VSENSE which compares the voltages and triggers an automatic
> on/off request.
> - PB in which case it can be configured to trigger an interrupt
> to the SoC.
> ti,power-button reflects the last one of those options
> where the board has a button wired to the pin and triggers
> an interrupt on pressing it.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@xxxxxxxxxxxx>
> Signed-off-by: Jerome Neanne <jneanne@xxxxxxxxxxxx>
> ---
>  .../bindings/regulator/ti,tps65219.yaml       | 164 ++++++++++++++++++
>  1 file changed, 164 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/ti,tps65219.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/ti,tps65219.yaml b/Documentation/devicetree/bindings/regulator/ti,tps65219.yaml
> new file mode 100644
> index 000000000000..8fca4db6c64c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/ti,tps65219.yaml

...

> +
> +  power-button:
> +    type: boolean
> +    description: Optional property that sets the EN/PB/VSENSE pin to be a
> +      power-button.

It seems something went wrong here. This should have been
ti,power-button and have a similar description as the commit message,
right?

Best,
Markus

> +
> +patternProperties:
> +  "^buck[1-3]-supply$":
> +    description: Input supply phandle of one regulator.
> +
> +  "^ldo[1-4]-supply$":
> +    description: Input supply phandle of one regulator.
> +
> +  regulators:
> +    type: object
> +    description: |
> +      list of regulators provided by this controller
> +
> +    patternProperties:
> +      "^ldo[1-4]$":
> +        type: object
> +        $ref: regulator.yaml#
> +        description:
> +          Properties for single LDO regulator.
> +
> +        unevaluatedProperties: false
> +
> +      "^buck[1-3]$":
> +        type: object
> +        $ref: regulator.yaml#
> +        description:
> +          Properties for single BUCK regulator.
> +
> +        unevaluatedProperties: false
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - regulators
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        tps65219: pmic@30 {
> +            compatible = "ti,tps65219";
> +            reg = <0x30>;
> +            buck1-supply = <&vcc_3v3_sys>;
> +            buck2-supply = <&vcc_3v3_sys>;
> +            buck3-supply = <&vcc_3v3_sys>;
> +            ldo1-supply = <&vcc_3v3_sys>;
> +            ldo2-supply = <&buck2_reg>;
> +            ldo3-supply = <&vcc_3v3_sys>;
> +            ldo4-supply = <&vcc_3v3_sys>;
> +
> +            pinctrl-0 = <&pmic_irq_pins_default>;
> +
> +            interrupt-parent = <&gic500>;
> +            interrupts = <GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-controller;
> +            #interrupt-cells = <1>;
> +
> +            regulators {
> +                buck1_reg: buck1 {
> +                    regulator-name = "VDD_CORE";
> +                    regulator-min-microvolt = <750000>;
> +                    regulator-max-microvolt = <750000>;
> +                    regulator-boot-on;
> +                    regulator-always-on;
> +                };
> +
> +                buck2_reg: buck2 {
> +                    regulator-name = "VCC1V8";
> +                    regulator-min-microvolt = <1800000>;
> +                    regulator-max-microvolt = <1800000>;
> +                    regulator-boot-on;
> +                    regulator-always-on;
> +                };
> +
> +                buck3_reg: buck3 {
> +                    regulator-name = "VDD_LPDDR4";
> +                    regulator-min-microvolt = <1100000>;
> +                    regulator-max-microvolt = <1100000>;
> +                    regulator-boot-on;
> +                    regulator-always-on;
> +                };
> +
> +                ldo1_reg: ldo1 {
> +                    regulator-name = "VDDSHV_SD_IO_PMIC";
> +                    regulator-min-microvolt = <33000000>;
> +                    regulator-max-microvolt = <33000000>;
> +                };
> +
> +                ldo2_reg: ldo2 {
> +                    regulator-name = "VDDAR_CORE";
> +                    regulator-min-microvolt = <850000>;
> +                    regulator-max-microvolt = <850000>;
> +                    regulator-boot-on;
> +                    regulator-always-on;
> +                };
> +
> +                ldo3_reg: ldo3 {
> +                    regulator-name = "VDDA_1V8";
> +                    regulator-min-microvolt = <18000000>;
> +                    regulator-max-microvolt = <18000000>;
> +                    regulator-boot-on;
> +                    regulator-always-on;
> +                };
> +
> +                ldo4_reg: ldo4 {
> +                    regulator-name = "VDD_PHY_2V5";
> +                    regulator-min-microvolt = <25000000>;
> +                    regulator-max-microvolt = <25000000>;
> +                    regulator-boot-on;
> +                    regulator-always-on;
> +                };
> +            };
> +        };
> +    };
> -- 
> 2.17.1
>