[PATCH 1/2] dt-bindings: regulator: Add pf8x00 regulator

[PATCH 1/2] dt-bindings: regulator: Add pf8x00 regulator

[Adrien Grassein]

Add dt-bindings for the pf8x00 driver.

Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com>
---
 .../regulator/nxp,pf8x00-regulator.yaml       | 223 ++++++++++++++++++
 MAINTAINERS                                   |   6 +
 2 files changed, 229 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml

diff --git a/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
new file mode 100644
index 000000000000..f4e4f545c5a3
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
@@ -0,0 +1,223 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/nxp,pf8x00-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP PF8x00 Power Management Integrated Circuit regulators
+
+maintainers:
+  - Adrien Grassein <adrien.grassein@gmail.com>
+
+description: |
+  pf8x00 are a 12-chanel regulator PMIC family. Regulators nodes should
+  be named to ldo<>, sw<> and vnss. The definition for each of these nodes
+  is defined using the standard binding for regulators at
+  Documentation/devicetree/bindings/regulator/regulator.txt.
+  Datasheet is available at https://www.nxp.com/docs/en/data-sheet/PF8100_PF8200.pdf
+
+properties:
+  compatible:
+    enum:
+      - nxp,pf8x00
+
+  reg:
+    maxItems: 1
+
+  regulators:
+    type: object
+    description: |
+      list of regulators provided by this controller
+
+    patternProperties:
+      "^ldo[1-4]$":
+        type: object
+        $ref: regulator.yaml#
+        description: |
+          Properties for single LDO regulator.
+
+        properties:
+          regulator-name:
+            pattern: "^ldo[1-4]$"
+            description: |
+              should be ldo1", ..., "ldo4"
+
+          nxp,hw-en:
+            $ref: /schemas/types.yaml#definitions/flag
+            description: |
+              Only available for ldo2. Used to enable or disable ld02.
+
+          nxp,vselect-en:
+            $ref: /schemas/types.yaml#definitions/flag
+            description: |
+              Only available for ldo2. When specified, use the VSELECT pin
+              of the chip to control the output voltage of the ldo02 regulator.
+
+        unevaluatedProperties: false
+
+      "^sw[1-7]$":
+        type: object
+        $ref: regulator.yaml#
+        description: |
+          Properties for single SW regulator.
+
+        properties:
+          regulator-name:
+            pattern: "^sw[1-7]$"
+            description: |
+              should be sw1", ..., "sw7"
+
+          nxp,ilim-ma:
+            $ref: /schemas/types.yaml#definitions/uint32
+            minimum: 2100
+            maximum: 4500
+            default: 2100
+            enum: [ 2100, 2600, 3000, 4500 ]
+            description: |
+              Defines the maximum current delivered by the regulator (in mA).
+
+          nxp,phase:
+            $ref: /schemas/types.yaml#definitions/uint32
+            minimum: 0
+            maximum: 315
+            default: 0
+            enum: [ 0, 45, 90, 135, 180, 225, 270, 315 ]
+            description: |
+               This controls the phase shift of the switching frequency.
+
+          nxp,fsl,fast-slew:
+            $ref: /schemas/types.yaml#definitions/flag
+            description: |
+              Controls the DVS ramp of the regulator.
+
+          nxp,quad-phase:
+            $ref: /schemas/types.yaml#definitions/flag
+            description: |
+              This allow regulators  sw1 and sw2, or sw3 and sw4 or sw4 and sw5
+              to work together to deliver a maximum 10A current.
+
+          nxp,dual-phase:
+            $ref: /schemas/types.yaml#definitions/flag
+            description: |
+              This allow regulators  sw1, sw2, sw3 and sw4 to work together
+              to deliver a maximum 5A current.
+
+        unevaluatedProperties: false
+
+      "^vsnvs$":
+        type: object
+        $ref: regulator.yaml#
+        description: |
+          Properties for vsnvs regulator.
+
+        unevaluatedProperties: false
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - regulators
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      pmic: pf8100@8 {
+        compatible = "nxp,pf8x00";
+        reg = <0x08>;
+
+        regulators {
+            reg_ldo1: ldo1 {
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <5000000>;
+              regulator-min-microvolt = <1500000>;
+            };
+
+            reg_ldo2: ldo2 {
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <5000000>;
+              regulator-min-microvolt = <1500000>;
+            };
+
+            reg_ldo3: ldo3 {
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <5000000>;
+              regulator-min-microvolt = <1500000>;
+            };
+
+            reg_ldo4: ldo4 {
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <5000000>;
+              regulator-min-microvolt = <1500000>;
+            };
+
+            reg_sw1: sw1 {
+              nxp,phase = <0>;
+              nxp,ilim-ma = <4500>;
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <1800000>;
+              regulator-min-microvolt =  <400000>;
+            };
+
+            reg_sw2: sw2 {
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <1800000>;
+              regulator-min-microvolt =  <400000>;
+            };
+
+            reg_sw3: sw3 {
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <1800000>;
+              regulator-min-microvolt =  <400000>;
+              nxp,fast-slew = <1>;
+            };
+
+            reg_sw4: sw4 {
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <1800000>;
+              regulator-min-microvolt =  <400000>;
+              nxp,fast-slew = <1>;
+            };
+
+            reg_sw5: sw5 {
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <1800000>;
+              regulator-min-microvolt =  <400000>;
+            };
+
+           reg_sw6: sw6 {
+             regulator-always-on;
+             regulator-boot-on;
+             regulator-max-microvolt = <1800000>;
+             regulator-min-microvolt =  <400000>;
+           };
+
+           reg_sw7: sw7 {
+             regulator-always-on;
+             regulator-boot-on;
+             regulator-max-microvolt = <4100000>;
+             regulator-min-microvolt = <1000000>;
+           };
+
+          reg_vsnvs: vsnvs {
+            regulator-always-on;
+            regulator-boot-on;
+            regulator-max-microvolt = <3300000>;
+            regulator-min-microvolt = <1800000>;
+          };
+        };
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index ebe4829cdd4d..4e266f1e9a2f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13730,6 +13730,12 @@ S:	Maintained
 F:	include/linux/personality.h
 F:	include/uapi/linux/personality.h
 
+PF8x00 PMIC REGULATORS DRIVERS
+M:	Adrien Grassein <adrien.grassein@gmail.com>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
+
 PHOENIX RC FLIGHT CONTROLLER ADAPTER
 M:	Marcus Folkesson <marcus.folkesson@gmail.com>
 L:	linux-input@vger.kernel.org
-- 
2.20.1

[PATCH 2/2] regulator: pf8x00: add support of nxp pf8x00 regulator

[Adrien Grassein]

NXP pf8100 and pf8200 are 12-channel
PMIC for high performance applications.

This driver introduces the support of the 12
regulators available on the PMIC.

Imported from Boundary Devices kernel
with some modifications.

This driver was teste using a Boundary Nitrogen 8M Mini
board that features this PMIC.

Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com>
---
 MAINTAINERS                          |   1 +
 drivers/regulator/Kconfig            |  12 +
 drivers/regulator/Makefile           |   1 +
 drivers/regulator/pf8x00-regulator.c | 870 +++++++++++++++++++++++++++
 4 files changed, 884 insertions(+)
 create mode 100644 drivers/regulator/pf8x00-regulator.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 4e266f1e9a2f..72522a7fd329 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13735,6 +13735,7 @@ M:	Adrien Grassein <adrien.grassein@gmail.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
+F:	drivers/regulator/pf8x00-regulator.c
 
 PHOENIX RC FLIGHT CONTROLLER ADAPTER
 M:	Marcus Folkesson <marcus.folkesson@gmail.com>
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 020a00d6696b..d2aba4b487f4 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -791,6 +791,18 @@ config REGULATOR_PCF50633
 	 Say Y here to support the voltage regulators and converters
 	 on PCF50633
 
+config REGULATOR_PF8X00
+	tristate "NXP PF8x00 regulator driver"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	 Say y here to support the regulators found on the NXP
+	 PF8100/PF8200 PMIC
+
+	 Say M here if you want to include support for tte regulators found
+	 on the NXP PF8100/PF8200 PMIC. The module will be named
+	 "pf8x00-regulator".
+
 config REGULATOR_PFUZE100
 	tristate "Freescale PFUZE100/200/3000/3001 regulator driver"
 	depends on I2C
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 6ebae516258e..a9fb449a0dc8 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -100,6 +100,7 @@ obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_USB_VBUS) += qcom_usb_vbus-regulator.o
 obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o
 obj-$(CONFIG_REGULATOR_PCA9450) += pca9450-regulator.o
+obj-$(CONFIG_REGULATOR_PF8X00) += pf8x00-regulator.o
 obj-$(CONFIG_REGULATOR_PFUZE100) += pfuze100-regulator.o
 obj-$(CONFIG_REGULATOR_PV88060) += pv88060-regulator.o
 obj-$(CONFIG_REGULATOR_PV88080) += pv88080-regulator.o
diff --git a/drivers/regulator/pf8x00-regulator.c b/drivers/regulator/pf8x00-regulator.c
new file mode 100644
index 000000000000..84a1f2ddbf74
--- /dev/null
+++ b/drivers/regulator/pf8x00-regulator.c
@@ -0,0 +1,870 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
+// Copyright 2017 NXP
+// Copyright 2019 Boundary Devices
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/regmap.h>
+
+#define PF8X00_DEVICEID		0x00
+#define PF8X00_REVID		0x01
+#define PF8X00_EMREV		0x02
+#define PF8X00_PROGID		0x03
+
+#define PF8X00_IMS_INT		0x04
+#define PF8X00_IMS_THERM	0x07
+#define PF8X00_SW_MODE_INT	0x0a
+#define PF8X00_SW_MODE_MASK	0x0b
+
+#define IMS_INT			0
+#define IMS_STATUS		0
+#define IMS_MASK		1
+#define IMS_SENSE		2
+
+#define PF8X00_IMS_SW_ILIM	0x12
+#define PF8X00_IMS_LDO_ILIM	0x15
+#define PF8X00_IMS_SW_UV	0x18
+#define PF8X00_IMS_SW_OV	0x1b
+#define PF8X00_IMS_LDO_UV	0x1e
+#define PF8X00_IMS_LDO_OV	0x21
+#define PF8X00_IMS_PWRON	0x24
+#define PF8X00_SYS_INT		0x27
+
+#define PF8X00_HARD_FAULT	0x29
+#define PF8X00_FSOB_FLAGS	0x2a
+#define PF8X00_FSOB_SELECT	0x2b
+#define PF8X00_ABIST_OV1	0x2c
+#define PF8X00_ABIST_OV2	0x2d
+#define PF8X00_ABIST_UV1	0x2e
+#define PF8X00_ABIST_UV2	0x2f
+#define PF8X00_TEST_FLAGS	0x30
+#define PF8X00_ABIST_RUN	0x31
+
+#define PF8X00_RANDOM_GEN	0x33
+#define PF8X00_RANDOM_CHK	0x34
+#define PF8X00_VMONEN1		0x35
+#define PF8X00_VMONEN2		0x36
+#define PF8X00_CTRL1		0x37
+#define PF8X00_CTRL2		0x38
+#define PF8X00_CTRL3		0x39
+#define PF8X00_PWRUP_CTRL	0x3a
+
+#define PF8X00_RESETBMCU	0x3c
+#define PF8X00_PGOOD		0x3d
+#define PF8X00_PWRDN_DLY1	0x3e
+#define PF8X00_PWRDN_DLY2	0x3f
+#define PF8X00_FREQ_CTRL	0x40
+#define PF8X00_COINCELL_CTRL	0x41
+#define PF8X00_PWRON		0x42
+#define PF8X00_WD_CONFIG	0x43
+#define PF8X00_WD_CLEAR		0x44
+#define PF8X00_WD_EXPIRE	0x45
+#define PF8X00_WD_COUNTER	0x46
+#define PF8X00_FAULT_COUNTER	0x47
+#define PF8X00_FSAFE_COUNTER	0x48
+#define PF8X00_FAULT_TIMER	0x49
+#define PF8X00_AMUX		0x4a
+
+#define SW_CONFIG1	0
+#define SW_CONFIG2	1
+#define SW_PWRUP	2
+#define SW_MODE1	3
+#define SW_RUN_VOLT	4
+#define SW_STBY_VOLT	5
+
+/* i is in REG_SW1..REG_SW7 */
+#define PF8X00_SW(i)		(8 * (i - REG_SW1) + 0x4d)
+
+#define LDO_CONFIG1	0
+#define LDO_CONFIG2	1
+#define LDO_PWRUP	2
+#define LDO_RUN_VOLT	3
+#define LDO_STBY_VOLT	4
+
+/* i is in REG_LDO1..REG_LDO4 */
+#define PF8X00_LDO(i)		(6 * (i - REG_LDO1) + 0x85)
+
+#define PF8X00_VSNVS_CONFIG1	0x9d
+
+#define PF8X00_PAGE_SELECT	0x9f
+#define PF8X00_OTP_FSOB_SELECT	0xa0
+#define PF8X00_OTP_I2C		0xa1
+#define PF8X00_OTP_CTRL1	0xa2
+#define PF8X00_OTP_CTRL2	0xa3
+#define PF8X00_OTP_CTRL3	0xa4
+
+#define PF8X00_OTP_FREQ_CTRL	0xa5
+#define PF8X00_OTP_COINCELL	0xa6
+#define PF8X00_OTP_PWRON	0xa7
+#define PF8X00_OTP_WD_CONFIG	0xa8
+#define PF8X00_OTP_WD_EXPIRE	0xa9
+#define PF8X00_OTP_WD_COUNTER	0xaa
+#define PF8X00_OTP_FAULT_COUNTER 0xab
+#define PF8X00_OTP_FAULT_TIMER	0xac
+#define PF8X00_OTP_PWRDN_DLY1	0xad
+#define PF8X00_OTP_PWRDN_DLY2	0xae
+#define PF8X00_OTP_PWRUP_CTRL	0xaf
+#define PF8X00_OTP_RESETBMCU	0xb0
+#define PF8X00_OTP_PGOOD	0xb1
+
+#define OTP_SW_VOLT	0
+#define OTP_SW_PWRUP	1
+#define OTP_SW_CONFIG1	2
+#define OTP_SW_CONFIG2	3
+
+/* i is in REG_SW1..REG_SW7 */
+#define PF8X00_OTP_SW(i)	(4 * (i - REG_SW1) + 0xb2)
+
+#define OTP_LDO_VOLT	0
+#define OTP_LDO_PWRUP	1
+#define OTP_LDO_CONFIG	2
+
+/* i is in REG_LDO1..REG_LDO4 */
+#define PF8X00_OTP_LDO(i)	(3 * (i - REG_LDO1) + 0xce)
+
+#define PF8X00_OTP_VSNVS_CONFIG	0xda
+#define PF8X00_OTP_OV_BYPASS1	0xdb
+#define PF8X00_OTP_OV_BYPASS2	0xdc
+#define PF8X00_OTP_UV_BYPASS1	0xdd
+#define PF8X00_OTP_UV_BYPASS2	0xde
+#define PF8X00_OTP_ILIM_BYPASS1	0xdf
+#define PF8X00_OTP_ILIM_BYPASS2	0xe0
+
+#define PF8X00_OTP_DEBUG1	0xe3
+#define PF8X_NUMREGS		0xe4
+
+#define REG_LDO1		0
+#define REG_LDO2		1
+#define REG_LDO3		2
+#define REG_LDO4		3
+#define REG_SW1			4
+#define REG_SW2			5
+#define REG_SW3			6
+#define REG_SW4			7
+#define REG_SW5			8
+#define REG_SW6			9
+#define REG_SW7			10
+#define REG_VSNVS		11
+#define REG_NUM_REGULATORS	(4 + 7 + 1)
+
+enum chips {
+	PF8100 = 0x40,
+	PF8121A = 0x42,
+	PF8200 = 0x48,
+};
+
+struct id_name {
+	enum chips id;
+	const char *name;
+};
+
+struct pf8x_regulator {
+	struct regulator_desc desc;
+	unsigned char stby_reg;
+	unsigned char stby_mask;
+	int ilim;
+	int phase;
+	unsigned char hw_en;
+	unsigned char vselect_en;
+	unsigned char quad_phase;
+	unsigned char dual_phase;
+	unsigned char fast_slew;
+};
+
+struct pf8x_chip {
+	int	chip_id;
+	int	prog_id;
+	int	clk_freq;
+	struct regmap *regmap;
+	struct device *dev;
+	struct pf8x_regulator regulator_descs[REG_NUM_REGULATORS];
+	struct regulator_dev *regulators[REG_NUM_REGULATORS];
+};
+
+/* Output: 1.5V to 5.0V, LDO2 can use VSELECT */
+static const int pf8x00_ldo_voltages[] = {
+	1500000, 1600000, 1800000, 1850000, 2150000, 2500000, 2800000, 3000000,
+	3100000, 3150000, 3200000, 3300000, 3350000, 4000000, 4900000, 5000000,
+};
+
+/* sw1 to sw6 voltages, from 0.4V to 1.49375V with 0.006250V increments */
+#define SWV(i)	(6250 * i + 400000)
+
+static const int pf8x00_sw1_to_6_voltages[] = {
+	SWV(0), SWV(1), SWV(2), SWV(3), SWV(4), SWV(5), SWV(6), SWV(7),
+	SWV(8), SWV(9), SWV(10), SWV(11), SWV(12), SWV(13), SWV(14), SWV(15),
+	SWV(16), SWV(17), SWV(18), SWV(19), SWV(20), SWV(21), SWV(22), SWV(23),
+	SWV(24), SWV(25), SWV(26), SWV(27), SWV(28), SWV(29), SWV(30), SWV(31),
+	SWV(32), SWV(33), SWV(34), SWV(35), SWV(36), SWV(37), SWV(38), SWV(39),
+	SWV(40), SWV(41), SWV(42), SWV(43), SWV(44), SWV(45), SWV(46), SWV(47),
+	SWV(48), SWV(49), SWV(50), SWV(51), SWV(52), SWV(53), SWV(54), SWV(55),
+	SWV(56), SWV(57), SWV(58), SWV(59), SWV(60), SWV(61), SWV(62), SWV(63),
+	SWV(64), SWV(65), SWV(66), SWV(67), SWV(68), SWV(69), SWV(70), SWV(71),
+	SWV(72), SWV(73), SWV(74), SWV(75), SWV(76), SWV(77), SWV(78), SWV(79),
+	SWV(80), SWV(81), SWV(82), SWV(83), SWV(84), SWV(85), SWV(86), SWV(87),
+	SWV(88), SWV(89), SWV(90), SWV(91), SWV(92), SWV(93), SWV(94), SWV(95),
+	SWV(96), SWV(97), SWV(98), SWV(99), SWV(100), SWV(101), SWV(102), SWV(103),
+	SWV(104), SWV(105), SWV(106), SWV(107), SWV(108), SWV(109), SWV(110), SWV(111),
+	SWV(112), SWV(113), SWV(114), SWV(115), SWV(116), SWV(117), SWV(118), SWV(119),
+	SWV(120), SWV(121), SWV(122), SWV(123), SWV(124), SWV(125), SWV(126), SWV(127),
+	SWV(128), SWV(129), SWV(130), SWV(131), SWV(132), SWV(133), SWV(134), SWV(135),
+	SWV(136), SWV(137), SWV(138), SWV(139), SWV(140), SWV(141), SWV(142), SWV(143),
+	SWV(144), SWV(145), SWV(146), SWV(147), SWV(148), SWV(149), SWV(150), SWV(151),
+	SWV(152), SWV(153), SWV(154), SWV(155), SWV(156), SWV(157), SWV(158), SWV(159),
+	SWV(160), SWV(161), SWV(162), SWV(163), SWV(164), SWV(165), SWV(166), SWV(167),
+	SWV(168), SWV(169), SWV(170), SWV(171), SWV(172), SWV(173), SWV(174), SWV(175),
+	1500000, 1800000,
+};
+
+/* Output: 1.0V to 4.1V */
+static const int pf8x00_sw7_voltages[] = {
+	1000000, 1100000, 1200000, 1250000, 1300000, 1350000, 1500000, 1600000,
+	1800000, 1850000, 2000000, 2100000, 2150000, 2250000, 2300000, 2400000,
+	2500000, 2800000, 3150000, 3200000, 3250000, 3300000, 3350000, 3400000,
+	3500000, 3800000, 4000000, 4100000, 4100000, 4100000, 4100000, 4100000,
+};
+
+/* Output: 1.8V, 3.0V, or 3.3V */
+static const int pf8x00_vsnvs_voltages[] = {
+	0, 1800000, 3000000, 3300000,
+};
+
+static const struct i2c_device_id pf8x_device_id[] = {
+	{.name = "pf8x00",},
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, pf8x_device_id);
+
+static const struct of_device_id pf8x_dt_ids[] = {
+	{ .compatible = "nxp,pf8x00",},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pf8x_dt_ids);
+
+const struct id_name id_list[] = {
+	{PF8100, "PF8100"},
+	{PF8121A, "PF8121A"},
+	{PF8200, "PF8200"},
+	{0, "???"},
+};
+
+const struct id_name *get_id_name(enum chips id)
+{
+	const struct id_name *p = id_list;
+
+	while (p->id) {
+		if (p->id == id)
+			break;
+		p++;
+	}
+	return p;
+}
+
+struct dvs_ramp {
+	unsigned short up_down_slow_fast[4];
+};
+
+/* Units uV/uS */
+struct dvs_ramp ramp_table[] = {
+/*			up_slow,	down_slow,	up_fast,	down_fast */
+	[0]  = {{ 7813,		5208,		15625,		10417 }},
+	[1]  = {{ 8203,		5469,		16406,		10938 }},
+	[2]  = {{ 8594,		5729,		17188,		11458 }},
+	[3]  = {{ 8984,		5990,		17969,		11979 }},
+	[4]  = {{ 9375,		6250,		18750,		12500 }},
+	[9]  = {{ 6250,		4167,		12500,		 8333 }},
+	[10] = {{ 6641,		4427,		13281,		 8854 }},
+	[11] = {{ 7031,		4688,		14063,		 9375 }},
+	[12] = {{ 7422,		4948,		14844,		 9896 }},
+};
+
+static int pf8x00_regulator_set_voltage_time_sel(struct regulator_dev *rdev,
+		unsigned int old_sel, unsigned int new_sel)
+{
+	struct pf8x_chip *pf = rdev_get_drvdata(rdev);
+	struct pf8x_regulator *rdesc = container_of(rdev->desc, struct pf8x_regulator, desc);
+	const unsigned int *volt_table = rdev->desc->volt_table;
+	int old_v = volt_table[old_sel];
+	int new_v = volt_table[new_sel];
+	int change = (new_v - old_v);
+	unsigned int index;
+	unsigned int slew;
+
+	index = (rdesc->fast_slew & 1) ? 2 : 0;
+	if (change < 0)
+		index++;
+	slew = ramp_table[pf->clk_freq].up_down_slow_fast[index];
+
+	return DIV_ROUND_UP(abs(change), slew);
+}
+
+static short ilim_table[] = {
+	2100, 2600, 3000, /* 4500, */
+};
+
+static int encode_ilim(struct pf8x_chip *pf, int ilim)
+{
+	int i = 0;
+
+	if (ilim <= 0)
+		return -1;
+	while (i < ARRAY_SIZE(ilim_table)) {
+		if (ilim <= ilim_table[i])
+			break;
+		i++;
+	}
+	return i;
+}
+
+static int encode_phase(struct pf8x_chip *pf, int phase)
+{
+	int ph;
+
+	if (phase < 0)
+		return -1;
+	ph = phase / 45;
+	if ((ph * 45) != phase) {
+		dev_err(pf->dev, "ignoring, illegal phase %d\n", phase);
+		return -1;
+	}
+	return (ph >= 1) ? ph - 1 : 7;
+}
+
+static int pf8x00_of_parse_cb(struct device_node *np,
+		const struct regulator_desc *desc,
+		struct regulator_config *config)
+{
+	struct pf8x_chip *pf = config->driver_data;
+	struct pf8x_regulator *rdesc = &pf->regulator_descs[desc->id];
+	unsigned char hw_en = 0;
+	unsigned char vselect_en = 0;
+	unsigned char quad_phase = 0;
+	unsigned char dual_phase = 0;
+	int ilim;
+	int phase;
+	int fast_slew;
+	int ret;
+
+	ret = of_property_read_u32(np, "nxp,ilim-ma",
+			&ilim);
+	if (ret)
+		ilim = -1;
+	ret = of_property_read_u32(np, "nxp,phase",
+			&phase);
+	if (ret)
+		phase = -1;
+	ilim = encode_ilim(pf, ilim);
+	phase = encode_phase(pf, phase);
+
+	ret = of_property_read_u32(np, "nxp,fast-slew", &fast_slew);
+	if (ret)
+		fast_slew = -1;
+
+	if (of_get_property(np, "nxp,hw-en", NULL))
+		hw_en = 1;
+	if (of_get_property(np, "nxp,quad-phase", NULL))
+		quad_phase = 1;
+	if (of_get_property(np, "nxp,dual-phase", NULL))
+		dual_phase = 1;
+	if (of_get_property(np, "nxp,vselect-en", NULL))
+		vselect_en = 1;
+
+	if ((desc->id != REG_SW1) && quad_phase) {
+		dev_err(pf->dev, "ignoring, only sw1 can use quad-phase\n");
+		quad_phase = 0;
+	}
+	if ((desc->id != REG_SW1) && (desc->id != REG_SW4)
+			 && (desc->id != REG_SW5) && dual_phase) {
+		dev_err(pf->dev, "ignoring, only sw1/sw4/sw5 can use dual-phase\n");
+		dual_phase = 0;
+	}
+	if ((desc->id != REG_LDO2) && vselect_en) {
+		/* LDO2 has gpio vselect */
+		dev_err(pf->dev, "ignoring, only ldo2 can use vselect-en\n");
+		vselect_en = 0;
+	}
+	if ((desc->id != REG_LDO2) && hw_en) {
+		/* LDO2 has gpio vselect */
+		dev_err(pf->dev, "ignoring, only ldo2 can use hw-en\n");
+		hw_en = 0;
+	}
+	if ((desc->id < REG_SW1) && (desc->id > REG_SW7)) {
+		if ((unsigned int)ilim <= 3) {
+			dev_err(pf->dev, "ignoring, only sw1-7 can use ilim\n");
+			ilim = -1;
+		}
+		if ((unsigned int)phase <= 7) {
+			dev_err(pf->dev, "ignoring, only sw1-7 can use phase\n");
+			ilim = -1;
+		}
+	}
+	rdesc->ilim = ilim;
+	rdesc->phase = phase;
+	rdesc->hw_en = hw_en;
+	rdesc->vselect_en = vselect_en;
+	rdesc->quad_phase = quad_phase;
+	rdesc->dual_phase = dual_phase;
+	rdesc->fast_slew = fast_slew;
+
+	return 0;
+}
+
+static const struct regulator_ops pf8x00_ldo_regulator_ops = {
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = regulator_list_voltage_table,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+};
+
+static const struct regulator_ops pf8x00_sw_regulator_ops = {
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = regulator_list_voltage_table,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_time_sel = pf8x00_regulator_set_voltage_time_sel,
+};
+
+static const struct regulator_ops pf8x00_vsnvs_regulator_ops = {
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = regulator_list_voltage_table,
+	.map_voltage = regulator_map_voltage_ascend,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+};
+
+#define STRUCT_LDO_REG(_id, _name, base, voltages)		\
+	[_id] = {						\
+		.desc = {					\
+			.name = #_name,				\
+			.of_match = of_match_ptr(#_name),	\
+			.regulators_node = of_match_ptr("regulators"), \
+			.of_parse_cb = pf8x00_of_parse_cb,	\
+			.n_voltages = ARRAY_SIZE(voltages),	\
+			.ops = &pf8x00_ldo_regulator_ops,	\
+			.type = REGULATOR_VOLTAGE,		\
+			.id = _id,				\
+			.owner = THIS_MODULE,			\
+			.volt_table = voltages,			\
+			.vsel_reg = (base) + LDO_RUN_VOLT,	\
+			.vsel_mask = 0xff,			\
+			.enable_reg = (base) + LDO_CONFIG2,	\
+			.enable_val = 0x2,			\
+			.disable_val = 0x0,			\
+			.enable_mask = 2,			\
+		},						\
+		.stby_reg = (base) + LDO_STBY_VOLT,		\
+		.stby_mask = 0x20,				\
+	}
+
+#define STRUCT_SW_REG(_id, _name, base, voltages)		\
+	[_id] = {						\
+		.desc = {					\
+			.name = #_name,				\
+			.of_match = of_match_ptr(#_name),	\
+			.regulators_node = of_match_ptr("regulators"), \
+			.of_parse_cb = pf8x00_of_parse_cb,	\
+			.n_voltages = ARRAY_SIZE(voltages),	\
+			.ops = &pf8x00_sw_regulator_ops,	\
+			.type = REGULATOR_VOLTAGE,		\
+			.id = _id,				\
+			.owner = THIS_MODULE,			\
+			.volt_table = voltages,			\
+			.vsel_reg = (base) + SW_RUN_VOLT,	\
+			.vsel_mask = 0xff,			\
+			.enable_reg = (base) + SW_MODE1,	\
+			.enable_val = 0x3,			\
+			.disable_val = 0x0,			\
+			.enable_mask = 0x3,			\
+			.enable_time = 500,			\
+		},						\
+		.stby_reg = (base) + SW_STBY_VOLT,		\
+		.stby_mask = 0xff,				\
+	}
+
+
+#define STRUCT_VSNVS_REG(_id, _name, base, voltages)		\
+	[_id] = {						\
+		.desc = {					\
+			.name = #_name,				\
+			.of_match = of_match_ptr(#_name),	\
+			.regulators_node = of_match_ptr("regulators"), \
+			.of_parse_cb = pf8x00_of_parse_cb,	\
+			.n_voltages = ARRAY_SIZE(voltages),	\
+			.ops = &pf8x00_vsnvs_regulator_ops,	\
+			.type = REGULATOR_VOLTAGE,		\
+			.id = _id,				\
+			.owner = THIS_MODULE,			\
+			.volt_table = voltages,			\
+			.vsel_reg = (base),			\
+			.vsel_mask = 0x3,			\
+		},						\
+	}
+
+static const struct pf8x_regulator pf8x00_regulators[] = {
+	STRUCT_LDO_REG(REG_LDO1, ldo1, PF8X00_LDO(REG_LDO1), pf8x00_ldo_voltages),
+	STRUCT_LDO_REG(REG_LDO2, ldo2, PF8X00_LDO(REG_LDO2), pf8x00_ldo_voltages),
+	STRUCT_LDO_REG(REG_LDO3, ldo3, PF8X00_LDO(REG_LDO3), pf8x00_ldo_voltages),
+	STRUCT_LDO_REG(REG_LDO4, ldo4, PF8X00_LDO(REG_LDO4), pf8x00_ldo_voltages),
+	STRUCT_SW_REG(REG_SW1, sw1, PF8X00_SW(REG_SW1), pf8x00_sw1_to_6_voltages),
+	STRUCT_SW_REG(REG_SW2, sw2, PF8X00_SW(REG_SW2), pf8x00_sw1_to_6_voltages),
+	STRUCT_SW_REG(REG_SW3, sw3, PF8X00_SW(REG_SW3), pf8x00_sw1_to_6_voltages),
+	STRUCT_SW_REG(REG_SW4, sw4, PF8X00_SW(REG_SW4), pf8x00_sw1_to_6_voltages),
+	STRUCT_SW_REG(REG_SW5, sw5, PF8X00_SW(REG_SW5), pf8x00_sw1_to_6_voltages),
+	STRUCT_SW_REG(REG_SW6, sw6, PF8X00_SW(REG_SW6), pf8x00_sw1_to_6_voltages),
+	STRUCT_SW_REG(REG_SW7, sw7, PF8X00_SW(REG_SW7), pf8x00_sw7_voltages),
+	STRUCT_VSNVS_REG(REG_VSNVS, vsnvs, PF8X00_VSNVS_CONFIG1, pf8x00_vsnvs_voltages),
+};
+
+#ifdef CONFIG_OF
+static struct of_regulator_match pf8x00_matches[] = {
+	{ .name = "ldo1",	},
+	{ .name = "ldo2",	},
+	{ .name = "ldo3",	},
+	{ .name = "ldo4",	},
+	{ .name = "sw1",	},
+	{ .name = "sw2",	},
+	{ .name = "sw3",	},
+	{ .name = "sw4",	},
+	{ .name = "sw5",	},
+	{ .name = "sw6",	},
+	{ .name = "sw7",	},
+	{ .name = "vsnvs",	},
+};
+
+static int pf8x_parse_regulators_dt(struct pf8x_chip *pf)
+{
+	struct device *dev = pf->dev;
+	struct device_node *np, *parent;
+	int ret;
+
+	np = of_node_get(dev->of_node);
+	if (!np)
+		return -EINVAL;
+
+	parent = of_get_child_by_name(np, "regulators");
+	if (!parent) {
+		dev_err(dev, "regulators node not found\n");
+		return -EINVAL;
+	}
+
+	ret = of_regulator_match(dev, parent, pf8x00_matches,
+				 ARRAY_SIZE(pf8x00_matches));
+
+	of_node_put(parent);
+	if (ret < 0) {
+		dev_err(dev, "Error parsing regulator init data: %d\n",
+			ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static inline struct regulator_init_data *match_init_data(int index)
+{
+	return pf8x00_matches[index].init_data;
+}
+
+static inline struct device_node *match_of_node(int index)
+{
+	return pf8x00_matches[index].of_node;
+}
+#else
+static int pf8x_parse_regulators_dt(struct pf8x_chip *pf)
+{
+	return 0;
+}
+
+static inline struct regulator_init_data *match_init_data(int index)
+{
+	return NULL;
+}
+
+static inline struct device_node *match_of_node(int index)
+{
+	return NULL;
+}
+#endif
+
+static int pf8x_identify(struct pf8x_chip *pf)
+{
+	const struct id_name *p;
+	unsigned int value, id1, id2;
+	int ret;
+
+	ret = regmap_read(pf->regmap, PF8X00_DEVICEID, &value);
+	if (ret)
+		return ret;
+
+	pf->chip_id = value;
+	p = get_id_name(value);
+	if (p->id != value) {
+		dev_warn(pf->dev, "Illegal ID: %x\n", value);
+		return -ENODEV;
+	}
+
+	ret = regmap_read(pf->regmap, PF8X00_REVID, &value);
+	if (ret)
+		value = 0;
+	ret = regmap_read(pf->regmap, PF8X00_EMREV, &id1);
+	if (ret)
+		id1 = 0;
+	ret = regmap_read(pf->regmap, PF8X00_PROGID, &id2);
+	if (ret)
+		id2 = 0;
+	pf->prog_id = (id1 << 8) | id2;
+
+	dev_info(pf->dev, "%s: Full layer: %x, Metal layer: %x, prog_id=0x%04x\n",
+		 p->name, (value & 0xf0) >> 4, value & 0x0f, pf->prog_id);
+
+	return 0;
+}
+
+static const struct regmap_config pf8x_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = PF8X_NUMREGS - 1,
+	.cache_type = REGCACHE_RBTREE,
+};
+
+struct otp_reg_lookup {
+	unsigned short prog_id;
+	unsigned char reg;
+	unsigned char value;
+};
+
+static const struct otp_reg_lookup otp_map[] = {
+	{ 0x401c, PF8X00_OTP_CTRL3, 0 },
+	{ 0x4008, PF8X00_OTP_CTRL3, 0x04 },
+	{ 0x301d, PF8X00_OTP_CTRL3, 0x04 },	/* test only */
+	{ 0, 0, 0 },
+};
+
+static int get_otp_reg(struct pf8x_chip *pf, unsigned char reg)
+{
+	const struct otp_reg_lookup *p = otp_map;
+
+	while (p->reg) {
+		if ((pf->prog_id == p->prog_id) && (reg == p->reg))
+			return p->value;
+		p++;
+	}
+
+	dev_err(pf->dev, "reg(0x%02x) not found for 0x%04x\n",
+		 reg, pf->prog_id);
+	return -EINVAL;
+}
+
+static int pf8x00_regulator_probe(struct i2c_client *client,
+				    const struct i2c_device_id *id)
+{
+	struct pf8x_chip *pf;
+	struct regulator_config config = { };
+	int i, ret;
+	u32 num_regulators;
+	unsigned int hw_en;
+	unsigned int vselect_en;
+	unsigned char quad_phase;
+	unsigned char dual_phase;
+	unsigned int val;
+	int ctrl3;
+	const char *format = NULL;
+	unsigned int clk_freq = 0;
+
+	pf = devm_kzalloc(&client->dev, sizeof(*pf),
+			GFP_KERNEL);
+	if (!pf)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, pf);
+	pf->dev = &client->dev;
+
+	pf->regmap = devm_regmap_init_i2c(client, &pf8x_regmap_config);
+	if (IS_ERR(pf->regmap)) {
+		ret = PTR_ERR(pf->regmap);
+		dev_err(&client->dev,
+			"regmap allocation failed with err %d\n", ret);
+		return ret;
+	}
+
+	ret = pf8x_identify(pf);
+	if (ret)
+		return ret;
+
+	dev_info(&client->dev, "%s found.\n",
+		get_id_name(pf->chip_id)->name);
+
+	ret = regmap_read(pf->regmap, PF8X00_FREQ_CTRL, &clk_freq);
+	clk_freq &= 0xf;
+	if (ret < 0)
+		clk_freq = 0;
+	if (((clk_freq & 7) > 4) || (clk_freq == 8))
+		clk_freq = 0;
+	pf->clk_freq = clk_freq;
+
+	memcpy(pf->regulator_descs, pf8x00_regulators,
+		sizeof(pf->regulator_descs));
+
+	ret = pf8x_parse_regulators_dt(pf);
+	if (ret)
+		return ret;
+
+	num_regulators = ARRAY_SIZE(pf->regulator_descs);
+	for (i = 0; i < num_regulators; i++) {
+		struct regulator_init_data *init_data;
+		struct regulator_desc *desc;
+
+		desc = &pf->regulator_descs[i].desc;
+		init_data = match_init_data(i);
+
+		config.dev = &client->dev;
+		config.init_data = init_data;
+		config.driver_data = pf;
+		config.of_node = match_of_node(i);
+		config.ena_gpiod = NULL;
+
+		pf->regulators[i] =
+			devm_regulator_register(&client->dev, desc, &config);
+		if (IS_ERR(pf->regulators[i])) {
+			dev_err(&client->dev, "register regulator%s failed\n",
+				desc->name);
+			return PTR_ERR(pf->regulators[i]);
+		}
+		if ((i >= REG_SW1) && (i <= REG_SW7)) {
+			unsigned int phase = pf->regulator_descs[i].phase;
+			unsigned int ilim = pf->regulator_descs[i].ilim;
+			unsigned int mask = 0;
+			unsigned int val = 0;
+			unsigned int reg = PF8X00_SW(i) + SW_CONFIG2;
+			unsigned int fast_slew = pf->regulator_descs[i].fast_slew;
+
+			if (phase <= 7) {
+				mask |= 7;
+				val |= phase;
+			}
+			if (ilim <= 3) {
+				mask |= 3 << 3;
+				val |= ilim << 3;
+			}
+			if (fast_slew <= 1) {
+				mask |= 1 << 5;
+				val |= fast_slew << 5;
+			}
+			if (mask) {
+				ret = regmap_update_bits(pf->regmap, reg, mask,
+						val);
+			}
+			if (fast_slew > 1) {
+				ret = regmap_read(pf->regmap, reg, &fast_slew);
+				fast_slew &= 0x20;
+				if (ret < 0)
+					fast_slew = 0;
+				pf->regulator_descs[i].fast_slew = fast_slew >> 5;
+			}
+		}
+	}
+	hw_en = pf->regulator_descs[REG_LDO2].hw_en;
+	vselect_en = pf->regulator_descs[REG_LDO2].vselect_en;
+	val = vselect_en ? 8 : 0;
+	if (hw_en)
+		val |= 0x10;
+	ret = regmap_update_bits(pf->regmap,
+			PF8X00_LDO(REG_LDO2) + LDO_CONFIG2,
+				 0x18, val);
+
+	ctrl3 = get_otp_reg(pf, PF8X00_OTP_CTRL3);
+	if (ctrl3 >= 0) {
+		quad_phase = pf->regulator_descs[REG_SW1].quad_phase;
+		dual_phase = pf->regulator_descs[REG_SW1].dual_phase;
+		if (quad_phase) {
+			if ((ctrl3 & 3) != 2)
+				format = "sw1 quad_phase not set in otp_ctrl3 %x\n";
+
+		} else if (dual_phase) {
+			if ((ctrl3 & 3) != 1)
+				format = "sw1 dual_phase not set in otp_ctrl3 %x\n";
+		} else if (ctrl3 & 3) {
+			format = "sw1 single_phase not set in otp_ctrl3 %x\n";
+		}
+		if (!quad_phase) {
+			dual_phase = pf->regulator_descs[REG_SW4].dual_phase;
+			if (dual_phase) {
+				if ((ctrl3 & 0x0c) != 4)
+					format = "sw4 dual_phase not set in otp_ctrl3 %x\n";
+			} else if (ctrl3 & 0x0c) {
+				format = "sw4 single_phase not set in otp_ctrl3 %x\n";
+			}
+		}
+		dual_phase = pf->regulator_descs[REG_SW5].dual_phase;
+		if (dual_phase) {
+			if ((ctrl3 & 0x30) != 0x10)
+				format = "sw5 dual_phase not set in otp_ctrl3 %x\n";
+		} else if (ctrl3 & 0x30) {
+			format = "sw5 single_phase not set in otp_ctrl3 %x\n";
+		}
+		if (format) {
+			dev_err(pf->dev, format, ctrl3);
+			dev_err(pf->dev, "!!!try updating u-boot, boot.scr, or pmic\n");
+		}
+	}
+	return 0;
+}
+
+static int pf8x_suspend(struct device *dev)
+{
+	return 0;
+}
+
+static int pf8x_resume(struct device *dev)
+{
+	return 0;
+}
+
+static const struct dev_pm_ops pf8x_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pf8x_suspend, pf8x_resume)
+};
+
+static struct i2c_driver pf8x_driver = {
+	.id_table = pf8x_device_id,
+	.driver = {
+		.name = "pf8x00-regulator",
+		.of_match_table = pf8x_dt_ids,
+		.pm = &pf8x_pm_ops,
+	},
+	.probe = pf8x00_regulator_probe,
+};
+
+static int __init pf8x_init(void)
+{
+	return i2c_add_driver(&pf8x_driver);
+}
+subsys_initcall(pf8x_init);
+
+static void __exit pf8x_exit(void)
+{
+	i2c_del_driver(&pf8x_driver);
+}
+module_exit(pf8x_exit);
+
+MODULE_AUTHOR("Troy Kisky <troy.kisky@boundarydevices.com>");
+MODULE_AUTHOR("Adrien Grassein <adrien.grassein@gmail.com>");
+MODULE_DESCRIPTION("Regulator Driver for NXP's PF8100/PF8200 PMIC");
+MODULE_LICENSE("GPL v2");
-- 
2.20.1

Re: [PATCH 2/2] regulator: pf8x00: add support of nxp pf8x00 regulator

[Mark Brown]

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

On Sun, Dec 06, 2020 at 01:26:29AM +0100, Adrien Grassein wrote:

> +/* sw1 to sw6 voltages, from 0.4V to 1.49375V with 0.006250V increments */
> +#define SWV(i)	(6250 * i + 400000)
> +
> +static const int pf8x00_sw1_to_6_voltages[] = {
> +	SWV(0), SWV(1), SWV(2), SWV(3), SWV(4), SWV(5), SWV(6), SWV(7),

There are helpers for linear mappings, please use them directly rather
than open coding - this is clearer and much more efficient at runtime.

> +static const struct i2c_device_id pf8x_device_id[] = {
> +	{.name = "pf8x00",},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, pf8x_device_id);

There should be separate IDs for the individual parts as with the OF
compatible.

> +static int pf8x00_regulator_set_voltage_time_sel(struct regulator_dev *rdev,
> +		unsigned int old_sel, unsigned int new_sel)
> +{
> +	struct pf8x_chip *pf = rdev_get_drvdata(rdev);
> +	struct pf8x_regulator *rdesc = container_of(rdev->desc, struct pf8x_regulator, desc);
> +	const unsigned int *volt_table = rdev->desc->volt_table;
> +	int old_v = volt_table[old_sel];
> +	int new_v = volt_table[new_sel];
> +	int change = (new_v - old_v);
> +	unsigned int index;
> +	unsigned int slew;
> +
> +	index = (rdesc->fast_slew & 1) ? 2 : 0;

Please write normal conditional statements to make things easier to
read.

> +	if ((desc->id != REG_SW1) && quad_phase) {
> +		dev_err(pf->dev, "ignoring, only sw1 can use quad-phase\n");
> +		quad_phase = 0;
> +	}
> +	if ((desc->id != REG_SW1) && (desc->id != REG_SW4)
> +			 && (desc->id != REG_SW5) && dual_phase) {
> +		dev_err(pf->dev, "ignoring, only sw1/sw4/sw5 can use dual-phase\n");
> +		dual_phase = 0;
> +	}

This wasn't in the binding document...

> +static int pf8x_parse_regulators_dt(struct pf8x_chip *pf)
> +{
> +	struct device *dev = pf->dev;
> +	struct device_node *np, *parent;
> +	int ret;
> +
> +	np = of_node_get(dev->of_node);
> +	if (!np)
> +		return -EINVAL;
> +
> +	parent = of_get_child_by_name(np, "regulators");
> +	if (!parent) {
> +		dev_err(dev, "regulators node not found\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = of_regulator_match(dev, parent, pf8x00_matches,
> +				 ARRAY_SIZE(pf8x00_matches));

Don't open code binding parsing, just specify the relevant names in the
regulator decriptors, register them and and let the core parse things
for you.  

> +			if (ilim <= 3) {
> +				mask |= 3 << 3;
> +				val |= ilim << 3;
> +			}
> +			if (fast_slew <= 1) {
> +				mask |= 1 << 5;
> +				val |= fast_slew << 5;
> +			}

More blank lines between blocks please.

> +static int pf8x_suspend(struct device *dev)
> +{
> +	return 0;
> +}
> +
> +static int pf8x_resume(struct device *dev)
> +{
> +	return 0;
> +}

Remove empty functions.

> +static int __init pf8x_init(void)
> +{
> +	return i2c_add_driver(&pf8x_driver);
> +}
> +subsys_initcall(pf8x_init);

You should be able to use module_i2c_driver() here.

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

Re: [PATCH 2/2] regulator: pf8x00: add support of nxp pf8x00 regulator

[kernel test robot]

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

Hi Adrien,

I love your patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v5.10-rc7]
[cannot apply to regulator/for-next next-20201207]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Adrien-Grassein/dt-bindings-regulator-Add-pf8x00-regulator/20201206-083433
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: s390-randconfig-r036-20201207 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project a2f922140f5380571fb74179f2bf622b3b925697)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/0day-ci/linux/commit/efbdd07e33c1265802f429333ea350f206b26406
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Adrien-Grassein/dt-bindings-regulator-Add-pf8x00-regulator/20201206-083433
        git checkout efbdd07e33c1265802f429333ea350f206b26406
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:22:
   In file included from include/linux/writeback.h:14:
   In file included from include/linux/blk-cgroup.h:23:
   In file included from include/linux/blkdev.h:26:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:80:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
           ___constant_swab32(x) :                 \
                              ^
   include/uapi/linux/swab.h:21:12: note: expanded from macro '___constant_swab32'
           (((__u32)(x) & (__u32)0x00ff0000UL) >>  8) |            \
                     ^
   In file included from drivers/regulator/pf8x00-regulator.c:15:
   In file included from include/linux/regulator/driver.h:18:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:22:
   In file included from include/linux/writeback.h:14:
   In file included from include/linux/blk-cgroup.h:23:
   In file included from include/linux/blkdev.h:26:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:80:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
           ___constant_swab32(x) :                 \
                              ^
   include/uapi/linux/swab.h:22:12: note: expanded from macro '___constant_swab32'
           (((__u32)(x) & (__u32)0xff000000UL) >> 24)))
                     ^
   In file included from drivers/regulator/pf8x00-regulator.c:15:
   In file included from include/linux/regulator/driver.h:18:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:22:
   In file included from include/linux/writeback.h:14:
   In file included from include/linux/blk-cgroup.h:23:
   In file included from include/linux/blkdev.h:26:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:80:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:120:12: note: expanded from macro '__swab32'
           __fswab32(x))
                     ^
   In file included from drivers/regulator/pf8x00-regulator.c:15:
   In file included from include/linux/regulator/driver.h:18:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:22:
   In file included from include/linux/writeback.h:14:
   In file included from include/linux/blk-cgroup.h:23:
   In file included from include/linux/blkdev.h:26:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:80:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
>> drivers/regulator/pf8x00-regulator.c:263:23: warning: no previous prototype for function 'get_id_name' [-Wmissing-prototypes]
   const struct id_name *get_id_name(enum chips id)
                         ^
   drivers/regulator/pf8x00-regulator.c:263:7: note: declare 'static' if the function is not intended to be used outside of this translation unit
   const struct id_name *get_id_name(enum chips id)
         ^
   static 
   drivers/regulator/pf8x00-regulator.c:831:12: warning: unused function 'pf8x_suspend' [-Wunused-function]
   static int pf8x_suspend(struct device *dev)
              ^
   drivers/regulator/pf8x00-regulator.c:836:12: warning: unused function 'pf8x_resume' [-Wunused-function]
   static int pf8x_resume(struct device *dev)
              ^
   23 warnings generated.

vim +/get_id_name +263 drivers/regulator/pf8x00-regulator.c

   262	
 > 263	const struct id_name *get_id_name(enum chips id)
   264	{
   265		const struct id_name *p = id_list;
   266	
   267		while (p->id) {
   268			if (p->id == id)
   269				break;
   270			p++;
   271		}
   272		return p;
   273	}
   274	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30392 bytes --]

Re: [PATCH 1/2] dt-bindings: regulator: Add pf8x00 regulator

[Mark Brown]

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

On Sun, Dec 06, 2020 at 01:26:28AM +0100, Adrien Grassein wrote:
> Add dt-bindings for the pf8x00 driver.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

> +  compatible:
> +    enum:
> +      - nxp,pf8x00

Compatible strings should be for specific devices not wildcards.

> +          nxp,hw-en:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: |
> +              Only available for ldo2. Used to enable or disable ld02.

I don't understand what this is documenting - what is "hw-en" and how is
it used to enable or disable LDO2?

> +          nxp,vselect-en:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: |
> +              Only available for ldo2. When specified, use the VSELECT pin
> +              of the chip to control the output voltage of the ldo02 regulator.

Shouldn't there be a GPIO specified somewhere or something so that the
VSELECT pin can be controlled?  

> +          nxp,ilim-ma:
> +            $ref: /schemas/types.yaml#definitions/uint32
> +            minimum: 2100
> +            maximum: 4500
> +            default: 2100
> +            enum: [ 2100, 2600, 3000, 4500 ]
> +            description: |
> +              Defines the maximum current delivered by the regulator (in mA).

Is this not a fixed property of the regulator?

> +          nxp,quad-phase:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: |
> +              This allow regulators  sw1 and sw2, or sw3 and sw4 or sw4 and sw5
> +              to work together to deliver a maximum 10A current.

Presumably this must be set on both the regulators being grouped
together?

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

Re: [PATCH 1/2] dt-bindings: regulator: Add pf8x00 regulator

[Adrien Grassein]

Hi,

Thanks for reviewing my patches.

Le lun. 7 déc. 2020 à 14:55, Mark Brown <broonie@kernel.org> a écrit :
>
> On Sun, Dec 06, 2020 at 01:26:28AM +0100, Adrien Grassein wrote:
> > Add dt-bindings for the pf8x00 driver.
>
> Please submit patches using subject lines reflecting the style for the
> subsystem, this makes it easier for people to identify relevant patches.
> Look at what existing commits in the area you're changing are doing and
> make sure your subject lines visually resemble what they're doing.
> There's no need to resubmit to fix this alone.
>

For v2 I just copy-paste another commit message to be sure to be conform.

> > +  compatible:
> > +    enum:
> > +      - nxp,pf8x00
>
> Compatible strings should be for specific devices not wildcards.
>
> > +          nxp,hw-en:
> > +            $ref: /schemas/types.yaml#definitions/flag
> > +            description: |
> > +              Only available for ldo2. Used to enable or disable ld02.
>
> I don't understand what this is documenting - what is "hw-en" and how is
> it used to enable or disable LDO2?
I think I read better documentation for this point. Sorry, it was very unclear.
>
> > +          nxp,vselect-en:
> > +            $ref: /schemas/types.yaml#definitions/flag
> > +            description: |
> > +              Only available for ldo2. When specified, use the VSELECT pin
> > +              of the chip to control the output voltage of the ldo02 regulator.
>
> Shouldn't there be a GPIO specified somewhere or something so that the
> VSELECT pin can be controlled?

I think I read better documentation for this point. Sorry, it was very unclear.
VSELECT is in fact an input pin of the chip. The configuration just enabled it.
>
> > +          nxp,ilim-ma:
> > +            $ref: /schemas/types.yaml#definitions/uint32
> > +            minimum: 2100
> > +            maximum: 4500
> > +            default: 2100
> > +            enum: [ 2100, 2600, 3000, 4500 ]
> > +            description: |
> > +              Defines the maximum current delivered by the regulator (in mA).
>
> Is this not a fixed property of the regulator?
It's not. It's configurable by software.
>
> > +          nxp,quad-phase:
> > +            $ref: /schemas/types.yaml#definitions/flag
> > +            description: |
> > +              This allow regulators  sw1 and sw2, or sw3 and sw4 or sw4 and sw5
> > +              to work together to deliver a maximum 10A current.
>
> Presumably this must be set on both the regulators being grouped
> together?
Not. Only the sw1 configuration will be taken in account.

Thanks again,
Adrien

Re: [PATCH 1/2] dt-bindings: regulator: Add pf8x00 regulator

[Mark Brown]

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

On Thu, Dec 10, 2020 at 11:24:17PM +0100, Adrien Grassein wrote:
> Le lun. 7 déc. 2020 à 14:55, Mark Brown <broonie@kernel.org> a écrit :
> > On Sun, Dec 06, 2020 at 01:26:28AM +0100, Adrien Grassein wrote:

> > > Add dt-bindings for the pf8x00 driver.

> > Please submit patches using subject lines reflecting the style for the
> > subsystem, this makes it easier for people to identify relevant patches.
> > Look at what existing commits in the area you're changing are doing and
> > make sure your subject lines visually resemble what they're doing.
> > There's no need to resubmit to fix this alone.

> For v2 I just copy-paste another commit message to be sure to be conform.

For patches for a given subsystem you should use the prefix the
subsystem uses, for regulator that's "regulator: ".

> > > +            $ref: /schemas/types.yaml#definitions/flag
> > > +            description: |
> > > +              Only available for ldo2. When specified, use the VSELECT pin
> > > +              of the chip to control the output voltage of the ldo02 regulator.

> > Shouldn't there be a GPIO specified somewhere or something so that the
> > VSELECT pin can be controlled?

> I think I read better documentation for this point. Sorry, it was very unclear.
> VSELECT is in fact an input pin of the chip. The configuration just enabled it.

Then presumably you need some binding to specify how to control this
input too?

> > > +          nxp,quad-phase:
> > > +            $ref: /schemas/types.yaml#definitions/flag
> > > +            description: |
> > > +              This allow regulators  sw1 and sw2, or sw3 and sw4 or sw4 and sw5
> > > +              to work together to deliver a maximum 10A current.

> > Presumably this must be set on both the regulators being grouped
> > together?

> Not. Only the sw1 configuration will be taken in account.

That needs to be documented then.

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

Re: [PATCH 1/2] dt-bindings: regulator: Add pf8x00 regulator

[Rob Herring]

On Sun, Dec 06, 2020 at 01:26:28AM +0100, Adrien Grassein wrote:
> Add dt-bindings for the pf8x00 driver.
> 
> Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com>
> ---
>  .../regulator/nxp,pf8x00-regulator.yaml       | 223 ++++++++++++++++++
>  MAINTAINERS                                   |   6 +
>  2 files changed, 229 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
> new file mode 100644
> index 000000000000..f4e4f545c5a3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
> @@ -0,0 +1,223 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/nxp,pf8x00-regulator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP PF8x00 Power Management Integrated Circuit regulators
> +
> +maintainers:
> +  - Adrien Grassein <adrien.grassein@gmail.com>
> +
> +description: |
> +  pf8x00 are a 12-chanel regulator PMIC family. Regulators nodes should
> +  be named to ldo<>, sw<> and vnss. The definition for each of these nodes
> +  is defined using the standard binding for regulators at
> +  Documentation/devicetree/bindings/regulator/regulator.txt.
> +  Datasheet is available at https://www.nxp.com/docs/en/data-sheet/PF8100_PF8200.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - nxp,pf8x00
> +
> +  reg:
> +    maxItems: 1
> +
> +  regulators:
> +    type: object
> +    description: |

Don't need '|' if no formatting.

> +      list of regulators provided by this controller
> +
> +    patternProperties:
> +      "^ldo[1-4]$":
> +        type: object
> +        $ref: regulator.yaml#
> +        description: |
> +          Properties for single LDO regulator.
> +
> +        properties:
> +          regulator-name:
> +            pattern: "^ldo[1-4]$"
> +            description: |
> +              should be ldo1", ..., "ldo4"
> +
> +          nxp,hw-en:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: |
> +              Only available for ldo2. Used to enable or disable ld02.
> +
> +          nxp,vselect-en:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: |
> +              Only available for ldo2. When specified, use the VSELECT pin
> +              of the chip to control the output voltage of the ldo02 regulator.
> +
> +        unevaluatedProperties: false
> +
> +      "^sw[1-7]$":
> +        type: object
> +        $ref: regulator.yaml#
> +        description: |
> +          Properties for single SW regulator.
> +
> +        properties:
> +          regulator-name:
> +            pattern: "^sw[1-7]$"
> +            description: |
> +              should be sw1", ..., "sw7"
> +
> +          nxp,ilim-ma:

Use unit suffix defined in property-units.txt.

> +            $ref: /schemas/types.yaml#definitions/uint32
> +            minimum: 2100
> +            maximum: 4500
> +            default: 2100
> +            enum: [ 2100, 2600, 3000, 4500 ]
> +            description: |
> +              Defines the maximum current delivered by the regulator (in mA).
> +
> +          nxp,phase:
> +            $ref: /schemas/types.yaml#definitions/uint32
> +            minimum: 0
> +            maximum: 315
> +            default: 0
> +            enum: [ 0, 45, 90, 135, 180, 225, 270, 315 ]
> +            description: |
> +               This controls the phase shift of the switching frequency.
> +
> +          nxp,fsl,fast-slew:

nxp or fsl?

> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: |
> +              Controls the DVS ramp of the regulator.
> +
> +          nxp,quad-phase:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: |
> +              This allow regulators  sw1 and sw2, or sw3 and sw4 or sw4 and sw5
> +              to work together to deliver a maximum 10A current.
> +
> +          nxp,dual-phase:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: |
> +              This allow regulators  sw1, sw2, sw3 and sw4 to work together
> +              to deliver a maximum 5A current.
> +
> +        unevaluatedProperties: false
> +
> +      "^vsnvs$":

Move to 'properties', not a pattern.

> +        type: object
> +        $ref: regulator.yaml#
> +        description: |
> +          Properties for vsnvs regulator.
> +
> +        unevaluatedProperties: false
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - regulators
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      pmic: pf8100@8 {

pmic@8

> +        compatible = "nxp,pf8x00";
> +        reg = <0x08>;
> +
> +        regulators {
> +            reg_ldo1: ldo1 {
> +              regulator-always-on;
> +              regulator-boot-on;
> +              regulator-max-microvolt = <5000000>;
> +              regulator-min-microvolt = <1500000>;
> +            };
> +
> +            reg_ldo2: ldo2 {
> +              regulator-always-on;
> +              regulator-boot-on;
> +              regulator-max-microvolt = <5000000>;
> +              regulator-min-microvolt = <1500000>;
> +            };
> +
> +            reg_ldo3: ldo3 {
> +              regulator-always-on;
> +              regulator-boot-on;
> +              regulator-max-microvolt = <5000000>;
> +              regulator-min-microvolt = <1500000>;
> +            };
> +
> +            reg_ldo4: ldo4 {
> +              regulator-always-on;
> +              regulator-boot-on;
> +              regulator-max-microvolt = <5000000>;
> +              regulator-min-microvolt = <1500000>;
> +            };
> +
> +            reg_sw1: sw1 {
> +              nxp,phase = <0>;
> +              nxp,ilim-ma = <4500>;
> +              regulator-always-on;
> +              regulator-boot-on;
> +              regulator-max-microvolt = <1800000>;
> +              regulator-min-microvolt =  <400000>;
> +            };
> +
> +            reg_sw2: sw2 {
> +              regulator-always-on;
> +              regulator-boot-on;
> +              regulator-max-microvolt = <1800000>;
> +              regulator-min-microvolt =  <400000>;
> +            };
> +
> +            reg_sw3: sw3 {
> +              regulator-always-on;
> +              regulator-boot-on;
> +              regulator-max-microvolt = <1800000>;
> +              regulator-min-microvolt =  <400000>;
> +              nxp,fast-slew = <1>;
> +            };
> +
> +            reg_sw4: sw4 {
> +              regulator-always-on;
> +              regulator-boot-on;
> +              regulator-max-microvolt = <1800000>;
> +              regulator-min-microvolt =  <400000>;
> +              nxp,fast-slew = <1>;
> +            };
> +
> +            reg_sw5: sw5 {
> +              regulator-always-on;
> +              regulator-boot-on;
> +              regulator-max-microvolt = <1800000>;
> +              regulator-min-microvolt =  <400000>;
> +            };
> +
> +           reg_sw6: sw6 {
> +             regulator-always-on;
> +             regulator-boot-on;
> +             regulator-max-microvolt = <1800000>;
> +             regulator-min-microvolt =  <400000>;
> +           };
> +
> +           reg_sw7: sw7 {
> +             regulator-always-on;
> +             regulator-boot-on;
> +             regulator-max-microvolt = <4100000>;
> +             regulator-min-microvolt = <1000000>;
> +           };
> +
> +          reg_vsnvs: vsnvs {
> +            regulator-always-on;
> +            regulator-boot-on;
> +            regulator-max-microvolt = <3300000>;
> +            regulator-min-microvolt = <1800000>;
> +          };
> +        };
> +      };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ebe4829cdd4d..4e266f1e9a2f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13730,6 +13730,12 @@ S:	Maintained
>  F:	include/linux/personality.h
>  F:	include/uapi/linux/personality.h
>  
> +PF8x00 PMIC REGULATORS DRIVERS
> +M:	Adrien Grassein <adrien.grassein@gmail.com>
> +L:	linux-kernel@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
> +
>  PHOENIX RC FLIGHT CONTROLLER ADAPTER
>  M:	Marcus Folkesson <marcus.folkesson@gmail.com>
>  L:	linux-input@vger.kernel.org
> -- 
> 2.20.1
> 

[PATCH v2 1/2] dt-bindings: regulator: add pf8x00 PMIC

[Adrien Grassein]

Add a devicetree binding documentation for the pf8x00 regulator driver.

Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com>
---
 .../regulator/nxp,pf8x00-regulator.yaml       | 232 ++++++++++++++++++
 MAINTAINERS                                   |   6 +
 2 files changed, 238 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml

diff --git a/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
new file mode 100644
index 000000000000..98cb0d281bf1
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
@@ -0,0 +1,232 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/nxp,pf8x00-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP PF8x00 Power Management Integrated Circuit regulators
+
+maintainers:
+  - Adrien Grassein <adrien.grassein@gmail.com>
+
+description:
+  pf8x00 are a 12-chanel regulator PMIC family. Regulators nodes should
+  be named to ldo<>, sw<> and vnss. The definition for each of these nodes
+  is defined using the standard binding for regulators at
+  Documentation/devicetree/bindings/regulator/regulator.txt.
+  Datasheet is available at https://www.nxp.com/docs/en/data-sheet/PF8100_PF8200.pdf
+
+properties:
+  compatible:
+    enum:
+      - nxp,pf8100
+      - nxp,pf8121a
+      - nxp,pf8200
+
+  reg:
+    maxItems: 1
+
+  regulators:
+    type: object
+    description:
+      list of regulators provided by this controller
+
+    patternProperties:
+      "^ldo[1-4]$":
+        type: object
+        $ref: regulator.yaml#
+        description:
+          Properties for single LDO regulator.
+
+        properties:
+          regulator-name:
+            pattern: "^ldo[1-4]$"
+            description:
+              should be ldo1", ..., "ldo4"
+
+          nxp,hw-en:
+            $ref: /schemas/types.yaml#definitions/flag
+            description:
+              Only available for ldo2. If present, use LDO2EN input pin
+              to enable or disable ldo2. (LDO2EN should be in HIGH state
+              to enable LDO2 if nxp,hw-en is specified).
+
+          nxp,vselect-en:
+            $ref: /schemas/types.yaml#definitions/flag
+            description:
+              Only available for ldo2. When specified, use the VSELECT
+              input pin of the chip to control the output voltage of the
+              ldo02 regulator. (3.3V if VSELECT is LOW, 1.8V if HIGH).
+
+        unevaluatedProperties: false
+
+      "^sw[1-7]$":
+        type: object
+        $ref: regulator.yaml#
+        description:
+          Properties for single SW regulator.
+
+        properties:
+          regulator-name:
+            pattern: "^sw[1-7]$"
+            description:
+              should be sw1", ..., "sw7"
+
+          nxp,ilim-microamp:
+            $ref: /schemas/types.yaml#definitions/uint32
+            minimum: 2100
+            maximum: 4500
+            default: 2100
+            enum: [ 2100, 2600, 3000, 4500 ]
+            description:
+              Defines the maximum current delivered by the regulator in microamp.
+
+          nxp,phase:
+            $ref: /schemas/types.yaml#definitions/uint32
+            minimum: 0
+            maximum: 315
+            default: 0
+            enum: [ 0, 45, 90, 135, 180, 225, 270, 315 ]
+            description:
+               This controls the phase shift of the switching frequency.
+
+          nxp,fast-slew:
+            $ref: /schemas/types.yaml#definitions/flag
+            description:
+              Enables the fast DVS ramp of the regulator.
+              Default is slow DVS ramp.
+
+          nxp,dual-phase:
+            $ref: /schemas/types.yaml#definitions/flag
+            description:
+              This allow regulators  sw1 and sw2, to work together to
+              deliver a maximum 5A current. Should be only specified
+              for sw1. If set, sw2 configuration will be totally ignored.
+
+          nxp,quad-phase:
+            $ref: /schemas/types.yaml#definitions/flag
+            description:
+              This allow regulators  sw1, sw2, sw3 and sw4 to work together
+              to deliver a maximum 10A current. Should be only specified for
+              sw1. If set, sw2, sw3 and sw4 configuration will be totally ignored.
+
+        unevaluatedProperties: false
+
+    properties:
+      vsnvs:
+        type: object
+        $ref: regulator.yaml#
+        description:
+          Properties for vsnvs regulator.
+
+        unevaluatedProperties: false
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - regulators
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      pmic@8 {
+        compatible = "nxp,pf8100";
+        reg = <0x08>;
+
+        regulators {
+            reg_ldo1: ldo1 {
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <5000000>;
+              regulator-min-microvolt = <1500000>;
+            };
+
+            reg_ldo2: ldo2 {
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <5000000>;
+              regulator-min-microvolt = <1500000>;
+            };
+
+            reg_ldo3: ldo3 {
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <5000000>;
+              regulator-min-microvolt = <1500000>;
+            };
+
+            reg_ldo4: ldo4 {
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <5000000>;
+              regulator-min-microvolt = <1500000>;
+            };
+
+            reg_sw1: sw1 {
+              nxp,phase = <0>;
+              nxp,ilim-microamp = <4500>;
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <1800000>;
+              regulator-min-microvolt =  <400000>;
+            };
+
+            reg_sw2: sw2 {
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <1800000>;
+              regulator-min-microvolt =  <400000>;
+            };
+
+            reg_sw3: sw3 {
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <1800000>;
+              regulator-min-microvolt =  <400000>;
+              nxp,fast-slew;
+            };
+
+            reg_sw4: sw4 {
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <1800000>;
+              regulator-min-microvolt =  <400000>;
+              nxp,fast-slew;
+            };
+
+            reg_sw5: sw5 {
+              regulator-always-on;
+              regulator-boot-on;
+              regulator-max-microvolt = <1800000>;
+              regulator-min-microvolt =  <400000>;
+            };
+
+           reg_sw6: sw6 {
+             regulator-always-on;
+             regulator-boot-on;
+             regulator-max-microvolt = <1800000>;
+             regulator-min-microvolt =  <400000>;
+           };
+
+           reg_sw7: sw7 {
+             regulator-always-on;
+             regulator-boot-on;
+             regulator-max-microvolt = <4100000>;
+             regulator-min-microvolt = <1000000>;
+           };
+
+          reg_vsnvs: vsnvs {
+            regulator-always-on;
+            regulator-boot-on;
+            regulator-max-microvolt = <3300000>;
+            regulator-min-microvolt = <1800000>;
+          };
+        };
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 52086876ce40..71b4476a7619 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13737,6 +13737,12 @@ S:	Maintained
 F:	include/linux/personality.h
 F:	include/uapi/linux/personality.h
 
+PF8x00 PMIC REGULATORS DRIVERS
+M:	Adrien Grassein <adrien.grassein@gmail.com>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
+
 PHOENIX RC FLIGHT CONTROLLER ADAPTER
 M:	Marcus Folkesson <marcus.folkesson@gmail.com>
 L:	linux-input@vger.kernel.org
-- 
2.20.1

[PATCH v2 2/2] regulator: pf8x00: add support of nxp pf8x00 regulator

[Adrien Grassein]

NXP pf8100 and pf8200 are 12-channel
PMIC for high performance applications.

This driver introduces the support of the 12
regulators available on the PMIC.

Imported from Boundary Devices kernel
with some modifications.

This driver was teste using a Boundary Nitrogen 8M Mini
board that features this PMIC.

Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com>
---
 MAINTAINERS                          |   1 +
 drivers/regulator/Kconfig            |  12 +
 drivers/regulator/Makefile           |   1 +
 drivers/regulator/pf8x00-regulator.c | 844 +++++++++++++++++++++++++++
 4 files changed, 858 insertions(+)
 create mode 100644 drivers/regulator/pf8x00-regulator.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 71b4476a7619..46401dbfe991 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13742,6 +13742,7 @@ M:	Adrien Grassein <adrien.grassein@gmail.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
+F:	drivers/regulator/pf8x00-regulator.c
 
 PHOENIX RC FLIGHT CONTROLLER ADAPTER
 M:	Marcus Folkesson <marcus.folkesson@gmail.com>
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 020a00d6696b..d2aba4b487f4 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -791,6 +791,18 @@ config REGULATOR_PCF50633
 	 Say Y here to support the voltage regulators and converters
 	 on PCF50633
 
+config REGULATOR_PF8X00
+	tristate "NXP PF8x00 regulator driver"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	 Say y here to support the regulators found on the NXP
+	 PF8100/PF8200 PMIC
+
+	 Say M here if you want to include support for tte regulators found
+	 on the NXP PF8100/PF8200 PMIC. The module will be named
+	 "pf8x00-regulator".
+
 config REGULATOR_PFUZE100
 	tristate "Freescale PFUZE100/200/3000/3001 regulator driver"
 	depends on I2C
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 6ebae516258e..a9fb449a0dc8 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -100,6 +100,7 @@ obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_USB_VBUS) += qcom_usb_vbus-regulator.o
 obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o
 obj-$(CONFIG_REGULATOR_PCA9450) += pca9450-regulator.o
+obj-$(CONFIG_REGULATOR_PF8X00) += pf8x00-regulator.o
 obj-$(CONFIG_REGULATOR_PFUZE100) += pfuze100-regulator.o
 obj-$(CONFIG_REGULATOR_PV88060) += pv88060-regulator.o
 obj-$(CONFIG_REGULATOR_PV88080) += pv88080-regulator.o
diff --git a/drivers/regulator/pf8x00-regulator.c b/drivers/regulator/pf8x00-regulator.c
new file mode 100644
index 000000000000..f9e7caa90051
--- /dev/null
+++ b/drivers/regulator/pf8x00-regulator.c
@@ -0,0 +1,844 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
+// Copyright 2017 NXP
+// Copyright 2019 Boundary Devices
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/regmap.h>
+
+#define PF8X00_DEVICEID		0x00
+#define PF8X00_REVID		0x01
+#define PF8X00_EMREV		0x02
+#define PF8X00_PROGID		0x03
+
+#define PF8X00_IMS_INT		0x04
+#define PF8X00_IMS_THERM	0x07
+#define PF8X00_SW_MODE_INT	0x0a
+#define PF8X00_SW_MODE_MASK	0x0b
+
+#define IMS_INT			0
+#define IMS_STATUS		0
+#define IMS_MASK		1
+#define IMS_SENSE		2
+
+#define PF8X00_IMS_SW_ILIM	0x12
+#define PF8X00_IMS_LDO_ILIM	0x15
+#define PF8X00_IMS_SW_UV	0x18
+#define PF8X00_IMS_SW_OV	0x1b
+#define PF8X00_IMS_LDO_UV	0x1e
+#define PF8X00_IMS_LDO_OV	0x21
+#define PF8X00_IMS_PWRON	0x24
+#define PF8X00_SYS_INT		0x27
+
+#define PF8X00_HARD_FAULT	0x29
+#define PF8X00_FSOB_FLAGS	0x2a
+#define PF8X00_FSOB_SELECT	0x2b
+#define PF8X00_ABIST_OV1	0x2c
+#define PF8X00_ABIST_OV2	0x2d
+#define PF8X00_ABIST_UV1	0x2e
+#define PF8X00_ABIST_UV2	0x2f
+#define PF8X00_TEST_FLAGS	0x30
+#define PF8X00_ABIST_RUN	0x31
+
+#define PF8X00_RANDOM_GEN	0x33
+#define PF8X00_RANDOM_CHK	0x34
+#define PF8X00_VMONEN1		0x35
+#define PF8X00_VMONEN2		0x36
+#define PF8X00_CTRL1		0x37
+#define PF8X00_CTRL2		0x38
+#define PF8X00_CTRL3		0x39
+#define PF8X00_PWRUP_CTRL	0x3a
+
+#define PF8X00_RESETBMCU	0x3c
+#define PF8X00_PGOOD		0x3d
+#define PF8X00_PWRDN_DLY1	0x3e
+#define PF8X00_PWRDN_DLY2	0x3f
+#define PF8X00_FREQ_CTRL	0x40
+#define PF8X00_COINCELL_CTRL	0x41
+#define PF8X00_PWRON		0x42
+#define PF8X00_WD_CONFIG	0x43
+#define PF8X00_WD_CLEAR		0x44
+#define PF8X00_WD_EXPIRE	0x45
+#define PF8X00_WD_COUNTER	0x46
+#define PF8X00_FAULT_COUNTER	0x47
+#define PF8X00_FSAFE_COUNTER	0x48
+#define PF8X00_FAULT_TIMER	0x49
+#define PF8X00_AMUX		0x4a
+
+#define SW_CONFIG1	0
+#define SW_CONFIG2	1
+#define SW_PWRUP	2
+#define SW_MODE1	3
+#define SW_RUN_VOLT	4
+#define SW_STBY_VOLT	5
+
+/* i is in REG_SW1..REG_SW7 */
+#define PF8X00_SW(i)		(8 * (i - REG_SW1) + 0x4d)
+
+#define LDO_CONFIG1	0
+#define LDO_CONFIG2	1
+#define LDO_PWRUP	2
+#define LDO_RUN_VOLT	3
+#define LDO_STBY_VOLT	4
+
+/* i is in REG_LDO1..REG_LDO4 */
+#define PF8X00_LDO(i)		(6 * (i - REG_LDO1) + 0x85)
+
+#define PF8X00_VSNVS_CONFIG1	0x9d
+
+#define PF8X00_PAGE_SELECT	0x9f
+#define PF8X00_OTP_FSOB_SELECT	0xa0
+#define PF8X00_OTP_I2C		0xa1
+#define PF8X00_OTP_CTRL1	0xa2
+#define PF8X00_OTP_CTRL2	0xa3
+#define PF8X00_OTP_CTRL3	0xa4
+
+#define PF8X00_OTP_FREQ_CTRL	0xa5
+#define PF8X00_OTP_COINCELL	0xa6
+#define PF8X00_OTP_PWRON	0xa7
+#define PF8X00_OTP_WD_CONFIG	0xa8
+#define PF8X00_OTP_WD_EXPIRE	0xa9
+#define PF8X00_OTP_WD_COUNTER	0xaa
+#define PF8X00_OTP_FAULT_COUNTER 0xab
+#define PF8X00_OTP_FAULT_TIMER	0xac
+#define PF8X00_OTP_PWRDN_DLY1	0xad
+#define PF8X00_OTP_PWRDN_DLY2	0xae
+#define PF8X00_OTP_PWRUP_CTRL	0xaf
+#define PF8X00_OTP_RESETBMCU	0xb0
+#define PF8X00_OTP_PGOOD	0xb1
+
+#define OTP_SW_VOLT	0
+#define OTP_SW_PWRUP	1
+#define OTP_SW_CONFIG1	2
+#define OTP_SW_CONFIG2	3
+
+/* i is in REG_SW1..REG_SW7 */
+#define PF8X00_OTP_SW(i)	(4 * (i - REG_SW1) + 0xb2)
+
+#define OTP_LDO_VOLT	0
+#define OTP_LDO_PWRUP	1
+#define OTP_LDO_CONFIG	2
+
+/* i is in REG_LDO1..REG_LDO4 */
+#define PF8X00_OTP_LDO(i)	(3 * (i - REG_LDO1) + 0xce)
+
+#define PF8X00_OTP_VSNVS_CONFIG	0xda
+#define PF8X00_OTP_OV_BYPASS1	0xdb
+#define PF8X00_OTP_OV_BYPASS2	0xdc
+#define PF8X00_OTP_UV_BYPASS1	0xdd
+#define PF8X00_OTP_UV_BYPASS2	0xde
+#define PF8X00_OTP_ILIM_BYPASS1	0xdf
+#define PF8X00_OTP_ILIM_BYPASS2	0xe0
+
+#define PF8X00_OTP_DEBUG1	0xe3
+#define PF8X_NUMREGS		0xe4
+
+#define REG_LDO1		0
+#define REG_LDO2		1
+#define REG_LDO3		2
+#define REG_LDO4		3
+#define REG_SW1			4
+#define REG_SW2			5
+#define REG_SW3			6
+#define REG_SW4			7
+#define REG_SW5			8
+#define REG_SW6			9
+#define REG_SW7			10
+#define REG_VSNVS		11
+#define REG_NUM_REGULATORS	(4 + 7 + 1)
+
+enum chips {
+	PF8100 = 0x40,
+	PF8121A = 0x42,
+	PF8200 = 0x48,
+};
+
+struct id_name {
+	enum chips id;
+	const char *name;
+};
+
+struct pf8x_regulator {
+	struct regulator_desc desc;
+	unsigned char stby_reg;
+	unsigned char stby_mask;
+	int ilim;
+	int phase;
+	unsigned char hw_en;
+	unsigned char vselect_en;
+	unsigned char quad_phase;
+	unsigned char dual_phase;
+	unsigned char fast_slew;
+};
+
+struct pf8x_chip {
+	int	chip_id;
+	int	prog_id;
+	int	clk_freq;
+	struct regmap *regmap;
+	struct device *dev;
+	struct pf8x_regulator regulator_descs[REG_NUM_REGULATORS];
+	struct regulator_dev *regulators[REG_NUM_REGULATORS];
+};
+
+/* Output: 1.5V to 5.0V, LDO2 can use VSELECT */
+static const int pf8x00_ldo_voltages[] = {
+	1500000, 1600000, 1800000, 1850000, 2150000, 2500000, 2800000, 3000000,
+	3100000, 3150000, 3200000, 3300000, 3350000, 4000000, 4900000, 5000000,
+};
+
+/* Output: 0.4V to 1.8V */
+#define PF8XOO_SW1_6_VOLTAGE_NUM 0xB2
+static const struct linear_range pf8x00_sw1_to_6_voltages[] = {
+	REGULATOR_LINEAR_RANGE(400000, 0x00, 0xB0, 6250),
+	REGULATOR_LINEAR_RANGE(1800000, 0xB1, 0xB1, 0),
+};
+
+/* Output: 1.0V to 4.1V */
+static const int pf8x00_sw7_voltages[] = {
+	1000000, 1100000, 1200000, 1250000, 1300000, 1350000, 1500000, 1600000,
+	1800000, 1850000, 2000000, 2100000, 2150000, 2250000, 2300000, 2400000,
+	2500000, 2800000, 3150000, 3200000, 3250000, 3300000, 3350000, 3400000,
+	3500000, 3800000, 4000000, 4100000, 4100000, 4100000, 4100000, 4100000,
+};
+
+/* Output: 1.8V, 3.0V, or 3.3V */
+static const int pf8x00_vsnvs_voltages[] = {
+	0, 1800000, 3000000, 3300000,
+};
+
+static const struct i2c_device_id pf8x_device_id[] = {
+	{.name = "pf8x00",},
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, pf8x_device_id);
+
+static const struct of_device_id pf8x_dt_ids[] = {
+	{ .compatible = "nxp,pf8100",},
+	{ .compatible = "nxp,pf8121a",},
+	{ .compatible = "nxp,pf8200",},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pf8x_dt_ids);
+
+const struct id_name id_list[] = {
+	{PF8100, "PF8100"},
+	{PF8121A, "PF8121A"},
+	{PF8200, "PF8200"},
+	{0, "???"},
+};
+
+static const struct id_name *get_id_name(enum chips id)
+{
+	const struct id_name *p = id_list;
+
+	while (p->id) {
+		if (p->id == id)
+			break;
+		p++;
+	}
+	return p;
+}
+
+struct dvs_ramp {
+	unsigned short up_down_slow_fast[4];
+};
+
+/* Units uV/uS */
+struct dvs_ramp ramp_table[] = {
+/*			up_slow,	down_slow,	up_fast,	down_fast */
+	[0]  = {{ 7813,		5208,		15625,		10417 }},
+	[1]  = {{ 8203,		5469,		16406,		10938 }},
+	[2]  = {{ 8594,		5729,		17188,		11458 }},
+	[3]  = {{ 8984,		5990,		17969,		11979 }},
+	[4]  = {{ 9375,		6250,		18750,		12500 }},
+	[9]  = {{ 6250,		4167,		12500,		 8333 }},
+	[10] = {{ 6641,		4427,		13281,		 8854 }},
+	[11] = {{ 7031,		4688,		14063,		 9375 }},
+	[12] = {{ 7422,		4948,		14844,		 9896 }},
+};
+
+static int pf8x00_regulator_set_voltage_time_sel(struct regulator_dev *rdev,
+		unsigned int old_sel, unsigned int new_sel)
+{
+	struct pf8x_chip *pf = rdev_get_drvdata(rdev);
+	struct pf8x_regulator *rdesc = container_of(rdev->desc, struct pf8x_regulator, desc);
+	const unsigned int *volt_table = rdev->desc->volt_table;
+	int old_v = volt_table[old_sel];
+	int new_v = volt_table[new_sel];
+	int change = (new_v - old_v);
+	unsigned int index = 0;
+	unsigned int slew;
+
+	if (rdesc->fast_slew & 1)
+		index = 2;
+
+	if (change < 0)
+		index++;
+	slew = ramp_table[pf->clk_freq].up_down_slow_fast[index];
+
+	return DIV_ROUND_UP(abs(change), slew);
+}
+
+static short ilim_table[] = {
+	2100, 2600, 3000, /* 4500, */
+};
+
+static int encode_ilim(struct pf8x_chip *pf, int ilim)
+{
+	int i = 0;
+
+	if (ilim <= 0)
+		return -1;
+
+	while (i < ARRAY_SIZE(ilim_table)) {
+		if (ilim <= ilim_table[i])
+			break;
+		i++;
+	}
+
+	return i;
+}
+
+static int encode_phase(struct pf8x_chip *pf, int phase)
+{
+	int ph;
+
+	if (phase < 0)
+		return -1;
+
+	ph = phase / 45;
+	if ((ph * 45) != phase) {
+		dev_err(pf->dev, "ignoring, illegal phase %d\n", phase);
+		return -1;
+	}
+
+	return (ph >= 1) ? ph - 1 : 7;
+}
+
+static int pf8x00_of_parse_cb(struct device_node *np,
+		const struct regulator_desc *desc,
+		struct regulator_config *config)
+{
+	struct pf8x_chip *pf = config->driver_data;
+	struct pf8x_regulator *rdesc = &pf->regulator_descs[desc->id];
+	unsigned char hw_en = 0;
+	unsigned char vselect_en = 0;
+	unsigned char quad_phase = 0;
+	unsigned char dual_phase = 0;
+	unsigned char fast_slew = 0;
+	int ilim;
+	int phase;
+	int ret;
+
+	ret = of_property_read_u32(np, "nxp,ilim-microamp",
+			&ilim);
+	if (ret)
+		ilim = -1;
+	ret = of_property_read_u32(np, "nxp,phase",
+			&phase);
+	if (ret)
+		phase = -1;
+	ilim = encode_ilim(pf, ilim);
+	phase = encode_phase(pf, phase);
+
+	if (of_get_property(np, "nxp,fast-slew", NULL))
+		fast_slew = 1;
+	if (of_get_property(np, "nxp,hw-en", NULL))
+		hw_en = 1;
+	if (of_get_property(np, "nxp,quad-phase", NULL))
+		quad_phase = 1;
+	if (of_get_property(np, "nxp,dual-phase", NULL))
+		dual_phase = 1;
+	if (of_get_property(np, "nxp,vselect-en", NULL))
+		vselect_en = 1;
+
+	if ((desc->id != REG_SW1) && quad_phase) {
+		dev_err(pf->dev, "ignoring, only sw1 can use quad-phase\n");
+		quad_phase = 0;
+	}
+	if ((desc->id != REG_SW1) && dual_phase) {
+		dev_err(pf->dev, "ignoring, only sw1 can use dual-phase\n");
+		dual_phase = 0;
+	}
+	if ((desc->id != REG_LDO2) && vselect_en) {
+		/* LDO2 has gpio vselect */
+		dev_err(pf->dev, "ignoring, only ldo2 can use vselect-en\n");
+		vselect_en = 0;
+	}
+	if ((desc->id != REG_LDO2) && hw_en) {
+		/* LDO2 has gpio vselect */
+		dev_err(pf->dev, "ignoring, only ldo2 can use hw-en\n");
+		hw_en = 0;
+	}
+	if ((desc->id < REG_SW1) && (desc->id > REG_SW7)) {
+		if ((unsigned int)ilim <= 3) {
+			dev_err(pf->dev, "ignoring, only sw1-7 can use ilim\n");
+			ilim = -1;
+		}
+		if ((unsigned int)phase <= 7) {
+			dev_err(pf->dev, "ignoring, only sw1-7 can use phase\n");
+			ilim = -1;
+		}
+	}
+	rdesc->ilim = ilim;
+	rdesc->phase = phase;
+	rdesc->hw_en = hw_en;
+	rdesc->vselect_en = vselect_en;
+	rdesc->quad_phase = quad_phase;
+	rdesc->dual_phase = dual_phase;
+	rdesc->fast_slew = fast_slew;
+
+	return 0;
+}
+
+static const struct regulator_ops pf8x00_ldo_regulator_ops = {
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = regulator_list_voltage_table,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+};
+
+static const struct regulator_ops pf8x00_sw1_6_regulator_ops = {
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = regulator_list_voltage_linear_range,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_time_sel = pf8x00_regulator_set_voltage_time_sel,
+};
+
+static const struct regulator_ops pf8x00_sw7_regulator_ops = {
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = regulator_list_voltage_table,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_time_sel = pf8x00_regulator_set_voltage_time_sel,
+};
+
+static const struct regulator_ops pf8x00_vsnvs_regulator_ops = {
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = regulator_list_voltage_table,
+	.map_voltage = regulator_map_voltage_ascend,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+};
+
+#define STRUCT_LDO_REG(_id, _name, base)		\
+	[_id] = {						\
+		.desc = {					\
+			.name = #_name,				\
+			.of_match = of_match_ptr(#_name),	\
+			.regulators_node = of_match_ptr("regulators"), \
+			.of_parse_cb = pf8x00_of_parse_cb,	\
+			.n_voltages = ARRAY_SIZE(pf8x00_ldo_voltages),	\
+			.ops = &pf8x00_ldo_regulator_ops,	\
+			.type = REGULATOR_VOLTAGE,		\
+			.id = _id,				\
+			.owner = THIS_MODULE,			\
+			.volt_table = pf8x00_ldo_voltages,			\
+			.vsel_reg = (base) + LDO_RUN_VOLT,	\
+			.vsel_mask = 0xff,			\
+			.enable_reg = (base) + LDO_CONFIG2,	\
+			.enable_val = 0x2,			\
+			.disable_val = 0x0,			\
+			.enable_mask = 2,			\
+		},						\
+		.stby_reg = (base) + LDO_STBY_VOLT,		\
+		.stby_mask = 0x20,				\
+	}
+
+#define STRUCT_SW_REG(_id, _name, base)		\
+	[_id] = {						\
+		.desc = {					\
+			.name = #_name,				\
+			.of_match = of_match_ptr(#_name),	\
+			.regulators_node = of_match_ptr("regulators"), \
+			.of_parse_cb = pf8x00_of_parse_cb,	\
+			.n_voltages = PF8XOO_SW1_6_VOLTAGE_NUM, \
+			.ops = &pf8x00_sw1_6_regulator_ops,	\
+			.type = REGULATOR_VOLTAGE,		\
+			.id = _id,				\
+			.owner = THIS_MODULE,			\
+			.linear_ranges = pf8x00_sw1_to_6_voltages, \
+			.n_linear_ranges = \
+				ARRAY_SIZE(pf8x00_sw1_to_6_voltages), \
+			.vsel_reg = (base) + SW_RUN_VOLT,	\
+			.vsel_mask = 0xff,			\
+			.enable_reg = (base) + SW_MODE1,	\
+			.enable_val = 0x3,			\
+			.disable_val = 0x0,			\
+			.enable_mask = 0x3,			\
+			.enable_time = 500,			\
+		},						\
+		.stby_reg = (base) + SW_STBY_VOLT,		\
+		.stby_mask = 0xff,				\
+	}
+
+#define STRUCT_SW7_REG(_id, _name, base)		\
+	[_id] = {						\
+		.desc = {					\
+			.name = #_name,				\
+			.of_match = of_match_ptr(#_name),	\
+			.regulators_node = of_match_ptr("regulators"), \
+			.of_parse_cb = pf8x00_of_parse_cb,	\
+			.n_voltages = ARRAY_SIZE(pf8x00_sw7_voltages),	\
+			.ops = &pf8x00_sw7_regulator_ops,	\
+			.type = REGULATOR_VOLTAGE,		\
+			.id = _id,				\
+			.owner = THIS_MODULE,			\
+			.volt_table = pf8x00_sw7_voltages,			\
+			.vsel_reg = (base) + SW_RUN_VOLT,	\
+			.vsel_mask = 0xff,			\
+			.enable_reg = (base) + SW_MODE1,	\
+			.enable_val = 0x3,			\
+			.disable_val = 0x0,			\
+			.enable_mask = 0x3,			\
+			.enable_time = 500,			\
+		},						\
+		.stby_reg = (base) + SW_STBY_VOLT,		\
+		.stby_mask = 0xff,				\
+	}
+
+
+#define STRUCT_VSNVS_REG(_id, _name, base)		\
+	[_id] = {						\
+		.desc = {					\
+			.name = #_name,				\
+			.of_match = of_match_ptr(#_name),	\
+			.regulators_node = of_match_ptr("regulators"), \
+			.of_parse_cb = pf8x00_of_parse_cb,	\
+			.n_voltages = ARRAY_SIZE(pf8x00_vsnvs_voltages),	\
+			.ops = &pf8x00_vsnvs_regulator_ops,	\
+			.type = REGULATOR_VOLTAGE,		\
+			.id = _id,				\
+			.owner = THIS_MODULE,			\
+			.volt_table = pf8x00_vsnvs_voltages,			\
+			.vsel_reg = (base),			\
+			.vsel_mask = 0x3,			\
+		},						\
+	}
+
+static const struct pf8x_regulator pf8x00_regulators[] = {
+	STRUCT_LDO_REG(REG_LDO1, ldo1, PF8X00_LDO(REG_LDO1)),
+	STRUCT_LDO_REG(REG_LDO2, ldo2, PF8X00_LDO(REG_LDO2)),
+	STRUCT_LDO_REG(REG_LDO3, ldo3, PF8X00_LDO(REG_LDO3)),
+	STRUCT_LDO_REG(REG_LDO4, ldo4, PF8X00_LDO(REG_LDO4)),
+	STRUCT_SW_REG(REG_SW1, sw1, PF8X00_SW(REG_SW1)),
+	STRUCT_SW_REG(REG_SW2, sw2, PF8X00_SW(REG_SW2)),
+	STRUCT_SW_REG(REG_SW3, sw3, PF8X00_SW(REG_SW3)),
+	STRUCT_SW_REG(REG_SW4, sw4, PF8X00_SW(REG_SW4)),
+	STRUCT_SW_REG(REG_SW5, sw5, PF8X00_SW(REG_SW5)),
+	STRUCT_SW_REG(REG_SW6, sw6, PF8X00_SW(REG_SW6)),
+	STRUCT_SW7_REG(REG_SW7, sw7, PF8X00_SW(REG_SW7)),
+	STRUCT_VSNVS_REG(REG_VSNVS, vsnvs, PF8X00_VSNVS_CONFIG1),
+};
+
+#ifdef CONFIG_OF
+static struct of_regulator_match pf8x00_matches[] = {
+	{ .name = "ldo1",	},
+	{ .name = "ldo2",	},
+	{ .name = "ldo3",	},
+	{ .name = "ldo4",	},
+	{ .name = "sw1",	},
+	{ .name = "sw2",	},
+	{ .name = "sw3",	},
+	{ .name = "sw4",	},
+	{ .name = "sw5",	},
+	{ .name = "sw6",	},
+	{ .name = "sw7",	},
+	{ .name = "vsnvs",	},
+};
+
+static inline struct regulator_init_data *match_init_data(int index)
+{
+	return pf8x00_matches[index].init_data;
+}
+
+static inline struct device_node *match_of_node(int index)
+{
+	return pf8x00_matches[index].of_node;
+}
+#else
+static int pf8x_parse_regulators_dt(struct pf8x_chip *pf)
+{
+	return 0;
+}
+
+static inline struct regulator_init_data *match_init_data(int index)
+{
+	return NULL;
+}
+
+static inline struct device_node *match_of_node(int index)
+{
+	return NULL;
+}
+#endif
+
+static int pf8x_identify(struct pf8x_chip *pf)
+{
+	const struct id_name *p;
+	unsigned int value, id1, id2;
+	int ret;
+
+	ret = regmap_read(pf->regmap, PF8X00_DEVICEID, &value);
+	if (ret)
+		return ret;
+
+	pf->chip_id = value;
+	p = get_id_name(value);
+	if (p->id != value) {
+		dev_warn(pf->dev, "Illegal ID: %x\n", value);
+		return -ENODEV;
+	}
+
+	ret = regmap_read(pf->regmap, PF8X00_REVID, &value);
+	if (ret)
+		value = 0;
+	ret = regmap_read(pf->regmap, PF8X00_EMREV, &id1);
+	if (ret)
+		id1 = 0;
+	ret = regmap_read(pf->regmap, PF8X00_PROGID, &id2);
+	if (ret)
+		id2 = 0;
+	pf->prog_id = (id1 << 8) | id2;
+
+	dev_info(pf->dev, "%s: Full layer: %x, Metal layer: %x, prog_id=0x%04x\n",
+		 p->name, (value & 0xf0) >> 4, value & 0x0f, pf->prog_id);
+
+	return 0;
+}
+
+static const struct regmap_config pf8x_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = PF8X_NUMREGS - 1,
+	.cache_type = REGCACHE_RBTREE,
+};
+
+struct otp_reg_lookup {
+	unsigned short prog_id;
+	unsigned char reg;
+	unsigned char value;
+};
+
+static const struct otp_reg_lookup otp_map[] = {
+	{ 0x401c, PF8X00_OTP_CTRL3, 0 },
+	{ 0x4008, PF8X00_OTP_CTRL3, 0x04 },
+	{ 0x301d, PF8X00_OTP_CTRL3, 0x04 },	/* test only */
+	{ 0, 0, 0 },
+};
+
+static int get_otp_reg(struct pf8x_chip *pf, unsigned char reg)
+{
+	const struct otp_reg_lookup *p = otp_map;
+
+	while (p->reg) {
+		if ((pf->prog_id == p->prog_id) && (reg == p->reg))
+			return p->value;
+		p++;
+	}
+
+	dev_err(pf->dev, "reg(0x%02x) not found for 0x%04x\n",
+		 reg, pf->prog_id);
+	return -EINVAL;
+}
+
+static int pf8x00_regulator_probe(struct i2c_client *client,
+				    const struct i2c_device_id *id)
+{
+	struct pf8x_chip *pf;
+	struct regulator_config config = { };
+	int i, ret;
+	u32 num_regulators;
+	unsigned int hw_en;
+	unsigned int vselect_en;
+	unsigned char quad_phase;
+	unsigned char dual_phase;
+	unsigned int val;
+	int ctrl3;
+	const char *format = NULL;
+	unsigned int clk_freq = 0;
+
+	pf = devm_kzalloc(&client->dev, sizeof(*pf),
+			GFP_KERNEL);
+	if (!pf)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, pf);
+	pf->dev = &client->dev;
+
+	pf->regmap = devm_regmap_init_i2c(client, &pf8x_regmap_config);
+	if (IS_ERR(pf->regmap)) {
+		ret = PTR_ERR(pf->regmap);
+		dev_err(&client->dev,
+			"regmap allocation failed with err %d\n", ret);
+		return ret;
+	}
+
+	ret = pf8x_identify(pf);
+	if (ret)
+		return ret;
+
+	dev_info(&client->dev, "%s found.\n",
+		get_id_name(pf->chip_id)->name);
+
+	ret = regmap_read(pf->regmap, PF8X00_FREQ_CTRL, &clk_freq);
+	clk_freq &= 0xf;
+	if (ret < 0)
+		clk_freq = 0;
+	if (((clk_freq & 7) > 4) || (clk_freq == 8))
+		clk_freq = 0;
+	pf->clk_freq = clk_freq;
+
+	memcpy(pf->regulator_descs, pf8x00_regulators,
+		sizeof(pf->regulator_descs));
+
+	num_regulators = ARRAY_SIZE(pf->regulator_descs);
+	for (i = 0; i < num_regulators; i++) {
+		struct regulator_init_data *init_data;
+		struct regulator_desc *desc;
+
+		desc = &pf->regulator_descs[i].desc;
+		init_data = match_init_data(i);
+
+		config.dev = &client->dev;
+		config.init_data = init_data;
+		config.driver_data = pf;
+		config.of_node = match_of_node(i);
+		config.ena_gpiod = NULL;
+
+		pf->regulators[i] =
+			devm_regulator_register(&client->dev, desc, &config);
+		if (IS_ERR(pf->regulators[i])) {
+			dev_err(&client->dev, "register regulator%s failed\n",
+				desc->name);
+			return PTR_ERR(pf->regulators[i]);
+		}
+		if ((i >= REG_SW1) && (i <= REG_SW7)) {
+			unsigned int phase = pf->regulator_descs[i].phase;
+			unsigned int ilim = pf->regulator_descs[i].ilim;
+			unsigned int mask = 0;
+			unsigned int val = 0;
+			unsigned int reg = PF8X00_SW(i) + SW_CONFIG2;
+			unsigned int fast_slew = pf->regulator_descs[i].fast_slew;
+
+			if (phase <= 7) {
+				mask |= 7;
+				val |= phase;
+			}
+
+			if (ilim <= 3) {
+				mask |= 3 << 3;
+				val |= ilim << 3;
+			}
+
+			if (fast_slew <= 1) {
+				mask |= 1 << 5;
+				val |= fast_slew << 5;
+			}
+
+			if (mask) {
+				ret = regmap_update_bits(pf->regmap, reg, mask,
+						val);
+			}
+
+			if (fast_slew > 1) {
+				ret = regmap_read(pf->regmap, reg, &fast_slew);
+				fast_slew &= 0x20;
+				if (ret < 0)
+					fast_slew = 0;
+				pf->regulator_descs[i].fast_slew = fast_slew >> 5;
+			}
+		}
+	}
+	hw_en = pf->regulator_descs[REG_LDO2].hw_en;
+	vselect_en = pf->regulator_descs[REG_LDO2].vselect_en;
+	val = 0;
+	if (vselect_en)
+		val |= 0x08;
+	if (hw_en)
+		val |= 0x10;
+	ret = regmap_update_bits(pf->regmap,
+			PF8X00_LDO(REG_LDO2) + LDO_CONFIG2,
+				 0x18, val);
+
+	ctrl3 = get_otp_reg(pf, PF8X00_OTP_CTRL3);
+	if (ctrl3 >= 0) {
+		quad_phase = pf->regulator_descs[REG_SW1].quad_phase;
+		dual_phase = pf->regulator_descs[REG_SW1].dual_phase;
+		if (quad_phase) {
+			if ((ctrl3 & 3) != 2)
+				format = "sw1 quad_phase not set in otp_ctrl3 %x\n";
+
+		} else if (dual_phase) {
+			if ((ctrl3 & 3) != 1)
+				format = "sw1 dual_phase not set in otp_ctrl3 %x\n";
+		} else if (ctrl3 & 3) {
+			format = "sw1 single_phase not set in otp_ctrl3 %x\n";
+		}
+		if (!quad_phase) {
+			dual_phase = pf->regulator_descs[REG_SW4].dual_phase;
+			if (dual_phase) {
+				if ((ctrl3 & 0x0c) != 4)
+					format = "sw4 dual_phase not set in otp_ctrl3 %x\n";
+			} else if (ctrl3 & 0x0c) {
+				format = "sw4 single_phase not set in otp_ctrl3 %x\n";
+			}
+		}
+		dual_phase = pf->regulator_descs[REG_SW5].dual_phase;
+		if (dual_phase) {
+			if ((ctrl3 & 0x30) != 0x10)
+				format = "sw5 dual_phase not set in otp_ctrl3 %x\n";
+		} else if (ctrl3 & 0x30) {
+			format = "sw5 single_phase not set in otp_ctrl3 %x\n";
+		}
+		if (format) {
+			dev_err(pf->dev, format, ctrl3);
+			dev_err(pf->dev, "!!!try updating u-boot, boot.scr, or pmic\n");
+		}
+	}
+	return 0;
+}
+
+static struct i2c_driver pf8x_driver = {
+	.id_table = pf8x_device_id,
+	.driver = {
+		.name = "pf8x00-regulator",
+		.of_match_table = pf8x_dt_ids,
+	},
+	.probe = pf8x00_regulator_probe,
+};
+
+module_i2c_driver(pf8x_driver);
+
+static void __exit pf8x_exit(void)
+{
+	i2c_del_driver(&pf8x_driver);
+}
+module_exit(pf8x_exit);
+
+MODULE_AUTHOR("Troy Kisky <troy.kisky@boundarydevices.com>");
+MODULE_AUTHOR("Adrien Grassein <adrien.grassein@gmail.com>");
+MODULE_DESCRIPTION("Regulator Driver for NXP's PF8100/PF8200 PMIC");
+MODULE_LICENSE("GPL v2");
-- 
2.20.1

Re: [PATCH v2 2/2] regulator: pf8x00: add support of nxp pf8x00 regulator

[Mark Brown]

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

On Thu, Dec 10, 2020 at 11:16:29PM +0100, Adrien Grassein wrote:

> +	{ .compatible = "nxp,pf8200",},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, pf8x_dt_ids);
> +
> +const struct id_name id_list[] = {
> +	{PF8100, "PF8100"},
> +	{PF8121A, "PF8121A"},
> +	{PF8200, "PF8200"},
> +	{0, "???"},
> +};

It's strange to see the list terminated with this ??? entry - these
things usually have a NULL for both the ID and the string.  The string
on the terminator is never used so may as well stick with the usual
pattern.

> +static int encode_phase(struct pf8x_chip *pf, int phase)
> +{
> +	int ph;
> +
> +	if (phase < 0)
> +		return -1;
> +
> +	ph = phase / 45;
> +	if ((ph * 45) != phase) {
> +		dev_err(pf->dev, "ignoring, illegal phase %d\n", phase);
> +		return -1;
> +	}
> +
> +	return (ph >= 1) ? ph - 1 : 7;

As I said on the previousl review please write normal condidional
statements to improve the legibility of the code.

> +static inline struct regulator_init_data *match_init_data(int index)
> +{
> +	return pf8x00_matches[index].init_data;
> +}
> +
> +static inline struct device_node *match_of_node(int index)
> +{
> +	return pf8x00_matches[index].of_node;
> +}

If you need these your driver has a problem, as previously advised
please use the standard DT parsing support.

> +	memcpy(pf->regulator_descs, pf8x00_regulators,
> +		sizeof(pf->regulator_descs));

Why do you need to take a copy of this?  The descriptors should not be
being modified.

> +	num_regulators = ARRAY_SIZE(pf->regulator_descs);
> +	for (i = 0; i < num_regulators; i++) {
> +		struct regulator_init_data *init_data;
> +		struct regulator_desc *desc;
> +
> +		desc = &pf->regulator_descs[i].desc;
> +		init_data = match_init_data(i);
> +
> +		config.dev = &client->dev;
> +		config.init_data = init_data;
> +		config.driver_data = pf;
> +		config.of_node = match_of_node(i);
> +		config.ena_gpiod = NULL;

You've not done anything to parse the init data (which is good) so the
init data handling is at best redundant.

> +		if ((i >= REG_SW1) && (i <= REG_SW7)) {

> +			if (mask) {
> +				ret = regmap_update_bits(pf->regmap, reg, mask,
> +						val);
> +			}
> +
> +			if (fast_slew > 1) {
> +				ret = regmap_read(pf->regmap, reg, &fast_slew);
> +				fast_slew &= 0x20;
> +				if (ret < 0)
> +					fast_slew = 0;
> +				pf->regulator_descs[i].fast_slew = fast_slew >> 5;
> +			}

May as well just write configurations out when you parse them, no need
to defer to here and it makes things simpler.

> +module_i2c_driver(pf8x_driver);
> +
> +static void __exit pf8x_exit(void)
> +{
> +	i2c_del_driver(&pf8x_driver);
> +}
> +module_exit(pf8x_exit);

Does this build cleanly?  module_i2c_driver() does both the add and
delete.

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

Re: [PATCH v2 1/2] dt-bindings: regulator: add pf8x00 PMIC

[Mark Brown]

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

On Thu, Dec 10, 2020 at 11:16:28PM +0100, Adrien Grassein wrote:
> Add a devicetree binding documentation for the pf8x00 regulator driver.

Please don't send new patches in reply to old threads, it makes it hard
to keep track of what is going on.

> +          regulator-name:
> +            pattern: "^ldo[1-4]$"
> +            description:
> +              should be ldo1", ..., "ldo4"

This is part of the generic regulator binding and since it's for board
specific usage it should not be constrained by the chip binding.

> +          nxp,vselect-en:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description:
> +              Only available for ldo2. When specified, use the VSELECT
> +              input pin of the chip to control the output voltage of the
> +              ldo02 regulator. (3.3V if VSELECT is LOW, 1.8V if HIGH).

How is VSELECT used without a binding specifying some mechanism for
control?

> +          nxp,ilim-microamp:
> +            $ref: /schemas/types.yaml#definitions/uint32
> +            minimum: 2100
> +            maximum: 4500
> +            default: 2100
> +            enum: [ 2100, 2600, 3000, 4500 ]
> +            description:
> +              Defines the maximum current delivered by the regulator in microamp.

Instead of implementing a custom property this should use the already
existing properties for current limits, this is a common thing for
hardware to have so we shouldn't have custom bindings.  We'll need to
relax the check the code currently has for a non-zero minimum limit but
otherwise everything should already be there.

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

Re: [PATCH v2 1/2] dt-bindings: regulator: add pf8x00 PMIC

[Adrien Grassein]

Hello,

Le ven. 11 déc. 2020 à 15:04, Mark Brown <broonie@kernel.org> a écrit :
>
> On Thu, Dec 10, 2020 at 11:16:28PM +0100, Adrien Grassein wrote:
> > Add a devicetree binding documentation for the pf8x00 regulator driver.
>
> Please don't send new patches in reply to old threads, it makes it hard
> to keep track of what is going on.

Sorry. Should I create a new mail each time I send a new version of the patch?

>
> > +          regulator-name:
> > +            pattern: "^ldo[1-4]$"
> > +            description:
> > +              should be ldo1", ..., "ldo4"
>
> This is part of the generic regulator binding and since it's for board
> specific usage it should not be constrained by the chip binding.

Ok.
>
> > +          nxp,vselect-en:
> > +            $ref: /schemas/types.yaml#definitions/flag
> > +            description:
> > +              Only available for ldo2. When specified, use the VSELECT
> > +              input pin of the chip to control the output voltage of the
> > +              ldo02 regulator. (3.3V if VSELECT is LOW, 1.8V if HIGH).
>
> How is VSELECT used without a binding specifying some mechanism for
> control?

I think that VSELECT input should be controlled by the sub system that
uses it (via maybe a GPIO).
On my board, it's directly controlled by another chip (so without a GPIO).

>
> > +          nxp,ilim-microamp:
> > +            $ref: /schemas/types.yaml#definitions/uint32
> > +            minimum: 2100
> > +            maximum: 4500
> > +            default: 2100
> > +            enum: [ 2100, 2600, 3000, 4500 ]
> > +            description:
> > +              Defines the maximum current delivered by the regulator in microamp.
>
> Instead of implementing a custom property this should use the already
> existing properties for current limits, this is a common thing for
> hardware to have so we shouldn't have custom bindings.  We'll need to
> relax the check the code currently has for a non-zero minimum limit but
> otherwise everything should already be there.

Ok I now use "regulator-max-microamp" property from the regulator that
acts like my property.
I was wrong with the default value. I re-read the documentation and
the default value is stored in OTP
memory. So if someone skipped this property, it's OK to not send any value.


Thanks again,
Regards,