From dfcba200679dc3f62212154b65b40b835ce69ab7 Mon Sep 17 00:00:00 2001 From: Richard Purdie Date: Thu, 8 Feb 2007 00:06:32 +0000 Subject: backlight: Remove unneeded owner field Remove uneeded owner field from backlight_properties structure. Nothing uses it and it is unlikely that it will ever be used. The backlight class uses other means to ensure that nothing references unloaded code. Based on a patch from Dmitry Torokhov Signed-off-by: Richard Purdie --- include/linux/backlight.h | 3 --- include/linux/lcd.h | 2 -- 2 files changed, 5 deletions(-) (limited to 'include/linux') diff --git a/include/linux/backlight.h b/include/linux/backlight.h index a5cf1beacb44..287c62d956f2 100644 --- a/include/linux/backlight.h +++ b/include/linux/backlight.h @@ -17,9 +17,6 @@ struct fb_info; /* This structure defines all the properties of a backlight (usually attached to a LCD). */ struct backlight_properties { - /* Owner module */ - struct module *owner; - /* Notify the backlight driver some property has changed */ int (*update_status)(struct backlight_device *); /* Return the current backlight brightness (accounting for power, diff --git a/include/linux/lcd.h b/include/linux/lcd.h index d739b2e7eac2..8a468f168c45 100644 --- a/include/linux/lcd.h +++ b/include/linux/lcd.h @@ -16,8 +16,6 @@ struct fb_info; /* This structure defines all the properties of a LCD flat panel. */ struct lcd_properties { - /* Owner module */ - struct module *owner; /* Get the LCD panel power status (0: full on, 1..3: controller power on, flat panel power off, 4: full off), see FB_BLANK_XXX */ int (*get_power)(struct lcd_device *); -- cgit v1.2.2 From 28ee086d5b36aab2931f6740e409bb0fb6c65e5f Mon Sep 17 00:00:00 2001 From: Richard Purdie Date: Thu, 8 Feb 2007 22:25:09 +0000 Subject: backlight: Fix external uses of backlight internal semaphore backlight_device->sem has a very specific use as documented in the header file. The external users of this are using it for a different reason, to serialise access to the update_status() method. backlight users were supposed to implement their own internal serialisation of update_status() if needed but everyone is doing things differently and incorrectly. Therefore add a global mutex to take care of serialisation for everyone, once and for all. Locking for get_brightness remains optional since most users don't need it. Also update the lcd class in a similar way. Signed-off-by: Richard Purdie --- include/linux/backlight.h | 26 ++++++++++++++++++++++++++ include/linux/lcd.h | 26 ++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) (limited to 'include/linux') diff --git a/include/linux/backlight.h b/include/linux/backlight.h index 287c62d956f2..d1426b852bdf 100644 --- a/include/linux/backlight.h +++ b/include/linux/backlight.h @@ -9,8 +9,24 @@ #define _LINUX_BACKLIGHT_H #include +#include #include +/* Notes on locking: + * + * backlight_device->sem is an internal backlight lock protecting the props + * field and no code outside the core should need to touch it. + * + * Access to update_status() is serialised by the update_lock mutex since + * most drivers seem to need this and historically get it wrong. + * + * Most drivers don't need locking on their get_brightness() method. + * If yours does, you need to implement it in the driver. You can use the + * update_lock mutex if appropriate. + * + * Any other use of the locks below is probably wrong. + */ + struct backlight_device; struct fb_info; @@ -44,12 +60,22 @@ struct backlight_device { struct semaphore sem; /* If this is NULL, the backing module is unloaded */ struct backlight_properties *props; + /* Serialise access to update_status method */ + struct mutex update_lock; /* The framebuffer notifier block */ struct notifier_block fb_notif; /* The class device structure */ struct class_device class_dev; }; +static inline void backlight_update_status(struct backlight_device *bd) +{ + mutex_lock(&bd->update_lock); + if (bd->props && bd->props->update_status) + bd->props->update_status(bd); + mutex_unlock(&bd->update_lock); +} + extern struct backlight_device *backlight_device_register(const char *name, struct device *dev,void *devdata,struct backlight_properties *bp); extern void backlight_device_unregister(struct backlight_device *bd); diff --git a/include/linux/lcd.h b/include/linux/lcd.h index 8a468f168c45..bfbf6552eb51 100644 --- a/include/linux/lcd.h +++ b/include/linux/lcd.h @@ -9,8 +9,24 @@ #define _LINUX_LCD_H #include +#include #include +/* Notes on locking: + * + * lcd_device->sem is an internal backlight lock protecting the props + * field and no code outside the core should need to touch it. + * + * Access to set_power() is serialised by the update_lock mutex since + * most drivers seem to need this and historically get it wrong. + * + * Most drivers don't need locking on their get_power() method. + * If yours does, you need to implement it in the driver. You can use the + * update_lock mutex if appropriate. + * + * Any other use of the locks below is probably wrong. + */ + struct lcd_device; struct fb_info; @@ -39,12 +55,22 @@ struct lcd_device { struct semaphore sem; /* If this is NULL, the backing module is unloaded */ struct lcd_properties *props; + /* Serialise access to set_power method */ + struct mutex update_lock; /* The framebuffer notifier block */ struct notifier_block fb_notif; /* The class device structure */ struct class_device class_dev; }; +static inline void lcd_set_power(struct lcd_device *ld, int power) +{ + mutex_lock(&ld->update_lock); + if (ld->props && ld->props->set_power) + ld->props->set_power(ld, power); + mutex_unlock(&ld->update_lock); +} + extern struct lcd_device *lcd_device_register(const char *name, void *devdata, struct lcd_properties *lp); extern void lcd_device_unregister(struct lcd_device *ld); -- cgit v1.2.2 From 249040dc7fd391186f420fe23a9b59d357103cac Mon Sep 17 00:00:00 2001 From: Richard Purdie Date: Thu, 8 Feb 2007 22:53:55 +0000 Subject: backlight: Convert semaphore -> mutex Convert internal semaphore to a mutex Signed-off-by: Richard Purdie --- include/linux/backlight.h | 6 +++--- include/linux/lcd.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'include/linux') diff --git a/include/linux/backlight.h b/include/linux/backlight.h index d1426b852bdf..43c6d55644b5 100644 --- a/include/linux/backlight.h +++ b/include/linux/backlight.h @@ -14,8 +14,8 @@ /* Notes on locking: * - * backlight_device->sem is an internal backlight lock protecting the props - * field and no code outside the core should need to touch it. + * backlight_device->props_lock is an internal backlight lock protecting the + * props field and no code outside the core should need to touch it. * * Access to update_status() is serialised by the update_lock mutex since * most drivers seem to need this and historically get it wrong. @@ -57,7 +57,7 @@ struct backlight_device { /* This protects the 'props' field. If 'props' is NULL, the driver that registered this device has been unloaded, and if class_get_devdata() points to something in the body of that driver, it is also invalid. */ - struct semaphore sem; + struct mutex props_lock; /* If this is NULL, the backing module is unloaded */ struct backlight_properties *props; /* Serialise access to update_status method */ diff --git a/include/linux/lcd.h b/include/linux/lcd.h index bfbf6552eb51..46970af2ca89 100644 --- a/include/linux/lcd.h +++ b/include/linux/lcd.h @@ -14,7 +14,7 @@ /* Notes on locking: * - * lcd_device->sem is an internal backlight lock protecting the props + * lcd_device->props_lock is an internal backlight lock protecting the props * field and no code outside the core should need to touch it. * * Access to set_power() is serialised by the update_lock mutex since @@ -52,7 +52,7 @@ struct lcd_device { /* This protects the 'props' field. If 'props' is NULL, the driver that registered this device has been unloaded, and if class_get_devdata() points to something in the body of that driver, it is also invalid. */ - struct semaphore sem; + struct mutex props_lock; /* If this is NULL, the backing module is unloaded */ struct lcd_properties *props; /* Serialise access to set_power method */ -- cgit v1.2.2 From 994efacdf9a087b52f71e620b58dfa526b0cf928 Mon Sep 17 00:00:00 2001 From: Richard Purdie Date: Fri, 9 Feb 2007 09:46:45 +0000 Subject: backlight/fbcon: Add FB_EVENT_CONBLANK The backlight class wants notification whenever the console is blanked but doesn't get this when hardware blanking fails and software blanking is used. Changing FB_EVENT_BLANK to report both would be a behaviour change which could confuse the console layer so add a new event for software blanking and have the backlight class listen for both. Signed-off-by: Richard Purdie --- include/linux/fb.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'include/linux') diff --git a/include/linux/fb.h b/include/linux/fb.h index a78e25683f82..bf7158b59b25 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -516,13 +516,15 @@ struct fb_cursor_user { #define FB_EVENT_GET_CONSOLE_MAP 0x07 /* CONSOLE-SPECIFIC: set console to framebuffer mapping */ #define FB_EVENT_SET_CONSOLE_MAP 0x08 -/* A display blank is requested */ +/* A hardware display blank change occured */ #define FB_EVENT_BLANK 0x09 /* Private modelist is to be replaced */ #define FB_EVENT_NEW_MODELIST 0x0A /* The resolution of the passed in fb_info about to change and all vc's should be changed */ #define FB_EVENT_MODE_CHANGE_ALL 0x0B +/* A software display blank change occured */ +#define FB_EVENT_CONBLANK 0x0C struct fb_event { struct fb_info *info; -- cgit v1.2.2 From 37ce69a57ff217a4ca0871e9ee5aa58c052b7d86 Mon Sep 17 00:00:00 2001 From: Richard Purdie Date: Sat, 10 Feb 2007 14:10:33 +0000 Subject: backlight: Rework backlight/fb interaction simplifying, lots fb_info->bl_mutex is badly thought out and the backlight class doesn't need it if the framebuffer/backlight register/unregister order is consistent, particularly after the backlight locking fixes. Fix the drivers to use the order: backlight_device_register() register_framebuffer() unregister_framebuffer() backlight_device_unregister() and turn bl_mutex into a lock for the bl_curve data only. Signed-off-by: Richard Purdie --- include/linux/fb.h | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'include/linux') diff --git a/include/linux/fb.h b/include/linux/fb.h index bf7158b59b25..be913ec87169 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -769,16 +769,13 @@ struct fb_info { struct fb_videomode *mode; /* current mode */ #ifdef CONFIG_FB_BACKLIGHT - /* Lock ordering: - * bl_mutex (protects bl_dev and bl_curve) - * bl_dev->sem (backlight class) - */ - struct mutex bl_mutex; - /* assigned backlight device */ + /* set before framebuffer registration, + remove after unregister */ struct backlight_device *bl_dev; /* Backlight level curve */ + struct mutex bl_curve_mutex; u8 bl_curve[FB_BACKLIGHT_LEVELS]; #endif -- cgit v1.2.2 From 599a52d12629394236d785615808845823875868 Mon Sep 17 00:00:00 2001 From: Richard Purdie Date: Sat, 10 Feb 2007 23:07:48 +0000 Subject: backlight: Separate backlight properties from backlight ops pointers Per device data such as brightness belongs to the indivdual device and should therefore be separate from the the backlight operation function pointers. This patch splits the two types of data and allows simplifcation of some code. Signed-off-by: Richard Purdie --- include/linux/backlight.h | 33 +++++++++++++++++++-------------- include/linux/lcd.h | 23 +++++++++++++---------- 2 files changed, 32 insertions(+), 24 deletions(-) (limited to 'include/linux') diff --git a/include/linux/backlight.h b/include/linux/backlight.h index 43c6d55644b5..1023ba0d6e55 100644 --- a/include/linux/backlight.h +++ b/include/linux/backlight.h @@ -14,8 +14,8 @@ /* Notes on locking: * - * backlight_device->props_lock is an internal backlight lock protecting the - * props field and no code outside the core should need to touch it. + * backlight_device->ops_lock is an internal backlight lock protecting the + * ops pointer and no code outside the core should need to touch it. * * Access to update_status() is serialised by the update_lock mutex since * most drivers seem to need this and historically get it wrong. @@ -30,9 +30,7 @@ struct backlight_device; struct fb_info; -/* This structure defines all the properties of a backlight - (usually attached to a LCD). */ -struct backlight_properties { +struct backlight_ops { /* Notify the backlight driver some property has changed */ int (*update_status)(struct backlight_device *); /* Return the current backlight brightness (accounting for power, @@ -41,7 +39,10 @@ struct backlight_properties { /* Check if given framebuffer device is the one bound to this backlight; return 0 if not, !=0 if it is. If NULL, backlight always matches the fb. */ int (*check_fb)(struct fb_info *); +}; +/* This structure defines all the properties of a backlight */ +struct backlight_properties { /* Current User requested brightness (0 - max_brightness) */ int brightness; /* Maximal value for brightness (read-only) */ @@ -54,14 +55,18 @@ struct backlight_properties { }; struct backlight_device { - /* This protects the 'props' field. If 'props' is NULL, the driver that - registered this device has been unloaded, and if class_get_devdata() - points to something in the body of that driver, it is also invalid. */ - struct mutex props_lock; - /* If this is NULL, the backing module is unloaded */ - struct backlight_properties *props; + /* Backlight properties */ + struct backlight_properties props; + /* Serialise access to update_status method */ struct mutex update_lock; + + /* This protects the 'ops' field. If 'ops' is NULL, the driver that + registered this device has been unloaded, and if class_get_devdata() + points to something in the body of that driver, it is also invalid. */ + struct mutex ops_lock; + struct backlight_ops *ops; + /* The framebuffer notifier block */ struct notifier_block fb_notif; /* The class device structure */ @@ -71,13 +76,13 @@ struct backlight_device { static inline void backlight_update_status(struct backlight_device *bd) { mutex_lock(&bd->update_lock); - if (bd->props && bd->props->update_status) - bd->props->update_status(bd); + if (bd->ops && bd->ops->update_status) + bd->ops->update_status(bd); mutex_unlock(&bd->update_lock); } extern struct backlight_device *backlight_device_register(const char *name, - struct device *dev,void *devdata,struct backlight_properties *bp); + struct device *dev, void *devdata, struct backlight_ops *ops); extern void backlight_device_unregister(struct backlight_device *bd); #define to_backlight_device(obj) container_of(obj, struct backlight_device, class_dev) diff --git a/include/linux/lcd.h b/include/linux/lcd.h index 46970af2ca89..598793c0745b 100644 --- a/include/linux/lcd.h +++ b/include/linux/lcd.h @@ -14,7 +14,7 @@ /* Notes on locking: * - * lcd_device->props_lock is an internal backlight lock protecting the props + * lcd_device->ops_lock is an internal backlight lock protecting the ops * field and no code outside the core should need to touch it. * * Access to set_power() is serialised by the update_lock mutex since @@ -30,15 +30,17 @@ struct lcd_device; struct fb_info; -/* This structure defines all the properties of a LCD flat panel. */ struct lcd_properties { + /* The maximum value for contrast (read-only) */ + int max_contrast; +}; + +struct lcd_ops { /* Get the LCD panel power status (0: full on, 1..3: controller power on, flat panel power off, 4: full off), see FB_BLANK_XXX */ int (*get_power)(struct lcd_device *); /* Enable or disable power to the LCD (0: on; 4: off, see FB_BLANK_XXX) */ int (*set_power)(struct lcd_device *, int power); - /* The maximum value for contrast (read-only) */ - int max_contrast; /* Get the current contrast setting (0-max_contrast) */ int (*get_contrast)(struct lcd_device *); /* Set LCD panel contrast */ @@ -49,12 +51,13 @@ struct lcd_properties { }; struct lcd_device { - /* This protects the 'props' field. If 'props' is NULL, the driver that + struct lcd_properties props; + /* This protects the 'ops' field. If 'ops' is NULL, the driver that registered this device has been unloaded, and if class_get_devdata() points to something in the body of that driver, it is also invalid. */ - struct mutex props_lock; + struct mutex ops_lock; /* If this is NULL, the backing module is unloaded */ - struct lcd_properties *props; + struct lcd_ops *ops; /* Serialise access to set_power method */ struct mutex update_lock; /* The framebuffer notifier block */ @@ -66,13 +69,13 @@ struct lcd_device { static inline void lcd_set_power(struct lcd_device *ld, int power) { mutex_lock(&ld->update_lock); - if (ld->props && ld->props->set_power) - ld->props->set_power(ld, power); + if (ld->ops && ld->ops->set_power) + ld->ops->set_power(ld, power); mutex_unlock(&ld->update_lock); } extern struct lcd_device *lcd_device_register(const char *name, - void *devdata, struct lcd_properties *lp); + void *devdata, struct lcd_ops *ops); extern void lcd_device_unregister(struct lcd_device *ld); #define to_lcd_device(obj) container_of(obj, struct lcd_device, class_dev) -- cgit v1.2.2