linux-kernel - Re: pwm: atmel: PWM may not properly disable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABDcavZv3KSz4a5s3f5iusF6rCnyT2-10HCR1Ns5SaJuAwptxQ@mail.gmail.com>
Date:	Thu, 12 May 2016 13:49:12 +0200
From:	Guillermo Rodriguez Garcia <guille.rodriguez@...il.com>
To:	Thierry Reding <thierry.reding@...il.com>
Cc:	linux-kernel@...r.kernel.org, linux-pwm@...r.kernel.org,
	Nicolas Ferre <nicolas.ferre@...el.com>
Subject: Re: pwm: atmel: PWM may not properly disable

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@...il.com