From: Guenter Roeck <linux@roeck-us.net>
To: Josh Lehan <krellan@google.com>
Cc: linux-hwmon@vger.kernel.org, jdelvare@suse.com
Subject: Re: [PATCH] hwmon: (pmbus/core) Use s64 instead of long for calculations
Date: Thu, 25 Jun 2020 18:06:47 -0700 [thread overview]
Message-ID: <20200626010647.GA191227@roeck-us.net> (raw)
In-Reply-To: <20200626001343.162071-1-krellan@google.com>
On Thu, Jun 25, 2020 at 05:13:43PM -0700, Josh Lehan wrote:
> Using s64 type, instead of long type,
> for internal calculations and the sysfs interface.
>
> This allows 64-bit values to appear correctly on 32-bit kernels.
> As wattage is reported in microwatts,
> monitoring a power supply over 2KW requires this.
>
> Although it may seem unlikely to run a 32-bit kernel on such
> a large machine, enterprise servers often include a BMC,
> and the BMC might be running a 32-bit kernel....
>
> Effort: semitruck
> Google-Bug-Id: 159769574
> Signed-off-by: Josh Lehan <krellan@google.com>
> Change-Id: If89882e20387454189b2ebe20c793d4cf166dfa8
Applied to hwmon-next after removing irrelevant tags.
Thanks,
Guenter
> ---
> drivers/hwmon/pmbus/pmbus_core.c | 66 +++++++++++++++-----------------
> 1 file changed, 31 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 51eb6d56d29c..52bf71ef5ac4 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -646,12 +646,12 @@ static struct pmbus_data *pmbus_update_device(struct device *dev)
> * Convert linear sensor values to milli- or micro-units
> * depending on sensor type.
> */
> -static long pmbus_reg2data_linear(struct pmbus_data *data,
> - struct pmbus_sensor *sensor)
> +static s64 pmbus_reg2data_linear(struct pmbus_data *data,
> + struct pmbus_sensor *sensor)
> {
> s16 exponent;
> s32 mantissa;
> - long val;
> + s64 val;
>
> if (sensor->class == PSC_VOLTAGE_OUT) { /* LINEAR16 */
> exponent = data->exponent[sensor->page];
> @@ -665,11 +665,11 @@ static long pmbus_reg2data_linear(struct pmbus_data *data,
>
> /* scale result to milli-units for all sensors except fans */
> if (sensor->class != PSC_FAN)
> - val = val * 1000L;
> + val = val * 1000LL;
>
> /* scale result to micro-units for power sensors */
> if (sensor->class == PSC_POWER)
> - val = val * 1000L;
> + val = val * 1000LL;
>
> if (exponent >= 0)
> val <<= exponent;
> @@ -683,8 +683,8 @@ static long pmbus_reg2data_linear(struct pmbus_data *data,
> * Convert direct sensor values to milli- or micro-units
> * depending on sensor type.
> */
> -static long pmbus_reg2data_direct(struct pmbus_data *data,
> - struct pmbus_sensor *sensor)
> +static s64 pmbus_reg2data_direct(struct pmbus_data *data,
> + struct pmbus_sensor *sensor)
> {
> s64 b, val = (s16)sensor->data;
> s32 m, R;
> @@ -720,15 +720,15 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,
> }
>
> val = div_s64(val - b, m);
> - return clamp_val(val, LONG_MIN, LONG_MAX);
> + return val;
> }
>
> /*
> * Convert VID sensor values to milli- or micro-units
> * depending on sensor type.
> */
> -static long pmbus_reg2data_vid(struct pmbus_data *data,
> - struct pmbus_sensor *sensor)
> +static s64 pmbus_reg2data_vid(struct pmbus_data *data,
> + struct pmbus_sensor *sensor)
> {
> long val = sensor->data;
> long rv = 0;
> @@ -750,9 +750,9 @@ static long pmbus_reg2data_vid(struct pmbus_data *data,
> return rv;
> }
>
> -static long pmbus_reg2data(struct pmbus_data *data, struct pmbus_sensor *sensor)
> +static s64 pmbus_reg2data(struct pmbus_data *data, struct pmbus_sensor *sensor)
> {
> - long val;
> + s64 val;
>
> if (!sensor->convert)
> return sensor->data;
> @@ -776,7 +776,7 @@ static long pmbus_reg2data(struct pmbus_data *data, struct pmbus_sensor *sensor)
> #define MIN_MANTISSA (511 * 1000)
>
> static u16 pmbus_data2reg_linear(struct pmbus_data *data,
> - struct pmbus_sensor *sensor, long val)
> + struct pmbus_sensor *sensor, s64 val)
> {
> s16 exponent = 0, mantissa;
> bool negative = false;
> @@ -798,8 +798,8 @@ static u16 pmbus_data2reg_linear(struct pmbus_data *data,
> val <<= -data->exponent[sensor->page];
> else
> val >>= data->exponent[sensor->page];
> - val = DIV_ROUND_CLOSEST(val, 1000);
> - return val & 0xffff;
> + val = DIV_ROUND_CLOSEST_ULL(val, 1000);
> + return clamp_val(val, 0, 0xffff);
> }
>
> if (val < 0) {
> @@ -809,14 +809,14 @@ static u16 pmbus_data2reg_linear(struct pmbus_data *data,
>
> /* Power is in uW. Convert to mW before converting. */
> if (sensor->class == PSC_POWER)
> - val = DIV_ROUND_CLOSEST(val, 1000L);
> + val = DIV_ROUND_CLOSEST_ULL(val, 1000);
>
> /*
> * For simplicity, convert fan data to milli-units
> * before calculating the exponent.
> */
> if (sensor->class == PSC_FAN)
> - val = val * 1000;
> + val = val * 1000LL;
>
> /* Reduce large mantissa until it fits into 10 bit */
> while (val >= MAX_MANTISSA && exponent < 15) {
> @@ -830,11 +830,7 @@ static u16 pmbus_data2reg_linear(struct pmbus_data *data,
> }
>
> /* Convert mantissa from milli-units to units */
> - mantissa = DIV_ROUND_CLOSEST(val, 1000);
> -
> - /* Ensure that resulting number is within range */
> - if (mantissa > 0x3ff)
> - mantissa = 0x3ff;
> + mantissa = clamp_val(DIV_ROUND_CLOSEST_ULL(val, 1000), 0, 0x3ff);
>
> /* restore sign */
> if (negative)
> @@ -845,9 +841,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)
> + struct pmbus_sensor *sensor, s64 val)
> {
> - s64 b, val64 = val;
> + s64 b;
> s32 m, R;
>
> m = data->info->m[sensor->class];
> @@ -865,30 +861,30 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data,
> R -= 3; /* Adjust R and b for data in milli-units */
> b *= 1000;
> }
> - val64 = val64 * m + b;
> + val = val * m + b;
>
> while (R > 0) {
> - val64 *= 10;
> + val *= 10;
> R--;
> }
> while (R < 0) {
> - val64 = div_s64(val64 + 5LL, 10L); /* round closest */
> + val = div_s64(val + 5LL, 10L); /* round closest */
> R++;
> }
>
> - return (u16)clamp_val(val64, S16_MIN, S16_MAX);
> + return (u16)clamp_val(val, S16_MIN, S16_MAX);
> }
>
> static u16 pmbus_data2reg_vid(struct pmbus_data *data,
> - struct pmbus_sensor *sensor, long val)
> + struct pmbus_sensor *sensor, s64 val)
> {
> val = clamp_val(val, 500, 1600);
>
> - return 2 + DIV_ROUND_CLOSEST((1600 - val) * 100, 625);
> + return 2 + DIV_ROUND_CLOSEST_ULL((1600LL - val) * 100LL, 625);
> }
>
> static u16 pmbus_data2reg(struct pmbus_data *data,
> - struct pmbus_sensor *sensor, long val)
> + struct pmbus_sensor *sensor, s64 val)
> {
> u16 regval;
>
> @@ -954,7 +950,7 @@ static int pmbus_get_boolean(struct pmbus_data *data, struct pmbus_boolean *b,
> WARN(1, "Bad boolean descriptor %p: s1=%p, s2=%p\n", b, s1, s2);
> return 0;
> } else {
> - long v1, v2;
> + s64 v1, v2;
>
> if (s1->data < 0)
> return s1->data;
> @@ -991,7 +987,7 @@ static ssize_t pmbus_show_sensor(struct device *dev,
> if (sensor->data < 0)
> return sensor->data;
>
> - return snprintf(buf, PAGE_SIZE, "%ld\n", pmbus_reg2data(data, sensor));
> + return snprintf(buf, PAGE_SIZE, "%lld\n", pmbus_reg2data(data, sensor));
> }
>
> static ssize_t pmbus_set_sensor(struct device *dev,
> @@ -1002,11 +998,11 @@ static ssize_t pmbus_set_sensor(struct device *dev,
> struct pmbus_data *data = i2c_get_clientdata(client);
> struct pmbus_sensor *sensor = to_pmbus_sensor(devattr);
> ssize_t rv = count;
> - long val = 0;
> + s64 val;
> int ret;
> u16 regval;
>
> - if (kstrtol(buf, 10, &val) < 0)
> + if (kstrtos64(buf, 10, &val) < 0)
> return -EINVAL;
>
> mutex_lock(&data->update_lock);
> --
> 2.27.0.212.ge8ba1cc988-goog
>
prev parent reply other threads:[~2020-06-26 1:06 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-26 0:13 [PATCH] hwmon: (pmbus/core) Use s64 instead of long for calculations Josh Lehan
2020-06-26 1:06 ` Guenter Roeck [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200626010647.GA191227@roeck-us.net \
--to=linux@roeck-us.net \
--cc=jdelvare@suse.com \
--cc=krellan@google.com \
--cc=linux-hwmon@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox