RE: pwm: rcar: improve calculation of divider

[Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>]

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