All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/2] Add Richtek RTQ2208 SubPMIC support
@ 2023-07-26  7:13 Alina Yu
  2023-07-26  7:13 ` [PATCH v5 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC Alina Yu
  2023-07-26  7:13 ` [PATCH v5 2/2] regulator: rtq2208: Add Richtek RTQ2208 SubPMIC driver Alina Yu
  0 siblings, 2 replies; 10+ messages in thread
From: Alina Yu @ 2023-07-26  7:13 UTC (permalink / raw)
  To: lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt
  Cc: linux-kernel, devicetree, alina_yu

This patch series adds support for RTQ2208 SubPMIC regulators.               
The RTQ2208 is a multi-phase, programmable power management IC that    
integrate with dual multi-configurable, synchronous buck converters    
and two ldos. The bucks features wide output voltage range from 0.4V to 2.05V
and the capability to configure the corresponding power stages.

Thank you,
Alina yu
---
Change in v5:
- In Patch 1/2:
	- Revise filename from $id
	- Remove "regulator-compatible" for ldos
	- Add missing "high" for "richtek,mtp-sel-high"
	- Remove "regulator-mode" property
	- Add and modify "unevaluatedProperties: false" and "additionalProperties: false"
	- Remove "richtek,fixed-uV"
	- Remove redundant space in "buck-a" and "ldo2"
- In Patch 2/2:
	- Modify 'rdesc->fixed_uV' get, becasue "richtek,fixed-uV" is removed from yaml
	- Modify 'mtp_sel' get because read property is changed from "richtek,mtp-sel" to
	  "richtek,mtp-sel-high" in yaml
	- Add missing regulators_node points to regulator node in yaml
	- Include <linux/bitfield.h> for 'FIELD_PREP' reported by kernel test robot
Change in v4:
- In Patch 1/2:
	- Modify filename to "richtek,rtq2208"
	- Add more desciptions for "regulator-allowed-modes"
Change in v3:
- In Patch 1/2:
	- Fix some typos
	- Modify the descriptions for "richtek,mtp-sel"
	- Modify the node name to lowercase and remove underscore
	- Remove '|' from description
	- Remove "regulator-compatible" from property
	- Remove "regulator-state-mem" from pattern
	- Modify node name to generic one
---
Alina Yu (2):
  regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC
  regulator: rtq2208: Add Richtek RTQ2208 SubPMIC driver

 .../bindings/regulator/richtek,rtq2208.yaml        | 196 ++++++++
 drivers/regulator/Kconfig                          |  11 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/rtq2208-regulator.c              | 549 +++++++++++++++++++++
 4 files changed, 757 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
 create mode 100644 drivers/regulator/rtq2208-regulator.c

-- 
2.7.4


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

* [PATCH v5 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC
  2023-07-26  7:13 [PATCH v5 0/2] Add Richtek RTQ2208 SubPMIC support Alina Yu
@ 2023-07-26  7:13 ` Alina Yu
  2023-07-26  9:53   ` Krzysztof Kozlowski
  2023-08-03  6:36   ` ChiYuan Huang
  2023-07-26  7:13 ` [PATCH v5 2/2] regulator: rtq2208: Add Richtek RTQ2208 SubPMIC driver Alina Yu
  1 sibling, 2 replies; 10+ messages in thread
From: Alina Yu @ 2023-07-26  7:13 UTC (permalink / raw)
  To: lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt
  Cc: linux-kernel, devicetree, alina_yu

Add bindings for Richtek RTQ2208 IC controlled SubPMIC

Signed-off-by: Alina Yu <alina_yu@richtek.com>
---
v5
- Revise filename from $id
- Remove "regulator-compatible" for ldos
- Add missing "high" for "richtek,mtp-sel-high"
- Remove "regulator-mode" property
- Add and modify "unevaluatedProperties: false" and "additionalProperties: false"
- Remove "richtek,fixed-uV"
- Remove redundant space in "buck-a" and "ldo2"
v4
- Modify filename to "richtek,rtq2208"
- Add more desciptions for "regulator-allowed-modes"
---
 .../bindings/regulator/richtek,rtq2208.yaml        | 196 +++++++++++++++++++++
 1 file changed, 196 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml

diff --git a/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml b/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
new file mode 100644
index 0000000..63453f2
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
@@ -0,0 +1,196 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/richtek,rtq2208.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Richtek RTQ2208 SubPMIC Regulator
+
+maintainers:
+  - Alina Yu <alina_yu@richtek.com>
+
+description: |
+  RTQ2208 is a highly integrated power converter that offers functional safety dual
+  multi-configurable synchronous buck converters and two LDOs.
+
+  Bucks support "regulator-allowed-modes" and "regulator-mode". The former defines the permitted
+  switching operation in normal mode; the latter defines the operation in suspend to RAM mode.
+
+  No matter the RTQ2208 is configured to normal or suspend to RAM mode, there are two switching
+  operation modes for all buck rails, automatic power saving mode (Auto mode) and forced continuous
+  conduction mode (FCCM).
+
+  The definition of modes is in the datasheet which is available in below link
+  and their meaning is::
+    0 - Auto mode for power saving, which reducing the switching frequency at light load condition
+    to maintain high frequency.
+    1 - FCCM to meet the strict voltage regulation accuracy, which keeping constant switching frequency.
+
+  Datasheet will be available soon at
+  https://www.richtek.com/assets/Products
+
+properties:
+  compatible:
+    enum:
+      - richtek,rtq2208
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+    
+  richtek,mtp-sel-high:
+    type: boolean
+    description:
+      vout register selection based on this boolean value.
+      false - Using DVS0 register setting to adjust vout
+      true - Using DVS1 register setting to adjust vout
+
+  regulators:
+    type: object
+
+    patternProperties:
+      "^buck-[a-h]$":
+        type: object
+        $ref: regulator.yaml#
+        unevaluatedProperties: false
+        description:
+          description for buck-[a-h] regulator.
+
+        properties:
+          regulator-allowed-modes:
+            description:
+              two buck modes in different switching accuracy.
+              0 - Auto mode
+              1 - FCCM
+            items:
+              enum: [0, 1]
+
+      "^ldo[1-2]$":
+        type: object
+        $ref: regulator.yaml#
+        unevaluatedProperties: false
+        description:
+          regulator description for ldo[1-2].
+
+required:
+  - compatible
+  - reg
+  - regulators
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      pmic@10 {
+        compatible = "richtek,rtq2208";
+        reg = <0x10>;
+        interrupts-extended = <&gpio26 0 IRQ_TYPE_LEVEL_LOW>;
+        richtek,mtp-sel-high;
+
+        regulators {
+          buck-a {
+            regulator-min-microvolt = <400000>;
+            regulator-max-microvolt = <2050000>;
+            regulator-allowed-modes = <0 1>;
+            regulator-always-on;
+            regulator-state-mem {
+              regulator-on-in-suspend;
+              regulator-mode = <1>;
+            };
+          };
+          buck-b {
+            regulator-min-microvolt = <400000>;
+            regulator-max-microvolt = <2050000>;
+            regulator-allowed-modes = <0 1>;
+            regulator-always-on;
+            regulator-state-mem {
+              regulator-on-in-suspend;
+              regulator-mode = <1>;
+            };
+          };
+          buck-c {
+            regulator-min-microvolt = <400000>;
+            regulator-max-microvolt = <2050000>;
+            regulator-allowed-modes = <0 1>;
+            regulator-always-on;
+            regulator-state-mem {
+              regulator-on-in-suspend;
+              regulator-mode = <1>;
+            };
+          };
+          buck-d {
+            regulator-min-microvolt = <400000>;
+            regulator-max-microvolt = <2050000>;
+            regulator-allowed-modes = <0 1>;
+            regulator-always-on;
+            regulator-state-mem {
+              regulator-on-in-suspend;
+              regulator-mode = <1>;
+            };
+          };
+          buck-e {
+            regulator-min-microvolt = <400000>;
+            regulator-max-microvolt = <2050000>;
+            regulator-allowed-modes = <0 1>;
+            regulator-always-on;
+            regulator-state-mem {
+              regulator-on-in-suspend;
+              regulator-mode = <1>;
+            };
+          };
+          buck-f {
+            regulator-min-microvolt = <400000>;
+            regulator-max-microvolt = <2050000>;
+            regulator-allowed-modes = <0 1>;
+            regulator-always-on;
+            regulator-state-mem {
+              regulator-on-in-suspend;
+              regulator-mode = <1>;
+            };
+          };
+          buck-g {
+            regulator-min-microvolt = <400000>;
+            regulator-max-microvolt = <2050000>;
+            regulator-allowed-modes = <0 1>;
+            regulator-always-on;
+            regulator-state-mem {
+              regulator-on-in-suspend;
+              regulator-mode = <1>;
+            };
+          };
+          buck-h {
+            regulator-min-microvolt = <400000>;
+            regulator-max-microvolt = <2050000>;
+            regulator-allowed-modes = <0 1>;
+            regulator-always-on;
+            regulator-state-mem {
+              regulator-on-in-suspend;
+              regulator-mode = <1>;
+            };
+          };
+          ldo1 {
+            regulator-min-microvolt = <1200000>;
+            regulator-max-microvolt = <1200000>;
+            regulator-always-on;
+            regulator-state-mem {
+              regulator-on-in-suspend;
+            };
+          };
+          ldo2 {
+            regulator-min-microvolt = <3300000>;
+            regulator-max-microvolt = <3300000>;
+            regulator-always-on;
+            regulator-state-mem {
+              regulator-on-in-suspend;
+            };
+          };
+        };
+      };
+    };
-- 
2.7.4


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

* [PATCH v5 2/2] regulator: rtq2208: Add Richtek RTQ2208 SubPMIC driver
  2023-07-26  7:13 [PATCH v5 0/2] Add Richtek RTQ2208 SubPMIC support Alina Yu
  2023-07-26  7:13 ` [PATCH v5 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC Alina Yu
@ 2023-07-26  7:13 ` Alina Yu
  2023-08-03  7:24   ` ChiYuan Huang
  1 sibling, 1 reply; 10+ messages in thread
From: Alina Yu @ 2023-07-26  7:13 UTC (permalink / raw)
  To: lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt
  Cc: linux-kernel, devicetree, alina_yu

Add support for the RTQ2208 SubPMIC
This ic integrates with configurable, synchrnous buck converters and two
ldos.

Signed-off-by: Alina Yu <alina_yu@richtek.com>
Reported-by: kernel test robot <lkp@intel.com>
---
v5
- Modify 'rdesc->fixed_uV' get, becasue "richtek,fixed-uV" is removed from yaml
- Modify 'mtp_sel' get because read property is changed from "richtek,mtp-sel" to
  "richtek,mtp-sel-high" in yaml
- Add missing regulators_node points to regulator node in yaml
- Include <linux/bitfield.h> for 'FIELD_PREP' reported by kernel test robot
---
 drivers/regulator/Kconfig             |  11 +
 drivers/regulator/Makefile            |   1 +
 drivers/regulator/rtq2208-regulator.c | 549 ++++++++++++++++++++++++++++++++++
 3 files changed, 561 insertions(+)
 create mode 100644 drivers/regulator/rtq2208-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index e5f3613..a6b2c84 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1213,6 +1213,17 @@ config REGULATOR_RTQ6752
 	  synchronous boost converters for PAVDD, and one synchronous NAVDD
 	  buck-boost. This device is suitable for automotive TFT-LCD panel.
 
+config REGULATOR_RTQ2208
+	tristate "Richtek RTQ2208 SubPMIC Regulator"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  This driver adds support for RTQ2208 SubPMIC regulators.
+	  The RTQ2208 is a multi-phase, programmable power management IC that
+	  integrate with dual multi-configurable, synchronous buck converters
+	  and two ldos. It features wide output voltage range from 0.4V to 2.05V
+	  and the capability to configure the corresponding power stages.
+
 config REGULATOR_S2MPA01
 	tristate "Samsung S2MPA01 voltage regulator"
 	depends on MFD_SEC_CORE || COMPILE_TEST
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 58dfe01..04cbad17 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -143,6 +143,7 @@ obj-$(CONFIG_REGULATOR_RT6245)	+= rt6245-regulator.o
 obj-$(CONFIG_REGULATOR_RTMV20)	+= rtmv20-regulator.o
 obj-$(CONFIG_REGULATOR_RTQ2134) += rtq2134-regulator.o
 obj-$(CONFIG_REGULATOR_RTQ6752)	+= rtq6752-regulator.o
+obj-$(CONFIG_REGULATOR_RTQ2208) += rtq2208-regulator.o
 obj-$(CONFIG_REGULATOR_S2MPA01) += s2mpa01.o
 obj-$(CONFIG_REGULATOR_S2MPS11) += s2mps11.o
 obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
diff --git a/drivers/regulator/rtq2208-regulator.c b/drivers/regulator/rtq2208-regulator.c
new file mode 100644
index 0000000..2cfbc68
--- /dev/null
+++ b/drivers/regulator/rtq2208-regulator.c
@@ -0,0 +1,549 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <linux/bitops.h>
+#include <linux/bitfield.h>
+#include <linux/util_macros.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+
+/* Register */
+#define RTQ2208_REG_GLOBAL_INT1			0x12
+#define RTQ2208_REG_FLT_RECORDBUCK_CB		0x18
+#define RTQ2208_REG_GLOBAL_INT1_MASK		0x1D
+#define RTQ2208_REG_FLT_MASKBUCK_CB		0x1F
+#define RTQ2208_REG_BUCK_C_CFG0			0x32
+#define RTQ2208_REG_BUCK_B_CFG0			0x42
+#define RTQ2208_REG_BUCK_A_CFG0			0x52
+#define RTQ2208_REG_BUCK_D_CFG0			0x62
+#define RTQ2208_REG_BUCK_G_CFG0			0x72
+#define RTQ2208_REG_BUCK_F_CFG0			0x82
+#define RTQ2208_REG_BUCK_E_CFG0			0x92
+#define RTQ2208_REG_BUCK_H_CFG0			0xA2
+#define RTQ2208_REG_LDO1_CFG			0xB1
+#define RTQ2208_REG_LDO2_CFG			0xC1
+
+/* Mask */
+#define RTQ2208_BUCK_NR_MTP_SEL_MASK		GENMASK(7, 0)
+#define RTQ2208_BUCK_EN_NR_MTP_SEL0_MASK	BIT(0)
+#define RTQ2208_BUCK_EN_NR_MTP_SEL1_MASK	BIT(1)
+#define RTQ2208_BUCK_RSPUP_MASK			GENMASK(6, 4)
+#define RTQ2208_BUCK_RSPDN_MASK			GENMASK(2, 0)
+#define RTQ2208_BUCK_NRMODE_MASK		BIT(5)
+#define RTQ2208_BUCK_STRMODE_MASK		BIT(5)
+#define RTQ2208_BUCK_EN_STR_MASK		BIT(0)
+#define RTQ2208_LDO_EN_STR_MASK			BIT(7)
+#define RTQ2208_EN_DIS_MASK			BIT(0)
+#define RTQ2208_BUCK_RAMP_SEL_MASK		GENMASK(2, 0)
+#define RTQ2208_HD_INT_MASK			BIT(0)
+
+/* Size */
+#define RTQ2208_VOUT_MAXNUM			256
+#define RTQ2208_BUCK_NUM_IRQ_REGS		5
+#define RTQ2208_STS_NUM_IRQ_REGS		2
+
+/* Value */
+#define RTQ2208_RAMP_VALUE_MIN_uV		500
+#define RTQ2208_RAMP_VALUE_MAX_uV		64000
+
+#define RTQ2208_BUCK_MASK(uv_irq, ov_irq)	(1 << ((uv_irq) % 8) | 1 << ((ov_irq) % 8))
+
+enum {
+	RTQ2208_BUCK_B = 0,
+	RTQ2208_BUCK_C,
+	RTQ2208_BUCK_D,
+	RTQ2208_BUCK_A,
+	RTQ2208_BUCK_F,
+	RTQ2208_BUCK_G,
+	RTQ2208_BUCK_H,
+	RTQ2208_BUCK_E,
+	RTQ2208_LDO2,
+	RTQ2208_LDO1,
+	RTQ2208_LDO_MAX,
+};
+
+enum {
+	RTQ2208_AUTO_MODE = 0,
+	RTQ2208_FCCM,
+};
+
+struct rtq2208_regulator_desc {
+	struct regulator_desc desc;
+	unsigned int mtp_sel_reg;
+	unsigned int mtp_sel_mask;
+	unsigned int mode_reg;
+	unsigned int mode_mask;
+	unsigned int suspend_config_reg;
+	unsigned int suspend_enable_mask;
+	unsigned int suspend_mode_mask;
+	unsigned int fixed_uV;
+};
+
+struct rtq2208_rdev_map {
+	struct regulator_dev *rdev[RTQ2208_LDO_MAX];
+	struct regmap *regmap;
+	struct device *dev;
+};
+
+/* set Normal Auto/FCCM mode */
+static int rtq2208_set_mode(struct regulator_dev *rdev, unsigned int mode)
+{
+	struct rtq2208_regulator_desc *rdesc =
+		(struct rtq2208_regulator_desc *)rdev->desc;
+	unsigned int val, shift;
+
+	switch (mode) {
+	case REGULATOR_MODE_NORMAL:
+		val = RTQ2208_AUTO_MODE;
+		break;
+	case REGULATOR_MODE_FAST:
+		val = RTQ2208_FCCM;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	shift = ffs(rdesc->mode_mask) - 1;
+	return regmap_update_bits(rdev->regmap, rdesc->mode_reg,
+				  rdesc->mode_mask, val << shift);
+}
+
+static unsigned int rtq2208_get_mode(struct regulator_dev *rdev)
+{
+	struct rtq2208_regulator_desc *rdesc =
+		(struct rtq2208_regulator_desc *)rdev->desc;
+	unsigned int mode_val;
+	int ret;
+
+	ret = regmap_read(rdev->regmap, rdesc->mode_reg, &mode_val);
+	if (ret)
+		return REGULATOR_MODE_INVALID;
+
+	return (mode_val & rdesc->mode_mask) ? REGULATOR_MODE_FAST : REGULATOR_MODE_NORMAL;
+}
+
+static int rtq2208_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
+{
+	const struct regulator_desc *rdesc = rdev->desc;
+	unsigned int sel = 0, val;
+
+	ramp_delay = max(ramp_delay, RTQ2208_RAMP_VALUE_MIN_uV);
+	ramp_delay = min(ramp_delay, RTQ2208_RAMP_VALUE_MAX_uV);
+
+	ramp_delay /= RTQ2208_RAMP_VALUE_MIN_uV;
+
+	/*
+	 * fls(ramp_delay) - 1: doing LSB shift, let it starts from 0
+	 *
+	 * RTQ2208_BUCK_RAMP_SEL_MASK - sel: doing descending order shifting.
+	 * Because the relation of seleltion and value is like that
+	 *
+	 * seletion: value
+	 * 000: 64mv
+	 * 001: 32mv
+	 * ...
+	 * 111: 0.5mv
+	 *
+	 * For example, if I would like to select 64mv, the fls(ramp_delay) - 1 will be 0b111,
+	 * and I need to use 0b111 - sel to do the shifting
+	 */
+
+	sel = fls(ramp_delay) - 1;
+	sel = RTQ2208_BUCK_RAMP_SEL_MASK - sel;
+
+	val = FIELD_PREP(RTQ2208_BUCK_RSPUP_MASK, sel) | FIELD_PREP(RTQ2208_BUCK_RSPDN_MASK, sel);
+
+	return regmap_update_bits(rdev->regmap, rdesc->ramp_reg,
+				  RTQ2208_BUCK_RSPUP_MASK | RTQ2208_BUCK_RSPDN_MASK, val);
+}
+
+static int rtq2208_set_suspend_enable(struct regulator_dev *rdev)
+{
+	struct rtq2208_regulator_desc *rdesc =
+		(struct rtq2208_regulator_desc *)rdev->desc;
+
+	return regmap_set_bits(rdev->regmap, rdesc->suspend_config_reg, rdesc->suspend_enable_mask);
+}
+
+static int rtq2208_set_suspend_disable(struct regulator_dev *rdev)
+{
+	struct rtq2208_regulator_desc *rdesc =
+		(struct rtq2208_regulator_desc *)rdev->desc;
+
+	return regmap_update_bits(rdev->regmap, rdesc->suspend_config_reg, rdesc->suspend_enable_mask, 0);
+}
+
+static int rtq2208_set_suspend_mode(struct regulator_dev *rdev, unsigned int mode)
+{
+	struct rtq2208_regulator_desc *rdesc =
+		(struct rtq2208_regulator_desc *)rdev->desc;
+	unsigned int val, shift;
+
+	switch (mode) {
+	case REGULATOR_MODE_NORMAL:
+		val = RTQ2208_AUTO_MODE;
+		break;
+	case REGULATOR_MODE_FAST:
+		val = RTQ2208_FCCM;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	shift = ffs(rdesc->suspend_mode_mask) - 1;
+
+	return regmap_update_bits(rdev->regmap, rdesc->suspend_config_reg,
+			rdesc->suspend_mode_mask, val << shift);
+}
+
+static int rtq2208_ldo_get_voltage(struct regulator_dev *rdev)
+{
+	struct rtq2208_regulator_desc *rdesc =
+		(struct rtq2208_regulator_desc *)rdev->desc;
+
+	return rdesc->fixed_uV;
+}
+
+static const struct regulator_ops rtq2208_regulator_buck_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_mode = rtq2208_set_mode,
+	.get_mode = rtq2208_get_mode,
+	.set_ramp_delay = rtq2208_set_ramp_delay,
+	.set_active_discharge = regulator_set_active_discharge_regmap,
+	.set_suspend_enable = rtq2208_set_suspend_enable,
+	.set_suspend_disable = rtq2208_set_suspend_disable,
+	.set_suspend_mode = rtq2208_set_suspend_mode,
+};
+
+static const struct regulator_ops rtq2208_regulator_ldo_ops = {
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.set_active_discharge = regulator_set_active_discharge_regmap,
+	.set_suspend_enable = rtq2208_set_suspend_enable,
+	.set_suspend_disable = rtq2208_set_suspend_disable,
+	.get_voltage = rtq2208_ldo_get_voltage,
+};
+
+static int rtq2208_of_parse_cb(struct device_node *np,
+				const struct regulator_desc *desc,
+				struct regulator_config *config)
+{
+	struct rtq2208_regulator_desc *rdesc =
+		(struct rtq2208_regulator_desc *)desc;
+	u32 min_uV, max_uV;
+	int ret;
+
+	ret = of_property_read_u32(np, "regulator-min-microvolt", &min_uV);
+	if (ret)
+		return ret;
+
+	ret = of_property_read_u32(np, "regulator-max-microvolt", &max_uV);
+	if (ret)
+		return ret;
+
+	if (min_uV == max_uV)
+		rdesc->fixed_uV = min_uV;
+
+	return ret;
+}
+
+static unsigned int rtq2208_of_map_mode(unsigned int mode)
+{
+	switch (mode) {
+	case RTQ2208_AUTO_MODE:
+		return REGULATOR_MODE_NORMAL;
+	case RTQ2208_FCCM:
+		return REGULATOR_MODE_FAST;
+	default:
+		return REGULATOR_MODE_INVALID;
+	}
+}
+
+static int rtq2208_init_irq_mask(struct rtq2208_rdev_map *rdev_map, unsigned int *buck_masks)
+{
+	unsigned char buck_clr_masks[5] = {0x33, 0x33, 0x33, 0x33, 0x33},
+		      sts_clr_masks[2] = {0xE7, 0xF7}, sts_masks[2] = {0xE6, 0xF6};
+	int ret;
+
+	/* write clear all buck irq once */
+	ret = regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_FLT_RECORDBUCK_CB, buck_clr_masks, 5);
+	if (ret)
+		return dev_err_probe(rdev_map->dev, ret, "Failed to clr buck irqs\n");
+
+	/* write clear general irq once */
+	ret = regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_GLOBAL_INT1, sts_clr_masks, 2);
+	if (ret)
+		return dev_err_probe(rdev_map->dev, ret, "Failed to clr general irqs\n");
+
+	/* unmask buck ov/uv irq */
+	ret = regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_FLT_MASKBUCK_CB, buck_masks, 5);
+	if (ret)
+		return dev_err_probe(rdev_map->dev, ret, "Failed to unmask buck irqs\n");
+
+	/* unmask needed general irq */
+	return regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_GLOBAL_INT1_MASK, sts_masks, 2);
+}
+
+static irqreturn_t rtq2208_irq_handler(int irqno, void *devid)
+{
+	unsigned char buck_flags[RTQ2208_BUCK_NUM_IRQ_REGS], sts_flags[RTQ2208_STS_NUM_IRQ_REGS];
+	int ret = 0, i, uv_bit, ov_bit;
+	struct rtq2208_rdev_map *rdev_map = devid;
+	struct regulator_dev *rdev;
+
+	if (!rdev_map)
+		return IRQ_NONE;
+
+	/* read irq event */
+	ret = regmap_bulk_read(rdev_map->regmap, RTQ2208_REG_FLT_RECORDBUCK_CB,
+				buck_flags, ARRAY_SIZE(buck_flags));
+	if (ret)
+		return IRQ_NONE;
+
+	ret = regmap_bulk_read(rdev_map->regmap, RTQ2208_REG_GLOBAL_INT1,
+				sts_flags, ARRAY_SIZE(sts_flags));
+	if (ret)
+		return IRQ_NONE;
+
+	/* clear irq event */
+	ret = regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_FLT_RECORDBUCK_CB,
+				buck_flags, ARRAY_SIZE(buck_flags));
+	if (ret)
+		return IRQ_NONE;
+
+	ret = regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_GLOBAL_INT1,
+				sts_flags, ARRAY_SIZE(sts_flags));
+	if (ret)
+		return IRQ_NONE;
+
+	for (i = 0; i < RTQ2208_LDO_MAX; i++) {
+		if (!rdev_map->rdev[i])
+			continue;
+
+		rdev = rdev_map->rdev[i];
+		/* uv irq */
+		uv_bit = (i & 1) ? 4 : 0;
+		if (buck_flags[i >> 1] & (1 << uv_bit))
+			regulator_notifier_call_chain(rdev,
+					REGULATOR_EVENT_UNDER_VOLTAGE, NULL);
+		/* ov irq */
+		ov_bit = uv_bit + 1;
+		if (buck_flags[i >> 1] & (1 << ov_bit))
+			regulator_notifier_call_chain(rdev,
+					REGULATOR_EVENT_REGULATION_OUT, NULL);
+
+		/* hd irq */
+		if (sts_flags[1] & RTQ2208_HD_INT_MASK)
+			regulator_notifier_call_chain(rdev,
+					REGULATOR_EVENT_OVER_TEMP, NULL);
+	}
+
+	return IRQ_HANDLED;
+}
+
+#define RTQ2208_REGULATOR_INFO(_name, _base) \
+{ \
+	.name = #_name, \
+	.base = _base, \
+}
+#define BUCK_RG_BASE(_id)	RTQ2208_REG_BUCK_##_id##_CFG0
+#define BUCK_RG_SHIFT(_base, _shift)	(_base + _shift)
+#define LDO_RG_BASE(_id)	RTQ2208_REG_LDO##_id##_CFG
+#define LDO_RG_SHIFT(_base, _shift)	(_base + _shift)
+#define	VSEL_SHIFT(_sel)	(_sel ? 3 : 1)
+#define MTP_SEL_MASK(_sel)	RTQ2208_BUCK_EN_NR_MTP_SEL##_sel##_MASK
+
+static const struct linear_range rtq2208_vout_range[] = {
+	REGULATOR_LINEAR_RANGE(400000, 0, 180, 5000),
+	REGULATOR_LINEAR_RANGE(1310000, 181, 255, 10000),
+};
+
+static void rtq2208_init_regulator_desc(struct rtq2208_regulator_desc *rdesc, int mtp_sel, int idx)
+{
+	struct regulator_desc *desc;
+	static const struct {
+		char *name;
+		int base;
+	} regulator_info[] = {
+		RTQ2208_REGULATOR_INFO(buck-b, BUCK_RG_BASE(B)),
+		RTQ2208_REGULATOR_INFO(buck-c, BUCK_RG_BASE(C)),
+		RTQ2208_REGULATOR_INFO(buck-d, BUCK_RG_BASE(D)),
+		RTQ2208_REGULATOR_INFO(buck-a, BUCK_RG_BASE(A)),
+		RTQ2208_REGULATOR_INFO(buck-f, BUCK_RG_BASE(F)),
+		RTQ2208_REGULATOR_INFO(buck-g, BUCK_RG_BASE(G)),
+		RTQ2208_REGULATOR_INFO(buck-h, BUCK_RG_BASE(H)),
+		RTQ2208_REGULATOR_INFO(buck-e, BUCK_RG_BASE(E)),
+		RTQ2208_REGULATOR_INFO(ldo2, LDO_RG_BASE(2)),
+		RTQ2208_REGULATOR_INFO(ldo1, LDO_RG_BASE(1)),
+	}, *curr_info;
+
+	curr_info = regulator_info + idx;
+	desc = &rdesc->desc;
+	desc->name = curr_info->name;
+	desc->of_match = of_match_ptr(curr_info->name);
+	desc->regulators_node = of_match_ptr("regulators");
+	desc->id = idx;
+	desc->owner = THIS_MODULE;
+	desc->type = REGULATOR_VOLTAGE;
+	desc->enable_mask = mtp_sel ? MTP_SEL_MASK(1) : MTP_SEL_MASK(0);
+	desc->active_discharge_on = RTQ2208_EN_DIS_MASK;
+	desc->active_discharge_off = 0;
+	desc->active_discharge_mask = RTQ2208_EN_DIS_MASK;
+
+	rdesc->mode_mask = RTQ2208_BUCK_NRMODE_MASK;
+
+	if (idx >= RTQ2208_BUCK_B && idx <= RTQ2208_BUCK_E) {
+		/* init buck desc */
+		desc->enable_reg = BUCK_RG_SHIFT(curr_info->base, 2);
+		desc->ops = &rtq2208_regulator_buck_ops;
+		desc->vsel_reg = curr_info->base + VSEL_SHIFT(mtp_sel);
+		desc->vsel_mask = RTQ2208_BUCK_NR_MTP_SEL_MASK;
+		desc->n_voltages = RTQ2208_VOUT_MAXNUM;
+		desc->linear_ranges = rtq2208_vout_range;
+		desc->n_linear_ranges = ARRAY_SIZE(rtq2208_vout_range);
+		desc->ramp_reg = BUCK_RG_SHIFT(curr_info->base, 5);
+		desc->active_discharge_reg = curr_info->base;
+		desc->of_map_mode = rtq2208_of_map_mode;
+
+		rdesc->mode_reg = BUCK_RG_SHIFT(curr_info->base, 2);
+		rdesc->suspend_config_reg = BUCK_RG_SHIFT(curr_info->base, 4);
+		rdesc->suspend_enable_mask = RTQ2208_BUCK_EN_STR_MASK;
+		rdesc->suspend_mode_mask = RTQ2208_BUCK_STRMODE_MASK;
+	} else {
+		/* init ldo desc */
+		desc->enable_reg = curr_info->base;
+		desc->ops = &rtq2208_regulator_ldo_ops;
+		desc->n_voltages = 1;
+		desc->active_discharge_reg = LDO_RG_SHIFT(curr_info->base, 2);
+		desc->of_parse_cb = rtq2208_of_parse_cb;
+
+		rdesc->suspend_config_reg = curr_info->base;
+		rdesc->suspend_enable_mask = RTQ2208_LDO_EN_STR_MASK;
+	}
+}
+
+/** different slave address corresponds different used bucks
+ * slave address 0x10: BUCK[BCA FGE]
+ * slave address 0x20: BUCK[BC FGHE]
+ * slave address 0x40: BUCK[C G]
+ */
+static int rtq2208_regulator_check(int slave_addr, int *num, int *regulator_idx_table, unsigned int *buck_masks)
+{
+	static bool rtq2208_used_table[3][RTQ2208_LDO_MAX] = {
+		/* BUCK[BCA FGE], LDO[12] */
+		{1, 1, 0, 1, 1, 1, 0, 1, 1, 1},
+		/* BUCK[BC FGHE], LDO[12]*/
+		{1, 1, 0, 0, 1, 1, 1, 1, 1, 1},
+		/* BUCK[C G], LDO[12] */
+		{0, 1, 0, 0, 0, 1, 0, 0, 1, 1},
+	};
+	int i, idx = ffs(slave_addr >> 4) - 1;
+	u8 mask;
+
+	for (i = 0; i < RTQ2208_LDO_MAX; i++) {
+		if (!rtq2208_used_table[idx][i])
+			continue;
+
+		regulator_idx_table[(*num)++] = i;
+
+		mask = RTQ2208_BUCK_MASK(4 * i, 4 * i + 1);
+		buck_masks[i >> 1] &= ~mask;
+	}
+
+	return 0;
+}
+
+static const struct regmap_config rtq2208_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0xEF,
+};
+
+static int rtq2208_probe(struct i2c_client *i2c)
+{
+	struct device *dev = &i2c->dev;
+	struct regmap *regmap;
+	struct rtq2208_regulator_desc *rdesc;
+	struct regulator_dev *rdev;
+	struct regulator_config cfg;
+	struct rtq2208_rdev_map *rdev_map;
+	int i, ret = 0, idx, mtp_sel, n_regulator = 0;
+	unsigned int regulator_idx_table[RTQ2208_LDO_MAX],
+		     buck_masks[RTQ2208_BUCK_NUM_IRQ_REGS] = {0x33, 0x33, 0x33, 0x33, 0x33};
+
+	rdev_map = devm_kzalloc(dev, sizeof(struct rtq2208_rdev_map), GFP_KERNEL);
+	if (!rdev_map)
+		return -ENOMEM;
+
+	/* get mtp_sel0 or mtp_sel1 */
+	mtp_sel = device_property_read_bool(dev, "richtek,mtp-sel-high");
+
+	regmap = devm_regmap_init_i2c(i2c, &rtq2208_regmap_config);
+	if (IS_ERR(regmap))
+		return dev_err_probe(dev, PTR_ERR(regmap), "Failed to allocate regmap\n");
+
+	/* get needed regulator */
+	ret = rtq2208_regulator_check(i2c->addr, &n_regulator, regulator_idx_table, buck_masks);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to check used regulators\n");
+
+	rdev_map->regmap = regmap;
+	rdev_map->dev = dev;
+
+	cfg.dev = dev;
+
+	for (i = 0; i < n_regulator; i++) {
+		idx = regulator_idx_table[i];
+
+		rdesc = devm_kcalloc(dev, 1, sizeof(struct rtq2208_regulator_desc), GFP_KERNEL);
+		if (!rdesc)
+			return -ENOMEM;
+
+		/* init regulator desc */
+		rtq2208_init_regulator_desc(rdesc, mtp_sel, idx);
+
+		/* regiser regulator */
+		rdev = devm_regulator_register(dev, &rdesc->desc, &cfg);
+		if (IS_ERR(rdev))
+			return PTR_ERR(rdev);
+
+		rdev_map->rdev[idx] = rdev;
+	}
+
+	/* init interrupt mask */
+	ret = rtq2208_init_irq_mask(rdev_map, buck_masks);
+	if (ret)
+		return ret;
+
+	/* register interrupt */
+	ret = devm_request_threaded_irq(dev, i2c->irq, NULL, rtq2208_irq_handler,
+					IRQF_ONESHOT, dev_name(dev), rdev_map);
+
+	return ret;
+}
+
+static const struct of_device_id __maybe_unused rtq2208_device_tables[] = {
+	{ .compatible = "richtek,rtq2208", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, rtq2208_device_tables);
+
+static struct i2c_driver rtq2208_driver = {
+	.driver = {
+		.name = "rtq2208",
+		.of_match_table = rtq2208_device_tables,
+	},
+	.probe_new = rtq2208_probe,
+};
+module_i2c_driver(rtq2208_driver);
+
+MODULE_AUTHOR("Alina Yu <alina_yu@richtek.com>");
+MODULE_DESCRIPTION("Richtek RTQ2208 Regulator Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


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

* Re: [PATCH v5 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC
  2023-07-26  7:13 ` [PATCH v5 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC Alina Yu
@ 2023-07-26  9:53   ` Krzysztof Kozlowski
  2023-08-03  6:36   ` ChiYuan Huang
  1 sibling, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2023-07-26  9:53 UTC (permalink / raw)
  To: Alina Yu, lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt
  Cc: linux-kernel, devicetree

On 26/07/2023 09:13, Alina Yu wrote:
> Add bindings for Richtek RTQ2208 IC controlled SubPMIC
> 
> Signed-off-by: Alina Yu <alina_yu@richtek.com>

Thank you for your patch. There is something to discuss/improve.

> +    
> +  richtek,mtp-sel-high:
> +    type: boolean
> +    description:
> +      vout register selection based on this boolean value.
> +      false - Using DVS0 register setting to adjust vout
> +      true - Using DVS1 register setting to adjust vout
> +
> +  regulators:
> +    type: object
> +

On this level of indentation:

    additionalProperties: false

I think I missed this in previous versions. I am sorry.

Rest looks good to me, but this needs to be fixed.

Best regards,
Krzysztof


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

* Re: [PATCH v5 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC
  2023-07-26  7:13 ` [PATCH v5 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC Alina Yu
  2023-07-26  9:53   ` Krzysztof Kozlowski
@ 2023-08-03  6:36   ` ChiYuan Huang
  2023-08-05 21:27     ` Krzysztof Kozlowski
  1 sibling, 1 reply; 10+ messages in thread
From: ChiYuan Huang @ 2023-08-03  6:36 UTC (permalink / raw)
  To: Alina Yu
  Cc: lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-kernel, devicetree

On Wed, Jul 26, 2023 at 03:13:11PM +0800, Alina Yu wrote:
> Add bindings for Richtek RTQ2208 IC controlled SubPMIC
> 
> Signed-off-by: Alina Yu <alina_yu@richtek.com>
Hi,
> ---
> v5
> - Revise filename from $id
> - Remove "regulator-compatible" for ldos
> - Add missing "high" for "richtek,mtp-sel-high"
> - Remove "regulator-mode" property
> - Add and modify "unevaluatedProperties: false" and "additionalProperties: false"
> - Remove "richtek,fixed-uV"
> - Remove redundant space in "buck-a" and "ldo2"
> v4
> - Modify filename to "richtek,rtq2208"
> - Add more desciptions for "regulator-allowed-modes"
> ---
>  .../bindings/regulator/richtek,rtq2208.yaml        | 196 +++++++++++++++++++++
>  1 file changed, 196 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml b/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
> new file mode 100644
> index 0000000..63453f2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
> @@ -0,0 +1,196 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/richtek,rtq2208.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Richtek RTQ2208 SubPMIC Regulator
> +
> +maintainers:
> +  - Alina Yu <alina_yu@richtek.com>
> +
> +description: |
> +  RTQ2208 is a highly integrated power converter that offers functional safety dual
> +  multi-configurable synchronous buck converters and two LDOs.
> +
> +  Bucks support "regulator-allowed-modes" and "regulator-mode". The former defines the permitted
> +  switching operation in normal mode; the latter defines the operation in suspend to RAM mode.
> +
> +  No matter the RTQ2208 is configured to normal or suspend to RAM mode, there are two switching
> +  operation modes for all buck rails, automatic power saving mode (Auto mode) and forced continuous
> +  conduction mode (FCCM).
> +
> +  The definition of modes is in the datasheet which is available in below link
> +  and their meaning is::
> +    0 - Auto mode for power saving, which reducing the switching frequency at light load condition
> +    to maintain high frequency.
> +    1 - FCCM to meet the strict voltage regulation accuracy, which keeping constant switching frequency.
> +
> +  Datasheet will be available soon at
> +  https://www.richtek.com/assets/Products
> +
> +properties:
> +  compatible:
> +    enum:
> +      - richtek,rtq2208
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +    
> +  richtek,mtp-sel-high:
> +    type: boolean
> +    description:
> +      vout register selection based on this boolean value.
> +      false - Using DVS0 register setting to adjust vout
> +      true - Using DVS1 register setting to adjust vout
> +
> +  regulators:
> +    type: object
Just curious.
It seems this PMIC only support buck/ldo ouput.
Since Krzysztof already reviewed it, not sure whether to group it into one node is common or not.
AFAIK, if there's no more function included, to put all on the top level may be better.
> +
> +    patternProperties:
> +      "^buck-[a-h]$":
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +        description:
> +          description for buck-[a-h] regulator.
> +
> +        properties:
> +          regulator-allowed-modes:
> +            description:
> +              two buck modes in different switching accuracy.
> +              0 - Auto mode
> +              1 - FCCM
> +            items:
> +              enum: [0, 1]
> +
> +      "^ldo[1-2]$":
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +        description:
> +          regulator description for ldo[1-2].
> +
> +required:
> +  - compatible
> +  - reg
> +  - regulators
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      pmic@10 {
> +        compatible = "richtek,rtq2208";
> +        reg = <0x10>;
> +        interrupts-extended = <&gpio26 0 IRQ_TYPE_LEVEL_LOW>;
> +        richtek,mtp-sel-high;
> +
> +        regulators {
> +          buck-a {
> +            regulator-min-microvolt = <400000>;
> +            regulator-max-microvolt = <2050000>;
> +            regulator-allowed-modes = <0 1>;
> +            regulator-always-on;
> +            regulator-state-mem {
> +              regulator-on-in-suspend;
> +              regulator-mode = <1>;
> +            };
> +          };
> +          buck-b {
> +            regulator-min-microvolt = <400000>;
> +            regulator-max-microvolt = <2050000>;
> +            regulator-allowed-modes = <0 1>;
> +            regulator-always-on;
> +            regulator-state-mem {
> +              regulator-on-in-suspend;
> +              regulator-mode = <1>;
> +            };
> +          };
> +          buck-c {
> +            regulator-min-microvolt = <400000>;
> +            regulator-max-microvolt = <2050000>;
> +            regulator-allowed-modes = <0 1>;
> +            regulator-always-on;
> +            regulator-state-mem {
> +              regulator-on-in-suspend;
> +              regulator-mode = <1>;
> +            };
> +          };
> +          buck-d {
> +            regulator-min-microvolt = <400000>;
> +            regulator-max-microvolt = <2050000>;
> +            regulator-allowed-modes = <0 1>;
> +            regulator-always-on;
> +            regulator-state-mem {
> +              regulator-on-in-suspend;
> +              regulator-mode = <1>;
> +            };
> +          };
> +          buck-e {
> +            regulator-min-microvolt = <400000>;
> +            regulator-max-microvolt = <2050000>;
> +            regulator-allowed-modes = <0 1>;
> +            regulator-always-on;
> +            regulator-state-mem {
> +              regulator-on-in-suspend;
> +              regulator-mode = <1>;
> +            };
> +          };
> +          buck-f {
> +            regulator-min-microvolt = <400000>;
> +            regulator-max-microvolt = <2050000>;
> +            regulator-allowed-modes = <0 1>;
> +            regulator-always-on;
> +            regulator-state-mem {
> +              regulator-on-in-suspend;
> +              regulator-mode = <1>;
> +            };
> +          };
> +          buck-g {
> +            regulator-min-microvolt = <400000>;
> +            regulator-max-microvolt = <2050000>;
> +            regulator-allowed-modes = <0 1>;
> +            regulator-always-on;
> +            regulator-state-mem {
> +              regulator-on-in-suspend;
> +              regulator-mode = <1>;
> +            };
> +          };
> +          buck-h {
> +            regulator-min-microvolt = <400000>;
> +            regulator-max-microvolt = <2050000>;
> +            regulator-allowed-modes = <0 1>;
> +            regulator-always-on;
> +            regulator-state-mem {
> +              regulator-on-in-suspend;
> +              regulator-mode = <1>;
> +            };
> +          };
> +          ldo1 {
> +            regulator-min-microvolt = <1200000>;
> +            regulator-max-microvolt = <1200000>;
> +            regulator-always-on;
> +            regulator-state-mem {
> +              regulator-on-in-suspend;
> +            };
> +          };
> +          ldo2 {
> +            regulator-min-microvolt = <3300000>;
> +            regulator-max-microvolt = <3300000>;
> +            regulator-always-on;
> +            regulator-state-mem {
> +              regulator-on-in-suspend;
> +            };
> +          };
> +        };
> +      };
> +    };
> -- 
> 2.7.4
> 

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

* Re: [PATCH v5 2/2] regulator: rtq2208: Add Richtek RTQ2208 SubPMIC driver
  2023-07-26  7:13 ` [PATCH v5 2/2] regulator: rtq2208: Add Richtek RTQ2208 SubPMIC driver Alina Yu
@ 2023-08-03  7:24   ` ChiYuan Huang
  2023-08-03 11:08     ` Mark Brown
  0 siblings, 1 reply; 10+ messages in thread
From: ChiYuan Huang @ 2023-08-03  7:24 UTC (permalink / raw)
  To: Alina Yu
  Cc: lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-kernel, devicetree

Hi,
On Wed, Jul 26, 2023 at 03:13:12PM +0800, Alina Yu wrote:
> Add support for the RTQ2208 SubPMIC
> This ic integrates with configurable, synchrnous buck converters and two
> ldos.
> 
> Signed-off-by: Alina Yu <alina_yu@richtek.com>
> Reported-by: kernel test robot <lkp@intel.com>
This patch series is to add new drivers, not fix.
Why do you add the Reported-by tag?
> ---
> v5
> - Modify 'rdesc->fixed_uV' get, becasue "richtek,fixed-uV" is removed from yaml
> - Modify 'mtp_sel' get because read property is changed from "richtek,mtp-sel" to
>   "richtek,mtp-sel-high" in yaml
> - Add missing regulators_node points to regulator node in yaml
> - Include <linux/bitfield.h> for 'FIELD_PREP' reported by kernel test robot
> ---
>  drivers/regulator/Kconfig             |  11 +
>  drivers/regulator/Makefile            |   1 +
>  drivers/regulator/rtq2208-regulator.c | 549 ++++++++++++++++++++++++++++++++++
>  3 files changed, 561 insertions(+)
>  create mode 100644 drivers/regulator/rtq2208-regulator.c
> 
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index e5f3613..a6b2c84 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -1213,6 +1213,17 @@ config REGULATOR_RTQ6752
>  	  synchronous boost converters for PAVDD, and one synchronous NAVDD
>  	  buck-boost. This device is suitable for automotive TFT-LCD panel.
>  
> +config REGULATOR_RTQ2208
> +	tristate "Richtek RTQ2208 SubPMIC Regulator"
> +	depends on I2C
> +	select REGMAP_I2C
> +	help
> +	  This driver adds support for RTQ2208 SubPMIC regulators.
> +	  The RTQ2208 is a multi-phase, programmable power management IC that
> +	  integrate with dual multi-configurable, synchronous buck converters
> +	  and two ldos. It features wide output voltage range from 0.4V to 2.05V
> +	  and the capability to configure the corresponding power stages.
> +
>  config REGULATOR_S2MPA01
>  	tristate "Samsung S2MPA01 voltage regulator"
>  	depends on MFD_SEC_CORE || COMPILE_TEST
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 58dfe01..04cbad17 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -143,6 +143,7 @@ obj-$(CONFIG_REGULATOR_RT6245)	+= rt6245-regulator.o
>  obj-$(CONFIG_REGULATOR_RTMV20)	+= rtmv20-regulator.o
>  obj-$(CONFIG_REGULATOR_RTQ2134) += rtq2134-regulator.o
>  obj-$(CONFIG_REGULATOR_RTQ6752)	+= rtq6752-regulator.o
> +obj-$(CONFIG_REGULATOR_RTQ2208) += rtq2208-regulator.o
>  obj-$(CONFIG_REGULATOR_S2MPA01) += s2mpa01.o
>  obj-$(CONFIG_REGULATOR_S2MPS11) += s2mps11.o
>  obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
> diff --git a/drivers/regulator/rtq2208-regulator.c b/drivers/regulator/rtq2208-regulator.c
> new file mode 100644
> index 0000000..2cfbc68
> --- /dev/null
> +++ b/drivers/regulator/rtq2208-regulator.c
> @@ -0,0 +1,549 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include <linux/bitops.h>
> +#include <linux/bitfield.h>
> +#include <linux/util_macros.h>
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/driver.h>
It seems you forgot to include mod_devicetable.h
> +
> +/* Register */
> +#define RTQ2208_REG_GLOBAL_INT1			0x12
> +#define RTQ2208_REG_FLT_RECORDBUCK_CB		0x18
> +#define RTQ2208_REG_GLOBAL_INT1_MASK		0x1D
> +#define RTQ2208_REG_FLT_MASKBUCK_CB		0x1F
> +#define RTQ2208_REG_BUCK_C_CFG0			0x32
> +#define RTQ2208_REG_BUCK_B_CFG0			0x42
> +#define RTQ2208_REG_BUCK_A_CFG0			0x52
> +#define RTQ2208_REG_BUCK_D_CFG0			0x62
> +#define RTQ2208_REG_BUCK_G_CFG0			0x72
> +#define RTQ2208_REG_BUCK_F_CFG0			0x82
> +#define RTQ2208_REG_BUCK_E_CFG0			0x92
> +#define RTQ2208_REG_BUCK_H_CFG0			0xA2
> +#define RTQ2208_REG_LDO1_CFG			0xB1
> +#define RTQ2208_REG_LDO2_CFG			0xC1
> +
> +/* Mask */
> +#define RTQ2208_BUCK_NR_MTP_SEL_MASK		GENMASK(7, 0)
> +#define RTQ2208_BUCK_EN_NR_MTP_SEL0_MASK	BIT(0)
> +#define RTQ2208_BUCK_EN_NR_MTP_SEL1_MASK	BIT(1)
> +#define RTQ2208_BUCK_RSPUP_MASK			GENMASK(6, 4)
> +#define RTQ2208_BUCK_RSPDN_MASK			GENMASK(2, 0)
> +#define RTQ2208_BUCK_NRMODE_MASK		BIT(5)
> +#define RTQ2208_BUCK_STRMODE_MASK		BIT(5)
> +#define RTQ2208_BUCK_EN_STR_MASK		BIT(0)
> +#define RTQ2208_LDO_EN_STR_MASK			BIT(7)
> +#define RTQ2208_EN_DIS_MASK			BIT(0)
> +#define RTQ2208_BUCK_RAMP_SEL_MASK		GENMASK(2, 0)
> +#define RTQ2208_HD_INT_MASK			BIT(0)
> +
> +/* Size */
> +#define RTQ2208_VOUT_MAXNUM			256
> +#define RTQ2208_BUCK_NUM_IRQ_REGS		5
> +#define RTQ2208_STS_NUM_IRQ_REGS		2
> +
> +/* Value */
> +#define RTQ2208_RAMP_VALUE_MIN_uV		500
> +#define RTQ2208_RAMP_VALUE_MAX_uV		64000
> +
> +#define RTQ2208_BUCK_MASK(uv_irq, ov_irq)	(1 << ((uv_irq) % 8) | 1 << ((ov_irq) % 8))
> +
> +enum {
> +	RTQ2208_BUCK_B = 0,
> +	RTQ2208_BUCK_C,
> +	RTQ2208_BUCK_D,
> +	RTQ2208_BUCK_A,
> +	RTQ2208_BUCK_F,
> +	RTQ2208_BUCK_G,
> +	RTQ2208_BUCK_H,
> +	RTQ2208_BUCK_E,
> +	RTQ2208_LDO2,
> +	RTQ2208_LDO1,
> +	RTQ2208_LDO_MAX,
> +};
> +
> +enum {
> +	RTQ2208_AUTO_MODE = 0,
> +	RTQ2208_FCCM,
> +};
> +
> +struct rtq2208_regulator_desc {
> +	struct regulator_desc desc;
> +	unsigned int mtp_sel_reg;
> +	unsigned int mtp_sel_mask;
> +	unsigned int mode_reg;
> +	unsigned int mode_mask;
> +	unsigned int suspend_config_reg;
> +	unsigned int suspend_enable_mask;
> +	unsigned int suspend_mode_mask;
> +	unsigned int fixed_uV;
This is duplicated with the member 'fixed_uV' in struct regulator_desc.
> +};
> +
> +struct rtq2208_rdev_map {
> +	struct regulator_dev *rdev[RTQ2208_LDO_MAX];
> +	struct regmap *regmap;
> +	struct device *dev;
> +};
> +
> +/* set Normal Auto/FCCM mode */
> +static int rtq2208_set_mode(struct regulator_dev *rdev, unsigned int mode)
> +{
> +	struct rtq2208_regulator_desc *rdesc =
> +		(struct rtq2208_regulator_desc *)rdev->desc;
> +	unsigned int val, shift;
> +
> +	switch (mode) {
> +	case REGULATOR_MODE_NORMAL:
> +		val = RTQ2208_AUTO_MODE;
> +		break;
> +	case REGULATOR_MODE_FAST:
> +		val = RTQ2208_FCCM;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	shift = ffs(rdesc->mode_mask) - 1;
> +	return regmap_update_bits(rdev->regmap, rdesc->mode_reg,
> +				  rdesc->mode_mask, val << shift);
> +}
> +
> +static unsigned int rtq2208_get_mode(struct regulator_dev *rdev)
> +{
> +	struct rtq2208_regulator_desc *rdesc =
> +		(struct rtq2208_regulator_desc *)rdev->desc;
> +	unsigned int mode_val;
> +	int ret;
> +
> +	ret = regmap_read(rdev->regmap, rdesc->mode_reg, &mode_val);
> +	if (ret)
> +		return REGULATOR_MODE_INVALID;
> +
> +	return (mode_val & rdesc->mode_mask) ? REGULATOR_MODE_FAST : REGULATOR_MODE_NORMAL;
> +}
> +
> +static int rtq2208_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
> +{
> +	const struct regulator_desc *rdesc = rdev->desc;
> +	unsigned int sel = 0, val;
> +
> +	ramp_delay = max(ramp_delay, RTQ2208_RAMP_VALUE_MIN_uV);
> +	ramp_delay = min(ramp_delay, RTQ2208_RAMP_VALUE_MAX_uV);
> +
> +	ramp_delay /= RTQ2208_RAMP_VALUE_MIN_uV;
> +
> +	/*
> +	 * fls(ramp_delay) - 1: doing LSB shift, let it starts from 0
> +	 *
> +	 * RTQ2208_BUCK_RAMP_SEL_MASK - sel: doing descending order shifting.
> +	 * Because the relation of seleltion and value is like that
> +	 *
> +	 * seletion: value
> +	 * 000: 64mv
> +	 * 001: 32mv
> +	 * ...
> +	 * 111: 0.5mv
> +	 *
> +	 * For example, if I would like to select 64mv, the fls(ramp_delay) - 1 will be 0b111,
> +	 * and I need to use 0b111 - sel to do the shifting
> +	 */
> +
> +	sel = fls(ramp_delay) - 1;
> +	sel = RTQ2208_BUCK_RAMP_SEL_MASK - sel;
> +
> +	val = FIELD_PREP(RTQ2208_BUCK_RSPUP_MASK, sel) | FIELD_PREP(RTQ2208_BUCK_RSPDN_MASK, sel);
> +
> +	return regmap_update_bits(rdev->regmap, rdesc->ramp_reg,
> +				  RTQ2208_BUCK_RSPUP_MASK | RTQ2208_BUCK_RSPDN_MASK, val);
> +}
> +
> +static int rtq2208_set_suspend_enable(struct regulator_dev *rdev)
> +{
> +	struct rtq2208_regulator_desc *rdesc =
> +		(struct rtq2208_regulator_desc *)rdev->desc;
> +
> +	return regmap_set_bits(rdev->regmap, rdesc->suspend_config_reg, rdesc->suspend_enable_mask);
> +}
> +
> +static int rtq2208_set_suspend_disable(struct regulator_dev *rdev)
> +{
> +	struct rtq2208_regulator_desc *rdesc =
> +		(struct rtq2208_regulator_desc *)rdev->desc;
> +
> +	return regmap_update_bits(rdev->regmap, rdesc->suspend_config_reg, rdesc->suspend_enable_mask, 0);
> +}
> +
> +static int rtq2208_set_suspend_mode(struct regulator_dev *rdev, unsigned int mode)
> +{
> +	struct rtq2208_regulator_desc *rdesc =
> +		(struct rtq2208_regulator_desc *)rdev->desc;
> +	unsigned int val, shift;
> +
> +	switch (mode) {
> +	case REGULATOR_MODE_NORMAL:
> +		val = RTQ2208_AUTO_MODE;
> +		break;
> +	case REGULATOR_MODE_FAST:
> +		val = RTQ2208_FCCM;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	shift = ffs(rdesc->suspend_mode_mask) - 1;
> +
> +	return regmap_update_bits(rdev->regmap, rdesc->suspend_config_reg,
> +			rdesc->suspend_mode_mask, val << shift);
> +}
> +
> +static int rtq2208_ldo_get_voltage(struct regulator_dev *rdev)
If you use the 'fixed_uV' in regulator_desc, then this 'get_voltage' API is not needed.
> +{
> +	struct rtq2208_regulator_desc *rdesc =
> +		(struct rtq2208_regulator_desc *)rdev->desc;
> +
> +	return rdesc->fixed_uV;
> +}
> +
> +static const struct regulator_ops rtq2208_regulator_buck_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_mode = rtq2208_set_mode,
> +	.get_mode = rtq2208_get_mode,
> +	.set_ramp_delay = rtq2208_set_ramp_delay,
> +	.set_active_discharge = regulator_set_active_discharge_regmap,
> +	.set_suspend_enable = rtq2208_set_suspend_enable,
> +	.set_suspend_disable = rtq2208_set_suspend_disable,
> +	.set_suspend_mode = rtq2208_set_suspend_mode,
> +};
> +
> +static const struct regulator_ops rtq2208_regulator_ldo_ops = {
> +	.enable = regulator_enable_regmap,
> +	.disable = regulator_disable_regmap,
> +	.is_enabled = regulator_is_enabled_regmap,
> +	.set_active_discharge = regulator_set_active_discharge_regmap,
> +	.set_suspend_enable = rtq2208_set_suspend_enable,
> +	.set_suspend_disable = rtq2208_set_suspend_disable,
> +	.get_voltage = rtq2208_ldo_get_voltage,
> +};
> +
> +static int rtq2208_of_parse_cb(struct device_node *np,
> +				const struct regulator_desc *desc,
> +				struct regulator_config *config)
> +{
> +	struct rtq2208_regulator_desc *rdesc =
> +		(struct rtq2208_regulator_desc *)desc;
> +	u32 min_uV, max_uV;
> +	int ret;
> +
> +	ret = of_property_read_u32(np, "regulator-min-microvolt", &min_uV);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_property_read_u32(np, "regulator-max-microvolt", &max_uV);
> +	if (ret)
> +		return ret;
> +
> +	if (min_uV == max_uV)
> +		rdesc->fixed_uV = min_uV;
> +
> +	return ret;
> +}
> +
> +static unsigned int rtq2208_of_map_mode(unsigned int mode)
> +{
> +	switch (mode) {
> +	case RTQ2208_AUTO_MODE:
> +		return REGULATOR_MODE_NORMAL;
> +	case RTQ2208_FCCM:
> +		return REGULATOR_MODE_FAST;
> +	default:
> +		return REGULATOR_MODE_INVALID;
> +	}
> +}
> +
> +static int rtq2208_init_irq_mask(struct rtq2208_rdev_map *rdev_map, unsigned int *buck_masks)
> +{
> +	unsigned char buck_clr_masks[5] = {0x33, 0x33, 0x33, 0x33, 0x33},
> +		      sts_clr_masks[2] = {0xE7, 0xF7}, sts_masks[2] = {0xE6, 0xF6};
> +	int ret;
> +
> +	/* write clear all buck irq once */
> +	ret = regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_FLT_RECORDBUCK_CB, buck_clr_masks, 5);
> +	if (ret)
> +		return dev_err_probe(rdev_map->dev, ret, "Failed to clr buck irqs\n");
> +
> +	/* write clear general irq once */
> +	ret = regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_GLOBAL_INT1, sts_clr_masks, 2);
> +	if (ret)
> +		return dev_err_probe(rdev_map->dev, ret, "Failed to clr general irqs\n");
> +
> +	/* unmask buck ov/uv irq */
> +	ret = regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_FLT_MASKBUCK_CB, buck_masks, 5);
> +	if (ret)
> +		return dev_err_probe(rdev_map->dev, ret, "Failed to unmask buck irqs\n");
> +
> +	/* unmask needed general irq */
> +	return regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_GLOBAL_INT1_MASK, sts_masks, 2);
> +}
> +
> +static irqreturn_t rtq2208_irq_handler(int irqno, void *devid)
> +{
> +	unsigned char buck_flags[RTQ2208_BUCK_NUM_IRQ_REGS], sts_flags[RTQ2208_STS_NUM_IRQ_REGS];
> +	int ret = 0, i, uv_bit, ov_bit;
> +	struct rtq2208_rdev_map *rdev_map = devid;
> +	struct regulator_dev *rdev;
> +
> +	if (!rdev_map)
> +		return IRQ_NONE;
> +
> +	/* read irq event */
> +	ret = regmap_bulk_read(rdev_map->regmap, RTQ2208_REG_FLT_RECORDBUCK_CB,
> +				buck_flags, ARRAY_SIZE(buck_flags));
> +	if (ret)
> +		return IRQ_NONE;
> +
> +	ret = regmap_bulk_read(rdev_map->regmap, RTQ2208_REG_GLOBAL_INT1,
> +				sts_flags, ARRAY_SIZE(sts_flags));
> +	if (ret)
> +		return IRQ_NONE;
> +
> +	/* clear irq event */
> +	ret = regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_FLT_RECORDBUCK_CB,
> +				buck_flags, ARRAY_SIZE(buck_flags));
> +	if (ret)
> +		return IRQ_NONE;
> +
> +	ret = regmap_bulk_write(rdev_map->regmap, RTQ2208_REG_GLOBAL_INT1,
> +				sts_flags, ARRAY_SIZE(sts_flags));
> +	if (ret)
> +		return IRQ_NONE;
> +
> +	for (i = 0; i < RTQ2208_LDO_MAX; i++) {
> +		if (!rdev_map->rdev[i])
> +			continue;
> +
> +		rdev = rdev_map->rdev[i];
> +		/* uv irq */
> +		uv_bit = (i & 1) ? 4 : 0;
> +		if (buck_flags[i >> 1] & (1 << uv_bit))
> +			regulator_notifier_call_chain(rdev,
> +					REGULATOR_EVENT_UNDER_VOLTAGE, NULL);
> +		/* ov irq */
> +		ov_bit = uv_bit + 1;
> +		if (buck_flags[i >> 1] & (1 << ov_bit))
> +			regulator_notifier_call_chain(rdev,
> +					REGULATOR_EVENT_REGULATION_OUT, NULL);
> +
> +		/* hd irq */
> +		if (sts_flags[1] & RTQ2208_HD_INT_MASK)
> +			regulator_notifier_call_chain(rdev,
> +					REGULATOR_EVENT_OVER_TEMP, NULL);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +#define RTQ2208_REGULATOR_INFO(_name, _base) \
> +{ \
> +	.name = #_name, \
> +	.base = _base, \
> +}
> +#define BUCK_RG_BASE(_id)	RTQ2208_REG_BUCK_##_id##_CFG0
> +#define BUCK_RG_SHIFT(_base, _shift)	(_base + _shift)
> +#define LDO_RG_BASE(_id)	RTQ2208_REG_LDO##_id##_CFG
> +#define LDO_RG_SHIFT(_base, _shift)	(_base + _shift)
> +#define	VSEL_SHIFT(_sel)	(_sel ? 3 : 1)
> +#define MTP_SEL_MASK(_sel)	RTQ2208_BUCK_EN_NR_MTP_SEL##_sel##_MASK
> +
> +static const struct linear_range rtq2208_vout_range[] = {
> +	REGULATOR_LINEAR_RANGE(400000, 0, 180, 5000),
> +	REGULATOR_LINEAR_RANGE(1310000, 181, 255, 10000),
> +};
> +
> +static void rtq2208_init_regulator_desc(struct rtq2208_regulator_desc *rdesc, int mtp_sel, int idx)
> +{
> +	struct regulator_desc *desc;
> +	static const struct {
> +		char *name;
> +		int base;
> +	} regulator_info[] = {
> +		RTQ2208_REGULATOR_INFO(buck-b, BUCK_RG_BASE(B)),
> +		RTQ2208_REGULATOR_INFO(buck-c, BUCK_RG_BASE(C)),
> +		RTQ2208_REGULATOR_INFO(buck-d, BUCK_RG_BASE(D)),
> +		RTQ2208_REGULATOR_INFO(buck-a, BUCK_RG_BASE(A)),
> +		RTQ2208_REGULATOR_INFO(buck-f, BUCK_RG_BASE(F)),
> +		RTQ2208_REGULATOR_INFO(buck-g, BUCK_RG_BASE(G)),
> +		RTQ2208_REGULATOR_INFO(buck-h, BUCK_RG_BASE(H)),
> +		RTQ2208_REGULATOR_INFO(buck-e, BUCK_RG_BASE(E)),
> +		RTQ2208_REGULATOR_INFO(ldo2, LDO_RG_BASE(2)),
> +		RTQ2208_REGULATOR_INFO(ldo1, LDO_RG_BASE(1)),
> +	}, *curr_info;
> +
> +	curr_info = regulator_info + idx;
> +	desc = &rdesc->desc;
> +	desc->name = curr_info->name;
> +	desc->of_match = of_match_ptr(curr_info->name);
> +	desc->regulators_node = of_match_ptr("regulators");
> +	desc->id = idx;
> +	desc->owner = THIS_MODULE;
> +	desc->type = REGULATOR_VOLTAGE;
> +	desc->enable_mask = mtp_sel ? MTP_SEL_MASK(1) : MTP_SEL_MASK(0);
> +	desc->active_discharge_on = RTQ2208_EN_DIS_MASK;
> +	desc->active_discharge_off = 0;
> +	desc->active_discharge_mask = RTQ2208_EN_DIS_MASK;
> +
> +	rdesc->mode_mask = RTQ2208_BUCK_NRMODE_MASK;
> +
> +	if (idx >= RTQ2208_BUCK_B && idx <= RTQ2208_BUCK_E) {
> +		/* init buck desc */
> +		desc->enable_reg = BUCK_RG_SHIFT(curr_info->base, 2);
> +		desc->ops = &rtq2208_regulator_buck_ops;
> +		desc->vsel_reg = curr_info->base + VSEL_SHIFT(mtp_sel);
> +		desc->vsel_mask = RTQ2208_BUCK_NR_MTP_SEL_MASK;
> +		desc->n_voltages = RTQ2208_VOUT_MAXNUM;
> +		desc->linear_ranges = rtq2208_vout_range;
> +		desc->n_linear_ranges = ARRAY_SIZE(rtq2208_vout_range);
> +		desc->ramp_reg = BUCK_RG_SHIFT(curr_info->base, 5);
> +		desc->active_discharge_reg = curr_info->base;
> +		desc->of_map_mode = rtq2208_of_map_mode;
> +
> +		rdesc->mode_reg = BUCK_RG_SHIFT(curr_info->base, 2);
> +		rdesc->suspend_config_reg = BUCK_RG_SHIFT(curr_info->base, 4);
> +		rdesc->suspend_enable_mask = RTQ2208_BUCK_EN_STR_MASK;
> +		rdesc->suspend_mode_mask = RTQ2208_BUCK_STRMODE_MASK;
> +	} else {
> +		/* init ldo desc */
> +		desc->enable_reg = curr_info->base;
> +		desc->ops = &rtq2208_regulator_ldo_ops;
> +		desc->n_voltages = 1;
> +		desc->active_discharge_reg = LDO_RG_SHIFT(curr_info->base, 2);
> +		desc->of_parse_cb = rtq2208_of_parse_cb;
> +
> +		rdesc->suspend_config_reg = curr_info->base;
> +		rdesc->suspend_enable_mask = RTQ2208_LDO_EN_STR_MASK;
> +	}
> +}
> +
> +/** different slave address corresponds different used bucks
> + * slave address 0x10: BUCK[BCA FGE]
> + * slave address 0x20: BUCK[BC FGHE]
> + * slave address 0x40: BUCK[C G]
> + */
> +static int rtq2208_regulator_check(int slave_addr, int *num, int *regulator_idx_table, unsigned int *buck_masks)
> +{
> +	static bool rtq2208_used_table[3][RTQ2208_LDO_MAX] = {
> +		/* BUCK[BCA FGE], LDO[12] */
> +		{1, 1, 0, 1, 1, 1, 0, 1, 1, 1},
> +		/* BUCK[BC FGHE], LDO[12]*/
> +		{1, 1, 0, 0, 1, 1, 1, 1, 1, 1},
> +		/* BUCK[C G], LDO[12] */
> +		{0, 1, 0, 0, 0, 1, 0, 0, 1, 1},
> +	};
> +	int i, idx = ffs(slave_addr >> 4) - 1;
> +	u8 mask;
> +
> +	for (i = 0; i < RTQ2208_LDO_MAX; i++) {
> +		if (!rtq2208_used_table[idx][i])
> +			continue;
> +
> +		regulator_idx_table[(*num)++] = i;
> +
> +		mask = RTQ2208_BUCK_MASK(4 * i, 4 * i + 1);
> +		buck_masks[i >> 1] &= ~mask;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct regmap_config rtq2208_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = 0xEF,
> +};
> +
> +static int rtq2208_probe(struct i2c_client *i2c)
> +{
> +	struct device *dev = &i2c->dev;
> +	struct regmap *regmap;
> +	struct rtq2208_regulator_desc *rdesc;
> +	struct regulator_dev *rdev;
> +	struct regulator_config cfg;
> +	struct rtq2208_rdev_map *rdev_map;
> +	int i, ret = 0, idx, mtp_sel, n_regulator = 0;
> +	unsigned int regulator_idx_table[RTQ2208_LDO_MAX],
> +		     buck_masks[RTQ2208_BUCK_NUM_IRQ_REGS] = {0x33, 0x33, 0x33, 0x33, 0x33};
> +
> +	rdev_map = devm_kzalloc(dev, sizeof(struct rtq2208_rdev_map), GFP_KERNEL);
> +	if (!rdev_map)
> +		return -ENOMEM;
> +
> +	/* get mtp_sel0 or mtp_sel1 */
> +	mtp_sel = device_property_read_bool(dev, "richtek,mtp-sel-high");
> +
> +	regmap = devm_regmap_init_i2c(i2c, &rtq2208_regmap_config);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap), "Failed to allocate regmap\n");
> +
> +	/* get needed regulator */
> +	ret = rtq2208_regulator_check(i2c->addr, &n_regulator, regulator_idx_table, buck_masks);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to check used regulators\n");
> +
> +	rdev_map->regmap = regmap;
> +	rdev_map->dev = dev;
> +
> +	cfg.dev = dev;
> +
> +	for (i = 0; i < n_regulator; i++) {
> +		idx = regulator_idx_table[i];
> +
> +		rdesc = devm_kcalloc(dev, 1, sizeof(struct rtq2208_regulator_desc), GFP_KERNEL);
It's the same as devm_kzalloc(dev, sizeof(*rdesc), GFP_KERNEL). More simple?
> +		if (!rdesc)
> +			return -ENOMEM;
> +
> +		/* init regulator desc */
> +		rtq2208_init_regulator_desc(rdesc, mtp_sel, idx);
> +
> +		/* regiser regulator */
> +		rdev = devm_regulator_register(dev, &rdesc->desc, &cfg);
> +		if (IS_ERR(rdev))
> +			return PTR_ERR(rdev);
> +
> +		rdev_map->rdev[idx] = rdev;
> +	}
> +
> +	/* init interrupt mask */
> +	ret = rtq2208_init_irq_mask(rdev_map, buck_masks);
> +	if (ret)
> +		return ret;
> +
> +	/* register interrupt */
> +	ret = devm_request_threaded_irq(dev, i2c->irq, NULL, rtq2208_irq_handler,
> +					IRQF_ONESHOT, dev_name(dev), rdev_map);
> +
> +	return ret;
Why not directly write this as 'return devm_request_threaded_irq .......?
> +}
> +
> +static const struct of_device_id __maybe_unused rtq2208_device_tables[] = {
Remove __maybe_unused. it cannot be unsed.
The below 'of_match_table' will always need it.
> +	{ .compatible = "richtek,rtq2208", },
	                                ~~~
                               this comma can be removed.
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, rtq2208_device_tables);
> +
> +static struct i2c_driver rtq2208_driver = {
> +	.driver = {
> +		.name = "rtq2208",
> +		.of_match_table = rtq2208_device_tables,
> +	},
> +	.probe_new = rtq2208_probe,
> +};
> +module_i2c_driver(rtq2208_driver);
> +
> +MODULE_AUTHOR("Alina Yu <alina_yu@richtek.com>");
> +MODULE_DESCRIPTION("Richtek RTQ2208 Regulator Driver");
> +MODULE_LICENSE("GPL v2");
'GPL' only is fine. In SPDX license, you already said it 'GPL v2+'.
> -- 
> 2.7.4
> 

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

* Re: [PATCH v5 2/2] regulator: rtq2208: Add Richtek RTQ2208 SubPMIC driver
  2023-08-03  7:24   ` ChiYuan Huang
@ 2023-08-03 11:08     ` Mark Brown
  2023-08-07  6:34       ` Alina Yu
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2023-08-03 11:08 UTC (permalink / raw)
  To: ChiYuan Huang
  Cc: Alina Yu, lgirdwood, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-kernel, devicetree

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

On Thu, Aug 03, 2023 at 03:24:25PM +0800, ChiYuan Huang wrote:
> On Wed, Jul 26, 2023 at 03:13:12PM +0800, Alina Yu wrote:

> > Signed-off-by: Alina Yu <alina_yu@richtek.com>
> > Reported-by: kernel test robot <lkp@intel.com>
> This patch series is to add new drivers, not fix.
> Why do you add the Reported-by tag?

The LKP people have some boilerplate in their reports that tells people
to add Reported-bys for things they find based on patches, it's a
constant problem :(

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

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

* Re: [PATCH v5 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC
  2023-08-03  6:36   ` ChiYuan Huang
@ 2023-08-05 21:27     ` Krzysztof Kozlowski
  2023-08-07  6:13       ` Alina Yu
  0 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2023-08-05 21:27 UTC (permalink / raw)
  To: ChiYuan Huang, Alina Yu
  Cc: lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-kernel, devicetree

On 03/08/2023 08:36, ChiYuan Huang wrote:
> On Wed, Jul 26, 2023 at 03:13:11PM +0800, Alina Yu wrote:
>> Add bindings for Richtek RTQ2208 IC controlled SubPMIC
>>
>> Signed-off-by: Alina Yu <alina_yu@richtek.com>


>> +    
>> +  richtek,mtp-sel-high:
>> +    type: boolean
>> +    description:
>> +      vout register selection based on this boolean value.
>> +      false - Using DVS0 register setting to adjust vout
>> +      true - Using DVS1 register setting to adjust vout
>> +
>> +  regulators:
>> +    type: object
> Just curious.
> It seems this PMIC only support buck/ldo ouput.
> Since Krzysztof already reviewed it, not sure whether to group it into one node is common or not.
> AFAIK, if there's no more function included, to put all on the top level may be better.
>> +

Indeed we tend to avoid empty nodes just as a corresponding part to
Linux device.

Here, I don't know Rob's and Conor's opinion, but for cases of having
more than 2 regulators, having "regulators" node makes sense to me -
nicely organizes that piece of DT which can grow quite big. I can also
live without the "regulators" node.

Best regards,
Krzysztof


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

* Re: [PATCH v5 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC
  2023-08-05 21:27     ` Krzysztof Kozlowski
@ 2023-08-07  6:13       ` Alina Yu
  0 siblings, 0 replies; 10+ messages in thread
From: Alina Yu @ 2023-08-07  6:13 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: ChiYuan Huang, lgirdwood, broonie, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, linux-kernel, devicetree,
	alina_yu

Hi
Krzysztof,

On Sat, Aug 05, 2023 at 11:27:00PM +0200, Krzysztof Kozlowski wrote:
> On 03/08/2023 08:36, ChiYuan Huang wrote:
> > On Wed, Jul 26, 2023 at 03:13:11PM +0800, Alina Yu wrote:
> >> Add bindings for Richtek RTQ2208 IC controlled SubPMIC
> >>
> >> Signed-off-by: Alina Yu <alina_yu@richtek.com>
> 
> 
> >> +    
> >> +  richtek,mtp-sel-high:
> >> +    type: boolean
> >> +    description:
> >> +      vout register selection based on this boolean value.
> >> +      false - Using DVS0 register setting to adjust vout
> >> +      true - Using DVS1 register setting to adjust vout
> >> +
> >> +  regulators:
> >> +    type: object
> > Just curious.
> > It seems this PMIC only support buck/ldo ouput.
> > Since Krzysztof already reviewed it, not sure whether to group it into one node is common or not.
> > AFAIK, if there's no more function included, to put all on the top level may be better.
> >> +
> 
> Indeed we tend to avoid empty nodes just as a corresponding part to
> Linux device.
> 
> Here, I don't know Rob's and Conor's opinion, but for cases of having
> more than 2 regulators, having "regulators" node makes sense to me -
> nicely organizes that piece of DT which can grow quite big. I can also
> live without the "regulators" node.
>

Thank you for your kind advice.
I will keep regulator node in v7

Best regards,
Alina

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

* Re: [PATCH v5 2/2] regulator: rtq2208: Add Richtek RTQ2208 SubPMIC driver
  2023-08-03 11:08     ` Mark Brown
@ 2023-08-07  6:34       ` Alina Yu
  0 siblings, 0 replies; 10+ messages in thread
From: Alina Yu @ 2023-08-07  6:34 UTC (permalink / raw)
  To: Mark Brown
  Cc: ChiYuan Huang, lgirdwood, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, linux-kernel, devicetree, alina_yu

Hi
Mark,

On Thu, Aug 03, 2023 at 12:08:28PM +0100, Mark Brown wrote:
> On Thu, Aug 03, 2023 at 03:24:25PM +0800, ChiYuan Huang wrote:
> > On Wed, Jul 26, 2023 at 03:13:12PM +0800, Alina Yu wrote:
> 
> > > Signed-off-by: Alina Yu <alina_yu@richtek.com>
> > > Reported-by: kernel test robot <lkp@intel.com>
> > This patch series is to add new drivers, not fix.
> > Why do you add the Reported-by tag?
> 
> The LKP people have some boilerplate in their reports that tells people
> to add Reported-bys for things they find based on patches, it's a
> constant problem :(

Sorry, I misunerstood the meaning of Reported-by
I've removed that since v6.

Thanks, Alina


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

end of thread, other threads:[~2023-08-07  6:34 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-26  7:13 [PATCH v5 0/2] Add Richtek RTQ2208 SubPMIC support Alina Yu
2023-07-26  7:13 ` [PATCH v5 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC Alina Yu
2023-07-26  9:53   ` Krzysztof Kozlowski
2023-08-03  6:36   ` ChiYuan Huang
2023-08-05 21:27     ` Krzysztof Kozlowski
2023-08-07  6:13       ` Alina Yu
2023-07-26  7:13 ` [PATCH v5 2/2] regulator: rtq2208: Add Richtek RTQ2208 SubPMIC driver Alina Yu
2023-08-03  7:24   ` ChiYuan Huang
2023-08-03 11:08     ` Mark Brown
2023-08-07  6:34       ` Alina Yu

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