backlight: add CONFIG_PM_SLEEP to suspend/resume functions
Subsystems (5)
PATCH 1/1 backlight: add CONFIG_PM_SLEEP to suspend/resume functions
Add CONFIG_PM_SLEEP to suspend/resume functions to fix the following build warning when CONFIG_PM_SLEEP is not selected. This is because sleep PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when the CONFIG_PM_SLEEP is enabled.
drivers/video/backlight/backlight.c:211:12: warning: 'backlight_suspend' defined but not used [-Wunused-function] drivers/video/backlight/backlight.c:225:12: warning: 'backlight_resume' defined but not used [-Wunused-function]
--- drivers/video/backlight/backlight.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/video/backlight/backlight.c +2 -0
@@ -208,6 +208,7 @@ static ssize_t backlight_show_actual_brightness(struct device *dev,static struct class *backlight_class; +#ifdef CONFIG_PM_SLEEP static int backlight_suspend (struct device *dev) {struct backlight_device *bd =to_backlight_device (dev);
@@ -235,6 +236,7 @@ static int backlight_resume(struct device *dev)return 0 ; } +#endif static SIMPLE_DEV_PM_OPS(backlight_class_dev_pm_ops, backlight_suspend, backlight_resume);
Signature · 1 line
1.7.10.4
Quoted 5 lines · On Friday 07 June 2013 10:39:20 Jingoo Han wrote:
Add
CONFIG_PM_SLEEPto suspend/resume functions to fix the following build warning whenCONFIG_PM_SLEEPis not selected. This is because sleep PM callbacks defined bySIMPLE_DEV_PM_OPSare only used when theCONFIG_PM_SLEEPis enabled.
Quoted 7 lines
drivers/video/backlight/backlight.c:211:12: warning: 'backlight_suspend' defined but not used [-Wunused-function]drivers/video/backlight/backlight.c:225:12: warning: 'backlight_resume' defined but not used [-Wunused-function]Signed-off-by Jingoo Han <[email protected]>--- drivers/video/backlight/backlight.c | 2 ++ 1 file changed, 2 insertions(+)
Your patch looks ok, but I find it extremely annoying to have new warnings like this one come up every single day in linux-next. It really shouldn't be this hard to use a macro called SIMPLE_DEV_PM_OPS() correctly.
Below is an implementation of SIMPLE_DEV_PM_OPS and UNIVERSAL_DEV_PM_OPS that avoids this issue by introducing an unused reference to the suspend and resume functions. gcc is smart enough to leave out that unused code by itself, and it would actually improve compile-time coverage to have something like this, besides being harder to misuse.
This would be a better approach if we didn't already have all the "#ifdef CONFIG_PM_SLEEP" in place that hide the functions now. Unfortunately we already have over 300 uses of SIMPLE_DEV_PM_OPS/UNIVERSAL_DEV_PM_OPS in the kernel today, so removing all the #ifdef atomically without creating more build errors is rather hard to do.
Maybe someone has an idea how to extend my approach so it works with and without the #ifdef, to let us transition to a situation that no longer needs them.
Arnd
include/linux/pm.h +7 -2
@@ -320,6 +320,9 @@ struct dev_pm_ops {#define SET_RUNTIME_PM_OPS (suspend_fn, resume_fn, idle_fn)#endif +#define UNUSED_DEV_PM_OPS(name, func...) \ +static const int (*__unused_ ## name[])(struct device*) __maybe_unused = { func }; +/* * Use this if you want to use the same suspend and resume callbacks for suspend * to RAM and hibernation.
@@ -327,7 +332,8 @@ struct dev_pm_ops {#define SIMPLE_DEV_PM_OPS (name, suspend_fn, resume_fn) \ const struct dev_pm_ops name = { \ SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \ -} +}; \ +UNUSED_DEV_PM_OPS(name, suspend_fn, resume_fn)/* * Use this for defining a set of PM operations to be used in all situations
@@ -346,7 +352,8 @@ const struct dev_pm_ops name = { \const struct dev_pm_ops name = { \ SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \SET_RUNTIME_PM_OPS (suspend_fn, resume_fn, idle_fn) \ -} +}; \ +UNUSED_DEV_PM_OPS (name, suspend_fn, resume_fn, idle_fn)/** * PM_EVENT_ messages
Quoted 30 lines · On Fri, 07 Jun 2013 12:02:31 +0200 Arnd Bergmann <[email protected]> wrote:
Quoted 5 lines · On Friday 07 June 2013 10:39:20 Jingoo Han wrote:
Add
CONFIG_PM_SLEEPto suspend/resume functions to fix the following build warning whenCONFIG_PM_SLEEPis not selected. This is because sleep PM callbacks defined bySIMPLE_DEV_PM_OPSare only used when theCONFIG_PM_SLEEPis enabled.Quoted 7 lines
drivers/video/backlight/backlight.c:211:12: warning: 'backlight_suspend' defined but not used [-Wunused-function]drivers/video/backlight/backlight.c:225:12: warning: 'backlight_resume' defined but not used [-Wunused-function]Signed-off-by Jingoo Han <[email protected]>--- drivers/video/backlight/backlight.c | 2 ++ 1 file changed, 2 insertions(+)Your patch looks ok, but I find it extremely annoying to have new warnings like this one come up every single day in linux-next. It really shouldn't be this hard to use a macro called
SIMPLE_DEV_PM_OPS()correctly.Below is an implementation of
SIMPLE_DEV_PM_OPSandUNIVERSAL_DEV_PM_OPSthat avoids this issue by introducing an unused reference to the suspend and resume functions. gcc is smart enough to leave out that unused code by itself, and it would actually improve compile-time coverage to have something like this, besides being harder to misuse.This would be a better approach if we didn't already have all the "#ifdef
CONFIG_PM_SLEEP" in place that hide the functions now. Unfortunately we already have over 300 uses of SIMPLE_DEV_PM_OPS/UNIVERSAL_DEV_PM_OPS in the kernel today, so removing all the #ifdef atomically without creating more build errors is rather hard to do.
Quoted 3 lines
Maybe someone has an idea how to extend my approach so it works with and without the #ifdef, to let us transition to a situation that no longer needs them.
You could create new macros, and add a checkpatch rule to remind people to not use the old ones. Then people can migrate over from the old macros at a leisurely pace.
The problem will be in thinking up decent names for the new macros.
Quoted 36 lines · Andrew Morton
Quoted 30 lines · On Fri, 07 Jun 2013 12:02:31 +0200 Arnd Bergmann <[email protected]> wrote:
Quoted 5 lines · On Friday 07 June 2013 10:39:20 Jingoo Han wrote:
Add
CONFIG_PM_SLEEPto suspend/resume functions to fix the following build warning whenCONFIG_PM_SLEEPis not selected. This is because sleep PM callbacks defined bySIMPLE_DEV_PM_OPSare only used when theCONFIG_PM_SLEEPis enabled.Quoted 7 lines
drivers/video/backlight/backlight.c:211:12: warning: 'backlight_suspend' defined but not used [-Wunused-function]drivers/video/backlight/backlight.c:225:12: warning: 'backlight_resume' defined but not used [-Wunused-function]Signed-off-by Jingoo Han <[email protected]>--- drivers/video/backlight/backlight.c | 2 ++ 1 file changed, 2 insertions(+)Your patch looks ok, but I find it extremely annoying to have new warnings like this one come up every single day in linux-next. It really shouldn't be this hard to use a macro called
SIMPLE_DEV_PM_OPS()correctly.Below is an implementation of
SIMPLE_DEV_PM_OPSandUNIVERSAL_DEV_PM_OPSthat avoids this issue by introducing an unused reference to the suspend and resume functions. gcc is smart enough to leave out that unused code by itself, and it would actually improve compile-time coverage to have something like this, besides being harder to misuse.This would be a better approach if we didn't already have all the "#ifdef
CONFIG_PM_SLEEP" in place that hide the functions now. Unfortunately we already have over 300 uses of SIMPLE_DEV_PM_OPS/UNIVERSAL_DEV_PM_OPS in the kernel today, so removing all the #ifdef atomically without creating more build errors is rather hard to do.Quoted 3 lines
Maybe someone has an idea how to extend my approach so it works with and without the #ifdef, to let us transition to a situation that no longer needs them.
Quoted 5 lines
You could create new macros, and add a checkpatch rule to remind people to not use the old ones. Then people can migrate over from the old macros at a leisurely pace.
The problem will be in thinking up decent names for the new macros.
Agreed
Best Regards,
J.
Quoted 9 lines · On Friday 07 June 2013 10:39:20 Jingoo Han wrote:
Add
CONFIG_PM_SLEEPto suspend/resume functions to fix the following build warning whenCONFIG_PM_SLEEPis not selected. This is because sleep PM callbacks defined bySIMPLE_DEV_PM_OPSare only used when theCONFIG_PM_SLEEPis enabled.
drivers/video/backlight/backlight.c:211:12: warning: 'backlight_suspend' defined but not used [-Wunused-function]drivers/video/backlight/backlight.c:225:12: warning: 'backlight_resume' defined but not used [-Wunused-function]Signed-off-by Jingoo Han <[email protected]>
I forgot:
Quoted 6 lines · Arnd Bergmann
Quoted 5 lines · On Friday 07 June 2013 10:39:20 Jingoo Han wrote:
Add
CONFIG_PM_SLEEPto suspend/resume functions to fix the following build warning whenCONFIG_PM_SLEEPis not selected. This is because sleep PM callbacks defined bySIMPLE_DEV_PM_OPSare only used when theCONFIG_PM_SLEEPis enabled.
Quoted 9 lines
Quoted 4 lines
drivers/video/backlight/backlight.c:211:12: warning: 'backlight_suspend' defined but not used [-Wunused-function]drivers/video/backlight/backlight.c:225:12: warning: 'backlight_resume' defined but not used [-Wunused-function]Signed-off-by Jingoo Han <[email protected]>I forgot:
Acked-by Arnd Bergmann <[email protected]>
Jingoo Han, and Arnd,
Sorry I was on vacation all of last week. Thanks for fixing the problem. My apologies for introducing the warnings. I had CONFIG_PM and CONFIG_PM_SLEEP both enabled and didn't see this warning. I will be careful with subsequent patches in this area.
-- Shuah
Shuah Khan, Linux Kernel Developer - Open Source Group Samsung Research America (Silicon Valley) [email protected] | (970) 672-0658