| Message ID | 20201110175434.2591741-1-pbarker@konsulko.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4] hwmon: pwm-fan: Fix RPM calculation | expand |
On Tue, Nov 10, 2020 at 05:54:34PM +0000, Paul Barker wrote: > To convert the number of pulses counted into an RPM estimation, we need > to divide by the width of our measurement interval instead of > multiplying by it. If the width of the measurement interval is zero we > don't update the RPM value to avoid dividing by zero. > > We also don't need to do 64-bit division, with 32-bits we can handle a > fan running at over 4 million RPM. > > Signed-off-by: Paul Barker <pbarker@konsulko.com> > --- > > Changes from v2: > > * Don't update the RPM value if delta=0. > > drivers/hwmon/pwm-fan.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c > index bdba2143021a..d1fd50c32514 100644 > --- a/drivers/hwmon/pwm-fan.c > +++ b/drivers/hwmon/pwm-fan.c > @@ -54,14 +54,15 @@ static irqreturn_t pulse_handler(int irq, void *dev_id) > static void sample_timer(struct timer_list *t) > { > struct pwm_fan_ctx *ctx = from_timer(ctx, t, rpm_timer); > + unsigned int delta = ktime_ms_delta(ktime_get(), ctx->sample_start); > int pulses; > - u64 tmp; > > - pulses = atomic_read(&ctx->pulses); > - atomic_sub(pulses, &ctx->pulses); > - tmp = (u64)pulses * ktime_ms_delta(ktime_get(), ctx->sample_start) * 60; > - do_div(tmp, ctx->pulses_per_revolution * 1000); > - ctx->rpm = tmp; > + if (delta) { > + pulses = atomic_read(&ctx->pulses); > + atomic_sub(pulses, &ctx->pulses); > + ctx->rpm = (unsigned int)(pulses * 1000 * 60) / > + (ctx->pulses_per_revolution * delta); > + } > > ctx->sample_start = ktime_get(); I know I am a bit nitpicking here, but should ctx->sample_start only be updated if delta != 0 ? We don't reset pulses if delta == 0, meaning the next reading will be based on the old start time (even if the difference was less than 1 ms). Thanks, Guenter > mod_timer(&ctx->rpm_timer, jiffies + HZ); > > base-commit: f8394f232b1eab649ce2df5c5f15b0e528c92091 > -- > 2.29.2 >
diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c index bdba2143021a..d1fd50c32514 100644 --- a/drivers/hwmon/pwm-fan.c +++ b/drivers/hwmon/pwm-fan.c @@ -54,14 +54,15 @@ static irqreturn_t pulse_handler(int irq, void *dev_id) static void sample_timer(struct timer_list *t) { struct pwm_fan_ctx *ctx = from_timer(ctx, t, rpm_timer); + unsigned int delta = ktime_ms_delta(ktime_get(), ctx->sample_start); int pulses; - u64 tmp; - pulses = atomic_read(&ctx->pulses); - atomic_sub(pulses, &ctx->pulses); - tmp = (u64)pulses * ktime_ms_delta(ktime_get(), ctx->sample_start) * 60; - do_div(tmp, ctx->pulses_per_revolution * 1000); - ctx->rpm = tmp; + if (delta) { + pulses = atomic_read(&ctx->pulses); + atomic_sub(pulses, &ctx->pulses); + ctx->rpm = (unsigned int)(pulses * 1000 * 60) / + (ctx->pulses_per_revolution * delta); + } ctx->sample_start = ktime_get(); mod_timer(&ctx->rpm_timer, jiffies + HZ);
To convert the number of pulses counted into an RPM estimation, we need to divide by the width of our measurement interval instead of multiplying by it. If the width of the measurement interval is zero we don't update the RPM value to avoid dividing by zero. We also don't need to do 64-bit division, with 32-bits we can handle a fan running at over 4 million RPM. Signed-off-by: Paul Barker <pbarker@konsulko.com> --- Changes from v2: * Don't update the RPM value if delta=0. drivers/hwmon/pwm-fan.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) base-commit: f8394f232b1eab649ce2df5c5f15b0e528c92091