linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/2] regulator: rt5133: Add RT5133 PMIC regulator Support
@ 2025-07-21  6:01 jeff_chang
  2025-07-21  6:01 ` [PATCH v3 2/2] dt-bindings: regulator: Add Richtek RTR5133 DT Binding Documentation jeff_chang
  2025-07-21 12:18 ` [PATCH v3 1/2] regulator: rt5133: Add RT5133 PMIC regulator Support Mark Brown
  0 siblings, 2 replies; 5+ messages in thread
From: jeff_chang @ 2025-07-21  6:01 UTC (permalink / raw)
  To: lgirdwood, broonie, robh, krzk+dt, conor+dt, linux-kernel,
	devicetree
  Cc: jeff_chang

From: Jeff Chang <jeff_chang@richtek.com>

RT5133 is an intefrated 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. If LDO1 voltage output disabled,
GPO cannot be used to drive output high. It need to pay more attention
on the usage.

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

PATCH v3:
1. Fix typo intefrated -> integrated
2. Remove yaml file
3. Remove soft-start-time-sel property
4. Remove GPL-4.0, using GPL-2.0
5. Using C++ comment
6. Using standard dev_dbg
7. Remove driver version
8. Using dev_dbg for base event occured. And return IRQ_HANDLED 9. Because of CRC8 for IO, we don't use cache for regmap_config 10. Remove tracing logs. 
11. Set can_sleep = true for gpio operations. 
12. Kconfig add depends on OF
	

 drivers/regulator/Kconfig            |  13 +
 drivers/regulator/Makefile           |   1 +
 drivers/regulator/rt5133-regulator.c | 624 +++++++++++++++++++++++++++
 3 files changed, 638 insertions(+)
 create mode 100644 drivers/regulator/rt5133-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 6d8988387da4..2e0c35449fcb 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1229,6 +1229,19 @@ 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
+	  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. If LDO1 voltage output disabled,
+	  GPO cannot be used to drive output high. It need to pay more attention
+	  on the usage.
+
 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..7c5792b1f067
--- /dev/null
+++ b/drivers/regulator/rt5133-regulator.c
@@ -0,0 +1,624 @@
+// 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_BASE_MASK		0x16
+#define RT5133_REG_LDO_SHDN		0x19
+#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, "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, "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, vtables, _supply) \
+{\
+	.name = #_name,\
+	.id = RT5133_REGULATOR_##_name,\
+	.of_match = of_match_ptr(#_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, vout_type1_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO2, vout_type1_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO3, vout_type2_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO4, vout_type2_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO5, vout_type2_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO6, vout_type2_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO7, vout_type3_tables, "rt5133-ldo1"),
+	RT5133_REGULATOR_DESC(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, vout_type1_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO2, vout_type1_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO3, vout_type2_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO4, vout_type2_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO5, vout_type2_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO6, vout_type2_tables, "rt5133,base"),
+	RT5133_REGULATOR_DESC(LDO7, vout_type3_tables, "rt5133-ldo1"),
+	RT5133_REGULATOR_DESC(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_HANDLED;
+	}
+
+	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, "reg = 0x%02x, data = 0x%02x\n", 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, "reg = 0x%02x, data = 0x%02x\n", 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,
+};
+
+// Due to crc, cache is not necessary
+static const struct regmap_config rt5133_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = RT5133_REG_LDO8_CTRL4,
+};
+
+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


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

* [PATCH v3 2/2] dt-bindings: regulator: Add Richtek RTR5133 DT Binding Documentation
  2025-07-21  6:01 [PATCH v3 1/2] regulator: rt5133: Add RT5133 PMIC regulator Support jeff_chang
@ 2025-07-21  6:01 ` jeff_chang
  2025-07-21  8:04   ` Krzysztof Kozlowski
  2025-07-21 19:46   ` Rob Herring
  2025-07-21 12:18 ` [PATCH v3 1/2] regulator: rt5133: Add RT5133 PMIC regulator Support Mark Brown
  1 sibling, 2 replies; 5+ messages in thread
From: jeff_chang @ 2025-07-21  6:01 UTC (permalink / raw)
  To: lgirdwood, broonie, robh, krzk+dt, conor+dt, linux-kernel,
	devicetree
  Cc: jeff_chang

From: Jeff Chang <jeff_chang@richtek.com>

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

PATCH v3
1. fix Subject format
2. using correct patches version
3. remove '|'
4. remove allOf: &ref regulator.yaml#
5. remove redundant description
6. move BASE to base property with correct indentation
7. only using lowercase node name
8. make DT_CHECKER_FLAG=-m DT_SCHEMA_FILES=richtek,rt5133.yaml dt_binding_check pass


 .../bindings/regulator/richtek,rt5133.yaml    | 197 ++++++++++++++++++
 1 file changed, 197 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..a92e7f775832
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/richtek,rt5133.yaml
@@ -0,0 +1,197 @@
+# 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:
+  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. If LDO1 voltage output disabled, GPO cannot
+  be used to drive output high. It need to pay more attention on the usage.
+
+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 for force-off base circuit
+
+        properties:
+          regulator-compatible:
+            description: Compatible string for regulator
+            $ref: /schemas/types.yaml#/definitions/string
+
+          oc-shutdown-all:
+            type: boolean
+            description:
+              Anyone of LDO OC state, shut down all channels.
+
+          pgb-shutdown-all:
+            type: boolean
+            description:
+              Anyone of PGB OC state, shut down all channels.
+
+        required:
+          - regulator-name
+          - regulator-compatible
+
+    patternProperties:
+      "^ldo([1-6])$":
+        type: object
+        $ref: regulator.yaml#
+        unevaluatedProperties: false
+        description:
+          Properties for single LDO regulator
+
+        properties:
+          regulator-compatible:
+            description: Compatible string for regulator
+            $ref: /schemas/types.yaml#/definitions/string
+
+        required:
+          - regulator-name
+          - regulator-compatible
+
+      "^ldo([7-8])$":
+        type: object
+        $ref: regulator.yaml#
+        unevaluatedProperties: false
+        description:
+          Properties for single LDO regulator
+
+        properties:
+          regulator-compatible:
+            description: Compatible string for regulator
+            $ref: /schemas/types.yaml#/definitions/string
+
+          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
+          - regulator-compatible
+
+required:
+  - compatible
+  - reg
+  - interrupts-extended
+  - wakeup-source
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      rt5133: rt5133@18 {
+        compatible = "richtek,rt5133";
+        reg = <0x18>;
+        wakeup-source;
+        interrupts-extended = <&pio 187 0x0>;
+        enable-gpios = <&pio 186 0x0>;
+        gpio-controller;
+        #gpio-cells = <2>;
+        regulators {
+          base {
+            regulator-compatible = "BASE";
+            regulator-name = "rt5133,base";
+            oc-shutdown-all;
+            pgb-shutdown-all;
+          };
+          rt5133_ldo1: ldo1 {
+            regulator-compatible = "LDO1";
+            regulator-name = "rt5133-ldo1";
+            regulator-min-microvolt = <1800000>;
+            regulator-max-microvolt = <3199998>;
+            regulator-active-discharge = <1>;
+          };
+          rt5133_ldo2: ldo2 {
+            regulator-compatible = "LDO2";
+            regulator-name = "rt5133-ldo2";
+            regulator-min-microvolt = <1800000>;
+            regulator-max-microvolt = <3200000>;
+            regulator-active-discharge = <1>;
+          };
+          rt5133_ldo3: ldo3 {
+            regulator-compatible = "LDO3";
+            regulator-name = "rt5133-ldo3";
+            regulator-min-microvolt = <1700000>;
+            regulator-max-microvolt = <3000000>;
+            regulator-active-discharge = <1>;
+          };
+          rt5133_ldo4: ldo4 {
+            regulator-compatible = "LDO4";
+            regulator-name = "rt5133-ldo4";
+            regulator-min-microvolt = <1700000>;
+            regulator-max-microvolt = <3000000>;
+            regulator-active-discharge = <1>;
+          };
+          rt5133_ldo5: ldo5 {
+            regulator-compatible = "LDO5";
+            regulator-name = "rt5133-ldo5";
+            regulator-min-microvolt = <1700000>;
+            regulator-max-microvolt = <3000000>;
+            regulator-active-discharge = <1>;
+          };
+          rt5133_ldo6: ldo6 {
+            regulator-compatible = "LDO6";
+            regulator-name = "rt5133-ldo6";
+            regulator-min-microvolt = <1700000>;
+            regulator-max-microvolt = <3000000>;
+            regulator-active-discharge = <1>;
+          };
+          rt5133_ldo7: ldo7 {
+            regulator-compatible = "LDO7";
+            regulator-name = "rt5133-ldo7";
+            regulator-min-microvolt = <900000>;
+            regulator-max-microvolt = <1200000>;
+            regulator-active-discharge = <1>;
+            rt5133-ldo1-supply = <&rt5133_ldo1>;
+          };
+          rt5133_ldo8: ldo8 {
+            regulator-compatible = "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


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

* Re: [PATCH v3 2/2] dt-bindings: regulator: Add Richtek RTR5133 DT Binding Documentation
  2025-07-21  6:01 ` [PATCH v3 2/2] dt-bindings: regulator: Add Richtek RTR5133 DT Binding Documentation jeff_chang
@ 2025-07-21  8:04   ` Krzysztof Kozlowski
  2025-07-21 19:46   ` Rob Herring
  1 sibling, 0 replies; 5+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-21  8:04 UTC (permalink / raw)
  To: jeff_chang
  Cc: lgirdwood, broonie, robh, krzk+dt, conor+dt, linux-kernel,
	devicetree

On Mon, Jul 21, 2025 at 02:01:35PM +0800, jeff_chang@richtek.com wrote:
> From: Jeff Chang <jeff_chang@richtek.com>
> 

Missing commit msg.

Please run scripts/checkpatch.pl on the patches and fix reported
warnings. After that, run also 'scripts/checkpatch.pl --strict' on the
patches and (probably) fix more warnings. Some warnings can be ignored,
especially from --strict run, but the code here looks like it needs a
fix. Feel free to get in touch if the warning is not clear.


Please use subject prefixes matching the subsystem. You can get them for
example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

I already asked for it. Did you really read what I linked last time?

A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

I already asked for it, so you ignored another comment. Why are you not
responding or implementing the comments?

Bindings go before users, so please re-order patches (see submitting
patches in DT dir).

> Signed-off-by: Jeff Chang <jeff_chang@richtek.com>
> ---
> 
> PATCH v3
> 1. fix Subject format
> 2. using correct patches version
> 3. remove '|'
> 4. remove allOf: &ref regulator.yaml#
> 5. remove redundant description
> 6. move BASE to base property with correct indentation
> 7. only using lowercase node name
> 8. make DT_CHECKER_FLAG=-m DT_SCHEMA_FILES=richtek,rt5133.yaml dt_binding_check pass
> 
> 
>  .../bindings/regulator/richtek,rt5133.yaml    | 197 ++++++++++++++++++
>  1 file changed, 197 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..a92e7f775832
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/richtek,rt5133.yaml
> @@ -0,0 +1,197 @@
> +# 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:
> +  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. If LDO1 voltage output disabled, GPO cannot
> +  be used to drive output high. It need to pay more attention on the usage.

Last statement does not feel relevant.

> +
> +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 for force-off base circuit

That's not a regulator supply or you need to provide proper description
of hardware.

> +
> +        properties:
> +          regulator-compatible:
> +            description: Compatible string for regulator
> +            $ref: /schemas/types.yaml#/definitions/string

Drop property.

> +
> +          oc-shutdown-all:

Missing vendor prefix.

> +            type: boolean
> +            description:
> +              Anyone of LDO OC state, shut down all channels.

I don't understand the description at all.

> +
> +          pgb-shutdown-all:
> +            type: boolean
> +            description:
> +              Anyone of PGB OC state, shut down all channels.

Same problems here

> +
> +        required:
> +          - regulator-name
> +          - regulator-compatible

No, drop compatible. Please read the bindings.

> +
> +    patternProperties:
> +      "^ldo([1-6])$":
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +        description:
> +          Properties for single LDO regulator
> +
> +        properties:
> +          regulator-compatible:
> +            description: Compatible string for regulator
> +            $ref: /schemas/types.yaml#/definitions/string
> +
> +        required:
> +          - regulator-name
> +          - regulator-compatible
> +
> +      "^ldo([7-8])$":
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +        description:
> +          Properties for single LDO regulator
> +
> +        properties:
> +          regulator-compatible:
> +            description: Compatible string for regulator
> +            $ref: /schemas/types.yaml#/definitions/string
> +
> +          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
> +          - regulator-compatible
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts-extended

interrupts instead

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

Drop unused labels.


Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

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

Use proper defines

> +        enable-gpios = <&pio 186 0x0>;

Use proper defines.

> +        gpio-controller;
> +        #gpio-cells = <2>;
> +        regulators {
> +          base {
> +            regulator-compatible = "BASE";
> +            regulator-name = "rt5133,base";
> +            oc-shutdown-all;
> +            pgb-shutdown-all;
> +          };
> +          rt5133_ldo1: ldo1 {

Drop unused labels.

Best regards,
Krzysztof


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

* Re: [PATCH v3 1/2] regulator: rt5133: Add RT5133 PMIC regulator Support
  2025-07-21  6:01 [PATCH v3 1/2] regulator: rt5133: Add RT5133 PMIC regulator Support jeff_chang
  2025-07-21  6:01 ` [PATCH v3 2/2] dt-bindings: regulator: Add Richtek RTR5133 DT Binding Documentation jeff_chang
@ 2025-07-21 12:18 ` Mark Brown
  1 sibling, 0 replies; 5+ messages in thread
From: Mark Brown @ 2025-07-21 12:18 UTC (permalink / raw)
  To: jeff_chang; +Cc: lgirdwood, robh, krzk+dt, conor+dt, linux-kernel, devicetree

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

On Mon, Jul 21, 2025 at 02:01:34PM +0800, jeff_chang@richtek.com wrote:

> RT5133 is an intefrated 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. If LDO1 voltage output disabled,
> GPO cannot be used to drive output high. It need to pay more attention
> on the usage.

This looks mostly fine, a couple of very small nits below:

> +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_HANDLED;
> +	}
> +
> +	handle_evts = intr_evts & RT5133_BASE_EVT_MASK;

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

If there weren't any events flagged that we handled we should return
IRQ_NONE here, this supports shared interrupts and lets the error
handling in the interrupt core handle errors where the interrupt line
gets stuck.

> +// Due to crc, cache is not necessary
> +static const struct regmap_config rt5133_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = RT5133_REG_LDO8_CTRL4,
> +};

Might be worth enabling a cache?

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

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

* Re: [PATCH v3 2/2] dt-bindings: regulator: Add Richtek RTR5133 DT Binding Documentation
  2025-07-21  6:01 ` [PATCH v3 2/2] dt-bindings: regulator: Add Richtek RTR5133 DT Binding Documentation jeff_chang
  2025-07-21  8:04   ` Krzysztof Kozlowski
@ 2025-07-21 19:46   ` Rob Herring
  1 sibling, 0 replies; 5+ messages in thread
From: Rob Herring @ 2025-07-21 19:46 UTC (permalink / raw)
  To: jeff_chang
  Cc: lgirdwood, broonie, krzk+dt, conor+dt, linux-kernel, devicetree

On Mon, Jul 21, 2025 at 02:01:35PM +0800, jeff_chang@richtek.com wrote:
> From: Jeff Chang <jeff_chang@richtek.com>
> 
> Signed-off-by: Jeff Chang <jeff_chang@richtek.com>
> ---
> 
> PATCH v3
> 1. fix Subject format
> 2. using correct patches version
> 3. remove '|'
> 4. remove allOf: &ref regulator.yaml#
> 5. remove redundant description
> 6. move BASE to base property with correct indentation
> 7. only using lowercase node name
> 8. make DT_CHECKER_FLAG=-m DT_SCHEMA_FILES=richtek,rt5133.yaml dt_binding_check pass
> 
> 
>  .../bindings/regulator/richtek,rt5133.yaml    | 197 ++++++++++++++++++
>  1 file changed, 197 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..a92e7f775832
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/richtek,rt5133.yaml
> @@ -0,0 +1,197 @@
> +# 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:
> +  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. If LDO1 voltage output disabled, GPO cannot
> +  be used to drive output high. It need to pay more attention on the usage.
> +
> +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 for force-off base circuit
> +
> +        properties:
> +          regulator-compatible:

regulator-compatible was deprecated so long ago we don't even document 
it now. Why are you bringing it back?

> +            description: Compatible string for regulator
> +            $ref: /schemas/types.yaml#/definitions/string
> +
> +          oc-shutdown-all:
> +            type: boolean
> +            description:
> +              Anyone of LDO OC state, shut down all channels.
> +
> +          pgb-shutdown-all:
> +            type: boolean
> +            description:
> +              Anyone of PGB OC state, shut down all channels.

Is 'base' an actual regulator or are these just common properties for 
all regulators? If the latter, then just move them up a level and add a 
vendor prefix.

Rob


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

end of thread, other threads:[~2025-07-21 19:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-21  6:01 [PATCH v3 1/2] regulator: rt5133: Add RT5133 PMIC regulator Support jeff_chang
2025-07-21  6:01 ` [PATCH v3 2/2] dt-bindings: regulator: Add Richtek RTR5133 DT Binding Documentation jeff_chang
2025-07-21  8:04   ` Krzysztof Kozlowski
2025-07-21 19:46   ` Rob Herring
2025-07-21 12:18 ` [PATCH v3 1/2] regulator: rt5133: Add RT5133 PMIC regulator Support Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).