dt-bindings: hwmon: pmbus: add bindings for isl68137
Subsystems (3)
v5 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.
v5:
- Fix clang compilation errors related to C23 syntax
v4:
- Revert devicetree property name to "vout-voltage-divider" and refactor property description and driver implementation to match existing vout-voltage-divider implementation in max20730 as no suitable generic voltage divider schema exists.
- Minor fixes based on Guenter's review of v2.
- Initialize voltage dividers for all channels to defaults that simplify logic in PMBus word read/write functions.
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 | 147 ++++++++++++ drivers/hwmon/pmbus/isl68137.c | 209 +++++++++++++++++- 2 files changed, 351 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml
Signature · 1 line
2.39.5
PATCH v5 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.
--- drivers/hwmon/pmbus/isl68137.c | 209 ++++++++++++++++++++++++++++++++- 1 file changed, 204 insertions(+), 5 deletions(-)
drivers/hwmon/pmbus/isl68137.c +204 -5
@@ -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 4enum 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,13 +176,32 @@ 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; +u64 temp;switch (reg) {case PMBUS_VIRT_READ_VMON : ret =pmbus_read_word_data (client, page, phase,RAA_DMPVR2_READ_VMON );break ; +case PMBUS_READ_POUT : +case PMBUS_READ_VOUT : +/* +* 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 * Rtotal / Rout +*/ + ret =pmbus_read_word_data (client, page, phase, reg); +if (ret >0 ) { + temp =DIV_U64_ROUND_CLOSEST ((u64 )ret * + data->channel[page].vout_voltage_divider[1 ], + data->channel[page].vout_voltage_divider[0 ]); + ret =clamp_val (temp,0 ,0xffff ); + } +break ;default : ret = -ENODATA ;break ;
@@ -178,6 +210,40 @@ 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; +u64 temp; + +switch (reg) { +case PMBUS_VOUT_MAX : +case PMBUS_VOUT_MARGIN_HIGH : +case PMBUS_VOUT_MARGIN_LOW : +case PMBUS_VOUT_OV_FAULT_LIMIT : +case PMBUS_VOUT_UV_FAULT_LIMIT : +case PMBUS_VOUT_COMMAND : +/* +* 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 * Rout / Rtotal +*/ + temp =DIV_U64_ROUND_CLOSEST ((u64 )word * + data->channel[page].vout_voltage_divider[0 ], + data->channel[page].vout_voltage_divider[1 ]); + ret =clamp_val (temp,0 ,0xffff ); +break ; +default : + ret = -ENODATA ; +break ; + } +return ret; +} +static struct pmbus_driver_info raa_dmpvr_info = { .pages =3 , .format[PSC_VOLTAGE_IN ] = direct,
@@ -220,14 +286,91 @@ 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, rout, rtotal; +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 >= data->info.pages) { +dev_err (dev,"invalid reg %d of %pOFn\n" , channel, child); +return -EINVAL ; + } + + err =of_property_read_u32_array (child,"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 vout-voltage-divider value for channel %d\n" , + channel); +return err; + } + + rout = data->channel[channel].vout_voltage_divider[0 ]; + rtotal = data->channel[channel].vout_voltage_divider[1 ]; +if (rout ==0 ) { +dev_err (dev, +"Voltage divider output resistance must be greater than 0\n" ); +return -EINVAL ; + } +if (rtotal < rout) { +dev_err (dev, +"Voltage divider total resistance is less than output resistance\n" ); +return -EINVAL ; + } + +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)); + +/* +* Initialize all voltage dividers to Rout=1 and Rtotal=1 to simplify +* logic in PMBus word read/write functions +*/ +for (i =0 ; i <MAX_CHANNELS ; i++) +memset (data->channel[i].vout_voltage_divider, +1 , +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 +385,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 +394,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 +409,16 @@ 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 ; } + err =isl68137_probe_from_dt (dev, data); +if (err) +return err; +return pmbus_do_probe (client, info); }
@@ -318,11 +469,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
PATCH v5 2/2 dt-bindings: hwmon: isl68137: add bindings to support voltage dividers
In reply to Grant PeltierAdd 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.
--- .../hwmon/pmbus/renesas,isl68137.yaml | 147 ++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml
Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml new file +147 -0
@@ -0,0 +1,147 @@ +# 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 + + vout-voltage-divider: + description: | + Resistances of a voltage divider placed between Vout and the voltage + sense (Vsense) pin for the given channel (rail). It has two numbers + representing the resistances of the voltage divider provided as + <Rout Rtotal> which yields an adjusted Vout as + Vout_adj = Vout * Rtotal / Rout given the original Vout as reported + by the Vsense pin. Given a circuit configuration similar to the one + below, Rtotal = R1 + Rout. + + Vout ----. + | + .-----. + | R1 | + '-----' + | + +---- Vsense + | + .-----. + | Rout| + '-----' + | + GND + + $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>; + vout-voltage-divider = <1000 2000>; // Reported Vout/Pout would be scaled by 2 + }; + }; + };
Signature · 1 line
2.39.5
Re: [PATCH v5 2/2] dt-bindings: hwmon: isl68137: add bindings to support voltage dividers
Quoted 7 lines · 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.
Quoted 6 lines
Signed-off-by Grant Peltier <[email protected]>--- .../hwmon/pmbus/renesas,isl68137.yaml | 147 ++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/renesas,isl68137.yaml
Hi Grant,
Quoted 14 lines · Grant Peltier
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.
Quoted 2 lines
v5:
- Fix clang compilation errors related to C23 syntax
Thanks for the update!
Quoted 5 lines
v4:
- Revert devicetree property name to "vout-voltage-divider" and refactor property description and driver implementation to match existing vout-voltage-divider implementation in max20730 as no suitable generic voltage divider schema exists.
Can you please elaborate (or point to the email that did so, in case I missed it)?
In reply to v2, Günter wrote:
"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"
https://lore.kernel.org/all/[email protected]
Thanks!
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
Quoted 17 lines · Geert Uytterhoeven
Quoted 5 lines
v4:
- Revert devicetree property name to "vout-voltage-divider" and refactor property description and driver implementation to match existing vout-voltage-divider implementation in max20730 as no suitable generic voltage divider schema exists.Can you please elaborate (or point to the email that did so, in case I missed it)?
In reply to v2, Günter wrote:
"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"
Quoted 8 lines
https://lore.kernel.org/all/[email protected]
Thanks!
Gr{oetje,eeting}s,
Geert
Hi Geert,
After looking through existing bindings definitions, the only generic voltage divider property that I found was one that is intended to scale voltages for IIO io-channels. However, the use case here is to scale particular hwmon PMBus telemetry, which most closely matches the existing implementation of the vout-voltage-divider property in the max20730 driver so I opted to copy that implementation based on Guenter's preferences. Is there something that I missed or more information that you are looking for?
Thanks,
Grant
Guenter Roeck
maintainerQuoted 29 lines · Grant Peltier
Quoted 17 lines · Geert Uytterhoeven
Quoted 5 lines
v4:
- Revert devicetree property name to "vout-voltage-divider" and refactor property description and driver implementation to match existing vout-voltage-divider implementation in max20730 as no suitable generic voltage divider schema exists.Can you please elaborate (or point to the email that did so, in case I missed it)?
In reply to v2, Günter wrote:
"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"Quoted 8 lines
https://lore.kernel.org/all/[email protected]
Thanks!
Gr{oetje,eeting}s,
Geert
Hi Geert,
Quoted 9 lines
After looking through existing bindings definitions, the only generic voltage divider property that I found was one that is intended to scale voltages for IIO io-channels. However, the use case here is to scale particular hwmon PMBus telemetry, which most closely matches the existing implementation of the vout-voltage-divider property in the max20730 driver so I opted to copy that implementation based on Guenter's preferences. Is there something that I missed or more information that you are looking for?
I had a look myself. I agree with Grant, the existing bindings are really not a good match. They are directly tied to iio, the implementation in drivers/iio/afe/iio-rescale.c is completely iio specific, and I see no means to use it for anything but iio. The problem is that it is a "compatible" property, not parameter property (or set of properties) as I would have expected [sorry if I use the wrong terminology].
We could possibly use the actual properties, "output-ohms" and "full-ohms", but I am not really sure if that makes too much sense.
Note that there are a variety of voltage divider properties.
gw,voltage-divider-ohms
lltc,fb-voltage-divider
mps,fb-voltage-divider
vout-voltage-divider
All those specify a pair of resistors. I think that model is much better suited to the problem at hand than a "compatible".
I agree though that this should be explained in the patch description.
Guenter