pwm: tegra: Optimize period calculation
authorUwe Kleine-König <u.kleine-koenig@pengutronix.de>
Mon, 25 Apr 2022 13:22:44 +0000 (15:22 +0200)
committerThierry Reding <thierry.reding@gmail.com>
Fri, 20 May 2022 14:20:07 +0000 (16:20 +0200)
Dividing by the result of a division looses precision because the result is
rounded twice. E.g. with clk_rate = 48000000 and period = 32760033 the
following numbers result:

rate = pc->clk_rate >> PWM_DUTY_WIDTH = 187500
hz = DIV_ROUND_CLOSEST_ULL(100ULL * NSEC_PER_SEC, period_ns) = 3052
rate = DIV_ROUND_CLOSEST_ULL(100ULL * rate, hz) = 6144

The exact result would be 6142.5061875 and (apart from rounding) this is
found by using a single division. As a side effect is also a tad
cheaper to calculate.

Also using clk_rate >> PWM_DUTY_WIDTH looses precision. Consider for
example clk_rate = 47999999 and period = 106667:

mul_u64_u64_div_u64(pc->clk_rate >> PWM_DUTY_WIDTH, period_ns,
    NSEC_PER_SEC) = 19

mul_u64_u64_div_u64(pc->clk_rate, period_ns,
    NSEC_PER_SEC << PWM_DUTY_WIDTH) = 20

(The exact result is 20.000062083332033.)

With this optimizations also switch from round-closest to round-down for
the period calculation. Given that the calculations were non-optimal for
quite some time now with variations in both directions which nobody
reported as a problem, this is the opportunity to align the driver's
behavior to the requirements of new drivers. This has several upsides:

 - Implementation is easier as there are no round-nearest variants of
   mul_u64_u64_div_u64().
 - Requests for too small periods are now consistently refused. This was
   kind of arbitrary before, where period_ns < min_period_ns was
   refused, but in some cases min_period_ns isn't actually implementable
   and then values between min_period_ns and the actual minimum were
   rounded up to the actual minimum.

Note that the duty_cycle calculation isn't using the usual round-down
approach yet.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
drivers/pwm/pwm-tegra.c

index e5a9ffe..7fc03a9 100644 (file)
@@ -99,7 +99,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
                            int duty_ns, int period_ns)
 {
        struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
-       unsigned long long c = duty_ns, hz;
+       unsigned long long c = duty_ns;
        unsigned long rate, required_clk_rate;
        u32 val = 0;
        int err;
@@ -156,11 +156,9 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
                pc->clk_rate = clk_get_rate(pc->clk);
        }
 
-       rate = pc->clk_rate >> PWM_DUTY_WIDTH;
-
        /* Consider precision in PWM_SCALE_WIDTH rate calculation */
-       hz = DIV_ROUND_CLOSEST_ULL(100ULL * NSEC_PER_SEC, period_ns);
-       rate = DIV_ROUND_CLOSEST_ULL(100ULL * rate, hz);
+       rate = mul_u64_u64_div_u64(pc->clk_rate, period_ns,
+                                  (u64)NSEC_PER_SEC << PWM_DUTY_WIDTH);
 
        /*
         * Since the actual PWM divider is the register's frequency divider
@@ -169,6 +167,8 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
         */
        if (rate > 0)
                rate--;
+       else
+               return -EINVAL;
 
        /*
         * Make sure that the rate will fit in the register's frequency