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@xxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxx>
---
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
