[PATCH v1 0/2] pwm: stm32: A rounding fix and a cleanup

[PATCH v1 0/2] pwm: stm32: A rounding fix and a cleanup

[Uwe Kleine-König]

Hello,

I extended pwmtestperf (part of libpwm[1]) to allow testing inversed
polarity wave forms. I'm not sure if I should like it or not, but this
immediately showed a rounding issue in the stm32 pwm driver. This is
fixed in the first commit.

The 2nd commit is just a cleanup that I have on my todo list for a
while. This required the addition of mul_u64_u64_div_u64_roundup() which
exists since v6.19-rc1~70^2~90.

I intend to send the fix to Linus before 7.1, but will give it a bit of
time in next before. The cleanup will wait until the next merge window.

Best regards
Uwe

[1] https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/libpwm.git

Uwe Kleine-König (2):
  pwm: stm32: Fix rounding issue for requests with inverted polarity
  pwm: stm32: Make use of mul_u64_u64_div_u64_roundup()

 drivers/pwm/pwm-stm32.c | 49 ++++++++++++++---------------------------
 1 file changed, 17 insertions(+), 32 deletions(-)


base-commit: 028ef9c96e96197026887c0f092424679298aae8
-- 
2.47.3

[PATCH v1 1/2] pwm: stm32: Fix rounding issue for requests with inverted polarity

[Uwe Kleine-König]

The calculation of the number of pwm clk ticks from a time length in
nanoseconds involves a division and thus some rounding. That might
result in

	duty_ticks + offset_ticks < period_ticks

despite

	duty_length_ns + duty_offset_ns >= period_length_ns

. The stm32 PWM cannot configure offset_ticks freely, it can only select
0 or period_length_ns - duty_length_ns---that is the classic normal and
inverted polarity. The decision to select the hardware polarity must be
done using the ticks values and not the nanoseconds times to adhere to
the rounding rules by the pwm core.

With the pwm clk running at 208900 kHz on my test machine
(stm32mp135f-dk), a test case that was handled wrong is:

	# pwmround -P 9999962 -O 24970 -D 9974992
	period_length = 9999962
	duty_length = 9974840
	duty_offset = 25123

With this change applied the rounding is done correctly:

	# pwmround -P 9999962 -O 24970 -D 9974992
	period_length = 9999962
	duty_length = 9974840
	duty_offset = 0

Fixes: deaba9cff809 ("pwm: stm32: Implementation of the waveform callbacks")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/pwm/pwm-stm32.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 2594fb771b04..935257a890b0 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -68,7 +68,7 @@ static int stm32_pwm_round_waveform_tohw(struct pwm_chip *chip,
 	struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
 	unsigned int ch = pwm->hwpwm;
 	unsigned long rate;
-	u64 ccr, duty;
+	u64 duty_ticks, offset_ticks;
 	int ret;
 
 	if (wf->period_length_ns == 0) {
@@ -164,23 +164,25 @@ static int stm32_pwm_round_waveform_tohw(struct pwm_chip *chip,
 		wfhw->arr = min_t(u64, arr, priv->max_arr) - 1;
 	}
 
-	duty = mul_u64_u64_div_u64(wf->duty_length_ns, rate,
-				   (u64)NSEC_PER_SEC * (wfhw->psc + 1));
-	duty = min_t(u64, duty, wfhw->arr + 1);
+	duty_ticks = mul_u64_u64_div_u64(wf->duty_length_ns, rate,
+					 (u64)NSEC_PER_SEC * (wfhw->psc + 1));
+	duty_ticks = min_t(u64, duty_ticks, wfhw->arr + 1);
 
-	if (wf->duty_length_ns && wf->duty_offset_ns &&
-	    wf->duty_length_ns + wf->duty_offset_ns >= wf->period_length_ns) {
+	offset_ticks = mul_u64_u64_div_u64(wf->duty_offset_ns, rate,
+					   (u64)NSEC_PER_SEC * (wfhw->psc + 1));
+	offset_ticks = min_t(u64, offset_ticks, wfhw->arr + 1);
+
+	if (duty_ticks && offset_ticks &&
+	    duty_ticks + offset_ticks >= wfhw->arr + 1) {
 		wfhw->ccer |= TIM_CCER_CCxP(ch + 1);
 		if (priv->have_complementary_output)
 			wfhw->ccer |= TIM_CCER_CCxNP(ch + 1);
 
-		ccr = wfhw->arr + 1 - duty;
+		wfhw->ccr = wfhw->arr + 1 - duty_ticks;
 	} else {
-		ccr = duty;
+		wfhw->ccr = duty_ticks;
 	}
 
-	wfhw->ccr = min_t(u64, ccr, wfhw->arr + 1);
-
 out:
 	dev_dbg(&chip->dev, "pwm#%u: %lld/%lld [+%lld] @%lu -> CCER: %08x, PSC: %08x, ARR: %08x, CCR: %08x\n",
 		pwm->hwpwm, wf->duty_length_ns, wf->period_length_ns, wf->duty_offset_ns,
-- 
2.47.3

[PATCH v1 2/2] pwm: stm32: Make use of mul_u64_u64_div_u64_roundup()

[Uwe Kleine-König]

When the driver was converted to the waveform API the need for this
function arised but at that time this function didn't exist yet. In the
meantime it's available, so switch to the global function and drop the
driver specific implementation.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/pwm/pwm-stm32.c | 27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 935257a890b0..c708e4a7ad70 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -193,22 +193,6 @@ static int stm32_pwm_round_waveform_tohw(struct pwm_chip *chip,
 	return ret;
 }
 
-/*
- * This should be moved to lib/math/div64.c. Currently there are some changes
- * pending to mul_u64_u64_div_u64. Uwe will care for that when the dust settles.
- */
-static u64 stm32_pwm_mul_u64_u64_div_u64_roundup(u64 a, u64 b, u64 c)
-{
-	u64 res = mul_u64_u64_div_u64(a, b, c);
-	/* Those multiplications might overflow but it doesn't matter */
-	u64 rem = a * b - c * res;
-
-	if (rem)
-		res += 1;
-
-	return res;
-}
-
 static int stm32_pwm_round_waveform_fromhw(struct pwm_chip *chip,
 					   struct pwm_device *pwm,
 					   const void *_wfhw,
@@ -223,16 +207,15 @@ static int stm32_pwm_round_waveform_fromhw(struct pwm_chip *chip,
 		u64 ccr_ns;
 
 		/* The result doesn't overflow for rate >= 15259 */
-		wf->period_length_ns = stm32_pwm_mul_u64_u64_div_u64_roundup(((u64)wfhw->psc + 1) * (wfhw->arr + 1),
-									     NSEC_PER_SEC, rate);
+		wf->period_length_ns = mul_u64_u64_div_u64_roundup(((u64)wfhw->psc + 1) * (wfhw->arr + 1),
+								   NSEC_PER_SEC, rate);
 
-		ccr_ns = stm32_pwm_mul_u64_u64_div_u64_roundup(((u64)wfhw->psc + 1) * wfhw->ccr,
-							       NSEC_PER_SEC, rate);
+		ccr_ns = mul_u64_u64_div_u64_roundup(((u64)wfhw->psc + 1) * wfhw->ccr, NSEC_PER_SEC, rate);
 
 		if (wfhw->ccer & TIM_CCER_CCxP(ch + 1)) {
 			wf->duty_length_ns =
-				stm32_pwm_mul_u64_u64_div_u64_roundup(((u64)wfhw->psc + 1) * (wfhw->arr + 1 - wfhw->ccr),
-								      NSEC_PER_SEC, rate);
+				mul_u64_u64_div_u64_roundup(((u64)wfhw->psc + 1) * (wfhw->arr + 1 - wfhw->ccr),
+							    NSEC_PER_SEC, rate);
 
 			wf->duty_offset_ns = ccr_ns;
 		} else {
-- 
2.47.3