Commit 615f4e84 authored by Uwe Kleine-König's avatar Uwe Kleine-König Committed by Thierry Reding

pwm: renesas-tpu: Improve precision of period and duty_cycle calculation

Dividing by the result of a division looses precision. Consider for example
clk_rate = 33000000 and period_ns = 500001. Then

	clk_rate / (NSEC_PER_SEC / period_ns)

has the exact value 16500.033, but in C this evaluates to 16508. It gets
worse for even bigger values of period_ns, so with period_ns = 500000001,
the exact result is 16500000.033 while in C we get 33000000.

For that reason use

	clk_rate * period_ns / NSEC_PER_SEC

instead which doesn't suffer from this problem. To ensure this doesn't
overflow add a safeguard check for clk_rate.

Note that duty > period can never happen, so the respective check can be
dropped.

Incidentally this fixes a division by zero if period_ns > NSEC_PER_SEC.
Another side effect is that values bigger than INT_MAX for period and
duty_cyle are not wrongly discarded any more.

Fixes: 99b82abb ("pwm: Add Renesas TPU PWM driver")
Signed-off-by: default avatarUwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reviewed-by: default avatarGeert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: default avatarThierry Reding <thierry.reding@gmail.com>
parent 3c173376
...@@ -242,20 +242,29 @@ static void tpu_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) ...@@ -242,20 +242,29 @@ static void tpu_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
} }
static int tpu_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, static int tpu_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
int duty_ns, int period_ns, bool enabled) u64 duty_ns, u64 period_ns, bool enabled)
{ {
struct tpu_pwm_device *tpd = pwm_get_chip_data(pwm); struct tpu_pwm_device *tpd = pwm_get_chip_data(pwm);
struct tpu_device *tpu = to_tpu_device(chip); struct tpu_device *tpu = to_tpu_device(chip);
unsigned int prescaler; unsigned int prescaler;
bool duty_only = false; bool duty_only = false;
u32 clk_rate; u32 clk_rate;
u32 period; u64 period;
u32 duty; u32 duty;
int ret; int ret;
clk_rate = clk_get_rate(tpu->clk); clk_rate = clk_get_rate(tpu->clk);
if (unlikely(clk_rate > NSEC_PER_SEC)) {
/*
* This won't happen in the nearer future, so this is only a
* safeguard to prevent the following calculation from
* overflowing. With this clk_rate * period_ns / NSEC_PER_SEC is
* not greater than period_ns and so fits into an u64.
*/
return -EINVAL;
}
period = clk_rate / (NSEC_PER_SEC / period_ns); period = mul_u64_u64_div_u64(clk_rate, period_ns, NSEC_PER_SEC);
/* /*
* Find the minimal prescaler in [0..3] such that * Find the minimal prescaler in [0..3] such that
...@@ -292,18 +301,15 @@ static int tpu_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, ...@@ -292,18 +301,15 @@ static int tpu_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
period >>= 2 * prescaler; period >>= 2 * prescaler;
if (duty_ns) { if (duty_ns)
duty = (clk_rate >> 2 * prescaler) duty = mul_u64_u64_div_u64(clk_rate, duty_ns,
/ (NSEC_PER_SEC / duty_ns); (u64)NSEC_PER_SEC << (2 * prescaler));
if (duty > period) else
return -EINVAL;
} else {
duty = 0; duty = 0;
}
dev_dbg(&tpu->pdev->dev, dev_dbg(&tpu->pdev->dev,
"rate %u, prescaler %u, period %u, duty %u\n", "rate %u, prescaler %u, period %u, duty %u\n",
clk_rate, 1 << (2 * prescaler), period, duty); clk_rate, 1 << (2 * prescaler), (u32)period, duty);
if (tpd->prescaler == prescaler && tpd->period == period) if (tpd->prescaler == prescaler && tpd->period == period)
duty_only = true; duty_only = true;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment