A small prescaler is beneficial, as this improves the resolution of the duty_cycle configuration. However if the prescaler is too small, the maximal possible period becomes considerably smaller than the requested value.
One situation where this goes wrong is the following: With a parent clock rate of 208877930 Hz and max_arr = 0xffff = 65535, a request for period = 941243 ns currently results in PSC = 1. The value for ARR is then calculated to
PSC = 941243 * 208877930 / (1000000000 * 2) - 1 = 98301
This value is bigger than 65535 however and so doesn't fit into the respective register. In this particular case the PWM was configured for a period of 313733.4806027616 ns (with ARR = 98301 & 0xffff). Even if ARR was configured to its maximal value, only period = 627495.6861167669 ns would be achievable.
Fix the calculation accordingly and adapt the comment to match the new algorithm.
With the calculation fixed the above case results in PSC = 2 and so an actual period of 941229.1667195285 ns.
Fixes: 8002fbeef1e4 ("pwm: stm32: Calculate prescaler with a division instead of a loop") Cc: stable@vger.kernel.org Signed-off-by: Uwe Kleine-König u.kleine-koenig@baylibre.com --- drivers/pwm/pwm-stm32.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c index 3e7b2a8e34e7..2de7195e43a9 100644 --- a/drivers/pwm/pwm-stm32.c +++ b/drivers/pwm/pwm-stm32.c @@ -321,17 +321,24 @@ static int stm32_pwm_config(struct stm32_pwm *priv, unsigned int ch, * First we need to find the minimal value for prescaler such that * * period_ns * clkrate - * ------------------------------ + * ------------------------------ ≤ max_arr * NSEC_PER_SEC * (prescaler + 1) * - * isn't bigger than max_arr. + * This equation is equivalent to + * + * period_ns * clkrate + * ---------------------- ≤ prescaler + 1 + * NSEC_PER_SEC * max_arr + * + * As the left hand side might not be integer but the right hand side + * is, the division must be rounded up when doing integer math. There + * is no variant of mul_u64_u64_div_u64() that rounds up, so we're + * trading that against the +1 which results in a non-optimal prescaler + * only if the division's result is integer. */
prescaler = mul_u64_u64_div_u64(period_ns, clk_get_rate(priv->clk), (u64)NSEC_PER_SEC * priv->max_arr); - if (prescaler > 0) - prescaler -= 1; - if (prescaler > MAX_TIM_PSC) return -EINVAL;
On Wed, Jun 19, 2024 at 11:26:25AM +0200, Uwe Kleine-König wrote:
A small prescaler is beneficial, as this improves the resolution of the duty_cycle configuration. However if the prescaler is too small, the maximal possible period becomes considerably smaller than the requested value.
One situation where this goes wrong is the following: With a parent clock rate of 208877930 Hz and max_arr = 0xffff = 65535, a request for period = 941243 ns currently results in PSC = 1. The value for ARR is then calculated to
PSC = 941243 * 208877930 / (1000000000 * 2) - 1 = 98301
^ This ----' must be ARR of course.
This value is bigger than 65535 however and so doesn't fit into the respective register. In this particular case the PWM was configured for
While improving the commit log, I'll do s/register/register field/, too.
a period of 313733.4806027616 ns (with ARR = 98301 & 0xffff). Even if ARR was configured to its maximal value, only period = 627495.6861167669 ns would be achievable.
Best regards Uwe
linux-stable-mirror@lists.linaro.org