Skip to contentSkip to conversation

dt-bindings: hwmon: pmbus: add bindings for isl68137

PATCH Superseded Grant Peltier 3 participants 8 messages Oct 23, 2024 – Oct 24, 2024

Subsystems (3)
Device tree bindings Documentation Hardware monitoring

v2 of 7 Superseded Latest is v7 →

lore
Share & tools
lore

Grant Peltier

author
#0 lore raw

v2 cover dt-bindings: hwmon: pmbus: add bindings for isl68137

Renesas digital multiphase voltage regulators are capable of regulating output voltages that exceed the range that their Vsense pins can detect. In such applications, users may place a voltage divider between Vout and the Vsense pin for a given rail. However, the driver currently has no way of knowing if a voltage divider is being used which results in erroneous telemetry being reported over hwmon.

This patch set defines a devicetree bindings schema for Renesas digital multiphase voltage regulators that are supported by the isl68137 driver to allow users to add voltage divider definitions for any rail powered by the device. This patch set also includes the required changes to the isl68137 driver to enable scaling Vout/Pout telemetry for rails with a defined voltage divider.

v2:
- Fix devicetree bindings schema errors
- Add "renesas," vendor prefix to "vout-voltage-divider" property
- Rebase patch series on v6.12-rc1

Grant Peltier (2):
hwmon: (pmbus/isl68137) add support for voltage divider on Vout
dt-bindings: hwmon: isl68137: add bindings to support voltage dividers

 .../hwmon/pmbus/renesas,isl68137.yaml         | 131 ++++++++++++
 drivers/hwmon/pmbus/isl68137.c                | 199 +++++++++++++++++-
 2 files changed, 325 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml
Signature · 1 line
2.39.5
author
#1 lore raw

PATCH v2 1/2 hwmon: (pmbus/isl68137) add support for voltage divider on Vout

Some applications require Vout to be higher than the detectable voltage range of the Vsense pin for a given rail. In such applications, a voltage divider may be placed between Vout and the Vsense pin, but this results in erroneous telemetry being read back from the part. This change adds support for a voltage divider to be defined in the devicetree for a (or multiple) specific rail(s) for a supported digital multiphase device and for the applicable Vout telemetry to be scaled based on the voltage divider configuration.

Signed-off-by Grant Peltier <[email protected]>
---
 drivers/hwmon/pmbus/isl68137.c | 199 ++++++++++++++++++++++++++++++++-
 1 file changed, 194 insertions(+), 5 deletions(-)
drivers/hwmon/pmbus/isl68137.c +194 -5
index 7e53fb1d5ea3..b4f581e1d560 100644
@@ -13,6 +13,7 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/string.h>
 #include <linux/sysfs.h>
 

@@ -20,6 +21,7 @@
 
 #define ISL68137_VOUT_AVS	0x30
 #define RAA_DMPVR2_READ_VMON	0xc8
+#define MAX_CHANNELS            4
 
 enum chips {
 	isl68137,

@@ -72,6 +74,17 @@ enum variants {
 	raa_dmpvr2_hv,
 };
 
+struct isl68137_channel {
+	u32 vout_voltage_divider[2];
+};
+
+struct isl68137_data {
+	struct pmbus_driver_info info;
+	struct isl68137_channel channel[MAX_CHANNELS];
+};
+
+#define to_isl68137_data(x)	container_of(x, struct isl68137_data, info)
+
 static const struct i2c_device_id raa_dmpvr_id[];
 
 static ssize_t isl68137_avs_enable_show_page(struct i2c_client *client,

@@ -163,6 +176,8 @@ static const struct attribute_group *isl68137_attribute_groups[] = {
 static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page,
 				     int phase, int reg)
 {
+	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
+	const struct isl68137_data *data = to_isl68137_data(info);
 	int ret;
 
 	switch (reg) {

@@ -170,6 +185,25 @@ static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page,
 		ret = pmbus_read_word_data(client, page, phase,
 					   RAA_DMPVR2_READ_VMON);
 		break;
+	case PMBUS_READ_POUT:
+		/*
+		 * In cases where a voltage divider is attached to the target
+		 * rail between Vout and the Vsense pin, both Vout and Pout
+		 * should be scaled by the voltage divider scaling factor.
+		 * I.e. Vout = Vsense * (R1 + R2) / R2
+		 */
+		fallthrough;
+	case PMBUS_READ_VOUT:
+		ret = pmbus_read_word_data(client, page, phase, reg);
+		if (ret > 0 && data->channel[page].vout_voltage_divider[0]
+			&& data->channel[page].vout_voltage_divider[1]) {
+			u64 temp = DIV_ROUND_CLOSEST_ULL((u64)ret *
+				(data->channel[page].vout_voltage_divider[0]
+				+ data->channel[page].vout_voltage_divider[1]),
+				data->channel[page].vout_voltage_divider[1]);
+			ret = clamp_val(temp, 0, 0xffff);
+		}
+		break;
 	default:
 		ret = -ENODATA;
 		break;

@@ -178,6 +212,50 @@ static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page,
 	return ret;
 }
 
+static int raa_dmpvr2_write_word_data(struct i2c_client *client, int page,
+				      int reg, u16 word)
+{
+	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
+	const struct isl68137_data *data = to_isl68137_data(info);
+	int ret;
+
+	switch (reg) {
+	case PMBUS_VOUT_MAX:
+		/*
+		 * In cases where a voltage divider is attached to the target
+		 * rail between Vout and the Vsense pin, Vout related PMBus
+		 * commands should be scaled based on the expected voltage
+		 * at the Vsense pin.
+		 * I.e. Vsense = Vout * R2 / (R1 + R2)
+		 */
+		fallthrough;
+	case PMBUS_VOUT_MARGIN_HIGH:
+		fallthrough;
+	case PMBUS_VOUT_MARGIN_LOW:
+		fallthrough;
+	case PMBUS_VOUT_OV_FAULT_LIMIT:
+		fallthrough;
+	case PMBUS_VOUT_UV_FAULT_LIMIT:
+		fallthrough;
+	case PMBUS_VOUT_COMMAND:
+		if (data->channel[page].vout_voltage_divider[0]
+			&& data->channel[page].vout_voltage_divider[1]) {
+			u64 temp = DIV_ROUND_CLOSEST_ULL((u64)word *
+				data->channel[page].vout_voltage_divider[1],
+				(data->channel[page].vout_voltage_divider[0] +
+				 data->channel[page].vout_voltage_divider[1]));
+			ret = clamp_val(temp, 0, 0xffff);
+		} else {
+			ret = -ENODATA;
+		}
+		break;
+	default:
+		ret = -ENODATA;
+		break;
+	}
+	return ret;
+}
+
 static struct pmbus_driver_info raa_dmpvr_info = {
 	.pages = 3,
 	.format[PSC_VOLTAGE_IN] = direct,

@@ -220,14 +298,67 @@ static struct pmbus_driver_info raa_dmpvr_info = {
 	    | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_POUT,
 };
 
+static int isl68137_probe_child_from_dt(struct device *dev,
+					struct device_node *child,
+					struct isl68137_data *data)
+{
+	u32 channel;
+	int err;
+
+	err = of_property_read_u32(child, "reg", &channel);
+	if (err) {
+		dev_err(dev, "missing reg property of %pOFn\n", child);
+		return err;
+	}
+	if (channel >= MAX_CHANNELS) {
+		dev_err(dev, "invalid reg %d of %pOFn\n", channel, child);
+		return -EINVAL;
+	}
+
+	of_property_read_u32_array(child, "renesas,vout-voltage-divider",
+				data->channel[channel].vout_voltage_divider,
+				ARRAY_SIZE(data->channel[channel].vout_voltage_divider));
+
+	return 0;
+}
+
+static int isl68137_probe_from_dt(struct device *dev,
+				  struct isl68137_data *data)
+{
+	const struct device_node *np = dev->of_node;
+	struct device_node *child;
+	int err;
+
+	for_each_child_of_node(np, child) {
+		if (strcmp(child->name, "channel"))
+			continue;
+
+		err = isl68137_probe_child_from_dt(dev, child, data);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 static int isl68137_probe(struct i2c_client *client)
 {
+	struct device *dev = &client->dev;
 	struct pmbus_driver_info *info;
+	struct isl68137_data *data;
+	int i, err;
 
-	info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);
-	if (!info)
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
 		return -ENOMEM;
-	memcpy(info, &raa_dmpvr_info, sizeof(*info));
+
+	for (i = 0; i < MAX_CHANNELS; i++)
+		memset(data->channel[i].vout_voltage_divider,
+			0,
+			sizeof(data->channel[i].vout_voltage_divider));
+
+	memcpy(&data->info, &raa_dmpvr_info, sizeof(data->info));
+	info = &data->info;
 
 	switch (i2c_match_id(raa_dmpvr_id, client)->driver_data) {
 	case raa_dmpvr1_2rail:

@@ -242,6 +373,7 @@ static int isl68137_probe(struct i2c_client *client)
 	case raa_dmpvr2_1rail:
 		info->pages = 1;
 		info->read_word_data = raa_dmpvr2_read_word_data;
+		info->write_word_data = raa_dmpvr2_write_word_data;
 		break;
 	case raa_dmpvr2_2rail_nontc:
 		info->func[0] &= ~PMBUS_HAVE_TEMP3;

@@ -250,9 +382,11 @@ static int isl68137_probe(struct i2c_client *client)
 	case raa_dmpvr2_2rail:
 		info->pages = 2;
 		info->read_word_data = raa_dmpvr2_read_word_data;
+		info->write_word_data = raa_dmpvr2_write_word_data;
 		break;
 	case raa_dmpvr2_3rail:
 		info->read_word_data = raa_dmpvr2_read_word_data;
+		info->write_word_data = raa_dmpvr2_write_word_data;
 		break;
 	case raa_dmpvr2_hv:
 		info->pages = 1;

@@ -263,11 +397,18 @@ static int isl68137_probe(struct i2c_client *client)
 		info->m[PSC_POWER] = 2;
 		info->R[PSC_POWER] = -1;
 		info->read_word_data = raa_dmpvr2_read_word_data;
+		info->write_word_data = raa_dmpvr2_write_word_data;
 		break;
 	default:
 		return -ENODEV;
 	}
 
+	if (dev->of_node) {
+		err = isl68137_probe_from_dt(dev, data);
+		if (err)
+			return err;
+	}
+
 	return pmbus_do_probe(client, info);
 }
 

@@ -318,11 +459,59 @@ static const struct i2c_device_id raa_dmpvr_id[] = {
 
 MODULE_DEVICE_TABLE(i2c, raa_dmpvr_id);
 
+static const struct of_device_id isl68137_of_match[] = {
+	{ .compatible = "renesas,isl68137", .data = (void *)raa_dmpvr1_2rail },
+	{ .compatible = "renesas,isl68220", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl68221", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl68222", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl68223", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl68224", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl68225", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl68226", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl68227", .data = (void *)raa_dmpvr2_1rail },
+	{ .compatible = "renesas,isl68229", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl68233", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl68239", .data = (void *)raa_dmpvr2_3rail },
+
+	{ .compatible = "renesas,isl69222", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69223", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl69224", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69225", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69227", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl69228", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl69234", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69236", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69239", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl69242", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69243", .data = (void *)raa_dmpvr2_1rail },
+	{ .compatible = "renesas,isl69247", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69248", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69254", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69255", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69256", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69259", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69260", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69268", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69269", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl69298", .data = (void *)raa_dmpvr2_2rail },
+
+	{ .compatible = "renesas,raa228000", .data = (void *)raa_dmpvr2_hv },
+	{ .compatible = "renesas,raa228004", .data = (void *)raa_dmpvr2_hv },
+	{ .compatible = "renesas,raa228006", .data = (void *)raa_dmpvr2_hv },
+	{ .compatible = "renesas,raa228228", .data = (void *)raa_dmpvr2_2rail_nontc },
+	{ .compatible = "renesas,raa229001", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,raa229004", .data = (void *)raa_dmpvr2_2rail },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, isl68137_of_match);
+
 /* This is the driver that will be inserted */
 static struct i2c_driver isl68137_driver = {
 	.driver = {
-		   .name = "isl68137",
-		   },
+		.name = "isl68137",
+		.of_match_table = isl68137_of_match,
+	},
 	.probe = isl68137_probe,
 	.id_table = raa_dmpvr_id,
 };
Signature · 1 line
2.39.5
#2 lore raw

Re: [PATCH v2 1/2] hwmon: (pmbus/isl68137) add support for voltage divider on Vout

Hi Grant,

Quoted 10 lines · Grant Peltier

Some applications require Vout to be higher than the detectable voltage range of the Vsense pin for a given rail. In such applications, a voltage divider may be placed between Vout and the Vsense pin, but this results in erroneous telemetry being read back from the part. This change adds support for a voltage divider to be defined in the devicetree for a (or multiple) specific rail(s) for a supported digital multiphase device and for the applicable Vout telemetry to be scaled based on the voltage divider configuration.

Signed-off-by Grant Peltier <[email protected]>

Thanks for your patch!

Quoted 13 lines
drivers/hwmon/pmbus/isl68137.c +6 -0
@@ -170,6 +185,25 @@ static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page,
                ret = pmbus_read_word_data(client, page, phase,
                                           RAA_DMPVR2_READ_VMON);
                break;
+       case PMBUS_READ_POUT:
+               /*
+                * In cases where a voltage divider is attached to the target
+                * rail between Vout and the Vsense pin, both Vout and Pout
+                * should be scaled by the voltage divider scaling factor.
+                * I.e. Vout = Vsense * (R1 + R2) / R2

drivers/hwmon/pmbus/isl68137.c static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page, · line 203

Quoted 10 lines
drivers/hwmon/pmbus/isl68137.c +10 -0
+                */
+               fallthrough;
+       case PMBUS_READ_VOUT:
+               ret = pmbus_read_word_data(client, page, phase, reg);
+               if (ret > 0 && data->channel[page].vout_voltage_divider[0]
+                       && data->channel[page].vout_voltage_divider[1]) {
+                       u64 temp = DIV_ROUND_CLOSEST_ULL((u64)ret *
+                               (data->channel[page].vout_voltage_divider[0]
+                               + data->channel[page].vout_voltage_divider[1]),
+                               data->channel[page].vout_voltage_divider[1]);

You are casting "ret" to u64 to force a 64-bit multiplication, as the product may not fit in 32 bits. However, DIV_ROUND_CLOSEST_ULL() does a 32-bit division on 32-bit platforms. So this should use DIV_U64_ROUND_CLOSEST() instead.
The sum of vout_voltage_divider[0] + vout_voltage_divider[1] might not fit in 32 bits, so that should be changed to a 64-bit addition. Unfortunately there is no rounding version of mul_u64_u32_div() yet, so you have to open-code it.

Quoted 32 lines
drivers/hwmon/pmbus/isl68137.c +25 -0
+                       ret = clamp_val(temp, 0, 0xffff);
+               }
+               break;
        default:
                ret = -ENODATA;
                break;

@@ -178,6 +212,50 @@ static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page,
        return ret;
 }
 
+static int raa_dmpvr2_write_word_data(struct i2c_client *client, int page,
+                                     int reg, u16 word)
+{
+       const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
+       const struct isl68137_data *data = to_isl68137_data(info);
+       int ret;
+
+       switch (reg) {
+       case PMBUS_VOUT_MAX:
+               /*
+                * In cases where a voltage divider is attached to the target
+                * rail between Vout and the Vsense pin, Vout related PMBus
+                * commands should be scaled based on the expected voltage
+                * at the Vsense pin.
+                * I.e. Vsense = Vout * R2 / (R1 + R2)
+                */
+               fallthrough;
+       case PMBUS_VOUT_MARGIN_HIGH:
+               fallthrough;
+       case PMBUS_VOUT_MARGIN_LOW:
+               fallthrough;
+       case PMBUS_VOUT_OV_FAULT_LIMIT:

drivers/hwmon/pmbus/isl68137.c static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page, · line 246

Quoted 10 lines
drivers/hwmon/pmbus/isl68137.c +10 -0
+               fallthrough;
+       case PMBUS_VOUT_UV_FAULT_LIMIT:
+               fallthrough;
+       case PMBUS_VOUT_COMMAND:
+               if (data->channel[page].vout_voltage_divider[0]
+                       && data->channel[page].vout_voltage_divider[1]) {
+                       u64 temp = DIV_ROUND_CLOSEST_ULL((u64)word *
+                               data->channel[page].vout_voltage_divider[1],
+                               (data->channel[page].vout_voltage_divider[0] +
+                                data->channel[page].vout_voltage_divider[1]));

Similar comments, but here the sum is the divisor, so you have to use a full 64-by-64 division, using DIV64_U64_ROUND_CLOSEST().

Quoted 29 lines
drivers/hwmon/pmbus/isl68137.c +22 -0
+                       ret = clamp_val(temp, 0, 0xffff);
+               } else {
+                       ret = -ENODATA;
+               }
+               break;
+       default:
+               ret = -ENODATA;
+               break;
+       }
+       return ret;
+}
+
 static struct pmbus_driver_info raa_dmpvr_info = {
        .pages = 3,
        .format[PSC_VOLTAGE_IN] = direct,

@@ -220,14 +298,67 @@ static struct pmbus_driver_info raa_dmpvr_info = {
            | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_POUT,
 };
 
+static int isl68137_probe_child_from_dt(struct device *dev,
+                                       struct device_node *child,
+                                       struct isl68137_data *data)
+{
+       u32 channel;
+       int err;
+
+       err = of_property_read_u32(child, "reg", &channel);
+       if (err) {
+               dev_err(dev, "missing reg property of %pOFn\n", child);

drivers/hwmon/pmbus/isl68137.c static struct pmbus_driver_info raa_dmpvr_info = { · line 320

Quoted 10 lines
drivers/hwmon/pmbus/isl68137.c +10 -0
+               return err;
+       }
+       if (channel >= MAX_CHANNELS) {
+               dev_err(dev, "invalid reg %d of %pOFn\n", channel, child);
+               return -EINVAL;
+       }
+
+       of_property_read_u32_array(child, "renesas,vout-voltage-divider",
+                               data->channel[channel].vout_voltage_divider,
+                               ARRAY_SIZE(data->channel[channel].vout_voltage_divider));

Shouldn't the return value be checked for errors different from -EINVAL?

drivers/hwmon/pmbus/isl68137.c static struct pmbus_driver_info raa_dmpvr_info = { · line 323

Quoted 3 lines
drivers/hwmon/pmbus/isl68137.c +3 -0
+
+       return 0;
+}

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
author
#3 lore raw

Re: [PATCH v2 1/2] hwmon: (pmbus/isl68137) add support for voltage divider on Vout

Hi Geert,

Quoted 43 lines · Geert Uytterhoeven

drivers/hwmon/pmbus/isl68137.c static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page, · line 203

Quoted 9 lines

[...]

drivers/hwmon/pmbus/isl68137.c +8 -0
+       case PMBUS_READ_VOUT:
+               ret = pmbus_read_word_data(client, page, phase, reg);
+               if (ret > 0 && data->channel[page].vout_voltage_divider[0]
+                       && data->channel[page].vout_voltage_divider[1]) {
+                       u64 temp = DIV_ROUND_CLOSEST_ULL((u64)ret *
+                               (data->channel[page].vout_voltage_divider[0]
+                               + data->channel[page].vout_voltage_divider[1]),
+                               data->channel[page].vout_voltage_divider[1]);

You are casting "ret" to u64 to force a 64-bit multiplication, as the product may not fit in 32 bits. However, DIV_ROUND_CLOSEST_ULL() does a 32-bit division on 32-bit platforms. So this should use DIV_U64_ROUND_CLOSEST() instead.
The sum of vout_voltage_divider[0] + vout_voltage_divider[1] might not fit in 32 bits, so that should be changed to a 64-bit addition. Unfortunately there is no rounding version of mul_u64_u32_div() yet, so you have to open-code it.

drivers/hwmon/pmbus/isl68137.c static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page,

Quoted 10 lines
drivers/hwmon/pmbus/isl68137.c +7 -0
+                       ret = clamp_val(temp, 0, 0xffff);
+               }
+               break;
        default:
                ret = -ENODATA;
[...]
+                       u64 temp = DIV_ROUND_CLOSEST_ULL((u64)word *
+                               data->channel[page].vout_voltage_divider[1],
+                               (data->channel[page].vout_voltage_divider[0] +
+                                data->channel[page].vout_voltage_divider[1]));

Similar comments, but here the sum is the divisor, so you have to use a full 64-by-64 division, using DIV64_U64_ROUND_CLOSEST().

Quoted 10 lines
drivers/hwmon/pmbus/isl68137.c +10 -0
+                       ret = clamp_val(temp, 0, 0xffff);
+               } else {
+                       ret = -ENODATA;
+               }
+               break;
+       default:
+               ret = -ENODATA;
+               break;
+       }
+       return ret;
Quoted 9 lines

drivers/hwmon/pmbus/isl68137.c static struct pmbus_driver_info raa_dmpvr_info = { · line 320

Quoted 6 lines
drivers/hwmon/pmbus/isl68137.c +5 -0
+}
[...]
+
+       of_property_read_u32_array(child, "renesas,vout-voltage-divider",
+                               data->channel[channel].vout_voltage_divider,
+                               ARRAY_SIZE(data->channel[channel].vout_voltage_divider));

Shouldn't the return value be checked for errors different from -EINVAL?

Thank you for your review! I will make the requested changes and submit a new version.

Best regards,
Grant

maintainer
#4 lore raw

Re: [PATCH v2 1/2] hwmon: (pmbus/isl68137) add support for voltage divider on Vout

In reply to Grant Peltier
Quoted 106 lines · Grant Peltier

Some applications require Vout to be higher than the detectable voltage range of the Vsense pin for a given rail. In such applications, a voltage divider may be placed between Vout and the Vsense pin, but this results in erroneous telemetry being read back from the part. This change adds support for a voltage divider to be defined in the devicetree for a (or multiple) specific rail(s) for a supported digital multiphase device and for the applicable Vout telemetry to be scaled based on the voltage divider configuration.

Signed-off-by Grant Peltier <[email protected]>
---
  drivers/hwmon/pmbus/isl68137.c | 199 ++++++++++++++++++++++++++++++++-
  1 file changed, 194 insertions(+), 5 deletions(-)
drivers/hwmon/pmbus/isl68137.c +49 -0
index 7e53fb1d5ea3..b4f581e1d560 100644
@@ -13,6 +13,7 @@
  #include <linux/init.h>
  #include <linux/kernel.h>
  #include <linux/module.h>
+#include <linux/of.h>
  #include <linux/string.h>
  #include <linux/sysfs.h>
 

@@ -20,6 +21,7 @@
 
  #define ISL68137_VOUT_AVS	0x30
  #define RAA_DMPVR2_READ_VMON	0xc8
+#define MAX_CHANNELS            4
 
  enum chips {
  	isl68137,

@@ -72,6 +74,17 @@ enum variants {
  	raa_dmpvr2_hv,
  };
 
+struct isl68137_channel {
+	u32 vout_voltage_divider[2];
+};
+
+struct isl68137_data {
+	struct pmbus_driver_info info;
+	struct isl68137_channel channel[MAX_CHANNELS];
+};
+
+#define to_isl68137_data(x)	container_of(x, struct isl68137_data, info)
+
  static const struct i2c_device_id raa_dmpvr_id[];
 
  static ssize_t isl68137_avs_enable_show_page(struct i2c_client *client,

@@ -163,6 +176,8 @@ static const struct attribute_group *isl68137_attribute_groups[] = {
  static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page,
  				     int phase, int reg)
  {
+	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
+	const struct isl68137_data *data = to_isl68137_data(info);
  	int ret;
 
  	switch (reg) {

@@ -170,6 +185,25 @@ static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page,
  		ret = pmbus_read_word_data(client, page, phase,
  					   RAA_DMPVR2_READ_VMON);
  		break;
+	case PMBUS_READ_POUT:
+		/*
+		 * In cases where a voltage divider is attached to the target
+		 * rail between Vout and the Vsense pin, both Vout and Pout
+		 * should be scaled by the voltage divider scaling factor.
+		 * I.e. Vout = Vsense * (R1 + R2) / R2
+		 */
+		fallthrough;
+	case PMBUS_READ_VOUT:
+		ret = pmbus_read_word_data(client, page, phase, reg);
+		if (ret > 0 && data->channel[page].vout_voltage_divider[0]
+			&& data->channel[page].vout_voltage_divider[1]) {
+			u64 temp = DIV_ROUND_CLOSEST_ULL((u64)ret *
+				(data->channel[page].vout_voltage_divider[0]
+				+ data->channel[page].vout_voltage_divider[1]),
+				data->channel[page].vout_voltage_divider[1]);
+			ret = clamp_val(temp, 0, 0xffff);
+		}
+		break;
  	default:
  		ret = -ENODATA;
  		break;

@@ -178,6 +212,50 @@ static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page,
  	return ret;
  }
 
+static int raa_dmpvr2_write_word_data(struct i2c_client *client, int page,
+				      int reg, u16 word)
+{
+	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
+	const struct isl68137_data *data = to_isl68137_data(info);
+	int ret;
+
+	switch (reg) {
+	case PMBUS_VOUT_MAX:
+		/*
+		 * In cases where a voltage divider is attached to the target
+		 * rail between Vout and the Vsense pin, Vout related PMBus
+		 * commands should be scaled based on the expected voltage
+		 * at the Vsense pin.
+		 * I.e. Vsense = Vout * R2 / (R1 + R2)

drivers/hwmon/pmbus/isl68137.c static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page, · line 239

Quoted 10 lines
drivers/hwmon/pmbus/isl68137.c +10 -0
+		 */
+		fallthrough;
+	case PMBUS_VOUT_MARGIN_HIGH:
+		fallthrough;
+	case PMBUS_VOUT_MARGIN_LOW:
+		fallthrough;
+	case PMBUS_VOUT_OV_FAULT_LIMIT:
+		fallthrough;
+	case PMBUS_VOUT_UV_FAULT_LIMIT:
+		fallthrough;

Just add the comment after the last case and drop all the fallthrough; Same above.

drivers/hwmon/pmbus/isl68137.c static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page, · line 242

Quoted 3 lines
drivers/hwmon/pmbus/isl68137.c +3 -0
+	case PMBUS_VOUT_COMMAND:
+		if (data->channel[page].vout_voltage_divider[0]
+			&& data->channel[page].vout_voltage_divider[1]) {

It would be better to set defaults instead of having to check this for every executed command (for example by setting R1:=0 and R2:=1).

Quoted 26 lines
drivers/hwmon/pmbus/isl68137.c +19 -0
+			u64 temp = DIV_ROUND_CLOSEST_ULL((u64)word *
+				data->channel[page].vout_voltage_divider[1],
+				(data->channel[page].vout_voltage_divider[0] +
+				 data->channel[page].vout_voltage_divider[1]));
+			ret = clamp_val(temp, 0, 0xffff);
+		} else {
+			ret = -ENODATA;
+		}
+		break;
+	default:
+		ret = -ENODATA;
+		break;
+	}
+	return ret;
+}
+
  static struct pmbus_driver_info raa_dmpvr_info = {
  	.pages = 3,
  	.format[PSC_VOLTAGE_IN] = direct,

@@ -220,14 +298,67 @@ static struct pmbus_driver_info raa_dmpvr_info = {
  	    | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_POUT,
  };
 
+static int isl68137_probe_child_from_dt(struct device *dev,
+					struct device_node *child,
+					struct isl68137_data *data)

drivers/hwmon/pmbus/isl68137.c static struct pmbus_driver_info raa_dmpvr_info = { · line 313

Quoted 10 lines
drivers/hwmon/pmbus/isl68137.c +10 -0
+{
+	u32 channel;
+	int err;
+
+	err = of_property_read_u32(child, "reg", &channel);
+	if (err) {
+		dev_err(dev, "missing reg property of %pOFn\n", child);
+		return err;
+	}
+	if (channel >= MAX_CHANNELS) {

The actual number of channels (pages) supported by the chip is known here and should be checked, either by passing the number of channels or a pointer to the entire info structure to this function.

drivers/hwmon/pmbus/isl68137.c static struct pmbus_driver_info raa_dmpvr_info = { · line 318

Quoted 5 lines
drivers/hwmon/pmbus/isl68137.c +5 -0
+		dev_err(dev, "invalid reg %d of %pOFn\n", channel, child);
+		return -EINVAL;
+	}
+
+	of_property_read_u32_array(child, "renesas,vout-voltage-divider",

Ultimately this potentially applies to _all_ hardware monitoring chips, so I would very much prefer a generic voltage divider property definition.

drivers/hwmon/pmbus/isl68137.c static struct pmbus_driver_info raa_dmpvr_info = { · line 320

Quoted 2 lines
drivers/hwmon/pmbus/isl68137.c +2 -0
+				data->channel[channel].vout_voltage_divider,
+				ARRAY_SIZE(data->channel[channel].vout_voltage_divider));

The returned data should be be validated here.

Quoted 31 lines
drivers/hwmon/pmbus/isl68137.c +26 -1
+
+	return 0;
+}
+
+static int isl68137_probe_from_dt(struct device *dev,
+				  struct isl68137_data *data)
+{
+	const struct device_node *np = dev->of_node;
+	struct device_node *child;
+	int err;
+
+	for_each_child_of_node(np, child) {
+		if (strcmp(child->name, "channel"))
+			continue;
+
+		err = isl68137_probe_child_from_dt(dev, child, data);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
  static int isl68137_probe(struct i2c_client *client)
  {
+	struct device *dev = &client->dev;
  	struct pmbus_driver_info *info;
+	struct isl68137_data *data;
+	int i, err;
 
-	info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);

drivers/hwmon/pmbus/isl68137.c static struct pmbus_driver_info raa_dmpvr_info = { · line 358

Quoted 10 lines
drivers/hwmon/pmbus/isl68137.c +7 -2
-	if (!info)
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
  		return -ENOMEM;
-	memcpy(info, &raa_dmpvr_info, sizeof(*info));
+
+	for (i = 0; i < MAX_CHANNELS; i++)
+		memset(data->channel[i].vout_voltage_divider,
+			0,
+			sizeof(data->channel[i].vout_voltage_divider));

Under what circumstance would this not already be 0 after devm_kzalloc() ?

Quoted 27 lines
drivers/hwmon/pmbus/isl68137.c +6 -0
+
+	memcpy(&data->info, &raa_dmpvr_info, sizeof(data->info));
+	info = &data->info;
 
  	switch (i2c_match_id(raa_dmpvr_id, client)->driver_data) {
  	case raa_dmpvr1_2rail:

@@ -242,6 +373,7 @@ static int isl68137_probe(struct i2c_client *client)
  	case raa_dmpvr2_1rail:
  		info->pages = 1;
  		info->read_word_data = raa_dmpvr2_read_word_data;
+		info->write_word_data = raa_dmpvr2_write_word_data;
  		break;
  	case raa_dmpvr2_2rail_nontc:
  		info->func[0] &= ~PMBUS_HAVE_TEMP3;

@@ -250,9 +382,11 @@ static int isl68137_probe(struct i2c_client *client)
  	case raa_dmpvr2_2rail:
  		info->pages = 2;
  		info->read_word_data = raa_dmpvr2_read_word_data;
+		info->write_word_data = raa_dmpvr2_write_word_data;
  		break;
  	case raa_dmpvr2_3rail:
  		info->read_word_data = raa_dmpvr2_read_word_data;
+		info->write_word_data = raa_dmpvr2_write_word_data;
  		break;
  	case raa_dmpvr2_hv:
  		info->pages = 1;

drivers/hwmon/pmbus/isl68137.c static int isl68137_probe(struct i2c_client *client) · line 406

Quoted 10 lines
drivers/hwmon/pmbus/isl68137.c +2 -0
@@ -263,11 +397,18 @@ static int isl68137_probe(struct i2c_client *client)
  		info->m[PSC_POWER] = 2;
  		info->R[PSC_POWER] = -1;
  		info->read_word_data = raa_dmpvr2_read_word_data;
+		info->write_word_data = raa_dmpvr2_write_word_data;
  		break;
  	default:
  		return -ENODEV;
  	}
 
+	if (dev->of_node) {

This conditional should not be necessary because for_each_child_of_node() ultimately calls __of_get_next_child() which checks if the node pointer is NULL.

Quoted 70 lines
drivers/hwmon/pmbus/isl68137.c +55 -2
+		err = isl68137_probe_from_dt(dev, data);
+		if (err)
+			return err;
+	}
+
  	return pmbus_do_probe(client, info);
  }
 

@@ -318,11 +459,59 @@ static const struct i2c_device_id raa_dmpvr_id[] = {
 
  MODULE_DEVICE_TABLE(i2c, raa_dmpvr_id);
 
+static const struct of_device_id isl68137_of_match[] = {
+	{ .compatible = "renesas,isl68137", .data = (void *)raa_dmpvr1_2rail },
+	{ .compatible = "renesas,isl68220", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl68221", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl68222", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl68223", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl68224", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl68225", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl68226", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl68227", .data = (void *)raa_dmpvr2_1rail },
+	{ .compatible = "renesas,isl68229", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl68233", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl68239", .data = (void *)raa_dmpvr2_3rail },
+
+	{ .compatible = "renesas,isl69222", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69223", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl69224", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69225", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69227", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl69228", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl69234", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69236", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69239", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl69242", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69243", .data = (void *)raa_dmpvr2_1rail },
+	{ .compatible = "renesas,isl69247", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69248", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69254", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69255", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69256", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69259", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69260", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69268", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,isl69269", .data = (void *)raa_dmpvr2_3rail },
+	{ .compatible = "renesas,isl69298", .data = (void *)raa_dmpvr2_2rail },
+
+	{ .compatible = "renesas,raa228000", .data = (void *)raa_dmpvr2_hv },
+	{ .compatible = "renesas,raa228004", .data = (void *)raa_dmpvr2_hv },
+	{ .compatible = "renesas,raa228006", .data = (void *)raa_dmpvr2_hv },
+	{ .compatible = "renesas,raa228228", .data = (void *)raa_dmpvr2_2rail_nontc },
+	{ .compatible = "renesas,raa229001", .data = (void *)raa_dmpvr2_2rail },
+	{ .compatible = "renesas,raa229004", .data = (void *)raa_dmpvr2_2rail },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, isl68137_of_match);
+
  /* This is the driver that will be inserted */
  static struct i2c_driver isl68137_driver = {
  	.driver = {
-		   .name = "isl68137",
-		   },
+		.name = "isl68137",
+		.of_match_table = isl68137_of_match,
+	},
  	.probe = isl68137_probe,
  	.id_table = raa_dmpvr_id,
  };
author
#5 lore raw

Re: [PATCH v2 1/2] hwmon: (pmbus/isl68137) add support for voltage divider on Vout

Hi Guenter,

Thank you for the review!

Quoted 15 lines · Guenter Roeck
Quoted 14 lines · Grant Peltier

+ [...]

drivers/hwmon/pmbus/isl68137.c +13 -0
+	switch (reg) {
+	case PMBUS_VOUT_MAX:
+		/*
+		 * In cases where a voltage divider is attached to the target
+		 * rail between Vout and the Vsense pin, Vout related PMBus
+		 * commands should be scaled based on the expected voltage
+		 * at the Vsense pin.
+		 * I.e. Vsense = Vout * R2 / (R1 + R2)
+		 */
+		fallthrough;
+	case PMBUS_VOUT_MARGIN_HIGH:
+		fallthrough;
+	case PMBUS_VOUT_MARGIN_LOW:
Quoted 9 lines

drivers/hwmon/pmbus/isl68137.c static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page, · line 239

Quoted 5 lines
drivers/hwmon/pmbus/isl68137.c +5 -0
+		fallthrough;
+	case PMBUS_VOUT_OV_FAULT_LIMIT:
+		fallthrough;
+	case PMBUS_VOUT_UV_FAULT_LIMIT:
+		fallthrough;

Just add the comment after the last case and drop all the fallthrough; Same above.

Will fix in v4

Quoted 7 lines

drivers/hwmon/pmbus/isl68137.c static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page, · line 242

Quoted 3 lines
drivers/hwmon/pmbus/isl68137.c +3 -0
+	case PMBUS_VOUT_COMMAND:
+		if (data->channel[page].vout_voltage_divider[0]
+			&& data->channel[page].vout_voltage_divider[1]) {

It would be better to set defaults instead of having to check this for every executed command (for example by setting R1:=0 and R2:=1).

Sounds reasonable. I will adjust the channel initialization process to set defaults instead and will remove the checks in v4.

Quoted 10 lines
Quoted 10 lines

[...]

drivers/hwmon/pmbus/isl68137.c +9 -0
+static int isl68137_probe_child_from_dt(struct device *dev,
+					struct device_node *child,
+					struct isl68137_data *data)
+{
+	u32 channel;
+	int err;
+
+	err = of_property_read_u32(child, "reg", &channel);
+	if (err) {
Quoted 9 lines

drivers/hwmon/pmbus/isl68137.c static struct pmbus_driver_info raa_dmpvr_info = { · line 313

Quoted 4 lines
drivers/hwmon/pmbus/isl68137.c +4 -0
+		dev_err(dev, "missing reg property of %pOFn\n", child);
+		return err;
+	}
+	if (channel >= MAX_CHANNELS) {

The actual number of channels (pages) supported by the chip is known here and should be checked, either by passing the number of channels or a pointer to the entire info structure to this function.

Will fix in v4.

Quoted 9 lines

drivers/hwmon/pmbus/isl68137.c static struct pmbus_driver_info raa_dmpvr_info = { · line 318

Quoted 5 lines
drivers/hwmon/pmbus/isl68137.c +5 -0
+		dev_err(dev, "invalid reg %d of %pOFn\n", channel, child);
+		return -EINVAL;
+	}
+
+	of_property_read_u32_array(child, "renesas,vout-voltage-divider",

Ultimately this potentially applies to _all_ hardware monitoring chips, so I would very much prefer a generic voltage divider property definition.

There is a parallel conversation on PATCH v3 2/2 about this. Would you prefer that I match the implementation for maxim20730?

Quoted 5 lines

drivers/hwmon/pmbus/isl68137.c static struct pmbus_driver_info raa_dmpvr_info = { · line 320

Quoted 2 lines
drivers/hwmon/pmbus/isl68137.c +2 -0
+				data->channel[channel].vout_voltage_divider,
+				ARRAY_SIZE(data->channel[channel].vout_voltage_divider));

The returned data should be be validated here.

Fixed in v3.

Quoted 34 lines
Quoted 34 lines
drivers/hwmon/pmbus/isl68137.c +28 -2
+
+	return 0;
+}
+
+static int isl68137_probe_from_dt(struct device *dev,
+				  struct isl68137_data *data)
+{
+	const struct device_node *np = dev->of_node;
+	struct device_node *child;
+	int err;
+
+	for_each_child_of_node(np, child) {
+		if (strcmp(child->name, "channel"))
+			continue;
+
+		err = isl68137_probe_child_from_dt(dev, child, data);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
  static int isl68137_probe(struct i2c_client *client)
  {
+	struct device *dev = &client->dev;
  	struct pmbus_driver_info *info;
+	struct isl68137_data *data;
+	int i, err;
-	info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL);
-	if (!info)
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
  		return -ENOMEM;
Quoted 9 lines

drivers/hwmon/pmbus/isl68137.c static struct pmbus_driver_info raa_dmpvr_info = { · line 358

Quoted 6 lines
drivers/hwmon/pmbus/isl68137.c +5 -1
-	memcpy(info, &raa_dmpvr_info, sizeof(*info));
+
+	for (i = 0; i < MAX_CHANNELS; i++)
+		memset(data->channel[i].vout_voltage_divider,
+			0,
+			sizeof(data->channel[i].vout_voltage_divider));

Under what circumstance would this not already be 0 after devm_kzalloc() ?

Mental lapse on my end. Will change to set harmless defaults discussed above.

Quoted 6 lines

drivers/hwmon/pmbus/isl68137.c static int isl68137_probe(struct i2c_client *client) · line 406

Quoted 2 lines

+ [...]

drivers/hwmon/pmbus/isl68137.c +1 -0
+	if (dev->of_node) {

This conditional should not be necessary because for_each_child_of_node() ultimately calls __of_get_next_child() which checks if the node pointer is NULL.

Will remove in v4.

Quoted 4 lines

drivers/hwmon/pmbus/isl68137.c static int isl68137_probe(struct i2c_client *client)

Quoted 4 lines
drivers/hwmon/pmbus/isl68137.c +4 -0
+		err = isl68137_probe_from_dt(dev, data);
+		if (err)
+			return err;
+ [...]

Thanks again,
Grant

maintainer
#6 lore raw

Re: [PATCH v2 1/2] hwmon: (pmbus/isl68137) add support for voltage divider on Vout

On 10/24/24 12:43, Grant Peltier wrote:
[ ... ]

Quoted 9 lines
Quoted 5 lines

drivers/hwmon/pmbus/isl68137.c static struct pmbus_driver_info raa_dmpvr_info = { · line 318

Quoted 1 line
drivers/hwmon/pmbus/isl68137.c +1 -0
+	of_property_read_u32_array(child, "renesas,vout-voltage-divider",

Ultimately this potentially applies to _all_ hardware monitoring chips, so I would very much prefer a generic voltage divider property definition.

There is a parallel conversation on PATCH v3 2/2 about this. Would you prefer that I match the implementation for maxim20730?

I would prefer, in the order of preference,

1) an applicable generic property definition
2) a definition that is already used elsewhere
3) a new chips specific definition

From my perspective, matching the maxim20730 implementation should only be considered if the generic definition does not meet the chip requirements.

Thanks,
Guenter

author
#7 lore raw

PATCH v2 2/2 dt-bindings: hwmon: isl68137: add bindings to support voltage dividers

In reply to Grant Peltier

Add devicetree bindings to support declaring optional voltage dividers to the rail outputs of supported digital multiphase regulators. Some applications require Vout to exceed the voltage range that the Vsense pin can detect. This binding definition allows users to define the characteristics of a voltage divider placed between Vout and the Vsense pin for any rail powered by the device.

Signed-off-by Grant Peltier <[email protected]>
---
 .../hwmon/pmbus/renesas,isl68137.yaml         | 131 ++++++++++++++++++
 1 file changed, 131 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml
Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml new file +131 -0
new file mode 100644
index 000000000000..af10c55d547f
@@ -0,0 +1,131 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/pmbus/renesas,isl68137.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas Digital Multiphase Voltage Regulators with PMBus
+
+maintainers:
+  - Grant Peltier <[email protected]>
+
+description: |
+  Renesas digital multiphase voltage regulators with PMBus.
+  https://www.renesas.com/en/products/power-management/multiphase-power/multiphase-dcdc-switching-controllers
+
+properties:
+  compatible:
+    enum:
+      - renesas,isl68220
+      - renesas,isl68221
+      - renesas,isl68222
+      - renesas,isl68223
+      - renesas,isl68224
+      - renesas,isl68225
+      - renesas,isl68226
+      - renesas,isl68227
+      - renesas,isl68229
+      - renesas,isl68233
+      - renesas,isl68239
+      - renesas,isl69222
+      - renesas,isl69223
+      - renesas,isl69224
+      - renesas,isl69225
+      - renesas,isl69227
+      - renesas,isl69228
+      - renesas,isl69234
+      - renesas,isl69236
+      - renesas,isl69239
+      - renesas,isl69242
+      - renesas,isl69243
+      - renesas,isl69247
+      - renesas,isl69248
+      - renesas,isl69254
+      - renesas,isl69255
+      - renesas,isl69256
+      - renesas,isl69259
+      - renesas,isl69260
+      - renesas,isl69268
+      - renesas,isl69269
+      - renesas,isl69298
+      - renesas,raa228000
+      - renesas,raa228004
+      - renesas,raa228006
+      - renesas,raa228228
+      - renesas,raa229001
+      - renesas,raa229004
+
+  reg:
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+patternProperties:
+  "^channel@([0-3])$":
+    type: object
+    description:
+      Container for properties specific to a particular channel (rail).
+
+    properties:
+      reg:
+        description: The channel (rail) index.
+        items:
+          minimum: 0
+          maximum: 3
+
+      renesas,vout-voltage-divider:
+        description:
+          Resistances of a voltage divider placed between Vout and the voltage
+          sense pin for the given channel (rail). It has two numbers
+          representing the resistances of the voltage divider provided as
+          <R1 R2> which yields an adjusted Vout as
+          Vout_adj = Vout * (R1 + R2) / R2 given the original Vout as reported
+          by the Vsense pin.
+        $ref: /schemas/types.yaml#/definitions/uint32-array
+        minItems: 2
+        maxItems: 2
+
+    required:
+      - reg
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      isl68239@60 {
+        compatible = "renesas,isl68239";
+        reg = <0x60>;
+      };
+    };
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      isl68239@60 {
+        compatible = "renesas,isl68239";
+        reg = <0x60>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        channel@0 {
+          reg = <0>;
+          renesas,vout-voltage-divider = <1000 1000>;  // Reported Vout/Pout would be scaled by 2
+        };
+      };
+    };
Signature · 1 line
2.39.5