On 06/07/17 10:24, 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. ... and you are right on both counts! Thanks for the detailed reply. Daniel.
