Messages in this thread | | | Date | Thu, 25 Sep 2014 08:52:30 +0200 | From | Thierry Reding <> | Subject | Re: [PATCH] pwm: atmel-pwm: fix calculation of prescale value |
| |
On Tue, Sep 23, 2014 at 03:30:21PM +0200, Nikolaus Voss wrote: > The prescale value used for calculating the period was incremented > afterwards, thus the resulting prescale value is by one too high. > This resulted in a pwm frequency only half as high as requested. > > This patch moves the 64 bit division out of the prescale loop to > correct the above issue and make the calculation more efficient. > > Signed-off-by: Nikolaus Voss <n.voss@weinmann-emt.de> > --- > drivers/pwm/pwm-atmel.c | 24 +++++++++++------------- > 1 file changed, 11 insertions(+), 13 deletions(-)
Hi Nikolaus,
Please Cc the linux-pwm@vger.kernel.org mailing list for PWM-related patches in the future.
Also a couple more comments:
In the patch description: "pwm frequency" should be "PWM frequency".
> diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c > index 6e700a5..ff17b5d 100644 > --- a/drivers/pwm/pwm-atmel.c > +++ b/drivers/pwm/pwm-atmel.c > @@ -102,7 +102,7 @@ static int atmel_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, > int duty_ns, int period_ns) > { > struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip); > - unsigned long clk_rate, prd, dty; > + unsigned long prd, dty; > unsigned long long div; > unsigned int pres = 0; > u32 val; > @@ -113,20 +113,18 @@ static int atmel_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, > return -EBUSY; > } > > - clk_rate = clk_get_rate(atmel_pwm->clk); > - div = clk_rate; > + /* Calculate the period cycles and prescale value */ > + div = (unsigned long long)clk_get_rate(atmel_pwm->clk) * period_ns; > + do_div(div, (int)1e9);
1e9 should be NSEC_PER_SEC.
> - /* Calculate the period cycles */ > while (div > PWM_MAX_PRD) { > - div = clk_rate / (1 << pres); > - div = div * period_ns; > - /* 1/Hz = 100000000 ns */ > - do_div(div, 1000000000); > - > - if (pres++ > PRD_MAX_PRES) { > - dev_err(chip->dev, "pres exceeds the maximum value\n"); > - return -EINVAL; > - } > + div >>= 1; > + ++pres;
Unless you really need the prefix increment behaviour (you don't in this case) I prefer using the postfix operator because it is slightly more idiomatic.
No need for you to respin the patch, I've fixed up the above when applying.
Thierry [unhandled content-type:application/pgp-signature] | |