Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752456AbdGFSXa (ORCPT ); Thu, 6 Jul 2017 14:23:30 -0400 Received: from mail-it0-f44.google.com ([209.85.214.44]:35620 "EHLO mail-it0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751816AbdGFSX2 (ORCPT ); Thu, 6 Jul 2017 14:23:28 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170630112109.13785-1-enric.balletbo@collabora.com> From: Enric Balletbo Serra Date: Thu, 6 Jul 2017 20:23:26 +0200 Message-ID: Subject: Re: [PATCH v2 1/4] dt-bindings: pwm-backlight: add pwm-delay-us property To: Rob Herring Cc: Enric Balletbo i Serra , Thierry Reding , Lee Jones , Daniel Thompson , Jingoo Han , Bartlomiej Zolnierkiewicz , Pavel Machek , Richard Purdie , Jacek Anaszewski , Heiko Stuebner , Linux PWM List , "linux-fbdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Guenter Roeck , "open list:ARM/Rockchip SoC..." , huang lin Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3435 Lines: 75 Hi Rob, 2017-07-06 19:07 GMT+02:00 Rob Herring : > On Fri, Jun 30, 2017 at 6:21 AM, Enric Balletbo i Serra > wrote: >> From: huang lin >> >> Add a pwm-delay-us property to specify the delay between setting an >> initial (non-zero) PWM value and enabling the backlight, and also the >> delay between disabling the backlight and setting PWM value to 0. >> >> Signed-off-by: huang lin >> Signed-off-by: Enric Balletbo i Serra >> --- >> Changes since v1: >> - As suggested by Daniel Thompson >> - Do not assume power-on delay and power-off delay will be the same >> >> v1: https://lkml.org/lkml/2017/6/28/219 >> >> Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt >> index 764db86..49b037e 100644 >> --- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt >> +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt >> @@ -17,6 +17,11 @@ Optional properties: >> "pwms" property (see PWM binding[0]) >> - enable-gpios: contains a single GPIO specifier for the GPIO which enables >> and disables the backlight (see GPIO binding[1]) >> + - pwm-delay-us: delay between setting an initial (non-zero) PWM value and >> + enabling the backlight, and also the delay between disabling >> + the backlight and setting PWM value to 0. >> + The 1st cell is the pre-delay in micro seconds. >> + The 2nd cell is the post-delay in micro seconds. > > pre and post imply a time before and after a certain event, but these > are for 2 different events. These are more like an enable/on delay and > disable/off delay which probably should be separate properties. What > happens when we need the opposite sequence or a different sequence? > Maybe some panel requires the PWM to be 0 until some time after > enabling. > Maybe, Only I can say that the panels I checked always first enable the PWM and then set the ENABLE signal, but of course I didn't check all the panels. Would be more acceptable have enable-delay-us and disable-delay-us proprieties? > I don't understand why you even need a post delay. The PWM can be set > to 0 while enabled, right? So if the PWM is set to 0 while enabled and > then disable the backlight, then there's no delay. Plus this doesn't > make much sense to me electrically either. The PWM duty cycle is going > to be completely async to the enable line change. The PWM state could > go from 1 to 0 at any point in time relative to the enable line > change. > To be honest I'm also not sure why is required the post delay, some panels specify a 0 but others specifies a minimum value between you off the panel and disable the PWM. The only reason I added the post delay is because the different datasheets specifies it, I don't have a use case that the post delay is used to fix something. Thanks, Enric > These issues are the problem with generic bindings. Adding 1 property > is no big deal, but then what happens with the next variation. These > timing constraints need to be able to be implied by the panel's > compatible. > > Rob