From: Uwe Kleine-König Date: Fri, 25 Jul 2025 15:45:10 +0000 (+0200) Subject: pwm: mediatek: Fix various issues in the .apply() callback X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=849b064c16977202298ac411f80c83ea047fe466;p=thirdparty%2Fkernel%2Fstable.git 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 Reviewed-by: AngeloGioacchino Del Regno Link: https://lore.kernel.org/r/20250725154506.2610172-16-u.kleine-koenig@baylibre.com Signed-off-by: Uwe Kleine-König --- diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c index 2a5323e9fa50c..434ddc57f5dc3 100644 --- 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);