* [PATCH v3 0/2] dt-bindings: hwmon: pmbus: add bindings for isl68137 @ 2024-10-23 20:52 Grant Peltier 2024-10-23 20:53 ` [PATCH v3 1/2] hwmon: (pmbus/isl68137) add support for voltage divider on Vout Grant Peltier 2024-10-23 20:53 ` [PATCH v3 2/2] dt-bindings: hwmon: isl68137: add bindings to support voltage dividers Grant Peltier 0 siblings, 2 replies; 6+ messages in thread From: Grant Peltier @ 2024-10-23 20:52 UTC (permalink / raw) To: robh, linux, geert+renesas, magnus.damm Cc: grant.peltier.jg, brandon.howell.jg, linux-hwmon, devicetree, linux-doc 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. v3: - Report and return errors reading the vout-voltage-divider property from the devicetree when the property is defined - Change u64 division/rounding operations to use explicit math64 macros 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 | 204 +++++++++++++++++- 2 files changed, 330 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml -- 2.39.5 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/2] hwmon: (pmbus/isl68137) add support for voltage divider on Vout 2024-10-23 20:52 [PATCH v3 0/2] dt-bindings: hwmon: pmbus: add bindings for isl68137 Grant Peltier @ 2024-10-23 20:53 ` Grant Peltier 2024-10-23 20:53 ` [PATCH v3 2/2] dt-bindings: hwmon: isl68137: add bindings to support voltage dividers Grant Peltier 1 sibling, 0 replies; 6+ messages in thread From: Grant Peltier @ 2024-10-23 20:53 UTC (permalink / raw) To: robh, linux, geert+renesas, magnus.damm Cc: grant.peltier.jg, brandon.howell.jg, linux-hwmon, devicetree, linux-doc 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 <grantpeltier93@gmail.com> --- drivers/hwmon/pmbus/isl68137.c | 204 ++++++++++++++++++++++++++++++++- 1 file changed, 199 insertions(+), 5 deletions(-) diff --git a/drivers/hwmon/pmbus/isl68137.c b/drivers/hwmon/pmbus/isl68137.c index 7e53fb1d5ea3..24b5adffb526 100644 --- a/drivers/hwmon/pmbus/isl68137.c +++ b/drivers/hwmon/pmbus/isl68137.c @@ -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_U64_ROUND_CLOSEST((u64)ret * + ((u64)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 = DIV64_U64_ROUND_CLOSEST((u64)word * + data->channel[page].vout_voltage_divider[1], + ((u64)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,72 @@ 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; + } + + err = of_property_read_u32_array(child, "renesas,vout-voltage-divider", + data->channel[channel].vout_voltage_divider, + ARRAY_SIZE(data->channel[channel].vout_voltage_divider)); + if (err && err != -EINVAL) { + dev_err(dev, + "malformed renesas,vout-voltage-divider value for channel %d\n", + channel); + return err; + } + 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 +378,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 +387,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 +402,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 +464,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, }; -- 2.39.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] dt-bindings: hwmon: isl68137: add bindings to support voltage dividers 2024-10-23 20:52 [PATCH v3 0/2] dt-bindings: hwmon: pmbus: add bindings for isl68137 Grant Peltier 2024-10-23 20:53 ` [PATCH v3 1/2] hwmon: (pmbus/isl68137) add support for voltage divider on Vout Grant Peltier @ 2024-10-23 20:53 ` Grant Peltier 2024-10-24 17:01 ` Conor Dooley 1 sibling, 1 reply; 6+ messages in thread From: Grant Peltier @ 2024-10-23 20:53 UTC (permalink / raw) To: robh, linux, geert+renesas, magnus.damm Cc: grant.peltier.jg, brandon.howell.jg, linux-hwmon, devicetree, linux-doc 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 <grantpeltier93@gmail.com> --- .../hwmon/pmbus/renesas,isl68137.yaml | 131 ++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml new file mode 100644 index 000000000000..af10c55d547f --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml @@ -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 <grant.peltier.jg@renesas.com> + +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 + }; + }; + }; -- 2.39.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] dt-bindings: hwmon: isl68137: add bindings to support voltage dividers 2024-10-23 20:53 ` [PATCH v3 2/2] dt-bindings: hwmon: isl68137: add bindings to support voltage dividers Grant Peltier @ 2024-10-24 17:01 ` Conor Dooley 2024-10-24 18:32 ` Grant Peltier 0 siblings, 1 reply; 6+ messages in thread From: Conor Dooley @ 2024-10-24 17:01 UTC (permalink / raw) To: Grant Peltier Cc: robh, linux, geert+renesas, magnus.damm, grant.peltier.jg, brandon.howell.jg, linux-hwmon, devicetree, linux-doc, jic23, Peter Rosin [-- Attachment #1: Type: text/plain, Size: 5399 bytes --] On Wed, Oct 23, 2024 at 03:53:51PM -0500, Grant Peltier wrote: > 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 <grantpeltier93@gmail.com> > --- > .../hwmon/pmbus/renesas,isl68137.yaml | 131 ++++++++++++++++++ > 1 file changed, 131 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml > new file mode 100644 > index 000000000000..af10c55d547f > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml > @@ -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 <grant.peltier.jg@renesas.com> > + > +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 Damn, that;s a list and a half, innit! Looking briefly at the driver change, the match data implies that quite a few of these actually would be suitable for fallback compatibles. > + > + 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: There's already a binding for voltage dividers: voltage-divider.yaml That said, I have no idea how that would work with an extant driver for the hardware like we have here. I'd imagine it would really have to be used with iio-hwmon? + Peter and Jonathan, since I don't know how the driver side of using the voltage divider works. > + 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>; > + }; > + }; Without any channels, what does this actually do? If you've got no channels you cannot measure anything making this example invalid? Thanks, Conor. > + - | > + 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 > + }; > + }; > + }; > -- > 2.39.5 > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] dt-bindings: hwmon: isl68137: add bindings to support voltage dividers 2024-10-24 17:01 ` Conor Dooley @ 2024-10-24 18:32 ` Grant Peltier 2024-10-31 12:55 ` Conor Dooley 0 siblings, 1 reply; 6+ messages in thread From: Grant Peltier @ 2024-10-24 18:32 UTC (permalink / raw) To: Conor Dooley Cc: robh, linux, geert+renesas, magnus.damm, grant.peltier.jg, brandon.howell.jg, linux-hwmon, devicetree, linux-doc, jic23, Peter Rosin Hi Conor, Thank you for your review! On Thu, Oct 24, 2024 at 06:01:11PM +0100, Conor Dooley wrote: > On Wed, Oct 23, 2024 at 03:53:51PM -0500, Grant Peltier wrote: > > + [...] > > + > > +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 > > Damn, that;s a list and a half, innit! Looking briefly at the driver > change, the match data implies that quite a few of these actually would > be suitable for fallback compatibles. Yes, there are quite a few part numbers (and likely to be more in the future). My intention was to make the driver more user friendly since the variants listed in the driver do not map to something in any of the datasheets. So using those instead would require users to inspect the source of the driver instead of simply referencing their part number(s). > > + > > + 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: > > There's already a binding for voltage dividers: voltage-divider.yaml > That said, I have no idea how that would work with an extant driver for > the hardware like we have here. I'd imagine it would really have to be > used with iio-hwmon? + Peter and Jonathan, since I don't know how the > driver side of using the voltage divider works. In his recent revier, Guenter requested using a standard voltage divider schema as well. I see there is an implementation in maxim,maxim20730.yaml but that differs from the one in voltage-divider.yaml. Should I opt to match maxim,maxim20730.yaml? > > > + 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>; > > + }; > > + }; > > Without any channels, what does this actually do? If you've got no > channels you cannot measure anything making this example invalid? > > Thanks, > Conor. > > > + [...] The channel structures are optional to allow users to arbitrarily define voltage dividers for any particular rail. Omitting the channel definitions still allow the device to be instantiated and probed as an I2C device along with all related hwmon PMBus telemetry dictated by the part variant. Thanks again, Grant ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] dt-bindings: hwmon: isl68137: add bindings to support voltage dividers 2024-10-24 18:32 ` Grant Peltier @ 2024-10-31 12:55 ` Conor Dooley 0 siblings, 0 replies; 6+ messages in thread From: Conor Dooley @ 2024-10-31 12:55 UTC (permalink / raw) To: Grant Peltier Cc: robh, linux, geert+renesas, magnus.damm, grant.peltier.jg, brandon.howell.jg, linux-hwmon, devicetree, linux-doc, jic23, Peter Rosin [-- Attachment #1: Type: text/plain, Size: 4701 bytes --] On Thu, Oct 24, 2024 at 01:32:09PM -0500, Grant Peltier wrote: > On Thu, Oct 24, 2024 at 06:01:11PM +0100, Conor Dooley wrote: > > On Wed, Oct 23, 2024 at 03:53:51PM -0500, Grant Peltier wrote: > > > + [...] > > > + > > > +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 > > > > Damn, that;s a list and a half, innit! Looking briefly at the driver > > change, the match data implies that quite a few of these actually would > > be suitable for fallback compatibles. > > Yes, there are quite a few part numbers (and likely to be more in the > future). My intention was to make the driver more user friendly since the > variants listed in the driver do not map to something in any of the > datasheets. So using those instead would require users to inspect the > source of the driver instead of simply referencing their part number(s). I don't understand. How would a fallback materially change anything in that regard? You still put the compatible corresponding to the device you have in your dts. A fallback means having multiple compatible strings in the property, not a single one corresponding to another device. > > > + 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: > > > > There's already a binding for voltage dividers: voltage-divider.yaml > > That said, I have no idea how that would work with an extant driver for > > the hardware like we have here. I'd imagine it would really have to be > > used with iio-hwmon? + Peter and Jonathan, since I don't know how the > > driver side of using the voltage divider works. > > In his recent revier, Guenter requested using a standard voltage divider > schema as well. I see there is an implementation in maxim,maxim20730.yaml > but that differs from the one in voltage-divider.yaml. Should I opt to > match maxim,maxim20730.yaml? I would rather the standard binding was used, but it would probably involve having to hook up iio-rescale to hwmon? I don't know enough about that, which is why I Cced Peter and Jonathan. > > > +examples: > > > + - | > > > + i2c { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + isl68239@60 { > > > + compatible = "renesas,isl68239"; > > > + reg = <0x60>; > > > + }; > > > + }; > > > > Without any channels, what does this actually do? If you've got no > > channels you cannot measure anything making this example invalid? > > The channel structures are optional to allow users to arbitrarily define > voltage dividers for any particular rail. Omitting the channel definitions > still allow the device to be instantiated and probed as an I2C device > along with all related hwmon PMBus telemetry dictated by the part variant. I dunno, either the channels are hooked up to something or they are not. If they are, the channels should be populated in the devicetree. Cheers, Conor. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-10-31 12:55 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-23 20:52 [PATCH v3 0/2] dt-bindings: hwmon: pmbus: add bindings for isl68137 Grant Peltier 2024-10-23 20:53 ` [PATCH v3 1/2] hwmon: (pmbus/isl68137) add support for voltage divider on Vout Grant Peltier 2024-10-23 20:53 ` [PATCH v3 2/2] dt-bindings: hwmon: isl68137: add bindings to support voltage dividers Grant Peltier 2024-10-24 17:01 ` Conor Dooley 2024-10-24 18:32 ` Grant Peltier 2024-10-31 12:55 ` Conor Dooley
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).