Re: pwm: atmel: PWM may not properly disable

[Guillermo Rodriguez Garcia]

Hello,

[...]
>>> One thing that I'd request is that instead of the cpu_relax() you use a
>>> usleep_range() within the loop instead. I assume it can potentially take
>>> a long time for the current period to finish, so busy looping isn't such
>>> a great idea. You could possibly use the current period_ns to derive a
>>> meaningful value to pass to usleep_range().
>>
>> I am not sure yet but I believe disabling does not really need to wait for the
>> current period to finish (at least the datasheets do not mention this anywhere).
>> I think that the after writing to PWM_DIS, the actual disable operation is
>> initiated immediately in the PWM subsystem, but is executed asynchronously
>> and requires the pwm_clk to complete. If this assumption is correct, perhaps
>> it is enough to do one single read from PWM_SR so that the disable operation
>> has had the chance to propagate. This is again assuming that all operations
>> are executed sequentially within the PWM subsystem.
>>
>> If the above is correct, then we would not need a loop at all.
>
> I was wrong. The required delay indeed seems to depend on the current PWM
> frequency, suggesting that indeed disabling does not take effect until
> the current
> period is finished.
>
> I will prepare a patch using usleep_range instead of cpu_relax.

I have found a problem while preparing this. If I use usleep_range I
keep running
into "BUG: scheduling while atomic". This is because I am using the PWM to
drive a buzzer with pwm-beeper, and pwm-beeper currently crashes if the PWM
driver sleeps. Apparently this patch is needed:

https://lkml.org/lkml/2016/2/22/757

However this has not been merged yet.

How should I proceed ?

Guillermo Rodriguez Garcia
guille.rodriguez@xxxxxxxxx