tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

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>

authored by

Uwe Kleine-König and committed by
Uwe Kleine-König 849b064c edd6a37e

+43 -28
+43 -28
drivers/pwm/pwm-mediatek.c
··· 137 137 } 138 138 139 139 static int pwm_mediatek_config(struct pwm_chip *chip, struct pwm_device *pwm, 140 - int duty_ns, int period_ns) 140 + u64 duty_ns, u64 period_ns) 141 141 { 142 142 struct pwm_mediatek_chip *pc = to_pwm_mediatek_chip(chip); 143 - u32 clkdiv = 0, cnt_period, cnt_duty, reg_width = PWMDWIDTH, 144 - reg_thres = PWMTHRES; 143 + u32 clkdiv, enable; 144 + u32 reg_width = PWMDWIDTH, reg_thres = PWMTHRES; 145 + u64 cnt_period, cnt_duty; 145 146 unsigned long clk_rate; 146 - u64 resolution; 147 147 int ret; 148 148 149 149 ret = pwm_mediatek_clk_enable(pc, pwm->hwpwm); ··· 151 151 return ret; 152 152 153 153 clk_rate = clk_get_rate(pc->clk_pwms[pwm->hwpwm]); 154 - if (!clk_rate) { 154 + /* 155 + * With the clk running with not more than 1 GHz the calculations below 156 + * won't overflow 157 + */ 158 + if (!clk_rate || clk_rate > 1000000000) { 155 159 ret = -EINVAL; 156 160 goto out; 157 161 } ··· 164 160 if (pc->soc->pwm_ck_26m_sel_reg) 165 161 writel(0, pc->regs + pc->soc->pwm_ck_26m_sel_reg); 166 162 167 - /* Using resolution in picosecond gets accuracy higher */ 168 - resolution = (u64)NSEC_PER_SEC * 1000; 169 - do_div(resolution, clk_rate); 170 - 171 - cnt_period = DIV_ROUND_CLOSEST_ULL((u64)period_ns * 1000, resolution); 172 - if (!cnt_period) 173 - return -EINVAL; 174 - 175 - while (cnt_period - 1 > FIELD_MAX(PWMDWIDTH_PERIOD)) { 176 - resolution *= 2; 177 - clkdiv++; 178 - cnt_period = DIV_ROUND_CLOSEST_ULL((u64)period_ns * 1000, 179 - resolution); 180 - } 181 - 182 - if (clkdiv > FIELD_MAX(PWMCON_CLKDIV)) { 183 - dev_err(pwmchip_parent(chip), "period of %d ns not supported\n", period_ns); 184 - ret = -EINVAL; 163 + cnt_period = mul_u64_u64_div_u64(period_ns, clk_rate, NSEC_PER_SEC); 164 + if (cnt_period == 0) { 165 + ret = -ERANGE; 185 166 goto out; 186 167 } 168 + 169 + if (cnt_period > FIELD_MAX(PWMDWIDTH_PERIOD) + 1) { 170 + if (cnt_period >= ((FIELD_MAX(PWMDWIDTH_PERIOD) + 1) << FIELD_MAX(PWMCON_CLKDIV))) { 171 + clkdiv = FIELD_MAX(PWMCON_CLKDIV); 172 + cnt_period = FIELD_MAX(PWMDWIDTH_PERIOD) + 1; 173 + } else { 174 + clkdiv = ilog2(cnt_period) - ilog2(FIELD_MAX(PWMDWIDTH_PERIOD)); 175 + cnt_period >>= clkdiv; 176 + } 177 + } else { 178 + clkdiv = 0; 179 + } 180 + 181 + cnt_duty = mul_u64_u64_div_u64(duty_ns, clk_rate, NSEC_PER_SEC) >> clkdiv; 182 + if (cnt_duty > cnt_period) 183 + cnt_duty = cnt_period; 184 + 185 + if (cnt_duty) { 186 + cnt_duty -= 1; 187 + enable = BIT(pwm->hwpwm); 188 + } else { 189 + enable = 0; 190 + } 191 + 192 + cnt_period -= 1; 193 + 194 + dev_dbg(&chip->dev, "pwm#%u: %lld/%lld @%lu -> CON: %x, PERIOD: %llx, DUTY: %llx\n", 195 + pwm->hwpwm, duty_ns, period_ns, clk_rate, clkdiv, cnt_period, cnt_duty); 187 196 188 197 if (pc->soc->pwm45_fixup && pwm->hwpwm > 2) { 189 198 /* ··· 207 190 reg_thres = PWM45THRES_FIXUP; 208 191 } 209 192 210 - cnt_duty = DIV_ROUND_CLOSEST_ULL((u64)duty_ns * 1000, resolution); 211 - 212 193 pwm_mediatek_writel(pc, pwm->hwpwm, PWMCON, BIT(15) | clkdiv); 213 - pwm_mediatek_writel(pc, pwm->hwpwm, reg_width, cnt_period - 1); 194 + pwm_mediatek_writel(pc, pwm->hwpwm, reg_width, cnt_period); 214 195 215 - if (cnt_duty) { 216 - pwm_mediatek_writel(pc, pwm->hwpwm, reg_thres, cnt_duty - 1); 196 + if (enable) { 197 + pwm_mediatek_writel(pc, pwm->hwpwm, reg_thres, cnt_duty); 217 198 pwm_mediatek_enable(chip, pwm); 218 199 } else { 219 200 pwm_mediatek_disable(chip, pwm);