pwm: mediatek: Fix various issues in the .apply() callback

duty_cycle and period were silently cast from u64 to int losing
relevant bits. Dividing by the result of a division (resolution) looses
precision. clkdiv was determined using a loop while it can be done
without one. Also too low period values were not catched.

Improve all these issues. Handling period and duty_cycle being u64 now
requires a bit more care to prevent overflows, so mul_u64_u64_div_u64()
is used.

The changes implemented in this change also align the chosen hardware
settings to match the usual PWM rules (i.e. round down instead round
nearest) and so .apply() also matches .get_state() silencing several
warnings with PWM_DEBUG=y. While this probably doesn't result in
problems, this aspect makes this change---though it might be considered
a fix---unsuitable for backporting.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Link: https://lore.kernel.org/r/20250725154506.2610172-16-u.kleine-koenig@baylibre.com
Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org>

diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
index 2a5323e9fa50c8824194ec4ebf1ebddcba47f3c3..434ddc57f5dc3e338e88c2671ed22b85e6a91383 100644 (file)
--- a/drivers/pwm/pwm-mediatek.c
+++ b/drivers/pwm/pwm-mediatek.c
@@ -137,13 +137,13 @@ static void pwm_mediatek_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 }
 
 static int pwm_mediatek_config(struct pwm_chip *chip, struct pwm_device *pwm,
-                              int duty_ns, int period_ns)
+                              u64 duty_ns, u64 period_ns)
 {
        struct pwm_mediatek_chip *pc = to_pwm_mediatek_chip(chip);
-       u32 clkdiv = 0, cnt_period, cnt_duty, reg_width = PWMDWIDTH,
-           reg_thres = PWMTHRES;
+       u32 clkdiv, enable;
+       u32 reg_width = PWMDWIDTH, reg_thres = PWMTHRES;
+       u64 cnt_period, cnt_duty;
        unsigned long clk_rate;
-       u64 resolution;
        int ret;
 
        ret = pwm_mediatek_clk_enable(pc, pwm->hwpwm);
@@ -151,7 +151,11 @@ static int pwm_mediatek_config(struct pwm_chip *chip, struct pwm_device *pwm,
                return ret;
 
        clk_rate = clk_get_rate(pc->clk_pwms[pwm->hwpwm]);
-       if (!clk_rate) {
+       /*
+        * With the clk running with not more than 1 GHz the calculations below
+        * won't overflow
+        */
+       if (!clk_rate || clk_rate > 1000000000) {
                ret = -EINVAL;
                goto out;
        }
@@ -160,27 +164,40 @@ static int pwm_mediatek_config(struct pwm_chip *chip, struct pwm_device *pwm,
        if (pc->soc->pwm_ck_26m_sel_reg)
                writel(0, pc->regs + pc->soc->pwm_ck_26m_sel_reg);
 
-       /* Using resolution in picosecond gets accuracy higher */
-       resolution = (u64)NSEC_PER_SEC * 1000;
-       do_div(resolution, clk_rate);
-
-       cnt_period = DIV_ROUND_CLOSEST_ULL((u64)period_ns * 1000, resolution);
-       if (!cnt_period)
-               return -EINVAL;
+       cnt_period = mul_u64_u64_div_u64(period_ns, clk_rate, NSEC_PER_SEC);
+       if (cnt_period == 0) {
+               ret = -ERANGE;
+               goto out;
+       }
 
-       while (cnt_period - 1 > FIELD_MAX(PWMDWIDTH_PERIOD)) {
-               resolution *= 2;
-               clkdiv++;
-               cnt_period = DIV_ROUND_CLOSEST_ULL((u64)period_ns * 1000,
-                                                  resolution);
+       if (cnt_period > FIELD_MAX(PWMDWIDTH_PERIOD) + 1) {
+               if (cnt_period >= ((FIELD_MAX(PWMDWIDTH_PERIOD) + 1) << FIELD_MAX(PWMCON_CLKDIV))) {
+                       clkdiv = FIELD_MAX(PWMCON_CLKDIV);
+                       cnt_period = FIELD_MAX(PWMDWIDTH_PERIOD) + 1;
+               } else {
+                       clkdiv = ilog2(cnt_period) - ilog2(FIELD_MAX(PWMDWIDTH_PERIOD));
+                       cnt_period >>= clkdiv;
+               }
+       } else {
+               clkdiv = 0;
        }
 
-       if (clkdiv > FIELD_MAX(PWMCON_CLKDIV)) {
-               dev_err(pwmchip_parent(chip), "period of %d ns not supported\n", period_ns);
-               ret = -EINVAL;
-               goto out;
+       cnt_duty = mul_u64_u64_div_u64(duty_ns, clk_rate, NSEC_PER_SEC) >> clkdiv;
+       if (cnt_duty > cnt_period)
+               cnt_duty = cnt_period;
+
+       if (cnt_duty) {
+               cnt_duty -= 1;
+               enable = BIT(pwm->hwpwm);
+       } else {
+               enable = 0;
        }
 
+       cnt_period -= 1;
+
+       dev_dbg(&chip->dev, "pwm#%u: %lld/%lld @%lu -> CON: %x, PERIOD: %llx, DUTY: %llx\n",
+               pwm->hwpwm, duty_ns, period_ns, clk_rate, clkdiv, cnt_period, cnt_duty);
+
        if (pc->soc->pwm45_fixup && pwm->hwpwm > 2) {
                /*
                 * PWM[4,5] has distinct offset for PWMDWIDTH and PWMTHRES
@@ -190,13 +207,11 @@ static int pwm_mediatek_config(struct pwm_chip *chip, struct pwm_device *pwm,
                reg_thres = PWM45THRES_FIXUP;
        }
 
-       cnt_duty = DIV_ROUND_CLOSEST_ULL((u64)duty_ns * 1000, resolution);
-
        pwm_mediatek_writel(pc, pwm->hwpwm, PWMCON, BIT(15) | clkdiv);
-       pwm_mediatek_writel(pc, pwm->hwpwm, reg_width, cnt_period - 1);
+       pwm_mediatek_writel(pc, pwm->hwpwm, reg_width, cnt_period);
 
-       if (cnt_duty) {
-               pwm_mediatek_writel(pc, pwm->hwpwm, reg_thres, cnt_duty - 1);
+       if (enable) {
+               pwm_mediatek_writel(pc, pwm->hwpwm, reg_thres, cnt_duty);
                pwm_mediatek_enable(chip, pwm);
        } else {
                pwm_mediatek_disable(chip, pwm);