Hi! On Thu 2017-07-06 11:24:48, Thierry Reding wrote: > On Thu, Jul 06, 2017 at 10:17:18AM +0100, Daniel Thompson wrote: > > On 06/07/17 10:12, Pavel Machek wrote: > > > On Thu 2017-07-06 10:01:32, Thierry Reding wrote: > > > > On Fri, Jun 30, 2017 at 01:21:07PM +0200, Enric Balletbo i Serra wrote: > > > > > From: huang lin <hl at rock-chips.com> > > > > > > > > > > Some panels (i.e. N116BGE-L41), in their power sequence specifications, > > > > > request a delay between set the PWM signal and enable the backlight and > > > > > between clear the PWM signal and disable the backlight. Add support for > > > > > the new pwm-delay-us property to meet the timing. > > > > > > > > > > Note that this patch inverts current sequence. Before this patch the > > > > > enable signal was set before the PWM signal and vice-versa on power off. > > > > > > > > > > I assumed that this sequence was wrong, at least it is on different panel > > > > > datasheets that I checked, so I inverted the sequence to follow: > > > > > > > > > > On power on, set the PWM signal, wait, and set the LED_EN signal. > > > > > On power off, clear the LED_EN signal, wait, and stop the PWM signal. > > > > > > > > I think this should be two separate patches to make it easier to revert > > > > the inverted sequence should it prove to regress on other panels. > > > > > > Don't make this overly complex. This is trivial. No need to split it > > > into more patches. > > > > Agree. IMHO getting the code that reads the (optional) new parameter correct > > is the best way to manage risk of regression since in most cases the delay > > will be skipped anyway. > > The potential regression that I'm referring to would be caused by > inversing the sequence (GPIO enable -> PWM enable). That's completely > unrelated to the delays introduced by this patch. Many boards use this > driver and they've been running with the old sequence for many years. > Granted, it's fairly unlikely to regress, but it's still a > possibility. > > Given that both changes are logically separate, I think separate patches > are totally appropriate. I also don't think that this would overly > complicate things. Ah, yes, you are right; should be two patches. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 181 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-rockchip/attachments/20170706/9bcf87f4/attachment-0001.sig>
