pwm: atmel: problem when disable pwm

pwm: atmel: problem when disable pwm

[Raphaël Poggi]

Hello,

I'm using the pwm-atmel driver to control led, but i'm facing a problem.

When writing "echo 0 > /sys/class/leds/d1/brightness" (to switch off
the led) sometimes the led stayed active.

After investigation I think this problem is due to this function:

static void atmel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
{
    struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);

    atmel_pwm_writel(atmel_pwm, PWM_DIS, 1 << pwm->hwpwm);

    clk_disable(atmel_pwm->clk);
}

By disable the pwm channel, I think the duty cycle "0" has not been
update, and the channel is disable and stayed in active state (high or
low depending of polarity).

I have fixing the problem with this patch:

pwm: atmel: improve disable of pwm

Signed-off-by: Raphaël Poggi <poggi.raph@gmail.com>
---
 drivers/pwm/pwm-atmel.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index 0adc952..a227c89 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -236,16 +236,17 @@ static int atmel_pwm_enable(struct pwm_chip
*chip, struct pwm_device *pwm)
                return ret;
        }

-       atmel_pwm_writel(atmel_pwm, PWM_ENA, 1 << pwm->hwpwm);
-
        return 0;
 }

 static void atmel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
        struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
+       unsigned long clk_rate = clk_get_rate(atmel_pwm->clk);
+       unsigned long clk_period = 1000000000/clk_rate;
+       unsigned int pwm_period = atmel_pwm_ch_readl(atmel_pwm,
pwm->hwpwm, PWMV2_CPRD);

-       atmel_pwm_writel(atmel_pwm, PWM_DIS, 1 << pwm->hwpwm);
+       ndelay(pwm_period * clk_period);

        clk_disable(atmel_pwm->clk);
 }

With this patch instead of disable / enable pwm channel, we wait nsec
depending of MCK rate and pwm period and then disable the clock. By
doing this, we are sure that the duty cycle has been update and
disable in good state.

Is this seems right for you or my patch/fix is stupid ?

Maybe my problem is not due to this function, if so do you have any
idea of where it can come from ?

Thanks,
Raphaël

Re: pwm: atmel: problem when disable pwm

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]

Adding in the linux-pwm mailing list.

On Wed, Jun 04, 2014 at 02:56:13PM +0200, Raphaël Poggi wrote:
> Hello,
> 
> I'm using the pwm-atmel driver to control led, but i'm facing a problem.
> 
> When writing "echo 0 > /sys/class/leds/d1/brightness" (to switch off
> the led) sometimes the led stayed active.
> 
> After investigation I think this problem is due to this function:
> 
> static void atmel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> {
>     struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
> 
>     atmel_pwm_writel(atmel_pwm, PWM_DIS, 1 << pwm->hwpwm);
> 
>     clk_disable(atmel_pwm->clk);
> }
> 
> By disable the pwm channel, I think the duty cycle "0" has not been
> update, and the channel is disable and stayed in active state (high or
> low depending of polarity).

This sounds like a bug that could be fixed by this commit:

	916030db4399 pwm: atmel: correct CDTY calculation

What version of the kernel are you seeing this problem with? The above
commit was first added in v3.15-rc1, so maybe try that or any later
version to see if that fixes it.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: pwm: atmel: problem when disable pwm

[Raphaël Poggi]

Hello,

Thanks for your answer.

I'm seeing the problem with the version 3.10-at91. I tried with the
newer version of the driver which add the 916030db4399 commit and I
have the same bug.

2014-06-04 16:46 GMT+02:00 Thierry Reding <thierry.reding@gmail.com>:
> Adding in the linux-pwm mailing list.
>
> On Wed, Jun 04, 2014 at 02:56:13PM +0200, Raphaël Poggi wrote:
>> Hello,
>>
>> I'm using the pwm-atmel driver to control led, but i'm facing a problem.
>>
>> When writing "echo 0 > /sys/class/leds/d1/brightness" (to switch off
>> the led) sometimes the led stayed active.
>>
>> After investigation I think this problem is due to this function:
>>
>> static void atmel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
>> {
>>     struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
>>
>>     atmel_pwm_writel(atmel_pwm, PWM_DIS, 1 << pwm->hwpwm);
>>
>>     clk_disable(atmel_pwm->clk);
>> }
>>
>> By disable the pwm channel, I think the duty cycle "0" has not been
>> update, and the channel is disable and stayed in active state (high or
>> low depending of polarity).
>
> This sounds like a bug that could be fixed by this commit:
>
>         916030db4399 pwm: atmel: correct CDTY calculation
>
> What version of the kernel are you seeing this problem with? The above
> commit was first added in v3.15-rc1, so maybe try that or any later
> version to see if that fixes it.
>
> Thierry