pwm: bcm2835: Improve period and duty cycle calculation

With an input clk rate bigger than 2000000000, scaler would have been
zero which then would have resulted in a division by zero.

Also the originally implemented algorithm divided by the result of a
division. This nearly always looses precision. Consider a requested period
of 1000000 ns. With an input clock frequency of 32786885 Hz the hardware
was configured with an actual period of 983869.007 ns (PERIOD = 32258)
while the hardware can provide 1000003.508 ns (PERIOD = 32787).
And note if the input clock frequency was 32786886 Hz instead, the hardware
was configured to 1016656.477 ns (PERIOD = 33333) while the optimal
setting results in 1000003.477 ns (PERIOD = 32787).

This patch implements proper range checking and only divides once for
the calculation of period (and similar for duty_cycle).

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reviewed-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Tested-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>

diff --git a/drivers/pwm/pwm-bcm2835.c b/drivers/pwm/pwm-bcm2835.c
index 6ff5f04b3e07bfb338b1e1419dab58ecb489f718..d593cce249d94793ba1b0a170e64b2424c2b534d 100644 (file)
--- a/drivers/pwm/pwm-bcm2835.c
+++ b/drivers/pwm/pwm-bcm2835.c
@@ -64,8 +64,9 @@ static int bcm2835_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 
        struct bcm2835_pwm *pc = to_bcm2835_pwm(chip);
        unsigned long rate = clk_get_rate(pc->clk);
-       unsigned long long period;
-       unsigned long scaler;
+       unsigned long long period_cycles;
+       u64 max_period;
+
        u32 val;
 
        if (!rate) {
@@ -73,18 +74,36 @@ static int bcm2835_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
                return -EINVAL;
        }
 
-       scaler = DIV_ROUND_CLOSEST(NSEC_PER_SEC, rate);
+       /*
+        * period_cycles must be a 32 bit value, so period * rate / NSEC_PER_SEC
+        * must be <= U32_MAX. As U32_MAX * NSEC_PER_SEC < U64_MAX the
+        * multiplication period * rate doesn't overflow.
+        * To calculate the maximal possible period that guarantees the
+        * above inequality:
+        *
+        *     round(period * rate / NSEC_PER_SEC) <= U32_MAX
+        * <=> period * rate / NSEC_PER_SEC < U32_MAX + 0.5
+        * <=> period * rate < (U32_MAX + 0.5) * NSEC_PER_SEC
+        * <=> period < ((U32_MAX + 0.5) * NSEC_PER_SEC) / rate
+        * <=> period < ((U32_MAX * NSEC_PER_SEC + NSEC_PER_SEC/2) / rate
+        * <=> period <= ceil((U32_MAX * NSEC_PER_SEC + NSEC_PER_SEC/2) / rate) - 1
+        */
+       max_period = DIV_ROUND_UP_ULL((u64)U32_MAX * NSEC_PER_SEC + NSEC_PER_SEC / 2, rate) - 1;
+
+       if (state->period > max_period)
+               return -EINVAL;
+
        /* set period */
-       period = DIV_ROUND_CLOSEST_ULL(state->period, scaler);
+       period_cycles = DIV_ROUND_CLOSEST_ULL(state->period * rate, NSEC_PER_SEC);
 
-       /* dont accept a period that is too small or has been truncated */
-       if ((period < PERIOD_MIN) || (period > U32_MAX))
+       /* don't accept a period that is too small */
+       if (period_cycles < PERIOD_MIN)
                return -EINVAL;
 
-       writel(period, pc->base + PERIOD(pwm->hwpwm));
+       writel(period_cycles, pc->base + PERIOD(pwm->hwpwm));
 
        /* set duty cycle */
-       val = DIV_ROUND_CLOSEST_ULL(state->duty_cycle, scaler);
+       val = DIV_ROUND_CLOSEST_ULL(state->duty_cycle * rate, NSEC_PER_SEC);
        writel(val, pc->base + DUTY(pwm->hwpwm));
 
        /* set polarity */