Hi Uwe, Laurent,
Thank you very much for your patches!
I tested patches and both codes work correctly.
> From: Laurent Pinchart, Sent: Monday, December 10, 2018 5:55 AM
>
> 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);
This should be do_div64_u64() because the divider is 1,023,000,000,000 (over 32-bits).
> > 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.
So, I'd like to apply Uwe's code to mainline. Uwe, may I send such a patch
with your author and Singed-off-by?
Best regards,
Yoshihiro Shimoda
> 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
>
>
