Re: [PATCH v9 14/27] pwm: jz4740: Improve algorithm of clock calculation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Sat, Jan 5, 2019 at 4:57 PM, Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote:
On Thu, Dec 27, 2018 at 07:13:06PM +0100, Paul Cercueil wrote:
The previous algorithm hardcoded details about how the TCU clocks work. The new algorithm will use clk_round_rate to find the perfect clock rate
 for the PWM channel.

 Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
 ---

 Notes:
      v9: New patch

  drivers/pwm/pwm-jz4740.c | 26 +++++++++++++++-----------
  1 file changed, 15 insertions(+), 11 deletions(-)

 diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
 index c6136bd4434b..dd80a2cf6528 100644
 --- a/drivers/pwm/pwm-jz4740.c
 +++ b/drivers/pwm/pwm-jz4740.c
@@ -110,23 +110,27 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
  	struct jz4740_pwm_chip *jz4740 = to_jz4740(pwm->chip);
  	struct clk *clk = jz4740->clks[pwm->hwpwm],
  		   *parent_clk = clk_get_parent(clk);
 -	unsigned long rate, period, duty;
 +	unsigned long rate, new_rate, period, duty;
  	unsigned long long tmp;
 -	unsigned int prescaler = 0;

  	rate = clk_get_rate(parent_clk);
 -	tmp = (unsigned long long)rate * state->period;
 -	do_div(tmp, 1000000000);
 -	period = tmp;

 -	while (period > 0xffff && prescaler < 6) {
 -		period >>= 2;
 -		rate >>= 2;
 -		++prescaler;
 +	for (;;) {
 +		tmp = (unsigned long long)rate * state->period;
 +		do_div(tmp, 1000000000);

NSEC_PER_SEC?

Ok, didn't know about it.

 +
 +		if (tmp <= 0xffff)
 +			break;
 +
 +		new_rate = clk_round_rate(clk, rate - 1);
 +
 +		if (new_rate < rate)
 +			rate = new_rate;
 +		else
 +			return -EINVAL;

You are assuming stuff here about the parent clk which isn't guaranteed (AFAICT) by the clk framework: If you call clk_round_rate(clk, rate - 1)
this might well return rate even if the clock could run slower than
rate.

It may not be guaranteed by the clock framework itself, but it is guaranteed
to behave like that on this family of SoCs.

Wouldn't it make sense to start iterating with rate = 0xffff * 1e9 /
period? Otherwise you get bad configurations if rate is considerable
slower than necessary.

The algorithm will start with 'rate' being the parent clock's rate, which
will always be the highest rate that the child clock will support.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |





[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux