Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751862AbdGFRH7 (ORCPT ); Thu, 6 Jul 2017 13:07:59 -0400 Received: from mail.kernel.org ([198.145.29.99]:43366 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750970AbdGFRH6 (ORCPT ); Thu, 6 Jul 2017 13:07:58 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 565D922BD9 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=robh+dt@kernel.org MIME-Version: 1.0 In-Reply-To: <20170630112109.13785-1-enric.balletbo@collabora.com> References: <20170630112109.13785-1-enric.balletbo@collabora.com> From: Rob Herring Date: Thu, 6 Jul 2017 12:07:35 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 1/4] dt-bindings: pwm-backlight: add pwm-delay-us property To: Enric Balletbo i Serra Cc: 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: 2706 Lines: 55 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. 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. 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