| Message ID | 20170721104813.5389-3-enric.balletbo@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri 2017-07-21 12:48:11, 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 <enric.balletbo@collabora.com> For 3-5: Acked-by: Pavel Machek <pavel@ucw.cz> I guess you probably meant "properties" and I guess english could be improved in the comments, but... this is good enough and not worth more iterations.
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 <enric.balletbo@collabora.com> Acked-by: Daniel Thompson <daniel.thompson@linaro.org> > --- > 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 <linux/delay.h> > #include <linux/gpio/consumer.h> > #include <linux/gpio.h> > #include <linux/module.h> > @@ -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); >
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 <enric.balletbo@collabora.com> > > Acked-by: Daniel Thompson <daniel.thompson@linaro.org> Acked-by: Jingoo Han <jingoohan1@gmail.com> 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 <linux/delay.h> > > #include <linux/gpio/consumer.h> > > #include <linux/gpio.h> > > #include <linux/module.h> > > @@ -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); > >
Hi, 2017-07-26 19:01 GMT+02:00 Jingoo Han <jingoohan1@gmail.com>: > 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 <enric.balletbo@collabora.com> >> >> Acked-by: Daniel Thompson <daniel.thompson@linaro.org> > > Acked-by: Jingoo Han <jingoohan1@gmail.com> > 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 <linux/delay.h> >> > #include <linux/gpio/consumer.h> >> > #include <linux/gpio.h> >> > #include <linux/module.h> >> > @@ -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); >> > > >
On Fri, 01 Dec 2017, Enric Balletbo Serra wrote: > 2017-07-26 19:01 GMT+02:00 Jingoo Han <jingoohan1@gmail.com>: > > 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 <enric.balletbo@collabora.com> > >> > >> Acked-by: Daniel Thompson <daniel.thompson@linaro.org> > > > > Acked-by: Jingoo Han <jingoohan1@gmail.com> > > > > 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. Looks like you are still missing Acks on some of the patches and have questions outstanding. Best thing to do is rebase and resubmit with all of the Acks you've collected applied.
Hi, 2017-12-01 11:54 GMT+01:00 Lee Jones <lee.jones@linaro.org>: > On Fri, 01 Dec 2017, Enric Balletbo Serra wrote: >> 2017-07-26 19:01 GMT+02:00 Jingoo Han <jingoohan1@gmail.com>: >> > 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 <enric.balletbo@collabora.com> >> >> >> >> Acked-by: Daniel Thompson <daniel.thompson@linaro.org> >> > >> > Acked-by: Jingoo Han <jingoohan1@gmail.com> >> > >> >> 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. > > Looks like you are still missing Acks on some of the patches and have > questions outstanding. > > Best thing to do is rebase and resubmit with all of the Acks you've > collected applied. > Thanks will do that then. > -- > Lee Jones > Linaro STMicroelectronics Landing Team Lead > Linaro.org │ Open source software for ARM SoCs > Follow Linaro: Facebook | Twitter | Blog
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 <linux/delay.h> #include <linux/gpio/consumer.h> #include <linux/gpio.h> #include <linux/module.h> @@ -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);
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 <enric.balletbo@collabora.com> --- 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(+)