* [PATCH] hwmon: (pmbus) Use 64bit math for DIRECT format values
@ 2017-11-22 22:08 Robert Lippert
2017-11-22 22:28 ` Guenter Roeck
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Robert Lippert @ 2017-11-22 22:08 UTC (permalink / raw)
To: linux; +Cc: linux-hwmon, jdelvare, linux-kernel, xow, Robert Lippert
Power values in the 100s of watt range can easily blow past
32bit math limits when processing everything in microwatts.
Use 64bit math instead to avoid these issues on common 32bit ARM
BMC platforms.
Signed-off-by: Robert Lippert <rlippert@google.com>
---
drivers/hwmon/pmbus/pmbus_core.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 52a58b8b6e1b..f4efea9f282e 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -499,8 +499,8 @@ static long pmbus_reg2data_linear(struct pmbus_data *data,
static long pmbus_reg2data_direct(struct pmbus_data *data,
struct pmbus_sensor *sensor)
{
- long val = (s16) sensor->data;
- long m, b, R;
+ s64 val = (s16) sensor->data;
+ s64 m, b, R;
m = data->info->m[sensor->class];
b = data->info->b[sensor->class];
@@ -528,11 +528,13 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,
R--;
}
while (R < 0) {
- val = DIV_ROUND_CLOSEST(val, 10);
+ do_div(val, 10);
R++;
}
- return (val - b) / m;
+ val = val - b;
+ do_div(val, m);
+ return val;
}
/*
@@ -656,7 +658,8 @@ static u16 pmbus_data2reg_linear(struct pmbus_data *data,
static u16 pmbus_data2reg_direct(struct pmbus_data *data,
struct pmbus_sensor *sensor, long val)
{
- long m, b, R;
+ s64 m, b, R;
+ s64 val64 = val;
m = data->info->m[sensor->class];
b = data->info->b[sensor->class];
@@ -673,18 +676,18 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data,
R -= 3; /* Adjust R and b for data in milli-units */
b *= 1000;
}
- val = val * m + b;
+ val64 = val64 * m + b;
while (R > 0) {
- val *= 10;
+ val64 *= 10;
R--;
}
while (R < 0) {
- val = DIV_ROUND_CLOSEST(val, 10);
+ do_div(val64, 10);
R++;
}
- return val;
+ return (u16) val64;
}
static u16 pmbus_data2reg_vid(struct pmbus_data *data,
--
2.15.0.448.gf294e3d99a-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] hwmon: (pmbus) Use 64bit math for DIRECT format values
2017-11-22 22:08 [PATCH] hwmon: (pmbus) Use 64bit math for DIRECT format values Robert Lippert
@ 2017-11-22 22:28 ` Guenter Roeck
2017-11-27 21:42 ` Rob Lippert
2017-11-25 20:28 ` kbuild test robot
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2017-11-22 22:28 UTC (permalink / raw)
To: Robert Lippert; +Cc: linux-hwmon, jdelvare, linux-kernel, xow, Robert Lippert
On Wed, Nov 22, 2017 at 02:08:43PM -0800, Robert Lippert wrote:
> Power values in the 100s of watt range can easily blow past
> 32bit math limits when processing everything in microwatts.
>
> Use 64bit math instead to avoid these issues on common 32bit ARM
> BMC platforms.
>
> Signed-off-by: Robert Lippert <rlippert@google.com>
> ---
> drivers/hwmon/pmbus/pmbus_core.c | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 52a58b8b6e1b..f4efea9f282e 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -499,8 +499,8 @@ static long pmbus_reg2data_linear(struct pmbus_data *data,
> static long pmbus_reg2data_direct(struct pmbus_data *data,
> struct pmbus_sensor *sensor)
> {
> - long val = (s16) sensor->data;
> - long m, b, R;
> + s64 val = (s16) sensor->data;
> + s64 m, b, R;
>
> m = data->info->m[sensor->class];
> b = data->info->b[sensor->class];
> @@ -528,11 +528,13 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,
> R--;
> }
> while (R < 0) {
> - val = DIV_ROUND_CLOSEST(val, 10);
> + do_div(val, 10);
Any reason to not use DIV_ROUND_CLOSEST_ULL ?
> R++;
> }
>
> - return (val - b) / m;
> + val = val - b;
> + do_div(val, m);
> + return val;
Can this overflow ?
> }
>
> /*
> @@ -656,7 +658,8 @@ static u16 pmbus_data2reg_linear(struct pmbus_data *data,
> static u16 pmbus_data2reg_direct(struct pmbus_data *data,
> struct pmbus_sensor *sensor, long val)
> {
> - long m, b, R;
> + s64 m, b, R;
> + s64 val64 = val;
>
> m = data->info->m[sensor->class];
> b = data->info->b[sensor->class];
> @@ -673,18 +676,18 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data,
> R -= 3; /* Adjust R and b for data in milli-units */
> b *= 1000;
> }
> - val = val * m + b;
> + val64 = val64 * m + b;
>
> while (R > 0) {
> - val *= 10;
> + val64 *= 10;
> R--;
> }
> while (R < 0) {
> - val = DIV_ROUND_CLOSEST(val, 10);
> + do_div(val64, 10);
Same here.
> R++;
> }
>
> - return val;
> + return (u16) val64;
Can this overflow ?
> }
>
> static u16 pmbus_data2reg_vid(struct pmbus_data *data,
> --
> 2.15.0.448.gf294e3d99a-goog
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hwmon: (pmbus) Use 64bit math for DIRECT format values
2017-11-22 22:08 [PATCH] hwmon: (pmbus) Use 64bit math for DIRECT format values Robert Lippert
2017-11-22 22:28 ` Guenter Roeck
@ 2017-11-25 20:28 ` kbuild test robot
2017-11-27 21:39 ` [PATCH v2] " Robert Lippert
2017-11-27 23:51 ` [PATCH v3] " Robert Lippert
3 siblings, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2017-11-25 20:28 UTC (permalink / raw)
To: Robert Lippert
Cc: kbuild-all, linux, linux-hwmon, jdelvare, linux-kernel, xow,
Robert Lippert
[-- Attachment #1: Type: text/plain, Size: 3655 bytes --]
Hi Robert,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on v4.14 next-20171124]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Robert-Lippert/hwmon-pmbus-Use-64bit-math-for-DIRECT-format-values/20171126-031040
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All warnings (new ones prefixed by >>):
In file included from ./arch/xtensa/include/generated/asm/div64.h:1:0,
from include/linux/kernel.h:173,
from include/linux/list.h:9,
from include/linux/wait.h:7,
from include/linux/wait_bit.h:8,
from include/linux/fs.h:6,
from include/linux/debugfs.h:18,
from drivers/hwmon//pmbus/pmbus_core.c:22:
drivers/hwmon//pmbus/pmbus_core.c: In function 'pmbus_reg2data_direct':
include/asm-generic/div64.h:222:28: warning: comparison of distinct pointer types lacks a cast
(void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
^
>> drivers/hwmon//pmbus/pmbus_core.c:531:3: note: in expansion of macro 'do_div'
do_div(val, 10);
^
include/asm-generic/div64.h:222:28: warning: comparison of distinct pointer types lacks a cast
(void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
^
drivers/hwmon//pmbus/pmbus_core.c:536:2: note: in expansion of macro 'do_div'
do_div(val, m);
^
drivers/hwmon//pmbus/pmbus_core.c: In function 'pmbus_data2reg_direct':
include/asm-generic/div64.h:222:28: warning: comparison of distinct pointer types lacks a cast
(void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
^
drivers/hwmon//pmbus/pmbus_core.c:686:3: note: in expansion of macro 'do_div'
do_div(val64, 10);
^
vim +/do_div +531 drivers/hwmon//pmbus/pmbus_core.c
494
495 /*
496 * Convert direct sensor values to milli- or micro-units
497 * depending on sensor type.
498 */
499 static long pmbus_reg2data_direct(struct pmbus_data *data,
500 struct pmbus_sensor *sensor)
501 {
502 s64 val = (s16) sensor->data;
503 s64 m, b, R;
504
505 m = data->info->m[sensor->class];
506 b = data->info->b[sensor->class];
507 R = data->info->R[sensor->class];
508
509 if (m == 0)
510 return 0;
511
512 /* X = 1/m * (Y * 10^-R - b) */
513 R = -R;
514 /* scale result to milli-units for everything but fans */
515 if (sensor->class != PSC_FAN) {
516 R += 3;
517 b *= 1000;
518 }
519
520 /* scale result to micro-units for power sensors */
521 if (sensor->class == PSC_POWER) {
522 R += 3;
523 b *= 1000;
524 }
525
526 while (R > 0) {
527 val *= 10;
528 R--;
529 }
530 while (R < 0) {
> 531 do_div(val, 10);
532 R++;
533 }
534
535 val = val - b;
536 do_div(val, m);
537 return val;
538 }
539
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51899 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] hwmon: (pmbus) Use 64bit math for DIRECT format values
2017-11-22 22:08 [PATCH] hwmon: (pmbus) Use 64bit math for DIRECT format values Robert Lippert
2017-11-22 22:28 ` Guenter Roeck
2017-11-25 20:28 ` kbuild test robot
@ 2017-11-27 21:39 ` Robert Lippert
2017-11-27 22:11 ` Guenter Roeck
2017-11-27 23:51 ` [PATCH v3] " Robert Lippert
3 siblings, 1 reply; 9+ messages in thread
From: Robert Lippert @ 2017-11-27 21:39 UTC (permalink / raw)
To: linux; +Cc: linux-hwmon, jdelvare, linux-kernel, xow, Robert Lippert
Power values in the 100s of watt range can easily blow past
32bit math limits when processing everything in microwatts.
Use 64bit math instead to avoid these issues on common 32bit ARM
BMC platforms.
Signed-off-by: Robert Lippert <rlippert@google.com>
---
drivers/hwmon/pmbus/pmbus_core.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 52a58b8b6e1b..ff701502974a 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -21,6 +21,7 @@
#include <linux/debugfs.h>
#include <linux/kernel.h>
+#include <linux/math64.h>
#include <linux/module.h>
#include <linux/init.h>
#include <linux/err.h>
@@ -499,8 +500,9 @@ static long pmbus_reg2data_linear(struct pmbus_data *data,
static long pmbus_reg2data_direct(struct pmbus_data *data,
struct pmbus_sensor *sensor)
{
- long val = (s16) sensor->data;
- long m, b, R;
+ s64 val = (s16) sensor->data;
+ s64 b, R;
+ s32 m;
m = data->info->m[sensor->class];
b = data->info->b[sensor->class];
@@ -528,11 +530,12 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,
R--;
}
while (R < 0) {
- val = DIV_ROUND_CLOSEST(val, 10);
+ val = div_s64(val + 5LL, 10L); // round closest
R++;
}
- return (val - b) / m;
+ val = div_s64(val - b, m);
+ return clamp_val(val, LONG_MIN, LONG_MAX);
}
/*
@@ -656,7 +659,9 @@ static u16 pmbus_data2reg_linear(struct pmbus_data *data,
static u16 pmbus_data2reg_direct(struct pmbus_data *data,
struct pmbus_sensor *sensor, long val)
{
- long m, b, R;
+ s64 b, R;
+ s32 m;
+ s64 val64 = val;
m = data->info->m[sensor->class];
b = data->info->b[sensor->class];
@@ -673,18 +678,18 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data,
R -= 3; /* Adjust R and b for data in milli-units */
b *= 1000;
}
- val = val * m + b;
+ val64 = val64 * m + b;
while (R > 0) {
- val *= 10;
+ val64 *= 10;
R--;
}
while (R < 0) {
- val = DIV_ROUND_CLOSEST(val, 10);
+ val64 = div_s64(val64 + 5LL, 10L); // round closest
R++;
}
- return val;
+ return (u16) clamp_val(val64, S16_MIN, S16_MAX);
}
static u16 pmbus_data2reg_vid(struct pmbus_data *data,
--
2.15.0.417.g466bffb3ac-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] hwmon: (pmbus) Use 64bit math for DIRECT format values
2017-11-22 22:28 ` Guenter Roeck
@ 2017-11-27 21:42 ` Rob Lippert
0 siblings, 0 replies; 9+ messages in thread
From: Rob Lippert @ 2017-11-27 21:42 UTC (permalink / raw)
To: Guenter Roeck
Cc: Robert Lippert, linux-hwmon, jdelvare, linux-kernel, Xo Wang
On Wed, Nov 22, 2017 at 2:28 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On Wed, Nov 22, 2017 at 02:08:43PM -0800, Robert Lippert wrote:
>> Power values in the 100s of watt range can easily blow past
>> 32bit math limits when processing everything in microwatts.
>>
>> Use 64bit math instead to avoid these issues on common 32bit ARM
>> BMC platforms.
>>
>> Signed-off-by: Robert Lippert <rlippert@google.com>
>> ---
>> drivers/hwmon/pmbus/pmbus_core.c | 21 ++++++++++++---------
>> 1 file changed, 12 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
>> index 52a58b8b6e1b..f4efea9f282e 100644
>> --- a/drivers/hwmon/pmbus/pmbus_core.c
>> +++ b/drivers/hwmon/pmbus/pmbus_core.c
>> @@ -499,8 +499,8 @@ static long pmbus_reg2data_linear(struct pmbus_data *data,
>> static long pmbus_reg2data_direct(struct pmbus_data *data,
>> struct pmbus_sensor *sensor)
>> {
>> - long val = (s16) sensor->data;
>> - long m, b, R;
>> + s64 val = (s16) sensor->data;
>> + s64 m, b, R;
>>
>> m = data->info->m[sensor->class];
>> b = data->info->b[sensor->class];
>> @@ -528,11 +528,13 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,
>> R--;
>> }
>> while (R < 0) {
>> - val = DIV_ROUND_CLOSEST(val, 10);
>> + do_div(val, 10);
>
> Any reason to not use DIV_ROUND_CLOSEST_ULL ?
That macro doesn't quite work for signed division. v2 changes this to
use the signed 64bit division functions and I emulated the "round
closest" by just adding 5 before dividing.
>
>> R++;
>> }
>>
>> - return (val - b) / m;
>> + val = val - b;
>> + do_div(val, m);
>> + return val;
>
> Can this overflow ?
Added clamp() to the return values in v2 to prevent overflow when
returning a narrower type.
>
>> }
>>
>> /*
>> @@ -656,7 +658,8 @@ static u16 pmbus_data2reg_linear(struct pmbus_data *data,
>> static u16 pmbus_data2reg_direct(struct pmbus_data *data,
>> struct pmbus_sensor *sensor, long val)
>> {
>> - long m, b, R;
>> + s64 m, b, R;
>> + s64 val64 = val;
>>
>> m = data->info->m[sensor->class];
>> b = data->info->b[sensor->class];
>> @@ -673,18 +676,18 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data,
>> R -= 3; /* Adjust R and b for data in milli-units */
>> b *= 1000;
>> }
>> - val = val * m + b;
>> + val64 = val64 * m + b;
>>
>> while (R > 0) {
>> - val *= 10;
>> + val64 *= 10;
>> R--;
>> }
>> while (R < 0) {
>> - val = DIV_ROUND_CLOSEST(val, 10);
>> + do_div(val64, 10);
>
> Same here.
>
>> R++;
>> }
>>
>> - return val;
>> + return (u16) val64;
>
> Can this overflow ?
>
>> }
>>
>> static u16 pmbus_data2reg_vid(struct pmbus_data *data,
>> --
>> 2.15.0.448.gf294e3d99a-goog
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] hwmon: (pmbus) Use 64bit math for DIRECT format values
2017-11-27 21:39 ` [PATCH v2] " Robert Lippert
@ 2017-11-27 22:11 ` Guenter Roeck
2017-11-27 23:51 ` Rob Lippert
0 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2017-11-27 22:11 UTC (permalink / raw)
To: Robert Lippert; +Cc: linux-hwmon, jdelvare, linux-kernel, xow, Robert Lippert
On Mon, Nov 27, 2017 at 01:39:14PM -0800, Robert Lippert wrote:
> Power values in the 100s of watt range can easily blow past
> 32bit math limits when processing everything in microwatts.
>
> Use 64bit math instead to avoid these issues on common 32bit ARM
> BMC platforms.
>
> Signed-off-by: Robert Lippert <rlippert@google.com>
> ---
> drivers/hwmon/pmbus/pmbus_core.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 52a58b8b6e1b..ff701502974a 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -21,6 +21,7 @@
>
> #include <linux/debugfs.h>
> #include <linux/kernel.h>
> +#include <linux/math64.h>
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/err.h>
> @@ -499,8 +500,9 @@ static long pmbus_reg2data_linear(struct pmbus_data *data,
> static long pmbus_reg2data_direct(struct pmbus_data *data,
> struct pmbus_sensor *sensor)
> {
> - long val = (s16) sensor->data;
> - long m, b, R;
> + s64 val = (s16) sensor->data;
> + s64 b, R;
Do b and R have to be s64 ? Seems excessive.
> + s32 m;
>
> m = data->info->m[sensor->class];
> b = data->info->b[sensor->class];
> @@ -528,11 +530,12 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,
> R--;
> }
> while (R < 0) {
> - val = DIV_ROUND_CLOSEST(val, 10);
> + val = div_s64(val + 5LL, 10L); // round closest
> R++;
> }
>
> - return (val - b) / m;
> + val = div_s64(val - b, m);
> + return clamp_val(val, LONG_MIN, LONG_MAX);
> }
>
> /*
> @@ -656,7 +659,9 @@ static u16 pmbus_data2reg_linear(struct pmbus_data *data,
> static u16 pmbus_data2reg_direct(struct pmbus_data *data,
> struct pmbus_sensor *sensor, long val)
> {
> - long m, b, R;
> + s64 b, R;
Same question as above - do those have to be s64 ?
> + s32 m;
> + s64 val64 = val;
>
> m = data->info->m[sensor->class];
> b = data->info->b[sensor->class];
> @@ -673,18 +678,18 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data,
> R -= 3; /* Adjust R and b for data in milli-units */
> b *= 1000;
> }
> - val = val * m + b;
> + val64 = val64 * m + b;
>
> while (R > 0) {
> - val *= 10;
> + val64 *= 10;
> R--;
> }
> while (R < 0) {
> - val = DIV_ROUND_CLOSEST(val, 10);
> + val64 = div_s64(val64 + 5LL, 10L); // round closest
> R++;
> }
>
> - return val;
> + return (u16) clamp_val(val64, S16_MIN, S16_MAX);
> }
>
> static u16 pmbus_data2reg_vid(struct pmbus_data *data,
> --
> 2.15.0.417.g466bffb3ac-goog
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] hwmon: (pmbus) Use 64bit math for DIRECT format values
2017-11-27 22:11 ` Guenter Roeck
@ 2017-11-27 23:51 ` Rob Lippert
0 siblings, 0 replies; 9+ messages in thread
From: Rob Lippert @ 2017-11-27 23:51 UTC (permalink / raw)
To: Guenter Roeck
Cc: Robert Lippert, linux-hwmon, jdelvare, linux-kernel, Xo Wang
On Mon, Nov 27, 2017 at 2:11 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On Mon, Nov 27, 2017 at 01:39:14PM -0800, Robert Lippert wrote:
>> Power values in the 100s of watt range can easily blow past
>> 32bit math limits when processing everything in microwatts.
>>
>> Use 64bit math instead to avoid these issues on common 32bit ARM
>> BMC platforms.
>>
>> Signed-off-by: Robert Lippert <rlippert@google.com>
>> ---
>> drivers/hwmon/pmbus/pmbus_core.c | 23 ++++++++++++++---------
>> 1 file changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
>> index 52a58b8b6e1b..ff701502974a 100644
>> --- a/drivers/hwmon/pmbus/pmbus_core.c
>> +++ b/drivers/hwmon/pmbus/pmbus_core.c
>> @@ -21,6 +21,7 @@
>>
>> #include <linux/debugfs.h>
>> #include <linux/kernel.h>
>> +#include <linux/math64.h>
>> #include <linux/module.h>
>> #include <linux/init.h>
>> #include <linux/err.h>
>> @@ -499,8 +500,9 @@ static long pmbus_reg2data_linear(struct pmbus_data *data,
>> static long pmbus_reg2data_direct(struct pmbus_data *data,
>> struct pmbus_sensor *sensor)
>> {
>> - long val = (s16) sensor->data;
>> - long m, b, R;
>> + s64 val = (s16) sensor->data;
>> + s64 b, R;
>
> Do b and R have to be s64 ? Seems excessive.
b has a range of -32767 to 32768 and is multiplied in the code below
by 1,000,000 for a power sensor so I think needs to be 64bit to avoid
overflow.
R has limited range so changed in v3 to s32 type.
>
>> + s32 m;
>>
>> m = data->info->m[sensor->class];
>> b = data->info->b[sensor->class];
>> @@ -528,11 +530,12 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,
>> R--;
>> }
>> while (R < 0) {
>> - val = DIV_ROUND_CLOSEST(val, 10);
>> + val = div_s64(val + 5LL, 10L); // round closest
(also fixed up in v3 this C++ style comment that slipped in...)
>> R++;
>> }
>>
>> - return (val - b) / m;
>> + val = div_s64(val - b, m);
>> + return clamp_val(val, LONG_MIN, LONG_MAX);
>> }
>>
>> /*
>> @@ -656,7 +659,9 @@ static u16 pmbus_data2reg_linear(struct pmbus_data *data,
>> static u16 pmbus_data2reg_direct(struct pmbus_data *data,
>> struct pmbus_sensor *sensor, long val)
>> {
>> - long m, b, R;
>> + s64 b, R;
>
> Same question as above - do those have to be s64 ?
>
>> + s32 m;
>> + s64 val64 = val;
>>
>> m = data->info->m[sensor->class];
>> b = data->info->b[sensor->class];
>> @@ -673,18 +678,18 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data,
>> R -= 3; /* Adjust R and b for data in milli-units */
>> b *= 1000;
>> }
>> - val = val * m + b;
>> + val64 = val64 * m + b;
>>
>> while (R > 0) {
>> - val *= 10;
>> + val64 *= 10;
>> R--;
>> }
>> while (R < 0) {
>> - val = DIV_ROUND_CLOSEST(val, 10);
>> + val64 = div_s64(val64 + 5LL, 10L); // round closest
>> R++;
>> }
>>
>> - return val;
>> + return (u16) clamp_val(val64, S16_MIN, S16_MAX);
>> }
>>
>> static u16 pmbus_data2reg_vid(struct pmbus_data *data,
>> --
>> 2.15.0.417.g466bffb3ac-goog
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3] hwmon: (pmbus) Use 64bit math for DIRECT format values
2017-11-22 22:08 [PATCH] hwmon: (pmbus) Use 64bit math for DIRECT format values Robert Lippert
` (2 preceding siblings ...)
2017-11-27 21:39 ` [PATCH v2] " Robert Lippert
@ 2017-11-27 23:51 ` Robert Lippert
2017-11-28 4:12 ` [v3] " Guenter Roeck
3 siblings, 1 reply; 9+ messages in thread
From: Robert Lippert @ 2017-11-27 23:51 UTC (permalink / raw)
To: linux; +Cc: linux-hwmon, jdelvare, linux-kernel, xow, Robert Lippert
Power values in the 100s of watt range can easily blow past
32bit math limits when processing everything in microwatts.
Use 64bit math instead to avoid these issues on common 32bit ARM
BMC platforms.
Signed-off-by: Robert Lippert <rlippert@google.com>
---
drivers/hwmon/pmbus/pmbus_core.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 52a58b8b6e1b..09977440bbea 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -21,6 +21,7 @@
#include <linux/debugfs.h>
#include <linux/kernel.h>
+#include <linux/math64.h>
#include <linux/module.h>
#include <linux/init.h>
#include <linux/err.h>
@@ -499,8 +500,8 @@ static long pmbus_reg2data_linear(struct pmbus_data *data,
static long pmbus_reg2data_direct(struct pmbus_data *data,
struct pmbus_sensor *sensor)
{
- long val = (s16) sensor->data;
- long m, b, R;
+ s64 b, val = (s16) sensor->data;
+ s32 m, R;
m = data->info->m[sensor->class];
b = data->info->b[sensor->class];
@@ -528,11 +529,12 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,
R--;
}
while (R < 0) {
- val = DIV_ROUND_CLOSEST(val, 10);
+ val = div_s64(val + 5LL, 10L); /* round closest */
R++;
}
- return (val - b) / m;
+ val = div_s64(val - b, m);
+ return clamp_val(val, LONG_MIN, LONG_MAX);
}
/*
@@ -656,7 +658,8 @@ static u16 pmbus_data2reg_linear(struct pmbus_data *data,
static u16 pmbus_data2reg_direct(struct pmbus_data *data,
struct pmbus_sensor *sensor, long val)
{
- long m, b, R;
+ s64 b, val64 = val;
+ s32 m, R;
m = data->info->m[sensor->class];
b = data->info->b[sensor->class];
@@ -673,18 +676,18 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data,
R -= 3; /* Adjust R and b for data in milli-units */
b *= 1000;
}
- val = val * m + b;
+ val64 = val64 * m + b;
while (R > 0) {
- val *= 10;
+ val64 *= 10;
R--;
}
while (R < 0) {
- val = DIV_ROUND_CLOSEST(val, 10);
+ val64 = div_s64(val64 + 5LL, 10L); /* round closest */
R++;
}
- return val;
+ return (u16) clamp_val(val64, S16_MIN, S16_MAX);
}
static u16 pmbus_data2reg_vid(struct pmbus_data *data,
--
2.15.0.417.g466bffb3ac-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [v3] hwmon: (pmbus) Use 64bit math for DIRECT format values
2017-11-27 23:51 ` [PATCH v3] " Robert Lippert
@ 2017-11-28 4:12 ` Guenter Roeck
0 siblings, 0 replies; 9+ messages in thread
From: Guenter Roeck @ 2017-11-28 4:12 UTC (permalink / raw)
To: Robert Lippert; +Cc: linux-hwmon, jdelvare, linux-kernel, xow, Robert Lippert
On Mon, Nov 27, 2017 at 03:51:55PM -0800, Robert Lippert wrote:
> Power values in the 100s of watt range can easily blow past
> 32bit math limits when processing everything in microwatts.
>
> Use 64bit math instead to avoid these issues on common 32bit ARM
> BMC platforms.
>
> Signed-off-by: Robert Lippert <rlippert@google.com>
Applied (silently fixed two checkpatch CHECK messages)
Guenter
> ---
> drivers/hwmon/pmbus/pmbus_core.c | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 52a58b8b6e1b..09977440bbea 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -21,6 +21,7 @@
>
> #include <linux/debugfs.h>
> #include <linux/kernel.h>
> +#include <linux/math64.h>
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/err.h>
> @@ -499,8 +500,8 @@ static long pmbus_reg2data_linear(struct pmbus_data *data,
> static long pmbus_reg2data_direct(struct pmbus_data *data,
> struct pmbus_sensor *sensor)
> {
> - long val = (s16) sensor->data;
> - long m, b, R;
> + s64 b, val = (s16)sensor->data;
> + s32 m, R;
>
> m = data->info->m[sensor->class];
> b = data->info->b[sensor->class];
> @@ -528,11 +529,12 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,
> R--;
> }
> while (R < 0) {
> - val = DIV_ROUND_CLOSEST(val, 10);
> + val = div_s64(val + 5LL, 10L); /* round closest */
> R++;
> }
>
> - return (val - b) / m;
> + val = div_s64(val - b, m);
> + return clamp_val(val, LONG_MIN, LONG_MAX);
> }
>
> /*
> @@ -656,7 +658,8 @@ static u16 pmbus_data2reg_linear(struct pmbus_data *data,
> static u16 pmbus_data2reg_direct(struct pmbus_data *data,
> struct pmbus_sensor *sensor, long val)
> {
> - long m, b, R;
> + s64 b, val64 = val;
> + s32 m, R;
>
> m = data->info->m[sensor->class];
> b = data->info->b[sensor->class];
> @@ -673,18 +676,18 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data,
> R -= 3; /* Adjust R and b for data in milli-units */
> b *= 1000;
> }
> - val = val * m + b;
> + val64 = val64 * m + b;
>
> while (R > 0) {
> - val *= 10;
> + val64 *= 10;
> R--;
> }
> while (R < 0) {
> - val = DIV_ROUND_CLOSEST(val, 10);
> + val64 = div_s64(val64 + 5LL, 10L); /* round closest */
> R++;
> }
>
> - return val;
> + return (u16)clamp_val(val64, S16_MIN, S16_MAX);
> }
>
> static u16 pmbus_data2reg_vid(struct pmbus_data *data,
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-11-28 4:12 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-22 22:08 [PATCH] hwmon: (pmbus) Use 64bit math for DIRECT format values Robert Lippert
2017-11-22 22:28 ` Guenter Roeck
2017-11-27 21:42 ` Rob Lippert
2017-11-25 20:28 ` kbuild test robot
2017-11-27 21:39 ` [PATCH v2] " Robert Lippert
2017-11-27 22:11 ` Guenter Roeck
2017-11-27 23:51 ` Rob Lippert
2017-11-27 23:51 ` [PATCH v3] " Robert Lippert
2017-11-28 4:12 ` [v3] " Guenter Roeck
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.