Messages in this thread | | | From | Emil Lenngren <> | Date | Tue, 17 Mar 2020 20:35:51 +0100 | Subject | Re: [RFC PATCH 0/4] pwm: sun4i: Properly turn pwm off and fix stuck output state |
| |
Hi,
Den tis 17 mars 2020 19:16Pascal Roeleven <dev@pascalroeleven.nl> skrev: > > On 2020-03-17 17:45, Emil Lenngren wrote: > > Hi all, > > > > Den tis 17 mars 2020 kl 17:00 skrev Pascal Roeleven > > <dev@pascalroeleven.nl>: > >> > >> Hi all, > >> > >> For the last few days I've been debugging a lot to get pwm working > >> again since > >> recent changes in 5.6-rc1 broke it for me. > >> > >> Testing shows the pwm controller crashes (or the output gets stuck) > >> when the > >> period register is written when the channel is disabled while the > >> clock gate is > >> still on. Usually after multiple writes, but one write can also lead > >> to > >> unpredictable behaviour. Patch 3 and 4 fix this. > >> > >> Patch 2 contains a fix which wouldn't completely turn off the pwm if > >> the > >> output is disabled. The clock gate needs to stay on for at least one > >> more > >> period to ensure the output is properly disabled. This issue has been > >> around > >> for a long time but has probably stayed unnoticed because if the > >> duty_cycle is > >> also changed to 0, you can't tell the difference. > >> > >> Patch 1 removes some leftovers which aren't needed anymore. > >> > >> Obviously these patches work for my device, but I'd like to hear your > >> opinion > >> if any of these changes make sense. After days, this one is a bit > >> blurry for me. > >> > >> Thanks to Uwe for some help with debugging. > >> > >> Pascal. > >> > >> Pascal Roeleven (4): > >> pwm: sun4i: Remove redundant needs_delay > >> pwm: sun4i: Disable pwm before turning off clock gate > >> pwm: sun4i: Move delay to function > >> pwm: sun4i: Delay after writing the period > >> > >> drivers/pwm/pwm-sun4i.c | 53 > >> ++++++++++++++++++++--------------------- > >> 1 file changed, 26 insertions(+), 27 deletions(-) > >> > >> -- > >> 2.20.1 > >> > > > > I also worked on sun4i-pwm some time ago, fixing a bunch of issues. > > One was that disabling the pwm sometimes didn't turn off the signal, > > because the gate and enable bit were modified in the same clock cycle. > > Another was that the current code used an unnecessary sleep of a whole > > period length (or more?) in case of an update to the period, which > > could be very time-consuming if it's a very long interval, like 2 > > seconds. > > > > Note that the behaviour is not unpredictable, if you know how it works > > ;) > > I fiddled around a long time with devmem2, an oscilloscope and the > > prescaler set to max to figure out how works internally. > > > > Please try my version I just posted at https://pastebin.com/GWrhWzPJ. > > It is based on this version from May 28, 2019: > > https://github.com/torvalds/linux/blob/f50a7f3d9225dd374455f28138f79ae3074a7a3d/drivers/pwm/pwm-sun4i.c. > > Sorry for not posting it inline, but GMail would break the formatting. > > It contains quite many comments about how it works internally. I also > > wrote a section at http://linux-sunxi.org/PWM_Controller, but it might > > be a bit old (two years), so please rather look at the code and the > > comments. > > > > /Emil > > Hi Emil, > > Thank you very much, this is helpful. Ah it was your note on the wiki. > That is indeed where I took the idea of keeping the gate on and > disabling the panel from. As a scope is still on my wishlist, the rest > was just trial-and-error. Judging from your code, there are more edge > cases which might occur. I will test your code and try to integrate it. > If it's okay with you, I can post it on your behalf?
Sure! I was thinking of sending a patch last year but never got the time :/ The devices I have tested on are Allwinner GR8 (A13) and V3s.
> > If you ask me, it's really unfortunate Allwinner didn't provide a timing > diagram for such a picky controller.
/Emil
| |