On 5/8/19 11:32 AM, Guenter Roeck wrote: > Hi Florian, > > On Wed, May 08, 2019 at 10:00:35AM -0700, Florian Fainelli wrote: >> If the SCMI firmware implementation is reporting values in a scale that >> is different from the HWMON units, we need to scale up or down the value >> according to how far appart they are. >> >> Signed-off-by: Florian Fainelli <[email protected]> >> --- >> drivers/hwmon/scmi-hwmon.c | 46 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 46 insertions(+) >> >> diff --git a/drivers/hwmon/scmi-hwmon.c b/drivers/hwmon/scmi-hwmon.c >> index a80183a488c5..4399372e2131 100644 >> --- a/drivers/hwmon/scmi-hwmon.c >> +++ b/drivers/hwmon/scmi-hwmon.c >> @@ -7,6 +7,7 @@ >> */ >> >> #include <linux/hwmon.h> >> +#include <linux/limits.h> >> #include <linux/module.h> >> #include <linux/scmi_protocol.h> >> #include <linux/slab.h> >> @@ -18,6 +19,47 @@ struct scmi_sensors { >> const struct scmi_sensor_info **info[hwmon_max]; >> }; >> >> +static inline u64 __pow10(u8 x) >> +{ >> + u64 r = 1; >> + >> + while (x--) >> + r *= 10; >> + >> + return r; >> +} >> + >> +static int scmi_hwmon_scale(const struct scmi_sensor_info *sensor, u64 >> *value) >> +{ >> + s8 scale = sensor->scale; >> + u64 f; >> + >> + switch (sensor->type) { >> + case TEMPERATURE_C: >> + case VOLTAGE: >> + case CURRENT: >> + scale += 3; >> + break; >> + case POWER: >> + case ENERGY: >> + scale += 6; >> + break; >> + default: >> + break; >> + } >> + >> + f = __pow10(abs(scale)); >> + if (f == U64_MAX) >> + return -E2BIG; > > Unfortunately that is not how integer overflows work. > > A test program with increasing values of scale reports: > > 0: 1 > ... > 18: 1000000000000000000 > 19: 10000000000000000000 > 20: 7766279631452241920 > 21: 3875820019684212736 > 22: 1864712049423024128 > 23: 200376420520689664 > 24: 2003764205206896640 > ... > 61: 11529215046068469760 > 62: 4611686018427387904 > 63: 9223372036854775808 > 64: 0 > ... > > You'll have to check for abs(scale) > 19 if you want to report overflows.
Yes silly me, my test program was flawed, thanks for pointing out that. You are okay with returning E2BIG when we overflow? -- Florian
