Re: [PATCH 0/6] pwm: sun4i: only wait 2 cycles prior to disabling

[Pascal Roeleven <dev@pascalroeleven.nl>]

On 2021-05-31 21:07, Pascal Roeleven wrote:
> On 2021-05-31 06:46, Roman Beranek wrote:
>> As Emil Lenngren has previously shown [1], actually only 1-2 cycles of
>> the prescaler-divided clock are necessary to pass before the PWM turns
>> off, not a full period.
>>
>> To avoid having the PWM re-enabled from another thread while asleep,
>> ctrl_lock spinlock was converted to a mutex so that it can be released
>> only after the clock gate has finally been turned on.
>>
>> [1] https://linux-sunxi.org/PWM_Controller_Register_Guide
>>
>> Roman Beranek (6):
>>   pwm: sun4i: enable clk prior to getting its rate
>>   pwm: sun4i: disable EN bit prior to the delay
>>   pwm: sun4i: replace spinlock with a mutex
>>   pwm: sun4i: simplify calculation of the delay time
>>   pwm: sun4i: shorten the delay to 2 cycles
>>   pwm: sun4i: don't delay if the PWM is already off
>>
>>  drivers/pwm/pwm-sun4i.c | 56 +++++++++++++++++++----------------------
>>  1 file changed, 26 insertions(+), 30 deletions(-)
> 
> Hi Roman,
> 
> Thanks for your attempt to fix this.
> 
> Unfortunately on my A10 device (Topwise A721), the controller still gets
> stuck in an unrecoverable state after disabling and re-enabling the PWM
> when it was already on (set in U-Boot), or when enabling it when it was
> off. In this state, any changes to the period register (using devmem)
> don't seem to have any effect. It seems to be stuck in the state it was
> before disabling. The only thing which still works is enabling and
> disabling.
> 
> I can't reproduce this behavior manually so I'm not sure what is causing
> this.
> 
> Regarding the amount of cycles of sleep; Using a prescaler of 72000 the
> PWM clock is 3 ms. Although timing tests using devmem seem unreliable
> (too much overhead?), in U-Boot I need to 'sleep' for at least 7 ms
> between the commands to make sure the output doesn't sometimes get stuck
> in the enabled state. So in my case it seems to be at least 3 cycles. I
> am not sure how reliable this method is. However even if I can get it
> stuck in the enabled state using a shorter time, it doesn't cause the
> behavior I mentioned before. I was always able to recover it manually.
> Increasing the number of cycles to sleep therefore also doesn't solve my
> problem. Until we can solve that I cannot confirm nor deny if 2 cycles
> is enough.
> 
> Regards,
> Pascal

Turns out, what I'm referring to here is actually a different issue not
related to this patch series. A different series might be sent later to
address that. 

So no objections from my side for this one.

Regards,
Pascal