[PATCH v4 0/2] Add Richtek RTQ2208 SubPMIC support

[PATCH v4 0/2] Add Richtek RTQ2208 SubPMIC support

[alina_yu]

From: alinayu <alina_yu@richtek.com>

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 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
---
alinayu (2):
  regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC
  regulator: rtq2208: Add Richtek RTQ2208 SubPMIC driver

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

-- 
2.7.4

[PATCH v4 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC

[alina_yu]

From: alinayu <alina_yu@richtek.com>

Add bindings for Richtek RTQ2208 IC controlled SubPMIC

Signed-off-by: Alina Yu <alina_yu@richtek.com>
---
v4
- Modify filename to "richtek,rtq2208"
- Add more desciptions for "regulator-allowed-modes"
---
 .../bindings/regulator/richtek,rtq2208.yaml        | 208 +++++++++++++++++++++
 1 file changed, 208 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..6cc441f
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
@@ -0,0 +1,208 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/richtek,rtq2208-regulator.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:
+    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]
+
+          regulator-mode:
+            enum: [0, 1]
+            description:
+              describe buck initial operating mode in suspend state.
+
+      "^ldo[1-2]$":
+        type: object
+        $ref: regulator.yaml#
+        description:
+          regulator description for ldo[1-2].
+
+        properties:
+          regulator-compatible:
+            pattern: "^LDO[1-2]$"
+
+          richtek,fixed-uV:
+            $ref: "/schemas/types.yaml#/definitions/uint32"
+            enum: [ 900000, 1200000, 1800000, 3300000 ]
+            description:
+              the fixed voltage in micro volt which is decided at the factory.
+
+required:
+  - compatible
+  - reg
+  - regulators
+
+unevaluatedProperties: 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;
+
+        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 {
+            richtek,fixed-uV = <1200000>;
+            regulator-always-on;
+            regulator-state-mem {
+              regulator-on-in-suspend;
+            };
+          };
+         ldo2 {
+            regulator-always-on;
+            richtek,fixed-uV = <3300000>;
+            regulator-state-mem {
+              regulator-on-in-suspend;
+            };
+          };
+        };
+      };
+    };
-- 
2.7.4

[PATCH v4 2/2] regulator: rtq2208: Add Richtek RTQ2208 SubPMIC driver

[alina_yu]

From: alinayu <alina_yu@richtek.com>

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>
---
 drivers/regulator/Kconfig             |  11 +
 drivers/regulator/Makefile            |   1 +
 drivers/regulator/rtq2208-regulator.c | 538 ++++++++++++++++++++++++++++++++++
 3 files changed, 550 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..05bcd8f
--- /dev/null
+++ b/drivers/regulator/rtq2208-regulator.c
@@ -0,0 +1,538 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <linux/bitops.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;
+
+	if (desc->id != RTQ2208_LDO1 && desc->id != RTQ2208_LDO2)
+		return 0;
+
+	/* ldo voltage depends on factory setting */
+	return of_property_read_u32(np, "richtek,fixed-uV", &rdesc->fixed_uV);
+}
+
+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->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");
+
+	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

Re: [PATCH v4 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC

[Krzysztof Kozlowski]

On 19/07/2023 11:24, alina_yu@richtek.com wrote:
> From: alinayu <alina_yu@richtek.com>
> 
> Add bindings for Richtek RTQ2208 IC controlled SubPMIC
> 
> Signed-off-by: Alina Yu <alina_yu@richtek.com>
> ---
> v4
> - Modify filename to "richtek,rtq2208"
> - Add more desciptions for "regulator-allowed-modes"
> ---
>  .../bindings/regulator/richtek,rtq2208.yaml        | 208 +++++++++++++++++++++
>  1 file changed, 208 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..6cc441f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
> @@ -0,0 +1,208 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/richtek,rtq2208-regulator.yaml#

Please test the patch before sending.

It does not look like you tested the bindings, at least after quick
look. Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).

Also, one patchset version per day... give people time to review.


> +$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:
> +    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]
> +
> +          regulator-mode:
> +            enum: [0, 1]
> +            description:
> +              describe buck initial operating mode in suspend state.

There is no such property on this level. Aren't you mixing initial one?

> +
> +      "^ldo[1-2]$":
> +        type: object
> +        $ref: regulator.yaml#

Missing unevaluatedProperties: false.

> +        description:
> +          regulator description for ldo[1-2].
> +
> +        properties:
> +          regulator-compatible:
> +            pattern: "^LDO[1-2]$"
> +
> +          richtek,fixed-uV:
> +            $ref: "/schemas/types.yaml#/definitions/uint32"

This is pointed out by schema, so standard text:
It does not look like you tested the bindings, at least after quick
look. Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.

> +            enum: [ 900000, 1200000, 1800000, 3300000 ]
> +            description:
> +              the fixed voltage in micro volt which is decided at the factory.

I don't understand this property. Why this is different from min/max
microvolt? Plus, you use incorrect unit suffix.

> +
> +required:
> +  - compatible
> +  - reg
> +  - regulators
> +
> +unevaluatedProperties: false

Instead: 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;
> +
> +        regulators {
> +         buck-a {

Wrong indentation. If you use 2 spaces, use it consistently.

> +            regulator-min-microvolt = <400000>;
> +            regulator-max-microvolt = <2050000>;
> +            regulator-allowed-modes = <0 1>;

...

> +          };
> +         ldo2 {
> +            regulator-always-on;

And three spaces here?

> +            richtek,fixed-uV = <3300000>;
> +            regulator-state-mem {
> +              regulator-on-in-suspend;
> +            };
> +          };
> +        };
> +      };
> +    };

Best regards,
Krzysztof

Re: [PATCH v4 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC

[Rob Herring]

On Wed, 19 Jul 2023 17:24:45 +0800, alina_yu@richtek.com wrote:
> From: alinayu <alina_yu@richtek.com>
> 
> Add bindings for Richtek RTQ2208 IC controlled SubPMIC
> 
> Signed-off-by: Alina Yu <alina_yu@richtek.com>
> ---
> v4
> - Modify filename to "richtek,rtq2208"
> - Add more desciptions for "regulator-allowed-modes"
> ---
>  .../bindings/regulator/richtek,rtq2208.yaml        | 208 +++++++++++++++++++++
>  1 file changed, 208 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml: $id: Cannot determine base path from $id, relative path/filename doesn't match actual path or filename
 	 $id: http://devicetree.org/schemas/regulator/richtek,rtq2208-regulator.yaml
 	file: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/1689758686-14409-2-git-send-email-alina_yu@richtek.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v4 2/2] regulator: rtq2208: Add Richtek RTQ2208 SubPMIC driver

[Mark Brown]

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

On Wed, Jul 19, 2023 at 05:24:46PM +0800, alina_yu@richtek.com wrote:
> From: alinayu <alina_yu@richtek.com>
> 

It'd be better to format this like you do in your e-mail and signoff so
that git doesn't generate the extra line and the logs look a bit neater
(there might also be some warnings about signoffs due to the mismatch).
'git commit --amend --author=' should help.

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

The driver looks good, just needs the bindings sorting.

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

Re: [PATCH v4 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC

[Alina Yu]

Hi,
Krzysztof:

On Wed, Jul 19, 2023 at 11:44:45AM +0200, Krzysztof Kozlowski wrote:
> On 19/07/2023 11:24, alina_yu@richtek.com wrote:
> > From: alinayu <alina_yu@richtek.com>
> > 
> > Add bindings for Richtek RTQ2208 IC controlled SubPMIC
> > 
> > Signed-off-by: Alina Yu <alina_yu@richtek.com>
> > ---
> > v4
> > - Modify filename to "richtek,rtq2208"
> > - Add more desciptions for "regulator-allowed-modes"

...

> > +
> > +          regulator-mode:
> > +            enum: [0, 1]
> > +            description:
> > +              describe buck initial operating mode in suspend state.
> 
> There is no such property on this level. Aren't you mixing initial one?

It's the initial mode in suspend-mem state, should I modify that like this ?
        patternProperties:
          "^regulator-state-(standby|mem|disk)$":
	    type: object
	    $ref: regulator.yaml#
	    properties:
	      regulator-mode:
	        enum: [0, 1]
		description:
                  describe byck initial operating mode in suspend state.
...

> 
> > +            enum: [ 900000, 1200000, 1800000, 3300000 ]
> > +            description:
> > +              the fixed voltage in micro volt which is decided at the factory.
> 
> I don't understand this property. Why this is different from min/max


Because ldo has fixed voltage, so I thinks I could use a property to
represent the fixed voltage directly. Do you suggest me modifying that like this:

regulator-min-microvolt = <900000>;
regulator-max-microvolt = <900000>;

Using min voltage equals to max voltage to represent fixed voltage, instead of self-defined property ?


> microvolt? Plus, you use incorrect unit suffix.

if I change "richtek,fixed-uV" to "richtek, fixed-microvolt",
will it be a correct unit suffix ?

Best regards,
Alina

Re: [PATCH v4 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC

[Krzysztof Kozlowski]

On 20/07/2023 10:07, Alina Yu wrote:
> Hi,
> Krzysztof:
> 
> On Wed, Jul 19, 2023 at 11:44:45AM +0200, Krzysztof Kozlowski wrote:
>> On 19/07/2023 11:24, alina_yu@richtek.com wrote:
>>> From: alinayu <alina_yu@richtek.com>
>>>
>>> Add bindings for Richtek RTQ2208 IC controlled SubPMIC
>>>
>>> Signed-off-by: Alina Yu <alina_yu@richtek.com>
>>> ---
>>> v4
>>> - Modify filename to "richtek,rtq2208"
>>> - Add more desciptions for "regulator-allowed-modes"
> 
> ...
> 
>>> +
>>> +          regulator-mode:
>>> +            enum: [0, 1]
>>> +            description:
>>> +              describe buck initial operating mode in suspend state.
>>
>> There is no such property on this level. Aren't you mixing initial one?
> 
> It's the initial mode in suspend-mem state, should I modify that like this ?
>         patternProperties:
>           "^regulator-state-(standby|mem|disk)$":
> 	    type: object
> 	    $ref: regulator.yaml#
> 	    properties:
> 	      regulator-mode:
> 	        enum: [0, 1]
> 		description:
>                   describe byck initial operating mode in suspend state.

Please check how other bindings do it.

> ...
> 
>>
>>> +            enum: [ 900000, 1200000, 1800000, 3300000 ]
>>> +            description:
>>> +              the fixed voltage in micro volt which is decided at the factory.
>>
>> I don't understand this property. Why this is different from min/max
> 
> 
> Because ldo has fixed voltage, so I thinks I could use a property to
> represent the fixed voltage directly. Do you suggest me modifying that like this:
> 
> regulator-min-microvolt = <900000>;
> regulator-max-microvolt = <900000>;
> 
> Using min voltage equals to max voltage to represent fixed voltage, instead of self-defined property ?

Yes.



Best regards,
Krzysztof

Re: [PATCH v4 2/2] regulator: rtq2208: Add Richtek RTQ2208 SubPMIC driver

[kernel test robot]

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on broonie-regulator/for-next]
[also build test ERROR on linus/master v6.5-rc2 next-20230720]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/alina_yu-richtek-com/regulator-dt-bindings-rtq2208-Add-Richtek-RTQ2208-SubPMIC/20230719-172722
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
patch link:    https://lore.kernel.org/r/1689758686-14409-3-git-send-email-alina_yu%40richtek.com
patch subject: [PATCH v4 2/2] regulator: rtq2208: Add Richtek RTQ2208 SubPMIC driver
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20230721/202307210139.6iUpmzwe-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230721/202307210139.6iUpmzwe-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307210139.6iUpmzwe-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/regulator/rtq2208-regulator.c: In function 'rtq2208_set_ramp_delay':
>> drivers/regulator/rtq2208-regulator.c:154:15: error: implicit declaration of function 'FIELD_PREP' [-Werror=implicit-function-declaration]
     154 |         val = FIELD_PREP(RTQ2208_BUCK_RSPUP_MASK, sel) | FIELD_PREP(RTQ2208_BUCK_RSPDN_MASK, sel);
         |               ^~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/FIELD_PREP +154 drivers/regulator/rtq2208-regulator.c

   124	
   125	static int rtq2208_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
   126	{
   127		const struct regulator_desc *rdesc = rdev->desc;
   128		unsigned int sel = 0, val;
   129	
   130		ramp_delay = max(ramp_delay, RTQ2208_RAMP_VALUE_MIN_uV);
   131		ramp_delay = min(ramp_delay, RTQ2208_RAMP_VALUE_MAX_uV);
   132	
   133		ramp_delay /= RTQ2208_RAMP_VALUE_MIN_uV;
   134	
   135		/*
   136		 * fls(ramp_delay) - 1: doing LSB shift, let it starts from 0
   137		 *
   138		 * RTQ2208_BUCK_RAMP_SEL_MASK - sel: doing descending order shifting.
   139		 * Because the relation of seleltion and value is like that
   140		 *
   141		 * seletion: value
   142		 * 000: 64mv
   143		 * 001: 32mv
   144		 * ...
   145		 * 111: 0.5mv
   146		 *
   147		 * For example, if I would like to select 64mv, the fls(ramp_delay) - 1 will be 0b111,
   148		 * and I need to use 0b111 - sel to do the shifting
   149		 */
   150	
   151		sel = fls(ramp_delay) - 1;
   152		sel = RTQ2208_BUCK_RAMP_SEL_MASK - sel;
   153	
 > 154		val = FIELD_PREP(RTQ2208_BUCK_RSPUP_MASK, sel) | FIELD_PREP(RTQ2208_BUCK_RSPDN_MASK, sel);
   155	
   156		return regmap_update_bits(rdev->regmap, rdesc->ramp_reg,
   157					  RTQ2208_BUCK_RSPUP_MASK | RTQ2208_BUCK_RSPDN_MASK, val);
   158	}
   159	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC

[Alina Yu]

Hi,
Krzysztof:

> > ...
> > 
> >>> +
> >>> +          regulator-mode:
> >>> +            enum: [0, 1]
> >>> +            description:
> >>> +              describe buck initial operating mode in suspend state.
> >>
> >> There is no such property on this level. Aren't you mixing initial one?
> > 
> > It's the initial mode in suspend-mem state, should I modify that like this ?
> >         patternProperties:
> >           "^regulator-state-(standby|mem|disk)$":
> > 	    type: object
> > 	    $ref: regulator.yaml#
> > 	    properties:
> > 	      regulator-mode:
> > 	        enum: [0, 1]
> > 		description:
> >                   describe byck initial operating mode in suspend state.
> 
> Please check how other bindings do it.
>

If I modify that like this, will it be correct ?


...
          regulator-state-mem:
	    type: object
	    $ref: regulator.yaml#
	    properties:
	      regulator-mode:
	      description:
	        describe buck initial operating mode in suspend state.
		0 - Auto mode
		1 - FCCM

Best regards,
Alina

Re: [PATCH v4 1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC

[Alina Yu]

Hi,
Krzysztof:

> > > ...
> > > 
> > >>> +
> > >>> +          regulator-mode:
> > >>> +            enum: [0, 1]
> > >>> +            description:
> > >>> +              describe buck initial operating mode in suspend state.
> > >>
> > >> There is no such property on this level. Aren't you mixing initial one?
> > > 
> > > It's the initial mode in suspend-mem state, should I modify that like this ?
> > >         patternProperties:
> > >           "^regulator-state-(standby|mem|disk)$":
> > > 	    type: object
> > > 	    $ref: regulator.yaml#
> > > 	    properties:
> > > 	      regulator-mode:
> > > 	        enum: [0, 1]
> > > 		description:
> > >                   describe byck initial operating mode in suspend state.
> > 
> > Please check how other bindings do it.
> >
> 
> If I modify that like this, will it be correct ?
> 
> 
> ...
>           regulator-state-mem:
> 	    type: object
> 	    $ref: regulator.yaml#
> 	    properties:
> 	      regulator-mode:
> 	      description:
> 	        describe buck initial operating mode in suspend state.
> 		0 - Auto mode
> 		1 - FCCM
>

Sorry, I think I didn't explain well why to add "regulator-mode".
I just want to add description for this property, for people can get its meaning when they see the yaml.
If it's optional and it's already a general property in regulator.yaml.
May I remove it from my yaml ?

Best regards,
Alina