* Re: [PATCH] pwm: atmel-pwm: fix calculation of prescale value
[not found] <20140924055851.CE51140656@mail.steuer-voss.de>
@ 2014-09-25 6:52 ` Thierry Reding
2014-09-25 7:47 ` Nikolaus Voss
0 siblings, 1 reply; 6+ messages in thread
From: Thierry Reding @ 2014-09-25 6:52 UTC (permalink / raw)
To: Nikolaus Voss; +Cc: Bo Shen, Nicolas Ferre, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2328 bytes --]
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
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] pwm: atmel-pwm: fix calculation of prescale value
2014-09-25 6:52 ` [PATCH] pwm: atmel-pwm: fix calculation of prescale value Thierry Reding
@ 2014-09-25 7:47 ` Nikolaus Voss
2014-09-25 7:49 ` Thierry Reding
0 siblings, 1 reply; 6+ messages in thread
From: Nikolaus Voss @ 2014-09-25 7:47 UTC (permalink / raw)
To: Thierry Reding; +Cc: Bo Shen, Nicolas Ferre, linux-kernel
Hi Thierry,
On Thu, 25 Sep 2014, Thierry Reding wrote:
> Please Cc the linux-pwm@vger.kernel.org mailing list for PWM-related
> patches in the future.
ok, I ran get_maintainer.pl on an old kernel...
> Also a couple more comments:
>
> In the patch description: "pwm frequency" should be "PWM frequency".
ok.
>> + div = (unsigned long long)clk_get_rate(atmel_pwm->clk) * period_ns;
>> + do_div(div, (int)1e9);
>
> 1e9 should be NSEC_PER_SEC.
>
>> - }
>> + 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.
ok (as I mostly do C++ programming, I prefer the prefix as it is usually
more efficient for user-defined types).
> No need for you to respin the patch, I've fixed up the above when
> applying.
Thanks!
Niko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pwm: atmel-pwm: fix calculation of prescale value
2014-09-25 7:47 ` Nikolaus Voss
@ 2014-09-25 7:49 ` Thierry Reding
0 siblings, 0 replies; 6+ messages in thread
From: Thierry Reding @ 2014-09-25 7:49 UTC (permalink / raw)
To: Nikolaus Voss; +Cc: Bo Shen, Nicolas Ferre, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 498 bytes --]
On Thu, Sep 25, 2014 at 09:47:34AM +0200, Nikolaus Voss wrote:
> On Thu, 25 Sep 2014, Thierry Reding wrote:
[...]
> >>- }
> >>+ 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.
>
> ok (as I mostly do C++ programming, I prefer the prefix as it is usually
> more efficient for user-defined types).
It is? That's intriguing...
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] pwm: atmel-pwm: fix calculation of prescale value
@ 2014-09-23 13:30 Nikolaus Voss
2014-09-25 1:50 ` Bo Shen
0 siblings, 1 reply; 6+ messages in thread
From: Nikolaus Voss @ 2014-09-23 13:30 UTC (permalink / raw)
To: Bo Shen, Nicolas Ferre; +Cc: Thierry Reding, linux-kernel
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(-)
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);
- /* 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;
+ }
+
+ if (pres > PRD_MAX_PRES) {
+ dev_err(chip->dev, "pres exceeds the maximum value\n");
+ return -EINVAL;
}
/* Calculate the duty cycles */
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] pwm: atmel-pwm: fix calculation of prescale value
2014-09-23 13:30 Nikolaus Voss
@ 2014-09-25 1:50 ` Bo Shen
2014-09-25 1:53 ` Bo Shen
0 siblings, 1 reply; 6+ messages in thread
From: Bo Shen @ 2014-09-25 1:50 UTC (permalink / raw)
To: Nikolaus Voss, Nicolas Ferre; +Cc: Thierry Reding, linux-kernel
Hi Nikolaus Voss,
On 09/23/2014 09:30 PM, 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.
Thanks for your patch.
> Signed-off-by: Nikolaus Voss <n.voss@weinmann-emt.de>
Tested-by: Bo Shen <voice.shen@atmel.com>
Acked-by: Bo Shen <voice.shen@atmel.com>
> ---
> drivers/pwm/pwm-atmel.c | 24 +++++++++++-------------
> 1 file changed, 11 insertions(+), 13 deletions(-)
>
> 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);
>
> - /* 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;
> + }
> +
> + if (pres > PRD_MAX_PRES) {
> + dev_err(chip->dev, "pres exceeds the maximum value\n");
> + return -EINVAL;
> }
>
> /* Calculate the duty cycles */
>
Best Regards,
Bo Shen
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] pwm: atmel-pwm: fix calculation of prescale value
2014-09-25 1:50 ` Bo Shen
@ 2014-09-25 1:53 ` Bo Shen
0 siblings, 0 replies; 6+ messages in thread
From: Bo Shen @ 2014-09-25 1:53 UTC (permalink / raw)
To: Nikolaus Voss, Nicolas Ferre,
thierry.reding@gmail.com >> Thierry Reding
Cc: linux-kernel, linux-arm-kernel@lists.infradead.org
Hi,
Correct the PWM maintainer Thierry Reding's e-mail address.
and Add linux-arm-kernel ML.
On 09/25/2014 09:50 AM, Bo Shen wrote:
> Hi Nikolaus Voss,
>
> On 09/23/2014 09:30 PM, 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.
>
> Thanks for your patch.
>
>> Signed-off-by: Nikolaus Voss <n.voss@weinmann-emt.de>
>
> Tested-by: Bo Shen <voice.shen@atmel.com>
> Acked-by: Bo Shen <voice.shen@atmel.com>
>
>> ---
>> drivers/pwm/pwm-atmel.c | 24 +++++++++++-------------
>> 1 file changed, 11 insertions(+), 13 deletions(-)
>>
>> 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);
>>
>> - /* 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;
>> + }
>> +
>> + if (pres > PRD_MAX_PRES) {
>> + dev_err(chip->dev, "pres exceeds the maximum value\n");
>> + return -EINVAL;
>> }
>>
>> /* Calculate the duty cycles */
>>
>
> Best Regards,
> Bo Shen
Best Regards,
Bo Shen
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-09-25 7:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20140924055851.CE51140656@mail.steuer-voss.de>
2014-09-25 6:52 ` [PATCH] pwm: atmel-pwm: fix calculation of prescale value Thierry Reding
2014-09-25 7:47 ` Nikolaus Voss
2014-09-25 7:49 ` Thierry Reding
2014-09-23 13:30 Nikolaus Voss
2014-09-25 1:50 ` Bo Shen
2014-09-25 1:53 ` Bo Shen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).