Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752202AbdLAKjo (ORCPT ); Fri, 1 Dec 2017 05:39:44 -0500 Received: from mail-qk0-f193.google.com ([209.85.220.193]:45104 "EHLO mail-qk0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751089AbdLAKjm (ORCPT ); Fri, 1 Dec 2017 05:39:42 -0500 X-Google-Smtp-Source: AGs4zMYyFoCCUPrWPEkSoLZ+/qvGgzS9U4+pZvhanUm4qtmm40Qyp6QUZf+KDDarjFgfq5lMAMezkH0GsjtqR7/WYiE= MIME-Version: 1.0 In-Reply-To: <000001d30630$e2a3d160$a7eb7420$@gmail.com> References: <20170721104813.5389-1-enric.balletbo@collabora.com> <20170721104813.5389-3-enric.balletbo@collabora.com> <0e2d4f9b-0f27-3793-0c27-2a2df5c32cf0@linaro.org> <000001d30630$e2a3d160$a7eb7420$@gmail.com> From: Enric Balletbo Serra Date: Fri, 1 Dec 2017 11:39:40 +0100 Message-ID: Subject: Re: [PATCH v4 3/5] pwm-backlight: add support for PWM delays proprieties. To: Jingoo Han Cc: Daniel Thompson , Enric Balletbo i Serra , Thierry Reding , Lee Jones , Bartlomiej Zolnierkiewicz , Rob Herring , Pavel Machek , Richard Purdie , Jacek Anaszewski , Heiko Stuebner , Linux PWM List , linux-fbdev@vger.kernel.org, linux-kernel , Guenter Roeck , "open list:ARM/Rockchip SoC..." 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: 4793 Lines: 135 Hi, 2017-07-26 19:01 GMT+02:00 Jingoo Han : > On Monday, July 24, 2017 11:22 AM, Daniel Thompson wrote: >> On 21/07/17 11:48, Enric Balletbo i Serra wrote: >> > 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 post-pwm-on-delay-ms and pwm-off-delay-ms proprieties to meet >> > the timings. >> > >> > Signed-off-by: Enric Balletbo i Serra >> >> Acked-by: Daniel Thompson > > Acked-by: Jingoo Han > A lot of time has passed since these patches received the acks but I'm still without seeing the patches in linux-next. There is some action I need to do ? (rebase?). I'd really like to see those to land in next merge window if all is ok. Thanks, Enric > Best regards, > Jingoo Han > >> >> >> > --- >> > Changes since v3: >> > - Use two named members instead of pwm_delay[] (Daniel and Pavel) >> > - Use msleep instead of usleep_range. (Pavel) >> > Changes since v2: >> > - Move the pwm/enable sequence to another patch (Thierry Reding) >> > Changes since v1: >> > - As suggested by Daniel Thompson >> > - Do not assume power-on delay and power-off delay will be the same >> > - Move the check of dt property to the parse dt function. >> > >> > drivers/video/backlight/pwm_bl.c | 19 +++++++++++++++++++ >> > include/linux/pwm_backlight.h | 2 ++ >> > 2 files changed, 21 insertions(+) >> > >> > diff --git a/drivers/video/backlight/pwm_bl.c >> b/drivers/video/backlight/pwm_bl.c >> > index 909a686..6cf6109 100644 >> > --- a/drivers/video/backlight/pwm_bl.c >> > +++ b/drivers/video/backlight/pwm_bl.c >> > @@ -10,6 +10,7 @@ >> > * published by the Free Software Foundation. >> > */ >> > >> > +#include >> > #include >> > #include >> > #include >> > @@ -35,6 +36,8 @@ struct pwm_bl_data { >> > struct gpio_desc *enable_gpio; >> > unsigned int scale; >> > bool legacy; >> > + unsigned int post_pwm_on_delay; >> > + unsigned int pwm_off_delay; >> > int (*notify)(struct device *, >> > int brightness); >> > void (*notify_after)(struct device *, >> > @@ -56,6 +59,9 @@ static void pwm_backlight_power_on(struct pwm_bl_data >> *pb, int brightness) >> > >> > pwm_enable(pb->pwm); >> > >> > + if (pb->post_pwm_on_delay) >> > + msleep(pb->post_pwm_on_delay); >> > + >> > if (pb->enable_gpio) >> > gpiod_set_value_cansleep(pb->enable_gpio, 1); >> > >> > @@ -70,6 +76,9 @@ static void pwm_backlight_power_off(struct pwm_bl_data >> *pb) >> > if (pb->enable_gpio) >> > gpiod_set_value_cansleep(pb->enable_gpio, 0); >> > >> > + if (pb->pwm_off_delay) >> > + msleep(pb->pwm_off_delay); >> > + >> > pwm_config(pb->pwm, 0, pb->period); >> > pwm_disable(pb->pwm); >> > >> > @@ -175,6 +184,14 @@ static int pwm_backlight_parse_dt(struct device >> *dev, >> > data->max_brightness--; >> > } >> > >> > + /* >> > + * These values are optional and set as 0 by default, the out >> values >> > + * are modified only if a valid u32 value can be decoded. >> > + */ >> > + of_property_read_u32(node, "post-pwm-on-delay-ms", >> > + &data->post_pwm_on_delay); >> > + of_property_read_u32(node, "pwm-off-delay-ms", &data- >> >pwm_off_delay); >> > + >> > data->enable_gpio = -EINVAL; >> > return 0; >> > } >> > @@ -273,6 +290,8 @@ static int pwm_backlight_probe(struct >> platform_device *pdev) >> > pb->exit = data->exit; >> > pb->dev = &pdev->dev; >> > pb->enabled = false; >> > + pb->post_pwm_on_delay = data->post_pwm_on_delay; >> > + pb->pwm_off_delay = data->pwm_off_delay; >> > >> > pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable", >> > GPIOD_ASIS); >> > diff --git a/include/linux/pwm_backlight.h >> b/include/linux/pwm_backlight.h >> > index efdd922..62f59c4 100644 >> > --- a/include/linux/pwm_backlight.h >> > +++ b/include/linux/pwm_backlight.h >> > @@ -13,6 +13,8 @@ struct platform_pwm_backlight_data { >> > unsigned int lth_brightness; >> > unsigned int pwm_period_ns; >> > unsigned int *levels; >> > + unsigned int post_pwm_on_delay; >> > + unsigned int pwm_off_delay; >> > /* TODO remove once all users are switched to gpiod_* API */ >> > int enable_gpio; >> > int (*init)(struct device *dev); >> > > >