* [PATCH 0/2] hwmon: ltc2945: Add binding and shunt resistor support
@ 2022-12-14 22:07 Cormier, Jonathan
2022-12-14 22:07 ` [PATCH 1/2] dt-bindings: hwmon: adi,ltc2945: Add binding Cormier, Jonathan
` (2 more replies)
0 siblings, 3 replies; 32+ messages in thread
From: Cormier, Jonathan @ 2022-12-14 22:07 UTC (permalink / raw)
To: linux-hwmon
Cc: Cormier, Jonathan, Jean Delvare, Guenter Roeck, linux-kernel,
devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke,
John Pruitt
Added the ability to specify the value of the shunt resistor in the
device tree instead of assuming it is 1 milliOhm.
Would be good to backport as well
Cormier, Jonathan (1):
dt-bindings: hwmon: adi,ltc2945: Add binding
John Pruitt (1):
hwmon: ltc2945: Allow setting shunt resistor
.../bindings/hwmon/adi,ltc2945.yaml | 50 +++++++++++
drivers/hwmon/ltc2945.c | 86 ++++++++++++++-----
2 files changed, 115 insertions(+), 21 deletions(-)
create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml
--
2.25.1
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 1/2] dt-bindings: hwmon: adi,ltc2945: Add binding 2022-12-14 22:07 [PATCH 0/2] hwmon: ltc2945: Add binding and shunt resistor support Cormier, Jonathan @ 2022-12-14 22:07 ` Cormier, Jonathan 2022-12-15 9:42 ` Krzysztof Kozlowski 2022-12-14 22:07 ` [PATCH 2/2] hwmon: ltc2945: Allow setting shunt resistor Cormier, Jonathan 2022-12-20 0:04 ` [PATCH v2 0/2] hwmon: ltc2945: Add binding and shunt resistor support Cormier, Jonathan 2 siblings, 1 reply; 32+ messages in thread From: Cormier, Jonathan @ 2022-12-14 22:07 UTC (permalink / raw) To: linux-hwmon Cc: Cormier, Jonathan, Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> --- .../bindings/hwmon/adi,ltc2945.yaml | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml new file mode 100644 index 000000000000..9ca7a886dec8 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml @@ -0,0 +1,50 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/adi,ltc2945.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices LTC2945 wide range i2c power monitor + +maintainers: + - Guenter Roeck <linux@roeck-us.net> + +description: | + Analog Devices LTC2945 wide range i2c power monitor over I2C. + + https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2945.pdf + +properties: + compatible: + enum: + - ltc2945 + + reg: + maxItems: 1 + + shunt-resistor-micro-ohms: + description: + Shunt resistor value in micro-Ohms + default: 1000 + +required: + - compatible + - reg + + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + ltc2945_i2c: ltc2945@6e { + compatible = "ltc2945"; + reg = <0x6e>; + /* 10 milli-Ohm shunt resistor */ + shunt-resistor-micro-ohms = <10000>; + }; + }; +... -- 2.25.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: adi,ltc2945: Add binding 2022-12-14 22:07 ` [PATCH 1/2] dt-bindings: hwmon: adi,ltc2945: Add binding Cormier, Jonathan @ 2022-12-15 9:42 ` Krzysztof Kozlowski [not found] ` <CADL8D3ZUE5WbV0oS6hEVUNh9asrhTKQeGR4McR6Kh6qykSFw=Q@mail.gmail.com> 0 siblings, 1 reply; 32+ messages in thread From: Krzysztof Kozlowski @ 2022-12-15 9:42 UTC (permalink / raw) To: Cormier, Jonathan, linux-hwmon Cc: Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt On 14/12/2022 23:07, Cormier, Jonathan wrote: Missing commit msg. Describe hardware. > Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> > --- > .../bindings/hwmon/adi,ltc2945.yaml | 50 +++++++++++++++++++ > 1 file changed, 50 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml > new file mode 100644 > index 000000000000..9ca7a886dec8 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml > @@ -0,0 +1,50 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hwmon/adi,ltc2945.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices LTC2945 wide range i2c power monitor > + > +maintainers: > + - Guenter Roeck <linux@roeck-us.net> > + > +description: | > + Analog Devices LTC2945 wide range i2c power monitor over I2C. > + > + https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2945.pdf > + > +properties: > + compatible: > + enum: > + - ltc2945 That's not a correct compatible. Missing vendor prefix. > + > + reg: > + maxItems: 1 > + > + shunt-resistor-micro-ohms: > + description: > + Shunt resistor value in micro-Ohms > + default: 1000 > + > +required: > + - compatible > + - reg > + > + Just one blank line. > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; Use 4 spaces for example indentation. > + > + ltc2945_i2c: ltc2945@6e { Node names should be generic. https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "ltc2945"; > + reg = <0x6e>; > + /* 10 milli-Ohm shunt resistor */ > + shunt-resistor-micro-ohms = <10000>; > + }; > + }; > +... Best regards, Krzysztof ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <CADL8D3ZUE5WbV0oS6hEVUNh9asrhTKQeGR4McR6Kh6qykSFw=Q@mail.gmail.com>]
* Re: [PATCH 1/2] dt-bindings: hwmon: adi,ltc2945: Add binding [not found] ` <CADL8D3ZUE5WbV0oS6hEVUNh9asrhTKQeGR4McR6Kh6qykSFw=Q@mail.gmail.com> @ 2022-12-15 14:37 ` Jon Cormier 0 siblings, 0 replies; 32+ messages in thread From: Jon Cormier @ 2022-12-15 14:37 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: linux-hwmon, Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt Resending reply in plain text On Thu, Dec 15, 2022 at 9:35 AM Jon Cormier <jcormier@criticallink.com> wrote: > > On Thu, Dec 15, 2022 at 4:42 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: >> >> On 14/12/2022 23:07, Cormier, Jonathan wrote: >> >> Missing commit msg. Describe hardware. > > Thanks >> >> >> >> >> > Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> >> > --- >> > .../bindings/hwmon/adi,ltc2945.yaml | 50 +++++++++++++++++++ >> > 1 file changed, 50 insertions(+) >> > create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml >> > >> > diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml >> > new file mode 100644 >> > index 000000000000..9ca7a886dec8 >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml >> > @@ -0,0 +1,50 @@ >> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> > +%YAML 1.2 >> > +--- >> > +$id: http://devicetree.org/schemas/hwmon/adi,ltc2945.yaml# >> > +$schema: http://devicetree.org/meta-schemas/core.yaml# >> > + >> > +title: Analog Devices LTC2945 wide range i2c power monitor >> > + >> > +maintainers: >> > + - Guenter Roeck <linux@roeck-us.net> >> > + >> > +description: | >> > + Analog Devices LTC2945 wide range i2c power monitor over I2C. >> > + >> > + https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2945.pdf >> > + >> > +properties: >> > + compatible: >> > + enum: >> > + - ltc2945 >> >> That's not a correct compatible. Missing vendor prefix. >> > Okay, I assume I'll need to update the driver as well to handle that. >> >> > + >> > + reg: >> > + maxItems: 1 >> > + >> > + shunt-resistor-micro-ohms: >> > + description: >> > + Shunt resistor value in micro-Ohms >> > + default: 1000 >> > + >> > +required: >> > + - compatible >> > + - reg >> > + >> > + >> >> Just one blank line. >> >> > +additionalProperties: false >> > + >> > +examples: >> > + - | >> > + i2c { >> > + #address-cells = <1>; >> > + #size-cells = <0>; >> >> Use 4 spaces for example indentation. >> >> > + >> > + ltc2945_i2c: ltc2945@6e { >> >> Node names should be generic. >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation >> >> > + compatible = "ltc2945"; >> > + reg = <0x6e>; >> > + /* 10 milli-Ohm shunt resistor */ >> > + shunt-resistor-micro-ohms = <10000>; >> > + }; >> > + }; >> > +... >> >> Best regards, >> Krzysztof >> > > > -- > Jonathan Cormier > Software Engineer > > Voice: 315.425.4045 x222 > > > > http://www.CriticalLink.com > 6712 Brooklawn Parkway, Syracuse, NY 13211 > > -- Jonathan Cormier Software Engineer Voice: 315.425.4045 x222 http://www.CriticalLink.com 6712 Brooklawn Parkway, Syracuse, NY 13211 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 2/2] hwmon: ltc2945: Allow setting shunt resistor 2022-12-14 22:07 [PATCH 0/2] hwmon: ltc2945: Add binding and shunt resistor support Cormier, Jonathan 2022-12-14 22:07 ` [PATCH 1/2] dt-bindings: hwmon: adi,ltc2945: Add binding Cormier, Jonathan @ 2022-12-14 22:07 ` Cormier, Jonathan 2022-12-15 15:11 ` Guenter Roeck 2023-01-09 23:35 ` [PATCH v3 0/2] hwmon: ltc2945: Add binding and shunt resistor support Jonathan Cormier 2022-12-20 0:04 ` [PATCH v2 0/2] hwmon: ltc2945: Add binding and shunt resistor support Cormier, Jonathan 2 siblings, 2 replies; 32+ messages in thread From: Cormier, Jonathan @ 2022-12-14 22:07 UTC (permalink / raw) To: linux-hwmon Cc: John Pruitt, Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, Cormier, Jonathan From: John Pruitt <jpruitt@criticallink.com> Added the ability to specify the value of the shunt resistor in the device tree instead of assuming it is 1 milliOhm. The value in the device tree has the name shunt-resistor-micro-ohms and the default value is 1000 micro-ohms in order to preserve the current behavior. Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> Signed-off-by: John Pruitt <jpruitt@criticallink.com> --- drivers/hwmon/ltc2945.c | 86 +++++++++++++++++++++++++++++++---------- 1 file changed, 65 insertions(+), 21 deletions(-) diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c index 9adebb59f604..4e8b92e53133 100644 --- a/drivers/hwmon/ltc2945.c +++ b/drivers/hwmon/ltc2945.c @@ -58,6 +58,11 @@ #define CONTROL_MULT_SELECT (1 << 0) #define CONTROL_TEST_MODE (1 << 4) +struct ltc2945_data { + struct regmap *regmap; + u32 shunt_resistor; // number of microohms +}; + static inline bool is_power_reg(u8 reg) { return reg < LTC2945_SENSE_H; @@ -66,7 +71,9 @@ static inline bool is_power_reg(u8 reg) /* Return the value from the given register in uW, mV, or mA */ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) { - struct regmap *regmap = dev_get_drvdata(dev); + struct ltc2945_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; + u32 shunt_resistor = data->shunt_resistor; unsigned int control; u8 buf[3]; long long val; @@ -93,8 +100,11 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) case LTC2945_MIN_POWER_THRES_H: /* * Convert to uW by assuming current is measured with - * an 1mOhm sense resistor, similar to current - * measurements. + * a 1000 microOhm sense resistor (aka shunt resistor) + * or what is specified in DT, similar to current + * measurements. The shunt_resistor value is in + * microOhms. + * * Control register bit 0 selects if voltage at SENSE+/VDD * or voltage at ADIN is used to measure power. */ @@ -108,6 +118,8 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) /* 0.5 mV * 25 uV = 0.0125 uV resolution. */ val = (val * 25LL) >> 1; } + val *= 1000; + val = DIV_ROUND_CLOSEST_ULL(val, shunt_resistor); break; case LTC2945_VIN_H: case LTC2945_MAX_VIN_H: @@ -132,12 +144,15 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) case LTC2945_MIN_SENSE_THRES_H: /* * 25 uV resolution. Convert to current as measured with - * an 1 mOhm sense resistor, in mA. If a different sense - * resistor is installed, calculate the actual current by - * dividing the reported current by the sense resistor value - * in mOhm. + * an 1000 microOhm sense (or shunt) resistor, in mA. + * If a different shunt resistor is installed, calculate + * the actual current by dividing the reported current + * by the shunt resistor value in microOhms. The actual + * The actual shunt resistor value can be specified + * in the device tree. */ - val *= 25; + val *= 25 * 1000; + val = DIV_ROUND_CLOSEST_ULL(val, shunt_resistor); break; default: return -EINVAL; @@ -148,7 +163,9 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) static int ltc2945_val_to_reg(struct device *dev, u8 reg, unsigned long val) { - struct regmap *regmap = dev_get_drvdata(dev); + struct ltc2945_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; + u32 shunt_resistor = data->shunt_resistor; unsigned int control; int ret; @@ -160,8 +177,10 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, case LTC2945_MIN_POWER_THRES_H: /* * Convert to register value by assuming current is measured - * with an 1mOhm sense resistor, similar to current + * with a 1000 microOhm sense resistor, (aka shunt resistor) + * or what is specified in DT, similar to current * measurements. + * * Control register bit 0 selects if voltage at SENSE+/VDD * or voltage at ADIN is used to measure power, which in turn * determines register calculations. @@ -171,14 +190,16 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, return ret; if (control & CONTROL_MULT_SELECT) { /* 25 mV * 25 uV = 0.625 uV resolution. */ - val = DIV_ROUND_CLOSEST(val, 625); + val *= shunt_resistor; + val = DIV_ROUND_CLOSEST(val, 625 * 1000); } else { /* * 0.5 mV * 25 uV = 0.0125 uV resolution. * Divide first to avoid overflow; * accept loss of accuracy. */ - val = DIV_ROUND_CLOSEST(val, 25) * 2; + val *= shunt_resistor; + val = DIV_ROUND_CLOSEST(val, 25 * 1000) * 2; } break; case LTC2945_VIN_H: @@ -204,12 +225,14 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, case LTC2945_MIN_SENSE_THRES_H: /* * 25 uV resolution. Convert to current as measured with - * an 1 mOhm sense resistor, in mA. If a different sense - * resistor is installed, calculate the actual current by - * dividing the reported current by the sense resistor value - * in mOhm. + * a 1000 microOhm sense (or shunt) resistor, in mA. If + * a different shunt resistor is installed, calculate + * the actual current by dividing the reported current + * by the sense resistor value in microOhms. The actual + * shunt resistor value can be specified in the device tree. */ - val = DIV_ROUND_CLOSEST(val, 25); + val *= shunt_resistor; + val = DIV_ROUND_CLOSEST(val, 25 * 1000); break; default: return -EINVAL; @@ -234,7 +257,8 @@ static ssize_t ltc2945_value_store(struct device *dev, const char *buf, size_t count) { struct sensor_device_attribute *attr = to_sensor_dev_attr(da); - struct regmap *regmap = dev_get_drvdata(dev); + struct ltc2945_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; u8 reg = attr->index; unsigned long val; u8 regbuf[3]; @@ -269,7 +293,8 @@ static ssize_t ltc2945_history_store(struct device *dev, const char *buf, size_t count) { struct sensor_device_attribute *attr = to_sensor_dev_attr(da); - struct regmap *regmap = dev_get_drvdata(dev); + struct ltc2945_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; u8 reg = attr->index; int num_regs = is_power_reg(reg) ? 3 : 2; u8 buf_min[3] = { 0xff, 0xff, 0xff }; @@ -321,7 +346,8 @@ static ssize_t ltc2945_bool_show(struct device *dev, struct device_attribute *da, char *buf) { struct sensor_device_attribute *attr = to_sensor_dev_attr(da); - struct regmap *regmap = dev_get_drvdata(dev); + struct ltc2945_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; unsigned int fault; int ret; @@ -450,6 +476,14 @@ static int ltc2945_probe(struct i2c_client *client) struct device *dev = &client->dev; struct device *hwmon_dev; struct regmap *regmap; + u32 shunt_resistor; + + struct ltc2945_data *data; + + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + dev_set_drvdata(dev, data); regmap = devm_regmap_init_i2c(client, <c2945_regmap_config); if (IS_ERR(regmap)) { @@ -457,11 +491,21 @@ static int ltc2945_probe(struct i2c_client *client) return PTR_ERR(regmap); } + data->regmap = regmap; + if (of_property_read_u32(client->dev.of_node, + "shunt-resistor-micro-ohms", &shunt_resistor)) + shunt_resistor = 1000; /* 1000 micro-Ohm if not set via DT */ + + if (shunt_resistor == 0) + return -EINVAL; + + data->shunt_resistor = shunt_resistor; + /* Clear faults */ regmap_write(regmap, LTC2945_FAULT, 0x00); hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name, - regmap, + data, ltc2945_groups); return PTR_ERR_OR_ZERO(hwmon_dev); } -- 2.25.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] hwmon: ltc2945: Allow setting shunt resistor 2022-12-14 22:07 ` [PATCH 2/2] hwmon: ltc2945: Allow setting shunt resistor Cormier, Jonathan @ 2022-12-15 15:11 ` Guenter Roeck 2022-12-15 19:42 ` Jon Cormier 2023-01-09 23:35 ` [PATCH v3 0/2] hwmon: ltc2945: Add binding and shunt resistor support Jonathan Cormier 1 sibling, 1 reply; 32+ messages in thread From: Guenter Roeck @ 2022-12-15 15:11 UTC (permalink / raw) To: Cormier, Jonathan, linux-hwmon Cc: John Pruitt, Jean Delvare, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke On 12/14/22 14:07, Cormier, Jonathan wrote: > From: John Pruitt <jpruitt@criticallink.com> > > Added the ability to specify the value of the shunt resistor in the > device tree instead of assuming it is 1 milliOhm. The value in the > device tree has the name shunt-resistor-micro-ohms and the > default value is 1000 micro-ohms in order to preserve the > current behavior. > Also needs a devicetree match table. > Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> > Signed-off-by: John Pruitt <jpruitt@criticallink.com> > --- > drivers/hwmon/ltc2945.c | 86 +++++++++++++++++++++++++++++++---------- > 1 file changed, 65 insertions(+), 21 deletions(-) > > diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c > index 9adebb59f604..4e8b92e53133 100644 > --- a/drivers/hwmon/ltc2945.c > +++ b/drivers/hwmon/ltc2945.c > @@ -58,6 +58,11 @@ > #define CONTROL_MULT_SELECT (1 << 0) > #define CONTROL_TEST_MODE (1 << 4) > > +struct ltc2945_data { > + struct regmap *regmap; > + u32 shunt_resistor; // number of microohms No C++ comments in this code, please. Also, "number of" is not needed. Something like "in microohms" or just "microohms" would be sufficient. > +}; > + > static inline bool is_power_reg(u8 reg) > { > return reg < LTC2945_SENSE_H; > @@ -66,7 +71,9 @@ static inline bool is_power_reg(u8 reg) > /* Return the value from the given register in uW, mV, or mA */ > static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > { > - struct regmap *regmap = dev_get_drvdata(dev); > + struct ltc2945_data *data = dev_get_drvdata(dev); > + struct regmap *regmap = data->regmap; > + u32 shunt_resistor = data->shunt_resistor; > unsigned int control; > u8 buf[3]; > long long val; > @@ -93,8 +100,11 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > case LTC2945_MIN_POWER_THRES_H: > /* > * Convert to uW by assuming current is measured with > - * an 1mOhm sense resistor, similar to current > - * measurements. > + * a 1000 microOhm sense resistor (aka shunt resistor) > + * or what is specified in DT, similar to current > + * measurements. The shunt_resistor value is in > + * microOhms. The units are already in the variable declaration. No need to repeat. There is no need to replace "1 mOhm" with "1000 microOhm". It might make sense to drop all those comments, add a kerneldoc comment to struct ltc2945_data, and explain it all there. > + * > * Control register bit 0 selects if voltage at SENSE+/VDD > * or voltage at ADIN is used to measure power. > */ > @@ -108,6 +118,8 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > /* 0.5 mV * 25 uV = 0.0125 uV resolution. */ > val = (val * 25LL) >> 1; > } > + val *= 1000; > + val = DIV_ROUND_CLOSEST_ULL(val, shunt_resistor); > break; > case LTC2945_VIN_H: > case LTC2945_MAX_VIN_H: > @@ -132,12 +144,15 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > case LTC2945_MIN_SENSE_THRES_H: > /* > * 25 uV resolution. Convert to current as measured with > - * an 1 mOhm sense resistor, in mA. If a different sense > - * resistor is installed, calculate the actual current by > - * dividing the reported current by the sense resistor value > - * in mOhm. > + * an 1000 microOhm sense (or shunt) resistor, in mA. > + * If a different shunt resistor is installed, calculate > + * the actual current by dividing the reported current > + * by the shunt resistor value in microOhms. The actual > + * The actual shunt resistor value can be specified > + * in the device tree. All but the last sentence just replace 1 mOhm with 1000 microOhm, which does not add any value. Applies to all similar changes; I won't comment on it anymore below. > */ > - val *= 25; > + val *= 25 * 1000; > + val = DIV_ROUND_CLOSEST_ULL(val, shunt_resistor); > break; > default: > return -EINVAL; > @@ -148,7 +163,9 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > static int ltc2945_val_to_reg(struct device *dev, u8 reg, > unsigned long val) > { > - struct regmap *regmap = dev_get_drvdata(dev); > + struct ltc2945_data *data = dev_get_drvdata(dev); > + struct regmap *regmap = data->regmap; > + u32 shunt_resistor = data->shunt_resistor; > unsigned int control; > int ret; > > @@ -160,8 +177,10 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, > case LTC2945_MIN_POWER_THRES_H: > /* > * Convert to register value by assuming current is measured > - * with an 1mOhm sense resistor, similar to current > + * with a 1000 microOhm sense resistor, (aka shunt resistor) > + * or what is specified in DT, similar to current > * measurements. > + * > * Control register bit 0 selects if voltage at SENSE+/VDD > * or voltage at ADIN is used to measure power, which in turn > * determines register calculations. > @@ -171,14 +190,16 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, > return ret; > if (control & CONTROL_MULT_SELECT) { > /* 25 mV * 25 uV = 0.625 uV resolution. */ > - val = DIV_ROUND_CLOSEST(val, 625); > + val *= shunt_resistor; This can now easily result in an overflow. > + val = DIV_ROUND_CLOSEST(val, 625 * 1000); > } else { > /* > * 0.5 mV * 25 uV = 0.0125 uV resolution. > * Divide first to avoid overflow; > * accept loss of accuracy. > */ > - val = DIV_ROUND_CLOSEST(val, 25) * 2; > + val *= shunt_resistor; > + val = DIV_ROUND_CLOSEST(val, 25 * 1000) * 2; > } > break; > case LTC2945_VIN_H: > @@ -204,12 +225,14 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, > case LTC2945_MIN_SENSE_THRES_H: > /* > * 25 uV resolution. Convert to current as measured with > - * an 1 mOhm sense resistor, in mA. If a different sense > - * resistor is installed, calculate the actual current by > - * dividing the reported current by the sense resistor value > - * in mOhm. > + * a 1000 microOhm sense (or shunt) resistor, in mA. If > + * a different shunt resistor is installed, calculate > + * the actual current by dividing the reported current > + * by the sense resistor value in microOhms. The actual > + * shunt resistor value can be specified in the device tree. > */ > - val = DIV_ROUND_CLOSEST(val, 25); > + val *= shunt_resistor; Needs to ensure that there are no overflows. Try to write the maximum value for unsigned long and see what happens. > + val = DIV_ROUND_CLOSEST(val, 25 * 1000); > break; > default: > return -EINVAL; > @@ -234,7 +257,8 @@ static ssize_t ltc2945_value_store(struct device *dev, > const char *buf, size_t count) > { > struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > - struct regmap *regmap = dev_get_drvdata(dev); > + struct ltc2945_data *data = dev_get_drvdata(dev); > + struct regmap *regmap = data->regmap; > u8 reg = attr->index; > unsigned long val; > u8 regbuf[3]; > @@ -269,7 +293,8 @@ static ssize_t ltc2945_history_store(struct device *dev, > const char *buf, size_t count) > { > struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > - struct regmap *regmap = dev_get_drvdata(dev); > + struct ltc2945_data *data = dev_get_drvdata(dev); > + struct regmap *regmap = data->regmap; > u8 reg = attr->index; > int num_regs = is_power_reg(reg) ? 3 : 2; > u8 buf_min[3] = { 0xff, 0xff, 0xff }; > @@ -321,7 +346,8 @@ static ssize_t ltc2945_bool_show(struct device *dev, > struct device_attribute *da, char *buf) > { > struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > - struct regmap *regmap = dev_get_drvdata(dev); > + struct ltc2945_data *data = dev_get_drvdata(dev); > + struct regmap *regmap = data->regmap; > unsigned int fault; > int ret; > > @@ -450,6 +476,14 @@ static int ltc2945_probe(struct i2c_client *client) > struct device *dev = &client->dev; > struct device *hwmon_dev; > struct regmap *regmap; > + u32 shunt_resistor; > + drop this empty line > + struct ltc2945_data *data; > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + dev_set_drvdata(dev, data); > > regmap = devm_regmap_init_i2c(client, <c2945_regmap_config); > if (IS_ERR(regmap)) { > @@ -457,11 +491,21 @@ static int ltc2945_probe(struct i2c_client *client) > return PTR_ERR(regmap); > } > > + data->regmap = regmap; > + if (of_property_read_u32(client->dev.of_node, > + "shunt-resistor-micro-ohms", &shunt_resistor)) Please consider using device_property_read_u32(). > + shunt_resistor = 1000; /* 1000 micro-Ohm if not set via DT */ "if not set via DT" -> "default value". With device_property_read_u32(), this can be set by other means, not just DT. > + > + if (shunt_resistor == 0) > + return -EINVAL; > + > + data->shunt_resistor = shunt_resistor; > + > /* Clear faults */ > regmap_write(regmap, LTC2945_FAULT, 0x00); > > hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name, > - regmap, > + data, > ltc2945_groups); > return PTR_ERR_OR_ZERO(hwmon_dev); > } ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] hwmon: ltc2945: Allow setting shunt resistor 2022-12-15 15:11 ` Guenter Roeck @ 2022-12-15 19:42 ` Jon Cormier 0 siblings, 0 replies; 32+ messages in thread From: Jon Cormier @ 2022-12-15 19:42 UTC (permalink / raw) To: Guenter Roeck Cc: linux-hwmon, John Pruitt, Jean Delvare, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke On Thu, Dec 15, 2022 at 10:11 AM Guenter Roeck <linux@roeck-us.net> wrote: > > On 12/14/22 14:07, Cormier, Jonathan wrote: > > From: John Pruitt <jpruitt@criticallink.com> > > > > Added the ability to specify the value of the shunt resistor in the > > device tree instead of assuming it is 1 milliOhm. The value in the > > device tree has the name shunt-resistor-micro-ohms and the > > default value is 1000 micro-ohms in order to preserve the > > current behavior. > > > > Also needs a devicetree match table. On it > > > Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> > > Signed-off-by: John Pruitt <jpruitt@criticallink.com> > > --- > > drivers/hwmon/ltc2945.c | 86 +++++++++++++++++++++++++++++++---------- > > 1 file changed, 65 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c > > index 9adebb59f604..4e8b92e53133 100644 > > --- a/drivers/hwmon/ltc2945.c > > +++ b/drivers/hwmon/ltc2945.c > > @@ -58,6 +58,11 @@ > > #define CONTROL_MULT_SELECT (1 << 0) > > #define CONTROL_TEST_MODE (1 << 4) > > > > +struct ltc2945_data { > > + struct regmap *regmap; > > + u32 shunt_resistor; // number of microohms > > No C++ comments in this code, please. Also, "number of" is not needed. > Something like "in microohms" or just "microohms" would be sufficient. > > > +}; > > + > > static inline bool is_power_reg(u8 reg) > > { > > return reg < LTC2945_SENSE_H; > > @@ -66,7 +71,9 @@ static inline bool is_power_reg(u8 reg) > > /* Return the value from the given register in uW, mV, or mA */ > > static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > > { > > - struct regmap *regmap = dev_get_drvdata(dev); > > + struct ltc2945_data *data = dev_get_drvdata(dev); > > + struct regmap *regmap = data->regmap; > > + u32 shunt_resistor = data->shunt_resistor; > > unsigned int control; > > u8 buf[3]; > > long long val; > > @@ -93,8 +100,11 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > > case LTC2945_MIN_POWER_THRES_H: > > /* > > * Convert to uW by assuming current is measured with > > - * an 1mOhm sense resistor, similar to current > > - * measurements. > > + * a 1000 microOhm sense resistor (aka shunt resistor) > > + * or what is specified in DT, similar to current > > + * measurements. The shunt_resistor value is in > > + * microOhms. > > The units are already in the variable declaration. No need to repeat. > There is no need to replace "1 mOhm" with "1000 microOhm". > > It might make sense to drop all those comments, add a kerneldoc comment > to struct ltc2945_data, and explain it all there. Will do > > > + * > > * Control register bit 0 selects if voltage at SENSE+/VDD > > * or voltage at ADIN is used to measure power. > > */ > > @@ -108,6 +118,8 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > > /* 0.5 mV * 25 uV = 0.0125 uV resolution. */ > > val = (val * 25LL) >> 1; > > } > > + val *= 1000; > > + val = DIV_ROUND_CLOSEST_ULL(val, shunt_resistor); > > break; > > case LTC2945_VIN_H: > > case LTC2945_MAX_VIN_H: > > @@ -132,12 +144,15 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > > case LTC2945_MIN_SENSE_THRES_H: > > /* > > * 25 uV resolution. Convert to current as measured with > > - * an 1 mOhm sense resistor, in mA. If a different sense > > - * resistor is installed, calculate the actual current by > > - * dividing the reported current by the sense resistor value > > - * in mOhm. > > + * an 1000 microOhm sense (or shunt) resistor, in mA. > > + * If a different shunt resistor is installed, calculate > > + * the actual current by dividing the reported current > > + * by the shunt resistor value in microOhms. The actual > > + * The actual shunt resistor value can be specified > > + * in the device tree. > > All but the last sentence just replace 1 mOhm with 1000 microOhm, which > does not add any value. Applies to all similar changes; I won't comment > on it anymore below. > > > */ > > - val *= 25; > > + val *= 25 * 1000; > > + val = DIV_ROUND_CLOSEST_ULL(val, shunt_resistor); > > break; > > default: > > return -EINVAL; > > @@ -148,7 +163,9 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > > static int ltc2945_val_to_reg(struct device *dev, u8 reg, > > unsigned long val) > > { > > - struct regmap *regmap = dev_get_drvdata(dev); > > + struct ltc2945_data *data = dev_get_drvdata(dev); > > + struct regmap *regmap = data->regmap; > > + u32 shunt_resistor = data->shunt_resistor; > > unsigned int control; > > int ret; > > > > @@ -160,8 +177,10 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, > > case LTC2945_MIN_POWER_THRES_H: > > /* > > * Convert to register value by assuming current is measured > > - * with an 1mOhm sense resistor, similar to current > > + * with a 1000 microOhm sense resistor, (aka shunt resistor) > > + * or what is specified in DT, similar to current > > * measurements. > > + * > > * Control register bit 0 selects if voltage at SENSE+/VDD > > * or voltage at ADIN is used to measure power, which in turn > > * determines register calculations. > > @@ -171,14 +190,16 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, > > return ret; > > if (control & CONTROL_MULT_SELECT) { > > /* 25 mV * 25 uV = 0.625 uV resolution. */ > > - val = DIV_ROUND_CLOSEST(val, 625); > > + val *= shunt_resistor; > > This can now easily result in an overflow. Okay will fix > > > + val = DIV_ROUND_CLOSEST(val, 625 * 1000); > > } else { > > /* > > * 0.5 mV * 25 uV = 0.0125 uV resolution. > > * Divide first to avoid overflow; > > * accept loss of accuracy. > > */ > > - val = DIV_ROUND_CLOSEST(val, 25) * 2; > > + val *= shunt_resistor; > > + val = DIV_ROUND_CLOSEST(val, 25 * 1000) * 2; > > } > > break; > > case LTC2945_VIN_H: > > @@ -204,12 +225,14 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, > > case LTC2945_MIN_SENSE_THRES_H: > > /* > > * 25 uV resolution. Convert to current as measured with > > - * an 1 mOhm sense resistor, in mA. If a different sense > > - * resistor is installed, calculate the actual current by > > - * dividing the reported current by the sense resistor value > > - * in mOhm. > > + * a 1000 microOhm sense (or shunt) resistor, in mA. If > > + * a different shunt resistor is installed, calculate > > + * the actual current by dividing the reported current > > + * by the sense resistor value in microOhms. The actual > > + * shunt resistor value can be specified in the device tree. > > */ > > - val = DIV_ROUND_CLOSEST(val, 25); > > + val *= shunt_resistor; > > Needs to ensure that there are no overflows. Try to write the maximum value > for unsigned long and see what happens. > > > + val = DIV_ROUND_CLOSEST(val, 25 * 1000); > > break; > > default: > > return -EINVAL; > > @@ -234,7 +257,8 @@ static ssize_t ltc2945_value_store(struct device *dev, > > const char *buf, size_t count) > > { > > struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > > - struct regmap *regmap = dev_get_drvdata(dev); > > + struct ltc2945_data *data = dev_get_drvdata(dev); > > + struct regmap *regmap = data->regmap; > > u8 reg = attr->index; > > unsigned long val; > > u8 regbuf[3]; > > @@ -269,7 +293,8 @@ static ssize_t ltc2945_history_store(struct device *dev, > > const char *buf, size_t count) > > { > > struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > > - struct regmap *regmap = dev_get_drvdata(dev); > > + struct ltc2945_data *data = dev_get_drvdata(dev); > > + struct regmap *regmap = data->regmap; > u8 reg = attr->index; > > int num_regs = is_power_reg(reg) ? 3 : 2; > > u8 buf_min[3] = { 0xff, 0xff, 0xff }; > > @@ -321,7 +346,8 @@ static ssize_t ltc2945_bool_show(struct device *dev, > > struct device_attribute *da, char *buf) > > { > > struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > > - struct regmap *regmap = dev_get_drvdata(dev); > > + struct ltc2945_data *data = dev_get_drvdata(dev); > > + struct regmap *regmap = data->regmap; > > unsigned int fault; > > int ret; > > > > @@ -450,6 +476,14 @@ static int ltc2945_probe(struct i2c_client *client) > > struct device *dev = &client->dev; > > struct device *hwmon_dev; > > struct regmap *regmap; > > + u32 shunt_resistor; > > + > drop this empty line > > > + struct ltc2945_data *data; > > + > > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > > + if (!data) > > + return -ENOMEM; > > + dev_set_drvdata(dev, data); > > > > regmap = devm_regmap_init_i2c(client, <c2945_regmap_config); > > if (IS_ERR(regmap)) { > > @@ -457,11 +491,21 @@ static int ltc2945_probe(struct i2c_client *client) > > return PTR_ERR(regmap); > > } > > > > + data->regmap = regmap; > > + if (of_property_read_u32(client->dev.of_node, > > + "shunt-resistor-micro-ohms", &shunt_resistor)) > > Please consider using device_property_read_u32(). > > > + shunt_resistor = 1000; /* 1000 micro-Ohm if not set via DT */ > > "if not set via DT" -> "default value". With device_property_read_u32(), > this can be set by other means, not just DT. > > > + > > + if (shunt_resistor == 0) > > + return -EINVAL; > > + > > + data->shunt_resistor = shunt_resistor; > > + > > /* Clear faults */ > > regmap_write(regmap, LTC2945_FAULT, 0x00); > > > > hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name, > > - regmap, > > + data, > > ltc2945_groups); > > return PTR_ERR_OR_ZERO(hwmon_dev); > > } > -- Jonathan Cormier Software Engineer Voice: 315.425.4045 x222 http://www.CriticalLink.com 6712 Brooklawn Parkway, Syracuse, NY 13211 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 0/2] hwmon: ltc2945: Add binding and shunt resistor support 2022-12-14 22:07 ` [PATCH 2/2] hwmon: ltc2945: Allow setting shunt resistor Cormier, Jonathan 2022-12-15 15:11 ` Guenter Roeck @ 2023-01-09 23:35 ` Jonathan Cormier 2023-01-09 23:35 ` [PATCH v3 1/5] dt-bindings: hwmon: adi,ltc2945: Add binding Jonathan Cormier ` (4 more replies) 1 sibling, 5 replies; 32+ messages in thread From: Jonathan Cormier @ 2023-01-09 23:35 UTC (permalink / raw) To: linux-hwmon Cc: Jonathan Cormier, Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt Added the ability to specify the value of the shunt resistor in the device tree instead of assuming it is 1 milliOhm. Would be good to backport as well Changes since v2: - Remove newline - Combined overflow fix with "Allow setting shunt resistor" commit - Add description to "Add devicetree match table" - Add fix for unhandled error case in ltc2945_value_store - Use imperative in descriptions - Remove unneeded overflow check from ltc2945_reg_to_val - Fix up DIV_ROUND_CLOSEST_ULL calls, per docs divisor should be 32-bit - Split one DIV_ROUND_CLOSEST_ULL change into separate commit per Guenter Roeck's comment Changes since v1: - Add devicetree match table - Add kerneldoc for the ltc2945_data struct - Cleanup excesive comments about the shunt resistor - Switch to device_property_read_u32() Cormier, Jonathan (1): dt-bindings: hwmon: adi,ltc2945: Add binding John Pruitt (1): hwmon: ltc2945: Allow setting shunt resistor .../bindings/hwmon/adi,ltc2945.yaml | 50 +++++++++++ drivers/hwmon/ltc2945.c | 86 ++++++++++++++----- 2 files changed, 115 insertions(+), 21 deletions(-) create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml -- 2.25.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 1/5] dt-bindings: hwmon: adi,ltc2945: Add binding 2023-01-09 23:35 ` [PATCH v3 0/2] hwmon: ltc2945: Add binding and shunt resistor support Jonathan Cormier @ 2023-01-09 23:35 ` Jonathan Cormier 2023-01-10 9:07 ` Krzysztof Kozlowski 2023-01-09 23:35 ` [PATCH v3 2/5] hwmon: ltc2945: Add devicetree match table Jonathan Cormier ` (3 subsequent siblings) 4 siblings, 1 reply; 32+ messages in thread From: Jonathan Cormier @ 2023-01-09 23:35 UTC (permalink / raw) To: linux-hwmon Cc: Jonathan Cormier, Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt Create initial binding for the LTC2945 I2C power monitor. Add shunt-resistor-micro-ohms parameter Signed-off-by: Jonathan Cormier <jcormier@criticallink.com> --- .../bindings/hwmon/adi,ltc2945.yaml | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml new file mode 100644 index 000000000000..5cb66e97e816 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml @@ -0,0 +1,49 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/adi,ltc2945.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices LTC2945 wide range i2c power monitor + +maintainers: + - Guenter Roeck <linux@roeck-us.net> + +description: | + Analog Devices LTC2945 wide range i2c power monitor over I2C. + + https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2945.pdf + +properties: + compatible: + enum: + - adi,ltc2945 + + reg: + maxItems: 1 + + shunt-resistor-micro-ohms: + description: + Shunt resistor value in micro-Ohms + default: 1000 + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + power-monitor@6e { + compatible = "adi,ltc2945"; + reg = <0x6e>; + /* 10 milli-Ohm shunt resistor */ + shunt-resistor-micro-ohms = <10000>; + }; + }; +... -- 2.25.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v3 1/5] dt-bindings: hwmon: adi,ltc2945: Add binding 2023-01-09 23:35 ` [PATCH v3 1/5] dt-bindings: hwmon: adi,ltc2945: Add binding Jonathan Cormier @ 2023-01-10 9:07 ` Krzysztof Kozlowski 0 siblings, 0 replies; 32+ messages in thread From: Krzysztof Kozlowski @ 2023-01-10 9:07 UTC (permalink / raw) To: Jonathan Cormier, linux-hwmon Cc: Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt On 10/01/2023 00:35, Jonathan Cormier wrote: > Create initial binding for the LTC2945 I2C power monitor. > Add shunt-resistor-micro-ohms parameter If you want to keep that sentence, sure, but let's make it a sentence - missing full stop. Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 2/5] hwmon: ltc2945: Add devicetree match table 2023-01-09 23:35 ` [PATCH v3 0/2] hwmon: ltc2945: Add binding and shunt resistor support Jonathan Cormier 2023-01-09 23:35 ` [PATCH v3 1/5] dt-bindings: hwmon: adi,ltc2945: Add binding Jonathan Cormier @ 2023-01-09 23:35 ` Jonathan Cormier 2023-01-09 23:35 ` [PATCH v3 3/5] hwmon: ltc2945: Handle error case in ltc2945_value_store Jonathan Cormier ` (2 subsequent siblings) 4 siblings, 0 replies; 32+ messages in thread From: Jonathan Cormier @ 2023-01-09 23:35 UTC (permalink / raw) To: linux-hwmon Cc: Jonathan Cormier, Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt Add adi,ltc2945 compatible Signed-off-by: Jonathan Cormier <jcormier@criticallink.com> --- drivers/hwmon/ltc2945.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c index 9adebb59f604..9af3e3821152 100644 --- a/drivers/hwmon/ltc2945.c +++ b/drivers/hwmon/ltc2945.c @@ -58,6 +58,12 @@ #define CONTROL_MULT_SELECT (1 << 0) #define CONTROL_TEST_MODE (1 << 4) +static const struct of_device_id __maybe_unused ltc2945_of_match[] = { + { .compatible = "adi,ltc2945" }, + { } +}; +MODULE_DEVICE_TABLE(of, ltc2945_of_match); + static inline bool is_power_reg(u8 reg) { return reg < LTC2945_SENSE_H; @@ -475,8 +481,9 @@ MODULE_DEVICE_TABLE(i2c, ltc2945_id); static struct i2c_driver ltc2945_driver = { .driver = { - .name = "ltc2945", - }, + .name = "ltc2945", + .of_match_table = of_match_ptr(ltc2945_of_match), + }, .probe_new = ltc2945_probe, .id_table = ltc2945_id, }; -- 2.25.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 3/5] hwmon: ltc2945: Handle error case in ltc2945_value_store 2023-01-09 23:35 ` [PATCH v3 0/2] hwmon: ltc2945: Add binding and shunt resistor support Jonathan Cormier 2023-01-09 23:35 ` [PATCH v3 1/5] dt-bindings: hwmon: adi,ltc2945: Add binding Jonathan Cormier 2023-01-09 23:35 ` [PATCH v3 2/5] hwmon: ltc2945: Add devicetree match table Jonathan Cormier @ 2023-01-09 23:35 ` Jonathan Cormier 2023-01-10 0:04 ` Guenter Roeck 2023-01-09 23:35 ` [PATCH v3 4/5] hwmon: ltc2945: Allow setting shunt resistor Jonathan Cormier 2023-01-09 23:35 ` [PATCH v3 5/5] hwmon: ltc2945: Convert division to DIV_ROUND_CLOSEST_ULL Jonathan Cormier 4 siblings, 1 reply; 32+ messages in thread From: Jonathan Cormier @ 2023-01-09 23:35 UTC (permalink / raw) To: linux-hwmon Cc: Jonathan Cormier, Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt ltc2945_val_to_reg errors were not being handled which would have resulted in register being set to 0 (clamped) instead of being left alone. Change reg_to_val and val_to_reg to return values via parameters to make it more obvious when an error case isn't handled. Also to allow the regval type to be the correct sign in prep for next commits. Fixes: 6700ce035f83 ("hwmon: Driver for Linear Technologies LTC2945") Signed-off-by: Jonathan Cormier <jcormier@criticallink.com> --- drivers/hwmon/ltc2945.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c index 9af3e3821152..c66acf8d2124 100644 --- a/drivers/hwmon/ltc2945.c +++ b/drivers/hwmon/ltc2945.c @@ -70,12 +70,12 @@ static inline bool is_power_reg(u8 reg) } /* Return the value from the given register in uW, mV, or mA */ -static long long ltc2945_reg_to_val(struct device *dev, u8 reg) +static int ltc2945_reg_to_val(struct device *dev, u8 reg, u64 *regval) { struct regmap *regmap = dev_get_drvdata(dev); unsigned int control; u8 buf[3]; - long long val; + u64 val; int ret; ret = regmap_bulk_read(regmap, reg, buf, @@ -148,11 +148,12 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) default: return -EINVAL; } - return val; + *regval = val; + return 0; } static int ltc2945_val_to_reg(struct device *dev, u8 reg, - unsigned long val) + unsigned long val, unsigned long *regval) { struct regmap *regmap = dev_get_drvdata(dev); unsigned int control; @@ -220,19 +221,21 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, default: return -EINVAL; } - return val; + *regval = val; + return 0; } static ssize_t ltc2945_value_show(struct device *dev, struct device_attribute *da, char *buf) { struct sensor_device_attribute *attr = to_sensor_dev_attr(da); - long long value; + int ret; + u64 value; - value = ltc2945_reg_to_val(dev, attr->index); - if (value < 0) - return value; - return sysfs_emit(buf, "%lld\n", value); + ret = ltc2945_reg_to_val(dev, attr->index, &value); + if (ret < 0) + return ret; + return sysfs_emit(buf, "%llu\n", value); } static ssize_t ltc2945_value_store(struct device *dev, @@ -245,7 +248,7 @@ static ssize_t ltc2945_value_store(struct device *dev, unsigned long val; u8 regbuf[3]; int num_regs; - int regval; + unsigned long regval; int ret; ret = kstrtoul(buf, 10, &val); @@ -253,7 +256,10 @@ static ssize_t ltc2945_value_store(struct device *dev, return ret; /* convert to register value, then clamp and write result */ - regval = ltc2945_val_to_reg(dev, reg, val); + ret = ltc2945_val_to_reg(dev, reg, val, ®val); + if (ret < 0) + return ret; + if (is_power_reg(reg)) { regval = clamp_val(regval, 0, 0xffffff); regbuf[0] = regval >> 16; -- 2.25.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v3 3/5] hwmon: ltc2945: Handle error case in ltc2945_value_store 2023-01-09 23:35 ` [PATCH v3 3/5] hwmon: ltc2945: Handle error case in ltc2945_value_store Jonathan Cormier @ 2023-01-10 0:04 ` Guenter Roeck 2023-01-10 18:19 ` Jon Cormier 0 siblings, 1 reply; 32+ messages in thread From: Guenter Roeck @ 2023-01-10 0:04 UTC (permalink / raw) To: Jonathan Cormier, linux-hwmon Cc: Jean Delvare, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt On 1/9/23 15:35, Jonathan Cormier wrote: > ltc2945_val_to_reg errors were not being handled > which would have resulted in register being set to > 0 (clamped) instead of being left alone. > > Change reg_to_val and val_to_reg to return values > via parameters to make it more obvious when an > error case isn't handled. Also to allow > the regval type to be the correct sign in prep for > next commits. > Sorry, I don't see that as reason or argument for such invasive changes. As far as I can see, a two-liner to check the return value of val_to_reg() should have been sufficient. Most of the rest, such as splitting the return value into two elements, is POV and just adds additional code and complexity for zero gain. Guenter > Fixes: 6700ce035f83 ("hwmon: Driver for Linear Technologies LTC2945") > > Signed-off-by: Jonathan Cormier <jcormier@criticallink.com> > --- > drivers/hwmon/ltc2945.c | 30 ++++++++++++++++++------------ > 1 file changed, 18 insertions(+), 12 deletions(-) > > diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c > index 9af3e3821152..c66acf8d2124 100644 > --- a/drivers/hwmon/ltc2945.c > +++ b/drivers/hwmon/ltc2945.c > @@ -70,12 +70,12 @@ static inline bool is_power_reg(u8 reg) > } > > /* Return the value from the given register in uW, mV, or mA */ > -static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > +static int ltc2945_reg_to_val(struct device *dev, u8 reg, u64 *regval) > { > struct regmap *regmap = dev_get_drvdata(dev); > unsigned int control; > u8 buf[3]; > - long long val; > + u64 val; > int ret; > > ret = regmap_bulk_read(regmap, reg, buf, > @@ -148,11 +148,12 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > default: > return -EINVAL; > } > - return val; > + *regval = val; > + return 0; > } > > static int ltc2945_val_to_reg(struct device *dev, u8 reg, > - unsigned long val) > + unsigned long val, unsigned long *regval) > { > struct regmap *regmap = dev_get_drvdata(dev); > unsigned int control; > @@ -220,19 +221,21 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, > default: > return -EINVAL; > } > - return val; > + *regval = val; > + return 0; > } > > static ssize_t ltc2945_value_show(struct device *dev, > struct device_attribute *da, char *buf) > { > struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > - long long value; > + int ret; > + u64 value; > > - value = ltc2945_reg_to_val(dev, attr->index); > - if (value < 0) > - return value; > - return sysfs_emit(buf, "%lld\n", value); > + ret = ltc2945_reg_to_val(dev, attr->index, &value); > + if (ret < 0) > + return ret; > + return sysfs_emit(buf, "%llu\n", value); > } > > static ssize_t ltc2945_value_store(struct device *dev, > @@ -245,7 +248,7 @@ static ssize_t ltc2945_value_store(struct device *dev, > unsigned long val; > u8 regbuf[3]; > int num_regs; > - int regval; > + unsigned long regval; > int ret; > > ret = kstrtoul(buf, 10, &val); > @@ -253,7 +256,10 @@ static ssize_t ltc2945_value_store(struct device *dev, > return ret; > > /* convert to register value, then clamp and write result */ > - regval = ltc2945_val_to_reg(dev, reg, val); > + ret = ltc2945_val_to_reg(dev, reg, val, ®val); > + if (ret < 0) > + return ret; > + > if (is_power_reg(reg)) { > regval = clamp_val(regval, 0, 0xffffff); > regbuf[0] = regval >> 16; ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 3/5] hwmon: ltc2945: Handle error case in ltc2945_value_store 2023-01-10 0:04 ` Guenter Roeck @ 2023-01-10 18:19 ` Jon Cormier 2023-01-10 18:22 ` Guenter Roeck 0 siblings, 1 reply; 32+ messages in thread From: Jon Cormier @ 2023-01-10 18:19 UTC (permalink / raw) To: Guenter Roeck Cc: linux-hwmon, Jean Delvare, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt, Dan Vincelette On Mon, Jan 9, 2023 at 7:04 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On 1/9/23 15:35, Jonathan Cormier wrote: > > ltc2945_val_to_reg errors were not being handled > > which would have resulted in register being set to > > 0 (clamped) instead of being left alone. > > > > Change reg_to_val and val_to_reg to return values > > via parameters to make it more obvious when an > > error case isn't handled. Also to allow > > the regval type to be the correct sign in prep for > > next commits. > > > > Sorry, I don't see that as reason or argument for such invasive changes. > As far as I can see, a two-liner to check the return value of val_to_reg() > should have been sufficient. Most of the rest, such as splitting > the return value into two elements, is POV and just adds additional code > and complexity for zero gain. I can do that. However, you had also mentioned changing the return type to match what the calling function was expecting, an unsigned long. But I can't do that since error codes are negative so it would be a signed long which would lose precision and seemingly defeat the point of matching the variable type the caller wants. I could make it a signed long long but that still doesn't match. So it seemed saner to just return the error and the value separately, that way the function declaration was explicit about the types it wanted/returned, and less room for error. Would love to know your preferred solution. > > Guenter > > > Fixes: 6700ce035f83 ("hwmon: Driver for Linear Technologies LTC2945") > > > > Signed-off-by: Jonathan Cormier <jcormier@criticallink.com> > > --- > > drivers/hwmon/ltc2945.c | 30 ++++++++++++++++++------------ > > 1 file changed, 18 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c > > index 9af3e3821152..c66acf8d2124 100644 > > --- a/drivers/hwmon/ltc2945.c > > +++ b/drivers/hwmon/ltc2945.c > > @@ -70,12 +70,12 @@ static inline bool is_power_reg(u8 reg) > > } > > > > /* Return the value from the given register in uW, mV, or mA */ > > -static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > > +static int ltc2945_reg_to_val(struct device *dev, u8 reg, u64 *regval) > > { > > struct regmap *regmap = dev_get_drvdata(dev); > > unsigned int control; > > u8 buf[3]; > > - long long val; > > + u64 val; > > int ret; > > > > ret = regmap_bulk_read(regmap, reg, buf, > > @@ -148,11 +148,12 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > > default: > > return -EINVAL; > > } > > - return val; > > + *regval = val; > > + return 0; > > } > > > > static int ltc2945_val_to_reg(struct device *dev, u8 reg, > > - unsigned long val) > > + unsigned long val, unsigned long *regval) > > { > > struct regmap *regmap = dev_get_drvdata(dev); > > unsigned int control; > > @@ -220,19 +221,21 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, > > default: > > return -EINVAL; > > } > > - return val; > > + *regval = val; > > + return 0; > > } > > > > static ssize_t ltc2945_value_show(struct device *dev, > > struct device_attribute *da, char *buf) > > { > > struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > > - long long value; > > + int ret; > > + u64 value; > > > > - value = ltc2945_reg_to_val(dev, attr->index); > > - if (value < 0) > > - return value; > > - return sysfs_emit(buf, "%lld\n", value); > > + ret = ltc2945_reg_to_val(dev, attr->index, &value); > > + if (ret < 0) > > + return ret; > > + return sysfs_emit(buf, "%llu\n", value); > > } > > > > static ssize_t ltc2945_value_store(struct device *dev, > > @@ -245,7 +248,7 @@ static ssize_t ltc2945_value_store(struct device *dev, > > unsigned long val; > > u8 regbuf[3]; > > int num_regs; > > - int regval; > > + unsigned long regval; > > int ret; > > > > ret = kstrtoul(buf, 10, &val); > > @@ -253,7 +256,10 @@ static ssize_t ltc2945_value_store(struct device *dev, > > return ret; > > > > /* convert to register value, then clamp and write result */ > > - regval = ltc2945_val_to_reg(dev, reg, val); > > + ret = ltc2945_val_to_reg(dev, reg, val, ®val); > > + if (ret < 0) > > + return ret; > > + > > if (is_power_reg(reg)) { > > regval = clamp_val(regval, 0, 0xffffff); > > regbuf[0] = regval >> 16; > -- Jonathan Cormier Software Engineer Voice: 315.425.4045 x222 http://www.CriticalLink.com 6712 Brooklawn Parkway, Syracuse, NY 13211 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 3/5] hwmon: ltc2945: Handle error case in ltc2945_value_store 2023-01-10 18:19 ` Jon Cormier @ 2023-01-10 18:22 ` Guenter Roeck 2023-01-10 19:25 ` Jon Cormier 0 siblings, 1 reply; 32+ messages in thread From: Guenter Roeck @ 2023-01-10 18:22 UTC (permalink / raw) To: Jon Cormier Cc: linux-hwmon, Jean Delvare, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt, Dan Vincelette On 1/10/23 10:19, Jon Cormier wrote: > On Mon, Jan 9, 2023 at 7:04 PM Guenter Roeck <linux@roeck-us.net> wrote: >> >> On 1/9/23 15:35, Jonathan Cormier wrote: >>> ltc2945_val_to_reg errors were not being handled >>> which would have resulted in register being set to >>> 0 (clamped) instead of being left alone. >>> >>> Change reg_to_val and val_to_reg to return values >>> via parameters to make it more obvious when an >>> error case isn't handled. Also to allow >>> the regval type to be the correct sign in prep for >>> next commits. >>> >> >> Sorry, I don't see that as reason or argument for such invasive changes. >> As far as I can see, a two-liner to check the return value of val_to_reg() >> should have been sufficient. Most of the rest, such as splitting >> the return value into two elements, is POV and just adds additional code >> and complexity for zero gain. > I can do that. However, you had also mentioned changing the return > type to match what the calling function was expecting, an unsigned > long. But I can't do that since error codes are negative so it would > be a signed long which would lose precision and seemingly defeat the > point of matching the variable type the caller wants. I could make it > a signed long long but that still doesn't match. So it seemed saner > to just return the error and the value separately, that way the > function declaration was explicit about the types it wanted/returned, > and less room for error. Would love to know your preferred solution. > That is only true if the upper bit is actually ever set in that signed long. Which means I'll have to verify if "would lose precision" is actually a correct statement. Guenter >> >> Guenter >> >>> Fixes: 6700ce035f83 ("hwmon: Driver for Linear Technologies LTC2945") >>> >>> Signed-off-by: Jonathan Cormier <jcormier@criticallink.com> >>> --- >>> drivers/hwmon/ltc2945.c | 30 ++++++++++++++++++------------ >>> 1 file changed, 18 insertions(+), 12 deletions(-) >>> >>> diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c >>> index 9af3e3821152..c66acf8d2124 100644 >>> --- a/drivers/hwmon/ltc2945.c >>> +++ b/drivers/hwmon/ltc2945.c >>> @@ -70,12 +70,12 @@ static inline bool is_power_reg(u8 reg) >>> } >>> >>> /* Return the value from the given register in uW, mV, or mA */ >>> -static long long ltc2945_reg_to_val(struct device *dev, u8 reg) >>> +static int ltc2945_reg_to_val(struct device *dev, u8 reg, u64 *regval) >>> { >>> struct regmap *regmap = dev_get_drvdata(dev); >>> unsigned int control; >>> u8 buf[3]; >>> - long long val; >>> + u64 val; >>> int ret; >>> >>> ret = regmap_bulk_read(regmap, reg, buf, >>> @@ -148,11 +148,12 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) >>> default: >>> return -EINVAL; >>> } >>> - return val; >>> + *regval = val; >>> + return 0; >>> } >>> >>> static int ltc2945_val_to_reg(struct device *dev, u8 reg, >>> - unsigned long val) >>> + unsigned long val, unsigned long *regval) >>> { >>> struct regmap *regmap = dev_get_drvdata(dev); >>> unsigned int control; >>> @@ -220,19 +221,21 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, >>> default: >>> return -EINVAL; >>> } >>> - return val; >>> + *regval = val; >>> + return 0; >>> } >>> >>> static ssize_t ltc2945_value_show(struct device *dev, >>> struct device_attribute *da, char *buf) >>> { >>> struct sensor_device_attribute *attr = to_sensor_dev_attr(da); >>> - long long value; >>> + int ret; >>> + u64 value; >>> >>> - value = ltc2945_reg_to_val(dev, attr->index); >>> - if (value < 0) >>> - return value; >>> - return sysfs_emit(buf, "%lld\n", value); >>> + ret = ltc2945_reg_to_val(dev, attr->index, &value); >>> + if (ret < 0) >>> + return ret; >>> + return sysfs_emit(buf, "%llu\n", value); >>> } >>> >>> static ssize_t ltc2945_value_store(struct device *dev, >>> @@ -245,7 +248,7 @@ static ssize_t ltc2945_value_store(struct device *dev, >>> unsigned long val; >>> u8 regbuf[3]; >>> int num_regs; >>> - int regval; >>> + unsigned long regval; >>> int ret; >>> >>> ret = kstrtoul(buf, 10, &val); >>> @@ -253,7 +256,10 @@ static ssize_t ltc2945_value_store(struct device *dev, >>> return ret; >>> >>> /* convert to register value, then clamp and write result */ >>> - regval = ltc2945_val_to_reg(dev, reg, val); >>> + ret = ltc2945_val_to_reg(dev, reg, val, ®val); >>> + if (ret < 0) >>> + return ret; >>> + >>> if (is_power_reg(reg)) { >>> regval = clamp_val(regval, 0, 0xffffff); >>> regbuf[0] = regval >> 16; >> > > > -- > Jonathan Cormier > Software Engineer > > Voice: 315.425.4045 x222 > > > > http://www.CriticalLink.com > 6712 Brooklawn Parkway, Syracuse, NY 13211 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 3/5] hwmon: ltc2945: Handle error case in ltc2945_value_store 2023-01-10 18:22 ` Guenter Roeck @ 2023-01-10 19:25 ` Jon Cormier 2023-01-12 0:44 ` Guenter Roeck 0 siblings, 1 reply; 32+ messages in thread From: Jon Cormier @ 2023-01-10 19:25 UTC (permalink / raw) To: Guenter Roeck Cc: linux-hwmon, Jean Delvare, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt, Dan Vincelette On Tue, Jan 10, 2023 at 1:22 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On 1/10/23 10:19, Jon Cormier wrote: > > On Mon, Jan 9, 2023 at 7:04 PM Guenter Roeck <linux@roeck-us.net> wrote: > >> > >> On 1/9/23 15:35, Jonathan Cormier wrote: > >>> ltc2945_val_to_reg errors were not being handled > >>> which would have resulted in register being set to > >>> 0 (clamped) instead of being left alone. > >>> > >>> Change reg_to_val and val_to_reg to return values > >>> via parameters to make it more obvious when an > >>> error case isn't handled. Also to allow > >>> the regval type to be the correct sign in prep for > >>> next commits. > >>> > >> > >> Sorry, I don't see that as reason or argument for such invasive changes. > >> As far as I can see, a two-liner to check the return value of val_to_reg() > >> should have been sufficient. Most of the rest, such as splitting > >> the return value into two elements, is POV and just adds additional code > >> and complexity for zero gain. > > I can do that. However, you had also mentioned changing the return > > type to match what the calling function was expecting, an unsigned > > long. But I can't do that since error codes are negative so it would > > be a signed long which would lose precision and seemingly defeat the > > point of matching the variable type the caller wants. I could make it > > a signed long long but that still doesn't match. So it seemed saner > > to just return the error and the value separately, that way the > > function declaration was explicit about the types it wanted/returned, > > and less room for error. Would love to know your preferred solution. > > > > That is only true if the upper bit is actually ever set in that signed long. > Which means I'll have to verify if "would lose precision" is actually > a correct statement. I'd like to argue that is another reason to go with this change instead of working out the math of just how many bits are needed in the worst case and having to document it. And potentially getting that calculation wrong. But I can if you'd like me to. > > Guenter > > >> > >> Guenter > >> > >>> Fixes: 6700ce035f83 ("hwmon: Driver for Linear Technologies LTC2945") > >>> > >>> Signed-off-by: Jonathan Cormier <jcormier@criticallink.com> > >>> --- > >>> drivers/hwmon/ltc2945.c | 30 ++++++++++++++++++------------ > >>> 1 file changed, 18 insertions(+), 12 deletions(-) > >>> > >>> diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c > >>> index 9af3e3821152..c66acf8d2124 100644 > >>> --- a/drivers/hwmon/ltc2945.c > >>> +++ b/drivers/hwmon/ltc2945.c > >>> @@ -70,12 +70,12 @@ static inline bool is_power_reg(u8 reg) > >>> } > >>> > >>> /* Return the value from the given register in uW, mV, or mA */ > >>> -static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > >>> +static int ltc2945_reg_to_val(struct device *dev, u8 reg, u64 *regval) > >>> { > >>> struct regmap *regmap = dev_get_drvdata(dev); > >>> unsigned int control; > >>> u8 buf[3]; > >>> - long long val; > >>> + u64 val; > >>> int ret; > >>> > >>> ret = regmap_bulk_read(regmap, reg, buf, > >>> @@ -148,11 +148,12 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > >>> default: > >>> return -EINVAL; > >>> } > >>> - return val; > >>> + *regval = val; > >>> + return 0; > >>> } > >>> > >>> static int ltc2945_val_to_reg(struct device *dev, u8 reg, > >>> - unsigned long val) > >>> + unsigned long val, unsigned long *regval) > >>> { > >>> struct regmap *regmap = dev_get_drvdata(dev); > >>> unsigned int control; > >>> @@ -220,19 +221,21 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, > >>> default: > >>> return -EINVAL; > >>> } > >>> - return val; > >>> + *regval = val; > >>> + return 0; > >>> } > >>> > >>> static ssize_t ltc2945_value_show(struct device *dev, > >>> struct device_attribute *da, char *buf) > >>> { > >>> struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > >>> - long long value; > >>> + int ret; > >>> + u64 value; > >>> > >>> - value = ltc2945_reg_to_val(dev, attr->index); > >>> - if (value < 0) > >>> - return value; > >>> - return sysfs_emit(buf, "%lld\n", value); > >>> + ret = ltc2945_reg_to_val(dev, attr->index, &value); > >>> + if (ret < 0) > >>> + return ret; > >>> + return sysfs_emit(buf, "%llu\n", value); > >>> } > >>> > >>> static ssize_t ltc2945_value_store(struct device *dev, > >>> @@ -245,7 +248,7 @@ static ssize_t ltc2945_value_store(struct device *dev, > >>> unsigned long val; > >>> u8 regbuf[3]; > >>> int num_regs; > >>> - int regval; > >>> + unsigned long regval; > >>> int ret; > >>> > >>> ret = kstrtoul(buf, 10, &val); > >>> @@ -253,7 +256,10 @@ static ssize_t ltc2945_value_store(struct device *dev, > >>> return ret; > >>> > >>> /* convert to register value, then clamp and write result */ > >>> - regval = ltc2945_val_to_reg(dev, reg, val); > >>> + ret = ltc2945_val_to_reg(dev, reg, val, ®val); > >>> + if (ret < 0) > >>> + return ret; > >>> + > >>> if (is_power_reg(reg)) { > >>> regval = clamp_val(regval, 0, 0xffffff); > >>> regbuf[0] = regval >> 16; > >> > > > > > > -- > > Jonathan Cormier > > Software Engineer > > > > Voice: 315.425.4045 x222 > > > > > > > > http://www.CriticalLink.com > > 6712 Brooklawn Parkway, Syracuse, NY 13211 > -- Jonathan Cormier Software Engineer Voice: 315.425.4045 x222 http://www.CriticalLink.com 6712 Brooklawn Parkway, Syracuse, NY 13211 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 3/5] hwmon: ltc2945: Handle error case in ltc2945_value_store 2023-01-10 19:25 ` Jon Cormier @ 2023-01-12 0:44 ` Guenter Roeck 2023-01-18 18:32 ` Jon Cormier 0 siblings, 1 reply; 32+ messages in thread From: Guenter Roeck @ 2023-01-12 0:44 UTC (permalink / raw) To: Jon Cormier Cc: linux-hwmon, Jean Delvare, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt, Dan Vincelette On Tue, Jan 10, 2023 at 02:25:37PM -0500, Jon Cormier wrote: > On Tue, Jan 10, 2023 at 1:22 PM Guenter Roeck <linux@roeck-us.net> wrote: > > > > On 1/10/23 10:19, Jon Cormier wrote: > > > On Mon, Jan 9, 2023 at 7:04 PM Guenter Roeck <linux@roeck-us.net> wrote: > > >> > > >> On 1/9/23 15:35, Jonathan Cormier wrote: > > >>> ltc2945_val_to_reg errors were not being handled > > >>> which would have resulted in register being set to > > >>> 0 (clamped) instead of being left alone. > > >>> > > >>> Change reg_to_val and val_to_reg to return values > > >>> via parameters to make it more obvious when an > > >>> error case isn't handled. Also to allow > > >>> the regval type to be the correct sign in prep for > > >>> next commits. > > >>> > > >> > > >> Sorry, I don't see that as reason or argument for such invasive changes. > > >> As far as I can see, a two-liner to check the return value of val_to_reg() > > >> should have been sufficient. Most of the rest, such as splitting > > >> the return value into two elements, is POV and just adds additional code > > >> and complexity for zero gain. > > > I can do that. However, you had also mentioned changing the return > > > type to match what the calling function was expecting, an unsigned > > > long. But I can't do that since error codes are negative so it would > > > be a signed long which would lose precision and seemingly defeat the > > > point of matching the variable type the caller wants. I could make it > > > a signed long long but that still doesn't match. So it seemed saner > > > to just return the error and the value separately, that way the > > > function declaration was explicit about the types it wanted/returned, > > > and less room for error. Would love to know your preferred solution. > > > > > > > That is only true if the upper bit is actually ever set in that signed long. > > Which means I'll have to verify if "would lose precision" is actually > > a correct statement. > I'd like to argue that is another reason to go with this change > instead of working out the math of just how many bits are needed in > the worst case and having to document it. And potentially getting that > calculation wrong. But I can if you'd like me to. You are turning things on its head. We don't make changes like that because of maybe. It is you who has to show that the change is necessary, and that there is indeed a loss of precision otherwise. Guenter ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3 3/5] hwmon: ltc2945: Handle error case in ltc2945_value_store 2023-01-12 0:44 ` Guenter Roeck @ 2023-01-18 18:32 ` Jon Cormier 0 siblings, 0 replies; 32+ messages in thread From: Jon Cormier @ 2023-01-18 18:32 UTC (permalink / raw) To: Guenter Roeck Cc: linux-hwmon, Jean Delvare, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt, Dan Vincelette Alright, I'll take another pass at it. On Wed, Jan 11, 2023 at 7:44 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On Tue, Jan 10, 2023 at 02:25:37PM -0500, Jon Cormier wrote: > > On Tue, Jan 10, 2023 at 1:22 PM Guenter Roeck <linux@roeck-us.net> wrote: > > > > > > On 1/10/23 10:19, Jon Cormier wrote: > > > > On Mon, Jan 9, 2023 at 7:04 PM Guenter Roeck <linux@roeck-us.net> wrote: > > > >> > > > >> On 1/9/23 15:35, Jonathan Cormier wrote: > > > >>> ltc2945_val_to_reg errors were not being handled > > > >>> which would have resulted in register being set to > > > >>> 0 (clamped) instead of being left alone. > > > >>> > > > >>> Change reg_to_val and val_to_reg to return values > > > >>> via parameters to make it more obvious when an > > > >>> error case isn't handled. Also to allow > > > >>> the regval type to be the correct sign in prep for > > > >>> next commits. > > > >>> > > > >> > > > >> Sorry, I don't see that as reason or argument for such invasive changes. > > > >> As far as I can see, a two-liner to check the return value of val_to_reg() > > > >> should have been sufficient. Most of the rest, such as splitting > > > >> the return value into two elements, is POV and just adds additional code > > > >> and complexity for zero gain. > > > > I can do that. However, you had also mentioned changing the return > > > > type to match what the calling function was expecting, an unsigned > > > > long. But I can't do that since error codes are negative so it would > > > > be a signed long which would lose precision and seemingly defeat the > > > > point of matching the variable type the caller wants. I could make it > > > > a signed long long but that still doesn't match. So it seemed saner > > > > to just return the error and the value separately, that way the > > > > function declaration was explicit about the types it wanted/returned, > > > > and less room for error. Would love to know your preferred solution. > > > > > > > > > > That is only true if the upper bit is actually ever set in that signed long. > > > Which means I'll have to verify if "would lose precision" is actually > > > a correct statement. > > I'd like to argue that is another reason to go with this change > > instead of working out the math of just how many bits are needed in > > the worst case and having to document it. And potentially getting that > > calculation wrong. But I can if you'd like me to. > > You are turning things on its head. We don't make changes like that > because of maybe. It is you who has to show that the change is > necessary, and that there is indeed a loss of precision otherwise. > > Guenter -- Jonathan Cormier Software Engineer Voice: 315.425.4045 x222 http://www.CriticalLink.com 6712 Brooklawn Parkway, Syracuse, NY 13211 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 4/5] hwmon: ltc2945: Allow setting shunt resistor 2023-01-09 23:35 ` [PATCH v3 0/2] hwmon: ltc2945: Add binding and shunt resistor support Jonathan Cormier ` (2 preceding siblings ...) 2023-01-09 23:35 ` [PATCH v3 3/5] hwmon: ltc2945: Handle error case in ltc2945_value_store Jonathan Cormier @ 2023-01-09 23:35 ` Jonathan Cormier 2023-01-09 23:35 ` [PATCH v3 5/5] hwmon: ltc2945: Convert division to DIV_ROUND_CLOSEST_ULL Jonathan Cormier 4 siblings, 0 replies; 32+ messages in thread From: Jonathan Cormier @ 2023-01-09 23:35 UTC (permalink / raw) To: linux-hwmon Cc: John Pruitt, Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, Jonathan Cormier From: John Pruitt <jpruitt@criticallink.com> Add the ability to specify the value of the shunt resistor in the device tree instead of assuming it is 1 milliOhm. The value in the device tree has the name shunt-resistor-micro-ohms and the default value is 1000 micro-ohms in order to preserve the current behavior. Signed-off-by: Jonathan Cormier <jcormier@criticallink.com> Signed-off-by: John Pruitt <jpruitt@criticallink.com> --- drivers/hwmon/ltc2945.c | 85 +++++++++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 32 deletions(-) diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c index c66acf8d2124..0d743d75459c 100644 --- a/drivers/hwmon/ltc2945.c +++ b/drivers/hwmon/ltc2945.c @@ -64,6 +64,16 @@ static const struct of_device_id __maybe_unused ltc2945_of_match[] = { }; MODULE_DEVICE_TABLE(of, ltc2945_of_match); +/** + * struct ltc2945_data - LTC2945 device data + * @regmap: regmap device + * @shunt_resistor: shunt resistor value in micro ohms (1000 by default) + */ +struct ltc2945_data { + struct regmap *regmap; + u32 shunt_resistor; +}; + static inline bool is_power_reg(u8 reg) { return reg < LTC2945_SENSE_H; @@ -72,7 +82,9 @@ static inline bool is_power_reg(u8 reg) /* Return the value from the given register in uW, mV, or mA */ static int ltc2945_reg_to_val(struct device *dev, u8 reg, u64 *regval) { - struct regmap *regmap = dev_get_drvdata(dev); + struct ltc2945_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; + u32 shunt_resistor = data->shunt_resistor; unsigned int control; u8 buf[3]; u64 val; @@ -98,9 +110,7 @@ static int ltc2945_reg_to_val(struct device *dev, u8 reg, u64 *regval) case LTC2945_MAX_POWER_THRES_H: case LTC2945_MIN_POWER_THRES_H: /* - * Convert to uW by assuming current is measured with - * an 1mOhm sense resistor, similar to current - * measurements. + * Convert to uW * Control register bit 0 selects if voltage at SENSE+/VDD * or voltage at ADIN is used to measure power. */ @@ -114,6 +124,9 @@ static int ltc2945_reg_to_val(struct device *dev, u8 reg, u64 *regval) /* 0.5 mV * 25 uV = 0.0125 uV resolution. */ val = (val * 25LL) >> 1; } + val *= 1000; + val = DIV_ROUND_CLOSEST_ULL(val, shunt_resistor); + break; case LTC2945_VIN_H: case LTC2945_MAX_VIN_H: @@ -136,14 +149,9 @@ static int ltc2945_reg_to_val(struct device *dev, u8 reg, u64 *regval) case LTC2945_MIN_SENSE_H: case LTC2945_MAX_SENSE_THRES_H: case LTC2945_MIN_SENSE_THRES_H: - /* - * 25 uV resolution. Convert to current as measured with - * an 1 mOhm sense resistor, in mA. If a different sense - * resistor is installed, calculate the actual current by - * dividing the reported current by the sense resistor value - * in mOhm. - */ - val *= 25; + /* 25 uV resolution. Convert to mA. */ + val *= 25 * 1000; + val = DIV_ROUND_CLOSEST_ULL(val, shunt_resistor); break; default: return -EINVAL; @@ -153,9 +161,11 @@ static int ltc2945_reg_to_val(struct device *dev, u8 reg, u64 *regval) } static int ltc2945_val_to_reg(struct device *dev, u8 reg, - unsigned long val, unsigned long *regval) + u64 val, u64 *regval) { - struct regmap *regmap = dev_get_drvdata(dev); + struct ltc2945_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; + u32 shunt_resistor = data->shunt_resistor; unsigned int control; int ret; @@ -166,9 +176,6 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, case LTC2945_MAX_POWER_THRES_H: case LTC2945_MIN_POWER_THRES_H: /* - * Convert to register value by assuming current is measured - * with an 1mOhm sense resistor, similar to current - * measurements. * Control register bit 0 selects if voltage at SENSE+/VDD * or voltage at ADIN is used to measure power, which in turn * determines register calculations. @@ -178,14 +185,16 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, return ret; if (control & CONTROL_MULT_SELECT) { /* 25 mV * 25 uV = 0.625 uV resolution. */ - val = DIV_ROUND_CLOSEST(val, 625); + val *= shunt_resistor; + val = DIV_ROUND_CLOSEST_ULL(val, 625 * 1000); } else { /* * 0.5 mV * 25 uV = 0.0125 uV resolution. * Divide first to avoid overflow; * accept loss of accuracy. */ - val = DIV_ROUND_CLOSEST(val, 25) * 2; + val *= shunt_resistor; + val = DIV_ROUND_CLOSEST_ULL(val, 25 * 1000) * 2; } break; case LTC2945_VIN_H: @@ -209,14 +218,9 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, case LTC2945_MIN_SENSE_H: case LTC2945_MAX_SENSE_THRES_H: case LTC2945_MIN_SENSE_THRES_H: - /* - * 25 uV resolution. Convert to current as measured with - * an 1 mOhm sense resistor, in mA. If a different sense - * resistor is installed, calculate the actual current by - * dividing the reported current by the sense resistor value - * in mOhm. - */ - val = DIV_ROUND_CLOSEST(val, 25); + /* 25 uV resolution. Convert to mA. */ + val *= shunt_resistor; + val = DIV_ROUND_CLOSEST_ULL(val, 25 * 1000); break; default: return -EINVAL; @@ -243,12 +247,13 @@ static ssize_t ltc2945_value_store(struct device *dev, const char *buf, size_t count) { struct sensor_device_attribute *attr = to_sensor_dev_attr(da); - struct regmap *regmap = dev_get_drvdata(dev); + struct ltc2945_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; u8 reg = attr->index; unsigned long val; u8 regbuf[3]; int num_regs; - unsigned long regval; + u64 regval; int ret; ret = kstrtoul(buf, 10, &val); @@ -281,7 +286,8 @@ static ssize_t ltc2945_history_store(struct device *dev, const char *buf, size_t count) { struct sensor_device_attribute *attr = to_sensor_dev_attr(da); - struct regmap *regmap = dev_get_drvdata(dev); + struct ltc2945_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; u8 reg = attr->index; int num_regs = is_power_reg(reg) ? 3 : 2; u8 buf_min[3] = { 0xff, 0xff, 0xff }; @@ -333,7 +339,8 @@ static ssize_t ltc2945_bool_show(struct device *dev, struct device_attribute *da, char *buf) { struct sensor_device_attribute *attr = to_sensor_dev_attr(da); - struct regmap *regmap = dev_get_drvdata(dev); + struct ltc2945_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; unsigned int fault; int ret; @@ -462,6 +469,12 @@ static int ltc2945_probe(struct i2c_client *client) struct device *dev = &client->dev; struct device *hwmon_dev; struct regmap *regmap; + struct ltc2945_data *data; + + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + dev_set_drvdata(dev, data); regmap = devm_regmap_init_i2c(client, <c2945_regmap_config); if (IS_ERR(regmap)) { @@ -469,11 +482,19 @@ static int ltc2945_probe(struct i2c_client *client) return PTR_ERR(regmap); } + data->regmap = regmap; + if (device_property_read_u32(dev, "shunt-resistor-micro-ohms", + &data->shunt_resistor)) + data->shunt_resistor = 1000; + + if (data->shunt_resistor == 0) + return -EINVAL; + /* Clear faults */ regmap_write(regmap, LTC2945_FAULT, 0x00); hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name, - regmap, + data, ltc2945_groups); return PTR_ERR_OR_ZERO(hwmon_dev); } -- 2.25.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 5/5] hwmon: ltc2945: Convert division to DIV_ROUND_CLOSEST_ULL 2023-01-09 23:35 ` [PATCH v3 0/2] hwmon: ltc2945: Add binding and shunt resistor support Jonathan Cormier ` (3 preceding siblings ...) 2023-01-09 23:35 ` [PATCH v3 4/5] hwmon: ltc2945: Allow setting shunt resistor Jonathan Cormier @ 2023-01-09 23:35 ` Jonathan Cormier 4 siblings, 0 replies; 32+ messages in thread From: Jonathan Cormier @ 2023-01-09 23:35 UTC (permalink / raw) To: linux-hwmon Cc: Jonathan Cormier, Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt Convert division to DIV_ROUND_CLOSEST_ULL to match code in same function Signed-off-by: Jonathan Cormier <jcormier@criticallink.com> --- drivers/hwmon/ltc2945.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c index 0d743d75459c..5456c1b60b8b 100644 --- a/drivers/hwmon/ltc2945.c +++ b/drivers/hwmon/ltc2945.c @@ -203,7 +203,7 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, case LTC2945_MAX_VIN_THRES_H: case LTC2945_MIN_VIN_THRES_H: /* 25 mV resolution. */ - val /= 25; + val = DIV_ROUND_CLOSEST_ULL(val, 25); break; case LTC2945_ADIN_H: case LTC2945_MAX_ADIN_H: -- 2.25.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 0/2] hwmon: ltc2945: Add binding and shunt resistor support 2022-12-14 22:07 [PATCH 0/2] hwmon: ltc2945: Add binding and shunt resistor support Cormier, Jonathan 2022-12-14 22:07 ` [PATCH 1/2] dt-bindings: hwmon: adi,ltc2945: Add binding Cormier, Jonathan 2022-12-14 22:07 ` [PATCH 2/2] hwmon: ltc2945: Allow setting shunt resistor Cormier, Jonathan @ 2022-12-20 0:04 ` Cormier, Jonathan 2022-12-20 0:04 ` [PATCH v2 1/4] dt-bindings: hwmon: adi,ltc2945: Add binding Cormier, Jonathan ` (3 more replies) 2 siblings, 4 replies; 32+ messages in thread From: Cormier, Jonathan @ 2022-12-20 0:04 UTC (permalink / raw) To: linux-hwmon Cc: Cormier, Jonathan, Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt Added the ability to specify the value of the shunt resistor in the device tree instead of assuming it is 1 milliOhm. Would be good to backport as well Changes since v1: - Add devicetree match table - Add kerneldoc for the ltc2945_data struct - Cleanup excesive comments about the shunt resistor - Switch to device_property_read_u32() Cormier, Jonathan (1): dt-bindings: hwmon: adi,ltc2945: Add binding John Pruitt (1): hwmon: ltc2945: Allow setting shunt resistor .../bindings/hwmon/adi,ltc2945.yaml | 50 +++++++++++ drivers/hwmon/ltc2945.c | 86 ++++++++++++++----- 2 files changed, 115 insertions(+), 21 deletions(-) create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml -- 2.25.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/4] dt-bindings: hwmon: adi,ltc2945: Add binding 2022-12-20 0:04 ` [PATCH v2 0/2] hwmon: ltc2945: Add binding and shunt resistor support Cormier, Jonathan @ 2022-12-20 0:04 ` Cormier, Jonathan 2022-12-20 10:15 ` Krzysztof Kozlowski 2022-12-20 0:04 ` [PATCH v2 2/4] hwmon: ltc2945: Add devicetree match table Cormier, Jonathan ` (2 subsequent siblings) 3 siblings, 1 reply; 32+ messages in thread From: Cormier, Jonathan @ 2022-12-20 0:04 UTC (permalink / raw) To: linux-hwmon Cc: Cormier, Jonathan, Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt Create initial binding for the LTC2945 I2C power monitor. Also adds shunt-resistor-micro-ohms parameter Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> --- .../bindings/hwmon/adi,ltc2945.yaml | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml new file mode 100644 index 000000000000..f90d40919ee6 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml @@ -0,0 +1,50 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/adi,ltc2945.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices LTC2945 wide range i2c power monitor + +maintainers: + - Guenter Roeck <linux@roeck-us.net> + +description: | + Analog Devices LTC2945 wide range i2c power monitor over I2C. + + https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2945.pdf + +properties: + compatible: + enum: + - adi,ltc2945 + + reg: + maxItems: 1 + + shunt-resistor-micro-ohms: + description: + Shunt resistor value in micro-Ohms + default: 1000 + +required: + - compatible + - reg + + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + power-monitor@6e { + compatible = "adi,ltc2945"; + reg = <0x6e>; + /* 10 milli-Ohm shunt resistor */ + shunt-resistor-micro-ohms = <10000>; + }; + }; +... -- 2.25.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: hwmon: adi,ltc2945: Add binding 2022-12-20 0:04 ` [PATCH v2 1/4] dt-bindings: hwmon: adi,ltc2945: Add binding Cormier, Jonathan @ 2022-12-20 10:15 ` Krzysztof Kozlowski 2022-12-20 14:35 ` Jon Cormier 0 siblings, 1 reply; 32+ messages in thread From: Krzysztof Kozlowski @ 2022-12-20 10:15 UTC (permalink / raw) To: Cormier, Jonathan, linux-hwmon Cc: Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt On 20/12/2022 01:04, Cormier, Jonathan wrote: > Create initial binding for the LTC2945 I2C power monitor. > Also adds shunt-resistor-micro-ohms parameter The last sentence does not make sense. I propose to skip it. > > Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> > --- > .../bindings/hwmon/adi,ltc2945.yaml | 50 +++++++++++++++++++ > 1 file changed, 50 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml > new file mode 100644 > index 000000000000..f90d40919ee6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml > @@ -0,0 +1,50 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hwmon/adi,ltc2945.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices LTC2945 wide range i2c power monitor > + > +maintainers: > + - Guenter Roeck <linux@roeck-us.net> Maintainer of binding is person interested in the device, e.g. having the hardware or datasheet. Not the subsystem maintainer. Unless by coincidence this is the same person here? > + > +description: | > + Analog Devices LTC2945 wide range i2c power monitor over I2C. > + > + https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2945.pdf > + > +properties: > + compatible: > + enum: > + - adi,ltc2945 > + > + reg: > + maxItems: 1 > + > + shunt-resistor-micro-ohms: > + description: > + Shunt resistor value in micro-Ohms > + default: 1000 > + > +required: > + - compatible > + - reg > + > + This is a friendly reminder during the review process. It seems my previous comments were not fully addressed. Maybe my feedback got lost between the quotes, maybe you just forgot to apply it. Please go back to the previous discussion and either implement all requested changes or keep discussing them. Thank you. > +additionalProperties: false > + Best regards, Krzysztof ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: hwmon: adi,ltc2945: Add binding 2022-12-20 10:15 ` Krzysztof Kozlowski @ 2022-12-20 14:35 ` Jon Cormier 2022-12-20 14:46 ` Guenter Roeck 0 siblings, 1 reply; 32+ messages in thread From: Jon Cormier @ 2022-12-20 14:35 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: linux-hwmon, Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt On Tue, Dec 20, 2022 at 5:15 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 20/12/2022 01:04, Cormier, Jonathan wrote: > > Create initial binding for the LTC2945 I2C power monitor. > > Also adds shunt-resistor-micro-ohms parameter > > The last sentence does not make sense. I propose to skip it. Ok > > > > > Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> > > --- > > .../bindings/hwmon/adi,ltc2945.yaml | 50 +++++++++++++++++++ > > 1 file changed, 50 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml > > > > diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml > > new file mode 100644 > > index 000000000000..f90d40919ee6 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml > > @@ -0,0 +1,50 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/hwmon/adi,ltc2945.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Analog Devices LTC2945 wide range i2c power monitor > > + > > +maintainers: > > + - Guenter Roeck <linux@roeck-us.net> > > Maintainer of binding is person interested in the device, e.g. having > the hardware or datasheet. Not the subsystem maintainer. Unless by > coincidence this is the same person here? What do you do with a basic kernel driver which hasn't been touched since it was introduced except for various refactors? He seems to be the one who introduced it and most consistently made changes to it. > > > > + > > +description: | > > + Analog Devices LTC2945 wide range i2c power monitor over I2C. > > + > > + https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2945.pdf > > + > > +properties: > > + compatible: > > + enum: > > + - adi,ltc2945 > > + > > + reg: > > + maxItems: 1 > > + > > + shunt-resistor-micro-ohms: > > + description: > > + Shunt resistor value in micro-Ohms > > + default: 1000 > > + > > +required: > > + - compatible > > + - reg > > + > > + > This is a friendly reminder during the review process. > > It seems my previous comments were not fully addressed. Maybe my > feedback got lost between the quotes, maybe you just forgot to apply it. > Please go back to the previous discussion and either implement all > requested changes or keep discussing them. My bad, > > Thank you. > > > +additionalProperties: false > > + > > Best regards, > Krzysztof > -- Jonathan Cormier Software Engineer Voice: 315.425.4045 x222 http://www.CriticalLink.com 6712 Brooklawn Parkway, Syracuse, NY 13211 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: hwmon: adi,ltc2945: Add binding 2022-12-20 14:35 ` Jon Cormier @ 2022-12-20 14:46 ` Guenter Roeck 2022-12-20 21:47 ` Jon Cormier 0 siblings, 1 reply; 32+ messages in thread From: Guenter Roeck @ 2022-12-20 14:46 UTC (permalink / raw) To: Jon Cormier Cc: Krzysztof Kozlowski, linux-hwmon, Jean Delvare, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt On Tue, Dec 20, 2022 at 09:35:38AM -0500, Jon Cormier wrote: > On Tue, Dec 20, 2022 at 5:15 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: > > > > On 20/12/2022 01:04, Cormier, Jonathan wrote: > > > Create initial binding for the LTC2945 I2C power monitor. > > > Also adds shunt-resistor-micro-ohms parameter > > > > The last sentence does not make sense. I propose to skip it. > Ok > > > > > > > > Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> > > > --- > > > .../bindings/hwmon/adi,ltc2945.yaml | 50 +++++++++++++++++++ > > > 1 file changed, 50 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml > > > new file mode 100644 > > > index 000000000000..f90d40919ee6 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml > > > @@ -0,0 +1,50 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/hwmon/adi,ltc2945.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Analog Devices LTC2945 wide range i2c power monitor > > > + > > > +maintainers: > > > + - Guenter Roeck <linux@roeck-us.net> > > > > Maintainer of binding is person interested in the device, e.g. having > > the hardware or datasheet. Not the subsystem maintainer. Unless by > > coincidence this is the same person here? > What do you do with a basic kernel driver which hasn't been touched > since it was introduced except for various refactors? He seems to be > the one who introduced it and most consistently made changes to it. FWIW, if I was not ok with being listed as maintainer I would have objected. Having said that, it is just as fine with me to list someone else. Guenter > > > > > > > + > > > +description: | > > > + Analog Devices LTC2945 wide range i2c power monitor over I2C. > > > + > > > + https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2945.pdf > > > + > > > +properties: > > > + compatible: > > > + enum: > > > + - adi,ltc2945 > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + shunt-resistor-micro-ohms: > > > + description: > > > + Shunt resistor value in micro-Ohms > > > + default: 1000 > > > + > > > +required: > > > + - compatible > > > + - reg > > > + > > > + > > This is a friendly reminder during the review process. > > > > It seems my previous comments were not fully addressed. Maybe my > > feedback got lost between the quotes, maybe you just forgot to apply it. > > Please go back to the previous discussion and either implement all > > requested changes or keep discussing them. > My bad, > > > > Thank you. > > > > > +additionalProperties: false > > > + > > > > Best regards, > > Krzysztof > > > > > -- > Jonathan Cormier > Software Engineer > > Voice: 315.425.4045 x222 > > > > http://www.CriticalLink.com > 6712 Brooklawn Parkway, Syracuse, NY 13211 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: hwmon: adi,ltc2945: Add binding 2022-12-20 14:46 ` Guenter Roeck @ 2022-12-20 21:47 ` Jon Cormier 0 siblings, 0 replies; 32+ messages in thread From: Jon Cormier @ 2022-12-20 21:47 UTC (permalink / raw) To: Guenter Roeck Cc: Krzysztof Kozlowski, linux-hwmon, Jean Delvare, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt Okay so for V3, I see only 2 changes. Please let me know if I've overlooked something else. - Remove "Also adds shunt-resistor-micro-ohms parameter" from commit description - Remove extra newline from binding doc Resending in plaintext... On Tue, Dec 20, 2022 at 9:46 AM Guenter Roeck <linux@roeck-us.net> wrote: > > On Tue, Dec 20, 2022 at 09:35:38AM -0500, Jon Cormier wrote: > > On Tue, Dec 20, 2022 at 5:15 AM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > > > > > > On 20/12/2022 01:04, Cormier, Jonathan wrote: > > > > Create initial binding for the LTC2945 I2C power monitor. > > > > Also adds shunt-resistor-micro-ohms parameter > > > > > > The last sentence does not make sense. I propose to skip it. > > Ok > > > > > > > > > > > Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> > > > > --- > > > > .../bindings/hwmon/adi,ltc2945.yaml | 50 +++++++++++++++++++ > > > > 1 file changed, 50 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml > > > > > > > > diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml > > > > new file mode 100644 > > > > index 000000000000..f90d40919ee6 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2945.yaml > > > > @@ -0,0 +1,50 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/hwmon/adi,ltc2945.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Analog Devices LTC2945 wide range i2c power monitor > > > > + > > > > +maintainers: > > > > + - Guenter Roeck <linux@roeck-us.net> > > > > > > Maintainer of binding is person interested in the device, e.g. having > > > the hardware or datasheet. Not the subsystem maintainer. Unless by > > > coincidence this is the same person here? > > What do you do with a basic kernel driver which hasn't been touched > > since it was introduced except for various refactors? He seems to be > > the one who introduced it and most consistently made changes to it. > > FWIW, if I was not ok with being listed as maintainer I would have objected. > > Having said that, it is just as fine with me to list someone else. > > Guenter > > > > > > > > > > > + > > > > +description: | > > > > + Analog Devices LTC2945 wide range i2c power monitor over I2C. > > > > + > > > > + https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2945.pdf > > > > + > > > > +properties: > > > > + compatible: > > > > + enum: > > > > + - adi,ltc2945 > > > > + > > > > + reg: > > > > + maxItems: 1 > > > > + > > > > + shunt-resistor-micro-ohms: > > > > + description: > > > > + Shunt resistor value in micro-Ohms > > > > + default: 1000 > > > > + > > > > +required: > > > > + - compatible > > > > + - reg > > > > + > > > > + > > > This is a friendly reminder during the review process. > > > > > > It seems my previous comments were not fully addressed. Maybe my > > > feedback got lost between the quotes, maybe you just forgot to apply it. > > > Please go back to the previous discussion and either implement all > > > requested changes or keep discussing them. > > My bad, > > > > > > Thank you. > > > > > > > +additionalProperties: false > > > > + > > > > > > Best regards, > > > Krzysztof > > > > > > > > > -- > > Jonathan Cormier > > Software Engineer > > > > Voice: 315.425.4045 x222 > > > > > > > > http://www.CriticalLink.com > > 6712 Brooklawn Parkway, Syracuse, NY 13211 -- Jonathan Cormier Software Engineer Voice: 315.425.4045 x222 http://www.CriticalLink.com 6712 Brooklawn Parkway, Syracuse, NY 13211 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 2/4] hwmon: ltc2945: Add devicetree match table 2022-12-20 0:04 ` [PATCH v2 0/2] hwmon: ltc2945: Add binding and shunt resistor support Cormier, Jonathan 2022-12-20 0:04 ` [PATCH v2 1/4] dt-bindings: hwmon: adi,ltc2945: Add binding Cormier, Jonathan @ 2022-12-20 0:04 ` Cormier, Jonathan 2022-12-28 16:43 ` Guenter Roeck 2022-12-20 0:04 ` [PATCH v2 3/4] hwmon: ltc2945: Allow setting shunt resistor Cormier, Jonathan 2022-12-20 0:04 ` [PATCH v2 4/4] hwmon: ltc2945: Fix possible overflows Cormier, Jonathan 3 siblings, 1 reply; 32+ messages in thread From: Cormier, Jonathan @ 2022-12-20 0:04 UTC (permalink / raw) To: linux-hwmon Cc: Cormier, Jonathan, Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> --- drivers/hwmon/ltc2945.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c index 9adebb59f604..9af3e3821152 100644 --- a/drivers/hwmon/ltc2945.c +++ b/drivers/hwmon/ltc2945.c @@ -58,6 +58,12 @@ #define CONTROL_MULT_SELECT (1 << 0) #define CONTROL_TEST_MODE (1 << 4) +static const struct of_device_id __maybe_unused ltc2945_of_match[] = { + { .compatible = "adi,ltc2945" }, + { } +}; +MODULE_DEVICE_TABLE(of, ltc2945_of_match); + static inline bool is_power_reg(u8 reg) { return reg < LTC2945_SENSE_H; @@ -475,8 +481,9 @@ MODULE_DEVICE_TABLE(i2c, ltc2945_id); static struct i2c_driver ltc2945_driver = { .driver = { - .name = "ltc2945", - }, + .name = "ltc2945", + .of_match_table = of_match_ptr(ltc2945_of_match), + }, .probe_new = ltc2945_probe, .id_table = ltc2945_id, }; -- 2.25.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/4] hwmon: ltc2945: Add devicetree match table 2022-12-20 0:04 ` [PATCH v2 2/4] hwmon: ltc2945: Add devicetree match table Cormier, Jonathan @ 2022-12-28 16:43 ` Guenter Roeck 0 siblings, 0 replies; 32+ messages in thread From: Guenter Roeck @ 2022-12-28 16:43 UTC (permalink / raw) To: Cormier, Jonathan Cc: linux-hwmon, Jean Delvare, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, John Pruitt On Mon, Dec 19, 2022 at 07:04:55PM -0500, Cormier, Jonathan wrote: > Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> There should still be some description here. > --- > drivers/hwmon/ltc2945.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c > index 9adebb59f604..9af3e3821152 100644 > --- a/drivers/hwmon/ltc2945.c > +++ b/drivers/hwmon/ltc2945.c > @@ -58,6 +58,12 @@ > #define CONTROL_MULT_SELECT (1 << 0) > #define CONTROL_TEST_MODE (1 << 4) > > +static const struct of_device_id __maybe_unused ltc2945_of_match[] = { > + { .compatible = "adi,ltc2945" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ltc2945_of_match); > + > static inline bool is_power_reg(u8 reg) > { > return reg < LTC2945_SENSE_H; > @@ -475,8 +481,9 @@ MODULE_DEVICE_TABLE(i2c, ltc2945_id); > > static struct i2c_driver ltc2945_driver = { > .driver = { > - .name = "ltc2945", > - }, > + .name = "ltc2945", > + .of_match_table = of_match_ptr(ltc2945_of_match), > + }, > .probe_new = ltc2945_probe, > .id_table = ltc2945_id, > }; > -- > 2.25.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 3/4] hwmon: ltc2945: Allow setting shunt resistor 2022-12-20 0:04 ` [PATCH v2 0/2] hwmon: ltc2945: Add binding and shunt resistor support Cormier, Jonathan 2022-12-20 0:04 ` [PATCH v2 1/4] dt-bindings: hwmon: adi,ltc2945: Add binding Cormier, Jonathan 2022-12-20 0:04 ` [PATCH v2 2/4] hwmon: ltc2945: Add devicetree match table Cormier, Jonathan @ 2022-12-20 0:04 ` Cormier, Jonathan 2022-12-28 16:49 ` Guenter Roeck 2022-12-20 0:04 ` [PATCH v2 4/4] hwmon: ltc2945: Fix possible overflows Cormier, Jonathan 3 siblings, 1 reply; 32+ messages in thread From: Cormier, Jonathan @ 2022-12-20 0:04 UTC (permalink / raw) To: linux-hwmon Cc: John Pruitt, Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, Cormier, Jonathan From: John Pruitt <jpruitt@criticallink.com> Added the ability to specify the value of the shunt resistor in the device tree instead of assuming it is 1 milliOhm. The value in the device tree has the name shunt-resistor-micro-ohms and the default value is 1000 micro-ohms in order to preserve the current behavior. Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> Signed-off-by: John Pruitt <jpruitt@criticallink.com> --- drivers/hwmon/ltc2945.c | 80 +++++++++++++++++++++++++---------------- 1 file changed, 50 insertions(+), 30 deletions(-) diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c index 9af3e3821152..fc7d399b2c85 100644 --- a/drivers/hwmon/ltc2945.c +++ b/drivers/hwmon/ltc2945.c @@ -64,6 +64,16 @@ static const struct of_device_id __maybe_unused ltc2945_of_match[] = { }; MODULE_DEVICE_TABLE(of, ltc2945_of_match); +/** + * struct ltc2945_data - LTC2945 device data + * @regmap: regmap device + * @shunt_resistor: shunt resistor value in micro ohms (1000 by default) + */ +struct ltc2945_data { + struct regmap *regmap; + u32 shunt_resistor; +}; + static inline bool is_power_reg(u8 reg) { return reg < LTC2945_SENSE_H; @@ -72,7 +82,9 @@ static inline bool is_power_reg(u8 reg) /* Return the value from the given register in uW, mV, or mA */ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) { - struct regmap *regmap = dev_get_drvdata(dev); + struct ltc2945_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; + u32 shunt_resistor = data->shunt_resistor; unsigned int control; u8 buf[3]; long long val; @@ -98,9 +110,7 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) case LTC2945_MAX_POWER_THRES_H: case LTC2945_MIN_POWER_THRES_H: /* - * Convert to uW by assuming current is measured with - * an 1mOhm sense resistor, similar to current - * measurements. + * Convert to uW * Control register bit 0 selects if voltage at SENSE+/VDD * or voltage at ADIN is used to measure power. */ @@ -114,6 +124,8 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) /* 0.5 mV * 25 uV = 0.0125 uV resolution. */ val = (val * 25LL) >> 1; } + val *= 1000; + val = DIV_ROUND_CLOSEST_ULL(val, shunt_resistor); break; case LTC2945_VIN_H: case LTC2945_MAX_VIN_H: @@ -136,14 +148,9 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) case LTC2945_MIN_SENSE_H: case LTC2945_MAX_SENSE_THRES_H: case LTC2945_MIN_SENSE_THRES_H: - /* - * 25 uV resolution. Convert to current as measured with - * an 1 mOhm sense resistor, in mA. If a different sense - * resistor is installed, calculate the actual current by - * dividing the reported current by the sense resistor value - * in mOhm. - */ - val *= 25; + /* 25 uV resolution. Convert to mA. */ + val *= 25 * 1000; + val = DIV_ROUND_CLOSEST_ULL(val, shunt_resistor); break; default: return -EINVAL; @@ -154,7 +161,9 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) static int ltc2945_val_to_reg(struct device *dev, u8 reg, unsigned long val) { - struct regmap *regmap = dev_get_drvdata(dev); + struct ltc2945_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; + u32 shunt_resistor = data->shunt_resistor; unsigned int control; int ret; @@ -165,9 +174,6 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, case LTC2945_MAX_POWER_THRES_H: case LTC2945_MIN_POWER_THRES_H: /* - * Convert to register value by assuming current is measured - * with an 1mOhm sense resistor, similar to current - * measurements. * Control register bit 0 selects if voltage at SENSE+/VDD * or voltage at ADIN is used to measure power, which in turn * determines register calculations. @@ -177,14 +183,16 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, return ret; if (control & CONTROL_MULT_SELECT) { /* 25 mV * 25 uV = 0.625 uV resolution. */ - val = DIV_ROUND_CLOSEST(val, 625); + val *= shunt_resistor; + val = DIV_ROUND_CLOSEST(val, 625 * 1000); } else { /* * 0.5 mV * 25 uV = 0.0125 uV resolution. * Divide first to avoid overflow; * accept loss of accuracy. */ - val = DIV_ROUND_CLOSEST(val, 25) * 2; + val *= shunt_resistor; + val = DIV_ROUND_CLOSEST(val, 25 * 1000) * 2; } break; case LTC2945_VIN_H: @@ -208,14 +216,9 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, case LTC2945_MIN_SENSE_H: case LTC2945_MAX_SENSE_THRES_H: case LTC2945_MIN_SENSE_THRES_H: - /* - * 25 uV resolution. Convert to current as measured with - * an 1 mOhm sense resistor, in mA. If a different sense - * resistor is installed, calculate the actual current by - * dividing the reported current by the sense resistor value - * in mOhm. - */ - val = DIV_ROUND_CLOSEST(val, 25); + /* 25 uV resolution. Convert to mA. */ + val *= shunt_resistor; + val = DIV_ROUND_CLOSEST(val, 25 * 1000); break; default: return -EINVAL; @@ -240,7 +243,8 @@ static ssize_t ltc2945_value_store(struct device *dev, const char *buf, size_t count) { struct sensor_device_attribute *attr = to_sensor_dev_attr(da); - struct regmap *regmap = dev_get_drvdata(dev); + struct ltc2945_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; u8 reg = attr->index; unsigned long val; u8 regbuf[3]; @@ -275,7 +279,8 @@ static ssize_t ltc2945_history_store(struct device *dev, const char *buf, size_t count) { struct sensor_device_attribute *attr = to_sensor_dev_attr(da); - struct regmap *regmap = dev_get_drvdata(dev); + struct ltc2945_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; u8 reg = attr->index; int num_regs = is_power_reg(reg) ? 3 : 2; u8 buf_min[3] = { 0xff, 0xff, 0xff }; @@ -327,7 +332,8 @@ static ssize_t ltc2945_bool_show(struct device *dev, struct device_attribute *da, char *buf) { struct sensor_device_attribute *attr = to_sensor_dev_attr(da); - struct regmap *regmap = dev_get_drvdata(dev); + struct ltc2945_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; unsigned int fault; int ret; @@ -456,6 +462,12 @@ static int ltc2945_probe(struct i2c_client *client) struct device *dev = &client->dev; struct device *hwmon_dev; struct regmap *regmap; + struct ltc2945_data *data; + + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + dev_set_drvdata(dev, data); regmap = devm_regmap_init_i2c(client, <c2945_regmap_config); if (IS_ERR(regmap)) { @@ -463,11 +475,19 @@ static int ltc2945_probe(struct i2c_client *client) return PTR_ERR(regmap); } + data->regmap = regmap; + if (device_property_read_u32(dev, "shunt-resistor-micro-ohms", + &data->shunt_resistor)) + data->shunt_resistor = 1000; + + if (data->shunt_resistor == 0) + return -EINVAL; + /* Clear faults */ regmap_write(regmap, LTC2945_FAULT, 0x00); hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name, - regmap, + data, ltc2945_groups); return PTR_ERR_OR_ZERO(hwmon_dev); } -- 2.25.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/4] hwmon: ltc2945: Allow setting shunt resistor 2022-12-20 0:04 ` [PATCH v2 3/4] hwmon: ltc2945: Allow setting shunt resistor Cormier, Jonathan @ 2022-12-28 16:49 ` Guenter Roeck 0 siblings, 0 replies; 32+ messages in thread From: Guenter Roeck @ 2022-12-28 16:49 UTC (permalink / raw) To: Cormier, Jonathan Cc: linux-hwmon, John Pruitt, Jean Delvare, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke On Mon, Dec 19, 2022 at 07:04:56PM -0500, Cormier, Jonathan wrote: > From: John Pruitt <jpruitt@criticallink.com> > > Added the ability to specify the value of the shunt resistor in the a/Added/Add/ Per Documentation/process/submitting-patches.rst: " Describe your changes in imperative mood, e.g. "make xyzzy do frotz" instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy to do frotz", as if you are giving orders to the codebase to change its behaviour. " > device tree instead of assuming it is 1 milliOhm. The value in the > device tree has the name shunt-resistor-micro-ohms and the > default value is 1000 micro-ohms in order to preserve the > current behavior. > > Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> > Signed-off-by: John Pruitt <jpruitt@criticallink.com> > --- > drivers/hwmon/ltc2945.c | 80 +++++++++++++++++++++++++---------------- > 1 file changed, 50 insertions(+), 30 deletions(-) > > diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c > index 9af3e3821152..fc7d399b2c85 100644 > --- a/drivers/hwmon/ltc2945.c > +++ b/drivers/hwmon/ltc2945.c > @@ -64,6 +64,16 @@ static const struct of_device_id __maybe_unused ltc2945_of_match[] = { > }; > MODULE_DEVICE_TABLE(of, ltc2945_of_match); > > +/** > + * struct ltc2945_data - LTC2945 device data > + * @regmap: regmap device > + * @shunt_resistor: shunt resistor value in micro ohms (1000 by default) > + */ > +struct ltc2945_data { > + struct regmap *regmap; > + u32 shunt_resistor; > +}; > + > static inline bool is_power_reg(u8 reg) > { > return reg < LTC2945_SENSE_H; > @@ -72,7 +82,9 @@ static inline bool is_power_reg(u8 reg) > /* Return the value from the given register in uW, mV, or mA */ > static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > { > - struct regmap *regmap = dev_get_drvdata(dev); > + struct ltc2945_data *data = dev_get_drvdata(dev); > + struct regmap *regmap = data->regmap; > + u32 shunt_resistor = data->shunt_resistor; > unsigned int control; > u8 buf[3]; > long long val; > @@ -98,9 +110,7 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > case LTC2945_MAX_POWER_THRES_H: > case LTC2945_MIN_POWER_THRES_H: > /* > - * Convert to uW by assuming current is measured with > - * an 1mOhm sense resistor, similar to current > - * measurements. > + * Convert to uW > * Control register bit 0 selects if voltage at SENSE+/VDD > * or voltage at ADIN is used to measure power. > */ > @@ -114,6 +124,8 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > /* 0.5 mV * 25 uV = 0.0125 uV resolution. */ > val = (val * 25LL) >> 1; > } > + val *= 1000; > + val = DIV_ROUND_CLOSEST_ULL(val, shunt_resistor); > break; > case LTC2945_VIN_H: > case LTC2945_MAX_VIN_H: > @@ -136,14 +148,9 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > case LTC2945_MIN_SENSE_H: > case LTC2945_MAX_SENSE_THRES_H: > case LTC2945_MIN_SENSE_THRES_H: > - /* > - * 25 uV resolution. Convert to current as measured with > - * an 1 mOhm sense resistor, in mA. If a different sense > - * resistor is installed, calculate the actual current by > - * dividing the reported current by the sense resistor value > - * in mOhm. > - */ > - val *= 25; > + /* 25 uV resolution. Convert to mA. */ > + val *= 25 * 1000; > + val = DIV_ROUND_CLOSEST_ULL(val, shunt_resistor); > break; > default: > return -EINVAL; > @@ -154,7 +161,9 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > static int ltc2945_val_to_reg(struct device *dev, u8 reg, > unsigned long val) > { > - struct regmap *regmap = dev_get_drvdata(dev); > + struct ltc2945_data *data = dev_get_drvdata(dev); > + struct regmap *regmap = data->regmap; > + u32 shunt_resistor = data->shunt_resistor; > unsigned int control; > int ret; > > @@ -165,9 +174,6 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, > case LTC2945_MAX_POWER_THRES_H: > case LTC2945_MIN_POWER_THRES_H: > /* > - * Convert to register value by assuming current is measured > - * with an 1mOhm sense resistor, similar to current > - * measurements. > * Control register bit 0 selects if voltage at SENSE+/VDD > * or voltage at ADIN is used to measure power, which in turn > * determines register calculations. > @@ -177,14 +183,16 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, > return ret; > if (control & CONTROL_MULT_SELECT) { > /* 25 mV * 25 uV = 0.625 uV resolution. */ > - val = DIV_ROUND_CLOSEST(val, 625); > + val *= shunt_resistor; > + val = DIV_ROUND_CLOSEST(val, 625 * 1000); Overflows are introduced and need to be be handled in this patch. It doesn't make sense to introduce them and fix them in the next patch. > } else { > /* > * 0.5 mV * 25 uV = 0.0125 uV resolution. > * Divide first to avoid overflow; > * accept loss of accuracy. > */ > - val = DIV_ROUND_CLOSEST(val, 25) * 2; > + val *= shunt_resistor; > + val = DIV_ROUND_CLOSEST(val, 25 * 1000) * 2; > } > break; > case LTC2945_VIN_H: > @@ -208,14 +216,9 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, > case LTC2945_MIN_SENSE_H: > case LTC2945_MAX_SENSE_THRES_H: > case LTC2945_MIN_SENSE_THRES_H: > - /* > - * 25 uV resolution. Convert to current as measured with > - * an 1 mOhm sense resistor, in mA. If a different sense > - * resistor is installed, calculate the actual current by > - * dividing the reported current by the sense resistor value > - * in mOhm. > - */ > - val = DIV_ROUND_CLOSEST(val, 25); > + /* 25 uV resolution. Convert to mA. */ > + val *= shunt_resistor; > + val = DIV_ROUND_CLOSEST(val, 25 * 1000); > break; > default: > return -EINVAL; > @@ -240,7 +243,8 @@ static ssize_t ltc2945_value_store(struct device *dev, > const char *buf, size_t count) > { > struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > - struct regmap *regmap = dev_get_drvdata(dev); > + struct ltc2945_data *data = dev_get_drvdata(dev); > + struct regmap *regmap = data->regmap; > u8 reg = attr->index; > unsigned long val; > u8 regbuf[3]; > @@ -275,7 +279,8 @@ static ssize_t ltc2945_history_store(struct device *dev, > const char *buf, size_t count) > { > struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > - struct regmap *regmap = dev_get_drvdata(dev); > + struct ltc2945_data *data = dev_get_drvdata(dev); > + struct regmap *regmap = data->regmap; > u8 reg = attr->index; > int num_regs = is_power_reg(reg) ? 3 : 2; > u8 buf_min[3] = { 0xff, 0xff, 0xff }; > @@ -327,7 +332,8 @@ static ssize_t ltc2945_bool_show(struct device *dev, > struct device_attribute *da, char *buf) > { > struct sensor_device_attribute *attr = to_sensor_dev_attr(da); > - struct regmap *regmap = dev_get_drvdata(dev); > + struct ltc2945_data *data = dev_get_drvdata(dev); > + struct regmap *regmap = data->regmap; > unsigned int fault; > int ret; > > @@ -456,6 +462,12 @@ static int ltc2945_probe(struct i2c_client *client) > struct device *dev = &client->dev; > struct device *hwmon_dev; > struct regmap *regmap; > + struct ltc2945_data *data; > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + dev_set_drvdata(dev, data); > > regmap = devm_regmap_init_i2c(client, <c2945_regmap_config); > if (IS_ERR(regmap)) { > @@ -463,11 +475,19 @@ static int ltc2945_probe(struct i2c_client *client) > return PTR_ERR(regmap); > } > > + data->regmap = regmap; > + if (device_property_read_u32(dev, "shunt-resistor-micro-ohms", > + &data->shunt_resistor)) > + data->shunt_resistor = 1000; > + > + if (data->shunt_resistor == 0) > + return -EINVAL; > + > /* Clear faults */ > regmap_write(regmap, LTC2945_FAULT, 0x00); > > hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name, > - regmap, > + data, > ltc2945_groups); > return PTR_ERR_OR_ZERO(hwmon_dev); > } > -- > 2.25.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 4/4] hwmon: ltc2945: Fix possible overflows 2022-12-20 0:04 ` [PATCH v2 0/2] hwmon: ltc2945: Add binding and shunt resistor support Cormier, Jonathan ` (2 preceding siblings ...) 2022-12-20 0:04 ` [PATCH v2 3/4] hwmon: ltc2945: Allow setting shunt resistor Cormier, Jonathan @ 2022-12-20 0:04 ` Cormier, Jonathan 2022-12-28 17:01 ` Guenter Roeck 3 siblings, 1 reply; 32+ messages in thread From: Cormier, Jonathan @ 2022-12-20 0:04 UTC (permalink / raw) To: linux-hwmon Cc: John Pruitt, Jean Delvare, Guenter Roeck, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke, Cormier, Jonathan From: John Pruitt <jpruitt@criticallink.com> Use 64-bit values for intermediate calculations. Check for overflows and return INT_MAX if overflows happened. Signed-off-by: John Pruitt <jpruitt@criticallink.com> Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> --- drivers/hwmon/ltc2945.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c index fc7d399b2c85..7239422fc6db 100644 --- a/drivers/hwmon/ltc2945.c +++ b/drivers/hwmon/ltc2945.c @@ -126,6 +126,10 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) } val *= 1000; val = DIV_ROUND_CLOSEST_ULL(val, shunt_resistor); + /* check for overflow, use MAX value if it happened */ + if (val > INT_MAX) + val = INT_MAX; + break; case LTC2945_VIN_H: case LTC2945_MAX_VIN_H: @@ -159,12 +163,14 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) } static int ltc2945_val_to_reg(struct device *dev, u8 reg, - unsigned long val) + unsigned long val_32) { struct ltc2945_data *data = dev_get_drvdata(dev); struct regmap *regmap = data->regmap; u32 shunt_resistor = data->shunt_resistor; unsigned int control; + /* use 64-bit val for intermediate calculations */ + unsigned long long val = val_32; int ret; switch (reg) { @@ -184,7 +190,7 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, if (control & CONTROL_MULT_SELECT) { /* 25 mV * 25 uV = 0.625 uV resolution. */ val *= shunt_resistor; - val = DIV_ROUND_CLOSEST(val, 625 * 1000); + val = DIV_ROUND_CLOSEST_ULL(val, 625LL * 1000LL); } else { /* * 0.5 mV * 25 uV = 0.0125 uV resolution. @@ -192,7 +198,7 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, * accept loss of accuracy. */ val *= shunt_resistor; - val = DIV_ROUND_CLOSEST(val, 25 * 1000) * 2; + val = DIV_ROUND_CLOSEST_ULL(val, 25LL * 1000LL) * 2; } break; case LTC2945_VIN_H: @@ -201,7 +207,7 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, case LTC2945_MAX_VIN_THRES_H: case LTC2945_MIN_VIN_THRES_H: /* 25 mV resolution. */ - val /= 25; + val = DIV_ROUND_CLOSEST_ULL(val, 25LL); break; case LTC2945_ADIN_H: case LTC2945_MAX_ADIN_H: @@ -218,11 +224,15 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, case LTC2945_MIN_SENSE_THRES_H: /* 25 uV resolution. Convert to mA. */ val *= shunt_resistor; - val = DIV_ROUND_CLOSEST(val, 25 * 1000); + val = DIV_ROUND_CLOSEST_ULL(val, 25LL * 1000LL); break; default: return -EINVAL; } + /* If val is too large, just return the max value */ + if (val > INT_MAX) + return INT_MAX; + return val; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v2 4/4] hwmon: ltc2945: Fix possible overflows 2022-12-20 0:04 ` [PATCH v2 4/4] hwmon: ltc2945: Fix possible overflows Cormier, Jonathan @ 2022-12-28 17:01 ` Guenter Roeck 0 siblings, 0 replies; 32+ messages in thread From: Guenter Roeck @ 2022-12-28 17:01 UTC (permalink / raw) To: Cormier, Jonathan Cc: linux-hwmon, John Pruitt, Jean Delvare, linux-kernel, devicetree, Rob Herring, Krzysztof Kozlowski, Bob Duke On Mon, Dec 19, 2022 at 07:04:57PM -0500, Cormier, Jonathan wrote: > From: John Pruitt <jpruitt@criticallink.com> > > Use 64-bit values for intermediate calculations. Check for > overflows and return INT_MAX if overflows happened. > > Signed-off-by: John Pruitt <jpruitt@criticallink.com> > Signed-off-by: "Cormier, Jonathan" <jcormier@criticallink.com> The problems here are introduced with the previous patch and thus would need a Fixes: tag. It just doesn't make sense to submit that as separate patch. > --- > drivers/hwmon/ltc2945.c | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/drivers/hwmon/ltc2945.c b/drivers/hwmon/ltc2945.c > index fc7d399b2c85..7239422fc6db 100644 > --- a/drivers/hwmon/ltc2945.c > +++ b/drivers/hwmon/ltc2945.c > @@ -126,6 +126,10 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > } > val *= 1000; > val = DIV_ROUND_CLOSEST_ULL(val, shunt_resistor); > + /* check for overflow, use MAX value if it happened */ > + if (val > INT_MAX) > + val = INT_MAX; > + ltc2945_reg_to_val returns long long, and the calling code expects long long. How would this ever overflow ? > break; > case LTC2945_VIN_H: > case LTC2945_MAX_VIN_H: > @@ -159,12 +163,14 @@ static long long ltc2945_reg_to_val(struct device *dev, u8 reg) > } > > static int ltc2945_val_to_reg(struct device *dev, u8 reg, > - unsigned long val) > + unsigned long val_32) > { > struct ltc2945_data *data = dev_get_drvdata(dev); > struct regmap *regmap = data->regmap; > u32 shunt_resistor = data->shunt_resistor; > unsigned int control; > + /* use 64-bit val for intermediate calculations */ > + unsigned long long val = val_32; This is unnnecessary. The parameter can be unsigned long long, making the conversion automatic. > int ret; > > switch (reg) { > @@ -184,7 +190,7 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, > if (control & CONTROL_MULT_SELECT) { > /* 25 mV * 25 uV = 0.625 uV resolution. */ > val *= shunt_resistor; > - val = DIV_ROUND_CLOSEST(val, 625 * 1000); > + val = DIV_ROUND_CLOSEST_ULL(val, 625LL * 1000LL); > } else { > /* > * 0.5 mV * 25 uV = 0.0125 uV resolution. > @@ -192,7 +198,7 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, > * accept loss of accuracy. > */ > val *= shunt_resistor; > - val = DIV_ROUND_CLOSEST(val, 25 * 1000) * 2; > + val = DIV_ROUND_CLOSEST_ULL(val, 25LL * 1000LL) * 2; > } > break; > case LTC2945_VIN_H: > @@ -201,7 +207,7 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, > case LTC2945_MAX_VIN_THRES_H: > case LTC2945_MIN_VIN_THRES_H: > /* 25 mV resolution. */ > - val /= 25; > + val = DIV_ROUND_CLOSEST_ULL(val, 25LL); Unrelated change causing behavioral change. Not that I mind, but it is still unrelated and would have to be a separate patch. > break; > case LTC2945_ADIN_H: > case LTC2945_MAX_ADIN_H: > @@ -218,11 +224,15 @@ static int ltc2945_val_to_reg(struct device *dev, u8 reg, > case LTC2945_MIN_SENSE_THRES_H: > /* 25 uV resolution. Convert to mA. */ > val *= shunt_resistor; > - val = DIV_ROUND_CLOSEST(val, 25 * 1000); > + val = DIV_ROUND_CLOSEST_ULL(val, 25LL * 1000LL); > break; > default: > return -EINVAL; > } > + /* If val is too large, just return the max value */ > + if (val > INT_MAX) > + return INT_MAX; > + While the return value is declared as int, the calling code expects unsigned long. It would be better to adjust the return value and clamp against ULONG_MAX. > return val; > } > > -- > 2.25.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2023-01-18 18:33 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-14 22:07 [PATCH 0/2] hwmon: ltc2945: Add binding and shunt resistor support Cormier, Jonathan
2022-12-14 22:07 ` [PATCH 1/2] dt-bindings: hwmon: adi,ltc2945: Add binding Cormier, Jonathan
2022-12-15 9:42 ` Krzysztof Kozlowski
[not found] ` <CADL8D3ZUE5WbV0oS6hEVUNh9asrhTKQeGR4McR6Kh6qykSFw=Q@mail.gmail.com>
2022-12-15 14:37 ` Jon Cormier
2022-12-14 22:07 ` [PATCH 2/2] hwmon: ltc2945: Allow setting shunt resistor Cormier, Jonathan
2022-12-15 15:11 ` Guenter Roeck
2022-12-15 19:42 ` Jon Cormier
2023-01-09 23:35 ` [PATCH v3 0/2] hwmon: ltc2945: Add binding and shunt resistor support Jonathan Cormier
2023-01-09 23:35 ` [PATCH v3 1/5] dt-bindings: hwmon: adi,ltc2945: Add binding Jonathan Cormier
2023-01-10 9:07 ` Krzysztof Kozlowski
2023-01-09 23:35 ` [PATCH v3 2/5] hwmon: ltc2945: Add devicetree match table Jonathan Cormier
2023-01-09 23:35 ` [PATCH v3 3/5] hwmon: ltc2945: Handle error case in ltc2945_value_store Jonathan Cormier
2023-01-10 0:04 ` Guenter Roeck
2023-01-10 18:19 ` Jon Cormier
2023-01-10 18:22 ` Guenter Roeck
2023-01-10 19:25 ` Jon Cormier
2023-01-12 0:44 ` Guenter Roeck
2023-01-18 18:32 ` Jon Cormier
2023-01-09 23:35 ` [PATCH v3 4/5] hwmon: ltc2945: Allow setting shunt resistor Jonathan Cormier
2023-01-09 23:35 ` [PATCH v3 5/5] hwmon: ltc2945: Convert division to DIV_ROUND_CLOSEST_ULL Jonathan Cormier
2022-12-20 0:04 ` [PATCH v2 0/2] hwmon: ltc2945: Add binding and shunt resistor support Cormier, Jonathan
2022-12-20 0:04 ` [PATCH v2 1/4] dt-bindings: hwmon: adi,ltc2945: Add binding Cormier, Jonathan
2022-12-20 10:15 ` Krzysztof Kozlowski
2022-12-20 14:35 ` Jon Cormier
2022-12-20 14:46 ` Guenter Roeck
2022-12-20 21:47 ` Jon Cormier
2022-12-20 0:04 ` [PATCH v2 2/4] hwmon: ltc2945: Add devicetree match table Cormier, Jonathan
2022-12-28 16:43 ` Guenter Roeck
2022-12-20 0:04 ` [PATCH v2 3/4] hwmon: ltc2945: Allow setting shunt resistor Cormier, Jonathan
2022-12-28 16:49 ` Guenter Roeck
2022-12-20 0:04 ` [PATCH v2 4/4] hwmon: ltc2945: Fix possible overflows Cormier, Jonathan
2022-12-28 17:01 ` Guenter Roeck
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).