* [PATCH] pwm: atmel: Fix disabling of PWM channels
@ 2016-05-13 11:09 Guillermo Rodriguez
2016-06-09 16:36 ` Guillermo Rodriguez Garcia
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Guillermo Rodriguez @ 2016-05-13 11:09 UTC (permalink / raw)
To: Thierry Reding, linux-kernel, linux-pwm, Nicolas Ferre
Cc: Guillermo Rodriguez
When disabling a PWM channel, the PWM clock was being stopped
immediately after writing to PWM_DIS. As a result, the disabling
of the PWM channel did not complete properly, and the PWM output
might be left at the wrong level.
Fix this by waiting for the channel to be effectively disabled
(by checking the PWM_SR register) before disabling the clock.
Signed-off-by: Guillermo Rodriguez <guille.rodriguez@gmail.com>
---
drivers/pwm/pwm-atmel.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index 0e4bd4e..a714434 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -271,6 +271,16 @@ static void atmel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
mutex_unlock(&atmel_pwm->isr_lock);
atmel_pwm_writel(atmel_pwm, PWM_DIS, 1 << pwm->hwpwm);
+ /*
+ * Wait for the PWM channel disable operation to be effective before
+ * stopping the clock.
+ */
+ timeout = jiffies + 2 * HZ;
+ while ((atmel_pwm_readl(atmel_pwm, PWM_SR)& (1 << pwm->hwpwm)) &&
+ time_before(jiffies, timeout)) {
+ usleep_range(10, 100);
+ }
+
clk_disable(atmel_pwm->clk);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] pwm: atmel: Fix disabling of PWM channels
2016-05-13 11:09 [PATCH] pwm: atmel: Fix disabling of PWM channels Guillermo Rodriguez
@ 2016-06-09 16:36 ` Guillermo Rodriguez Garcia
2016-06-10 8:29 ` Alexandre Belloni
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Guillermo Rodriguez Garcia @ 2016-06-09 16:36 UTC (permalink / raw)
To: Thierry Reding, linux-kernel, linux-pwm, Nicolas Ferre
Cc: Guillermo Rodriguez
Hello,
Any comments on this patch?
Guillermo
2016-05-13 13:09 GMT+02:00 Guillermo Rodriguez <guille.rodriguez@gmail.com>:
> When disabling a PWM channel, the PWM clock was being stopped
> immediately after writing to PWM_DIS. As a result, the disabling
> of the PWM channel did not complete properly, and the PWM output
> might be left at the wrong level.
>
> Fix this by waiting for the channel to be effectively disabled
> (by checking the PWM_SR register) before disabling the clock.
>
> Signed-off-by: Guillermo Rodriguez <guille.rodriguez@gmail.com>
> ---
> drivers/pwm/pwm-atmel.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
> index 0e4bd4e..a714434 100644
> --- a/drivers/pwm/pwm-atmel.c
> +++ b/drivers/pwm/pwm-atmel.c
> @@ -271,6 +271,16 @@ static void atmel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> mutex_unlock(&atmel_pwm->isr_lock);
> atmel_pwm_writel(atmel_pwm, PWM_DIS, 1 << pwm->hwpwm);
>
> + /*
> + * Wait for the PWM channel disable operation to be effective before
> + * stopping the clock.
> + */
> + timeout = jiffies + 2 * HZ;
> + while ((atmel_pwm_readl(atmel_pwm, PWM_SR)& (1 << pwm->hwpwm)) &&
> + time_before(jiffies, timeout)) {
> + usleep_range(10, 100);
> + }
> +
> clk_disable(atmel_pwm->clk);
> }
>
> --
> 1.7.9.5
>
--
Guillermo Rodriguez Garcia
guille.rodriguez@gmail.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: pwm: atmel: Fix disabling of PWM channels
2016-05-13 11:09 [PATCH] pwm: atmel: Fix disabling of PWM channels Guillermo Rodriguez
2016-06-09 16:36 ` Guillermo Rodriguez Garcia
@ 2016-06-10 8:29 ` Alexandre Belloni
2016-06-13 10:31 ` Guillermo Rodriguez Garcia
2016-06-13 9:50 ` Alexandre Belloni
2016-07-11 10:16 ` [PATCH] " Thierry Reding
3 siblings, 1 reply; 6+ messages in thread
From: Alexandre Belloni @ 2016-06-10 8:29 UTC (permalink / raw)
To: Guillermo Rodriguez
Cc: Thierry Reding, linux-kernel, linux-pwm, Nicolas Ferre
Hi,
On 13/05/2016 at 13:09:37 +0200, Guillermo Rodriguez wrote :
> When disabling a PWM channel, the PWM clock was being stopped
> immediately after writing to PWM_DIS. As a result, the disabling
> of the PWM channel did not complete properly, and the PWM output
> might be left at the wrong level.
>
> Fix this by waiting for the channel to be effectively disabled
> (by checking the PWM_SR register) before disabling the clock.
>
> Signed-off-by: Guillermo Rodriguez <guille.rodriguez@gmail.com>
> ---
> drivers/pwm/pwm-atmel.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
> index 0e4bd4e..a714434 100644
> --- a/drivers/pwm/pwm-atmel.c
> +++ b/drivers/pwm/pwm-atmel.c
> @@ -271,6 +271,16 @@ static void atmel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> mutex_unlock(&atmel_pwm->isr_lock);
> atmel_pwm_writel(atmel_pwm, PWM_DIS, 1 << pwm->hwpwm);
>
> + /*
> + * Wait for the PWM channel disable operation to be effective before
> + * stopping the clock.
> + */
> + timeout = jiffies + 2 * HZ;
> + while ((atmel_pwm_readl(atmel_pwm, PWM_SR)& (1 << pwm->hwpwm)) &&
> + time_before(jiffies, timeout)) {
> + usleep_range(10, 100);
> + }
> +
While this seems good, can you tell on which SoC you observed that? I'd
like to understand whether this is only on v1 or v2 or both.
Thanks!
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: pwm: atmel: Fix disabling of PWM channels
2016-05-13 11:09 [PATCH] pwm: atmel: Fix disabling of PWM channels Guillermo Rodriguez
2016-06-09 16:36 ` Guillermo Rodriguez Garcia
2016-06-10 8:29 ` Alexandre Belloni
@ 2016-06-13 9:50 ` Alexandre Belloni
2016-07-11 10:16 ` [PATCH] " Thierry Reding
3 siblings, 0 replies; 6+ messages in thread
From: Alexandre Belloni @ 2016-06-13 9:50 UTC (permalink / raw)
To: Guillermo Rodriguez
Cc: Thierry Reding, linux-kernel, linux-pwm, Nicolas Ferre
On 13/05/2016 at 13:09:37 +0200, Guillermo Rodriguez wrote :
> When disabling a PWM channel, the PWM clock was being stopped
> immediately after writing to PWM_DIS. As a result, the disabling
> of the PWM channel did not complete properly, and the PWM output
> might be left at the wrong level.
>
> Fix this by waiting for the channel to be effectively disabled
> (by checking the PWM_SR register) before disabling the clock.
>
> Signed-off-by: Guillermo Rodriguez <guille.rodriguez@gmail.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
> drivers/pwm/pwm-atmel.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
> index 0e4bd4e..a714434 100644
> --- a/drivers/pwm/pwm-atmel.c
> +++ b/drivers/pwm/pwm-atmel.c
> @@ -271,6 +271,16 @@ static void atmel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> mutex_unlock(&atmel_pwm->isr_lock);
> atmel_pwm_writel(atmel_pwm, PWM_DIS, 1 << pwm->hwpwm);
>
> + /*
> + * Wait for the PWM channel disable operation to be effective before
> + * stopping the clock.
> + */
> + timeout = jiffies + 2 * HZ;
> + while ((atmel_pwm_readl(atmel_pwm, PWM_SR)& (1 << pwm->hwpwm)) &&
> + time_before(jiffies, timeout)) {
> + usleep_range(10, 100);
> + }
> +
> clk_disable(atmel_pwm->clk);
> }
>
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: pwm: atmel: Fix disabling of PWM channels
2016-06-10 8:29 ` Alexandre Belloni
@ 2016-06-13 10:31 ` Guillermo Rodriguez Garcia
0 siblings, 0 replies; 6+ messages in thread
From: Guillermo Rodriguez Garcia @ 2016-06-13 10:31 UTC (permalink / raw)
To: linux-kernel, linux-pwm
(resending to MLs for the record -- previous message bounced)
El viernes, 10 de junio de 2016, Alexandre Belloni
<alexandre.belloni@free-electrons.com> escribió:
>
> Hi,
>
> On 13/05/2016 at 13:09:37 +0200, Guillermo Rodriguez wrote :
> > When disabling a PWM channel, the PWM clock was being stopped
> > immediately after writing to PWM_DIS. As a result, the disabling
> > of the PWM channel did not complete properly, and the PWM output
> > might be left at the wrong level.
> >
> > Fix this by waiting for the channel to be effectively disabled
> > (by checking the PWM_SR register) before disabling the clock.
> >
> > Signed-off-by: Guillermo Rodriguez <guille.rodriguez@gmail.com>
> > ---
> > drivers/pwm/pwm-atmel.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
> > index 0e4bd4e..a714434 100644
> > --- a/drivers/pwm/pwm-atmel.c
> > +++ b/drivers/pwm/pwm-atmel.c
> > @@ -271,6 +271,16 @@ static void atmel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> > mutex_unlock(&atmel_pwm->isr_lock);
> > atmel_pwm_writel(atmel_pwm, PWM_DIS, 1 << pwm->hwpwm);
> >
> > + /*
> > + * Wait for the PWM channel disable operation to be effective before
> > + * stopping the clock.
> > + */
> > + timeout = jiffies + 2 * HZ;
> > + while ((atmel_pwm_readl(atmel_pwm, PWM_SR)& (1 << pwm->hwpwm)) &&
> > + time_before(jiffies, timeout)) {
> > + usleep_range(10, 100);
> > + }
> > +
>
> While this seems good, can you tell on which SoC you observed that? I'd
> like to understand whether this is only on v1 or v2 or both.
I observed it on a sama5d3, however this article from atmel's KB would
seem to suggest that it is not specific to this SoC:
http://atmel.force.com/support/pkb_mobile#/articles/en_US/FAQ/Why-PWM-Channel-Polarity-Inversion
Guillermo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pwm: atmel: Fix disabling of PWM channels
2016-05-13 11:09 [PATCH] pwm: atmel: Fix disabling of PWM channels Guillermo Rodriguez
` (2 preceding siblings ...)
2016-06-13 9:50 ` Alexandre Belloni
@ 2016-07-11 10:16 ` Thierry Reding
3 siblings, 0 replies; 6+ messages in thread
From: Thierry Reding @ 2016-07-11 10:16 UTC (permalink / raw)
To: Guillermo Rodriguez; +Cc: linux-kernel, linux-pwm, Nicolas Ferre
[-- Attachment #1: Type: text/plain, Size: 635 bytes --]
On Fri, May 13, 2016 at 01:09:37PM +0200, Guillermo Rodriguez wrote:
> When disabling a PWM channel, the PWM clock was being stopped
> immediately after writing to PWM_DIS. As a result, the disabling
> of the PWM channel did not complete properly, and the PWM output
> might be left at the wrong level.
>
> Fix this by waiting for the channel to be effectively disabled
> (by checking the PWM_SR register) before disabling the clock.
>
> Signed-off-by: Guillermo Rodriguez <guille.rodriguez@gmail.com>
> ---
> drivers/pwm/pwm-atmel.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
Applied, thanks.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-11 10:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-13 11:09 [PATCH] pwm: atmel: Fix disabling of PWM channels Guillermo Rodriguez
2016-06-09 16:36 ` Guillermo Rodriguez Garcia
2016-06-10 8:29 ` Alexandre Belloni
2016-06-13 10:31 ` Guillermo Rodriguez Garcia
2016-06-13 9:50 ` Alexandre Belloni
2016-07-11 10:16 ` [PATCH] " Thierry Reding
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).