lkml.org 
[lkml]   [2019]   [Jan]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v9 14/27] pwm: jz4740: Improve algorithm of clock calculation
Hi,

On Sat, Jan 5, 2019 at 4:57 PM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> 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@crapouillou.net>
>> ---
>>
>> 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/ |

\
 
 \ /
  Last update: 2019-01-05 22:07    [W:0.084 / U:0.760 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site