Re: pwm: rcar: improve calculation of divider

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Uwe,

On Friday, 7 December 2018 23:49:32 EET Uwe Kleine-König wrote:
> Hello,
> 
> while looking at the driver I noticed another patch opportunity: In
> rcar_pwm_get_clock_division() there is a loop:
> 
> 	for (div = 0; div <= RCAR_PWM_MAX_DIVISION; div++) {
> 		max = (unsigned long long)NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE *
> 			(1 << div);
> 		do_div(max, clk_rate);
> 		if (period_ns <= max)
> 			break;
> 	}
> 
> The value of div should be calculatable without a loop. Something like:
> 
>    divider = NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE;
>    tmp = (unsigned long long)period_ns * clk_rate + (divider - 1);
>    do_div(tmp, divider);
>    div = ilog2(tmp - 1) + 1;
> 
> You might want to check if my maths are right, I didn't test.

I've noticed the same, and wrote the following patch last week, also untested.
I was planning to give it a try before sending it out, but as you've noticed
the same issue, here's the code if anyone wants to give it a try before I can.
Our calculations are similar, the main difference is the last line, and I
think yours read better.

From 22f7149916f590d3dbcc673dacc738441c741900 Mon Sep 17 00:00:00 2001
From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Date: Sun, 25 Nov 2018 16:02:39 +0200
Subject: [PATCH] pwm: rcar: Optimize rcar_pwm_get_clock_division()

Get rid of the loop over all possible divisor values by computing the
divisor directly.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/pwm/pwm-rcar.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
index a41812fc6f95..e6d73b94d5cf 100644
--- a/drivers/pwm/pwm-rcar.c
+++ b/drivers/pwm/pwm-rcar.c
@@ -68,21 +68,27 @@ static void rcar_pwm_update(struct rcar_pwm_chip *rp, u32 mask, u32 data,
 static int rcar_pwm_get_clock_division(struct rcar_pwm_chip *rp, int period_ns)
 {
 	unsigned long clk_rate = clk_get_rate(rp->clk);
-	unsigned long long max; /* max cycle / nanoseconds */
-	unsigned int div;
+	u64 max_period_ns;
+	u32 div;
 
 	if (clk_rate == 0)
 		return -EINVAL;
 
-	for (div = 0; div <= RCAR_PWM_MAX_DIVISION; div++) {
-		max = (unsigned long long)NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE *
-			(1 << div);
-		do_div(max, clk_rate);
-		if (period_ns <= max)
-			break;
-	}
+	/*
+	 * The maximum achievable period is 2^24 * 1023 cycles of the internal
+	 * bus clock.
+	 */
+	max_period_ns = (1ULL << RCAR_PWM_MAX_DIVISION) * RCAR_PWM_MAX_CYCLE
+		      * NSEC_PER_SEC;
+	do_div(max_period_ns, clk_rate);
+
+	if (period_ns > max_period_ns)
+		return -ERANGE;
 
-	return (div <= RCAR_PWM_MAX_DIVISION) ? div : -ERANGE;
+	/* Calculate the divisor and round it up to the next power of two. */
+	div = DIV64_U64_ROUND_UP((u64)period_ns * clk_rate,
+				 (u64)RCAR_PWM_MAX_CYCLE * NSEC_PER_SEC);
+	return fls(2 * div - 1) - 1;
 }
 
 static void rcar_pwm_set_clock_control(struct rcar_pwm_chip *rp,

-- 
Regards,

Laurent Pinchart