mbox series

[v3,0/4] mfd: lp87565: convert DT to yaml, ignore ENx pins and add LP87524-Q1

Message ID 20200622204329.11147-1-luca@lucaceresoli.net
Headers show
Series mfd: lp87565: convert DT to yaml, ignore ENx pins and add LP87524-Q1 | expand

Message

Luca Ceresoli June 22, 2020, 8:43 p.m. UTC
Hi,

the first patch in this series is a small but significant variation in how
the lp87565 driver enables the output rails, to allow the kernel to always
know when it is enabling an output. However it can change existing
behaviour (depending on the hardware setup) and thus it should be carefully
evaluated.

The following patches are a fairly straightforward addition of a new chip
variant along DT bindings conversion to yaml.

v3 fixes the yaml errors present in v2.

RFC,v1: https://lkml.org/lkml/2020/6/3/908
v2: https://lkml.org/lkml/2020/6/17/492

Luca

Luca Ceresoli (4):
  regulator: lp87565: enable voltage regardless of ENx pin
  dt-bindings: mfd: lp87565: convert to yaml
  dt-bindings: mfd: lp87565: add LP87524-Q1 variant
  mfd: lp87565: add LP87524-Q1 variant

 .../devicetree/bindings/mfd/lp87565.txt       |  79 ------
 .../devicetree/bindings/mfd/ti,lp875xx.yaml   | 225 ++++++++++++++++++
 drivers/mfd/lp87565.c                         |   4 +
 drivers/regulator/lp87565-regulator.c         |  21 +-
 include/linux/mfd/lp87565.h                   |   1 +
 5 files changed, 249 insertions(+), 81 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mfd/lp87565.txt
 create mode 100644 Documentation/devicetree/bindings/mfd/ti,lp875xx.yaml

Comments

Rob Herring July 13, 2020, 7:01 p.m. UTC | #1
On Mon, Jun 22, 2020 at 10:43:27PM +0200, Luca Ceresoli wrote:
> The definition of "xxx-in-supply" was generic, thus define in detail the

> possible cases for each chip variant.

> 

> Also document that the only possible I2C slave address is 0x60 as per the

> datasheet and fix the second example accordingly.

> 

> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>

> 

> ---

> 

> Changes in v3:

>  - fix yaml errors

> 

> Changes in v2:

>  - this patch replaces patch "regulator: lp87565: dt: remove duplicated

>    section" in RFC,v1 (Rob Herring)

>  - use capital letters consistently (Lee Jones)

>  - replace "regulator" -> "mfd" in subject line (Lee Jones)

>  - replace "dt:" suffix with "dt-bindings:" prefix in subject line

> ---

>  .../devicetree/bindings/mfd/lp87565.txt       |  79 ----------

>  .../devicetree/bindings/mfd/ti,lp875xx.yaml   | 142 ++++++++++++++++++

>  2 files changed, 142 insertions(+), 79 deletions(-)

>  delete mode 100644 Documentation/devicetree/bindings/mfd/lp87565.txt

>  create mode 100644 Documentation/devicetree/bindings/mfd/ti,lp875xx.yaml

> 

> diff --git a/Documentation/devicetree/bindings/mfd/lp87565.txt b/Documentation/devicetree/bindings/mfd/lp87565.txt

> deleted file mode 100644

> index 41671e0dc26b..000000000000

> --- a/Documentation/devicetree/bindings/mfd/lp87565.txt

> +++ /dev/null

> @@ -1,79 +0,0 @@

> -TI LP87565 PMIC MFD driver

> -

> -Required properties:

> -  - compatible:	"ti,lp87565", "ti,lp87565-q1"

> -  - reg:		I2C slave address.

> -  - gpio-controller:	Marks the device node as a GPIO Controller.

> -  - #gpio-cells:	Should be two.  The first cell is the pin number and

> -			the second cell is used to specify flags.

> -			See ../gpio/gpio.txt for more information.

> -  - xxx-in-supply:	Phandle to parent supply node of each regulator

> -			populated under regulators node. xxx should match

> -			the supply_name populated in driver.

> -Example:

> -

> -lp87565_pmic: pmic@60 {

> -	compatible = "ti,lp87565-q1";

> -	reg = <0x60>;

> -	gpio-controller;

> -	#gpio-cells = <2>;

> -

> -	buck10-in-supply = <&vsys_3v3>;

> -	buck23-in-supply = <&vsys_3v3>;

> -

> -	regulators: regulators {

> -		buck10_reg: buck10 {

> -			/* VDD_MPU */

> -			regulator-name = "buck10";

> -			regulator-min-microvolt = <850000>;

> -			regulator-max-microvolt = <1250000>;

> -			regulator-always-on;

> -			regulator-boot-on;

> -		};

> -

> -		buck23_reg: buck23 {

> -			/* VDD_GPU */

> -			regulator-name = "buck23";

> -			regulator-min-microvolt = <850000>;

> -			regulator-max-microvolt = <1250000>;

> -			regulator-boot-on;

> -			regulator-always-on;

> -		};

> -	};

> -};

> -

> -TI LP87561 PMIC:

> -

> -This is a single output 4-phase regulator configuration

> -

> -Required properties:

> -  - compatible:	"ti,lp87561-q1"

> -  - reg:		I2C slave address.

> -  - gpio-controller:	Marks the device node as a GPIO Controller.

> -  - #gpio-cells:	Should be two.  The first cell is the pin number and

> -			the second cell is used to specify flags.

> -			See ../gpio/gpio.txt for more information.

> -  - xxx-in-supply:	Phandle to parent supply node of each regulator

> -			populated under regulators node. xxx should match

> -			the supply_name populated in driver.

> -Example:

> -

> -lp87561_pmic: pmic@62 {

> -	compatible = "ti,lp87561-q1";

> -	reg = <0x62>;

> -	gpio-controller;

> -	#gpio-cells = <2>;

> -

> -	buck3210-in-supply = <&vsys_3v3>;

> -

> -	regulators: regulators {

> -		buck3210_reg: buck3210 {

> -			/* VDD_CORE */

> -			regulator-name = "buck3210";

> -			regulator-min-microvolt = <800000>;

> -			regulator-max-microvolt = <800000>;

> -			regulator-always-on;

> -			regulator-boot-on;

> -		};

> -	};

> -};

> diff --git a/Documentation/devicetree/bindings/mfd/ti,lp875xx.yaml b/Documentation/devicetree/bindings/mfd/ti,lp875xx.yaml

> new file mode 100644

> index 000000000000..2da703918d6a

> --- /dev/null

> +++ b/Documentation/devicetree/bindings/mfd/ti,lp875xx.yaml

> @@ -0,0 +1,142 @@

> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2

> +---

> +$id: http://devicetree.org/schemas/mfd/ti,lp875xx.yaml#

> +$schema: http://devicetree.org/meta-schemas/core.yaml#

> +

> +title: TI LP875xx PMIC MFD driver

> +

> +maintainers:

> +  - Keerthy <j-keerthy@ti.com>

> +

> +properties:

> +  compatible:

> +    oneOf:

> +      - const: ti,lp87565

> +      - const: ti,lp87565-q1

> +      - const: ti,lp87561-q1

> +

> +  reg:

> +    description: I2C slave address

> +    const: 0x60

> +

> +  gpio-controller: true

> +

> +  '#gpio-cells':

> +    description:

> +      The first cell is the pin number.

> +      The second cell is is used to specify flags.

> +      See ../gpio/gpio.txt for more information.

> +    const: 2

> +

> +required:

> +  - compatible

> +  - reg

> +  - gpio-controller

> +  - '#gpio-cells'

> +

> +allOf:

> +  - if:

> +      properties:

> +        compatible:

> +          contains:

> +            enum:

> +              - ti,lp87565

> +              - ti,lp87565-q1

> +    then:

> +      properties:

> +        buck10-in-supply:

> +          description:

> +            Phandle to parent supply node for BUCK0 and BUCK1 converters.

> +

> +        buck23-in-supply:

> +          description:

> +            Phandle to parent supply node for BUCK2 and BUCK3 converters.

> +

> +      required:

> +        - buck10-in-supply

> +        - buck23-in-supply

> +

> +  - if:

> +      properties:

> +        compatible:

> +          contains:

> +            enum:

> +              - ti,lp87561-q1

> +    then:

> +      properties:

> +        buck3210-in-supply:

> +          description:

> +            Phandle to parent supply node for all the four BUCK converters.

> +

> +      required:

> +        - buck3210-in-supply


I think this should probably be 2 schema documents (and a 3rd for the 
next patch)...

> +

> +examples:

> +  - |

> +    /* TI LP87565-Q1 PMIC (dual 2-phase output configuration) */

> +    i2c@0 {

> +        reg = <0x0 0x100>;

> +        #address-cells = <1>;

> +        #size-cells = <0>;

> +

> +        pmic@60 {

> +            compatible = "ti,lp87565-q1";

> +            reg = <0x60>;

> +            gpio-controller;

> +            #gpio-cells = <2>;

> +

> +            buck10-in-supply = <&vsys_3v3>;

> +            buck23-in-supply = <&vsys_3v3>;

> +

> +            regulators {

> +                buck10_reg: buck10 {


The regulators also need to be documented.

> +                    /* VDD_MPU */

> +                    regulator-name = "buck10";

> +                    regulator-min-microvolt = <850000>;

> +                    regulator-max-microvolt = <1250000>;

> +                    regulator-always-on;

> +                    regulator-boot-on;

> +                };

> +

> +                buck23_reg: buck23 {

> +                    /* VDD_GPU */

> +                    regulator-name = "buck23";

> +                    regulator-min-microvolt = <850000>;

> +                    regulator-max-microvolt = <1250000>;

> +                    regulator-boot-on;

> +                    regulator-always-on;

> +                };

> +            };

> +        };

> +    };

> +

> +  - |

> +    /* TI LP87561 PMIC (single 4-phase output configuration) */

> +    i2c@0 {

> +        reg = <0x0 0x100>;

> +        #address-cells = <1>;

> +        #size-cells = <0>;

> +

> +        pmic@60 {

> +            compatible = "ti,lp87561-q1";

> +            reg = <0x60>;

> +            gpio-controller;

> +            #gpio-cells = <2>;

> +

> +            buck3210-in-supply = <&vsys_3v3>;

> +

> +            regulators {

> +                buck3210_reg: buck3210 {

> +                    /* VDD_CORE */

> +                    regulator-name = "buck3210";

> +                    regulator-min-microvolt = <800000>;

> +                    regulator-max-microvolt = <800000>;

> +                    regulator-always-on;

> +                    regulator-boot-on;

> +                };

> +            };

> +        };

> +    };

> +

> +...

> -- 

> 2.27.0

>
Luca Ceresoli July 14, 2020, 9:26 a.m. UTC | #2
Hi Rob,

On 13/07/20 21:01, Rob Herring wrote:
> On Mon, Jun 22, 2020 at 10:43:27PM +0200, Luca Ceresoli wrote:

>> The definition of "xxx-in-supply" was generic, thus define in detail the

>> possible cases for each chip variant.

>>

>> Also document that the only possible I2C slave address is 0x60 as per the

>> datasheet and fix the second example accordingly.

>>

>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>


[...]

>> +allOf:

>> +  - if:

>> +      properties:

>> +        compatible:

>> +          contains:

>> +            enum:

>> +              - ti,lp87565

>> +              - ti,lp87565-q1

>> +    then:

>> +      properties:

>> +        buck10-in-supply:

>> +          description:

>> +            Phandle to parent supply node for BUCK0 and BUCK1 converters.

>> +

>> +        buck23-in-supply:

>> +          description:

>> +            Phandle to parent supply node for BUCK2 and BUCK3 converters.

>> +

>> +      required:

>> +        - buck10-in-supply

>> +        - buck23-in-supply

>> +

>> +  - if:

>> +      properties:

>> +        compatible:

>> +          contains:

>> +            enum:

>> +              - ti,lp87561-q1

>> +    then:

>> +      properties:

>> +        buck3210-in-supply:

>> +          description:

>> +            Phandle to parent supply node for all the four BUCK converters.

>> +

>> +      required:

>> +        - buck3210-in-supply

> 

> I think this should probably be 2 schema documents (and a 3rd for the 

> next patch)...


No problem, will do.

Is there a specific rule to know when to split, or is it just when the
amount of changes is relevant?

>> +

>> +examples:

>> +  - |

>> +    /* TI LP87565-Q1 PMIC (dual 2-phase output configuration) */

>> +    i2c@0 {

>> +        reg = <0x0 0x100>;

>> +        #address-cells = <1>;

>> +        #size-cells = <0>;

>> +

>> +        pmic@60 {

>> +            compatible = "ti,lp87565-q1";

>> +            reg = <0x60>;

>> +            gpio-controller;

>> +            #gpio-cells = <2>;

>> +

>> +            buck10-in-supply = <&vsys_3v3>;

>> +            buck23-in-supply = <&vsys_3v3>;

>> +

>> +            regulators {

>> +                buck10_reg: buck10 {

> 

> The regulators also need to be documented.


OK.

-- 
Luca