* [PATCH 01/10] pwm-backlight: Refactor backlight power on/off
2013-09-23 21:40 [PATCH 00/10] pwm-backlight: Add GPIO and power supply support Thierry Reding
@ 2013-09-23 21:40 ` Thierry Reding
[not found] ` <1379972467-11243-2-git-send-email-treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-09-23 21:40 ` [PATCH 02/10] pwm-backlight: Add optional enable GPIO Thierry Reding
` (8 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2013-09-23 21:40 UTC (permalink / raw)
To: Thierry Reding
Cc: Mark Rutland, devicetree, Kukjin Kim, Eric Miao, Pawel Moll,
Ian Campbell, Tony Lindgren, Stephen Warren, Magnus Damm,
linux-kernel, Rob Herring, linux-samsung-soc, linux-omap,
Simon Horman, linux-pwm, Haojian Zhuang, Ben Dooks, linux-sh,
openezx-devel, Guan Xuetao, linux-arm-kernel
In preparation for adding an optional regulator and enable GPIO to the
driver, split the power on and power off sequences into separate
functions to reduce code duplication at the multiple call sites.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/video/backlight/pwm_bl.c | 59 ++++++++++++++++++++++++----------------
1 file changed, 36 insertions(+), 23 deletions(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 36db5d9..8a49b30 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -35,6 +35,30 @@ struct pwm_bl_data {
void (*exit)(struct device *);
};
+static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness,
+ int max)
+{
+ int duty_cycle, err;
+
+ if (pb->levels) {
+ duty_cycle = pb->levels[brightness];
+ max = pb->levels[max];
+ } else {
+ duty_cycle = brightness;
+ }
+
+ duty_cycle = (duty_cycle * (pb->period - pb->lth_brightness) / max) +
+ pb->lth_brightness;
+
+ pwm_config(pb->pwm, duty_cycle, pb->period);
+ pwm_enable(pb->pwm);
+}
+
+static void pwm_backlight_power_off(struct pwm_bl_data *pb)
+{
+ pwm_disable(pb->pwm);
+}
+
static int pwm_backlight_update_status(struct backlight_device *bl)
{
struct pwm_bl_data *pb = bl_get_data(bl);
@@ -49,24 +73,10 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
if (pb->notify)
brightness = pb->notify(pb->dev, brightness);
- if (brightness == 0) {
- pwm_config(pb->pwm, 0, pb->period);
- pwm_disable(pb->pwm);
- } else {
- int duty_cycle;
-
- if (pb->levels) {
- duty_cycle = pb->levels[brightness];
- max = pb->levels[max];
- } else {
- duty_cycle = brightness;
- }
-
- duty_cycle = pb->lth_brightness +
- (duty_cycle * (pb->period - pb->lth_brightness) / max);
- pwm_config(pb->pwm, duty_cycle, pb->period);
- pwm_enable(pb->pwm);
- }
+ if (brightness > 0)
+ pwm_backlight_power_on(pb, brightness, max);
+ else
+ pwm_backlight_power_off(pb);
if (pb->notify_after)
pb->notify_after(pb->dev, brightness);
@@ -267,10 +277,11 @@ static int pwm_backlight_remove(struct platform_device *pdev)
struct pwm_bl_data *pb = bl_get_data(bl);
backlight_device_unregister(bl);
- pwm_config(pb->pwm, 0, pb->period);
- pwm_disable(pb->pwm);
+ pwm_backlight_power_off(pb);
+
if (pb->exit)
pb->exit(&pdev->dev);
+
return 0;
}
@@ -282,10 +293,12 @@ static int pwm_backlight_suspend(struct device *dev)
if (pb->notify)
pb->notify(pb->dev, 0);
- pwm_config(pb->pwm, 0, pb->period);
- pwm_disable(pb->pwm);
+
+ pwm_backlight_power_off(pb);
+
if (pb->notify_after)
pb->notify_after(pb->dev, 0);
+
return 0;
}
@@ -294,6 +307,7 @@ static int pwm_backlight_resume(struct device *dev)
struct backlight_device *bl = dev_get_drvdata(dev);
backlight_update_status(bl);
+
return 0;
}
#endif
@@ -317,4 +331,3 @@ module_platform_driver(pwm_backlight_driver);
MODULE_DESCRIPTION("PWM based Backlight Driver");
MODULE_LICENSE("GPL");
MODULE_ALIAS("platform:pwm-backlight");
-
--
1.8.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 02/10] pwm-backlight: Add optional enable GPIO
2013-09-23 21:40 [PATCH 00/10] pwm-backlight: Add GPIO and power supply support Thierry Reding
2013-09-23 21:40 ` [PATCH 01/10] pwm-backlight: Refactor backlight power on/off Thierry Reding
@ 2013-09-23 21:40 ` Thierry Reding
2013-09-23 21:41 ` [PATCH 03/10] ARM: OMAP: Initialize PWM backlight enable_gpio field Thierry Reding
` (7 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Thierry Reding @ 2013-09-23 21:40 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks,
Kukjin Kim, Simon Horman, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel
To support a wider variety of backlight setups, introduce an optional
enable GPIO. Legacy users of the platform data already have a means of
supporting GPIOs by using the .init(), .exit() and .notify() hooks. DT
users however cannot use those, so an alternative method is required.
In order to ease the introduction of the optional enable GPIO, make it
available in the platform data first, so that existing users can be
converted. Once that has happened a second patch will add code to make
use of it in the driver.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
include/linux/pwm_backlight.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/pwm_backlight.h b/include/linux/pwm_backlight.h
index 56f4a86..2de2e27 100644
--- a/include/linux/pwm_backlight.h
+++ b/include/linux/pwm_backlight.h
@@ -6,6 +6,9 @@
#include <linux/backlight.h>
+/* TODO: convert to gpiod_*() API once it has been merged */
+#define PWM_BACKLIGHT_GPIO_ACTIVE_LOW (1 << 0)
+
struct platform_pwm_backlight_data {
int pwm_id;
unsigned int max_brightness;
@@ -13,6 +16,8 @@ struct platform_pwm_backlight_data {
unsigned int lth_brightness;
unsigned int pwm_period_ns;
unsigned int *levels;
+ int enable_gpio;
+ unsigned long enable_gpio_flags;
int (*init)(struct device *dev);
int (*notify)(struct device *dev, int brightness);
void (*notify_after)(struct device *dev, int brightness);
--
1.8.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 03/10] ARM: OMAP: Initialize PWM backlight enable_gpio field
2013-09-23 21:40 [PATCH 00/10] pwm-backlight: Add GPIO and power supply support Thierry Reding
2013-09-23 21:40 ` [PATCH 01/10] pwm-backlight: Refactor backlight power on/off Thierry Reding
2013-09-23 21:40 ` [PATCH 02/10] pwm-backlight: Add optional enable GPIO Thierry Reding
@ 2013-09-23 21:41 ` Thierry Reding
2013-09-23 21:41 ` [PATCH 04/10] ARM: pxa: " Thierry Reding
` (6 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Thierry Reding @ 2013-09-23 21:41 UTC (permalink / raw)
To: Thierry Reding
Cc: Mark Rutland, devicetree, Kukjin Kim, Eric Miao, Pawel Moll,
Ian Campbell, Tony Lindgren, Stephen Warren, Magnus Damm,
linux-kernel, Rob Herring, linux-samsung-soc, linux-omap,
Simon Horman, linux-pwm, Haojian Zhuang, Ben Dooks, linux-sh,
openezx-devel, Guan Xuetao, linux-arm-kernel
The GPIO API defines 0 as being a valid GPIO number, so this field needs
to be initialized explicitly.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
arch/arm/mach-omap2/board-zoom-peripherals.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-omap2/board-zoom-peripherals.c b/arch/arm/mach-omap2/board-zoom-peripherals.c
index a90375d..5ed2884 100644
--- a/arch/arm/mach-omap2/board-zoom-peripherals.c
+++ b/arch/arm/mach-omap2/board-zoom-peripherals.c
@@ -227,6 +227,7 @@ static struct platform_pwm_backlight_data zoom_backlight_data = {
.max_brightness = 127,
.dft_brightness = 127,
.pwm_period_ns = 7812500,
+ .enable_gpio = -1,
};
static struct platform_device zoom_backlight_pwm = {
--
1.8.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 04/10] ARM: pxa: Initialize PWM backlight enable_gpio field
2013-09-23 21:40 [PATCH 00/10] pwm-backlight: Add GPIO and power supply support Thierry Reding
` (2 preceding siblings ...)
2013-09-23 21:41 ` [PATCH 03/10] ARM: OMAP: Initialize PWM backlight enable_gpio field Thierry Reding
@ 2013-09-23 21:41 ` Thierry Reding
2013-09-23 21:41 ` [PATCH 05/10] ARM: SAMSUNG: " Thierry Reding
` (5 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Thierry Reding @ 2013-09-23 21:41 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks,
Kukjin Kim, Simon Horman, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel
The GPIO API defines 0 as being a valid GPIO number, so this field needs
to be initialized explicitly.
A special case is the Palm Tungsten|C board. Since it doesn't use any
quirks that would require the existing .init() or .exit() hooks it can
simply use the new enable_gpio field.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
arch/arm/mach-pxa/cm-x300.c | 1 +
arch/arm/mach-pxa/colibri-pxa270-income.c | 1 +
arch/arm/mach-pxa/ezx.c | 1 +
arch/arm/mach-pxa/hx4700.c | 1 +
arch/arm/mach-pxa/lpd270.c | 1 +
arch/arm/mach-pxa/magician.c | 1 +
arch/arm/mach-pxa/mainstone.c | 1 +
arch/arm/mach-pxa/mioa701.c | 1 +
arch/arm/mach-pxa/palm27x.c | 1 +
arch/arm/mach-pxa/palmtc.c | 35 +------------------------------
arch/arm/mach-pxa/palmte2.c | 1 +
arch/arm/mach-pxa/pcm990-baseboard.c | 1 +
arch/arm/mach-pxa/raumfeld.c | 1 +
arch/arm/mach-pxa/tavorevb.c | 2 ++
arch/arm/mach-pxa/viper.c | 1 +
arch/arm/mach-pxa/z2.c | 2 ++
arch/arm/mach-pxa/zylonite.c | 1 +
17 files changed, 19 insertions(+), 34 deletions(-)
diff --git a/arch/arm/mach-pxa/cm-x300.c b/arch/arm/mach-pxa/cm-x300.c
index f942349..584439b 100644
--- a/arch/arm/mach-pxa/cm-x300.c
+++ b/arch/arm/mach-pxa/cm-x300.c
@@ -310,6 +310,7 @@ static struct platform_pwm_backlight_data cm_x300_backlight_data = {
.max_brightness = 100,
.dft_brightness = 100,
.pwm_period_ns = 10000,
+ .enable_gpio = -1,
};
static struct platform_device cm_x300_backlight_device = {
diff --git a/arch/arm/mach-pxa/colibri-pxa270-income.c b/arch/arm/mach-pxa/colibri-pxa270-income.c
index 2d4a7b4..3aa2646 100644
--- a/arch/arm/mach-pxa/colibri-pxa270-income.c
+++ b/arch/arm/mach-pxa/colibri-pxa270-income.c
@@ -189,6 +189,7 @@ static struct platform_pwm_backlight_data income_backlight_data = {
.max_brightness = 0x3ff,
.dft_brightness = 0x1ff,
.pwm_period_ns = 1000000,
+ .enable_gpio = -1,
};
static struct platform_device income_backlight = {
diff --git a/arch/arm/mach-pxa/ezx.c b/arch/arm/mach-pxa/ezx.c
index fe2eb83..ab93441 100644
--- a/arch/arm/mach-pxa/ezx.c
+++ b/arch/arm/mach-pxa/ezx.c
@@ -54,6 +54,7 @@ static struct platform_pwm_backlight_data ezx_backlight_data = {
.max_brightness = 1023,
.dft_brightness = 1023,
.pwm_period_ns = 78770,
+ .enable_gpio = -1,
};
static struct platform_device ezx_backlight_device = {
diff --git a/arch/arm/mach-pxa/hx4700.c b/arch/arm/mach-pxa/hx4700.c
index 133109e..a7c30eb 100644
--- a/arch/arm/mach-pxa/hx4700.c
+++ b/arch/arm/mach-pxa/hx4700.c
@@ -561,6 +561,7 @@ static struct platform_pwm_backlight_data backlight_data = {
.max_brightness = 200,
.dft_brightness = 100,
.pwm_period_ns = 30923,
+ .enable_gpio = -1,
};
static struct platform_device backlight = {
diff --git a/arch/arm/mach-pxa/lpd270.c b/arch/arm/mach-pxa/lpd270.c
index 1255ee0..9f6ec16 100644
--- a/arch/arm/mach-pxa/lpd270.c
+++ b/arch/arm/mach-pxa/lpd270.c
@@ -269,6 +269,7 @@ static struct platform_pwm_backlight_data lpd270_backlight_data = {
.max_brightness = 1,
.dft_brightness = 1,
.pwm_period_ns = 78770,
+ .enable_gpio = -1,
};
static struct platform_device lpd270_backlight_device = {
diff --git a/arch/arm/mach-pxa/magician.c b/arch/arm/mach-pxa/magician.c
index f44532f..fab30d6 100644
--- a/arch/arm/mach-pxa/magician.c
+++ b/arch/arm/mach-pxa/magician.c
@@ -378,6 +378,7 @@ static struct platform_pwm_backlight_data backlight_data = {
.max_brightness = 272,
.dft_brightness = 100,
.pwm_period_ns = 30923,
+ .enable_gpio = -1,
.init = magician_backlight_init,
.notify = magician_backlight_notify,
.exit = magician_backlight_exit,
diff --git a/arch/arm/mach-pxa/mainstone.c b/arch/arm/mach-pxa/mainstone.c
index dd70343..08ccc07 100644
--- a/arch/arm/mach-pxa/mainstone.c
+++ b/arch/arm/mach-pxa/mainstone.c
@@ -338,6 +338,7 @@ static struct platform_pwm_backlight_data mainstone_backlight_data = {
.max_brightness = 1023,
.dft_brightness = 1023,
.pwm_period_ns = 78770,
+ .enable_gpio = -1,
};
static struct platform_device mainstone_backlight_device = {
diff --git a/arch/arm/mach-pxa/mioa701.c b/arch/arm/mach-pxa/mioa701.c
index acc9d3c..f70583f 100644
--- a/arch/arm/mach-pxa/mioa701.c
+++ b/arch/arm/mach-pxa/mioa701.c
@@ -186,6 +186,7 @@ static struct platform_pwm_backlight_data mioa701_backlight_data = {
.max_brightness = 100,
.dft_brightness = 50,
.pwm_period_ns = 4000 * 1024, /* Fl = 250kHz */
+ .enable_gpio = -1,
};
/*
diff --git a/arch/arm/mach-pxa/palm27x.c b/arch/arm/mach-pxa/palm27x.c
index 17d4c53..e54a296 100644
--- a/arch/arm/mach-pxa/palm27x.c
+++ b/arch/arm/mach-pxa/palm27x.c
@@ -322,6 +322,7 @@ static struct platform_pwm_backlight_data palm27x_backlight_data = {
.max_brightness = 0xfe,
.dft_brightness = 0x7e,
.pwm_period_ns = 3500 * 1024,
+ .enable_gpio = -1,
.init = palm27x_backlight_init,
.notify = palm27x_backlight_notify,
.exit = palm27x_backlight_exit,
diff --git a/arch/arm/mach-pxa/palmtc.c b/arch/arm/mach-pxa/palmtc.c
index 100b176f..7691c97 100644
--- a/arch/arm/mach-pxa/palmtc.c
+++ b/arch/arm/mach-pxa/palmtc.c
@@ -166,45 +166,12 @@ static inline void palmtc_keys_init(void) {}
* Backlight
******************************************************************************/
#if defined(CONFIG_BACKLIGHT_PWM) || defined(CONFIG_BACKLIGHT_PWM_MODULE)
-static int palmtc_backlight_init(struct device *dev)
-{
- int ret;
-
- ret = gpio_request(GPIO_NR_PALMTC_BL_POWER, "BL POWER");
- if (ret)
- goto err;
- ret = gpio_direction_output(GPIO_NR_PALMTC_BL_POWER, 1);
- if (ret)
- goto err2;
-
- return 0;
-
-err2:
- gpio_free(GPIO_NR_PALMTC_BL_POWER);
-err:
- return ret;
-}
-
-static int palmtc_backlight_notify(struct device *dev, int brightness)
-{
- /* backlight is on when GPIO16 AF0 is high */
- gpio_set_value(GPIO_NR_PALMTC_BL_POWER, brightness);
- return brightness;
-}
-
-static void palmtc_backlight_exit(struct device *dev)
-{
- gpio_free(GPIO_NR_PALMTC_BL_POWER);
-}
-
static struct platform_pwm_backlight_data palmtc_backlight_data = {
.pwm_id = 1,
.max_brightness = PALMTC_MAX_INTENSITY,
.dft_brightness = PALMTC_MAX_INTENSITY,
.pwm_period_ns = PALMTC_PERIOD_NS,
- .init = palmtc_backlight_init,
- .notify = palmtc_backlight_notify,
- .exit = palmtc_backlight_exit,
+ .enable_gpio = GPIO_NR_PALMTC_BL_POWER,
};
static struct platform_device palmtc_backlight = {
diff --git a/arch/arm/mach-pxa/palmte2.c b/arch/arm/mach-pxa/palmte2.c
index 0742721..956fd24 100644
--- a/arch/arm/mach-pxa/palmte2.c
+++ b/arch/arm/mach-pxa/palmte2.c
@@ -165,6 +165,7 @@ static struct platform_pwm_backlight_data palmte2_backlight_data = {
.max_brightness = PALMTE2_MAX_INTENSITY,
.dft_brightness = PALMTE2_MAX_INTENSITY,
.pwm_period_ns = PALMTE2_PERIOD_NS,
+ .enable_gpio = -1,
.init = palmte2_backlight_init,
.notify = palmte2_backlight_notify,
.exit = palmte2_backlight_exit,
diff --git a/arch/arm/mach-pxa/pcm990-baseboard.c b/arch/arm/mach-pxa/pcm990-baseboard.c
index 3133ba8..9a4e470 100644
--- a/arch/arm/mach-pxa/pcm990-baseboard.c
+++ b/arch/arm/mach-pxa/pcm990-baseboard.c
@@ -153,6 +153,7 @@ static struct platform_pwm_backlight_data pcm990_backlight_data = {
.max_brightness = 1023,
.dft_brightness = 1023,
.pwm_period_ns = 78770,
+ .enable_gpio = -1,
};
static struct platform_device pcm990_backlight_device = {
diff --git a/arch/arm/mach-pxa/raumfeld.c b/arch/arm/mach-pxa/raumfeld.c
index 969b0ba..8386dc3 100644
--- a/arch/arm/mach-pxa/raumfeld.c
+++ b/arch/arm/mach-pxa/raumfeld.c
@@ -539,6 +539,7 @@ static struct platform_pwm_backlight_data raumfeld_pwm_backlight_data = {
.dft_brightness = 100,
/* 10000 ns = 10 ms ^= 100 kHz */
.pwm_period_ns = 10000,
+ .enable_gpio = -1,
};
static struct platform_device raumfeld_pwm_backlight_device = {
diff --git a/arch/arm/mach-pxa/tavorevb.c b/arch/arm/mach-pxa/tavorevb.c
index 4680efe..a71da84 100644
--- a/arch/arm/mach-pxa/tavorevb.c
+++ b/arch/arm/mach-pxa/tavorevb.c
@@ -175,6 +175,7 @@ static struct platform_pwm_backlight_data tavorevb_backlight_data[] = {
.max_brightness = 100,
.dft_brightness = 100,
.pwm_period_ns = 100000,
+ .enable_gpio = -1,
},
[1] = {
/* secondary backlight */
@@ -182,6 +183,7 @@ static struct platform_pwm_backlight_data tavorevb_backlight_data[] = {
.max_brightness = 100,
.dft_brightness = 100,
.pwm_period_ns = 100000,
+ .enable_gpio = -1,
},
};
diff --git a/arch/arm/mach-pxa/viper.c b/arch/arm/mach-pxa/viper.c
index 9c363c0..29905b1 100644
--- a/arch/arm/mach-pxa/viper.c
+++ b/arch/arm/mach-pxa/viper.c
@@ -401,6 +401,7 @@ static struct platform_pwm_backlight_data viper_backlight_data = {
.max_brightness = 100,
.dft_brightness = 100,
.pwm_period_ns = 1000000,
+ .enable_gpio = -1,
.init = viper_backlight_init,
.notify = viper_backlight_notify,
.exit = viper_backlight_exit,
diff --git a/arch/arm/mach-pxa/z2.c b/arch/arm/mach-pxa/z2.c
index 2513d8f..e1a121b 100644
--- a/arch/arm/mach-pxa/z2.c
+++ b/arch/arm/mach-pxa/z2.c
@@ -206,6 +206,7 @@ static struct platform_pwm_backlight_data z2_backlight_data[] = {
.max_brightness = 1023,
.dft_brightness = 0,
.pwm_period_ns = 1260320,
+ .enable_gpio = -1,
},
[1] = {
/* LCD Backlight */
@@ -213,6 +214,7 @@ static struct platform_pwm_backlight_data z2_backlight_data[] = {
.max_brightness = 1023,
.dft_brightness = 512,
.pwm_period_ns = 1260320,
+ .enable_gpio = -1,
},
};
diff --git a/arch/arm/mach-pxa/zylonite.c b/arch/arm/mach-pxa/zylonite.c
index 36cf7cf..77daea4 100644
--- a/arch/arm/mach-pxa/zylonite.c
+++ b/arch/arm/mach-pxa/zylonite.c
@@ -125,6 +125,7 @@ static struct platform_pwm_backlight_data zylonite_backlight_data = {
.max_brightness = 100,
.dft_brightness = 100,
.pwm_period_ns = 10000,
+ .enable_gpio = -1,
};
static struct platform_device zylonite_backlight_device = {
--
1.8.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 05/10] ARM: SAMSUNG: Initialize PWM backlight enable_gpio field
2013-09-23 21:40 [PATCH 00/10] pwm-backlight: Add GPIO and power supply support Thierry Reding
` (3 preceding siblings ...)
2013-09-23 21:41 ` [PATCH 04/10] ARM: pxa: " Thierry Reding
@ 2013-09-23 21:41 ` Thierry Reding
2013-10-01 18:31 ` Stephen Warren
2013-09-23 21:41 ` [PATCH 06/10] ARM: shmobile: " Thierry Reding
` (4 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2013-09-23 21:41 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks,
Kukjin Kim, Simon Horman, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel
The GPIO API defines 0 as being a valid GPIO number, so this field needs
to be initialized explicitly.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
arch/arm/mach-s3c24xx/mach-h1940.c | 1 +
arch/arm/mach-s3c24xx/mach-rx1950.c | 1 +
arch/arm/mach-s3c64xx/mach-crag6410.c | 1 +
arch/arm/mach-s3c64xx/mach-hmt.c | 1 +
arch/arm/mach-s3c64xx/mach-smartq.c | 1 +
arch/arm/mach-s3c64xx/mach-smdk6410.c | 1 +
arch/arm/mach-s5p64x0/mach-smdk6440.c | 1 +
arch/arm/mach-s5p64x0/mach-smdk6450.c | 1 +
arch/arm/mach-s5pc100/mach-smdkc100.c | 1 +
arch/arm/mach-s5pv210/mach-smdkv210.c | 1 +
arch/arm/plat-samsung/dev-backlight.c | 5 +++++
11 files changed, 15 insertions(+)
diff --git a/arch/arm/mach-s3c24xx/mach-h1940.c b/arch/arm/mach-s3c24xx/mach-h1940.c
index 74dd479..952b6a0 100644
--- a/arch/arm/mach-s3c24xx/mach-h1940.c
+++ b/arch/arm/mach-s3c24xx/mach-h1940.c
@@ -504,6 +504,7 @@ static struct platform_pwm_backlight_data backlight_data = {
.dft_brightness = 50,
/* tcnt = 0x31 */
.pwm_period_ns = 36296,
+ .enable_gpio = -1,
.init = h1940_backlight_init,
.notify = h1940_backlight_notify,
.exit = h1940_backlight_exit,
diff --git a/arch/arm/mach-s3c24xx/mach-rx1950.c b/arch/arm/mach-s3c24xx/mach-rx1950.c
index 206b1f7..034b7fe 100644
--- a/arch/arm/mach-s3c24xx/mach-rx1950.c
+++ b/arch/arm/mach-s3c24xx/mach-rx1950.c
@@ -522,6 +522,7 @@ static struct platform_pwm_backlight_data rx1950_backlight_data = {
.max_brightness = 24,
.dft_brightness = 4,
.pwm_period_ns = 48000,
+ .enable_gpio = -1,
.init = rx1950_backlight_init,
.notify = rx1950_backlight_notify,
.exit = rx1950_backlight_exit,
diff --git a/arch/arm/mach-s3c64xx/mach-crag6410.c b/arch/arm/mach-s3c64xx/mach-crag6410.c
index 1a911df..b319bf9 100644
--- a/arch/arm/mach-s3c64xx/mach-crag6410.c
+++ b/arch/arm/mach-s3c64xx/mach-crag6410.c
@@ -114,6 +114,7 @@ static struct platform_pwm_backlight_data crag6410_backlight_data = {
.max_brightness = 1000,
.dft_brightness = 600,
.pwm_period_ns = 100000, /* about 1kHz */
+ .enable_gpio = -1,
};
static struct platform_device crag6410_backlight_device = {
diff --git a/arch/arm/mach-s3c64xx/mach-hmt.c b/arch/arm/mach-s3c64xx/mach-hmt.c
index e806404..614a03a 100644
--- a/arch/arm/mach-s3c64xx/mach-hmt.c
+++ b/arch/arm/mach-s3c64xx/mach-hmt.c
@@ -114,6 +114,7 @@ static struct platform_pwm_backlight_data hmt_backlight_data = {
.max_brightness = 100 * 256,
.dft_brightness = 40 * 256,
.pwm_period_ns = 1000000000 / (100 * 256 * 20),
+ .enable_gpio = -1,
.init = hmt_bl_init,
.notify = hmt_bl_notify,
.exit = hmt_bl_exit,
diff --git a/arch/arm/mach-s3c64xx/mach-smartq.c b/arch/arm/mach-s3c64xx/mach-smartq.c
index 0f47237..a6b338f 100644
--- a/arch/arm/mach-s3c64xx/mach-smartq.c
+++ b/arch/arm/mach-s3c64xx/mach-smartq.c
@@ -151,6 +151,7 @@ static struct platform_pwm_backlight_data smartq_backlight_data = {
.max_brightness = 1000,
.dft_brightness = 600,
.pwm_period_ns = 1000000000 / (1000 * 20),
+ .enable_gpio = -1,
.init = smartq_bl_init,
};
diff --git a/arch/arm/mach-s3c64xx/mach-smdk6410.c b/arch/arm/mach-s3c64xx/mach-smdk6410.c
index 2a7b32c..d5ea938 100644
--- a/arch/arm/mach-s3c64xx/mach-smdk6410.c
+++ b/arch/arm/mach-s3c64xx/mach-smdk6410.c
@@ -625,6 +625,7 @@ static struct samsung_bl_gpio_info smdk6410_bl_gpio_info = {
static struct platform_pwm_backlight_data smdk6410_bl_data = {
.pwm_id = 1,
+ .enable_gpio = -1,
};
static struct s3c_hsotg_plat smdk6410_hsotg_pdata;
diff --git a/arch/arm/mach-s5p64x0/mach-smdk6440.c b/arch/arm/mach-s5p64x0/mach-smdk6440.c
index 0b00304..9efdcc0 100644
--- a/arch/arm/mach-s5p64x0/mach-smdk6440.c
+++ b/arch/arm/mach-s5p64x0/mach-smdk6440.c
@@ -223,6 +223,7 @@ static struct samsung_bl_gpio_info smdk6440_bl_gpio_info = {
static struct platform_pwm_backlight_data smdk6440_bl_data = {
.pwm_id = 1,
+ .enable_gpio = -1,
};
static void __init smdk6440_map_io(void)
diff --git a/arch/arm/mach-s5p64x0/mach-smdk6450.c b/arch/arm/mach-s5p64x0/mach-smdk6450.c
index 5949296..c3cacc0 100644
--- a/arch/arm/mach-s5p64x0/mach-smdk6450.c
+++ b/arch/arm/mach-s5p64x0/mach-smdk6450.c
@@ -242,6 +242,7 @@ static struct samsung_bl_gpio_info smdk6450_bl_gpio_info = {
static struct platform_pwm_backlight_data smdk6450_bl_data = {
.pwm_id = 1,
+ .enable_gpio = -1,
};
static void __init smdk6450_map_io(void)
diff --git a/arch/arm/mach-s5pc100/mach-smdkc100.c b/arch/arm/mach-s5pc100/mach-smdkc100.c
index 7c57a22..9e256b9 100644
--- a/arch/arm/mach-s5pc100/mach-smdkc100.c
+++ b/arch/arm/mach-s5pc100/mach-smdkc100.c
@@ -216,6 +216,7 @@ static struct samsung_bl_gpio_info smdkc100_bl_gpio_info = {
static struct platform_pwm_backlight_data smdkc100_bl_data = {
.pwm_id = 0,
+ .enable_gpio = -1,
};
static void __init smdkc100_map_io(void)
diff --git a/arch/arm/mach-s5pv210/mach-smdkv210.c b/arch/arm/mach-s5pv210/mach-smdkv210.c
index 6d72bb99..f52cc15 100644
--- a/arch/arm/mach-s5pv210/mach-smdkv210.c
+++ b/arch/arm/mach-s5pv210/mach-smdkv210.c
@@ -279,6 +279,7 @@ static struct samsung_bl_gpio_info smdkv210_bl_gpio_info = {
static struct platform_pwm_backlight_data smdkv210_bl_data = {
.pwm_id = 3,
.pwm_period_ns = 1000,
+ .enable_gpio = -1,
};
static void __init smdkv210_map_io(void)
diff --git a/arch/arm/plat-samsung/dev-backlight.c b/arch/arm/plat-samsung/dev-backlight.c
index d51f956..80e7450 100644
--- a/arch/arm/plat-samsung/dev-backlight.c
+++ b/arch/arm/plat-samsung/dev-backlight.c
@@ -70,6 +70,7 @@ static struct samsung_bl_drvdata samsung_dfl_bl_data __initdata = {
.max_brightness = 255,
.dft_brightness = 255,
.pwm_period_ns = 78770,
+ .enable_gpio = -1,
.init = samsung_bl_init,
.exit = samsung_bl_exit,
},
@@ -121,6 +122,10 @@ void __init samsung_bl_set(struct samsung_bl_gpio_info *gpio_info,
samsung_bl_data->lth_brightness = bl_data->lth_brightness;
if (bl_data->pwm_period_ns)
samsung_bl_data->pwm_period_ns = bl_data->pwm_period_ns;
+ if (bl_data->enable_gpio)
+ samsung_bl_data->enable_gpio = bl_data->enable_gpio;
+ if (bl_data->enable_gpio_flags)
+ samsung_bl_data->enable_gpio_flags = bl_data->enable_gpio_flags;
if (bl_data->init)
samsung_bl_data->init = bl_data->init;
if (bl_data->notify)
--
1.8.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 05/10] ARM: SAMSUNG: Initialize PWM backlight enable_gpio field
2013-09-23 21:41 ` [PATCH 05/10] ARM: SAMSUNG: " Thierry Reding
@ 2013-10-01 18:31 ` Stephen Warren
[not found] ` <524B14E8.5040302-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
0 siblings, 1 reply; 31+ messages in thread
From: Stephen Warren @ 2013-10-01 18:31 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks, Kukjin Kim,
Simon Horman, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel
On 09/23/2013 03:41 PM, Thierry Reding wrote:
> The GPIO API defines 0 as being a valid GPIO number, so this field needs
> to be initialized explicitly.
> static void __init smdkv210_map_io(void)
> @@ -70,6 +70,7 @@ static struct samsung_bl_drvdata samsung_dfl_bl_data __initdata = {
> .max_brightness = 255,
> .dft_brightness = 255,
> .pwm_period_ns = 78770,
> + .enable_gpio = -1,
> .init = samsung_bl_init,
> .exit = samsung_bl_exit,
> },
> @@ -121,6 +122,10 @@ void __init samsung_bl_set(struct samsung_bl_gpio_info *gpio_info,
> samsung_bl_data->lth_brightness = bl_data->lth_brightness;
> if (bl_data->pwm_period_ns)
> samsung_bl_data->pwm_period_ns = bl_data->pwm_period_ns;
> + if (bl_data->enable_gpio)
> + samsung_bl_data->enable_gpio = bl_data->enable_gpio;
> + if (bl_data->enable_gpio_flags)
> + samsung_bl_data->enable_gpio_flags = bl_data->enable_gpio_flags;
Won't this cause the core pwm_bl driver to request/manipulate the GPIO,
whereas this driver already does that inside the samsung_bl_init/exit
callbacks? I think you either need to adjust those callbacks, or not set
the new standard GPIO property in samsung_bl_data.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 06/10] ARM: shmobile: Initialize PWM backlight enable_gpio field
2013-09-23 21:40 [PATCH 00/10] pwm-backlight: Add GPIO and power supply support Thierry Reding
` (4 preceding siblings ...)
2013-09-23 21:41 ` [PATCH 05/10] ARM: SAMSUNG: " Thierry Reding
@ 2013-09-23 21:41 ` Thierry Reding
2013-09-25 5:40 ` Simon Horman
2013-09-23 21:41 ` [PATCH 07/10] unicore32: " Thierry Reding
` (3 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2013-09-23 21:41 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks,
Kukjin Kim, Simon Horman, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel
The GPIO API defines 0 as being a valid GPIO number, so this field needs
to be initialized explicitly.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
arch/arm/mach-shmobile/board-armadillo800eva.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index 7f8f607..6ccb854 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -423,6 +423,7 @@ static struct platform_pwm_backlight_data pwm_backlight_data = {
.max_brightness = 255,
.dft_brightness = 255,
.pwm_period_ns = 33333, /* 30kHz */
+ .enable_gpio = -1,
};
static struct platform_device pwm_backlight_device = {
--
1.8.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 06/10] ARM: shmobile: Initialize PWM backlight enable_gpio field
2013-09-23 21:41 ` [PATCH 06/10] ARM: shmobile: " Thierry Reding
@ 2013-09-25 5:40 ` Simon Horman
0 siblings, 0 replies; 31+ messages in thread
From: Simon Horman @ 2013-09-25 5:40 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks,
Kukjin Kim, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel
On Mon, Sep 23, 2013 at 11:41:03PM +0200, Thierry Reding wrote:
> The GPIO API defines 0 as being a valid GPIO number, so this field needs
> to be initialized explicitly.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> arch/arm/mach-shmobile/board-armadillo800eva.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> index 7f8f607..6ccb854 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -423,6 +423,7 @@ static struct platform_pwm_backlight_data pwm_backlight_data = {
> .max_brightness = 255,
> .dft_brightness = 255,
> .pwm_period_ns = 33333, /* 30kHz */
> + .enable_gpio = -1,
> };
>
> static struct platform_device pwm_backlight_device = {
Acked-by: Simon Horman <horms+renesas@verge.net.au>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 07/10] unicore32: Initialize PWM backlight enable_gpio field
2013-09-23 21:40 [PATCH 00/10] pwm-backlight: Add GPIO and power supply support Thierry Reding
` (5 preceding siblings ...)
2013-09-23 21:41 ` [PATCH 06/10] ARM: shmobile: " Thierry Reding
@ 2013-09-23 21:41 ` Thierry Reding
[not found] ` <1379972467-11243-1-git-send-email-treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
` (2 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Thierry Reding @ 2013-09-23 21:41 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks,
Kukjin Kim, Simon Horman, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel
The GPIO API defines 0 as being a valid GPIO number, so this field needs
to be initialized explicitly.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
arch/unicore32/kernel/puv3-nb0916.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/unicore32/kernel/puv3-nb0916.c b/arch/unicore32/kernel/puv3-nb0916.c
index 181108b..0c6618e 100644
--- a/arch/unicore32/kernel/puv3-nb0916.c
+++ b/arch/unicore32/kernel/puv3-nb0916.c
@@ -54,6 +54,7 @@ static struct platform_pwm_backlight_data nb0916_backlight_data = {
.max_brightness = 100,
.dft_brightness = 100,
.pwm_period_ns = 70 * 1024,
+ .enable_gpio = -1,
};
static struct gpio_keys_button nb0916_gpio_keys[] = {
--
1.8.4
^ permalink raw reply related [flat|nested] 31+ messages in thread[parent not found: <1379972467-11243-1-git-send-email-treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]
* [PATCH 08/10] pwm-backlight: Use new enable_gpio field
[not found] ` <1379972467-11243-1-git-send-email-treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2013-09-23 21:41 ` Thierry Reding
[not found] ` <1379972467-11243-9-git-send-email-treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-09-23 21:41 ` [PATCH 09/10] pwm-backlight: Use an optional power supply Thierry Reding
1 sibling, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2013-09-23 21:41 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks,
Kukjin Kim, Simon Horman, Magnus Damm, Guan Xuetao,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
openezx-devel-ZwoEplunGu3n3BO9LpVK+9i2O/JbrIOy,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-sh-u79uwXL29TY76Z2rM5mHXA, linux-pwm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Make use of the new enable_gpio field and allow it to be set from DT as
well. Now that all legacy users of platform data have been converted to
initialize this field to an invalid value, it is safe to use the field
from the driver.
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
.../bindings/video/backlight/pwm-backlight.txt | 3 ++
drivers/video/backlight/pwm_bl.c | 54 +++++++++++++++++++---
2 files changed, 50 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
index 1e4fc72..052eb03 100644
--- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
@@ -14,6 +14,7 @@ Required properties:
Optional properties:
- pwm-names: a list of names for the PWM devices specified in the
"pwms" property (see PWM binding[0])
+ - enable-gpios: a list of GPIOs to enable and disable the backlight
[0]: Documentation/devicetree/bindings/pwm/pwm.txt
@@ -25,4 +26,6 @@ Example:
brightness-levels = <0 4 8 16 32 64 128 255>;
default-brightness-level = <6>;
+
+ enable-gpios = <&gpio 58 0>;
};
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 8a49b30..506810c 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -10,6 +10,8 @@
* published by the Free Software Foundation.
*/
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/init.h>
@@ -27,6 +29,8 @@ struct pwm_bl_data {
unsigned int period;
unsigned int lth_brightness;
unsigned int *levels;
+ int enable_gpio;
+ unsigned long enable_gpio_flags;
int (*notify)(struct device *,
int brightness);
void (*notify_after)(struct device *,
@@ -51,12 +55,27 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness,
pb->lth_brightness;
pwm_config(pb->pwm, duty_cycle, pb->period);
+
+ if (gpio_is_valid(pb->enable_gpio)) {
+ if (pb->enable_gpio_flags & PWM_BACKLIGHT_GPIO_ACTIVE_LOW)
+ gpio_set_value(pb->enable_gpio, 0);
+ else
+ gpio_set_value(pb->enable_gpio, 1);
+ }
+
pwm_enable(pb->pwm);
}
static void pwm_backlight_power_off(struct pwm_bl_data *pb)
{
pwm_disable(pb->pwm);
+
+ if (gpio_is_valid(pb->enable_gpio)) {
+ if (pb->enable_gpio_flags & PWM_BACKLIGHT_GPIO_ACTIVE_LOW)
+ gpio_set_value(pb->enable_gpio, 1);
+ else
+ gpio_set_value(pb->enable_gpio, 0);
+ }
}
static int pwm_backlight_update_status(struct backlight_device *bl)
@@ -108,6 +127,7 @@ static int pwm_backlight_parse_dt(struct device *dev,
struct platform_pwm_backlight_data *data)
{
struct device_node *node = dev->of_node;
+ enum of_gpio_flags flags;
struct property *prop;
int length;
u32 value;
@@ -148,11 +168,10 @@ static int pwm_backlight_parse_dt(struct device *dev,
data->max_brightness--;
}
- /*
- * TODO: Most users of this driver use a number of GPIOs to control
- * backlight power. Support for specifying these needs to be
- * added.
- */
+ data->enable_gpio = of_get_named_gpio_flags(node, "enable-gpios", 0,
+ &flags);
+ if (gpio_is_valid(data->enable_gpio) && (flags & OF_GPIO_ACTIVE_LOW))
+ data->enable_gpio_flags |= PWM_BACKLIGHT_GPIO_ACTIVE_LOW;
return 0;
}
@@ -210,12 +229,30 @@ static int pwm_backlight_probe(struct platform_device *pdev)
} else
max = data->max_brightness;
+ pb->enable_gpio = data->enable_gpio;
+ pb->enable_gpio_flags = data->enable_gpio_flags;
pb->notify = data->notify;
pb->notify_after = data->notify_after;
pb->check_fb = data->check_fb;
pb->exit = data->exit;
pb->dev = &pdev->dev;
+ if (gpio_is_valid(pb->enable_gpio)) {
+ unsigned long flags;
+
+ if (pb->enable_gpio_flags & PWM_BACKLIGHT_GPIO_ACTIVE_LOW)
+ flags = GPIOF_OUT_INIT_HIGH;
+ else
+ flags = GPIOF_OUT_INIT_LOW;
+
+ ret = gpio_request_one(pb->enable_gpio, flags, "enable");
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to request GPIO#%d: %d\n",
+ pb->enable_gpio, ret);
+ goto err_alloc;
+ }
+ }
+
pb->pwm = devm_pwm_get(&pdev->dev, NULL);
if (IS_ERR(pb->pwm)) {
dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
@@ -224,7 +261,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
if (IS_ERR(pb->pwm)) {
dev_err(&pdev->dev, "unable to request legacy PWM\n");
ret = PTR_ERR(pb->pwm);
- goto err_alloc;
+ goto err_gpio;
}
}
@@ -249,7 +286,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
if (IS_ERR(bl)) {
dev_err(&pdev->dev, "failed to register backlight\n");
ret = PTR_ERR(bl);
- goto err_alloc;
+ goto err_gpio;
}
if (data->dft_brightness > data->max_brightness) {
@@ -265,6 +302,9 @@ static int pwm_backlight_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, bl);
return 0;
+err_gpio:
+ if (gpio_is_valid(pb->enable_gpio))
+ gpio_free(pb->enable_gpio);
err_alloc:
if (data->exit)
data->exit(&pdev->dev);
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 09/10] pwm-backlight: Use an optional power supply
[not found] ` <1379972467-11243-1-git-send-email-treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-09-23 21:41 ` [PATCH 08/10] pwm-backlight: Use new " Thierry Reding
@ 2013-09-23 21:41 ` Thierry Reding
2013-10-01 18:43 ` Stephen Warren
1 sibling, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2013-09-23 21:41 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks,
Kukjin Kim, Simon Horman, Magnus Damm, Guan Xuetao,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
openezx-devel-ZwoEplunGu3n3BO9LpVK+9i2O/JbrIOy,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-sh-u79uwXL29TY76Z2rM5mHXA, linux-pwm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Many backlights require a power supply to work properly. This commit
uses a power-supply regulator, if available, to power up and power down
the panel.
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
.../bindings/video/backlight/pwm-backlight.txt | 2 ++
drivers/video/backlight/pwm_bl.c | 21 +++++++++++++++++++++
2 files changed, 23 insertions(+)
diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
index 052eb03..3898f26 100644
--- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
@@ -15,6 +15,7 @@ Optional properties:
- pwm-names: a list of names for the PWM devices specified in the
"pwms" property (see PWM binding[0])
- enable-gpios: a list of GPIOs to enable and disable the backlight
+ - power-supply: regulator for supply voltage
[0]: Documentation/devicetree/bindings/pwm/pwm.txt
@@ -28,4 +29,5 @@ Example:
default-brightness-level = <6>;
enable-gpios = <&gpio 58 0>;
+ power-supply = <&vdd_bl_reg>;
};
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 506810c..a2b3876 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -21,6 +21,7 @@
#include <linux/err.h>
#include <linux/pwm.h>
#include <linux/pwm_backlight.h>
+#include <linux/regulator/consumer.h>
#include <linux/slab.h>
struct pwm_bl_data {
@@ -29,6 +30,7 @@ struct pwm_bl_data {
unsigned int period;
unsigned int lth_brightness;
unsigned int *levels;
+ struct regulator *power_supply;
int enable_gpio;
unsigned long enable_gpio_flags;
int (*notify)(struct device *,
@@ -56,6 +58,12 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness,
pwm_config(pb->pwm, duty_cycle, pb->period);
+ if (pb->power_supply) {
+ err = regulator_enable(pb->power_supply);
+ if (err < 0)
+ dev_err(pb->dev, "failed to enable power supply\n");
+ }
+
if (gpio_is_valid(pb->enable_gpio)) {
if (pb->enable_gpio_flags & PWM_BACKLIGHT_GPIO_ACTIVE_LOW)
gpio_set_value(pb->enable_gpio, 0);
@@ -76,6 +84,9 @@ static void pwm_backlight_power_off(struct pwm_bl_data *pb)
else
gpio_set_value(pb->enable_gpio, 0);
}
+
+ if (pb->power_supply)
+ regulator_disable(pb->power_supply);
}
static int pwm_backlight_update_status(struct backlight_device *bl)
@@ -253,6 +264,16 @@ static int pwm_backlight_probe(struct platform_device *pdev)
}
}
+ pb->power_supply = devm_regulator_get_optional(&pdev->dev, "power");
+ if (IS_ERR(pb->power_supply)) {
+ if (PTR_ERR(pb->power_supply) != -ENODEV) {
+ ret = PTR_ERR(pb->power_supply);
+ goto err_gpio;
+ }
+
+ pb->power_supply = NULL;
+ }
+
pb->pwm = devm_pwm_get(&pdev->dev, NULL);
if (IS_ERR(pb->pwm)) {
dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 09/10] pwm-backlight: Use an optional power supply
2013-09-23 21:41 ` [PATCH 09/10] pwm-backlight: Use an optional power supply Thierry Reding
@ 2013-10-01 18:43 ` Stephen Warren
2013-10-01 20:53 ` Thierry Reding
0 siblings, 1 reply; 31+ messages in thread
From: Stephen Warren @ 2013-10-01 18:43 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks, Kukjin Kim,
Simon Horman, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel
On 09/23/2013 03:41 PM, Thierry Reding wrote:
> Many backlights require a power supply to work properly. This commit
> uses a power-supply regulator, if available, to power up and power down
> the panel.
I think that all backlights require a power supply, albeit the supply
may not be SW-controllable. Hence, shouldn't the regulator be mandatory
in the binding, yet the driver be defensively coded such that if one
isn't specified, the driver continues to work?
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> @@ -253,6 +264,16 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> }
> }
>
> + pb->power_supply = devm_regulator_get_optional(&pdev->dev, "power");
... so I think that should be devm_regulator_get(), since the regulator
isn't really optional.
> + if (IS_ERR(pb->power_supply)) {
> + if (PTR_ERR(pb->power_supply) != -ENODEV) {
> + ret = PTR_ERR(pb->power_supply);
> + goto err_gpio;
> + }
> +
> + pb->power_supply = NULL;
If devm_regulator_get_optional() returns an error value or a valid
value, then I don't think that this driver should transmute error values
into NULL; NULL might be a perfectly valid regulator value. Related, I
think the if (pb->power_supply) tests should be replaced with if
(!IS_ERR(pb->power_supply)) instead.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 09/10] pwm-backlight: Use an optional power supply
2013-10-01 18:43 ` Stephen Warren
@ 2013-10-01 20:53 ` Thierry Reding
2013-10-01 20:59 ` Stephen Warren
0 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2013-10-01 20:53 UTC (permalink / raw)
To: Stephen Warren
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks, Kukjin Kim,
Simon Horman, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1904 bytes --]
On Tue, Oct 01, 2013 at 12:43:57PM -0600, Stephen Warren wrote:
> On 09/23/2013 03:41 PM, Thierry Reding wrote:
> > Many backlights require a power supply to work properly. This commit
> > uses a power-supply regulator, if available, to power up and power down
> > the panel.
>
> I think that all backlights require a power supply, albeit the supply
> may not be SW-controllable. Hence, shouldn't the regulator be mandatory
> in the binding, yet the driver be defensively coded such that if one
> isn't specified, the driver continues to work?
That has already changed in my local version of this patch.
> > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
>
> > @@ -253,6 +264,16 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> > }
> > }
> >
> > + pb->power_supply = devm_regulator_get_optional(&pdev->dev, "power");
>
> ... so I think that should be devm_regulator_get(), since the regulator
> isn't really optional.
>
> > + if (IS_ERR(pb->power_supply)) {
> > + if (PTR_ERR(pb->power_supply) != -ENODEV) {
> > + ret = PTR_ERR(pb->power_supply);
> > + goto err_gpio;
> > + }
> > +
> > + pb->power_supply = NULL;
>
> If devm_regulator_get_optional() returns an error value or a valid
> value, then I don't think that this driver should transmute error values
> into NULL; NULL might be a perfectly valid regulator value. Related, I
> think the if (pb->power_supply) tests should be replaced with if
> (!IS_ERR(pb->power_supply)) instead.
All of that is already done in my local tree. This actually turns out to
work rather smoothly with the new support for optional regulators. The
regulator core will give you a dummy regulator (assuming it's there
physically but hasn't been wired up in software) that's always on, so
the driver doesn't even have to special case it anymore.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 09/10] pwm-backlight: Use an optional power supply
2013-10-01 20:53 ` Thierry Reding
@ 2013-10-01 20:59 ` Stephen Warren
2013-10-01 21:31 ` Thierry Reding
0 siblings, 1 reply; 31+ messages in thread
From: Stephen Warren @ 2013-10-01 20:59 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks, Kukjin Kim,
Simon Horman, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel
On 10/01/2013 02:53 PM, Thierry Reding wrote:
> On Tue, Oct 01, 2013 at 12:43:57PM -0600, Stephen Warren wrote:
>> On 09/23/2013 03:41 PM, Thierry Reding wrote:
>>> Many backlights require a power supply to work properly. This
>>> commit uses a power-supply regulator, if available, to power up
>>> and power down the panel.
>>
>> I think that all backlights require a power supply, albeit the
>> supply may not be SW-controllable. Hence, shouldn't the regulator
>> be mandatory in the binding, yet the driver be defensively coded
>> such that if one isn't specified, the driver continues to work?
>
> That has already changed in my local version of this patch.
>
>>> diff --git a/drivers/video/backlight/pwm_bl.c
>>> b/drivers/video/backlight/pwm_bl.c
>>
>>> @@ -253,6 +264,16 @@ static int pwm_backlight_probe(struct
>>> platform_device *pdev) } }
>>>
>>> + pb->power_supply = devm_regulator_get_optional(&pdev->dev,
>>> "power");
>>
>> ... so I think that should be devm_regulator_get(), since the
>> regulator isn't really optional.
>>
>>> + if (IS_ERR(pb->power_supply)) { + if
>>> (PTR_ERR(pb->power_supply) != -ENODEV) { + ret =
>>> PTR_ERR(pb->power_supply); + goto err_gpio; + } + +
>>> pb->power_supply = NULL;
>>
>> If devm_regulator_get_optional() returns an error value or a
>> valid value, then I don't think that this driver should transmute
>> error values into NULL; NULL might be a perfectly valid regulator
>> value. Related, I think the if (pb->power_supply) tests should be
>> replaced with if (!IS_ERR(pb->power_supply)) instead.
>
> All of that is already done in my local tree. This actually turns
> out to work rather smoothly with the new support for optional
> regulators. The regulator core will give you a dummy regulator
> (assuming it's there physically but hasn't been wired up in
> software) that's always on, so the driver doesn't even have to
> special case it anymore.
OK, hopefully it (the regulator core) complains about the missing DT
property though; I assume you're using regulator_get() not
regulator_get_optional(), since the supply really is not optional.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 09/10] pwm-backlight: Use an optional power supply
2013-10-01 20:59 ` Stephen Warren
@ 2013-10-01 21:31 ` Thierry Reding
2013-10-02 10:35 ` Mark Brown
0 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2013-10-01 21:31 UTC (permalink / raw)
To: Stephen Warren
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks, Kukjin Kim,
Simon Horman, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel, Mark Brown
[-- Attachment #1: Type: text/plain, Size: 2751 bytes --]
On Tue, Oct 01, 2013 at 02:59:43PM -0600, Stephen Warren wrote:
> On 10/01/2013 02:53 PM, Thierry Reding wrote:
> > On Tue, Oct 01, 2013 at 12:43:57PM -0600, Stephen Warren wrote:
> >> On 09/23/2013 03:41 PM, Thierry Reding wrote:
> >>> Many backlights require a power supply to work properly. This
> >>> commit uses a power-supply regulator, if available, to power up
> >>> and power down the panel.
> >>
> >> I think that all backlights require a power supply, albeit the
> >> supply may not be SW-controllable. Hence, shouldn't the regulator
> >> be mandatory in the binding, yet the driver be defensively coded
> >> such that if one isn't specified, the driver continues to work?
> >
> > That has already changed in my local version of this patch.
> >
> >>> diff --git a/drivers/video/backlight/pwm_bl.c
> >>> b/drivers/video/backlight/pwm_bl.c
> >>
> >>> @@ -253,6 +264,16 @@ static int pwm_backlight_probe(struct
> >>> platform_device *pdev) } }
> >>>
> >>> + pb->power_supply = devm_regulator_get_optional(&pdev->dev,
> >>> "power");
> >>
> >> ... so I think that should be devm_regulator_get(), since the
> >> regulator isn't really optional.
> >>
> >>> + if (IS_ERR(pb->power_supply)) { + if
> >>> (PTR_ERR(pb->power_supply) != -ENODEV) { + ret =
> >>> PTR_ERR(pb->power_supply); + goto err_gpio; + } + +
> >>> pb->power_supply = NULL;
> >>
> >> If devm_regulator_get_optional() returns an error value or a
> >> valid value, then I don't think that this driver should transmute
> >> error values into NULL; NULL might be a perfectly valid regulator
> >> value. Related, I think the if (pb->power_supply) tests should be
> >> replaced with if (!IS_ERR(pb->power_supply)) instead.
> >
> > All of that is already done in my local tree. This actually turns
> > out to work rather smoothly with the new support for optional
> > regulators. The regulator core will give you a dummy regulator
> > (assuming it's there physically but hasn't been wired up in
> > software) that's always on, so the driver doesn't even have to
> > special case it anymore.
>
> OK, hopefully it (the regulator core) complains about the missing DT
> property though; I assume you're using regulator_get() not
> regulator_get_optional(), since the supply really is not optional.
It doesn't always. There's a pr_warn() in _regulator_get(), but that's
only for non-DT (since for DT, has_full_constraints is set to true in
regulator_init_complete()). Actually that would mean that the regulator
core will complain as long as init isn't complete. But since, like many
other drivers, pwm-backlight could use deferred probing it's likely to
end up being probed after init.
Cc'ing Mark Brown.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 09/10] pwm-backlight: Use an optional power supply
2013-10-01 21:31 ` Thierry Reding
@ 2013-10-02 10:35 ` Mark Brown
0 siblings, 0 replies; 31+ messages in thread
From: Mark Brown @ 2013-10-02 10:35 UTC (permalink / raw)
To: Thierry Reding
Cc: Stephen Warren, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks,
Kukjin Kim, Simon Horman, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]
On Tue, Oct 01, 2013 at 11:31:27PM +0200, Thierry Reding wrote:
> On Tue, Oct 01, 2013 at 02:59:43PM -0600, Stephen Warren wrote:
> > OK, hopefully it (the regulator core) complains about the missing DT
> > property though; I assume you're using regulator_get() not
> > regulator_get_optional(), since the supply really is not optional.
> It doesn't always. There's a pr_warn() in _regulator_get(), but that's
> only for non-DT (since for DT, has_full_constraints is set to true in
> regulator_init_complete()). Actually that would mean that the regulator
> core will complain as long as init isn't complete. But since, like many
> other drivers, pwm-backlight could use deferred probing it's likely to
> end up being probed after init.
So, part of the thing here is that nobody ever bothers actually creating
the supplies even when the device fails to load without them - everyone
is much more keen to complain about needing to do the work than actually
provide the hookup even though it's generally pretty quick and simple to
do so.
That said we probably should still moan, I'll go and do that.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 10/10] pwm-backlight: Allow backlight to remain disabled on boot
2013-09-23 21:40 [PATCH 00/10] pwm-backlight: Add GPIO and power supply support Thierry Reding
` (7 preceding siblings ...)
[not found] ` <1379972467-11243-1-git-send-email-treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2013-09-23 21:41 ` Thierry Reding
2013-10-01 18:50 ` Stephen Warren
2013-09-24 8:14 ` [PATCH 00/10] pwm-backlight: Add GPIO and power supply support Simon Horman
9 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2013-09-23 21:41 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks,
Kukjin Kim, Simon Horman, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel
The default for backlight devices is to be enabled immediately when
registering with the backlight core. This can be useful for setups that
use a simple framebuffer device and where the backlight cannot otherwise
be hooked up to the panel.
However, when dealing with more complex setups, such as those of recent
ARM SoCs, this can be problematic. Since the backlight is usually setup
separately from the display controller, the probe order is not usually
deterministic. That can lead to situations where the backlight will be
powered up and the panel will show an uninitialized framebuffer.
Furthermore, subsystems such as DRM have advanced functionality to set
the power mode of a panel. In order to allow such setups to power up the
panel at exactly the right moment, a way is needed to prevent the
backlight core from powering the backlight up automatically when it is
registered.
This commit introduces a new boot_off field in the platform data (and
also implements getting the same information from device tree). When set
the initial backlight power mode will be set to "off".
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Note: Perhaps it would be more useful to make this the default behaviour
in the backlight core? Many other subsystems and frameworks assume that
a resource is off unless explicitly enabled.
---
.../devicetree/bindings/video/backlight/pwm-backlight.txt | 1 +
drivers/video/backlight/pwm_bl.c | 8 ++++++++
include/linux/pwm_backlight.h | 2 ++
3 files changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
index 3898f26..1271886 100644
--- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
@@ -16,6 +16,7 @@ Optional properties:
"pwms" property (see PWM binding[0])
- enable-gpios: a list of GPIOs to enable and disable the backlight
- power-supply: regulator for supply voltage
+ - backlight-boot-off: keep the backlight disabled on boot
[0]: Documentation/devicetree/bindings/pwm/pwm.txt
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index a2b3876..3b2d9cf 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -184,6 +184,8 @@ static int pwm_backlight_parse_dt(struct device *dev,
if (gpio_is_valid(data->enable_gpio) && (flags & OF_GPIO_ACTIVE_LOW))
data->enable_gpio_flags |= PWM_BACKLIGHT_GPIO_ACTIVE_LOW;
+ data->boot_off = of_property_read_bool(node, "backlight-boot-off");
+
return 0;
}
@@ -318,6 +320,12 @@ static int pwm_backlight_probe(struct platform_device *pdev)
}
bl->props.brightness = data->dft_brightness;
+
+ if (data->boot_off)
+ bl->props.power = FB_BLANK_POWERDOWN;
+ else
+ bl->props.power = FB_BLANK_UNBLANK;
+
backlight_update_status(bl);
platform_set_drvdata(pdev, bl);
diff --git a/include/linux/pwm_backlight.h b/include/linux/pwm_backlight.h
index 2de2e27..ea4a239 100644
--- a/include/linux/pwm_backlight.h
+++ b/include/linux/pwm_backlight.h
@@ -18,6 +18,8 @@ struct platform_pwm_backlight_data {
unsigned int *levels;
int enable_gpio;
unsigned long enable_gpio_flags;
+ bool boot_off;
+
int (*init)(struct device *dev);
int (*notify)(struct device *dev, int brightness);
void (*notify_after)(struct device *dev, int brightness);
--
1.8.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 10/10] pwm-backlight: Allow backlight to remain disabled on boot
2013-09-23 21:41 ` [PATCH 10/10] pwm-backlight: Allow backlight to remain disabled on boot Thierry Reding
@ 2013-10-01 18:50 ` Stephen Warren
2013-10-01 21:14 ` Thierry Reding
2013-10-02 17:45 ` Thierry Reding
0 siblings, 2 replies; 31+ messages in thread
From: Stephen Warren @ 2013-10-01 18:50 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks, Kukjin Kim,
Simon Horman, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel
On 09/23/2013 03:41 PM, Thierry Reding wrote:
> The default for backlight devices is to be enabled immediately when
> registering with the backlight core. This can be useful for setups that
> use a simple framebuffer device and where the backlight cannot otherwise
> be hooked up to the panel.
>
> However, when dealing with more complex setups, such as those of recent
> ARM SoCs, this can be problematic. Since the backlight is usually setup
> separately from the display controller, the probe order is not usually
> deterministic. That can lead to situations where the backlight will be
> powered up and the panel will show an uninitialized framebuffer.
>
> Furthermore, subsystems such as DRM have advanced functionality to set
> the power mode of a panel. In order to allow such setups to power up the
> panel at exactly the right moment, a way is needed to prevent the
> backlight core from powering the backlight up automatically when it is
> registered.
>
> This commit introduces a new boot_off field in the platform data (and
> also implements getting the same information from device tree). When set
> the initial backlight power mode will be set to "off".
> diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
> + - backlight-boot-off: keep the backlight disabled on boot
A few thoughts:
1) Does this property indicate that:
a) The current state of the backlight at boot. In which case, this will
need bootloader involvement to modify the value in the DT at boot time
based on whether the bootloader turned on the backlight:-(
b) That the driver should not turn on the backlight immediately? That
seems to describe driver behaviour more than HW. Is it appropriate to
put that into DT?
Your suggestion to make the backlight not turn on by default might be a
better fix?
2) Should the driver instead attempt to read the current state of the
GPIO output to determine whether the backlight is on? This may not be
possible on all HW.
3) Doesn't the following code in probe() (added in a previous patch)
need to be updated?
> + if (gpio_is_valid(pb->enable_gpio)) {
> + if (pb->enable_gpio_flags & PWM_BACKLIGHT_GPIO_ACTIVE_LOW)
> + gpio_set_value(pb->enable_gpio, 0);
> + else
> + gpio_set_value(pb->enable_gpio, 1);
> + }
... That assumes that the backlight is on at boot, and hence presumably
after this patch still always turns on the backlight, only to turn it
off very quickly once the following code in this patch executes:
(and perhaps we also need to avoid turning the backlight off then on if
it was already on at boot)
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> @@ -318,6 +320,12 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> }
>
> bl->props.brightness = data->dft_brightness;
> +
> + if (data->boot_off)
> + bl->props.power = FB_BLANK_POWERDOWN;
> + else
> + bl->props.power = FB_BLANK_UNBLANK;
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 10/10] pwm-backlight: Allow backlight to remain disabled on boot
2013-10-01 18:50 ` Stephen Warren
@ 2013-10-01 21:14 ` Thierry Reding
2013-10-02 17:45 ` Thierry Reding
1 sibling, 0 replies; 31+ messages in thread
From: Thierry Reding @ 2013-10-01 21:14 UTC (permalink / raw)
To: Stephen Warren
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks, Kukjin Kim,
Simon Horman, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 5034 bytes --]
On Tue, Oct 01, 2013 at 12:50:51PM -0600, Stephen Warren wrote:
> On 09/23/2013 03:41 PM, Thierry Reding wrote:
> > The default for backlight devices is to be enabled immediately when
> > registering with the backlight core. This can be useful for setups that
> > use a simple framebuffer device and where the backlight cannot otherwise
> > be hooked up to the panel.
> >
> > However, when dealing with more complex setups, such as those of recent
> > ARM SoCs, this can be problematic. Since the backlight is usually setup
> > separately from the display controller, the probe order is not usually
> > deterministic. That can lead to situations where the backlight will be
> > powered up and the panel will show an uninitialized framebuffer.
> >
> > Furthermore, subsystems such as DRM have advanced functionality to set
> > the power mode of a panel. In order to allow such setups to power up the
> > panel at exactly the right moment, a way is needed to prevent the
> > backlight core from powering the backlight up automatically when it is
> > registered.
> >
> > This commit introduces a new boot_off field in the platform data (and
> > also implements getting the same information from device tree). When set
> > the initial backlight power mode will be set to "off".
>
> > diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
>
> > + - backlight-boot-off: keep the backlight disabled on boot
>
> A few thoughts:
>
> 1) Does this property indicate that:
>
> a) The current state of the backlight at boot. In which case, this will
> need bootloader involvement to modify the value in the DT at boot time
> based on whether the bootloader turned on the backlight:-(
I was pretty much following the regulator bindings here. But the meaning
is different indeed, see below.
> b) That the driver should not turn on the backlight immediately? That
> seems to describe driver behaviour more than HW. Is it appropriate to
> put that into DT?
That's what it was supposed to mean. The idea is, and I mentioned that
in the commit message, that when wired up with a higher-level API such
as DRM, then usually that framework knows exactly when to enable the
backlight. Having the backlight enable itself on probe may cause the
panel to light up with no content or garbage.
> Your suggestion to make the backlight not turn on by default might be a
> better fix?
I think so too, but the problem in this case is that users of this
driver heretofore assumed that it would be turned on by default. I think
that's been common practice for all backlight drivers. Adding a property
to DT was supposed to be a way to keep backwards compatibility with any
existing users while at the same time allowing the backlight to be used
in a more "modern" system. I don't really have any other ideas how to
achieve this.
It is quite possible that the only reason why turning on the backlight
at probe time is the current default is because backlight_properties'
.power field is by default initialized to 0, which turns out to be the
value of FB_BLANK_UNBLANK. That's been the source of quite a bit of
confusion (I could tell you stories of how people tried to convince me
that there must be a bug in the Linux kernel because writing a 0 to the
backlight's bl_power field in sysfs turns the backlight on instead of
off). The same is true for DPMS modes in DRM and X, where "on" has the
value 0.
Now there have been plans to overhaul the backlight subsystem for quite
some time. One of the goals was to decouple it from the FB subsystem,
which might help solve this to some degree. At the same time sysfs is an
ABI, so we can't just go and change it randomly. The same is true for
the default behaviour of enabling the backlight at boot. That can
equally be considered an ABI and changing it will cause the majority of
devices to regress, and that's not something that I look forward to
taking the blame for...
> 2) Should the driver instead attempt to read the current state of the
> GPIO output to determine whether the backlight is on? This may not be
> possible on all HW.
>
> 3) Doesn't the following code in probe() (added in a previous patch)
> need to be updated?
>
> > + if (gpio_is_valid(pb->enable_gpio)) {
> > + if (pb->enable_gpio_flags & PWM_BACKLIGHT_GPIO_ACTIVE_LOW)
> > + gpio_set_value(pb->enable_gpio, 0);
> > + else
> > + gpio_set_value(pb->enable_gpio, 1);
> > + }
>
> ... That assumes that the backlight is on at boot, and hence presumably
> after this patch still always turns on the backlight, only to turn it
> off very quickly once the following code in this patch executes:
>
> (and perhaps we also need to avoid turning the backlight off then on if
> it was already on at boot)
Yes, that's a good point. Depending on the outcome of the above
discussion I'll update this to match the expectations.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 10/10] pwm-backlight: Allow backlight to remain disabled on boot
2013-10-01 18:50 ` Stephen Warren
2013-10-01 21:14 ` Thierry Reding
@ 2013-10-02 17:45 ` Thierry Reding
1 sibling, 0 replies; 31+ messages in thread
From: Thierry Reding @ 2013-10-02 17:45 UTC (permalink / raw)
To: Stephen Warren
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks, Kukjin Kim,
Simon Horman, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1736 bytes --]
On Tue, Oct 01, 2013 at 12:50:51PM -0600, Stephen Warren wrote:
> On 09/23/2013 03:41 PM, Thierry Reding wrote:
[...]
> > + if (gpio_is_valid(pb->enable_gpio)) {
> > + if (pb->enable_gpio_flags & PWM_BACKLIGHT_GPIO_ACTIVE_LOW)
> > + gpio_set_value(pb->enable_gpio, 0);
> > + else
> > + gpio_set_value(pb->enable_gpio, 1);
> > + }
>
> ... That assumes that the backlight is on at boot, and hence presumably
> after this patch still always turns on the backlight, only to turn it
> off very quickly once the following code in this patch executes:
I was just going to fix this, but then saw that the code in .probe() is
actually this:
if (gpio_is_valid(pb->enable_gpio)) {
unsigned long flags;
if (pb->enable_gpio_flags & PWM_BACKLIGHT_GPIO_ACTIVE_LOW)
flags = GPIOF_OUT_INIT_HIGH;
else
flags = GPIOF_OUT_INIT_LOW;
ret = gpio_request_one(pb->enable_gpio, flags, "enable");
...
}
Which sets the backlight up to be disabled by default and is consistent
with the PWM and regulator setup. Only later, depending on the value of
boot_off it gets enabled (or stays disabled).
> (and perhaps we also need to avoid turning the backlight off then on if
> it was already on at boot)
That's true. Although I'm not sure if that's even possible. I think the
pinctrl driver doesn't touch the registers, but what about the GPIO and
PWM drivers. It might be impossible to always query the boot status and
set the internal state based on that. The easiest would probably be if
both the bootloader and the kernel use the same DT, in which case the
bootloader could set the backlight-boot-on property, in which case we
wouldn't need to do anything at .probe() time really.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 00/10] pwm-backlight: Add GPIO and power supply support
2013-09-23 21:40 [PATCH 00/10] pwm-backlight: Add GPIO and power supply support Thierry Reding
` (8 preceding siblings ...)
2013-09-23 21:41 ` [PATCH 10/10] pwm-backlight: Allow backlight to remain disabled on boot Thierry Reding
@ 2013-09-24 8:14 ` Simon Horman
[not found] ` <20130924081446.GA11981-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
9 siblings, 1 reply; 31+ messages in thread
From: Simon Horman @ 2013-09-24 8:14 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Tony Lindgren, Eric Miao, Haojian Zhuang, Ben Dooks,
Kukjin Kim, Magnus Damm, Guan Xuetao, devicetree,
linux-arm-kernel, linux-omap, openezx-devel, linux-samsung-soc,
linux-sh, linux-pwm, linux-kernel, Olof Johansson, Kevin Hilman,
Arnd Bergmann
[ Cc: Olof Johansson, Kevin Hilman and Arnd Bergman: arm-soc maintainers ]
On Mon, Sep 23, 2013 at 11:40:57PM +0200, Thierry Reding wrote:
> This series adds the ability to specify a GPIO and a power supply to
> enable a backlight.
>
> Patch 1 refactors the power on and power off sequences into separate
> functions in preparation for subsequent patches.
>
> Patch 2 adds an optional GPIO to enable a backlight. This patch only
> includes the field within the platform data so that it can be properly
> setup before actually being put to use.
>
> Patches 3 to 7 convert all users of the pwm-backlight driver to use the
> new field. For most of them, this just initializes the field to -1,
> marking the field as unused.
>
> Patch 8 uses the new field within the pwm-backlight driver and at the
> same time allows it to be parsed from device tree.
>
> Patch 9 implements support for an optional power supply. This relies on
> the regulator core to return a dummy regulator when no supply has been
> otherwise setup so the driver doesn't have to handle that specially nor
> require all users to be updated.
>
> Patch 10 adds a way to keep a backlight turned off at boot. This is
> useful when hooking up a backlight with a subsystem such as DRM which
> has more explicit semantics as to when a backlight should be turned on.
>
> Due to the dependencies within the series, I propose to take all these
> patches through the PWM tree, so I'll need acks from OMAP, PXA, Samsung,
> shmobile and Unicore32 maintainers.
I received some feedback regarding shmobile conflicts when
arm-soc was merged between v3.11 and v3.12-rc1. With this
in mind I now have a strong preference for changes inside
arch/arm/mach-shmobile/ to be taken through my renesas
tree and thus more importantly through arm-soc if possible.
> Thierry
>
> Thierry Reding (10):
> pwm-backlight: Refactor backlight power on/off
> pwm-backlight: Add optional enable GPIO
> ARM: OMAP: Initialize PWM backlight enable_gpio field
> ARM: pxa: Initialize PWM backlight enable_gpio field
> ARM: SAMSUNG: Initialize PWM backlight enable_gpio field
> ARM: shmobile: Initialize PWM backlight enable_gpio field
> unicore32: Initialize PWM backlight enable_gpio field
> pwm-backlight: Use new enable_gpio field
> pwm-backlight: Use an optional power supply
> pwm-backlight: Allow backlight to remain disabled on boot
>
> .../bindings/video/backlight/pwm-backlight.txt | 6 +
> arch/arm/mach-omap2/board-zoom-peripherals.c | 1 +
> arch/arm/mach-pxa/cm-x300.c | 1 +
> arch/arm/mach-pxa/colibri-pxa270-income.c | 1 +
> arch/arm/mach-pxa/ezx.c | 1 +
> arch/arm/mach-pxa/hx4700.c | 1 +
> arch/arm/mach-pxa/lpd270.c | 1 +
> arch/arm/mach-pxa/magician.c | 1 +
> arch/arm/mach-pxa/mainstone.c | 1 +
> arch/arm/mach-pxa/mioa701.c | 1 +
> arch/arm/mach-pxa/palm27x.c | 1 +
> arch/arm/mach-pxa/palmtc.c | 35 +----
> arch/arm/mach-pxa/palmte2.c | 1 +
> arch/arm/mach-pxa/pcm990-baseboard.c | 1 +
> arch/arm/mach-pxa/raumfeld.c | 1 +
> arch/arm/mach-pxa/tavorevb.c | 2 +
> arch/arm/mach-pxa/viper.c | 1 +
> arch/arm/mach-pxa/z2.c | 2 +
> arch/arm/mach-pxa/zylonite.c | 1 +
> arch/arm/mach-s3c24xx/mach-h1940.c | 1 +
> arch/arm/mach-s3c24xx/mach-rx1950.c | 1 +
> arch/arm/mach-s3c64xx/mach-crag6410.c | 1 +
> arch/arm/mach-s3c64xx/mach-hmt.c | 1 +
> arch/arm/mach-s3c64xx/mach-smartq.c | 1 +
> arch/arm/mach-s3c64xx/mach-smdk6410.c | 1 +
> arch/arm/mach-s5p64x0/mach-smdk6440.c | 1 +
> arch/arm/mach-s5p64x0/mach-smdk6450.c | 1 +
> arch/arm/mach-s5pc100/mach-smdkc100.c | 1 +
> arch/arm/mach-s5pv210/mach-smdkv210.c | 1 +
> arch/arm/mach-shmobile/board-armadillo800eva.c | 1 +
> arch/arm/plat-samsung/dev-backlight.c | 5 +
> arch/unicore32/kernel/puv3-nb0916.c | 1 +
> drivers/video/backlight/pwm_bl.c | 142 ++++++++++++++++-----
> include/linux/pwm_backlight.h | 7 +
> 34 files changed, 162 insertions(+), 64 deletions(-)
>
> --
> 1.8.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 31+ messages in thread