video: Added a callback 'notify_after' for backlight control

Started by unknown sender on Aug 8, 2011 · 2 messages from 2 authors · Last reply Aug 8, 2011

Subsystems (3)

Maintainers (5)

Show 1 more
STALE5577d

open the first message on its own page

Message details and related content
From: Dilan Lee <redacted>

We need a callback to do some things after pwm_enable,pwm_disable
and pwm_config.

For example, GPIO backlight_en has to be rised after pwm has been enabled
to meet panel power on sequence defined in panel specification.

Signed-off-by: Dilan Lee <redacted>
---
 drivers/video/backlight/pwm_bl.c |    9 +++++++++
 include/linux/pwm_backlight.h    |    1 +
 2 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index b8f38ec..d9a16e5 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -28,6 +28,8 @@ struct pwm_bl_data {
 	unsigned int		lth_brightness;
 	int			(*notify)(struct device *,
 					  int brightness);
+	void		(*notify_after)(struct device *,
+					  int brightness);
 	int			(*check_fb)(struct device *, struct fb_info *);
 };
 
@@ -55,6 +57,10 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
 		pwm_config(pb->pwm, brightness, pb->period);
 		pwm_enable(pb->pwm);
 	}
+
+	if (pb->notify_after)
+		pb->notify_after(pb->dev, brightness);
+
 	return 0;
 }
 
@@ -105,6 +111,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 
 	pb->period = data->pwm_period_ns;
 	pb->notify = data->notify;
+	pb->notify_after = data->notify_after;
 	pb->check_fb = data->check_fb;
 	pb->lth_brightness = data->lth_brightness *
 		(data->pwm_period_ns / data->max_brightness);
@@ -172,6 +179,8 @@ static int pwm_backlight_suspend(struct platform_device *pdev,
 		pb->notify(pb->dev, 0);
 	pwm_config(pb->pwm, 0, pb->period);
 	pwm_disable(pb->pwm);
+	if (pb->notify_after)
+		pb->notify_after(pb->dev, 0);
 	return 0;
 }
 
diff --git a/include/linux/pwm_backlight.h b/include/linux/pwm_backlight.h
index 5e3e25a..63d2df4 100644
--- a/include/linux/pwm_backlight.h
+++ b/include/linux/pwm_backlight.h
@@ -14,6 +14,7 @@ struct platform_pwm_backlight_data {
 	unsigned int pwm_period_ns;
 	int (*init)(struct device *dev);
 	int (*notify)(struct device *dev, int brightness);
+	void (*notify_after)(struct device *dev, int brightness);
 	void (*exit)(struct device *dev);
 	int (*check_fb)(struct device *dev, struct fb_info *info);
 };
-- 
1.7.5.4
Message details and related content
Looks fine to me in principle, but a few nitpicks.
First, the short commit message should be in imperative form, i.e., "Add
a callback..."

On Mon, Aug 08, 2011 at 03:37:55AM -0700, Dilan Lee wrote:
From: Dilan Lee <redacted>

We need a callback to do some things after pwm_enable,pwm_disable
and pwm_config.
Add a space between "pwm_enable,pwm_disable"
For example, GPIO backlight_en has to be rised after pwm has been enabled
to meet panel power on sequence defined in panel specification.
There's a typo in "raised" above, but really, this paragraph should be
omitted entirely.  Without context, it doesn't make any sense.  If
more motivation that the previous paragraph is necessary, maybe add
something like "This may be necessary to properly sequence timing on
certain devices."
quoted hunk
 	unsigned int		lth_brightness;
 	int			(*notify)(struct device *,
 					  int brightness);
+	void		(*notify_after)(struct device *,
+					  int brightness);
The indentation here looks a bit off; notify_after should be lined up
with notify and check_fb, and int brightness doesn't need the extra
spaces since the notify_after argument list starts on a tab boundary.

Otherwise,

Reviewed-by: Robert Morell <redacted>

Thanks,
Robert
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help