All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] regulator: da9121: extend support to variants, add features
@ 2020-11-20 12:14 Adam Ward
  2020-11-20 12:14 ` [PATCH 1/9] regulator: Update DA9121 dt-bindings Adam Ward
                   ` (8 more replies)
  0 siblings, 9 replies; 19+ messages in thread
From: Adam Ward @ 2020-11-20 12:14 UTC (permalink / raw)
  To: Mark Brown, Rob Herring
  Cc: Liam Girdwood, Vincent Whitchurch, linux-kernel, devicetree

s series extends the DA9121 driver to add support for related products:

  DA9130, a High-Performance, 10A, Dual-Phase > DC-DC Converter (Automotive Grade)
  DA9122, a High-Performance, 5A + 5A, Dual-Channel > DC-DC Converter
  DA9131, a High-Performance, 5A + 5A, Dual-Channel > DC-DC Converter (Automotive Grade)
  DA9220, a High-Performance, 3A + 3A, Dual-Channel > DC-DC Converter
  DA9132, a High-Performance, 3A + 3A, Dual-Channel > DC-DC Converter (Automotive Grade)
  DA9217, a High-Performance, 6A, Dual-Phase > DC-DC Converter

It also extends support to cover DT configured GPIO enable, current limit setting,
and interrupt handling for all devices.

The datasheets are currently available here:

https://www.dialog-semiconductor.com/sites/default/files/da9122_datasheet_2v1.pdf
https://www.dialog-semiconductor.com/sites/default/files/da9220_datasheet_2v1.pdf
https://www.dialog-semiconductor.com/sites/default/files/da9217_datasheet_2v1.pdf
https://www.dialog-semiconductor.com/sites/default/files/da9130-a_datasheet_1v0.pdf
https://www.dialog-semiconductor.com/sites/default/files/da9131-a_datasheet_1v0.pdf
https://www.dialog-semiconductor.com/sites/default/files/da9132-a_datasheet_1v0.pdf

Adam Ward (9):
  regulator: Update DA9121 dt-bindings
  regulator: da9121: Add header file
  regulator: da9121: Add device variants
  regulator: da9121: Add device variant details and respective regmaps
  regulator: da9121: Add support for device variants via devicetree
  regulator: da9121: Update registration to support multiple buck
    variants
  regulator: da9121: add current support
  regulator: da9121: add mode support
  regulator: da9121: add interrupt support

 .../devicetree/bindings/regulator/dlg,da9121.yaml  |  177 ++-
 MAINTAINERS                                        |    2 +
 drivers/regulator/Kconfig                          |   14 +-
 drivers/regulator/da9121-regulator.c               | 1429 +++++++++++++++++++-
 drivers/regulator/da9121-regulator.h               |  291 ++++
 .../dt-bindings/regulator/dlg,da9121-regulator.h   |   22 +
 include/linux/regulator/da9121.h                   |   36 +
 7 files changed, 1917 insertions(+), 54 deletions(-)
 create mode 100644 drivers/regulator/da9121-regulator.h
 create mode 100644 include/dt-bindings/regulator/dlg,da9121-regulator.h
 create mode 100644 include/linux/regulator/da9121.h

-- 
1.9.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/9] regulator: Update DA9121 dt-bindings
  2020-11-20 12:14 [PATCH 0/9] regulator: da9121: extend support to variants, add features Adam Ward
@ 2020-11-20 12:14 ` Adam Ward
  2020-11-20 13:47   ` Vincent Whitchurch
  2020-11-20 12:14 ` [PATCH 2/9] regulator: da9121: Add header file Adam Ward
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Adam Ward @ 2020-11-20 12:14 UTC (permalink / raw)
  To: Mark Brown, Rob Herring
  Cc: Liam Girdwood, Vincent Whitchurch, linux-kernel, devicetree

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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/9] regulator: da9121: Add header file
  2020-11-20 12:14 [PATCH 0/9] regulator: da9121: extend support to variants, add features Adam Ward
  2020-11-20 12:14 ` [PATCH 1/9] regulator: Update DA9121 dt-bindings Adam Ward
@ 2020-11-20 12:14 ` Adam Ward
  2020-11-20 12:14 ` [PATCH 3/9] regulator: da9121: Add device variants Adam Ward
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Adam Ward @ 2020-11-20 12:14 UTC (permalink / raw)
  To: Mark Brown, Rob Herring
  Cc: Liam Girdwood, Vincent Whitchurch, linux-kernel, devicetree

Add header file for Dialog Semiconductor DA9121 regulator and related devices,
mostly autogenerated from the chip design databases, and update driver to
replacee local defines with those from header.

Signed-off-by: Adam Ward <Adam.Ward.opensource@diasemi.com>
---
 drivers/regulator/da9121-regulator.c |  15 +-
 drivers/regulator/da9121-regulator.h | 291 +++++++++++++++++++++++++++++++++++
 2 files changed, 296 insertions(+), 10 deletions(-)
 create mode 100644 drivers/regulator/da9121-regulator.h

diff --git a/drivers/regulator/da9121-regulator.c b/drivers/regulator/da9121-regulator.c
index 66bdfd1..c11fe04 100644
--- a/drivers/regulator/da9121-regulator.c
+++ b/drivers/regulator/da9121-regulator.c
@@ -9,12 +9,7 @@
 #include <linux/regmap.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
-
-#define DA9121_BUCK_BUCK1_0			0x20
-#define DA9121_BUCK_BUCK1_0_CH1_EN		BIT(0)
-
-#define DA9121_BUCK_BUCK1_5			0x25
-#define DA9121_BUCK_BUCK1_5_CH1_A_VOUT		GENMASK(7, 0)
+#include "da9121-regulator.h"
 
 #define DA9121_MIN_MV		300
 #define DA9121_MAX_MV		1900
@@ -47,10 +42,10 @@
 	.min_uV = DA9121_MIN_MV * 1000,
 	.uV_step = DA9121_STEP_MV * 1000,
 	.linear_min_sel = DA9121_MIN_SEL,
-	.vsel_reg = DA9121_BUCK_BUCK1_5,
-	.vsel_mask = DA9121_BUCK_BUCK1_5_CH1_A_VOUT,
-	.enable_reg = DA9121_BUCK_BUCK1_0,
-	.enable_mask = DA9121_BUCK_BUCK1_0_CH1_EN,
+	.vsel_reg = DA9121_REG_BUCK_BUCK1_5,
+	.vsel_mask = DA9121_MASK_BUCK_BUCKx_5_CHx_A_VOUT,
+	.enable_reg = DA9121_REG_BUCK_BUCK1_0,
+	.enable_mask = DA9121_MASK_BUCK_BUCKx_0_CHx_EN,
 	/* Default value of BUCK_BUCK1_0.CH1_SRC_DVC_UP */
 	.ramp_delay = 20000,
 	/* tBUCK_EN */
diff --git a/drivers/regulator/da9121-regulator.h b/drivers/regulator/da9121-regulator.h
new file mode 100644
index 0000000..3c34cb8
--- /dev/null
+++ b/drivers/regulator/da9121-regulator.h
@@ -0,0 +1,291 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * DA9121 Single-channel dual-phase 10A buck converter
+ * DA9130 Single-channel dual-phase 10A buck converter (Automotive)
+ * DA9217 Single-channel dual-phase  6A buck converter
+ * DA9122 Dual-channel single-phase  5A buck converter
+ * DA9131 Dual-channel single-phase  5A buck converter (Automotive)
+ * DA9220 Dual-channel single-phase  3A buck converter
+ * DA9132 Dual-channel single-phase  3A buck converter (Automotive)
+ *
+ * Copyright (C) 2020  Dialog Semiconductor
+ *
+ * Authors: Steve Twiss, Dialog Semiconductor
+ *          Adam Ward, Dialog Semiconductor
+ */
+
+#ifndef __DA9121_REGISTERS_H__
+#define __DA9121_REGISTERS_H__
+
+/* Values for: DA9121_REG_BUCK_BUCKx_4 registers, fields CHx_y_MODE
+ *             DA9121_REG_BUCK_BUCKx_7 registers, fields CHx_RIPPLE_CANCEL
+ */
+#include <dt-bindings/regulator/dlg,da9121-regulator.h>
+
+enum da9121_variant {
+	DA9121_TYPE_DA9121_DA9130,
+	DA9121_TYPE_DA9220_DA9132,
+	DA9121_TYPE_DA9122_DA9131,
+	DA9121_TYPE_DA9217
+};
+
+/* Minimum, maximum and default polling millisecond periods are provided
+ * here as an example. It is expected that any final implementation will
+ * include a modification of these settings to match the required
+ * application.
+ */
+#define DA9121_DEFAULT_POLLING_PERIOD_MS	3000
+#define DA9121_MAX_POLLING_PERIOD_MS		10000
+#define DA9121_MIN_POLLING_PERIOD_MS		1000
+
+/* Registers */
+
+#define DA9121_REG_SYS_STATUS_0		0x01
+#define DA9121_REG_SYS_STATUS_1		0x02
+#define DA9121_REG_SYS_STATUS_2		0x03
+#define DA9121_REG_SYS_EVENT_0		0x04
+#define DA9121_REG_SYS_EVENT_1		0x05
+#define DA9121_REG_SYS_EVENT_2		0x06
+#define DA9121_REG_SYS_MASK_0		0x07
+#define DA9121_REG_SYS_MASK_1		0x08
+#define DA9121_REG_SYS_MASK_2		0x09
+#define DA9121_REG_SYS_MASK_3		0x0A
+#define DA9121_REG_SYS_CONFIG_0		0x0B
+#define DA9121_REG_SYS_CONFIG_1		0x0C
+#define DA9121_REG_SYS_CONFIG_2		0x0D
+#define DA9121_REG_SYS_CONFIG_3		0x0E
+#define DA9121_REG_SYS_GPIO0_0		0x10
+#define DA9121_REG_SYS_GPIO0_1		0x11
+#define DA9121_REG_SYS_GPIO1_0		0x12
+#define DA9121_REG_SYS_GPIO1_1		0x13
+#define DA9121_REG_SYS_GPIO2_0		0x14
+#define DA9121_REG_SYS_GPIO2_1		0x15
+#define DA9121_REG_BUCK_BUCK1_0		0x20
+#define DA9121_REG_BUCK_BUCK1_1		0x21
+#define DA9121_REG_BUCK_BUCK1_2		0x22
+#define DA9121_REG_BUCK_BUCK1_3		0x23
+#define DA9121_REG_BUCK_BUCK1_4		0x24
+#define DA9121_REG_BUCK_BUCK1_5		0x25
+#define DA9121_REG_BUCK_BUCK1_6		0x26
+#define DA9121_REG_BUCK_BUCK1_7		0x27
+#define DA9xxx_REG_BUCK_BUCK2_0		0x28
+#define DA9xxx_REG_BUCK_BUCK2_1		0x29
+#define DA9xxx_REG_BUCK_BUCK2_2		0x2A
+#define DA9xxx_REG_BUCK_BUCK2_3		0x2B
+#define DA9xxx_REG_BUCK_BUCK2_4		0x2C
+#define DA9xxx_REG_BUCK_BUCK2_5		0x2D
+#define DA9xxx_REG_BUCK_BUCK2_6		0x2E
+#define DA9xxx_REG_BUCK_BUCK2_7		0x2F
+#define DA9121_REG_OTP_DEVICE_ID	0x48
+#define DA9121_REG_OTP_VARIANT_ID	0x49
+#define DA9121_REG_OTP_CUSTOMER_ID	0x4A
+#define DA9121_REG_OTP_CONFIG_ID	0x4B
+
+/* Register bits */
+
+/* DA9121_REG_SYS_STATUS_0 */
+
+#define DA9xxx_MASK_SYS_STATUS_0_SG			BIT(2)
+#define DA9121_MASK_SYS_STATUS_0_TEMP_CRIT		BIT(1)
+#define DA9121_MASK_SYS_STATUS_0_TEMP_WARN		BIT(0)
+
+/* DA9121_REG_SYS_STATUS_1 */
+
+#define DA9xxx_MASK_SYS_STATUS_1_PG2			BIT(7)
+#define DA9xxx_MASK_SYS_STATUS_1_OV2			BIT(6)
+#define DA9xxx_MASK_SYS_STATUS_1_UV2			BIT(5)
+#define DA9xxx_MASK_SYS_STATUS_1_OC2			BIT(4)
+#define DA9121_MASK_SYS_STATUS_1_PG1			BIT(3)
+#define DA9121_MASK_SYS_STATUS_1_OV1			BIT(2)
+#define DA9121_MASK_SYS_STATUS_1_UV1			BIT(1)
+#define DA9121_MASK_SYS_STATUS_1_OC1			BIT(0)
+
+/* DA9121_REG_SYS_STATUS_2 */
+
+#define DA9121_MASK_SYS_STATUS_2_GPIO2			BIT(2)
+#define DA9121_MASK_SYS_STATUS_2_GPIO1			BIT(1)
+#define DA9121_MASK_SYS_STATUS_2_GPIO0			BIT(0)
+
+/* DA9121_REG_SYS_EVENT_0 */
+
+#define DA9xxx_MASK_SYS_EVENT_0_E_SG			BIT(2)
+#define DA9121_MASK_SYS_EVENT_0_E_TEMP_CRIT		BIT(1)
+#define DA9121_MASK_SYS_EVENT_0_E_TEMP_WARN		BIT(0)
+
+/* DA9121_REG_SYS_EVENT_1 */
+
+#define DA9xxx_MASK_SYS_EVENT_1_E_PG2			BIT(7)
+#define DA9xxx_MASK_SYS_EVENT_1_E_OV2			BIT(6)
+#define DA9xxx_MASK_SYS_EVENT_1_E_UV2			BIT(5)
+#define DA9xxx_MASK_SYS_EVENT_1_E_OC2			BIT(4)
+#define DA9121_MASK_SYS_EVENT_1_E_PG1			BIT(3)
+#define DA9121_MASK_SYS_EVENT_1_E_OV1			BIT(2)
+#define DA9121_MASK_SYS_EVENT_1_E_UV1			BIT(1)
+#define DA9121_MASK_SYS_EVENT_1_E_OC1			BIT(0)
+
+/* DA9121_REG_SYS_EVENT_2 */
+
+#define DA9121_MASK_SYS_EVENT_2_E_GPIO2			BIT(2)
+#define DA9121_MASK_SYS_EVENT_2_E_GPIO1			BIT(1)
+#define DA9121_MASK_SYS_EVENT_2_E_GPIO0			BIT(0)
+
+/* DA9121_REG_SYS_MASK_0 */
+
+#define DA9xxx_MASK_SYS_MASK_0_M_SG			BIT(2)
+#define DA9121_MASK_SYS_MASK_0_M_TEMP_CRIT		BIT(1)
+#define DA9121_MASK_SYS_MASK_0_M_TEMP_WARN		BIT(0)
+
+/* DA9121_REG_SYS_MASK_1 */
+
+#define DA9xxx_MASK_SYS_MASK_1_M_PG2			BIT(7)
+#define DA9xxx_MASK_SYS_MASK_1_M_OV2			BIT(6)
+#define DA9xxx_MASK_SYS_MASK_1_M_UV2			BIT(5)
+#define DA9xxx_MASK_SYS_MASK_1_M_OC2			BIT(4)
+#define DA9121_MASK_SYS_MASK_1_M_PG1			BIT(3)
+#define DA9121_MASK_SYS_MASK_1_M_OV1			BIT(2)
+#define DA9121_MASK_SYS_MASK_1_M_UV1			BIT(1)
+#define DA9121_MASK_SYS_MASK_1_M_OC1			BIT(0)
+
+/* DA9121_REG_SYS_MASK_2 */
+
+#define DA9121_MASK_SYS_MASK_2_M_GPIO2			BIT(2)
+#define DA9121_MASK_SYS_MASK_2_M_GPIO1			BIT(1)
+#define DA9121_MASK_SYS_MASK_2_M_GPIO0			BIT(0)
+
+/* DA9122_REG_SYS_MASK_3 */
+
+#define DA9121_MASK_SYS_MASK_3_M_VR_HOT			BIT(3)
+#define DA9xxx_MASK_SYS_MASK_3_M_SG_STAT		BIT(2)
+#define DA9xxx_MASK_SYS_MASK_3_M_PG2_STAT		BIT(1)
+#define DA9121_MASK_SYS_MASK_3_M_PG1_STAT		BIT(0)
+
+/* DA9121_REG_SYS_CONFIG_0 */
+
+#define DA9121_MASK_SYS_CONFIG_0_CH1_DIS_DLY		0xF0
+#define DA9121_MASK_SYS_CONFIG_0_CH1_EN_DLY		0x0F
+
+/* DA9xxx_REG_SYS_CONFIG_1 */
+
+#define DA9xxx_MASK_SYS_CONFIG_1_CH2_DIS_DLY		0xF0
+#define DA9xxx_MASK_SYS_CONFIG_1_CH2_EN_DLY		0x0F
+
+/* DA9121_REG_SYS_CONFIG_2 */
+
+#define DA9121_MASK_SYS_CONFIG_2_OC_LATCHOFF		0x60
+#define DA9121_MASK_SYS_CONFIG_2_OC_DVC_MASK		BIT(4)
+#define DA9121_MASK_SYS_CONFIG_2_PG_DVC_MASK		0x0C
+
+/* DA9121_REG_SYS_CONFIG_3 */
+
+#define DA9121_MASK_SYS_CONFIG_3_OSC_TUNE		0X70
+#define DA9121_MASK_SYS_CONFIG_3_I2C_TIMEOUT		BIT(1)
+
+/* DA9121_REG_SYS_GPIO0_0 */
+
+#define DA9121_MASK_SYS_GPIO0_0_GPIO0_MODE		0X1E
+#define DA9121_MASK_SYS_GPIO0_0_GPIO0_OBUF		BIT(0)
+
+/* DA9121_REG_SYS_GPIO0_1 */
+
+#define DA9121_MASK_SYS_GPIO0_1_GPIO0_DEB_FALL		BIT(7)
+#define DA9121_MASK_SYS_GPIO0_1_GPIO0_DEB_RISE		BIT(6)
+#define DA9121_MASK_SYS_GPIO0_1_GPIO0_DEB		0x30
+#define DA9121_MASK_SYS_GPIO0_1_GPIO0_PUPD		BIT(3)
+#define DA9121_MASK_SYS_GPIO0_1_GPIO0_POL		BIT(2)
+#define DA9121_MASK_SYS_GPIO0_1_GPIO0_TRIG		0x03
+
+/* DA9121_REG_SYS_GPIO1_0 */
+
+#define DA9121_MASK_SYS_GPIO1_0_GPIO1_MODE		0x1E
+#define DA9121_MASK_SYS_GPIO1_0_GPIO1_OBUF		BIT(0)
+
+/* DA9121_REG_SYS_GPIO1_1 */
+
+#define DA9121_MASK_SYS_GPIO1_1_GPIO1_DEB_FALL		BIT(7)
+#define DA9121_MASK_SYS_GPIO1_1_GPIO1_DEB_RISE		BIT(6)
+#define DA9121_MASK_SYS_GPIO1_1_GPIO1_DEB		0x30
+#define DA9121_MASK_SYS_GPIO1_1_GPIO1_PUPD		BIT(3)
+#define DA9121_MASK_SYS_GPIO1_1_GPIO1_POL		BIT(2)
+#define DA9121_MASK_SYS_GPIO1_1_GPIO1_TRIG		0x03
+
+/* DA9121_REG_SYS_GPIO2_0 */
+
+#define DA9121_MASK_SYS_GPIO2_0_GPIO2_MODE		0x1E
+#define DA9121_MASK_SYS_GPIO2_0_GPIO2_OBUF		BIT(0)
+
+/* DA9121_REG_SYS_GPIO2_1 */
+
+#define DA9121_MASK_SYS_GPIO2_1_GPIO2_DEB_FALL		BIT(7)
+#define DA9121_MASK_SYS_GPIO2_1_GPIO2_DEB_RISE		BIT(6)
+#define DA9121_MASK_SYS_GPIO2_1_GPIO2_DEB		0x30
+#define DA9121_MASK_SYS_GPIO2_1_GPIO2_PUPD		BIT(3)
+#define DA9121_MASK_SYS_GPIO2_1_GPIO2_POL		BIT(2)
+#define DA9121_MASK_SYS_GPIO2_1_GPIO2_TRIG		0x03
+
+/* DA9121_REG_BUCK_BUCK1_0 / DA9xxx_REG_BUCK_BUCK2_0 */
+
+#define DA9121_MASK_BUCK_BUCKx_0_CHx_SR_DVC_DWN		0x70
+#define DA9121_MASK_BUCK_BUCKx_0_CHx_SR_DVC_UP		0x0E
+#define DA9121_MASK_BUCK_BUCKx_0_CHx_EN			BIT(0)
+
+/* DA9121_REG_BUCK_BUCK1_1 / DA9xxx_REG_BUCK_BUCK2_1 */
+
+#define DA9121_MASK_BUCK_BUCKx_1_CHx_SR_SHDN		0x70
+#define DA9121_MASK_BUCK_BUCKx_1_CHx_SR_STARTUP		0x0E
+#define DA9121_MASK_BUCK_BUCKx_1_CHx_PD_DIS		BIT(0)
+
+/* DA9121_REG_BUCK_BUCK1_2 / DA9xxx_REG_BUCK_BUCK2_2 */
+
+#define DA9121_MASK_BUCK_BUCKx_2_CHx_ILIM		0x0F
+
+/* DA9121_REG_BUCK_BUCK1_3 / DA9xxx_REG_BUCK_BUCK2_3 */
+
+#define DA9121_MASK_BUCK_BUCKx_3_CHx_VMAX		0xFF
+
+/* DA9121_REG_BUCK_BUCK1_4 / DA9xxx_REG_BUCK_BUCK2_4 */
+
+#define DA9121_MASK_BUCK_BUCKx_4_CHx_VSEL		BIT(4)
+#define DA9121_MASK_BUCK_BUCKx_4_CHx_B_MODE		0x0C
+#define DA9121_MASK_BUCK_BUCKx_4_CHx_A_MODE		0x03
+
+/* DA9121_REG_BUCK_BUCK1_5 / DA9xxx_REG_BUCK_BUCK2_5 */
+
+#define DA9121_MASK_BUCK_BUCKx_5_CHx_A_VOUT		0xFF
+
+/* DA9121_REG_BUCK_BUCK1_6 / DA9xxx_REG_BUCK_BUCK2_6 */
+
+#define DA9121_MASK_BUCK_BUCKx_6_CHx_B_VOUT		0xFF
+
+/* DA9121_REG_BUCK_BUCK1_7 / DA9xxx_REG_BUCK_BUCK2_7 */
+
+#define DA9xxx_MASK_BUCK_BUCKx_7_CHx_RIPPLE_CANCEL	0x03
+
+
+/* DA9121_REG_OTP_DEVICE_ID */
+
+#define DA9121_MASK_OTP_DEVICE_ID_DEV_ID		0xFF
+
+#define DA9121_DEVICE_ID	0x05
+
+/* DA9121_REG_OTP_VARIANT_ID */
+
+#define DA9121_SHIFT_OTP_VARIANT_ID_MRC			4
+#define DA9121_MASK_OTP_VARIANT_ID_MRC			0xF0
+#define DA9121_SHIFT_OTP_VARIANT_ID_VRC			0
+#define DA9121_MASK_OTP_VARIANT_ID_VRC			0x0F
+
+#define DA9121_VARIANT_MRC_BASE	0x2
+#define DA9121_VARIANT_VRC	0x1
+#define DA9220_VARIANT_VRC	0x0
+#define DA9122_VARIANT_VRC	0x2
+#define DA9217_VARIANT_VRC	0x7
+
+/* DA9121_REG_OTP_CUSTOMER_ID */
+
+#define DA9121_MASK_OTP_CUSTOMER_ID_CUST_ID		0xFF
+
+/* DA9121_REG_OTP_CONFIG_ID */
+
+#define DA9121_MASK_OTP_CONFIG_ID_CONFIG_REV		0xFF
+
+#endif /* __DA9121_REGISTERS_H__ */
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/9] regulator: da9121: Add device variants
  2020-11-20 12:14 [PATCH 0/9] regulator: da9121: extend support to variants, add features Adam Ward
  2020-11-20 12:14 ` [PATCH 1/9] regulator: Update DA9121 dt-bindings Adam Ward
  2020-11-20 12:14 ` [PATCH 2/9] regulator: da9121: Add header file Adam Ward
@ 2020-11-20 12:14 ` Adam Ward
  2020-11-20 12:14 ` [PATCH 4/9] regulator: da9121: Add device variant details and respective regmaps Adam Ward
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Adam Ward @ 2020-11-20 12:14 UTC (permalink / raw)
  To: Mark Brown, Rob Herring
  Cc: Liam Girdwood, Vincent Whitchurch, linux-kernel, devicetree

Add basic support for configuration to reference variants of this device,
and track the selected variant within the driver.

Signed-off-by: Adam Ward <Adam.Ward.opensource@diasemi.com>
---
 drivers/regulator/da9121-regulator.c | 46 +++++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/da9121-regulator.c b/drivers/regulator/da9121-regulator.c
index c11fe04..5bebdb2 100644
--- a/drivers/regulator/da9121-regulator.c
+++ b/drivers/regulator/da9121-regulator.c
@@ -11,6 +11,12 @@
 #include <linux/i2c.h>
 #include "da9121-regulator.h"
 
+/* Chip data */
+struct da9121 {
+	struct device *dev;
+	int variant_id;
+};
+
 #define DA9121_MIN_MV		300
 #define DA9121_MAX_MV		1900
 #define DA9121_STEP_MV		10
@@ -53,19 +59,46 @@
 };
 
 static const struct of_device_id da9121_dt_ids[] = {
-	{ .compatible = "dlg,da9121", },
+	{ .compatible = "dlg,da9121", .data = (void *) DA9121_TYPE_DA9121_DA9130 },
+	{ .compatible = "dlg,da9130", .data = (void *) DA9121_TYPE_DA9121_DA9130 },
+	{ .compatible = "dlg,da9217", .data = (void *) DA9121_TYPE_DA9217 },
+	{ .compatible = "dlg,da9122", .data = (void *) DA9121_TYPE_DA9122_DA9131 },
+	{ .compatible = "dlg,da9131", .data = (void *) DA9121_TYPE_DA9122_DA9131 },
+	{ .compatible = "dlg,da9220", .data = (void *) DA9121_TYPE_DA9220_DA9132 },
+	{ .compatible = "dlg,da9132", .data = (void *) DA9121_TYPE_DA9220_DA9132 },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, da9121_dt_ids);
 
+static inline int da9121_of_get_id(struct device *dev)
+{
+	const struct of_device_id *id = of_match_device(da9121_dt_ids, dev);
+
+	if (!id) {
+		dev_err(dev, "%s: Failed\n", __func__);
+		return -EINVAL;
+	}
+	return (uintptr_t)id->data;
+}
+
 static int da9121_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
+	struct da9121 *chip;
+	int ret = 0;
 	struct device *dev = &i2c->dev;
 	struct regulator_config config = {};
 	struct regulator_dev *rdev;
 	struct regmap *regmap;
 
+	chip = devm_kzalloc(&i2c->dev, sizeof(struct da9121), GFP_KERNEL);
+	if (!chip) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	chip->variant_id = da9121_of_get_id(&i2c->dev);
+
 	regmap = devm_regmap_init_i2c(i2c, &da9121_regmap_config);
 	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
@@ -80,11 +113,18 @@ static int da9121_i2c_probe(struct i2c_client *i2c,
 		return PTR_ERR(rdev);
 	}
 
-	return 0;
+error:
+	return ret;
 }
 
 static const struct i2c_device_id da9121_i2c_id[] = {
-	{ "da9121", 0 },
+	{"da9121", DA9121_TYPE_DA9121_DA9130},
+	{"da9130", DA9121_TYPE_DA9121_DA9130},
+	{"da9217", DA9121_TYPE_DA9217},
+	{"da9122", DA9121_TYPE_DA9122_DA9131},
+	{"da9131", DA9121_TYPE_DA9122_DA9131},
+	{"da9220", DA9121_TYPE_DA9220_DA9132},
+	{"da9132", DA9121_TYPE_DA9220_DA9132},
 	{},
 };
 MODULE_DEVICE_TABLE(i2c, da9121_i2c_id);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/9] regulator: da9121: Add device variant details and respective regmaps
  2020-11-20 12:14 [PATCH 0/9] regulator: da9121: extend support to variants, add features Adam Ward
                   ` (2 preceding siblings ...)
  2020-11-20 12:14 ` [PATCH 3/9] regulator: da9121: Add device variants Adam Ward
@ 2020-11-20 12:14 ` Adam Ward
  2020-11-20 12:45   ` Mark Brown
  2020-11-20 12:14 ` [PATCH 5/9] regulator: da9121: Add support for device variants via devicetree Adam Ward
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Adam Ward @ 2020-11-20 12:14 UTC (permalink / raw)
  To: Mark Brown, Rob Herring
  Cc: Liam Girdwood, Vincent Whitchurch, linux-kernel, devicetree

Add ability to probe device and validate configuration, then apply a regmap
configuration for a single or dual buck device accordingly.

Signed-off-by: Adam Ward <Adam.Ward.opensource@diasemi.com>
---
 drivers/regulator/Kconfig            |  14 +-
 drivers/regulator/da9121-regulator.c | 382 +++++++++++++++++++++++++++++++++--
 include/linux/regulator/da9121.h     |  25 +++
 3 files changed, 401 insertions(+), 20 deletions(-)
 create mode 100644 include/linux/regulator/da9121.h

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 005a603..68f1bbd 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -304,13 +304,21 @@ config REGULATOR_DA9063
 	  will be called da9063-regulator.
 
 config REGULATOR_DA9121
-	tristate "Dialog Semiconductor DA9121 regulator"
+	tristate "Dialog Semiconductor DA9121/DA9122/DA9220/DA9217/DA9130/DA9131/DA9132 regulator"
 	depends on I2C && OF
 	select REGMAP_I2C
 	help
 	  Say y here to support for the Dialog Semiconductor DA9121.  The
-	  DA9210 is a dual-phase buck converter controlled through an I2C
-	  interface.
+	  DA9121 is a single channel dual-phase buck converter controlled
+	  through an I2C interface.
+
+	  DA9121 Single-channel dual-phase 10A buck converter
+	  DA9130 Single-channel dual-phase 10A buck converter (Automotive)
+	  DA9217 Single-channel dual-phase  6A buck converter
+	  DA9122 Dual-channel single-phase  5A buck converter
+	  DA9131 Dual-channel single-phase  5A buck converter (Automotive)
+	  DA9220 Dual-channel single-phase  3A buck converter
+	  DA9132 Dual-channel single-phase  3A buck converter (Automotive)
 
 	  This driver can also be built as a module. If so, the module
 	  will be called da9121-regulator.
diff --git a/drivers/regulator/da9121-regulator.c b/drivers/regulator/da9121-regulator.c
index 5bebdb2..76932ba 100644
--- a/drivers/regulator/da9121-regulator.c
+++ b/drivers/regulator/da9121-regulator.c
@@ -9,26 +9,18 @@
 #include <linux/regmap.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
+#include <linux/regulator/da9121.h>
+
 #include "da9121-regulator.h"
 
 /* Chip data */
 struct da9121 {
 	struct device *dev;
+	struct regmap *regmap;
+	struct regulator_dev *rdev[DA9121_IDX_MAX];
 	int variant_id;
 };
 
-#define DA9121_MIN_MV		300
-#define DA9121_MAX_MV		1900
-#define DA9121_STEP_MV		10
-#define DA9121_MIN_SEL		(DA9121_MIN_MV / DA9121_STEP_MV)
-#define DA9121_N_VOLTAGES	(((DA9121_MAX_MV - DA9121_MIN_MV) / DA9121_STEP_MV) \
-				 + 1 + DA9121_MIN_SEL)
-
-static const struct regmap_config da9121_regmap_config = {
-	.reg_bits = 8,
-	.val_bits = 8,
-};
-
 static const struct regulator_ops da9121_buck_ops = {
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -38,10 +30,24 @@ struct da9121 {
 	.list_voltage = regulator_list_voltage_linear,
 };
 
+static struct of_regulator_match da9121_matches[] = {
+	[DA9121_IDX_BUCK1] = { .name = "buck1" },
+	[DA9121_IDX_BUCK2] = { .name = "buck2" },
+};
+
+#define DA9121_MIN_MV		300
+#define DA9121_MAX_MV		1900
+#define DA9121_STEP_MV		10
+#define DA9121_MIN_SEL		(DA9121_MIN_MV / DA9121_STEP_MV)
+#define DA9121_N_VOLTAGES	(((DA9121_MAX_MV - DA9121_MIN_MV) / DA9121_STEP_MV) \
+				 + 1 + DA9121_MIN_SEL)
+
 static const struct regulator_desc da9121_reg = {
+	.id = DA9121_IDX_BUCK1,
 	.name = "da9121",
 	.of_match = "buck1",
 	.owner = THIS_MODULE,
+	.regulators_node = of_match_ptr("regulators"),
 	.ops = &da9121_buck_ops,
 	.type = REGULATOR_VOLTAGE,
 	.n_voltages = DA9121_N_VOLTAGES,
@@ -58,6 +64,349 @@ struct da9121 {
 	.enable_time = 20,
 };
 
+static const struct regulator_desc da9220_reg[2] = {
+	{
+		.id = DA9121_IDX_BUCK1,
+		.name = "DA9220/DA9132 BUCK1",
+		.of_match = "buck1",
+		.owner = THIS_MODULE,
+		.regulators_node = of_match_ptr("regulators"),
+		.ops = &da9121_buck_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = DA9121_N_VOLTAGES,
+		.min_uV = DA9121_MIN_MV * 1000,
+		.uV_step = DA9121_STEP_MV * 1000,
+		.linear_min_sel = DA9121_MIN_SEL,
+		.enable_reg = DA9121_REG_BUCK_BUCK1_0,
+		.enable_mask = DA9121_MASK_BUCK_BUCKx_0_CHx_EN,
+		.vsel_reg = DA9121_REG_BUCK_BUCK1_5,
+		.vsel_mask = DA9121_MASK_BUCK_BUCKx_5_CHx_A_VOUT,
+	},
+	{
+		.id = DA9121_IDX_BUCK2,
+		.name = "DA9220/DA9132 BUCK2",
+		.of_match = "buck2",
+		.owner = THIS_MODULE,
+		.regulators_node = of_match_ptr("regulators"),
+		.ops = &da9121_buck_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = DA9121_N_VOLTAGES,
+		.min_uV = DA9121_MIN_MV * 1000,
+		.uV_step = DA9121_STEP_MV * 1000,
+		.linear_min_sel = DA9121_MIN_SEL,
+		.enable_reg = DA9xxx_REG_BUCK_BUCK2_0,
+		.enable_mask = DA9121_MASK_BUCK_BUCKx_0_CHx_EN,
+		.vsel_reg = DA9xxx_REG_BUCK_BUCK2_5,
+		.vsel_mask = DA9121_MASK_BUCK_BUCKx_5_CHx_A_VOUT,
+	}
+};
+
+static const struct regulator_desc da9122_reg[2] = {
+	{
+		.id = DA9121_IDX_BUCK1,
+		.name = "DA9122/DA9131 BUCK1",
+		.of_match = "buck1",
+		.owner = THIS_MODULE,
+		.regulators_node = of_match_ptr("regulators"),
+		.ops = &da9121_buck_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = DA9121_N_VOLTAGES,
+		.min_uV = DA9121_MIN_MV * 1000,
+		.uV_step = DA9121_STEP_MV * 1000,
+		.linear_min_sel = DA9121_MIN_SEL,
+		.enable_reg = DA9121_REG_BUCK_BUCK1_0,
+		.enable_mask = DA9121_MASK_BUCK_BUCKx_0_CHx_EN,
+		.vsel_reg = DA9121_REG_BUCK_BUCK1_5,
+		.vsel_mask = DA9121_MASK_BUCK_BUCKx_5_CHx_A_VOUT,
+	},
+	{
+		.id = DA9121_IDX_BUCK2,
+		.name = "DA9122/DA9131 BUCK2",
+		.of_match = "buck2",
+		.owner = THIS_MODULE,
+		.regulators_node = of_match_ptr("regulators"),
+		.ops = &da9121_buck_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = DA9121_N_VOLTAGES,
+		.min_uV = DA9121_MIN_MV * 1000,
+		.uV_step = DA9121_STEP_MV * 1000,
+		.linear_min_sel = DA9121_MIN_SEL,
+		.enable_reg = DA9xxx_REG_BUCK_BUCK2_0,
+		.enable_mask = DA9121_MASK_BUCK_BUCKx_0_CHx_EN,
+		.vsel_reg = DA9xxx_REG_BUCK_BUCK2_5,
+		.vsel_mask = DA9121_MASK_BUCK_BUCKx_5_CHx_A_VOUT,
+	}
+};
+
+static const struct regulator_desc da9217_reg = {
+	.id = DA9121_IDX_BUCK1,
+	.name = "DA9217 BUCK1",
+	.of_match = "buck1",
+	.owner = THIS_MODULE,
+	.regulators_node = of_match_ptr("regulators"),
+	.ops = &da9121_buck_ops,
+	.type = REGULATOR_VOLTAGE,
+	.n_voltages = DA9121_N_VOLTAGES,
+	.min_uV = DA9121_MIN_MV * 1000,
+	.uV_step = DA9121_STEP_MV * 1000,
+	.linear_min_sel = DA9121_MIN_SEL,
+	.enable_reg = DA9121_REG_BUCK_BUCK1_0,
+	.enable_mask = DA9121_MASK_BUCK_BUCKx_0_CHx_EN,
+	.vsel_reg = DA9121_REG_BUCK_BUCK1_5,
+	.vsel_mask = DA9121_MASK_BUCK_BUCKx_5_CHx_A_VOUT,
+};
+
+static const struct regulator_desc *local_da9121_regulators[][DA9121_IDX_MAX] = {
+	[DA9121_TYPE_DA9121_DA9130] = { &da9121_reg, NULL },
+	[DA9121_TYPE_DA9220_DA9132] = { &da9220_reg[0], &da9220_reg[1] },
+	[DA9121_TYPE_DA9122_DA9131] = { &da9122_reg[0], &da9122_reg[1] },
+	[DA9121_TYPE_DA9217] = { &da9217_reg, NULL },
+};
+
+/* DA9121 chip register model */
+static const struct regmap_range da9121_1ch_readable_ranges[] = {
+	regmap_reg_range(DA9121_REG_SYS_STATUS_0, DA9121_REG_SYS_MASK_3),
+	regmap_reg_range(DA9121_REG_SYS_CONFIG_2, DA9121_REG_SYS_CONFIG_3),
+	regmap_reg_range(DA9121_REG_SYS_GPIO0_0, DA9121_REG_SYS_GPIO2_1),
+	regmap_reg_range(DA9121_REG_BUCK_BUCK1_0, DA9121_REG_BUCK_BUCK1_6),
+	regmap_reg_range(DA9121_REG_OTP_DEVICE_ID, DA9121_REG_OTP_CONFIG_ID),
+};
+
+static const struct regmap_access_table da9121_1ch_readable_table = {
+	.yes_ranges = da9121_1ch_readable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(da9121_1ch_readable_ranges),
+};
+
+static const struct regmap_range da9121_2ch_readable_ranges[] = {
+	regmap_reg_range(DA9121_REG_SYS_STATUS_0, DA9121_REG_SYS_MASK_3),
+	regmap_reg_range(DA9121_REG_SYS_CONFIG_2, DA9121_REG_SYS_CONFIG_3),
+	regmap_reg_range(DA9121_REG_SYS_GPIO0_0, DA9121_REG_SYS_GPIO2_1),
+	regmap_reg_range(DA9121_REG_BUCK_BUCK1_0, DA9121_REG_BUCK_BUCK1_7),
+	regmap_reg_range(DA9xxx_REG_BUCK_BUCK2_0, DA9xxx_REG_BUCK_BUCK2_7),
+	regmap_reg_range(DA9121_REG_OTP_DEVICE_ID, DA9121_REG_OTP_CONFIG_ID),
+};
+
+static const struct regmap_access_table da9121_2ch_readable_table = {
+	.yes_ranges = da9121_2ch_readable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(da9121_2ch_readable_ranges),
+};
+
+static const struct regmap_range da9121_1ch_writeable_ranges[] = {
+	regmap_reg_range(DA9121_REG_SYS_EVENT_0, DA9121_REG_SYS_MASK_3),
+	regmap_reg_range(DA9121_REG_SYS_CONFIG_2, DA9121_REG_SYS_CONFIG_3),
+	regmap_reg_range(DA9121_REG_SYS_GPIO0_0, DA9121_REG_SYS_GPIO2_1),
+	regmap_reg_range(DA9121_REG_BUCK_BUCK1_0, DA9121_REG_BUCK_BUCK1_2),
+	regmap_reg_range(DA9121_REG_BUCK_BUCK1_4, DA9121_REG_BUCK_BUCK1_6),
+};
+
+static const struct regmap_access_table da9121_1ch_writeable_table = {
+	.yes_ranges = da9121_1ch_writeable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(da9121_1ch_writeable_ranges),
+};
+
+static const struct regmap_range da9121_2ch_writeable_ranges[] = {
+	regmap_reg_range(DA9121_REG_SYS_EVENT_0, DA9121_REG_SYS_MASK_3),
+	regmap_reg_range(DA9121_REG_SYS_CONFIG_2, DA9121_REG_SYS_CONFIG_3),
+	regmap_reg_range(DA9121_REG_SYS_GPIO0_0, DA9121_REG_SYS_GPIO2_1),
+	regmap_reg_range(DA9121_REG_BUCK_BUCK1_0, DA9121_REG_BUCK_BUCK1_2),
+	regmap_reg_range(DA9121_REG_BUCK_BUCK1_4, DA9121_REG_BUCK_BUCK1_7),
+	regmap_reg_range(DA9xxx_REG_BUCK_BUCK2_0, DA9xxx_REG_BUCK_BUCK2_2),
+	regmap_reg_range(DA9xxx_REG_BUCK_BUCK2_4, DA9xxx_REG_BUCK_BUCK2_7),
+};
+
+static const struct regmap_access_table da9121_2ch_writeable_table = {
+	.yes_ranges = da9121_2ch_writeable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(da9121_2ch_writeable_ranges),
+};
+
+
+static const struct regmap_range da9121_volatile_ranges[] = {
+	regmap_reg_range(DA9121_REG_SYS_STATUS_0, DA9121_REG_SYS_EVENT_2),
+	regmap_reg_range(DA9121_REG_SYS_GPIO0_0, DA9121_REG_SYS_GPIO2_1),
+	regmap_reg_range(DA9121_REG_BUCK_BUCK1_0, DA9121_REG_BUCK_BUCK1_6),
+};
+
+static const struct regmap_access_table da9121_volatile_table = {
+	.yes_ranges = da9121_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(da9121_volatile_ranges),
+};
+
+/* DA9121 regmap config for 1 channel variants */
+static struct regmap_config da9121_1ch_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = DA9121_REG_OTP_CONFIG_ID,
+	.rd_table = &da9121_1ch_readable_table,
+	.wr_table = &da9121_1ch_writeable_table,
+	.volatile_table = &da9121_volatile_table,
+	.cache_type = REGCACHE_RBTREE,
+};
+
+/* DA9121 regmap config for 2 channel variants */
+static struct regmap_config da9121_2ch_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = DA9121_REG_OTP_CONFIG_ID,
+	.rd_table = &da9121_2ch_readable_table,
+	.wr_table = &da9121_2ch_writeable_table,
+	.volatile_table = &da9121_volatile_table,
+	.cache_type = REGCACHE_RBTREE,
+};
+
+static int da9121_i2c_reg_read(struct i2c_client *client, u8 addr,
+				    u8 *buf, int count)
+{
+	struct i2c_msg xfer[2];
+	int ret;
+
+	xfer[0].addr = client->addr;
+	xfer[0].flags = 0;
+	xfer[0].len = 1;
+	xfer[0].buf = &addr;
+
+	xfer[1].addr = client->addr;
+	xfer[1].flags = I2C_M_RD;
+	xfer[1].len = 1;
+	xfer[1].buf = buf;
+
+
+	ret = i2c_transfer(client->adapter, xfer, 2);
+	if (ret < 0) {
+		dev_err(&client->dev, "Device read failed: %d\n", ret);
+		return ret;
+	}
+
+	if (ret != 2) {
+		dev_err(&client->dev, "Device read failed to complete\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int da9121_get_device_type(struct i2c_client *i2c, struct da9121 *chip)
+{
+	u8 device_id;
+	u8 chip_id = chip->variant_id;
+	u8 variant_id;
+	u8 variant_mrc, variant_vrc;
+	char *type;
+	const char *name;
+	bool config_match = false;
+	int ret = 0;
+
+	ret = da9121_i2c_reg_read(i2c, DA9121_REG_OTP_DEVICE_ID,
+				    &device_id, 1);
+	if (ret < 0) {
+		dev_err(chip->dev, "Failed to read device ID: %d\n", ret);
+		goto error;
+	}
+
+	ret = da9121_i2c_reg_read(i2c, DA9121_REG_OTP_VARIANT_ID,
+				    &variant_id, 1);
+	if (ret < 0) {
+		dev_err(chip->dev, "Cannot read chip variant ID: %d\n", ret);
+		goto error;
+	}
+
+	if (device_id != DA9121_DEVICE_ID) {
+		dev_err(chip->dev, "Invalid device ID: 0x%02x\n", device_id);
+		ret = -ENODEV;
+		goto error;
+	}
+
+	name = of_get_property(chip->dev->of_node, "compatible", NULL);
+	if (!name) {
+		dev_err(chip->dev, "Cannot get device not compatible string.\n");
+		goto error;
+	}
+
+	variant_vrc = variant_id & DA9121_MASK_OTP_VARIANT_ID_VRC;
+
+	switch (variant_vrc) {
+	case DA9121_VARIANT_VRC:
+		type = "DA9121/DA9130";
+		config_match = (chip_id == DA9121_TYPE_DA9121_DA9130);
+		break;
+	case DA9220_VARIANT_VRC:
+		type = "DA9220/DA9132";
+		config_match = (chip_id == DA9121_TYPE_DA9220_DA9132);
+		break;
+	case DA9122_VARIANT_VRC:
+		type = "DA9122/DA9131";
+		config_match = (chip_id == DA9121_TYPE_DA9122_DA9131);
+		break;
+	case DA9217_VARIANT_VRC:
+		type = "DA9217";
+		config_match = (chip_id == DA9121_TYPE_DA9217);
+		break;
+	default:
+		type = "Unknown";
+		break;
+	}
+
+	dev_info(chip->dev,
+		 "Device detected (device-ID: 0x%02X, var-ID: 0x%02X, %s)\n",
+		 device_id, variant_id, type);
+
+	if (!config_match) {
+		dev_err(chip->dev, "Device tree configuration '%s' does not match detected device.\n", name);
+		ret = -EINVAL;
+		goto error;
+	}
+
+	variant_mrc = (variant_id & DA9121_MASK_OTP_VARIANT_ID_MRC)
+			>> DA9121_SHIFT_OTP_VARIANT_ID_MRC;
+
+	if ((device_id == DA9121_DEVICE_ID) &&
+	    (variant_mrc < DA9121_VARIANT_MRC_BASE)) {
+		dev_err(chip->dev,
+			"Cannot support variant MRC: 0x%02X\n", variant_mrc);
+		ret = -EINVAL;
+	}
+error:
+	return ret;
+}
+
+static int da9121_assign_chip_model(struct i2c_client *i2c,
+			struct da9121 *chip)
+{
+	struct regmap_config *regmap;
+	int ret = 0;
+
+	chip->dev = &i2c->dev;
+
+	ret = da9121_get_device_type(i2c, chip);
+	if (ret)
+		return ret;
+
+	switch (chip->variant_id) {
+	case DA9121_TYPE_DA9121_DA9130:
+		fallthrough;
+	case DA9121_TYPE_DA9217:
+		regmap = &da9121_1ch_regmap_config;
+		break;
+	case DA9121_TYPE_DA9122_DA9131:
+		fallthrough;
+	case DA9121_TYPE_DA9220_DA9132:
+		regmap = &da9121_2ch_regmap_config;
+		break;
+	}
+
+	/* Set these up for of_regulator_match call which may want .of_map_modes */
+	da9121_matches[0].desc = local_da9121_regulators[chip->variant_id][0];
+	da9121_matches[1].desc = local_da9121_regulators[chip->variant_id][1];
+
+	chip->regmap = devm_regmap_init_i2c(i2c, regmap);
+	if (IS_ERR(chip->regmap)) {
+		ret = PTR_ERR(chip->regmap);
+		dev_err(chip->dev, "Failed to configure a register map: %d\n",
+			ret);
+	}
+
+	return ret;
+}
+
 static const struct of_device_id da9121_dt_ids[] = {
 	{ .compatible = "dlg,da9121", .data = (void *) DA9121_TYPE_DA9121_DA9130 },
 	{ .compatible = "dlg,da9130", .data = (void *) DA9121_TYPE_DA9121_DA9130 },
@@ -89,7 +438,6 @@ static int da9121_i2c_probe(struct i2c_client *i2c,
 	struct device *dev = &i2c->dev;
 	struct regulator_config config = {};
 	struct regulator_dev *rdev;
-	struct regmap *regmap;
 
 	chip = devm_kzalloc(&i2c->dev, sizeof(struct da9121), GFP_KERNEL);
 	if (!chip) {
@@ -99,13 +447,13 @@ static int da9121_i2c_probe(struct i2c_client *i2c,
 
 	chip->variant_id = da9121_of_get_id(&i2c->dev);
 
-	regmap = devm_regmap_init_i2c(i2c, &da9121_regmap_config);
-	if (IS_ERR(regmap))
-		return PTR_ERR(regmap);
+	ret = da9121_assign_chip_model(i2c, chip);
+	if (ret < 0)
+		goto error;
 
 	config.dev = &i2c->dev;
 	config.of_node = dev->of_node;
-	config.regmap = regmap;
+	config.regmap = chip->regmap;
 
 	rdev = devm_regulator_register(&i2c->dev, &da9121_reg, &config);
 	if (IS_ERR(rdev)) {
diff --git a/include/linux/regulator/da9121.h b/include/linux/regulator/da9121.h
new file mode 100644
index 0000000..c31180d
--- /dev/null
+++ b/include/linux/regulator/da9121.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * DA9121 Single-channel dual-phase 10A buck converter
+ * DA9130 Single-channel dual-phase 10A buck converter (Automotive)
+ * DA9217 Single-channel dual-phase  6A buck converter
+ * DA9122 Dual-channel single-phase  5A buck converter
+ * DA9131 Dual-channel single-phase  5A buck converter (Automotive)
+ * DA9220 Dual-channel single-phase  3A buck converter
+ * DA9132 Dual-channel single-phase  3A buck converter (Automotive)
+ *
+ * Copyright (C) 2020  Dialog Semiconductor
+ *
+ * Authors: Adam Ward, Dialog Semiconductor
+ */
+
+#ifndef __LINUX_REGULATOR_DA9121_H
+#define __LINUX_REGULATOR_DA9121_H
+
+enum {
+	DA9121_IDX_BUCK1,
+	DA9121_IDX_BUCK2,
+	DA9121_IDX_MAX
+};
+
+#endif
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/9] regulator: da9121: Add support for device variants via devicetree
  2020-11-20 12:14 [PATCH 0/9] regulator: da9121: extend support to variants, add features Adam Ward
                   ` (3 preceding siblings ...)
  2020-11-20 12:14 ` [PATCH 4/9] regulator: da9121: Add device variant details and respective regmaps Adam Ward
@ 2020-11-20 12:14 ` Adam Ward
  2020-11-20 12:51   ` Mark Brown
  2020-11-20 12:14 ` [PATCH 6/9] regulator: da9121: Update registration to support multiple buck variants Adam Ward
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Adam Ward @ 2020-11-20 12:14 UTC (permalink / raw)
  To: Mark Brown, Rob Herring
  Cc: Liam Girdwood, Vincent Whitchurch, linux-kernel, devicetree

Add devicetree configuration and device variant parameters. Use the latter to
enable the check and use of parameters specific to dual buck variants.

Signed-off-by: Adam Ward <Adam.Ward.opensource@diasemi.com>
---
 drivers/regulator/da9121-regulator.c | 157 ++++++++++++++++++++++++++++++++++-
 include/linux/regulator/da9121.h     |  11 +++
 2 files changed, 167 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/da9121-regulator.c b/drivers/regulator/da9121-regulator.c
index 76932ba..5020774 100644
--- a/drivers/regulator/da9121-regulator.c
+++ b/drivers/regulator/da9121-regulator.c
@@ -1,7 +1,21 @@
 // SPDX-License-Identifier: GPL-2.0-only
-/* Copyright (C) 2020 Axis Communications AB */
+/* Copyright (C) 2020 Axis Communications AB
+ *
+ * DA9121 Single-channel dual-phase 10A buck converter
+ *
+ * Copyright (C) 2020 Dialog Semiconductor
+ *
+ * DA9130 Single-channel dual-phase 10A buck converter (Automotive)
+ * DA9217 Single-channel dual-phase  6A buck converter
+ * DA9122 Dual-channel single-phase  5A buck converter
+ * DA9131 Dual-channel single-phase  5A buck converter (Automotive)
+ * DA9220 Dual-channel single-phase  3A buck converter
+ * DA9132 Dual-channel single-phase  3A buck converter (Automotive)
+ *
+ */
 
 #include <linux/of_device.h>
+#include <linux/of_gpio.h>
 #include <linux/regulator/of_regulator.h>
 #include <linux/regulator/machine.h>
 #include <linux/regulator/driver.h>
@@ -16,11 +30,68 @@
 /* Chip data */
 struct da9121 {
 	struct device *dev;
+	struct da9121_pdata *pdata;
 	struct regmap *regmap;
 	struct regulator_dev *rdev[DA9121_IDX_MAX];
 	int variant_id;
 };
 
+/* Define ranges for different variants, enabling translation to/from
+ * registers. Maximums give scope to allow for transients.
+ */
+struct da9121_range {
+	int val_min;
+	int val_max;
+	int val_stp;
+	int reg_min;
+	int reg_max;
+};
+
+struct da9121_range da9121_10A_2phase_current = {
+	.val_min =  7000000,
+	.val_max = 20000000,
+	.val_stp =  1000000,
+	.reg_min = 1,
+	.reg_max = 14,
+};
+
+struct da9121_range da9121_6A_2phase_current = {
+	.val_min =  7000000,
+	.val_max = 12000000,
+	.val_stp =  1000000,
+	.reg_min = 1,
+	.reg_max = 6,
+};
+
+struct da9121_range da9121_5A_1phase_current = {
+	.val_min =  3500000,
+	.val_max = 10000000,
+	.val_stp =   500000,
+	.reg_min = 1,
+	.reg_max = 14,
+};
+
+struct da9121_range da9121_3A_1phase_current = {
+	.val_min = 3500000,
+	.val_max = 6000000,
+	.val_stp =  500000,
+	.reg_min = 1,
+	.reg_max = 6,
+};
+
+struct da9121_variant_info {
+	int num_bucks;
+	int num_phases;
+	struct da9121_range *current_range;
+};
+
+static const struct da9121_variant_info variant_parameters[] = {
+	{ 1, 2, &da9121_10A_2phase_current },	//DA9121_TYPE_DA9121_DA9130
+	{ 2, 1, &da9121_3A_1phase_current  },	//DA9121_TYPE_DA9220_DA9132
+	{ 2, 1, &da9121_5A_1phase_current  },	//DA9121_TYPE_DA9122_DA9131
+	{ 1, 2, &da9121_6A_2phase_current  },	//DA9121_TYPE_DA9217
+};
+
 static const struct regulator_ops da9121_buck_ops = {
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -163,6 +234,86 @@ struct da9121 {
 	[DA9121_TYPE_DA9217] = { &da9217_reg, NULL },
 };
 
+#ifdef CONFIG_OF
+static struct da9121_pdata *da9121_parse_regulators_dt(struct da9121 *chip)
+{
+	struct da9121_pdata *pdata;
+	struct device_node *node;
+	int num = 0;
+	int ret = 0;
+	int i, n;
+	enum gpiod_flags flags = GPIOD_OUT_HIGH | GPIOD_FLAGS_BIT_NONEXCLUSIVE;
+
+	node = of_get_child_by_name(chip->dev->of_node, "regulators");
+	if (!node) {
+		dev_err(chip->dev, "Regulators node not found\n");
+		return ERR_PTR(-ENODEV);
+	}
+
+	num = of_regulator_match(chip->dev, node, da9121_matches,
+				 ARRAY_SIZE(da9121_matches));
+	of_node_put(node);
+	if (num < 0) {
+		dev_err(chip->dev, "Failed to match regulators\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	/* interrupt assumptions require at least one buck to be configured */
+	if (num == 0) {
+		dev_err(chip->dev, "Did not match any regulators in the DT\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	pdata = devm_kzalloc(chip->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	pdata->num_buck = num;
+
+	n = 0;
+	for (i = 0; i < ARRAY_SIZE(da9121_matches); i++) {
+		if (!da9121_matches[i].init_data)
+			continue;
+
+		pdata->init_data[n] = da9121_matches[i].init_data;
+		pdata->reg_node[n] = da9121_matches[i].of_node;
+		pdata->gpiod_ren[n] = devm_gpiod_get_from_of_node(chip->dev,
+				da9121_matches[i].of_node,
+				"enable-gpios",
+				0,
+				flags,
+				"da9121-enable");
+
+		if (IS_ERR(pdata->gpiod_ren[n]))
+			pdata->gpiod_ren[n] = NULL;
+
+		if (variant_parameters[chip->variant_id].num_bucks == 2) {
+			uint32_t ripple_cancel;
+			uint32_t reg = (i ? DA9xxx_REG_BUCK_BUCK2_7
+					  : DA9121_REG_BUCK_BUCK1_7);
+			if (!of_property_read_u32(da9121_matches[i].of_node,
+				  "dlg,ripple-cancel",
+				  &ripple_cancel)) {
+				//write to BUCK_BUCKx_7 : CHx_RIPPLE_CANCEL
+				ret = regmap_update_bits(chip->regmap, reg,
+					DA9xxx_MASK_BUCK_BUCKx_7_CHx_RIPPLE_CANCEL,
+					ripple_cancel);
+				if (ret < 0)
+					dev_err(chip->dev, "Cannot update BUCK register %02x, err: %d\n", reg, ret);
+			}
+		}
+		n++;
+	}
+
+	return pdata;
+}
+#else
+static struct da9121_pdata *da9121_parse_regulators_dt(struct da9121 *chip)
+{
+	return ERR_PTR(-ENODEV);
+}
+#endif
+
 /* DA9121 chip register model */
 static const struct regmap_range da9121_1ch_readable_ranges[] = {
 	regmap_reg_range(DA9121_REG_SYS_STATUS_0, DA9121_REG_SYS_MASK_3),
@@ -445,12 +596,16 @@ static int da9121_i2c_probe(struct i2c_client *i2c,
 		goto error;
 	}
 
+	chip->pdata = i2c->dev.platform_data;
 	chip->variant_id = da9121_of_get_id(&i2c->dev);
 
 	ret = da9121_assign_chip_model(i2c, chip);
 	if (ret < 0)
 		goto error;
 
+	if (!chip->pdata)
+		chip->pdata = da9121_parse_regulators_dt(chip);
+
 	config.dev = &i2c->dev;
 	config.of_node = dev->of_node;
 	config.regmap = chip->regmap;
diff --git a/include/linux/regulator/da9121.h b/include/linux/regulator/da9121.h
index c31180d..62d9d257 100644
--- a/include/linux/regulator/da9121.h
+++ b/include/linux/regulator/da9121.h
@@ -16,10 +16,21 @@
 #ifndef __LINUX_REGULATOR_DA9121_H
 #define __LINUX_REGULATOR_DA9121_H
 
+#include <linux/regulator/machine.h>
+
+struct gpio_desc;
+
 enum {
 	DA9121_IDX_BUCK1,
 	DA9121_IDX_BUCK2,
 	DA9121_IDX_MAX
 };
 
+struct da9121_pdata {
+	int num_buck;
+	struct gpio_desc *gpiod_ren[DA9121_IDX_MAX];
+	struct device_node *reg_node[DA9121_IDX_MAX];
+	struct regulator_init_data *init_data[DA9121_IDX_MAX];
+};
+
 #endif
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/9] regulator: da9121: Update registration to support multiple buck variants
  2020-11-20 12:14 [PATCH 0/9] regulator: da9121: extend support to variants, add features Adam Ward
                   ` (4 preceding siblings ...)
  2020-11-20 12:14 ` [PATCH 5/9] regulator: da9121: Add support for device variants via devicetree Adam Ward
@ 2020-11-20 12:14 ` Adam Ward
  2020-11-20 13:06   ` Mark Brown
  2020-11-20 12:14 ` [PATCH 7/9] regulator: da9121: add current support Adam Ward
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Adam Ward @ 2020-11-20 12:14 UTC (permalink / raw)
  To: Mark Brown, Rob Herring
  Cc: Liam Girdwood, Vincent Whitchurch, linux-kernel, devicetree

Checks DT matches tally with variant maximum and register accordingly.

Signed-off-by: Adam Ward <Adam.Ward.opensource@diasemi.com>
---
 drivers/regulator/da9121-regulator.c | 81 +++++++++++++++++++++++++++++++-----
 1 file changed, 70 insertions(+), 11 deletions(-)

diff --git a/drivers/regulator/da9121-regulator.c b/drivers/regulator/da9121-regulator.c
index 5020774..13b0aad 100644
--- a/drivers/regulator/da9121-regulator.c
+++ b/drivers/regulator/da9121-regulator.c
@@ -314,6 +314,69 @@ static struct da9121_pdata *da9121_parse_regulators_dt(struct da9121 *chip)
 }
 #endif
 
+static int da9121_set_regulator_config(struct da9121 *chip)
+{
+	struct regulator_config config = { };
+	unsigned int max_matches = chip->pdata->num_buck;
+	int ret = 0;
+	int i;
+
+	if (max_matches > variant_parameters[chip->variant_id].num_bucks) {
+		dev_err(chip->dev, "Too many regulators in the DT\n");
+		ret = -EINVAL;
+		goto error;
+	}
+
+	for (i = 0; i < max_matches; i++) {
+		const struct regulator_desc *regl_desc =
+			local_da9121_regulators[chip->variant_id][i];
+		int id = regl_desc->id;
+		struct gpio_desc *gpio_ren;
+
+		if (chip->pdata->gpiod_ren[i])
+			gpio_ren = chip->pdata->gpiod_ren[i];
+		else
+			gpio_ren = NULL;
+
+		config.init_data = chip->pdata->init_data[i];
+		config.dev = chip->dev;
+		config.driver_data = chip;
+		config.regmap = chip->regmap;
+		config.of_node = chip->pdata->reg_node[i];
+
+		switch (id) {
+		case DA9121_IDX_BUCK1:
+		case DA9121_IDX_BUCK2:
+			config.ena_gpiod = gpio_ren;
+			break;
+		default:
+			dev_err(chip->dev, "Invalid regulator ID\n");
+			ret = -EINVAL;
+			goto error;
+		}
+
+		/*
+		 * Hand the GPIO descriptor management over to the regulator
+		 * core, remove it from GPIO devres management.
+		 */
+		if (config.ena_gpiod)
+			devm_gpiod_unhinge(chip->dev, config.ena_gpiod);
+
+		chip->rdev[i] = devm_regulator_register(chip->dev,
+					regl_desc,
+					&config);
+		if (IS_ERR(chip->rdev[i])) {
+			dev_err(chip->dev, "Failed to register regulator %s, %d/%d of_map_mode:%p\n",
+				regl_desc->name, (i+1), max_matches, regl_desc->of_map_mode);
+			ret = PTR_ERR(chip->rdev[i]);
+			goto error;
+		}
+	}
+
+error:
+	return ret;
+}
+
 /* DA9121 chip register model */
 static const struct regmap_range da9121_1ch_readable_ranges[] = {
 	regmap_reg_range(DA9121_REG_SYS_STATUS_0, DA9121_REG_SYS_MASK_3),
@@ -586,9 +649,6 @@ static int da9121_i2c_probe(struct i2c_client *i2c,
 {
 	struct da9121 *chip;
 	int ret = 0;
-	struct device *dev = &i2c->dev;
-	struct regulator_config config = {};
-	struct regulator_dev *rdev;
 
 	chip = devm_kzalloc(&i2c->dev, sizeof(struct da9121), GFP_KERNEL);
 	if (!chip) {
@@ -606,16 +666,15 @@ static int da9121_i2c_probe(struct i2c_client *i2c,
 	if (!chip->pdata)
 		chip->pdata = da9121_parse_regulators_dt(chip);
 
-	config.dev = &i2c->dev;
-	config.of_node = dev->of_node;
-	config.regmap = chip->regmap;
-
-	rdev = devm_regulator_register(&i2c->dev, &da9121_reg, &config);
-	if (IS_ERR(rdev)) {
-		dev_err(&i2c->dev, "Failed to register da9121 regulator\n");
-		return PTR_ERR(rdev);
+	if (IS_ERR(chip->pdata)) {
+		dev_err(chip->dev, "No regulators defined for the platform\n");
+		return PTR_ERR(chip->pdata);
 	}
 
+	ret = da9121_set_regulator_config(chip);
+	if (ret < 0)
+		goto error;
+
 error:
 	return ret;
 }
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 7/9] regulator: da9121: add current support
  2020-11-20 12:14 [PATCH 0/9] regulator: da9121: extend support to variants, add features Adam Ward
                   ` (5 preceding siblings ...)
  2020-11-20 12:14 ` [PATCH 6/9] regulator: da9121: Update registration to support multiple buck variants Adam Ward
@ 2020-11-20 12:14 ` Adam Ward
  2020-11-20 13:17   ` Mark Brown
  2020-11-20 12:14 ` [PATCH 8/9] regulator: da9121: add mode support Adam Ward
  2020-11-20 12:14 ` [PATCH 9/9] regulator: da9121: add interrupt support Adam Ward
  8 siblings, 1 reply; 19+ messages in thread
From: Adam Ward @ 2020-11-20 12:14 UTC (permalink / raw)
  To: Mark Brown, Rob Herring
  Cc: Liam Girdwood, Vincent Whitchurch, linux-kernel, devicetree

This commit adds support for getting/setting current for all supported
variants. Limits are adjusted per variant to match HW implementation.

Signed-off-by: Adam Ward <Adam.Ward.opensource@diasemi.com>
---
 drivers/regulator/da9121-regulator.c | 140 +++++++++++++++++++++++++++++++++++
 1 file changed, 140 insertions(+)

diff --git a/drivers/regulator/da9121-regulator.c b/drivers/regulator/da9121-regulator.c
index 13b0aad..5d11c22 100644
--- a/drivers/regulator/da9121-regulator.c
+++ b/drivers/regulator/da9121-regulator.c
@@ -92,6 +92,144 @@ struct da9121_variant_info {
 	{ 1, 2, &da9121_6A_2phase_current  },	//DA9121_TYPE_DA9217
 };
 
+static bool da9121_rdev_to_buck_reg_mask(struct regulator_dev *rdev, bool mode,
+					 unsigned int *reg, unsigned int *msk)
+{
+	struct da9121 *chip = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+
+	switch (id) {
+	case DA9121_IDX_BUCK1:
+		if (mode) {
+			*reg = DA9121_REG_BUCK_BUCK1_4;
+			*msk = DA9121_MASK_BUCK_BUCKx_4_CHx_A_MODE;
+		} else {
+			*reg = DA9121_REG_BUCK_BUCK1_2;
+			*msk = DA9121_MASK_BUCK_BUCKx_2_CHx_ILIM;
+		}
+		break;
+	case DA9121_IDX_BUCK2:
+		if (mode) {
+			*reg = DA9xxx_REG_BUCK_BUCK2_4;
+			*msk = DA9121_MASK_BUCK_BUCKx_4_CHx_A_MODE;
+		} else {
+			*reg = DA9xxx_REG_BUCK_BUCK2_2;
+			*msk = DA9121_MASK_BUCK_BUCKx_2_CHx_ILIM;
+		}
+		break;
+	default:
+		dev_err(chip->dev, "Invalid regulator ID\n");
+		return false;
+	}
+	return true;
+}
+
+static int da9121_get_current_limit(struct regulator_dev *rdev)
+{
+	struct da9121 *chip = rdev_get_drvdata(rdev);
+	struct da9121_range *range =
+		variant_parameters[chip->variant_id].current_range;
+	unsigned int reg = 0;
+	unsigned int msk = 0;
+	unsigned int val = 0;
+	int ret = 0;
+
+	if (!da9121_rdev_to_buck_reg_mask(rdev, false, &reg, &msk))
+		return -EINVAL;
+
+	ret = regmap_read(chip->regmap, reg, &val);
+	if (ret < 0) {
+		dev_err(chip->dev, "Cannot read BUCK register: %d\n", ret);
+		goto error;
+	}
+
+	if (val < range->reg_min) {
+		ret = -EACCES;
+		goto error;
+	}
+
+	if (val > range->reg_max) {
+		ret = -EINVAL;
+		goto error;
+	}
+
+	return range->val_min + (range->val_stp * (val - range->reg_min));
+error:
+	return ret;
+}
+
+static int da9121_ceiling_selector(struct regulator_dev *rdev,
+		int min, int max,
+		unsigned int *selector)
+{
+	struct da9121 *chip = rdev_get_drvdata(rdev);
+	struct da9121_range *range =
+		variant_parameters[chip->variant_id].current_range;
+	unsigned int level;
+	unsigned int i = 0;
+	unsigned int sel = 0;
+	int ret = 0;
+
+	if (range->val_min > max || range->val_max < min) {
+		dev_err(chip->dev,
+			"Requested current out of regulator capability\n");
+		ret = -EINVAL;
+		goto error;
+	}
+
+	level = range->val_max;
+	for (i = range->reg_max; i >= range->reg_min; i--) {
+		if (level <= max) {
+			sel = i;
+			break;
+		}
+		level -= range->val_stp;
+	}
+
+	if (level < min) {
+		dev_err(chip->dev,
+			"Best match falls below minimum requested current\n");
+		ret = -EINVAL;
+		goto error;
+	}
+
+	*selector = sel;
+error:
+	return ret;
+}
+
+static int da9121_set_current_limit(struct regulator_dev *rdev,
+				int min_ua, int max_ua)
+{
+	struct da9121 *chip = rdev_get_drvdata(rdev);
+	struct da9121_range *range =
+		variant_parameters[chip->variant_id].current_range;
+	unsigned int sel = 0;
+	unsigned int reg = 0;
+	unsigned int msk = 0;
+	int ret = 0;
+
+	if (min_ua < range->val_min ||
+	    max_ua > range->val_max) {
+		ret = -EINVAL;
+		goto error;
+	}
+
+	ret = da9121_ceiling_selector(rdev, min_ua, max_ua, &sel);
+	if (ret < 0)
+		goto error;
+
+	if (!da9121_rdev_to_buck_reg_mask(rdev, false, &reg, &msk))
+		return -EINVAL;
+
+	ret = regmap_update_bits(chip->regmap, reg, msk, (unsigned int)sel);
+	if (ret < 0)
+		dev_err(chip->dev, "Cannot update BUCK register %02x, err: %d\n", reg, ret);
+
+error:
+	return ret;
+}
+
 static const struct regulator_ops da9121_buck_ops = {
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -99,6 +237,8 @@ struct da9121_variant_info {
 	.set_voltage_sel = regulator_set_voltage_sel_regmap,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
 	.list_voltage = regulator_list_voltage_linear,
+	.get_current_limit = da9121_get_current_limit,
+	.set_current_limit = da9121_set_current_limit,
 };
 
 static struct of_regulator_match da9121_matches[] = {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 8/9] regulator: da9121: add mode support
  2020-11-20 12:14 [PATCH 0/9] regulator: da9121: extend support to variants, add features Adam Ward
                   ` (6 preceding siblings ...)
  2020-11-20 12:14 ` [PATCH 7/9] regulator: da9121: add current support Adam Ward
@ 2020-11-20 12:14 ` Adam Ward
  2020-11-20 12:14 ` [PATCH 9/9] regulator: da9121: add interrupt support Adam Ward
  8 siblings, 0 replies; 19+ messages in thread
From: Adam Ward @ 2020-11-20 12:14 UTC (permalink / raw)
  To: Mark Brown, Rob Herring
  Cc: Liam Girdwood, Vincent Whitchurch, linux-kernel, devicetree

Adds get/set for mode, and mapping from REGULATOR_MODE_* to select
PFM/PWM/Auto operation.

Signed-off-by: Adam Ward <Adam.Ward.opensource@diasemi.com>
---
 drivers/regulator/da9121-regulator.c | 74 ++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/drivers/regulator/da9121-regulator.c b/drivers/regulator/da9121-regulator.c
index 5d11c22..3addbd2 100644
--- a/drivers/regulator/da9121-regulator.c
+++ b/drivers/regulator/da9121-regulator.c
@@ -230,6 +230,72 @@ static int da9121_set_current_limit(struct regulator_dev *rdev,
 	return ret;
 }
 
+static unsigned int da9121_map_mode(unsigned int mode)
+{
+	switch (mode) {
+	case DA9121_BUCK_MODE_FORCE_PWM:
+		return REGULATOR_MODE_FAST;
+	case DA9121_BUCK_MODE_FORCE_PWM_SHEDDING:
+		return REGULATOR_MODE_NORMAL;
+	case DA9121_BUCK_MODE_AUTO:
+		return REGULATOR_MODE_IDLE;
+	case DA9121_BUCK_MODE_FORCE_PFM:
+		return REGULATOR_MODE_STANDBY;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int da9121_buck_set_mode(struct regulator_dev *rdev, unsigned int mode)
+{
+	struct da9121 *chip = rdev_get_drvdata(rdev);
+	unsigned int val;
+	unsigned int reg;
+	unsigned int msk;
+
+	switch (mode) {
+	case REGULATOR_MODE_FAST:
+		val = DA9121_BUCK_MODE_FORCE_PWM;
+		break;
+	case REGULATOR_MODE_NORMAL:
+		val = DA9121_BUCK_MODE_FORCE_PWM_SHEDDING;
+		break;
+	case REGULATOR_MODE_IDLE:
+		val = DA9121_BUCK_MODE_AUTO;
+		break;
+	case REGULATOR_MODE_STANDBY:
+		val = DA9121_BUCK_MODE_FORCE_PFM;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (!da9121_rdev_to_buck_reg_mask(rdev, true, &reg, &msk))
+		return -EINVAL;
+
+	return regmap_update_bits(chip->regmap, reg, msk, val);
+}
+
+static unsigned int da9121_buck_get_mode(struct regulator_dev *rdev)
+{
+	struct da9121 *chip = rdev_get_drvdata(rdev);
+	unsigned int reg;
+	unsigned int msk;
+	unsigned int val;
+	int ret = 0;
+
+	if (!da9121_rdev_to_buck_reg_mask(rdev, true, &reg, &msk))
+		return -EINVAL;
+
+	ret = regmap_read(chip->regmap, reg, &val);
+	if (ret < 0) {
+		dev_err(chip->dev, "Cannot read BUCK register: %d\n", ret);
+		return -EINVAL;
+	}
+
+	return da9121_map_mode(val & msk);
+}
+
 static const struct regulator_ops da9121_buck_ops = {
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -239,6 +305,8 @@ static int da9121_set_current_limit(struct regulator_dev *rdev,
 	.list_voltage = regulator_list_voltage_linear,
 	.get_current_limit = da9121_get_current_limit,
 	.set_current_limit = da9121_set_current_limit,
+	.set_mode = da9121_buck_set_mode,
+	.get_mode = da9121_buck_get_mode,
 };
 
 static struct of_regulator_match da9121_matches[] = {
@@ -258,6 +326,7 @@ static int da9121_set_current_limit(struct regulator_dev *rdev,
 	.name = "da9121",
 	.of_match = "buck1",
 	.owner = THIS_MODULE,
+	.of_map_mode = da9121_map_mode,
 	.regulators_node = of_match_ptr("regulators"),
 	.ops = &da9121_buck_ops,
 	.type = REGULATOR_VOLTAGE,
@@ -281,6 +350,7 @@ static int da9121_set_current_limit(struct regulator_dev *rdev,
 		.name = "DA9220/DA9132 BUCK1",
 		.of_match = "buck1",
 		.owner = THIS_MODULE,
+		.of_map_mode = da9121_map_mode,
 		.regulators_node = of_match_ptr("regulators"),
 		.ops = &da9121_buck_ops,
 		.type = REGULATOR_VOLTAGE,
@@ -298,6 +368,7 @@ static int da9121_set_current_limit(struct regulator_dev *rdev,
 		.name = "DA9220/DA9132 BUCK2",
 		.of_match = "buck2",
 		.owner = THIS_MODULE,
+		.of_map_mode = da9121_map_mode,
 		.regulators_node = of_match_ptr("regulators"),
 		.ops = &da9121_buck_ops,
 		.type = REGULATOR_VOLTAGE,
@@ -318,6 +389,7 @@ static int da9121_set_current_limit(struct regulator_dev *rdev,
 		.name = "DA9122/DA9131 BUCK1",
 		.of_match = "buck1",
 		.owner = THIS_MODULE,
+		.of_map_mode = da9121_map_mode,
 		.regulators_node = of_match_ptr("regulators"),
 		.ops = &da9121_buck_ops,
 		.type = REGULATOR_VOLTAGE,
@@ -335,6 +407,7 @@ static int da9121_set_current_limit(struct regulator_dev *rdev,
 		.name = "DA9122/DA9131 BUCK2",
 		.of_match = "buck2",
 		.owner = THIS_MODULE,
+		.of_map_mode = da9121_map_mode,
 		.regulators_node = of_match_ptr("regulators"),
 		.ops = &da9121_buck_ops,
 		.type = REGULATOR_VOLTAGE,
@@ -354,6 +427,7 @@ static int da9121_set_current_limit(struct regulator_dev *rdev,
 	.name = "DA9217 BUCK1",
 	.of_match = "buck1",
 	.owner = THIS_MODULE,
+	.of_map_mode = da9121_map_mode,
 	.regulators_node = of_match_ptr("regulators"),
 	.ops = &da9121_buck_ops,
 	.type = REGULATOR_VOLTAGE,
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 9/9] regulator: da9121: add interrupt support
  2020-11-20 12:14 [PATCH 0/9] regulator: da9121: extend support to variants, add features Adam Ward
                   ` (7 preceding siblings ...)
  2020-11-20 12:14 ` [PATCH 8/9] regulator: da9121: add mode support Adam Ward
@ 2020-11-20 12:14 ` Adam Ward
  2020-11-20 13:45   ` Mark Brown
  8 siblings, 1 reply; 19+ messages in thread
From: Adam Ward @ 2020-11-20 12:14 UTC (permalink / raw)
  To: Mark Brown, Rob Herring
  Cc: Liam Girdwood, Vincent Whitchurch, linux-kernel, devicetree

Adds interrupt handler for variants, and notifications for events; over
temperature/voltage/current.
Also handling of persistent events and respective timing configuration.

Signed-off-by: Adam Ward <Adam.Ward.opensource@diasemi.com>
---
 drivers/regulator/da9121-regulator.c | 548 +++++++++++++++++++++++++++++++++++
 1 file changed, 548 insertions(+)

diff --git a/drivers/regulator/da9121-regulator.c b/drivers/regulator/da9121-regulator.c
index 3addbd2..37a767e 100644
--- a/drivers/regulator/da9121-regulator.c
+++ b/drivers/regulator/da9121-regulator.c
@@ -24,15 +24,22 @@
 #include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/regulator/da9121.h>
+#include <linux/interrupt.h>
+#include <linux/workqueue.h>
 
 #include "da9121-regulator.h"
 
 /* Chip data */
 struct da9121 {
 	struct device *dev;
+	struct delayed_work work;
+	struct regmap *regmap;
 	struct da9121_pdata *pdata;
 	struct regmap *regmap;
 	struct regulator_dev *rdev[DA9121_IDX_MAX];
+	unsigned int persistent[2];
+	unsigned int passive_delay;
+	int chip_irq;
 	int variant_id;
 };
 
@@ -92,6 +99,104 @@ struct da9121_variant_info {
 	{ 1, 2, &da9121_6A_2phase_current  },	//DA9121_TYPE_DA9217
 };
 
+static void da9121_status_poll_on(struct work_struct *work)
+{
+	struct da9121 *chip = container_of(work, struct da9121, work.work);
+	enum { R0 = 0, R1, R2, REG_MAX_NUM };
+	int status[REG_MAX_NUM] = {0};
+	int clear[REG_MAX_NUM] = {0};
+	unsigned long delay;
+	int i;
+	int ret;
+
+	/* If persistent-notification, status will be true
+	 * If not persistent-notification any longer, status will be false
+	 */
+	ret = regmap_bulk_read(chip->regmap, DA9121_REG_SYS_STATUS_0,
+			(void *)status, (size_t)REG_MAX_NUM);
+	if (ret < 0) {
+		dev_err(chip->dev,
+			"Failed to read STATUS registers: %d\n", ret);
+		goto error;
+	}
+
+	/* 0 TEMP_CRIT */
+	if ((chip->persistent[R0] & DA9121_MASK_SYS_EVENT_0_E_TEMP_CRIT) &&
+	    !(status[R0] & DA9121_MASK_SYS_STATUS_0_TEMP_CRIT)) {
+		clear[R0] |= DA9121_MASK_SYS_MASK_0_M_TEMP_CRIT;
+		chip->persistent[R0] &= ~DA9121_MASK_SYS_EVENT_0_E_TEMP_CRIT;
+	}
+	/* 0 TEMP_WARN */
+	if ((chip->persistent[R0] & DA9121_MASK_SYS_EVENT_0_E_TEMP_WARN) &&
+	    !(status[R0] & DA9121_MASK_SYS_STATUS_0_TEMP_WARN)) {
+		clear[R0] |= DA9121_MASK_SYS_MASK_0_M_TEMP_WARN;
+		chip->persistent[R0] &= ~DA9121_MASK_SYS_EVENT_0_E_TEMP_WARN;
+	}
+
+	/* 1 OV1 */
+	if ((chip->persistent[R1] & DA9121_MASK_SYS_EVENT_1_E_OV1) &&
+	    !(status[R1] & DA9121_MASK_SYS_STATUS_1_OV1)) {
+		clear[R1] |= DA9121_MASK_SYS_MASK_1_M_OV1;
+		chip->persistent[R1] &= ~DA9121_MASK_SYS_EVENT_1_E_OV1;
+	}
+	/* 1 UV1 */
+	if ((chip->persistent[R1] & DA9121_MASK_SYS_EVENT_1_E_UV1) &&
+	    !(status[R1] & DA9121_MASK_SYS_STATUS_1_UV1)) {
+		clear[R1] |= DA9121_MASK_SYS_MASK_1_M_UV1;
+		chip->persistent[R1] &= ~DA9121_MASK_SYS_EVENT_1_E_UV1;
+	}
+	/* 1 OC1 */
+	if ((chip->persistent[R1] & DA9121_MASK_SYS_EVENT_1_E_OC1) &&
+	    !(status[R1] & DA9121_MASK_SYS_STATUS_1_OC1)) {
+		clear[R1] |= DA9121_MASK_SYS_MASK_1_M_OC1;
+		chip->persistent[R1] &= ~DA9121_MASK_SYS_EVENT_1_E_OC1;
+	}
+
+	if (variant_parameters[chip->variant_id].num_bucks == 2) {
+		/* 1 OV2 */
+		if ((chip->persistent[R1] & DA9xxx_MASK_SYS_EVENT_1_E_OV2) &&
+		    !(status[R1] & DA9xxx_MASK_SYS_STATUS_1_OV2)) {
+			clear[R1] |= DA9xxx_MASK_SYS_MASK_1_M_OV2;
+			chip->persistent[R1] &= ~DA9xxx_MASK_SYS_EVENT_1_E_OV2;
+		}
+		/* 1 UV2 */
+		if ((chip->persistent[R1] & DA9xxx_MASK_SYS_EVENT_1_E_UV2) &&
+		    !(status[R1] & DA9xxx_MASK_SYS_STATUS_1_UV2)) {
+			clear[R1] |= DA9xxx_MASK_SYS_MASK_1_M_UV2;
+			chip->persistent[R1] &= ~DA9xxx_MASK_SYS_EVENT_1_E_UV2;
+		}
+		/* 1 OC2 */
+		if ((chip->persistent[R1] & DA9xxx_MASK_SYS_EVENT_1_E_OC2) &&
+		    !(status[R1] & DA9xxx_MASK_SYS_STATUS_1_OC2)) {
+			clear[R1] |= DA9xxx_MASK_SYS_MASK_1_M_OC2;
+			chip->persistent[R1] &= ~DA9xxx_MASK_SYS_EVENT_1_E_OC2;
+		}
+	}
+
+	for (i = R0; i < REG_MAX_NUM-1; i++) {
+		if (clear[i]) {
+			unsigned int reg = DA9121_REG_SYS_MASK_0 + i;
+			unsigned int mbit = clear[i];
+
+			ret = regmap_update_bits(chip->regmap, reg, mbit, 0);
+			if (ret < 0) {
+				dev_err(chip->dev,
+					"Failed to unmask 0x%02x %d\n",
+					reg, ret);
+				goto error;
+			}
+		}
+	}
+
+	if (chip->persistent[R0] | chip->persistent[R1]) {
+		delay = msecs_to_jiffies(chip->passive_delay);
+		queue_delayed_work(system_freezable_wq, &chip->work, delay);
+	}
+
+error:
+	return;
+}
+
 static bool da9121_rdev_to_buck_reg_mask(struct regulator_dev *rdev, bool mode,
 					 unsigned int *reg, unsigned int *msk)
 {
@@ -528,6 +633,311 @@ static struct da9121_pdata *da9121_parse_regulators_dt(struct da9121 *chip)
 }
 #endif
 
+static inline int da9121_handle_notifier(
+		struct da9121 *chip, struct regulator_dev *rdev,
+		unsigned int event_bank, unsigned int event, unsigned int ebit)
+{
+	enum { R0 = 0, R1, R2, REG_MAX_NUM };
+	unsigned long notification = 0;
+	int ret = 0;
+
+	if (event & ebit) {
+		switch (event_bank) {
+		case DA9121_REG_SYS_EVENT_0:
+			switch (event & ebit) {
+			case DA9121_MASK_SYS_EVENT_0_E_TEMP_CRIT:
+				chip->persistent[R0] |= DA9121_MASK_SYS_EVENT_0_E_TEMP_CRIT;
+				notification |= REGULATOR_EVENT_OVER_TEMP |
+						REGULATOR_EVENT_DISABLE;
+				break;
+			case DA9121_MASK_SYS_EVENT_0_E_TEMP_WARN:
+				chip->persistent[R0] |= DA9121_MASK_SYS_EVENT_0_E_TEMP_WARN;
+				notification |= REGULATOR_EVENT_OVER_TEMP;
+				break;
+			default:
+				dev_warn(chip->dev,
+					 "Unhandled event in bank0 0x%02x\n",
+					 event & ebit);
+				ret = -EINVAL;
+				break;
+			}
+			break;
+		case DA9121_REG_SYS_EVENT_1:
+			switch (event & ebit) {
+			case DA9121_MASK_SYS_EVENT_1_E_OV1:
+				chip->persistent[R1] |= DA9121_MASK_SYS_EVENT_1_E_OV1;
+				notification |= REGULATOR_EVENT_REGULATION_OUT;
+				break;
+			case DA9121_MASK_SYS_EVENT_1_E_UV1:
+				chip->persistent[R1] |= DA9121_MASK_SYS_EVENT_1_E_UV1;
+				notification |= REGULATOR_EVENT_UNDER_VOLTAGE;
+				break;
+			case DA9121_MASK_SYS_EVENT_1_E_OC1:
+				chip->persistent[R1] |= DA9121_MASK_SYS_EVENT_1_E_OC1;
+				notification |= REGULATOR_EVENT_OVER_CURRENT;
+				break;
+			case DA9xxx_MASK_SYS_EVENT_1_E_OV2:
+				chip->persistent[R1] |= DA9xxx_MASK_SYS_EVENT_1_E_OV2;
+				notification |= REGULATOR_EVENT_REGULATION_OUT;
+				break;
+			case DA9xxx_MASK_SYS_EVENT_1_E_UV2:
+				chip->persistent[R1] |= DA9xxx_MASK_SYS_EVENT_1_E_UV2;
+				notification |= REGULATOR_EVENT_UNDER_VOLTAGE;
+				break;
+			case DA9xxx_MASK_SYS_EVENT_1_E_OC2:
+				chip->persistent[R1] |= DA9xxx_MASK_SYS_EVENT_1_E_OC2;
+				notification |= REGULATOR_EVENT_OVER_CURRENT;
+				break;
+			default:
+				dev_warn(chip->dev,
+					 "Unhandled event in bank1 0x%02x\n",
+					 event & ebit);
+				ret = -EINVAL;
+				break;
+			}
+			break;
+		default:
+			dev_warn(chip->dev,
+				 "Unhandled event bank 0x%02x\n", event_bank);
+			ret = -EINVAL;
+			goto error;
+		}
+
+		regulator_notifier_call_chain(rdev, notification, NULL);
+	}
+
+error:
+	return ret;
+}
+
+static irqreturn_t da9121_irq_handler(int irq, void *data)
+{
+	struct da9121 *chip = data;
+	struct regulator_dev *rdev;
+	enum { R0 = 0, R1, R2, REG_MAX_NUM };
+	int event[REG_MAX_NUM] = {0};
+	int handled[REG_MAX_NUM] = {0};
+	int mask[REG_MAX_NUM] = {0};
+	int ret = IRQ_NONE;
+	int i;
+	int err;
+
+	err = regmap_bulk_read(chip->regmap, DA9121_REG_SYS_EVENT_0,
+			(void *)event, (size_t)REG_MAX_NUM);
+	if (err < 0) {
+		dev_err(chip->dev, "Failed to read EVENT registers %d\n", err);
+		ret = IRQ_NONE;
+		goto error;
+	}
+
+	err = regmap_bulk_read(chip->regmap, DA9121_REG_SYS_MASK_0,
+			(void *)mask, (size_t)REG_MAX_NUM);
+	if (err < 0) {
+		dev_err(chip->dev,
+			"Failed to read MASK registers: %d\n", ret);
+		ret = IRQ_NONE;
+		goto error;
+	}
+
+	rdev = chip->rdev[DA9121_IDX_BUCK1];
+
+	/* 0 SYSTEM_GOOD */
+	if (!(mask[R0] & DA9xxx_MASK_SYS_MASK_0_M_SG) &&
+	    (event[R0] & DA9xxx_MASK_SYS_EVENT_0_E_SG)) {
+		dev_warn(chip->dev, "Handled E_SG\n");
+		handled[R0] |= DA9xxx_MASK_SYS_EVENT_0_E_SG;
+		ret = IRQ_HANDLED;
+	}
+
+	/* 0 TEMP_CRIT */
+	if (!(mask[R0] & DA9121_MASK_SYS_MASK_0_M_TEMP_CRIT) &&
+	    (event[R0] & DA9121_MASK_SYS_EVENT_0_E_TEMP_CRIT)) {
+		err = da9121_handle_notifier(chip, rdev,
+			DA9121_REG_SYS_EVENT_0,	event[R0],
+			DA9121_MASK_SYS_EVENT_0_E_TEMP_CRIT);
+		if (!err) {
+			handled[R0] |= DA9121_MASK_SYS_EVENT_0_E_TEMP_CRIT;
+			ret = IRQ_HANDLED;
+		}
+	}
+
+	/* 0 TEMP_WARN */
+	if (!(mask[R0] & DA9121_MASK_SYS_MASK_0_M_TEMP_WARN) &&
+	    (event[R0] & DA9121_MASK_SYS_EVENT_0_E_TEMP_WARN)) {
+		err = da9121_handle_notifier(chip, rdev,
+			DA9121_REG_SYS_EVENT_0, event[R0],
+			DA9121_MASK_SYS_EVENT_0_E_TEMP_WARN);
+		if (!err) {
+			handled[R0] |= DA9121_MASK_SYS_EVENT_0_E_TEMP_WARN;
+			ret = IRQ_HANDLED;
+		}
+	}
+
+	if (event[R0] != handled[R0]) {
+		dev_warn(chip->dev,
+			 "Unhandled event in bank0 0x%02x\n",
+			 event[R0] ^ handled[R0]);
+	}
+
+	/* 1 PG1 */
+	if (!(mask[R1] & DA9121_MASK_SYS_MASK_1_M_PG1) &&
+	    (event[R1] & DA9121_MASK_SYS_EVENT_1_E_PG1)) {
+		dev_warn(chip->dev, "Handled E_PG1\n");
+		handled[R1] |= DA9121_MASK_SYS_EVENT_1_E_PG1;
+		ret = IRQ_HANDLED;
+	}
+
+	/* 1 OV1 */
+	if (!(mask[R1] & DA9121_MASK_SYS_MASK_1_M_OV1) &&
+	    (event[R1] & DA9121_MASK_SYS_EVENT_1_E_OV1)) {
+		err = da9121_handle_notifier(chip, rdev,
+			DA9121_REG_SYS_EVENT_1,	event[R1],
+			DA9121_MASK_SYS_EVENT_1_E_OV1);
+		if (!err) {
+			handled[R1] |= DA9121_MASK_SYS_EVENT_1_E_OV1;
+			ret = IRQ_HANDLED;
+		}
+	}
+
+	/* 1 UV1 */
+	if (!(mask[R1] & DA9121_MASK_SYS_MASK_1_M_UV1) &&
+	    (event[R1] & DA9121_MASK_SYS_EVENT_1_E_UV1)) {
+		err = da9121_handle_notifier(chip, rdev,
+			DA9121_REG_SYS_EVENT_1,	event[R1],
+			DA9121_MASK_SYS_EVENT_1_E_UV1);
+		if (!err) {
+			handled[R1] |= DA9121_MASK_SYS_EVENT_1_E_UV1;
+			ret = IRQ_HANDLED;
+		}
+	}
+
+	/* 1 OC1 */
+	if (!(mask[R1] & DA9121_MASK_SYS_MASK_1_M_OC1) &&
+	    (event[R1] & DA9121_MASK_SYS_EVENT_1_E_OC1)) {
+		err = da9121_handle_notifier(chip, rdev,
+			DA9121_REG_SYS_EVENT_1,	event[R1],
+			DA9121_MASK_SYS_EVENT_1_E_OC1);
+		if (!err) {
+			handled[R1] |= DA9121_MASK_SYS_EVENT_1_E_OC1;
+			ret = IRQ_HANDLED;
+		}
+	}
+
+	if (variant_parameters[chip->variant_id].num_bucks == 2) {
+		struct regulator_dev *rdev2 = chip->rdev[DA9121_IDX_BUCK2];
+
+		/* 1 PG2 */
+		if (!(mask[R1] & DA9xxx_MASK_SYS_MASK_1_M_PG2) &&
+		    (event[R1] & DA9xxx_MASK_SYS_EVENT_1_E_PG2)) {
+			dev_warn(chip->dev, "Handled E_PG2\n");
+			handled[R1] |= DA9xxx_MASK_SYS_EVENT_1_E_PG2;
+			ret = IRQ_HANDLED;
+		}
+
+		/* 1 OV2 */
+		if (!(mask[R1] & DA9xxx_MASK_SYS_MASK_1_M_OV2) &&
+		    (event[R1] & DA9xxx_MASK_SYS_EVENT_1_E_OV2)) {
+			err = da9121_handle_notifier(chip, rdev2,
+				DA9121_REG_SYS_EVENT_1,	event[R1],
+				DA9xxx_MASK_SYS_EVENT_1_E_OV2);
+			if (!err) {
+				handled[R1] |= DA9xxx_MASK_SYS_EVENT_1_E_OV2;
+				ret = IRQ_HANDLED;
+			}
+		}
+
+		/* 1 UV2 */
+		if (!(mask[R1] & DA9xxx_MASK_SYS_MASK_1_M_UV2) &&
+		    (event[R1] & DA9xxx_MASK_SYS_EVENT_1_E_UV2)) {
+			err = da9121_handle_notifier(chip, rdev2,
+				DA9121_REG_SYS_EVENT_1,	event[R1],
+				DA9xxx_MASK_SYS_EVENT_1_E_UV2);
+			if (!err) {
+				handled[R1] |= DA9xxx_MASK_SYS_EVENT_1_E_UV2;
+				ret = IRQ_HANDLED;
+			}
+		}
+
+		/* 1 OC2 */
+		if (!(mask[R1] & DA9xxx_MASK_SYS_MASK_1_M_OC2) &&
+		    (event[R1] & DA9xxx_MASK_SYS_EVENT_1_E_OC2)) {
+			err = da9121_handle_notifier(chip, rdev2,
+				DA9121_REG_SYS_EVENT_1,	event[R1],
+				DA9xxx_MASK_SYS_EVENT_1_E_OC2);
+			if (!err) {
+				handled[R1] |= DA9xxx_MASK_SYS_EVENT_1_E_OC2;
+				ret = IRQ_HANDLED;
+			}
+		}
+	}
+
+	if (event[R1] != handled[R1]) {
+		dev_warn(chip->dev,
+			 "Unhandled event in bank1 0x%02x\n",
+			 event[R1] ^ handled[R1]);
+	}
+
+	/* DA9121_REG_SYS_EVENT_2 */
+	if (!(mask[R2] & DA9121_MASK_SYS_MASK_2_M_GPIO2) &&
+	    (event[R2] & DA9121_MASK_SYS_EVENT_2_E_GPIO2)) {
+		dev_warn(chip->dev, "Handled E_GPIO2\n");
+		handled[R2] |= DA9121_MASK_SYS_EVENT_2_E_GPIO2;
+		ret = IRQ_HANDLED;
+	}
+
+	if (!(mask[R2] & DA9121_MASK_SYS_MASK_2_M_GPIO1) &&
+	    (event[R2] & DA9121_MASK_SYS_EVENT_2_E_GPIO1)) {
+		dev_warn(chip->dev, "Handled E_GPIO1\n");
+		handled[R2] |= DA9121_MASK_SYS_EVENT_2_E_GPIO1;
+		ret = IRQ_HANDLED;
+	}
+
+	if (!(mask[R2] & DA9121_MASK_SYS_MASK_2_M_GPIO0) &&
+	    (event[R2] & DA9121_MASK_SYS_EVENT_2_E_GPIO0)) {
+		dev_warn(chip->dev, "Handled E_GPIO0\n");
+		handled[R2] |= DA9121_MASK_SYS_EVENT_2_E_GPIO0;
+		ret = IRQ_HANDLED;
+	}
+
+	if (event[R2] != handled[R2]) {
+		dev_warn(chip->dev,
+			 "Unhandled event in bank2 0x%02x\n",
+			 event[R2] ^ handled[R2]);
+	}
+
+	/* Mask the interrupts for persistent events OV, OC, UV, WARN, CRIT */
+	for (i = R0; i < REG_MAX_NUM-1; i++) {
+		if (handled[i]) {
+			unsigned int reg = DA9121_REG_SYS_MASK_0 + i;
+			unsigned int mbit = handled[i];
+
+			err = regmap_update_bits(chip->regmap, reg, mbit, mbit);
+			if (err < 0) {
+				dev_err(chip->dev,
+					"Failed to mask 0x%02x interrupt %d\n",
+					reg, err);
+				ret = IRQ_NONE;
+				goto error;
+			}
+		}
+	}
+
+	/* clear the events */
+	if (handled[R0] | handled[R1] | handled[R2]) {
+		err = regmap_bulk_write(chip->regmap, DA9121_REG_SYS_EVENT_0,
+				(const void *)handled, (size_t)REG_MAX_NUM);
+		if (err < 0) {
+			dev_err(chip->dev, "Fail to write EVENTs %d\n", err);
+			ret = IRQ_NONE;
+			goto error;
+		}
+	}
+
+	queue_delayed_work(system_freezable_wq, &chip->work, 0);
+error:
+	return ret;
+}
+
 static int da9121_set_regulator_config(struct da9121 *chip)
 {
 	struct regulator_config config = { };
@@ -835,6 +1245,117 @@ static int da9121_assign_chip_model(struct i2c_client *i2c,
 	return ret;
 }
 
+static int da9121_set_irq_masks(struct da9121 *chip, bool mask_irqs)
+{
+	unsigned int mask0, update0;
+	unsigned int mask1, update1;
+	unsigned int mask3;
+	int ret = 0;
+
+	if (chip->chip_irq != 0) {
+		mask0 = DA9121_MASK_SYS_MASK_0_M_TEMP_CRIT |
+			DA9121_MASK_SYS_MASK_0_M_TEMP_WARN;
+
+		mask1 = DA9121_MASK_SYS_MASK_1_M_OV1 |
+			DA9121_MASK_SYS_MASK_1_M_UV1 |
+			DA9121_MASK_SYS_MASK_1_M_OC1;
+
+		if (mask_irqs) {
+			update0 = mask0;
+			update1 = mask1;
+		} else {
+			update0 = 0;
+			update1 = 0;
+		}
+
+		ret = regmap_update_bits(chip->regmap,
+				DA9121_REG_SYS_MASK_0,
+				mask0,
+				update0);
+		if (ret < 0) {
+			dev_err(chip->dev, "Failed to write MASK 0 reg %d\n",
+				ret);
+			goto error;
+		}
+
+		ret = regmap_update_bits(chip->regmap,
+				DA9121_REG_SYS_MASK_1,
+				mask1,
+				update1);
+		if (ret < 0) {
+			dev_err(chip->dev, "Failed to write MASK 1 reg %d\n",
+				ret);
+			goto error;
+		}
+
+		/* permanently disable IRQs for VR_HOT and PG1_STAT */
+		mask3 = DA9121_MASK_SYS_MASK_3_M_VR_HOT |
+			DA9121_MASK_SYS_MASK_3_M_PG1_STAT;
+
+		ret = regmap_update_bits(chip->regmap,
+				DA9121_REG_SYS_MASK_3,
+				mask3,
+				mask3);
+		if (ret < 0) {
+			dev_err(chip->dev, "Failed to write MASK 3 reg %d\n",
+			ret);
+			goto error;
+		}
+	}
+
+error:
+	return ret;
+}
+
+static int da9121_config_irq(struct i2c_client *i2c,
+			struct da9121 *chip)
+{
+	unsigned int p_delay = DA9121_DEFAULT_POLLING_PERIOD_MS;
+	int ret = 0;
+
+	chip->chip_irq = i2c->irq;
+
+	if (chip->chip_irq != 0) {
+		if (!of_property_read_u32(chip->dev->of_node,
+					  "dlg,irq-polling-delay-passive",
+					  &p_delay)) {
+			if (p_delay < DA9121_MIN_POLLING_PERIOD_MS ||
+			    p_delay > DA9121_MAX_POLLING_PERIOD_MS) {
+				dev_warn(chip->dev,
+					 "Out-of-range polling period %d ms\n",
+					 p_delay);
+				p_delay = DA9121_DEFAULT_POLLING_PERIOD_MS;
+			}
+		}
+
+		chip->passive_delay = p_delay;
+
+		ret = devm_request_threaded_irq(chip->dev,
+					chip->chip_irq, NULL,
+					da9121_irq_handler,
+					IRQF_TRIGGER_LOW|IRQF_ONESHOT,
+					"da9121", chip);
+		if (ret != 0) {
+			dev_err(chip->dev, "Failed IRQ request: %d\n",
+				chip->chip_irq);
+			goto error;
+		}
+
+		ret = da9121_set_irq_masks(chip, false);
+		if (ret != 0) {
+			dev_err(chip->dev, "Failed to set IRQ masks: %d\n",
+				ret);
+			goto error;
+		}
+
+		INIT_DELAYED_WORK(&chip->work, da9121_status_poll_on);
+		dev_info(chip->dev, "Interrupt polling period set at %d ms\n",
+			 chip->passive_delay);
+	}
+error:
+	return ret;
+}
+
 static const struct of_device_id da9121_dt_ids[] = {
 	{ .compatible = "dlg,da9121", .data = (void *) DA9121_TYPE_DA9121_DA9130 },
 	{ .compatible = "dlg,da9130", .data = (void *) DA9121_TYPE_DA9121_DA9130 },
@@ -877,6 +1398,12 @@ static int da9121_i2c_probe(struct i2c_client *i2c,
 	if (ret < 0)
 		goto error;
 
+	ret = da9121_set_irq_masks(chip, true);
+	if (ret != 0) {
+		dev_err(chip->dev, "Failed to set IRQ masks: %d\n", ret);
+		goto error;
+	}
+
 	if (!chip->pdata)
 		chip->pdata = da9121_parse_regulators_dt(chip);
 
@@ -889,6 +1416,26 @@ static int da9121_i2c_probe(struct i2c_client *i2c,
 	if (ret < 0)
 		goto error;
 
+	ret = da9121_config_irq(i2c, chip);
+	if (ret < 0)
+		goto error;
+
+error:
+	return ret;
+}
+
+static int da9121_i2c_remove(struct i2c_client *i2c)
+{
+	struct da9121 *chip = i2c_get_clientdata(i2c);
+	int ret = 0;
+
+	ret = da9121_set_irq_masks(chip, true);
+	if (ret != 0) {
+		dev_err(chip->dev, "Failed to set IRQ masks: %d\n", ret);
+		goto error;
+	}
+
+	cancel_delayed_work(&chip->work);
 error:
 	return ret;
 }
@@ -911,6 +1458,7 @@ static int da9121_i2c_probe(struct i2c_client *i2c,
 		.of_match_table = of_match_ptr(da9121_dt_ids),
 	},
 	.probe = da9121_i2c_probe,
+	.remove = da9121_i2c_remove,
 	.id_table = da9121_i2c_id,
 };
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/9] regulator: da9121: Add device variant details and respective regmaps
  2020-11-20 12:14 ` [PATCH 4/9] regulator: da9121: Add device variant details and respective regmaps Adam Ward
@ 2020-11-20 12:45   ` Mark Brown
  0 siblings, 0 replies; 19+ messages in thread
From: Mark Brown @ 2020-11-20 12:45 UTC (permalink / raw)
  To: Adam Ward
  Cc: Rob Herring, Liam Girdwood, Vincent Whitchurch, linux-kernel, devicetree

[-- Attachment #1: Type: text/plain, Size: 907 bytes --]

On Fri, Nov 20, 2020 at 12:14:54PM +0000, Adam Ward wrote:

> Add ability to probe device and validate configuration, then apply a regmap
> configuration for a single or dual buck device accordingly.

This looks like it might benefit from being multiple commits - "X then
Y" type commit logs are often a warning sign of this, it's quite
difficult to review as it's doing several different things.

> +static int da9121_i2c_reg_read(struct i2c_client *client, u8 addr,
> +				    u8 *buf, int count)
> +{
> +	struct i2c_msg xfer[2];
> +	int ret;

Why is this open coding register I/O?

> +	name = of_get_property(chip->dev->of_node, "compatible", NULL);
> +	if (!name) {
> +		dev_err(chip->dev, "Cannot get device not compatible string.\n");
> +		goto error;
> +	}

You shouldn't need to query the compatible string as a property, why is
the code doing this?  You know what compatible was used from probe().

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/9] regulator: da9121: Add support for device variants via devicetree
  2020-11-20 12:14 ` [PATCH 5/9] regulator: da9121: Add support for device variants via devicetree Adam Ward
@ 2020-11-20 12:51   ` Mark Brown
  0 siblings, 0 replies; 19+ messages in thread
From: Mark Brown @ 2020-11-20 12:51 UTC (permalink / raw)
  To: Adam Ward
  Cc: Rob Herring, Liam Girdwood, Vincent Whitchurch, linux-kernel, devicetree

[-- Attachment #1: Type: text/plain, Size: 1192 bytes --]

On Fri, Nov 20, 2020 at 12:14:55PM +0000, Adam Ward wrote:

> @@ -1,7 +1,21 @@
>  // SPDX-License-Identifier: GPL-2.0-only
> -/* Copyright (C) 2020 Axis Communications AB */
> +/* Copyright (C) 2020 Axis Communications AB
> + *
> + * DA9121 Single-channel dual-phase 10A buck converter

Please make the entire comment a C++ one so things look more
intentional.

> +	node = of_get_child_by_name(chip->dev->of_node, "regulators");
> +	if (!node) {
> +		dev_err(chip->dev, "Regulators node not found\n");
> +		return ERR_PTR(-ENODEV);
> +	}
> +
> +	num = of_regulator_match(chip->dev, node, da9121_matches,
> +				 ARRAY_SIZE(da9121_matches));

Use of_parse_cb().

> +	/* interrupt assumptions require at least one buck to be configured */
> +	if (num == 0) {
> +		dev_err(chip->dev, "Did not match any regulators in the DT\n");
> +		return ERR_PTR(-EINVAL);
> +	}

The physical presence of the regulator is not going to change based on
the DT.

> +		if (variant_parameters[chip->variant_id].num_bucks == 2) {
> +			uint32_t ripple_cancel;
> +			uint32_t reg = (i ? DA9xxx_REG_BUCK_BUCK2_7
> +					  : DA9121_REG_BUCK_BUCK1_7);

Please write normal conditional statements to improve legibility.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/9] regulator: da9121: Update registration to support multiple buck variants
  2020-11-20 12:14 ` [PATCH 6/9] regulator: da9121: Update registration to support multiple buck variants Adam Ward
@ 2020-11-20 13:06   ` Mark Brown
  0 siblings, 0 replies; 19+ messages in thread
From: Mark Brown @ 2020-11-20 13:06 UTC (permalink / raw)
  To: Adam Ward
  Cc: Rob Herring, Liam Girdwood, Vincent Whitchurch, linux-kernel, devicetree

[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]

On Fri, Nov 20, 2020 at 12:14:56PM +0000, Adam Ward wrote:

> Checks DT matches tally with variant maximum and register accordingly.

This changelog doesn't really explain what the change is supposed to
be doing or why which makes it very hard to review, and the code is far
from obvious.  I can't really tie much of the change to the words here.

> +	if (max_matches > variant_parameters[chip->variant_id].num_bucks) {
> +		dev_err(chip->dev, "Too many regulators in the DT\n");
> +		ret = -EINVAL;
> +		goto error;
> +	}

If this validation is needed it should be in the regulator core.

> +	for (i = 0; i < max_matches; i++) {
> +		const struct regulator_desc *regl_desc =
> +			local_da9121_regulators[chip->variant_id][i];
> +		int id = regl_desc->id;
> +		struct gpio_desc *gpio_ren;
> +
> +		if (chip->pdata->gpiod_ren[i])
> +			gpio_ren = chip->pdata->gpiod_ren[i];
> +		else
> +			gpio_ren = NULL;
> +
> +		config.init_data = chip->pdata->init_data[i];
> +		config.dev = chip->dev;
> +		config.driver_data = chip;
> +		config.regmap = chip->regmap;
> +		config.of_node = chip->pdata->reg_node[i];

I *think* this is all open coding the core's DT parsing support.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 7/9] regulator: da9121: add current support
  2020-11-20 12:14 ` [PATCH 7/9] regulator: da9121: add current support Adam Ward
@ 2020-11-20 13:17   ` Mark Brown
  0 siblings, 0 replies; 19+ messages in thread
From: Mark Brown @ 2020-11-20 13:17 UTC (permalink / raw)
  To: Adam Ward
  Cc: Rob Herring, Liam Girdwood, Vincent Whitchurch, linux-kernel, devicetree

[-- Attachment #1: Type: text/plain, Size: 802 bytes --]

On Fri, Nov 20, 2020 at 12:14:57PM +0000, Adam Ward wrote:

> +static bool da9121_rdev_to_buck_reg_mask(struct regulator_dev *rdev, bool mode,
> +					 unsigned int *reg, unsigned int *msk)
> +{
> +	struct da9121 *chip = rdev_get_drvdata(rdev);
> +	int id = rdev_get_id(rdev);
> +
> +	switch (id) {
> +	case DA9121_IDX_BUCK1:
> +		if (mode) {
> +			*reg = DA9121_REG_BUCK_BUCK1_4;
> +			*msk = DA9121_MASK_BUCK_BUCKx_4_CHx_A_MODE;
> +		} else {
> +			*reg = DA9121_REG_BUCK_BUCK1_2;
> +			*msk = DA9121_MASK_BUCK_BUCKx_2_CHx_ILIM;
> +		}
> +		break;

This is really not an easy interface to understand, the boolean flag
saying which register set to pick is not particularly obvious to read
and won't scale.  The usual pattern with things like this is to store
information in driver data at probe time.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 9/9] regulator: da9121: add interrupt support
  2020-11-20 12:14 ` [PATCH 9/9] regulator: da9121: add interrupt support Adam Ward
@ 2020-11-20 13:45   ` Mark Brown
  0 siblings, 0 replies; 19+ messages in thread
From: Mark Brown @ 2020-11-20 13:45 UTC (permalink / raw)
  To: Adam Ward
  Cc: Rob Herring, Liam Girdwood, Vincent Whitchurch, linux-kernel, devicetree

[-- Attachment #1: Type: text/plain, Size: 3084 bytes --]

On Fri, Nov 20, 2020 at 12:14:59PM +0000, Adam Ward wrote:
> Adds interrupt handler for variants, and notifications for events; over
> temperature/voltage/current.
> Also handling of persistent events and respective timing configuration.

Again the "also" suggests that this should be multiple changes.

> +	struct da9121 *chip = container_of(work, struct da9121, work.work);
> +	enum { R0 = 0, R1, R2, REG_MAX_NUM };

This enum appears to map the numbers 0, 1 and 2 onto the constants 0, 1
and 2?  It also seems to be repeated in several functions?

> +	int status[REG_MAX_NUM] = {0};
> +	int clear[REG_MAX_NUM] = {0};
> +	unsigned long delay;
> +	int i;
> +	int ret;

> +	/* If persistent-notification, status will be true
> +	 * If not persistent-notification any longer, status will be false
> +	 */
> +	ret = regmap_bulk_read(chip->regmap, DA9121_REG_SYS_STATUS_0,
> +			(void *)status, (size_t)REG_MAX_NUM);

If these casts are needed something is very wrong with what you're
doing.

> +static inline int da9121_handle_notifier(
> +		struct da9121 *chip, struct regulator_dev *rdev,
> +		unsigned int event_bank, unsigned int event, unsigned int ebit)

Why is this flagged as inline?

> +	if (event & ebit) {
> +		switch (event_bank) {
> +		case DA9121_REG_SYS_EVENT_0:
> +			switch (event & ebit) {
> +			case DA9121_MASK_SYS_EVENT_0_E_TEMP_CRIT:

The structure of this code seems extremely confusing and hard to follow.
I really don't understand what purpose this function serves at all, it
appears to be factoring out the check to see if the bit is set and then
wrapping that in three layers of unpacking to work out setting the bit
in persistent and which notification to flag.  I don't understand why
the interrupt handler doesn't just directly do these things, this just
seems to be adding a lot of redundant code.

> +			case DA9xxx_MASK_SYS_EVENT_1_E_OC2:
> +				chip->persistent[R1] |= DA9xxx_MASK_SYS_EVENT_1_E_OC2;
> +				notification |= REGULATOR_EVENT_OVER_CURRENT;
> +				break;

> +		regulator_notifier_call_chain(rdev, notification, NULL);

The expectation is that one notification will be delivered per event,
which fortunately as far as I can see is pretty much what happens.

> +	/* 0 SYSTEM_GOOD */
> +	if (!(mask[R0] & DA9xxx_MASK_SYS_MASK_0_M_SG) &&
> +	    (event[R0] & DA9xxx_MASK_SYS_EVENT_0_E_SG)) {
> +		dev_warn(chip->dev, "Handled E_SG\n");
> +		handled[R0] |= DA9xxx_MASK_SYS_EVENT_0_E_SG;
> +		ret = IRQ_HANDLED;
> +	}

If the interrupt is saying that the system is good why are we logging
that as a warning?

> +static int da9121_i2c_remove(struct i2c_client *i2c)
> +{
> +	struct da9121 *chip = i2c_get_clientdata(i2c);
> +	int ret = 0;
> +
> +	ret = da9121_set_irq_masks(chip, true);
> +	if (ret != 0) {
> +		dev_err(chip->dev, "Failed to set IRQ masks: %d\n", ret);
> +		goto error;
> +	}

It would simplify the rest of the code to just unconditionally mask all
interrupts here.

> +
> +	cancel_delayed_work(&chip->work);

This doesn't synchronize with the work being cancelled, the driver may
be unregistered while the work is still running.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/9] regulator: Update DA9121 dt-bindings
  2020-11-20 12:14 ` [PATCH 1/9] regulator: Update DA9121 dt-bindings Adam Ward
@ 2020-11-20 13:47   ` Vincent Whitchurch
  2020-11-25  9:21     ` Vincent Whitchurch
  0 siblings, 1 reply; 19+ messages in thread
From: Vincent Whitchurch @ 2020-11-20 13:47 UTC (permalink / raw)
  To: Adam Ward
  Cc: Mark Brown, Rob Herring, Liam Girdwood, linux-kernel, devicetree

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/9] regulator: Update DA9121 dt-bindings
  2020-11-20 13:47   ` Vincent Whitchurch
@ 2020-11-25  9:21     ` Vincent Whitchurch
  2020-11-27 13:01       ` Adam Ward
  0 siblings, 1 reply; 19+ messages in thread
From: Vincent Whitchurch @ 2020-11-25  9:21 UTC (permalink / raw)
  To: Adam Ward
  Cc: Mark Brown, Rob Herring, Liam Girdwood, linux-kernel, devicetree

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?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH 1/9] regulator: Update DA9121 dt-bindings
  2020-11-25  9:21     ` Vincent Whitchurch
@ 2020-11-27 13:01       ` Adam Ward
  2020-11-27 14:59         ` Mark Brown
  0 siblings, 1 reply; 19+ messages in thread
From: Adam Ward @ 2020-11-27 13:01 UTC (permalink / raw)
  To: Vincent Whitchurch, Mark Brown
  Cc: Rob Herring, Liam Girdwood, linux-kernel, devicetree

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/9] regulator: Update DA9121 dt-bindings
  2020-11-27 13:01       ` Adam Ward
@ 2020-11-27 14:59         ` Mark Brown
  0 siblings, 0 replies; 19+ messages in thread
From: Mark Brown @ 2020-11-27 14:59 UTC (permalink / raw)
  To: Adam Ward
  Cc: Vincent Whitchurch, Rob Herring, Liam Girdwood, linux-kernel, devicetree

[-- Attachment #1: Type: text/plain, Size: 302 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2020-11-27 15:00 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20 12:14 [PATCH 0/9] regulator: da9121: extend support to variants, add features Adam Ward
2020-11-20 12:14 ` [PATCH 1/9] regulator: Update DA9121 dt-bindings Adam Ward
2020-11-20 13:47   ` Vincent Whitchurch
2020-11-25  9:21     ` Vincent Whitchurch
2020-11-27 13:01       ` Adam Ward
2020-11-27 14:59         ` Mark Brown
2020-11-20 12:14 ` [PATCH 2/9] regulator: da9121: Add header file Adam Ward
2020-11-20 12:14 ` [PATCH 3/9] regulator: da9121: Add device variants Adam Ward
2020-11-20 12:14 ` [PATCH 4/9] regulator: da9121: Add device variant details and respective regmaps Adam Ward
2020-11-20 12:45   ` Mark Brown
2020-11-20 12:14 ` [PATCH 5/9] regulator: da9121: Add support for device variants via devicetree Adam Ward
2020-11-20 12:51   ` Mark Brown
2020-11-20 12:14 ` [PATCH 6/9] regulator: da9121: Update registration to support multiple buck variants Adam Ward
2020-11-20 13:06   ` Mark Brown
2020-11-20 12:14 ` [PATCH 7/9] regulator: da9121: add current support Adam Ward
2020-11-20 13:17   ` Mark Brown
2020-11-20 12:14 ` [PATCH 8/9] regulator: da9121: add mode support Adam Ward
2020-11-20 12:14 ` [PATCH 9/9] regulator: da9121: add interrupt support Adam Ward
2020-11-20 13:45   ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.