[PATCH v4 1/5] pwm-backlight: enable/disable the PWM before/after LCD enable toggle.

[PATCH v4 1/5] pwm-backlight: enable/disable the PWM before/after LCD enable toggle.

[Enric Balletbo i Serra]

Before this patch the enable signal was set before the PWM signal and
vice-versa on power off. This sequence is wrong, at least, it is on
the different panels datasheets that I checked, so I inverted the sequence
to follow the specs.

For reference the following panels have the mentioned sequence:
  - N133HSE-EA1 (Innolux)
  - N116BGE (Innolux)
  - N156BGE-L21 (Innolux)
  - B101EAN0 (Auo)
  - B101AW03 (Auo)
  - LTN101NT05 (Samsung)
  - CLAA101WA01A (Chunghwa)

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes since v3:
 - List the part numbers for the panel checked (Daniel Thompson)
Changes since v2:
 - Add this as a separate patch (Thierry Reding)
Changes since v1:
 - None

 drivers/video/backlight/pwm_bl.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 002f1ce..909a686 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -54,10 +54,11 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness)
 	if (err < 0)
 		dev_err(pb->dev, "failed to enable power supply\n");
 
+	pwm_enable(pb->pwm);
+
 	if (pb->enable_gpio)
 		gpiod_set_value_cansleep(pb->enable_gpio, 1);
 
-	pwm_enable(pb->pwm);
 	pb->enabled = true;
 }
 
@@ -66,12 +67,12 @@ static void pwm_backlight_power_off(struct pwm_bl_data *pb)
 	if (!pb->enabled)
 		return;
 
-	pwm_config(pb->pwm, 0, pb->period);
-	pwm_disable(pb->pwm);
-
 	if (pb->enable_gpio)
 		gpiod_set_value_cansleep(pb->enable_gpio, 0);
 
+	pwm_config(pb->pwm, 0, pb->period);
+	pwm_disable(pb->pwm);
+
 	regulator_disable(pb->power_supply);
 	pb->enabled = false;
 }
-- 
2.9.3

[PATCH v4 1/5] pwm-backlight: enable/disable the PWM before/after LCD enable toggle.

[Enric Balletbo i Serra]

Before this patch the enable signal was set before the PWM signal and
vice-versa on power off. This sequence is wrong, at least, it is on
the different panels datasheets that I checked, so I inverted the sequence
to follow the specs.

For reference the following panels have the mentioned sequence:
  - N133HSE-EA1 (Innolux)
  - N116BGE (Innolux)
  - N156BGE-L21 (Innolux)
  - B101EAN0 (Auo)
  - B101AW03 (Auo)
  - LTN101NT05 (Samsung)
  - CLAA101WA01A (Chunghwa)

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes since v3:
 - List the part numbers for the panel checked (Daniel Thompson)
Changes since v2:
 - Add this as a separate patch (Thierry Reding)
Changes since v1:
 - None

 drivers/video/backlight/pwm_bl.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 002f1ce..909a686 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -54,10 +54,11 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness)
 	if (err < 0)
 		dev_err(pb->dev, "failed to enable power supply\n");
 
+	pwm_enable(pb->pwm);
+
 	if (pb->enable_gpio)
 		gpiod_set_value_cansleep(pb->enable_gpio, 1);
 
-	pwm_enable(pb->pwm);
 	pb->enabled = true;
 }
 
@@ -66,12 +67,12 @@ static void pwm_backlight_power_off(struct pwm_bl_data *pb)
 	if (!pb->enabled)
 		return;
 
-	pwm_config(pb->pwm, 0, pb->period);
-	pwm_disable(pb->pwm);
-
 	if (pb->enable_gpio)
 		gpiod_set_value_cansleep(pb->enable_gpio, 0);
 
+	pwm_config(pb->pwm, 0, pb->period);
+	pwm_disable(pb->pwm);
+
 	regulator_disable(pb->power_supply);
 	pb->enabled = false;
 }
-- 
2.9.3

[PATCH v4 2/5] dt-bindings: pwm-backlight: add PWM delay proprieties.

[Enric Balletbo i Serra]

Hardware needs a delay between setting an initial (non-zero) PWM and
enabling the backlight using GPIO. The post-pwm-on-delay-ms specifies
this delay in milli seconds. Hardware also needs a delay between disabing
the backlight using GPIO and setting PWM value to 0. The pwm-off-delay-ms
is this delay in milli seconds.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
Based on the original Huang Lin <hl@rock-chips.com> work.

Changes since v3:
 - Replace us for ms.
 - Add Acked-by: Pavel Machek <pavel@ucw.cz>
Changes since v2:
 - Use separate properties (Rob Herring)
Changes since v1:
 - As suggested by Daniel Thompson
   - Do not assume power-on delay and power-off delay will be the same

 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..3108109 100644
--- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
@@ -17,6 +17,10 @@ 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])
+  - post-pwm-on-delay-ms: Delay in ms between setting an initial (non-zero) PWM
+                          and enabling the backlight using GPIO.
+  - pwm-off-delay-ms: Delay in ms between disabling the backlight using GPIO
+                      and setting PWM value to 0.
 
 [0]: Documentation/devicetree/bindings/pwm/pwm.txt
 [1]: Documentation/devicetree/bindings/gpio/gpio.txt
@@ -32,4 +36,6 @@ Example:
 
 		power-supply = <&vdd_bl_reg>;
 		enable-gpios = <&gpio 58 0>;
+		post-pwm-on-delay-ms = <10>;
+		pwm-off-delay-ms = <10>;
 	};
-- 
2.9.3

[PATCH v4 2/5] dt-bindings: pwm-backlight: add PWM delay proprieties.

[Enric Balletbo i Serra]

Hardware needs a delay between setting an initial (non-zero) PWM and
enabling the backlight using GPIO. The post-pwm-on-delay-ms specifies
this delay in milli seconds. Hardware also needs a delay between disabing
the backlight using GPIO and setting PWM value to 0. The pwm-off-delay-ms
is this delay in milli seconds.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
---
Based on the original Huang Lin <hl-TNX95d0MmH7DzftRWevZcw@public.gmane.org> work.

Changes since v3:
 - Replace us for ms.
 - Add Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
Changes since v2:
 - Use separate properties (Rob Herring)
Changes since v1:
 - As suggested by Daniel Thompson
   - Do not assume power-on delay and power-off delay will be the same

 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..3108109 100644
--- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
@@ -17,6 +17,10 @@ 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])
+  - post-pwm-on-delay-ms: Delay in ms between setting an initial (non-zero) PWM
+                          and enabling the backlight using GPIO.
+  - pwm-off-delay-ms: Delay in ms between disabling the backlight using GPIO
+                      and setting PWM value to 0.
 
 [0]: Documentation/devicetree/bindings/pwm/pwm.txt
 [1]: Documentation/devicetree/bindings/gpio/gpio.txt
@@ -32,4 +36,6 @@ Example:
 
 		power-supply = <&vdd_bl_reg>;
 		enable-gpios = <&gpio 58 0>;
+		post-pwm-on-delay-ms = <10>;
+		pwm-off-delay-ms = <10>;
 	};
-- 
2.9.3

[PATCH v4 2/5] dt-bindings: pwm-backlight: add PWM delay proprieties.

[Enric Balletbo i Serra]

Hardware needs a delay between setting an initial (non-zero) PWM and
enabling the backlight using GPIO. The post-pwm-on-delay-ms specifies
this delay in milli seconds. Hardware also needs a delay between disabing
the backlight using GPIO and setting PWM value to 0. The pwm-off-delay-ms
is this delay in milli seconds.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
Based on the original Huang Lin <hl@rock-chips.com> work.

Changes since v3:
 - Replace us for ms.
 - Add Acked-by: Pavel Machek <pavel@ucw.cz>
Changes since v2:
 - Use separate properties (Rob Herring)
Changes since v1:
 - As suggested by Daniel Thompson
   - Do not assume power-on delay and power-off delay will be the same

 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..3108109 100644
--- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
@@ -17,6 +17,10 @@ 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])
+  - post-pwm-on-delay-ms: Delay in ms between setting an initial (non-zero) PWM
+                          and enabling the backlight using GPIO.
+  - pwm-off-delay-ms: Delay in ms between disabling the backlight using GPIO
+                      and setting PWM value to 0.
 
 [0]: Documentation/devicetree/bindings/pwm/pwm.txt
 [1]: Documentation/devicetree/bindings/gpio/gpio.txt
@@ -32,4 +36,6 @@ Example:
 
 		power-supply = <&vdd_bl_reg>;
 		enable-gpios = <&gpio 58 0>;
+		post-pwm-on-delay-ms = <10>;
+		pwm-off-delay-ms = <10>;
 	};
-- 
2.9.3

[PATCH v4 3/5] pwm-backlight: add support for PWM delays proprieties.

[Enric Balletbo i Serra]

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(+)

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);
-- 
2.9.3

[PATCH v4 3/5] pwm-backlight: add support for PWM delays proprieties.

[Enric Balletbo i Serra]

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(+)

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);
-- 
2.9.3

[PATCH v4 4/5] ARM: dts: rockchip: set PWM delay backlight settings for Veyron.

[Enric Balletbo i Serra]

For veyron the binding should provide both PWM timings, the delay between
you enable the PWM and set the enable signal, and the delay between you
disable the PWM signal and clear the enable signal. Update the binding
accordingly, in this case the panels connected to the veyron boards have
a symmetric power sequence, hence the same value is used.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes since v3:
 - Use new -ms names for proprieties.
Changes since v2:
 - Use new names for proprieties.
Changes since v1:
 - Add this new patch to fix current binding on veyron.

 arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
index d752a31..5a8c7f3 100644
--- a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
@@ -96,7 +96,8 @@
 		pinctrl-names = "default";
 		pinctrl-0 = <&bl_en>;
 		pwms = <&pwm0 0 1000000 0>;
-		pwm-delay-us = <10000>;
+		post-pwm-on-delay-ms = <10>;
+		pwm-off-delay-ms = <10>;
 	};
 
 	gpio-charger {
-- 
2.9.3

[PATCH v4 4/5] ARM: dts: rockchip: set PWM delay backlight settings for Veyron.

[Enric Balletbo i Serra]

For veyron the binding should provide both PWM timings, the delay between
you enable the PWM and set the enable signal, and the delay between you
disable the PWM signal and clear the enable signal. Update the binding
accordingly, in this case the panels connected to the veyron boards have
a symmetric power sequence, hence the same value is used.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes since v3:
 - Use new -ms names for proprieties.
Changes since v2:
 - Use new names for proprieties.
Changes since v1:
 - Add this new patch to fix current binding on veyron.

 arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
index d752a31..5a8c7f3 100644
--- a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
@@ -96,7 +96,8 @@
 		pinctrl-names = "default";
 		pinctrl-0 = <&bl_en>;
 		pwms = <&pwm0 0 1000000 0>;
-		pwm-delay-us = <10000>;
+		post-pwm-on-delay-ms = <10>;
+		pwm-off-delay-ms = <10>;
 	};
 
 	gpio-charger {
-- 
2.9.3

[PATCH v4 5/5] ARM: dts: rockchip: set PWM delay backlight settings for Minnie

[Enric Balletbo i Serra]

The minnie devices comes with an AUO B101EAN01 panel which is different
from default veyron devices, thus the power on/off timing sequence is
slightly different. The datasheet specifies a pwm delay of 200 ms, so
update the PMW delay proprieties accordingly.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Heiko,
 I'm not able to test this patch in a minnie device because I don't have
one, so could you do a quick try, please?

Changes since v3:
 - Use new -ms names for proprieties.
 - Fix the delay, should be 200ms instead of 20ms (Pavel)
Changes since v2:
 - Use new names for proprieties.
Changes since v1:
 - Add this new patch as minnie has differents timings

 arch/arm/boot/dts/rk3288-veyron-minnie.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-veyron-minnie.dts b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
index 544de60..4c5307e6 100644
--- a/arch/arm/boot/dts/rk3288-veyron-minnie.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
@@ -123,6 +123,8 @@
 			240 241 242 243 244 245 246 247
 			248 249 250 251 252 253 254 255>;
 	power-supply = <&backlight_regulator>;
+	post-pwm-on-delay-ms = <200>;
+	pwm-off-delay-ms = <200>;
 };
 
 &emmc {
-- 
2.9.3

[PATCH v4 5/5] ARM: dts: rockchip: set PWM delay backlight settings for Minnie

[Enric Balletbo i Serra]

The minnie devices comes with an AUO B101EAN01 panel which is different
from default veyron devices, thus the power on/off timing sequence is
slightly different. The datasheet specifies a pwm delay of 200 ms, so
update the PMW delay proprieties accordingly.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Heiko,
 I'm not able to test this patch in a minnie device because I don't have
one, so could you do a quick try, please?

Changes since v3:
 - Use new -ms names for proprieties.
 - Fix the delay, should be 200ms instead of 20ms (Pavel)
Changes since v2:
 - Use new names for proprieties.
Changes since v1:
 - Add this new patch as minnie has differents timings

 arch/arm/boot/dts/rk3288-veyron-minnie.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-veyron-minnie.dts b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
index 544de60..4c5307e6 100644
--- a/arch/arm/boot/dts/rk3288-veyron-minnie.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
@@ -123,6 +123,8 @@
 			240 241 242 243 244 245 246 247
 			248 249 250 251 252 253 254 255>;
 	power-supply = <&backlight_regulator>;
+	post-pwm-on-delay-ms = <200>;
+	pwm-off-delay-ms = <200>;
 };
 
 &emmc {
-- 
2.9.3

Re: [PATCH v4 3/5] pwm-backlight: add support for PWM delays proprieties.

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 801 bytes --]

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.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v4 3/5] pwm-backlight: add support for PWM delays proprieties.

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 801 bytes --]

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.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v4 1/5] pwm-backlight: enable/disable the PWM before/after LCD enable toggle.

[Daniel Thompson]

On 21/07/17 11:48, Enric Balletbo i Serra wrote:
> Before this patch the enable signal was set before the PWM signal and
> vice-versa on power off. This sequence is wrong, at least, it is on
> the different panels datasheets that I checked, so I inverted the sequence
> to follow the specs.
> 
> For reference the following panels have the mentioned sequence:
>    - N133HSE-EA1 (Innolux)
>    - N116BGE (Innolux)
>    - N156BGE-L21 (Innolux)
>    - B101EAN0 (Auo)
>    - B101AW03 (Auo)
>    - LTN101NT05 (Samsung)
>    - CLAA101WA01A (Chunghwa)
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Acked-by: Daniel Thompson <daniel.thompson@linaro.org>

> ---
> Changes since v3:
>   - List the part numbers for the panel checked (Daniel Thompson)
> Changes since v2:
>   - Add this as a separate patch (Thierry Reding)
> Changes since v1:
>   - None
> 
>   drivers/video/backlight/pwm_bl.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index 002f1ce..909a686 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -54,10 +54,11 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness)
>   	if (err < 0)
>   		dev_err(pb->dev, "failed to enable power supply\n");
>   
> +	pwm_enable(pb->pwm);
> +
>   	if (pb->enable_gpio)
>   		gpiod_set_value_cansleep(pb->enable_gpio, 1);
>   
> -	pwm_enable(pb->pwm);
>   	pb->enabled = true;
>   }
>   
> @@ -66,12 +67,12 @@ static void pwm_backlight_power_off(struct pwm_bl_data *pb)
>   	if (!pb->enabled)
>   		return;
>   
> -	pwm_config(pb->pwm, 0, pb->period);
> -	pwm_disable(pb->pwm);
> -
>   	if (pb->enable_gpio)
>   		gpiod_set_value_cansleep(pb->enable_gpio, 0);
>   
> +	pwm_config(pb->pwm, 0, pb->period);
> +	pwm_disable(pb->pwm);
> +
>   	regulator_disable(pb->power_supply);
>   	pb->enabled = false;
>   }
> 

Re: [PATCH v4 1/5] pwm-backlight: enable/disable the PWM before/after LCD enable toggle.

[Daniel Thompson]

On 21/07/17 11:48, Enric Balletbo i Serra wrote:
> Before this patch the enable signal was set before the PWM signal and
> vice-versa on power off. This sequence is wrong, at least, it is on
> the different panels datasheets that I checked, so I inverted the sequence
> to follow the specs.
> 
> For reference the following panels have the mentioned sequence:
>    - N133HSE-EA1 (Innolux)
>    - N116BGE (Innolux)
>    - N156BGE-L21 (Innolux)
>    - B101EAN0 (Auo)
>    - B101AW03 (Auo)
>    - LTN101NT05 (Samsung)
>    - CLAA101WA01A (Chunghwa)
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Acked-by: Daniel Thompson <daniel.thompson@linaro.org>

> ---
> Changes since v3:
>   - List the part numbers for the panel checked (Daniel Thompson)
> Changes since v2:
>   - Add this as a separate patch (Thierry Reding)
> Changes since v1:
>   - None
> 
>   drivers/video/backlight/pwm_bl.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index 002f1ce..909a686 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -54,10 +54,11 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness)
>   	if (err < 0)
>   		dev_err(pb->dev, "failed to enable power supply\n");
>   
> +	pwm_enable(pb->pwm);
> +
>   	if (pb->enable_gpio)
>   		gpiod_set_value_cansleep(pb->enable_gpio, 1);
>   
> -	pwm_enable(pb->pwm);
>   	pb->enabled = true;
>   }
>   
> @@ -66,12 +67,12 @@ static void pwm_backlight_power_off(struct pwm_bl_data *pb)
>   	if (!pb->enabled)
>   		return;
>   
> -	pwm_config(pb->pwm, 0, pb->period);
> -	pwm_disable(pb->pwm);
> -
>   	if (pb->enable_gpio)
>   		gpiod_set_value_cansleep(pb->enable_gpio, 0);
>   
> +	pwm_config(pb->pwm, 0, pb->period);
> +	pwm_disable(pb->pwm);
> +
>   	regulator_disable(pb->power_supply);
>   	pb->enabled = false;
>   }
> 

Re: [PATCH v4 2/5] dt-bindings: pwm-backlight: add PWM delay proprieties.

[Daniel Thompson]

On 21/07/17 11:48, Enric Balletbo i Serra wrote:
> Hardware needs a delay between setting an initial (non-zero) PWM and
> enabling the backlight using GPIO. The post-pwm-on-delay-ms specifies
> this delay in milli seconds. Hardware also needs a delay between disabing
> the backlight using GPIO and setting PWM value to 0. The pwm-off-delay-ms
> is this delay in milli seconds.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
> Based on the original Huang Lin <hl@rock-chips.com> work.
> 
> Changes since v3:
>   - Replace us for ms.
>   - Add Acked-by: Pavel Machek <pavel@ucw.cz>
> Changes since v2:
>   - Use separate properties (Rob Herring)
> Changes since v1:
>   - As suggested by Daniel Thompson
>     - Do not assume power-on delay and power-off delay will be the same
> 
>   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..3108109 100644
> --- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
> +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
> @@ -17,6 +17,10 @@ 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])
> +  - post-pwm-on-delay-ms: Delay in ms between setting an initial (non-zero) PWM
> +                          and enabling the backlight using GPIO.
> +  - pwm-off-delay-ms: Delay in ms between disabling the backlight using GPIO
> +                      and setting PWM value to 0.

Whilst it is strictly true that the delay you added to the driver is 
currently between disabling the backlight and setting PWM value to 0 I 
don't think the action should be described at this level of detail in 
the DT bindings.

The semantic action that is being performed is "stopping the PWM". This 
is currently implemented by setting the duty cycle to 0 and then calling 
disable but that could change (especially so since the current behavior 
looks asymmetric versus the enable sequence).


Daniel.


>   
>   [0]: Documentation/devicetree/bindings/pwm/pwm.txt
>   [1]: Documentation/devicetree/bindings/gpio/gpio.txt
> @@ -32,4 +36,6 @@ Example:
>   
>   		power-supply = <&vdd_bl_reg>;
>   		enable-gpios = <&gpio 58 0>;
> +		post-pwm-on-delay-ms = <10>;
> +		pwm-off-delay-ms = <10>;
>   	};
> 

Re: [PATCH v4 2/5] dt-bindings: pwm-backlight: add PWM delay proprieties.

[Daniel Thompson]

On 21/07/17 11:48, Enric Balletbo i Serra wrote:
> Hardware needs a delay between setting an initial (non-zero) PWM and
> enabling the backlight using GPIO. The post-pwm-on-delay-ms specifies
> this delay in milli seconds. Hardware also needs a delay between disabing
> the backlight using GPIO and setting PWM value to 0. The pwm-off-delay-ms
> is this delay in milli seconds.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
> Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> ---
> Based on the original Huang Lin <hl-TNX95d0MmH7DzftRWevZcw@public.gmane.org> work.
> 
> Changes since v3:
>   - Replace us for ms.
>   - Add Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> Changes since v2:
>   - Use separate properties (Rob Herring)
> Changes since v1:
>   - As suggested by Daniel Thompson
>     - Do not assume power-on delay and power-off delay will be the same
> 
>   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..3108109 100644
> --- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
> +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
> @@ -17,6 +17,10 @@ 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])
> +  - post-pwm-on-delay-ms: Delay in ms between setting an initial (non-zero) PWM
> +                          and enabling the backlight using GPIO.
> +  - pwm-off-delay-ms: Delay in ms between disabling the backlight using GPIO
> +                      and setting PWM value to 0.

Whilst it is strictly true that the delay you added to the driver is 
currently between disabling the backlight and setting PWM value to 0 I 
don't think the action should be described at this level of detail in 
the DT bindings.

The semantic action that is being performed is "stopping the PWM". This 
is currently implemented by setting the duty cycle to 0 and then calling 
disable but that could change (especially so since the current behavior 
looks asymmetric versus the enable sequence).


Daniel.


>   
>   [0]: Documentation/devicetree/bindings/pwm/pwm.txt
>   [1]: Documentation/devicetree/bindings/gpio/gpio.txt
> @@ -32,4 +36,6 @@ Example:
>   
>   		power-supply = <&vdd_bl_reg>;
>   		enable-gpios = <&gpio 58 0>;
> +		post-pwm-on-delay-ms = <10>;
> +		pwm-off-delay-ms = <10>;
>   	};
> 

Re: [PATCH v4 2/5] dt-bindings: pwm-backlight: add PWM delay proprieties.

[Daniel Thompson]

On 21/07/17 11:48, Enric Balletbo i Serra wrote:
> Hardware needs a delay between setting an initial (non-zero) PWM and
> enabling the backlight using GPIO. The post-pwm-on-delay-ms specifies
> this delay in milli seconds. Hardware also needs a delay between disabing
> the backlight using GPIO and setting PWM value to 0. The pwm-off-delay-ms
> is this delay in milli seconds.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
> Based on the original Huang Lin <hl@rock-chips.com> work.
> 
> Changes since v3:
>   - Replace us for ms.
>   - Add Acked-by: Pavel Machek <pavel@ucw.cz>
> Changes since v2:
>   - Use separate properties (Rob Herring)
> Changes since v1:
>   - As suggested by Daniel Thompson
>     - Do not assume power-on delay and power-off delay will be the same
> 
>   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..3108109 100644
> --- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
> +++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
> @@ -17,6 +17,10 @@ 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])
> +  - post-pwm-on-delay-ms: Delay in ms between setting an initial (non-zero) PWM
> +                          and enabling the backlight using GPIO.
> +  - pwm-off-delay-ms: Delay in ms between disabling the backlight using GPIO
> +                      and setting PWM value to 0.

Whilst it is strictly true that the delay you added to the driver is 
currently between disabling the backlight and setting PWM value to 0 I 
don't think the action should be described at this level of detail in 
the DT bindings.

The semantic action that is being performed is "stopping the PWM". This 
is currently implemented by setting the duty cycle to 0 and then calling 
disable but that could change (especially so since the current behavior 
looks asymmetric versus the enable sequence).


Daniel.


>   
>   [0]: Documentation/devicetree/bindings/pwm/pwm.txt
>   [1]: Documentation/devicetree/bindings/gpio/gpio.txt
> @@ -32,4 +36,6 @@ Example:
>   
>   		power-supply = <&vdd_bl_reg>;
>   		enable-gpios = <&gpio 58 0>;
> +		post-pwm-on-delay-ms = <10>;
> +		pwm-off-delay-ms = <10>;
>   	};
> 

Re: [PATCH v4 3/5] pwm-backlight: add support for PWM delays proprieties.

[Daniel Thompson]

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);
> 

Re: [PATCH v4 3/5] pwm-backlight: add support for PWM delays proprieties.

[Daniel Thompson]

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);
> 

Re: [PATCH v4 2/5] dt-bindings: pwm-backlight: add PWM delay proprieties.

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2788 bytes --]

On Mon 2017-07-24 16:21:44, Daniel Thompson wrote:
> On 21/07/17 11:48, Enric Balletbo i Serra wrote:
> >Hardware needs a delay between setting an initial (non-zero) PWM and
> >enabling the backlight using GPIO. The post-pwm-on-delay-ms specifies
> >this delay in milli seconds. Hardware also needs a delay between disabing
> >the backlight using GPIO and setting PWM value to 0. The pwm-off-delay-ms
> >is this delay in milli seconds.
> >
> >Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >Acked-by: Pavel Machek <pavel@ucw.cz>
> >---
> >Based on the original Huang Lin <hl@rock-chips.com> work.
> >
> >Changes since v3:
> >  - Replace us for ms.
> >  - Add Acked-by: Pavel Machek <pavel@ucw.cz>
> >Changes since v2:
> >  - Use separate properties (Rob Herring)
> >Changes since v1:
> >  - As suggested by Daniel Thompson
> >    - Do not assume power-on delay and power-off delay will be the same
> >
> >  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..3108109 100644
> >--- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
> >+++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
> >@@ -17,6 +17,10 @@ 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])
> >+  - post-pwm-on-delay-ms: Delay in ms between setting an initial (non-zero) PWM
> >+                          and enabling the backlight using GPIO.
> >+  - pwm-off-delay-ms: Delay in ms between disabling the backlight using GPIO
> >+                      and setting PWM value to 0.
> 
> Whilst it is strictly true that the delay you added to the driver is
> currently between disabling the backlight and setting PWM value to 0 I don't
> think the action should be described at this level of detail in the DT
> bindings.
> 
> The semantic action that is being performed is "stopping the PWM". This is
> currently implemented by setting the duty cycle to 0 and then calling
> disable but that could change (especially so since the current behavior
> looks asymmetric versus the enable sequence).

Well, datasheet say the delay is required between given actions, so
yes, I'd say that belongs in the device tree at this level of detail.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v4 2/5] dt-bindings: pwm-backlight: add PWM delay proprieties.

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2788 bytes --]

On Mon 2017-07-24 16:21:44, Daniel Thompson wrote:
> On 21/07/17 11:48, Enric Balletbo i Serra wrote:
> >Hardware needs a delay between setting an initial (non-zero) PWM and
> >enabling the backlight using GPIO. The post-pwm-on-delay-ms specifies
> >this delay in milli seconds. Hardware also needs a delay between disabing
> >the backlight using GPIO and setting PWM value to 0. The pwm-off-delay-ms
> >is this delay in milli seconds.
> >
> >Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >Acked-by: Pavel Machek <pavel@ucw.cz>
> >---
> >Based on the original Huang Lin <hl@rock-chips.com> work.
> >
> >Changes since v3:
> >  - Replace us for ms.
> >  - Add Acked-by: Pavel Machek <pavel@ucw.cz>
> >Changes since v2:
> >  - Use separate properties (Rob Herring)
> >Changes since v1:
> >  - As suggested by Daniel Thompson
> >    - Do not assume power-on delay and power-off delay will be the same
> >
> >  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..3108109 100644
> >--- a/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
> >+++ b/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.txt
> >@@ -17,6 +17,10 @@ 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])
> >+  - post-pwm-on-delay-ms: Delay in ms between setting an initial (non-zero) PWM
> >+                          and enabling the backlight using GPIO.
> >+  - pwm-off-delay-ms: Delay in ms between disabling the backlight using GPIO
> >+                      and setting PWM value to 0.
> 
> Whilst it is strictly true that the delay you added to the driver is
> currently between disabling the backlight and setting PWM value to 0 I don't
> think the action should be described at this level of detail in the DT
> bindings.
> 
> The semantic action that is being performed is "stopping the PWM". This is
> currently implemented by setting the duty cycle to 0 and then calling
> disable but that could change (especially so since the current behavior
> looks asymmetric versus the enable sequence).

Well, datasheet say the delay is required between given actions, so
yes, I'd say that belongs in the device tree at this level of detail.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v4 1/5] pwm-backlight: enable/disable the PWM before/after LCD enable toggle.

[Jingoo Han]

On Monday, July 24, 2017 11:14 AM, Daniel Thompson wrote:
> On 21/07/17 11:48, Enric Balletbo i Serra wrote:
> > Before this patch the enable signal was set before the PWM signal and
> > vice-versa on power off. This sequence is wrong, at least, it is on
> > the different panels datasheets that I checked, so I inverted the
> sequence
> > to follow the specs.
> >
> > For reference the following panels have the mentioned sequence:
> >    - N133HSE-EA1 (Innolux)
> >    - N116BGE (Innolux)
> >    - N156BGE-L21 (Innolux)
> >    - B101EAN0 (Auo)
> >    - B101AW03 (Auo)
> >    - LTN101NT05 (Samsung)
> >    - CLAA101WA01A (Chunghwa)
> >
> > 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:
> >   - List the part numbers for the panel checked (Daniel Thompson)
> > Changes since v2:
> >   - Add this as a separate patch (Thierry Reding)
> > Changes since v1:
> >   - None
> >
> >   drivers/video/backlight/pwm_bl.c | 9 +++++----
> >   1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/video/backlight/pwm_bl.c
> b/drivers/video/backlight/pwm_bl.c
> > index 002f1ce..909a686 100644
> > --- a/drivers/video/backlight/pwm_bl.c
> > +++ b/drivers/video/backlight/pwm_bl.c
> > @@ -54,10 +54,11 @@ static void pwm_backlight_power_on(struct
> pwm_bl_data *pb, int brightness)
> >   	if (err < 0)
> >   		dev_err(pb->dev, "failed to enable power supply\n");
> >
> > +	pwm_enable(pb->pwm);
> > +
> >   	if (pb->enable_gpio)
> >   		gpiod_set_value_cansleep(pb->enable_gpio, 1);
> >
> > -	pwm_enable(pb->pwm);
> >   	pb->enabled = true;
> >   }
> >
> > @@ -66,12 +67,12 @@ static void pwm_backlight_power_off(struct
> pwm_bl_data *pb)
> >   	if (!pb->enabled)
> >   		return;
> >
> > -	pwm_config(pb->pwm, 0, pb->period);
> > -	pwm_disable(pb->pwm);
> > -
> >   	if (pb->enable_gpio)
> >   		gpiod_set_value_cansleep(pb->enable_gpio, 0);
> >
> > +	pwm_config(pb->pwm, 0, pb->period);
> > +	pwm_disable(pb->pwm);
> > +
> >   	regulator_disable(pb->power_supply);
> >   	pb->enabled = false;
> >   }
> >

Re: [PATCH v4 1/5] pwm-backlight: enable/disable the PWM before/after LCD enable toggle.

[Jingoo Han]

On Monday, July 24, 2017 11:14 AM, Daniel Thompson wrote:
> On 21/07/17 11:48, Enric Balletbo i Serra wrote:
> > Before this patch the enable signal was set before the PWM signal and
> > vice-versa on power off. This sequence is wrong, at least, it is on
> > the different panels datasheets that I checked, so I inverted the
> sequence
> > to follow the specs.
> >
> > For reference the following panels have the mentioned sequence:
> >    - N133HSE-EA1 (Innolux)
> >    - N116BGE (Innolux)
> >    - N156BGE-L21 (Innolux)
> >    - B101EAN0 (Auo)
> >    - B101AW03 (Auo)
> >    - LTN101NT05 (Samsung)
> >    - CLAA101WA01A (Chunghwa)
> >
> > 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:
> >   - List the part numbers for the panel checked (Daniel Thompson)
> > Changes since v2:
> >   - Add this as a separate patch (Thierry Reding)
> > Changes since v1:
> >   - None
> >
> >   drivers/video/backlight/pwm_bl.c | 9 +++++----
> >   1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/video/backlight/pwm_bl.c
> b/drivers/video/backlight/pwm_bl.c
> > index 002f1ce..909a686 100644
> > --- a/drivers/video/backlight/pwm_bl.c
> > +++ b/drivers/video/backlight/pwm_bl.c
> > @@ -54,10 +54,11 @@ static void pwm_backlight_power_on(struct
> pwm_bl_data *pb, int brightness)
> >   	if (err < 0)
> >   		dev_err(pb->dev, "failed to enable power supply\n");
> >
> > +	pwm_enable(pb->pwm);
> > +
> >   	if (pb->enable_gpio)
> >   		gpiod_set_value_cansleep(pb->enable_gpio, 1);
> >
> > -	pwm_enable(pb->pwm);
> >   	pb->enabled = true;
> >   }
> >
> > @@ -66,12 +67,12 @@ static void pwm_backlight_power_off(struct
> pwm_bl_data *pb)
> >   	if (!pb->enabled)
> >   		return;
> >
> > -	pwm_config(pb->pwm, 0, pb->period);
> > -	pwm_disable(pb->pwm);
> > -
> >   	if (pb->enable_gpio)
> >   		gpiod_set_value_cansleep(pb->enable_gpio, 0);
> >
> > +	pwm_config(pb->pwm, 0, pb->period);
> > +	pwm_disable(pb->pwm);
> > +
> >   	regulator_disable(pb->power_supply);
> >   	pb->enabled = false;
> >   }
> >

Re: [PATCH v4 1/5] pwm-backlight: enable/disable the PWM before/after LCD enable toggle.

[Jingoo Han]

On Monday, July 24, 2017 11:14 AM, Daniel Thompson wrote:
> On 21/07/17 11:48, Enric Balletbo i Serra wrote:
> > Before this patch the enable signal was set before the PWM signal and
> > vice-versa on power off. This sequence is wrong, at least, it is on
> > the different panels datasheets that I checked, so I inverted the
> sequence
> > to follow the specs.
> >
> > For reference the following panels have the mentioned sequence:
> >    - N133HSE-EA1 (Innolux)
> >    - N116BGE (Innolux)
> >    - N156BGE-L21 (Innolux)
> >    - B101EAN0 (Auo)
> >    - B101AW03 (Auo)
> >    - LTN101NT05 (Samsung)
> >    - CLAA101WA01A (Chunghwa)
> >
> > 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:
> >   - List the part numbers for the panel checked (Daniel Thompson)
> > Changes since v2:
> >   - Add this as a separate patch (Thierry Reding)
> > Changes since v1:
> >   - None
> >
> >   drivers/video/backlight/pwm_bl.c | 9 +++++----
> >   1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/video/backlight/pwm_bl.c
> b/drivers/video/backlight/pwm_bl.c
> > index 002f1ce..909a686 100644
> > --- a/drivers/video/backlight/pwm_bl.c
> > +++ b/drivers/video/backlight/pwm_bl.c
> > @@ -54,10 +54,11 @@ static void pwm_backlight_power_on(struct
> pwm_bl_data *pb, int brightness)
> >   	if (err < 0)
> >   		dev_err(pb->dev, "failed to enable power supply\n");
> >
> > +	pwm_enable(pb->pwm);
> > +
> >   	if (pb->enable_gpio)
> >   		gpiod_set_value_cansleep(pb->enable_gpio, 1);
> >
> > -	pwm_enable(pb->pwm);
> >   	pb->enabled = true;
> >   }
> >
> > @@ -66,12 +67,12 @@ static void pwm_backlight_power_off(struct
> pwm_bl_data *pb)
> >   	if (!pb->enabled)
> >   		return;
> >
> > -	pwm_config(pb->pwm, 0, pb->period);
> > -	pwm_disable(pb->pwm);
> > -
> >   	if (pb->enable_gpio)
> >   		gpiod_set_value_cansleep(pb->enable_gpio, 0);
> >
> > +	pwm_config(pb->pwm, 0, pb->period);
> > +	pwm_disable(pb->pwm);
> > +
> >   	regulator_disable(pb->power_supply);
> >   	pb->enabled = false;
> >   }
> >

Re: [PATCH v4 3/5] pwm-backlight: add support for PWM delays proprieties.

[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 <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);
> >

Re: [PATCH v4 3/5] pwm-backlight: add support for PWM delays proprieties.

[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 <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);
> >

Re: [PATCH v4 3/5] pwm-backlight: add support for PWM delays proprieties.

[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 <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);
> >

Re: [PATCH v4 3/5] pwm-backlight: add support for PWM delays proprieties.

[Enric Balletbo Serra]

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);
>> >
>
>

Re: [PATCH v4 3/5] pwm-backlight: add support for PWM delays proprieties.

[Enric Balletbo Serra]

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);
>> >
>
>

Re: [PATCH v4 3/5] pwm-backlight: add support for PWM delays proprieties.

[Lee Jones]

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.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v4 3/5] pwm-backlight: add support for PWM delays proprieties.

[Lee Jones]

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.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v4 3/5] pwm-backlight: add support for PWM delays proprieties.

[Enric Balletbo Serra]

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

Re: [PATCH v4 3/5] pwm-backlight: add support for PWM delays proprieties.

[Enric Balletbo Serra]

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