diff mbox series

[1/9] regulator: Update DA9121 dt-bindings

Message ID a5a57b416a47c044797d9b669c7e021acd69abae.1605868780.git.Adam.Ward.opensource@diasemi.com
State Superseded, archived
Headers show
Series [1/9] regulator: Update DA9121 dt-bindings | expand

Checks

Context Check Description
robh/checkpatch warning total: 0 errors, 2 warnings, 237 lines checked

Commit Message

Adam Ward Nov. 20, 2020, 12:14 p.m. UTC 
Update bindings for the Dialog Semiconductor DA9121 voltage regulator to add device variants.

Signed-off-by: Adam Ward <Adam.Ward.opensource@diasemi.com>
---
 .../devicetree/bindings/regulator/dlg,da9121.yaml  | 177 +++++++++++++++++++--
 MAINTAINERS                                        |   2 +
 .../dt-bindings/regulator/dlg,da9121-regulator.h   |  22 +++
 3 files changed, 185 insertions(+), 16 deletions(-)
 create mode 100644 include/dt-bindings/regulator/dlg,da9121-regulator.h

Comments

Vincent Whitchurch Nov. 20, 2020, 1:47 p.m. UTC | #1 
On Fri, Nov 20, 2020 at 01:14:50PM +0100, Adam Ward wrote:
> Update bindings for the Dialog Semiconductor DA9121 voltage regulator to add device variants.
> 
> Signed-off-by: Adam Ward <Adam.Ward.opensource@diasemi.com>
> ---
>  .../devicetree/bindings/regulator/dlg,da9121.yaml  | 177 +++++++++++++++++++--
>  MAINTAINERS                                        |   2 +
>  .../dt-bindings/regulator/dlg,da9121-regulator.h   |  22 +++
>  3 files changed, 185 insertions(+), 16 deletions(-)
>  create mode 100644 include/dt-bindings/regulator/dlg,da9121-regulator.h
> 
> diff --git a/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml b/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml
> index cde0d82..1bd177d 100644
> --- a/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml
> +++ b/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml
> @@ -8,40 +8,185 @@ title: Dialog Semiconductor DA9121 voltage regulator
>  
>  maintainers:
>    - Vincent Whitchurch <vincent.whitchurch@axis.com>
> +  - Adam Ward <adam.ward@disasemi.com>

I'm quite happy to have myself removed from the list instead.  You are
in a much better position to maintain the bindings for these chips.

> -  buck1:
> -    description:
> -      Initial data for the Buck1 regulator.
> -    $ref: "regulator.yaml#"
> +  interrupt-parent:
> +    maxItems: 1
> +    description: Specifies the reference to the interrupt controller.
> +
> +  interrupts:
> +    maxItems: 1
> +    description: IRQ line information.
> +
> +  dlg,irq-polling-delay-passive:
> +    maxItems: 1
> +    description: |
> +      Specify the polling period, measured in milliseconds, between interrupt status
> +      update checks. Range 1000-10000 ms.
> +
> +  regulators:
>      type: object
> +    $ref: regulator.yaml#
> +    description: |
> +      This node defines the settings for the BUCK. The content of the
> +      sub-node is defined by the standard binding for regulators; see regulator.yaml.
> +      The DA9121 regulator is bound using their names listed below
> +      buck1 - BUCK1
> +      buck2 - BUCK2       //DA9122, DA9220, DA9131, DA9132 only

This move to a sub-node means that older devicetrees won't work. I
assume that's fine since the driver is only in linux-next at the moment,
but perhaps it's worth mentioning this in the commit message?

> +
> +    patternProperties:
> +      "^buck([0-1])$":
> +        type: object
> +        $ref: regulator.yaml#
> +
> +    properties:
> +      regulator-mode:
> +        maxItems: 1
> +        description: Defined in include/dt-bindings/regulator/dlg,da9121-regulator.h
> +
> +      regulator-initial-mode:
> +        maxItems: 1
> +        description: Defined in include/dt-bindings/regulator/dlg,da9121-regulator.h
>  
> -unevaluatedProperties: false

I think my s/unevaluatedProperties/additionalProperties/ fix is already
in linux-next, so this looks like it needs rebasing.
Vincent Whitchurch Nov. 25, 2020, 9:21 a.m. UTC | #2 
On Fri, Nov 20, 2020 at 02:47:42PM +0100, Vincent Whitchurch wrote:
> On Fri, Nov 20, 2020 at 01:14:50PM +0100, Adam Ward wrote:
> > -  buck1:
> > -    description:
> > -      Initial data for the Buck1 regulator.
> > -    $ref: "regulator.yaml#"
> > +  interrupt-parent:
> > +    maxItems: 1
> > +    description: Specifies the reference to the interrupt controller.
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +    description: IRQ line information.
> > +
> > +  dlg,irq-polling-delay-passive:
> > +    maxItems: 1
> > +    description: |
> > +      Specify the polling period, measured in milliseconds, between interrupt status
> > +      update checks. Range 1000-10000 ms.
> > +
> > +  regulators:
> >      type: object
> > +    $ref: regulator.yaml#
> > +    description: |
> > +      This node defines the settings for the BUCK. The content of the
> > +      sub-node is defined by the standard binding for regulators; see regulator.yaml.
> > +      The DA9121 regulator is bound using their names listed below
> > +      buck1 - BUCK1
> > +      buck2 - BUCK2       //DA9122, DA9220, DA9131, DA9132 only
> 
> This move to a sub-node means that older devicetrees won't work. I
> assume that's fine since the driver is only in linux-next at the moment,
> but perhaps it's worth mentioning this in the commit message?

Actually, perhaps I'm missing something, but I don't quite see why this
move to a sub-node is needed.  There is some flexibility in the
regulator framework for this as I noted earlier
(https://lore.kernel.org/lkml/20201102154848.tm5nsydaukyd7rrw@axis.com/).
For the case of an MFD it certainly makes sense to have a "regulators"
sub-node but for these chips it seems rather redundant.

Also, perhaps you could split this patch into logical pieces too as Mark
has suggested for some of the other patches in this series?
Adam Ward Nov. 27, 2020, 1:01 p.m. UTC | #3 
> On Fri, Nov 20, 2020 at 02:47:42PM +0100, Vincent Whitchurch wrote:
> > On Fri, Nov 20, 2020 at 01:14:50PM +0100, Adam Ward wrote:
> Actually, perhaps I'm missing something, but I don't quite see why this
> move to a sub-node is needed.  There is some flexibility in the
> regulator framework for this as I noted earlier
> (https://lore.kernel.org/lkml/20201102154848.tm5nsydaukyd7rrw@axis.com/).
> For the case of an MFD it certainly makes sense to have a "regulators"
> sub-node but for these chips it seems rather redundant.

This sub-node looks fairly well instituted for devices with multiple regulators.
There's also the possibility to add GPIO support into another sub-node for all the variants.

Mark, do you have a preference?
Mark Brown Nov. 27, 2020, 2:59 p.m. UTC | #4 
On Fri, Nov 27, 2020 at 01:01:24PM +0000, Adam Ward wrote:

> This sub-node looks fairly well instituted for devices with multiple regulators.
> There's also the possibility to add GPIO support into another sub-node for all the variants.

> Mark, do you have a preference?

Not really, either is fine.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml b/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml
index cde0d82..1bd177d 100644
--- a/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml
+++ b/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml
@@ -8,40 +8,185 @@  title: Dialog Semiconductor DA9121 voltage regulator
 
 maintainers:
   - Vincent Whitchurch <vincent.whitchurch@axis.com>
+  - Adam Ward <adam.ward@disasemi.com>
+
+description: |
+  Dialog Semiconductor DA9121 Single-channel 10A double-phase buck converter
+  Dialog Semiconductor DA9122 Double-channel  5A single-phase buck converter
+  Dialog Semiconductor DA9220 Double-channel  3A single-phase buck converter
+  Dialog Semiconductor DA9217 Single-channel  6A double-phase buck converter
+  Dialog Semiconductor DA9130 Single-channel 10A double-phase buck converter
+  Dialog Semiconductor DA9131 Double-channel  5A single-phase buck converter
+  Dialog Semiconductor DA9132 Double-channel  3A single-phase buck converter
+
+  Current limits
+
+  This is PER PHASE, and the current limit setting in the devices reflect
+  that with a maximum 10A limit. Allowing for transients at/near double
+  the rated current, this translates across the device range to per
+  channel figures as so...
+
+                               | DA9121    DA9122     DA9220    DA9217   DA9140
+                               | /DA9130   /DA9131    /DA9132
+    -----------------------------------------------------------------------------
+    Output current / channel   | 10000000   5000000   3000000   6000000  40000000
+    Output current / phase     |  5000000   5000000   3000000   3000000   9500000
+    -----------------------------------------------------------------------------
+    Min regulator-min-microvolt|   300000    300000    300000    300000    500000
+    Max regulator-max-microvolt|  1900000   1900000   1900000   1900000   1000000
+    Device hardware default    |  1000000   1000000   1000000   1000000   1000000
+    -----------------------------------------------------------------------------
+    Min regulator-min-microamp |  7000000   3500000   3500000   7000000  26000000
+    Max regulator-max-microamp | 20000000  10000000   6000000  12000000  78000000
+    Device hardware default    | 15000000   7500000   5500000  11000000  58000000
 
 properties:
+  $nodename:
+    pattern: "pmic@[0-9a-f]{1,2}"
   compatible:
-    const: dlg,da9121
+    enum:
+      - dlg,da9121
+      - dlg,da9122
+      - dlg,da9220
+      - dlg,da9217
+      - dlg,da9130
+      - dlg,da9131
+      - dlg,da9132
+      - dlg,da9140
 
   reg:
     maxItems: 1
+    description: Specifies the I2C slave address.
 
-  buck1:
-    description:
-      Initial data for the Buck1 regulator.
-    $ref: "regulator.yaml#"
+  interrupt-parent:
+    maxItems: 1
+    description: Specifies the reference to the interrupt controller.
+
+  interrupts:
+    maxItems: 1
+    description: IRQ line information.
+
+  dlg,irq-polling-delay-passive:
+    maxItems: 1
+    description: |
+      Specify the polling period, measured in milliseconds, between interrupt status
+      update checks. Range 1000-10000 ms.
+
+  regulators:
     type: object
+    $ref: regulator.yaml#
+    description: |
+      This node defines the settings for the BUCK. The content of the
+      sub-node is defined by the standard binding for regulators; see regulator.yaml.
+      The DA9121 regulator is bound using their names listed below
+      buck1 - BUCK1
+      buck2 - BUCK2       //DA9122, DA9220, DA9131, DA9132 only
+
+    patternProperties:
+      "^buck([0-1])$":
+        type: object
+        $ref: regulator.yaml#
+
+    properties:
+      regulator-mode:
+        maxItems: 1
+        description: Defined in include/dt-bindings/regulator/dlg,da9121-regulator.h
+
+      regulator-initial-mode:
+        maxItems: 1
+        description: Defined in include/dt-bindings/regulator/dlg,da9121-regulator.h
 
-unevaluatedProperties: false
+      enable-gpios:
+        maxItems: 1
+        description: Specify a valid GPIO for platform control of the regulator
+
+      dlg,ripple-cancel:
+        maxItems: 1
+        description: |
+          Defined in include/dt-bindings/regulator/dlg,da9121-regulator.h
+          Only present on multi-channel devices (DA9122, DA9220, DA9131, DA9132)
+
+    additionalProperties: false
 
 required:
   - compatible
   - reg
+  - regulators
+
+additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/regulator/dlg,da9121-regulator.h>
     i2c {
-      #address-cells = <1>;
-      #size-cells = <0>;
-      regulator@68 {
-        compatible = "dlg,da9121";
-        reg = <0x68>;
-
-        buck1 {
-          regulator-min-microvolt = <680000>;
-          regulator-max-microvolt = <820000>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        pmic: da9121@68 {
+            compatible = "dlg,da9121";
+            reg = <0x68>;
+
+            interrupt-parent = <&gpio6>;
+            interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
+
+            dlg,irq-polling-delay-passive = <2000>;
+
+            regulators {
+                DA9121_BUCK1: buck1 {
+                    regulator-name = "BUCK1";
+                    regulator-min-microvolt = <300000>;
+                    regulator-max-microvolt = <1900000>;
+                    regulator-min-microamp = <7000000>;
+                    regulator-max-microamp = <20000000>;
+                    regulator-boot-on;
+                    regulator-initial-mode = <DA9121_BUCK_MODE_AUTO>;
+                    enable-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
+                };
+            };
         };
-      };
     };
 
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/regulator/dlg,da9121-regulator.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        pmic: da9122@68 {
+            compatible = "dlg,da9122";
+            reg = <0x68>;
+
+            interrupt-parent = <&gpio6>;
+            interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
+
+            dlg,irq-polling-delay-passive = <2000>;
+
+            regulators {
+                DA9122_BUCK1: buck1 {
+                    regulator-name = "BUCK1";
+                    regulator-min-microvolt = <300000>;
+                    regulator-max-microvolt = <1900000>;
+                    regulator-min-microamp = <3500000>;
+                    regulator-max-microamp = <10000000>;
+                    regulator-boot-on;
+                    regulator-initial-mode = <DA9121_BUCK_MODE_AUTO>;
+                    enable-gpios = <&gpio6 1 GPIO_ACTIVE_HIGH>;
+                    dlg,ripple-cancel = <DA9121_BUCK_RIPPLE_CANCEL_NONE>;
+                };
+                DA9122_BUCK2: buck2 {
+                    regulator-name = "BUCK2";
+                    regulator-min-microvolt = <300000>;
+                    regulator-max-microvolt = <1900000>;
+                    regulator-min-microamp = <3500000>;
+                    regulator-max-microamp = <10000000>;
+                    regulator-boot-on;
+                    regulator-initial-mode = <DA9121_BUCK_MODE_AUTO>;
+                    enable-gpios = <&gpio6 2 GPIO_ACTIVE_HIGH>;
+                    dlg,ripple-cancel = <DA9121_BUCK_RIPPLE_CANCEL_NONE>;
+                };
+            };
+        };
+    };
 ...
diff --git a/MAINTAINERS b/MAINTAINERS
index 00584ca..622acba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5122,6 +5122,7 @@  S:	Supported
 W:	http://www.dialog-semiconductor.com/products
 F:	Documentation/devicetree/bindings/input/da90??-onkey.txt
 F:	Documentation/devicetree/bindings/mfd/da90*.txt
+F:	Documentation/devicetree/bindings/regulator/dlg,da9*.yaml
 F:	Documentation/devicetree/bindings/regulator/da92*.txt
 F:	Documentation/devicetree/bindings/regulator/slg51000.txt
 F:	Documentation/devicetree/bindings/sound/da[79]*.txt
@@ -5146,6 +5147,7 @@  F:	drivers/rtc/rtc-da90??.c
 F:	drivers/thermal/da90??-thermal.c
 F:	drivers/video/backlight/da90??_bl.c
 F:	drivers/watchdog/da90??_wdt.c
+F:	include/dt-bindings/regulator/dlg,da9*-regulator.h
 F:	include/linux/mfd/da903x.h
 F:	include/linux/mfd/da9052/
 F:	include/linux/mfd/da9055/
diff --git a/include/dt-bindings/regulator/dlg,da9121-regulator.h b/include/dt-bindings/regulator/dlg,da9121-regulator.h
new file mode 100644
index 0000000..954edf6
--- /dev/null
+++ b/include/dt-bindings/regulator/dlg,da9121-regulator.h
@@ -0,0 +1,22 @@ 
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef _DT_BINDINGS_REGULATOR_DLG_DA9121_H
+#define _DT_BINDINGS_REGULATOR_DLG_DA9121_H
+
+/*
+ * These buck mode constants may be used to specify values in device tree
+ * properties (e.g. regulator-initial-mode).
+ * A description of the following modes is in the manufacturers datasheet.
+ */
+
+#define DA9121_BUCK_MODE_FORCE_PFM		0
+#define DA9121_BUCK_MODE_FORCE_PWM		1
+#define DA9121_BUCK_MODE_FORCE_PWM_SHEDDING	2
+#define DA9121_BUCK_MODE_AUTO			3
+
+#define DA9121_BUCK_RIPPLE_CANCEL_NONE		0
+#define DA9121_BUCK_RIPPLE_CANCEL_SMALL		1
+#define DA9121_BUCK_RIPPLE_CANCEL_MID		2
+#define DA9121_BUCK_RIPPLE_CANCEL_LARGE		3
+
+#endif