Skip to contentSkip to conversation

hwmon: (pmbus) Use 64bit math for DIRECT format values

PATCH Superseded Robert Lippert 4 participants 9 messages Nov 22, 2017 – Nov 28, 2017 Hardware monitoring

lore
Share & tools
lore

Robert Lippert

author
#0 lore raw

PATCH 1/1 hwmon: (pmbus) Use 64bit math for DIRECT format values

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 <[email protected]>
---
 drivers/hwmon/pmbus/pmbus_core.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
drivers/hwmon/pmbus/pmbus_core.c +12 -9
index 52a58b8b6e1b..f4efea9f282e 100644
@@ -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,
Signature · 1 line
2.15.0.448.gf294e3d99a-goog
maintainer
#1 lore raw
Quoted 27 lines · 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 <[email protected]>
---
 drivers/hwmon/pmbus/pmbus_core.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
drivers/hwmon/pmbus/pmbus_core.c +2 -2
index 52a58b8b6e1b..f4efea9f282e 100644
@@ -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];

drivers/hwmon/pmbus/pmbus_core.c static long pmbus_reg2data_direct(struct pmbus_data *data, · line 531

Quoted 5 lines
drivers/hwmon/pmbus/pmbus_core.c +1 -1
@@ -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 ?

drivers/hwmon/pmbus/pmbus_core.c static long pmbus_reg2data_direct(struct pmbus_data *data, · line 537

Quoted 7 lines
drivers/hwmon/pmbus/pmbus_core.c +3 -1
 		R++;
 	}
 
-	return (val - b) / m;
+	val = val - b;
+	do_div(val, m);
+	return val;

Can this overflow ?

Quoted 18 lines
drivers/hwmon/pmbus/pmbus_core.c +2 -2
 }
 
 /*

@@ -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;

drivers/hwmon/pmbus/pmbus_core.c static u16 pmbus_data2reg_direct(struct pmbus_data *data, · line 686

Quoted 10 lines
drivers/hwmon/pmbus/pmbus_core.c +3 -2
+	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.

drivers/hwmon/pmbus/pmbus_core.c static u16 pmbus_data2reg_direct(struct pmbus_data *data, · line 690

Quoted 5 lines
drivers/hwmon/pmbus/pmbus_core.c +1 -1
 		R++;
 	}
 
-	return val;
+	return (u16) val64;

Can this overflow ?

Quoted 6 lines
drivers/hwmon/pmbus/pmbus_core.c +0 -0
 }
 
 static u16 pmbus_data2reg_vid(struct pmbus_data *data,
Signature · 1 line
2.15.0.448.gf294e3d99a-goog
#2 lore raw
Quoted 25 lines · Guenter Roeck
Quoted 24 lines · 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 <[email protected]>
---
 drivers/hwmon/pmbus/pmbus_core.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
drivers/hwmon/pmbus/pmbus_core.c +2 -2
index 52a58b8b6e1b..f4efea9f282e 100644
@@ -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;
Quoted 10 lines

drivers/hwmon/pmbus/pmbus_core.c static long pmbus_reg2data_direct(struct pmbus_data *data, · line 531

Quoted 8 lines
drivers/hwmon/pmbus/pmbus_core.c +1 -1
      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.

Quoted 10 lines

drivers/hwmon/pmbus/pmbus_core.c static long pmbus_reg2data_direct(struct pmbus_data *data, · line 537

Quoted 8 lines
drivers/hwmon/pmbus/pmbus_core.c +3 -1
              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.

Quoted 46 lines
Quoted 19 lines
drivers/hwmon/pmbus/pmbus_core.c +2 -2
 }
 
 /*

@@ -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;

drivers/hwmon/pmbus/pmbus_core.c static u16 pmbus_data2reg_direct(struct pmbus_data *data, · line 686

Quoted 10 lines
drivers/hwmon/pmbus/pmbus_core.c +3 -2
+     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.

drivers/hwmon/pmbus/pmbus_core.c static u16 pmbus_data2reg_direct(struct pmbus_data *data, · line 690

Quoted 5 lines
drivers/hwmon/pmbus/pmbus_core.c +1 -1
              R++;
      }
 
-     return val;
+     return (u16) val64;

Can this overflow ?

Quoted 6 lines
drivers/hwmon/pmbus/pmbus_core.c +0 -1
 }
 
 static u16 pmbus_data2reg_vid(struct pmbus_data *data,
--

2.15.0.448.gf294e3d99a-goog

build bot
#3 lore raw
In reply to Robert Lippert
Thank you for the patch! Perhaps something to improve:

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

author
#4 lore raw

PATCH v2 1/1 hwmon: (pmbus) Use 64bit math for DIRECT format values

In reply to 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 <[email protected]>
---
 drivers/hwmon/pmbus/pmbus_core.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)
drivers/hwmon/pmbus/pmbus_core.c +14 -9
index 52a58b8b6e1b..ff701502974a 100644
@@ -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,
Signature · 1 line
2.15.0.417.g466bffb3ac-goog
maintainer
#5 lore raw
Quoted 24 lines · 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 <[email protected]>
---
 drivers/hwmon/pmbus/pmbus_core.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)
drivers/hwmon/pmbus/pmbus_core.c +1 -0
index 52a58b8b6e1b..ff701502974a 100644
@@ -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>

drivers/hwmon/pmbus/pmbus_core.c static long pmbus_reg2data_linear(struct pmbus_data *data, · line 504

Quoted 7 lines
drivers/hwmon/pmbus/pmbus_core.c +2 -2
@@ -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.

Quoted 20 lines
drivers/hwmon/pmbus/pmbus_core.c +4 -2
+	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);
 }
 
 /*

drivers/hwmon/pmbus/pmbus_core.c static u16 pmbus_data2reg_linear(struct pmbus_data *data, · line 662

Quoted 5 lines
drivers/hwmon/pmbus/pmbus_core.c +1 -1
@@ -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 ?

Quoted 35 lines
drivers/hwmon/pmbus/pmbus_core.c +6 -4
+	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,
Signature · 6 lines
2.15.0.417.g466bffb3ac-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#6 lore raw
Quoted 18 lines · Guenter Roeck
Quoted 17 lines · 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 <[email protected]>
---
 drivers/hwmon/pmbus/pmbus_core.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)
drivers/hwmon/pmbus/pmbus_core.c +0 -0
index 52a58b8b6e1b..ff701502974a 100644
@@ -21,6 +21,7 @@
Quoted 16 lines
Quoted 7 lines
drivers/hwmon/pmbus/pmbus_core.c +1 -0
 #include <linux/debugfs.h>
 #include <linux/kernel.h>
+#include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/err.h>

drivers/hwmon/pmbus/pmbus_core.c static long pmbus_reg2data_linear(struct pmbus_data *data, · line 504

Quoted 7 lines
drivers/hwmon/pmbus/pmbus_core.c +2 -2
@@ -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.

Quoted 11 lines

drivers/hwmon/pmbus/pmbus_core.c static long pmbus_reg2data_direct(struct pmbus_data *data, · line 533

Quoted 11 lines
drivers/hwmon/pmbus/pmbus_core.c +2 -1
+     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...)

Quoted 53 lines
Quoted 10 lines
drivers/hwmon/pmbus/pmbus_core.c +2 -1
              R++;
      }
 
-     return (val - b) / m;
+     val = div_s64(val - b, m);
+     return clamp_val(val, LONG_MIN, LONG_MAX);
 }
 
 /*

drivers/hwmon/pmbus/pmbus_core.c static u16 pmbus_data2reg_linear(struct pmbus_data *data, · line 662

Quoted 5 lines
drivers/hwmon/pmbus/pmbus_core.c +1 -1
@@ -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 ?

Quoted 35 lines
drivers/hwmon/pmbus/pmbus_core.c +6 -5
+     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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#7 lore raw

PATCH v3 1/1 hwmon: (pmbus) Use 64bit math for DIRECT format values

In reply to 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 <[email protected]>
---
 drivers/hwmon/pmbus/pmbus_core.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
drivers/hwmon/pmbus/pmbus_core.c +12 -9
index 52a58b8b6e1b..09977440bbea 100644
@@ -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,
Signature · 1 line
2.15.0.417.g466bffb3ac-goog
maintainer
#8 lore raw
Quoted 7 lines · 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 <[email protected]>

Applied (silently fixed two checkpatch CHECK messages)

Guenter

Quoted 75 lines
---
 drivers/hwmon/pmbus/pmbus_core.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
drivers/hwmon/pmbus/pmbus_core.c +12 -9
index 52a58b8b6e1b..09977440bbea 100644
@@ -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,