Re: [PATCH] pwm-backlight: add regulator and GPIO support
From: Thierry Reding <hidden>
Date: 2012-06-30 18:37:55
Also in:
linux-tegra, lkml
On Fri, Jun 29, 2012 at 10:22:47PM +0900, Alexandre Courbot wrote:
Add support for an optional power regulator and enable/disable GPIO. This scheme is commonly used in embedded systems. Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
I've added some comments in addition to those by Stephen. [...]
quoted hunk ↗ jump to hunk
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index 057389d..821e03e 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c
[...]
quoted hunk ↗ jump to hunk
@@ -141,11 +178,14 @@ 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. - */ + ret = of_get_named_gpio(node, "enable-gpios", 0); + if (ret >= 0) { + data->enable_gpio = of_get_named_gpio(node, "enable-gpios", 0);
Can't you just reuse the value of ret here? [...]
quoted hunk ↗ jump to hunk
@@ -231,6 +271,22 @@ static int pwm_backlight_probe(struct platform_device *pdev) if (data->pwm_period_ns > 0) pwm_set_period(pb->pwm, data->pwm_period_ns); + + pb->power_reg = devm_regulator_get(&pdev->dev, "power"); + if (IS_ERR(pb->power_reg)) + return PTR_ERR(pb->power_reg); + + pb->enable_gpio = -EINVAL;
Perhaps initialize this to -1? Assigning standard error codes to a GPIO doesn't make much sense. [...]
quoted hunk ↗ jump to hunk
diff --git a/include/linux/pwm_backlight.h b/include/linux/pwm_backlight.h index 56f4a86..5ae2cd0 100644 --- a/include/linux/pwm_backlight.h +++ b/include/linux/pwm_backlight.h@@ -18,6 +18,11 @@ struct platform_pwm_backlight_data { void (*notify_after)(struct device *dev, int brightness); void (*exit)(struct device *dev); int (*check_fb)(struct device *dev, struct fb_info *info); + /* optional GPIO that enables/disables the backlight */ + int enable_gpio; + /* 0 (default initialization value) is a valid GPIO number. Make use of + * control gpio explicit to avoid bad surprises. */ + bool use_enable_gpio;
It's a shame we have to add workarounds like this... Also the canonical form to write multi-line comments would be: /* * 0 (default ... * ... surprises. */ Thierry
Attachments
- (unnamed) [application/pgp-signature] 836 bytes