diff options
author | Uwe Kleine-König <u.kleine-koenig@pengutronix.de> | 2022-04-20 15:12:40 +0300 |
---|---|---|
committer | Thierry Reding <thierry.reding@gmail.com> | 2022-05-20 17:18:40 +0300 |
commit | 615f4e84461b71e5fed01d9f6d9d98ef3dd1d452 (patch) | |
tree | d66b85a1e12e2f88f2e26106c1a21b742c98d799 | |
parent | 3c173376efc461dc670b02ba2846c2a533491104 (diff) | |
download | linux-615f4e84461b71e5fed01d9f6d9d98ef3dd1d452.tar.xz |
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: 99b82abb0a35 ("pwm: Add Renesas TPU PWM driver")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
-rw-r--r-- | drivers/pwm/pwm-renesas-tpu.c | 28 |
1 files changed, 17 insertions, 11 deletions
diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c index 4aff3870010c..d7311614c846 100644 --- a/drivers/pwm/pwm-renesas-tpu.c +++ b/drivers/pwm/pwm-renesas-tpu.c @@ -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, - 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_device *tpu = to_tpu_device(chip); unsigned int prescaler; bool duty_only = false; u32 clk_rate; - u32 period; + u64 period; u32 duty; int ret; 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 @@ -292,18 +301,15 @@ static int tpu_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, period >>= 2 * prescaler; - if (duty_ns) { - duty = (clk_rate >> 2 * prescaler) - / (NSEC_PER_SEC / duty_ns); - if (duty > period) - return -EINVAL; - } else { + if (duty_ns) + duty = mul_u64_u64_div_u64(clk_rate, duty_ns, + (u64)NSEC_PER_SEC << (2 * prescaler)); + else duty = 0; - } dev_dbg(&tpu->pdev->dev, "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) duty_only = true; |