[PATCH v4 1/2] dt-bindings: regulator: Add Richtek RTR5133 Support

[PATCH v4 1/2] dt-bindings: regulator: Add Richtek RTR5133 Support

[jeff_chang]

From: Jeff Chang <jeff_chang@richtek.com>

Add bindings for Richtek RT5133 IC Controlled PMIC

Signed-off-by: Jeff Chang <jeff_chang@richtek.com>
---

PATCH v4
1. Add commit message and also /script/checkpatch --strict to fix warning.
2. Using subject prefixes matching dt-binding subsystem.
3. Re-order patches. DT patch before driver patch.
4. Fix description of yaml.
5. Add more description for base regulator.
6. Drop regulator-compatible proeprty.
7. Add prefix for vendor property richtek,oc-shutdown-all and richtek,pgb-shutdown-all.
8. Add more description for shutdown-all property.
9. Interrupts-extended -> interrupts.
10. pio->gpio for proper defines.
11. Drop unused labels. Keep rt5133_ldo1 label for ldo7 and ldo8.

 .../bindings/regulator/richtek,rt5133.yaml    | 175 ++++++++++++++++++
 1 file changed, 175 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rt5133.yaml

diff --git a/Documentation/devicetree/bindings/regulator/richtek,rt5133.yaml b/Documentation/devicetree/bindings/regulator/richtek,rt5133.yaml
new file mode 100644
index 000000000000..0da725596a87
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/richtek,rt5133.yaml
@@ -0,0 +1,175 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/richtek,rt5133.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Richtek RT5133 PMIC Regulator
+
+maintainers:
+  - ShihChia Chang <jeff_chang@richtek.com>
+
+description:
+  The RT5133 is an integrated Power Management IC for portable devices, featuring
+  8 LDOs and 3 GPOs. It allows programmable output voltages, soft-start times,
+  and protections via I2C. GPO operation depends on LDO1 voltage.
+
+properties:
+  compatible:
+    enum:
+      - richtek,rt5133
+
+  reg:
+    maxItems: 1
+
+  enable-gpios:
+    maxItems: 1
+
+  wakeup-source: true
+
+  interrupts:
+    maxItems: 1
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  regulators:
+    type: object
+    additionalProperties: false
+
+    properties:
+      base:
+        type: object
+        $ref: regulator.yaml#
+        unevaluatedProperties: false
+        description:
+          Properties for base regulator which control force-off base circuit.
+          Base circuit is the power source for LDO1~LDO6. Disabling it will
+          reduce IQ for Chip.
+
+        properties:
+          richtek,oc-shutdown-all:
+            type: boolean
+            description:
+              Anyone of LDO is in OC state, shut down all channels to protect CHIP.
+              Without this property, only shut down the OC LDO channel.
+
+          richtek,pgb-shutdown-all:
+            type: boolean
+            description:
+              Anyone of LDO is in PGB state, shut down all channels to protect CHIP.
+              Without this property, only shut down the PGB LDO channel.
+
+        required:
+          - regulator-name
+
+    patternProperties:
+      "^ldo([1-6])$":
+        type: object
+        $ref: regulator.yaml#
+        unevaluatedProperties: false
+        description:
+          Properties for single LDO regulator
+
+        required:
+          - regulator-name
+
+      "^ldo([7-8])$":
+        type: object
+        $ref: regulator.yaml#
+        unevaluatedProperties: false
+        description:
+          Properties for single LDO regulator
+
+        properties:
+          rt5133-ldo1-supply:
+            description: |
+              Only for ldo7 ldo8, pvin7 and pvin8 reference design are RT5133 ldo1.
+              If not connect to ldo1 vout, this property for pvin7 and pvin8 is necessary.
+
+        required:
+          - regulator-name
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - wakeup-source
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      rt5133@18 {
+        compatible = "richtek,rt5133";
+        reg = <0x18>;
+        wakeup-source;
+        interrupts-extended = <&gpio 187 0x0>;
+        enable-gpios = <&gpio 186 0x0>;
+        gpio-controller;
+        #gpio-cells = <2>;
+        regulators {
+          base {
+            regulator-name = "rt5133,base";
+            richtek,oc-shutdown-all;
+            richtek,pgb-shutdown-all;
+          };
+          rt5133_ldo1: ldo1 {
+            regulator-name = "rt5133-ldo1";
+            regulator-min-microvolt = <1800000>;
+            regulator-max-microvolt = <3199998>;
+            regulator-active-discharge = <1>;
+          };
+          ldo2 {
+            regulator-name = "rt5133-ldo2";
+            regulator-min-microvolt = <1800000>;
+            regulator-max-microvolt = <3200000>;
+            regulator-active-discharge = <1>;
+          };
+          ldo3 {
+            regulator-name = "rt5133-ldo3";
+            regulator-min-microvolt = <1700000>;
+            regulator-max-microvolt = <3000000>;
+            regulator-active-discharge = <1>;
+          };
+          ldo4 {
+            regulator-name = "rt5133-ldo4";
+            regulator-min-microvolt = <1700000>;
+            regulator-max-microvolt = <3000000>;
+            regulator-active-discharge = <1>;
+          };
+          ldo5 {
+            regulator-name = "rt5133-ldo5";
+            regulator-min-microvolt = <1700000>;
+            regulator-max-microvolt = <3000000>;
+            regulator-active-discharge = <1>;
+          };
+          ldo6 {
+            regulator-name = "rt5133-ldo6";
+            regulator-min-microvolt = <1700000>;
+            regulator-max-microvolt = <3000000>;
+            regulator-active-discharge = <1>;
+          };
+          ldo7 {
+            regulator-name = "rt5133-ldo7";
+            regulator-min-microvolt = <900000>;
+            regulator-max-microvolt = <1200000>;
+            regulator-active-discharge = <1>;
+            rt5133-ldo1-supply = <&rt5133_ldo1>;
+          };
+          ldo8 {
+            regulator-name = "rt5133-ldo8";
+            regulator-min-microvolt = <855000>;
+            regulator-max-microvolt = <1200000>;
+            regulator-active-discharge = <1>;
+            rt5133-ldo1-supply = <&rt5133_ldo1>;
+          };
+        };
+      };
+    };
--
2.43.0

[PATCH v4 2/2] regulator: rt5133: Add RT5133 PMIC regulator Support

[jeff_chang]

From: Jeff Chang <jeff_chang@richtek.com>

RT5133 is a highly-integrated chip. It includes 8 LDOs and 3 GPOs that can
be used to drive output high/low purpose. The dependency of the GPO block is
internally LDO1 Voltage.

Signed-off-by: Jeff Chang <jeff_chang@richtek.com>
---

PATCH v4
1. Re-order patches. DT patch before driver patch.
2. Return IRQ_NONE if rt5133_intr_handler handle nothing.
3. Using cache for regmap_config.
4. Add prefix for vendor property richtek,oc-shutdown-all and richtek,pgb-shutdown-all.
5. Add _node_name to RT5133_REGULATOR_DESC for lowercase regulator node.

 drivers/regulator/Kconfig            |  12 +
 drivers/regulator/Makefile           |   1 +
 drivers/regulator/rt5133-regulator.c | 649 +++++++++++++++++++++++++++
 3 files changed, 662 insertions(+)
 create mode 100644 drivers/regulator/rt5133-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 6d8988387da4..bada46e86cd0 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1229,6 +1229,18 @@ config REGULATOR_RT5120
 	  600mV to 1395mV, per step 6.250mV. The others are all fixed voltage
 	  by external hardware circuit.

+config REGULATOR_RT5133
+	tristate "Richtek RT5133 PMIC Regulators"
+	depends on I2C && GPIOLIB && OF
+	select REGMAP
+	select CRC8
+	select OF_GPIO
+	help
+	  This driver adds support for RT5133 PMIC regulators.
+	  RT5133 is an integrated chip. It includes 8 LDOs and 3 GPOs that
+	  can be used to drive output high/low purpose. The dependency of the
+	  GPO block is internally LDO1 Voltage.
+
 config REGULATOR_RT5190A
 	tristate "Richtek RT5190A PMIC"
 	depends on I2C
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index c0bc7a0f4e67..709384ae0a0c 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -145,6 +145,7 @@ obj-$(CONFIG_REGULATOR_RT4803)	+= rt4803.o
 obj-$(CONFIG_REGULATOR_RT4831)	+= rt4831-regulator.o
 obj-$(CONFIG_REGULATOR_RT5033)	+= rt5033-regulator.o
 obj-$(CONFIG_REGULATOR_RT5120)	+= rt5120-regulator.o
+obj-$(CONFIG_REGULATOR_RT5133)	+= rt5133-regulator.o
 obj-$(CONFIG_REGULATOR_RT5190A) += rt5190a-regulator.o
 obj-$(CONFIG_REGULATOR_RT5739)	+= rt5739.o
 obj-$(CONFIG_REGULATOR_RT5759)	+= rt5759-regulator.o
diff --git a/drivers/regulator/rt5133-regulator.c b/drivers/regulator/rt5133-regulator.c
new file mode 100644
index 000000000000..48c09682c01a
--- /dev/null
+++ b/drivers/regulator/rt5133-regulator.c
@@ -0,0 +1,649 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2025 Richtek Technology Corp.
+// Author: ChiYuan Huang <cy_huang@richtek.com>
+// Author: ShihChia Chang <jeff_chang@richtek.com>
+
+#include <linux/crc8.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+
+#define RT5133_REG_CHIP_INFO		0x00
+#define RT5133_REG_RST_CTRL		0x06
+#define RT5133_REG_BASE_CTRL		0x09
+#define RT5133_REG_GPIO_CTRL		0x0B
+#define RT5133_REG_BASE_EVT		0x10
+#define RT5133_REG_LDO_PGB_STAT		0x15
+#define RT5133_REG_BASE_MASK		0x16
+#define RT5133_REG_LDO_SHDN		0x19
+#define RT5133_REG_LDO_ON		0x1A
+#define RT5133_REG_LDO_OFF		0x1B
+#define RT5133_REG_LDO1_CTRL1		0x20
+#define RT5133_REG_LDO1_CTRL2		0x21
+#define RT5133_REG_LDO1_CTRL3		0x22
+#define RT5133_REG_LDO2_CTRL1		0x24
+#define RT5133_REG_LDO2_CTRL2		0x25
+#define RT5133_REG_LDO2_CTRL3		0x26
+#define RT5133_REG_LDO3_CTRL1		0x28
+#define RT5133_REG_LDO3_CTRL2		0x29
+#define RT5133_REG_LDO3_CTRL3		0x2A
+#define RT5133_REG_LDO4_CTRL1		0x2C
+#define RT5133_REG_LDO4_CTRL2		0x2D
+#define RT5133_REG_LDO4_CTRL3		0x2E
+#define RT5133_REG_LDO5_CTRL1		0x30
+#define RT5133_REG_LDO5_CTRL2		0x31
+#define RT5133_REG_LDO5_CTRL3		0x32
+#define RT5133_REG_LDO6_CTRL1		0x34
+#define RT5133_REG_LDO6_CTRL2		0x35
+#define RT5133_REG_LDO6_CTRL3		0x36
+#define RT5133_REG_LDO7_CTRL1		0x38
+#define RT5133_REG_LDO7_CTRL2		0x39
+#define RT5133_REG_LDO7_CTRL3		0x3A
+#define RT5133_REG_LDO8_CTRL1		0x3C
+#define RT5133_REG_LDO8_CTRL2		0x3D
+#define RT5133_REG_LDO8_CTRL3		0x3E
+#define RT5133_REG_LDO8_CTRL4		0x3F
+
+#define RT5133_LDO_REG_BASE(_id)	(0x20 + ((_id) - 1) * 4)
+
+#define RT5133_VENDOR_ID_MASK		GENMASK(7, 4)
+#define RT5133_RESET_CODE		0xB1
+
+#define RT5133_FOFF_BASE_MASK		BIT(1)
+#define RT5133_OCSHDN_ALL_MASK		BIT(7)
+#define RT5133_OCSHDN_ALL_SHIFT		(7)
+#define RT5133_PGBSHDN_ALL_MASK		BIT(6)
+#define RT5133_PGBSHDN_ALL_SHIFT	(6)
+
+#define RT5133_OCPTSEL_MASK		BIT(5)
+#define RT5133_PGBPTSEL_MASK		BIT(4)
+#define RT5133_STBTDSEL_MASK		GENMASK(1, 0)
+
+#define RT5133_LDO_ENABLE_MASK		BIT(7)
+#define RT5133_LDO_VSEL_MASK		GENMASK(7, 5)
+#define RT5133_LDO_AD_MASK		BIT(2)
+#define RT5133_LDO_SOFT_START_MASK	GENMASK(1, 0)
+
+#define RT5133_GPIO_NR			3
+
+#define RT5133_LDO_PGB_EVT_MASK		GENMASK(23, 16)
+#define RT5133_LDO_PGB_EVT_SHIFT	16
+#define RT5133_LDO_OC_EVT_MASK		GENMASK(15, 8)
+#define RT5133_LDO_OC_EVT_SHIFT		8
+#define RT5133_VREF_EVT_MASK		BIT(6)
+#define RT5133_BASE_EVT_MASK		GENMASK(7, 0)
+#define RT5133_INTR_CLR_MASK		GENMASK(23, 0)
+#define RT5133_INTR_BYTE_NR		3
+
+#define RT5133_MAX_I2C_BLOCK_SIZE	1
+
+#define RT5133_CRC8_POLYNOMIAL		0x7
+
+#define RT5133_I2C_ADDR_LEN		1
+#define RT5133_PREDATA_LEN		2
+#define RT5133_I2C_CRC_LEN		1
+#define RT5133_REG_ADDR_LEN		1
+#define RT5133_I2C_DUMMY_LEN		1
+
+#define I2C_ADDR_XLATE_8BIT(_addr, _rw)	((((_addr) & 0x7F) << 1) | (_rw))
+
+enum {
+	RT5133_REGULATOR_BASE = 0,
+	RT5133_REGULATOR_LDO1,
+	RT5133_REGULATOR_LDO2,
+	RT5133_REGULATOR_LDO3,
+	RT5133_REGULATOR_LDO4,
+	RT5133_REGULATOR_LDO5,
+	RT5133_REGULATOR_LDO6,
+	RT5133_REGULATOR_LDO7,
+	RT5133_REGULATOR_LDO8,
+	RT5133_REGULATOR_MAX
+};
+
+struct chip_data {
+	const struct regulator_desc *regulators;
+	const u8 vendor_id;
+};
+
+struct rt5133_priv {
+	struct device *dev;
+	struct regmap *regmap;
+	struct gpio_desc *enable_gpio;
+	struct regulator_dev *rdev[RT5133_REGULATOR_MAX];
+	struct gpio_chip gc;
+	const struct chip_data *cdata;
+	unsigned int gpio_output_flag;
+	u8 crc8_tbls[CRC8_TABLE_SIZE];
+};
+
+static const unsigned int vout_type1_tables[] = {
+	1800000, 2500000, 2700000, 2800000, 2900000, 3000000, 3100000, 3200000
+};
+
+static const unsigned int vout_type2_tables[] = {
+	1700000, 1800000, 1900000, 2500000, 2700000, 2800000, 2900000, 3000000
+};
+
+static const unsigned int vout_type3_tables[] = {
+	900000, 950000, 1000000, 1050000, 1100000, 1150000, 1200000, 1800000
+};
+
+static const unsigned int vout_type4_tables[] = {
+	855000, 900000, 950000, 1000000, 1040000, 1090000, 1140000, 1710000
+};
+
+static const struct regulator_ops rt5133_regulator_ops = {
+	.list_voltage = regulator_list_voltage_table,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.set_active_discharge = regulator_set_active_discharge_regmap,
+};
+
+static const struct regulator_ops rt5133_base_regulator_ops = {
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+};
+
+static int rt5133_of_parse_cb(struct device_node *node,
+			      const struct regulator_desc *desc,
+			      struct regulator_config *config)
+{
+	struct rt5133_priv *priv = config->driver_data;
+	unsigned int val = 0;
+	int ret = 0;
+
+	switch (desc->id) {
+	case RT5133_REGULATOR_BASE:
+		if (!of_property_read_bool(node, "richtek,oc-shutdown-all"))
+			val = 0;
+		else
+			val = 1 << RT5133_OCSHDN_ALL_SHIFT;
+		ret = regmap_update_bits(priv->regmap, RT5133_REG_LDO_SHDN,
+					 RT5133_OCSHDN_ALL_MASK, val);
+		if (ret)
+			return ret;
+
+		if (!of_property_read_bool(node, "richtek,pgb-shutdown-all"))
+			val = 0;
+		else
+			val = 1 << RT5133_PGBSHDN_ALL_SHIFT;
+		return regmap_update_bits(priv->regmap, RT5133_REG_LDO_SHDN,
+					  RT5133_PGBSHDN_ALL_MASK, val);
+	default:
+		break;
+	};
+
+	return 0;
+}
+
+#define RT5133_REGULATOR_DESC(_name, _node_name, vtables, _supply) \
+{\
+	.name = #_name,\
+	.id = RT5133_REGULATOR_##_name,\
+	.of_match = of_match_ptr(#_node_name),\
+	.regulators_node = of_match_ptr("regulators"),\
+	.supply_name = _supply,\
+	.of_parse_cb = rt5133_of_parse_cb,\
+	.type = REGULATOR_VOLTAGE,\
+	.owner = THIS_MODULE,\
+	.ops = &rt5133_regulator_ops,\
+	.n_voltages = ARRAY_SIZE(vtables),\
+	.volt_table = vtables,\
+	.enable_reg = RT5133_REG_##_name##_CTRL1,\
+	.enable_mask = RT5133_LDO_ENABLE_MASK,\
+	.vsel_reg = RT5133_REG_##_name##_CTRL2,\
+	.vsel_mask = RT5133_LDO_VSEL_MASK,\
+	.active_discharge_reg = RT5133_REG_##_name##_CTRL3,\
+	.active_discharge_mask = RT5133_LDO_AD_MASK,\
+}
+
+static const struct regulator_desc rt5133_regulators[] = {
+	/* For digital part, base current control */
+	{
+		.name = "rt5133,base",
+		.id = RT5133_REGULATOR_BASE,
+		.of_match = of_match_ptr("base"),
+		.regulators_node = of_match_ptr("regulators"),
+		.of_parse_cb = rt5133_of_parse_cb,
+		.type = REGULATOR_VOLTAGE,
+		.owner = THIS_MODULE,
+		.ops = &rt5133_base_regulator_ops,
+		.enable_reg = RT5133_REG_BASE_CTRL,
+		.enable_mask = RT5133_FOFF_BASE_MASK,
+		.enable_is_inverted = true,
+	},
+	RT5133_REGULATOR_DESC(LDO1, ldo1, vout_type1_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO2, ldo2, vout_type1_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO3, ldo3, vout_type2_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO4, ldo4, vout_type2_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO5, ldo5, vout_type2_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO6, ldo6, vout_type2_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO7, ldo7, vout_type3_tables, "rt5133-ldo1"),
+	RT5133_REGULATOR_DESC(LDO8, ldo8, vout_type3_tables, "rt5133-ldo1"),
+};
+
+static const struct regulator_desc rt5133a_regulators[] = {
+	/* For digital part, base current control */
+	{
+		.name = "rt5133,base",
+		.id = RT5133_REGULATOR_BASE,
+		.of_match = of_match_ptr("base"),
+		.regulators_node = of_match_ptr("regulators"),
+		.of_parse_cb = rt5133_of_parse_cb,
+		.type = REGULATOR_VOLTAGE,
+		.owner = THIS_MODULE,
+		.ops = &rt5133_base_regulator_ops,
+		.enable_reg = RT5133_REG_BASE_CTRL,
+		.enable_mask = RT5133_FOFF_BASE_MASK,
+		.enable_is_inverted = true,
+	},
+	RT5133_REGULATOR_DESC(LDO1, ldo1, vout_type1_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO2, ldo2, vout_type1_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO3, ldo3, vout_type2_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO4, ldo4, vout_type2_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO5, ldo5, vout_type2_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO6, ldo6, vout_type2_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO7, ldo7, vout_type3_tables, "rt5133-ldo1"),
+	RT5133_REGULATOR_DESC(LDO8, ldo8, vout_type4_tables, "rt5133-ldo1"),
+};
+
+static const struct chip_data regulator_data[] = {
+	{ rt5133_regulators, 0x70},
+	{ rt5133a_regulators, 0x80},
+};
+
+static int rt5133_gpio_direction_output(struct gpio_chip *gpio,
+					unsigned int offset, int value)
+{
+	struct rt5133_priv *priv = gpiochip_get_data(gpio);
+
+	if (offset >= RT5133_GPIO_NR)
+		return -EINVAL;
+
+	return regmap_update_bits(priv->regmap, RT5133_REG_GPIO_CTRL,
+				  BIT(7 - offset) | BIT(3 - offset),
+				  value ? BIT(7 - offset) | BIT(3 - offset) : 0);
+}
+
+static int rt5133_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct rt5133_priv *priv = gpiochip_get_data(chip);
+
+	return !!(priv->gpio_output_flag & BIT(offset));
+}
+
+static int rt5133_get_gpioen_mask(unsigned int offset, unsigned int *mask)
+{
+	if (offset >= RT5133_GPIO_NR)
+		return -EINVAL;
+
+	*mask = (BIT(7 - offset) | BIT(3 - offset));
+
+	return 0;
+}
+
+static void rt5133_gpio_set(struct gpio_chip *chip, unsigned int offset,
+			    int set_val)
+{
+	struct rt5133_priv *priv = gpiochip_get_data(chip);
+	unsigned int mask = 0, val = 0, next_flag = priv->gpio_output_flag;
+	int ret = 0;
+
+	ret = rt5133_get_gpioen_mask(offset, &mask);
+	if (ret) {
+		dev_err(priv->dev, "%s get gpion en mask failed, offset(%d)\n", __func__, offset);
+		return;
+	}
+
+	val = set_val ? mask : 0;
+
+	if (set_val)
+		next_flag |= BIT(offset);
+	else
+		next_flag &= ~BIT(offset);
+
+	ret = regmap_update_bits(priv->regmap, RT5133_REG_GPIO_CTRL, mask, val);
+	if (ret) {
+		dev_err(priv->dev, "Failed to set gpio [%d] val %d\n", offset,
+			set_val);
+		return;
+	}
+
+	priv->gpio_output_flag = next_flag;
+}
+
+static irqreturn_t rt5133_intr_handler(int irq_number, void *data)
+{
+	struct rt5133_priv *priv = data;
+	u32 intr_evts = 0, handle_evts;
+	int i, ret;
+
+	ret = regmap_bulk_read(priv->regmap, RT5133_REG_BASE_EVT, &intr_evts,
+			       RT5133_INTR_BYTE_NR);
+	if (ret) {
+		dev_err(priv->dev, "%s, read event failed\n", __func__);
+		return IRQ_NONE;
+	}
+
+	handle_evts = intr_evts & RT5133_BASE_EVT_MASK;
+	/*
+	 * VREF_EVT is a special case, if base off
+	 * this event will also be trigger. Skip it
+	 */
+	if (handle_evts & ~RT5133_VREF_EVT_MASK)
+		dev_dbg(priv->dev, "base event occurred [0x%02x]\n",
+			handle_evts);
+
+	handle_evts = (intr_evts & RT5133_LDO_OC_EVT_MASK) >>
+		RT5133_LDO_OC_EVT_SHIFT;
+
+	for (i = RT5133_REGULATOR_LDO1; i < RT5133_REGULATOR_MAX && handle_evts; i++) {
+		if (!(handle_evts & BIT(i - 1)))
+			continue;
+		regulator_notifier_call_chain(priv->rdev[i],
+					      REGULATOR_EVENT_OVER_CURRENT,
+					      &i);
+	}
+
+	handle_evts = (intr_evts & RT5133_LDO_PGB_EVT_MASK) >>
+		RT5133_LDO_PGB_EVT_SHIFT;
+	for (i = RT5133_REGULATOR_LDO1; i < RT5133_REGULATOR_MAX && handle_evts; i++) {
+		if (!(handle_evts & BIT(i - 1)))
+			continue;
+		regulator_notifier_call_chain(priv->rdev[i],
+					      REGULATOR_EVENT_FAIL, &i);
+	}
+
+	ret = regmap_bulk_write(priv->regmap, RT5133_REG_BASE_EVT, &intr_evts,
+				RT5133_INTR_BYTE_NR);
+	if (ret)
+		dev_err(priv->dev, "%s, clear event failed\n", __func__);
+
+	return IRQ_HANDLED;
+}
+
+static int rt5133_enable_interrupts(int irq_no, struct rt5133_priv *priv)
+{
+	u32 mask = RT5133_INTR_CLR_MASK;
+	int ret;
+
+	/* Force to write clear all events */
+	ret = regmap_bulk_write(priv->regmap, RT5133_REG_BASE_EVT, &mask,
+				RT5133_INTR_BYTE_NR);
+	if (ret) {
+		dev_err(priv->dev, "Failed to clear all interrupts\n");
+		return ret;
+	}
+
+	/* Unmask all interrupts */
+	mask = 0;
+	ret = regmap_bulk_write(priv->regmap, RT5133_REG_BASE_MASK, &mask,
+				RT5133_INTR_BYTE_NR);
+	if (ret) {
+		dev_err(priv->dev, "Failed to unmask all interrupts\n");
+		return ret;
+	}
+
+	return devm_request_threaded_irq(priv->dev, irq_no, NULL,
+					 rt5133_intr_handler, IRQF_ONESHOT,
+					 dev_name(priv->dev), priv);
+}
+
+static int rt5133_regmap_hw_read(void *context, const void *reg_buf,
+				 size_t reg_size, void *val_buf,
+				 size_t val_size)
+{
+	struct rt5133_priv *priv = context;
+	struct i2c_client *client = to_i2c_client(priv->dev);
+	u8 reg = *(u8 *)reg_buf, crc;
+	u8 *buf;
+	int buf_len = RT5133_PREDATA_LEN + val_size + RT5133_I2C_CRC_LEN;
+	int read_len, ret;
+
+	buf = kzalloc(buf_len, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	buf[0] = I2C_ADDR_XLATE_8BIT(client->addr, I2C_SMBUS_READ);
+	buf[1] = reg;
+
+	read_len = val_size + RT5133_I2C_CRC_LEN;
+	ret = i2c_smbus_read_i2c_block_data(client, reg, read_len,
+					    buf + RT5133_PREDATA_LEN);
+
+	if (ret < 0)
+		goto out_read_err;
+
+	if (ret != read_len) {
+		ret = -EIO;
+		goto out_read_err;
+	}
+
+	crc = crc8(priv->crc8_tbls, buf, RT5133_PREDATA_LEN + val_size, 0);
+	if (crc != buf[RT5133_PREDATA_LEN + val_size]) {
+		ret = -EIO;
+		goto out_read_err;
+	}
+
+	memcpy(val_buf, buf + RT5133_PREDATA_LEN, val_size);
+	dev_dbg(priv->dev, "%s, reg = 0x%02x, data = 0x%02x\n", __func__, reg, *(u8 *)val_buf);
+
+out_read_err:
+	kfree(buf);
+	return (ret < 0) ? ret : 0;
+}
+
+static int rt5133_regmap_hw_write(void *context, const void *data, size_t count)
+{
+	struct rt5133_priv *priv = context;
+	struct i2c_client *client = to_i2c_client(priv->dev);
+	u8 reg = *(u8 *)data, crc;
+	u8 *buf;
+	int buf_len = RT5133_I2C_ADDR_LEN + count + RT5133_I2C_CRC_LEN +
+		RT5133_I2C_DUMMY_LEN;
+	int write_len, ret;
+
+	buf = kzalloc(buf_len, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	buf[0] = I2C_ADDR_XLATE_8BIT(client->addr, I2C_SMBUS_WRITE);
+	buf[1] = reg;
+	memcpy(buf + RT5133_PREDATA_LEN, data + RT5133_REG_ADDR_LEN,
+	       count - RT5133_REG_ADDR_LEN);
+
+	crc = crc8(priv->crc8_tbls, buf, RT5133_I2C_ADDR_LEN + count, 0);
+	buf[RT5133_I2C_ADDR_LEN + count] = crc;
+
+	write_len = count - RT5133_REG_ADDR_LEN + RT5133_I2C_CRC_LEN +
+		RT5133_I2C_DUMMY_LEN;
+	ret = i2c_smbus_write_i2c_block_data(client, reg, write_len,
+					     buf + RT5133_PREDATA_LEN);
+
+	dev_dbg(priv->dev, "%s, reg = 0x%02x, data = 0x%02x\n", __func__, reg,
+		*(u8 *)(buf + RT5133_PREDATA_LEN));
+	kfree(buf);
+	return ret;
+}
+
+static const struct regmap_bus rt5133_regmap_bus = {
+	.read = rt5133_regmap_hw_read,
+	.write = rt5133_regmap_hw_write,
+	/* Due to crc, the block read/write length has the limit */
+	.max_raw_read = RT5133_MAX_I2C_BLOCK_SIZE,
+	.max_raw_write = RT5133_MAX_I2C_BLOCK_SIZE,
+};
+
+static bool rt5133_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case RT5133_REG_CHIP_INFO:
+	case RT5133_REG_BASE_EVT...RT5133_REG_LDO_PGB_STAT:
+	case RT5133_REG_LDO_ON...RT5133_REG_LDO_OFF:
+	case RT5133_REG_LDO1_CTRL1:
+	case RT5133_REG_LDO2_CTRL1:
+	case RT5133_REG_LDO3_CTRL1:
+	case RT5133_REG_LDO4_CTRL1:
+	case RT5133_REG_LDO5_CTRL1:
+	case RT5133_REG_LDO6_CTRL1:
+	case RT5133_REG_LDO7_CTRL1:
+	case RT5133_REG_LDO8_CTRL1:
+		return true;
+	default:
+		return false;
+	};
+}
+
+static const struct regmap_config rt5133_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = RT5133_REG_LDO8_CTRL4,
+	.cache_type = REGCACHE_FLAT,
+	.num_reg_defaults_raw = RT5133_REG_LDO8_CTRL4 + 1,
+	.volatile_reg = rt5133_is_volatile_reg,
+};
+
+static int rt5133_chip_reset(struct rt5133_priv *priv)
+{
+	int ret;
+
+	ret = regmap_write(priv->regmap, RT5133_REG_RST_CTRL,
+			   RT5133_RESET_CODE);
+	if (ret)
+		return ret;
+
+	/* Wait for register reset to take effect */
+	udelay(2);
+
+	return 0;
+}
+
+static int rt5133_validate_vendor_info(struct rt5133_priv *priv)
+{
+	unsigned int val = 0;
+	int i, ret;
+
+	ret = regmap_read(priv->regmap, RT5133_REG_CHIP_INFO, &val);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(regulator_data); i++) {
+		if ((val & RT5133_VENDOR_ID_MASK) ==
+						regulator_data[i].vendor_id){
+			priv->cdata = &regulator_data[i];
+			break;
+		}
+	}
+	if (IS_ERR(priv->cdata)) {
+		dev_err(priv->dev, "Failed to find regualtor match version\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int rt5133_probe(struct i2c_client *i2c)
+{
+	struct rt5133_priv *priv;
+	struct regulator_config config = {0};
+	int i, ret;
+
+	priv = devm_kzalloc(&i2c->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = &i2c->dev;
+	crc8_populate_msb(priv->crc8_tbls, RT5133_CRC8_POLYNOMIAL);
+
+	priv->enable_gpio = devm_gpiod_get_optional(&i2c->dev, "enable",
+						    GPIOD_OUT_HIGH);
+	if (IS_ERR(priv->enable_gpio))
+		dev_err(&i2c->dev, "Failed to request HWEN gpio, check if default en=high\n");
+
+	priv->regmap = devm_regmap_init(&i2c->dev, &rt5133_regmap_bus, priv,
+					&rt5133_regmap_config);
+	if (IS_ERR(priv->regmap)) {
+		dev_err(&i2c->dev, "Failed to register regmap\n");
+		return PTR_ERR(priv->regmap);
+	}
+
+	ret = rt5133_validate_vendor_info(priv);
+	if (ret) {
+		dev_err(&i2c->dev, "Failed to check vendor info [%d]\n", ret);
+		return ret;
+	}
+
+	ret = rt5133_chip_reset(priv);
+	if (ret) {
+		dev_err(&i2c->dev, "Failed to execute sw reset\n");
+		return ret;
+	}
+
+	config.dev = &i2c->dev;
+	config.driver_data = priv;
+	config.regmap = priv->regmap;
+
+	for (i = 0; i < RT5133_REGULATOR_MAX; i++) {
+		priv->rdev[i] = devm_regulator_register(&i2c->dev,
+							priv->cdata->regulators + i,
+							&config);
+		if (IS_ERR(priv->rdev[i])) {
+			dev_err(&i2c->dev,
+				"Failed to register [%d] regulator\n", i);
+			return PTR_ERR(priv->rdev[i]);
+		}
+	}
+
+	priv->gc.label = dev_name(&i2c->dev);
+	priv->gc.parent = &i2c->dev;
+	priv->gc.base = -1;
+	priv->gc.ngpio = RT5133_GPIO_NR;
+	priv->gc.set = rt5133_gpio_set;
+	priv->gc.get = rt5133_gpio_get;
+	priv->gc.direction_output = rt5133_gpio_direction_output;
+	priv->gc.can_sleep = true;
+
+	ret = devm_gpiochip_add_data(&i2c->dev, &priv->gc, priv);
+	if (ret)
+		return ret;
+
+	ret = rt5133_enable_interrupts(i2c->irq, priv);
+	if (ret) {
+		dev_err(&i2c->dev, "enable interrupt failed\n");
+		return ret;
+	}
+
+	i2c_set_clientdata(i2c, priv);
+
+	return ret;
+}
+
+static const struct of_device_id __maybe_unused rt5133_of_match_table[] = {
+	{ .compatible = "richtek,rt5133", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, rt5133_of_match_table);
+
+static struct i2c_driver rt5133_driver = {
+	.driver = {
+		.name = "rt5133",
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+		.of_match_table = rt5133_of_match_table,
+	},
+	.probe = rt5133_probe,
+};
+module_i2c_driver(rt5133_driver);
+
+MODULE_DESCRIPTION("RT5133 Regulator Driver");
+MODULE_LICENSE("GPL v2");
--
2.43.0

Re: [PATCH v4 1/2] dt-bindings: regulator: Add Richtek RTR5133 Support

[Krzysztof Kozlowski]

On 22/07/2025 10:34, jeff_chang@richtek.com wrote:
> From: Jeff Chang <jeff_chang@richtek.com>
> 
> Add bindings for Richtek RT5133 IC Controlled PMIC

Subject - RTR or RT? Google tells me nothing about RTR.

> 
> Signed-off-by: Jeff Chang <jeff_chang@richtek.com>
> ---
> 
> PATCH v4
> 1. Add commit message and also /script/checkpatch --strict to fix warning.
> 2. Using subject prefixes matching dt-binding subsystem.
> 3. Re-order patches. DT patch before driver patch.
> 4. Fix description of yaml.
> 5. Add more description for base regulator.
> 6. Drop regulator-compatible proeprty.
> 7. Add prefix for vendor property richtek,oc-shutdown-all and richtek,pgb-shutdown-all.
> 8. Add more description for shutdown-all property.
> 9. Interrupts-extended -> interrupts.
> 10. pio->gpio for proper defines.
> 11. Drop unused labels. Keep rt5133_ldo1 label for ldo7 and ldo8.
> 
>  .../bindings/regulator/richtek,rt5133.yaml    | 175 ++++++++++++++++++
>  1 file changed, 175 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rt5133.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/richtek,rt5133.yaml b/Documentation/devicetree/bindings/regulator/richtek,rt5133.yaml
> new file mode 100644
> index 000000000000..0da725596a87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/richtek,rt5133.yaml
> @@ -0,0 +1,175 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/richtek,rt5133.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Richtek RT5133 PMIC Regulator
> +
> +maintainers:
> +  - ShihChia Chang <jeff_chang@richtek.com>
> +
> +description:
> +  The RT5133 is an integrated Power Management IC for portable devices, featuring
> +  8 LDOs and 3 GPOs. It allows programmable output voltages, soft-start times,
> +  and protections via I2C. GPO operation depends on LDO1 voltage.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - richtek,rt5133
> +
> +  reg:
> +    maxItems: 1
> +
> +  enable-gpios:
> +    maxItems: 1
> +
> +  wakeup-source: true
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  regulators:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      base:
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +        description:
> +          Properties for base regulator which control force-off base circuit.
> +          Base circuit is the power source for LDO1~LDO6. Disabling it will
> +          reduce IQ for Chip.

I don't understand what this regulator is for. Your example is also
incomplete - missing min/max constraints like voltage.

Explain, what is this output pin? I already asked for explanations. I
have diagram in front of me, so explain precisely instead of sending THE
SAME again - which pin is it?

Also, what is IQ? Except Intelligence Quotient?


> +
> +        properties:
> +          richtek,oc-shutdown-all:
> +            type: boolean
> +            description:
> +              Anyone of LDO is in OC state, shut down all channels to protect CHIP.
> +              Without this property, only shut down the OC LDO channel.

I don't understand this. I also do not understand why this is property
of "base" not the chip itself...

So don't send next version with the same.

> +
> +          richtek,pgb-shutdown-all:
> +            type: boolean
> +            description:
> +              Anyone of LDO is in PGB state, shut down all channels to protect CHIP.

CHIP is an acronym? Or chip?

> +              Without this property, only shut down the PGB LDO channel.
> +
> +        required:
> +          - regulator-name
> +
> +    patternProperties:
> +      "^ldo([1-6])$":
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +        description:
> +          Properties for single LDO regulator
> +
> +        required:
> +          - regulator-name
> +
> +      "^ldo([7-8])$":
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +        description:
> +          Properties for single LDO regulator
> +
> +        properties:
> +          rt5133-ldo1-supply:

supplies do not have vendor prefixes.

> +            description: |
> +              Only for ldo7 ldo8, pvin7 and pvin8 reference design are RT5133 ldo1.
> +              If not connect to ldo1 vout, this property for pvin7 and pvin8 is necessary.

I don't understand why LDO1 supply is here.

Again, which pin is it?

> +
> +        required:
> +          - regulator-name
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - wakeup-source
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      rt5133@18 {

Nothing improved.

> +        compatible = "richtek,rt5133";
> +        reg = <0x18>;
> +        wakeup-source;
> +        interrupts-extended = <&gpio 187 0x0>;

Nothing improved

Implement previous comments and respond to each of them to confirm you
understood them.

> 


Best regards,
Krzysztof

Re: [PATCH v4 1/2] dt-bindings: regulator: Add Richtek RTR5133 Support

[Mark Brown]

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

On Tue, Jul 22, 2025 at 11:04:51AM +0200, Krzysztof Kozlowski wrote:
> On 22/07/2025 10:34, jeff_chang@richtek.com wrote:

> > +      base:
> > +        type: object
> > +        $ref: regulator.yaml#
> > +        unevaluatedProperties: false
> > +        description:
> > +          Properties for base regulator which control force-off base circuit.
> > +          Base circuit is the power source for LDO1~LDO6. Disabling it will
> > +          reduce IQ for Chip.

> I don't understand what this regulator is for. Your example is also
> incomplete - missing min/max constraints like voltage.

> Explain, what is this output pin? I already asked for explanations. I
> have diagram in front of me, so explain precisely instead of sending THE
> SAME again - which pin is it?

It's the top level supply for the chip, it's likely not externally
visible and sounds like it's just an on/off switch rather than
regulating voltages.  This seems fairly clear with domain knowledge.

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

RE: [PATCH v4 1/2] dt-bindings: regulator: Add Richtek RTR5133 Support

[jeff_chang(張世佳)]

Confidential C - Richtek

Dear Krzysztof Kozlowski:

        Thanks for your reply.

        Please review my explanation for your questions.

        If I have misunderstood or if there are any parts that need correction, please let me know.

Thanks
Jeff


-----Original Message-----
From: Krzysztof Kozlowski <krzk@kernel.org>
Sent: Tuesday, July 22, 2025 5:05 PM
To: jeff_chang(張世佳) <jeff_chang@richtek.com>; lgirdwood@gmail.com; broonie@kernel.org; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
Subject: Re: [PATCH v4 1/2] dt-bindings: regulator: Add Richtek RTR5133 Support

On 22/07/2025 10:34, jeff_chang@richtek.com wrote:
> From: Jeff Chang <jeff_chang@richtek.com>
>
> Add bindings for Richtek RT5133 IC Controlled PMIC

Subject - RTR or RT? Google tells me nothing about RTR.

--> I will fix it to RT5133 in next patch.

>
> Signed-off-by: Jeff Chang <jeff_chang@richtek.com>
> ---
>
> PATCH v4
> 1. Add commit message and also /script/checkpatch --strict to fix warning.
> 2. Using subject prefixes matching dt-binding subsystem.
> 3. Re-order patches. DT patch before driver patch.
> 4. Fix description of yaml.
> 5. Add more description for base regulator.
> 6. Drop regulator-compatible proeprty.
> 7. Add prefix for vendor property richtek,oc-shutdown-all and richtek,pgb-shutdown-all.
> 8. Add more description for shutdown-all property.
> 9. Interrupts-extended -> interrupts.
> 10. pio->gpio for proper defines.
> 11. Drop unused labels. Keep rt5133_ldo1 label for ldo7 and ldo8.
>
>  .../bindings/regulator/richtek,rt5133.yaml    | 175 ++++++++++++++++++
>  1 file changed, 175 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/regulator/richtek,rt5133.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/regulator/richtek,rt5133.yaml
> b/Documentation/devicetree/bindings/regulator/richtek,rt5133.yaml
> new file mode 100644
> index 000000000000..0da725596a87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/richtek,rt5133.yaml
> @@ -0,0 +1,175 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/richtek,rt5133.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Richtek RT5133 PMIC Regulator
> +
> +maintainers:
> +  - ShihChia Chang <jeff_chang@richtek.com>
> +
> +description:
> +  The RT5133 is an integrated Power Management IC for portable
> +devices, featuring
> +  8 LDOs and 3 GPOs. It allows programmable output voltages,
> +soft-start times,
> +  and protections via I2C. GPO operation depends on LDO1 voltage.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - richtek,rt5133
> +
> +  reg:
> +    maxItems: 1
> +
> +  enable-gpios:
> +    maxItems: 1
> +
> +  wakeup-source: true
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  regulators:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      base:
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +        description:
> +          Properties for base regulator which control force-off base circuit.
> +          Base circuit is the power source for LDO1~LDO6. Disabling it will
> +          reduce IQ for Chip.

I don't understand what this regulator is for. Your example is also incomplete - missing min/max constraints like voltage.

Explain, what is this output pin? I already asked for explanations. I have diagram in front of me, so explain precisely instead of sending THE SAME again - which pin is it?

Also, what is IQ? Except Intelligence Quotient?

--> Thanks to Mark's Suggestion. It's the top-level supply for LDO1 to LDO6. It is not externally visible and functions merely as an on/off switch rather than regulating voltages.
   I will update the description as follows.

   Properties for the base regulator, which is the top-level supply for LDO1 to LDO6. It functions merely as an on/off switch rather than regulating voltages.
   If none of LDO1 to LDO6 are in use, switching off the base will reduce the quiescent current.

> +
> +        properties:
> +          richtek,oc-shutdown-all:
> +            type: boolean
> +            description:
> +              Anyone of LDO is in OC state, shut down all channels to protect CHIP.
> +              Without this property, only shut down the OC LDO channel.

I don't understand this. I also do not understand why this is property of "base" not the chip itself...

So don't send next version with the same.

--> It is a protection for LDO Over Current . The Description in datasheet like below.

Anyone of LDO OC state, shut down all LDO channels
0 : LDO OC only shut down itself (default)
1 : LDO OC shut down all channels
We set it as a property of "base" for using regulator_desc -> of_parse_cb.
Should I move them to chip property? We bind it to base regulator for easily programming.

> +
> +          richtek,pgb-shutdown-all:
> +            type: boolean
> +            description:
> +              Anyone of LDO is in PGB state, shut down all channels to protect CHIP.

CHIP is an acronym? Or chip?
--> chip! I will use "chip" in next patch.

> +              Without this property, only shut down the PGB LDO channel.
> +
> +        required:
> +          - regulator-name
> +
> +    patternProperties:
> +      "^ldo([1-6])$":
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +        description:
> +          Properties for single LDO regulator
> +
> +        required:
> +          - regulator-name
> +
> +      "^ldo([7-8])$":
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +        description:
> +          Properties for single LDO regulator
> +
> +        properties:
> +          rt5133-ldo1-supply:

supplies do not have vendor prefixes.

> +            description: |
> +              Only for ldo7 ldo8, pvin7 and pvin8 reference design are RT5133 ldo1.
> +              If not connect to ldo1 vout, this property for pvin7 and pvin8 is necessary.

I don't understand why LDO1 supply is here.

Again, which pin is it?


--> Please refer to PVIN7 and PVIN8 in the diagram. They are the power supplies for LDO7 and LDO8, respectively. The reference for PVIN7 and PVIN8 is the VOUT of LDO1 (VOUT1).
   In the driver, we set "rt5133-ldo1" as the supply_name in the regulator_desc for LDO7 and LDO8. Users can overwrite the rt5133-ldo1-supply property according to their own layout.
   I will just use vin-supply and I will remove vendor prefixes of supplies in next version.

> +
> +        required:
> +          - regulator-name
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - wakeup-source
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      rt5133@18 {

Nothing improved.

> +        compatible = "richtek,rt5133";
> +        reg = <0x18>;
> +        wakeup-source;
> +        interrupts-extended = <&gpio 187 0x0>;

Nothing improved

--> I will update them like below.
pmic@18 {
interrupts-extended = <&gpio 187 IRQ_TYPE_LEVEL_LOW>;


Implement previous comments and respond to each of them to confirm you understood them.

>


Best regards,
Krzysztof

Re: [PATCH v4 1/2] dt-bindings: regulator: Add Richtek RTR5133 Support

[Krzysztof Kozlowski]

On 23/07/2025 04:32, jeff_chang(張世佳) wrote:
> Confidential C - Richtek


I cannot work with confidential data. Fix your email setup.

> 
> Dear Krzysztof Kozlowski:
> 
>         Thanks for your reply.
> 
>         Please review my explanation for your questions.
> 
>         If I have misunderstood or if there are any parts that need correction, please let me know.
> 
> Thanks
> Jeff
> 
> 
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Tuesday, July 22, 2025 5:05 PM
> To: jeff_chang(張世佳) <jeff_chang@richtek.com>; lgirdwood@gmail.com; broonie@kernel.org; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> Subject: Re: [PATCH v4 1/2] dt-bindings: regulator: Add Richtek RTR5133 Support
> 
> On 22/07/2025 10:34, jeff_chang@richtek.com wrote:
>> From: Jeff Chang <jeff_chang@richtek.com>
>>
>> Add bindings for Richtek RT5133 IC Controlled PMIC
> 
> Subject - RTR or RT? Google tells me nothing about RTR.


Why are you pasting my reply as yours?

> 
> --> I will fix it to RT5133 in next patch.


Fix your email setup.

Best regards,
Krzysztof

Re: [PATCH v4 1/2] dt-bindings: regulator: Add Richtek RTR5133 Support

[jeff_chang]

Dear Krzysztof Kozlowski:

	Thanks for your reply.

	Please review my explanation for your questions.

	If I have misunderstood or if there are any parts that need correction, please let me know.

Thanks
Jeff


On 22/07/2025 10:34, jeff_chang@richtek.com wrote:
> From: Jeff Chang <jeff_chang@richtek.com>
> 
> Add bindings for Richtek RT5133 IC Controlled PMIC

Subject - RTR or RT? Google tells me nothing about RTR.

--> I will fix it to RT5133 in next patch.


> +
> +    properties:
> +      base:
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +        description:
> +          Properties for base regulator which control force-off base circuit.
> +          Base circuit is the power source for LDO1~LDO6. Disabling it will
> +          reduce IQ for Chip.

I don't understand what this regulator is for. Your example is also
incomplete - missing min/max constraints like voltage.

Explain, what is this output pin? I already asked for explanations. I
have diagram in front of me, so explain precisely instead of sending THE
SAME again - which pin is it?

Also, what is IQ? Except Intelligence Quotient?

--> Thanks to Mark's Suggestion. It's the top-level supply for LDO1 to LDO6.
    It is not externally visible and functions merely as an on/off switch rather
    than regulating voltages.
    I will update the description as follows.

   Properties for the base regulator, which is the top-level supply for LDO1 to LDO6.
   It functions merely as an on/off switch rather than regulating voltages.
   If none of LDO1 to LDO6 are in use, switching off the base will reduce the quiescent current.


> +
> +        properties:
> +          richtek,oc-shutdown-all:
> +            type: boolean
> +            description:
> +              Anyone of LDO is in OC state, shut down all channels to protect CHIP.
> +              Without this property, only shut down the OC LDO channel.

I don't understand this. I also do not understand why this is property
of "base" not the chip itself...

So don't send next version with the same.

--> It is a protection for LDO Over Current . The Description in datasheet like below. 

    Anyone of LDO OC state, shut down all LDO channels
    0 : LDO OC only shut down itself (default)
    1 : LDO OC shut down all channels
    We set it as a property of "base" for using regulator_desc -> of_parse_cb.
    Should I move them to chip property? We bind it to base regulator for easily programming.


> +
> +          richtek,pgb-shutdown-all:
> +            type: boolean
> +            description:
> +              Anyone of LDO is in PGB state, shut down all channels to protect CHIP.

CHIP is an acronym? Or chip?
--> chip! I will use "chip" in next patch.

> +              Without this property, only shut down the PGB LDO channel.
> +
> +        required:
> +          - regulator-name
> +
> +    patternProperties:
> +      "^ldo([1-6])$":
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +        description:
> +          Properties for single LDO regulator
> +
> +        required:
> +          - regulator-name
> +
> +      "^ldo([7-8])$":
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +        description:
> +          Properties for single LDO regulator
> +
> +        properties:
> +          rt5133-ldo1-supply:

supplies do not have vendor prefixes.

> +            description: |
> +              Only for ldo7 ldo8, pvin7 and pvin8 reference design are RT5133 ldo1.
> +              If not connect to ldo1 vout, this property for pvin7 and pvin8 is necessary.

I don't understand why LDO1 supply is here.

Again, which pin is it?

--> Please refer to PVIN7 and PVIN8 in the diagram. They are the power supplies
    for LDO7 and LDO8, respectively. The reference for PVIN7 and PVIN8 is the VOUT of LDO1 (VOUT1).
    In the driver, we set "rt5133-ldo1" as the supply_name in the regulator_desc
    for LDO7 and LDO8. Users can overwrite the rt5133-ldo1-supply property according to their own layout.
    Anyway I will just use vin-supply and I will remove vendor prefixes of supplies in next version.



> +
> +        required:
> +          - regulator-name
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - wakeup-source
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      rt5133@18 {

Nothing improved.

> +        compatible = "richtek,rt5133";
> +        reg = <0x18>;
> +        wakeup-source;
> +        interrupts-extended = <&gpio 187 0x0>;

Nothing improved

--> I will update them like below.

    pmic@18 {
    interrupts-extended = <&gpio 187 IRQ_TYPE_LEVEL_LOW>;


Implement previous comments and respond to each of them to confirm you
understood them.

> 


Best regards,
Krzysztof