With current implementation of PM_OPS initializers users should annotate
all PM callbacks with __maybe_unused attribute to prevent compiler from
complaining in case respective option is not enabled. Using ternary
operator with IS_ENABLED(symbol) as a condition allows to avoid marking
these functions with __maybe_unused.
Solution was tested successfully with multiple versions of gcc since
4.9.4 up to 7.2.1. No functional changes has been observed and callbacks
were compiled out if not used, as before.
Signed-off-by: Andrzej Hajda <[email protected]>
---
include/linux/pm.h | 61 ++++++++++++++++++----------------------------
1 file changed, 24 insertions(+), 37 deletions(-)
diff --git a/include/linux/pm.h b/include/linux/pm.h
index e723b78d8357..59f333922c15 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -313,50 +313,37 @@ struct dev_pm_ops {
int (*runtime_idle)(struct device *dev);
};
-#ifdef CONFIG_PM_SLEEP
+#define PM_SLEEP_PTR(ptr) (IS_ENABLED(CONFIG_PM_SLEEP) ? (ptr) : NULL)
+#define PM_PTR(ptr) (IS_ENABLED(CONFIG_PM) ? (ptr) : NULL)
+
#define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
- .suspend = suspend_fn, \
- .resume = resume_fn, \
- .freeze = suspend_fn, \
- .thaw = resume_fn, \
- .poweroff = suspend_fn, \
- .restore = resume_fn,
-#else
-#define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
-#endif
+ .suspend = PM_SLEEP_PTR(suspend_fn), \
+ .resume = PM_SLEEP_PTR(resume_fn), \
+ .freeze = PM_SLEEP_PTR(suspend_fn), \
+ .thaw = PM_SLEEP_PTR(resume_fn), \
+ .poweroff = PM_SLEEP_PTR(suspend_fn), \
+ .restore = PM_SLEEP_PTR(resume_fn),
-#ifdef CONFIG_PM_SLEEP
#define SET_LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
- .suspend_late = suspend_fn, \
- .resume_early = resume_fn, \
- .freeze_late = suspend_fn, \
- .thaw_early = resume_fn, \
- .poweroff_late = suspend_fn, \
- .restore_early = resume_fn,
-#else
-#define SET_LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
-#endif
+ .suspend_late = PM_SLEEP_PTR(suspend_fn), \
+ .resume_early = PM_SLEEP_PTR(resume_fn), \
+ .freeze_late = PM_SLEEP_PTR(suspend_fn), \
+ .thaw_early = PM_SLEEP_PTR(resume_fn), \
+ .poweroff_late = PM_SLEEP_PTR(suspend_fn), \
+ .restore_early = PM_SLEEP_PTR(resume_fn),
-#ifdef CONFIG_PM_SLEEP
#define SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
- .suspend_noirq = suspend_fn, \
- .resume_noirq = resume_fn, \
- .freeze_noirq = suspend_fn, \
- .thaw_noirq = resume_fn, \
- .poweroff_noirq = suspend_fn, \
- .restore_noirq = resume_fn,
-#else
-#define SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
-#endif
+ .suspend_noirq = PM_SLEEP_PTR(suspend_fn), \
+ .resume_noirq = PM_SLEEP_PTR(resume_fn), \
+ .freeze_noirq = PM_SLEEP_PTR(suspend_fn), \
+ .thaw_noirq = PM_SLEEP_PTR(resume_fn), \
+ .poweroff_noirq = PM_SLEEP_PTR(suspend_fn), \
+ .restore_noirq = PM_SLEEP_PTR(resume_fn),
-#ifdef CONFIG_PM
#define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
- .runtime_suspend = suspend_fn, \
- .runtime_resume = resume_fn, \
- .runtime_idle = idle_fn,
-#else
-#define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn)
-#endif
+ .runtime_suspend = PM_PTR(suspend_fn), \
+ .runtime_resume = PM_PTR(resume_fn), \
+ .runtime_idle = PM_PTR(idle_fn),
/*
* Use this if you want to use the same suspend and resume callbacks for suspend
--
2.17.0
Hi Andrzej,
I love your patch! Perhaps something to improve:
[auto build test WARNING on pm/linux-next]
[also build test WARNING on v4.17-rc5 next-20180511]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Andrzej-Hajda/PM-core-refactor-PM_OPS-initializers/20180514-172201
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: openrisc-or1ksim_defconfig (attached as .config)
compiler: or1k-linux-gcc (GCC) 6.0.0 20160327 (experimental)
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=openrisc
All warnings (new ones prefixed by >>):
In file included from include/linux/device.h:23:0,
from include/linux/cdev.h:8,
from include/linux/tty_driver.h:245,
from include/linux/tty.h:9,
from include/linux/serial_core.h:29,
from drivers/tty/serial/8250/8250_of.c:11:
drivers/tty/serial/8250/8250_of.c:293:44: error: 'of_serial_suspend' undeclared here (not in a function)
static SIMPLE_DEV_PM_OPS(of_serial_pm_ops, of_serial_suspend, of_serial_resume);
^
include/linux/pm.h:316:59: note: in definition of macro 'PM_SLEEP_PTR'
#define PM_SLEEP_PTR(ptr) (IS_ENABLED(CONFIG_PM_SLEEP) ? (ptr) : NULL)
^~~
include/linux/pm.h:354:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/tty/serial/8250/8250_of.c:293:8: note: in expansion of macro 'SIMPLE_DEV_PM_OPS'
static SIMPLE_DEV_PM_OPS(of_serial_pm_ops, of_serial_suspend, of_serial_resume);
^~~~~~~~~~~~~~~~~
drivers/tty/serial/8250/8250_of.c:293:63: error: 'of_serial_resume' undeclared here (not in a function)
static SIMPLE_DEV_PM_OPS(of_serial_pm_ops, of_serial_suspend, of_serial_resume);
^
include/linux/pm.h:316:59: note: in definition of macro 'PM_SLEEP_PTR'
#define PM_SLEEP_PTR(ptr) (IS_ENABLED(CONFIG_PM_SLEEP) ? (ptr) : NULL)
^~~
include/linux/pm.h:354:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/tty/serial/8250/8250_of.c:293:8: note: in expansion of macro 'SIMPLE_DEV_PM_OPS'
static SIMPLE_DEV_PM_OPS(of_serial_pm_ops, of_serial_suspend, of_serial_resume);
^~~~~~~~~~~~~~~~~
vim +/SIMPLE_DEV_PM_OPS +293 drivers/tty/serial/8250/8250_of.c
8ad3b1352 drivers/tty/serial/of_serial.c Jingchang Lu 2014-11-11 276
aa42db44f drivers/tty/serial/8250/8250_of.c Arnd Bergmann 2017-01-25 277 static int of_serial_resume(struct device *dev)
8ad3b1352 drivers/tty/serial/of_serial.c Jingchang Lu 2014-11-11 278 {
aa42db44f drivers/tty/serial/8250/8250_of.c Arnd Bergmann 2017-01-25 279 struct of_serial_info *info = dev_get_drvdata(dev);
8ad3b1352 drivers/tty/serial/of_serial.c Jingchang Lu 2014-11-11 280 struct uart_8250_port *port8250 = serial8250_get_port(info->line);
8ad3b1352 drivers/tty/serial/of_serial.c Jingchang Lu 2014-11-11 281 struct uart_port *port = &port8250->port;
8ad3b1352 drivers/tty/serial/of_serial.c Jingchang Lu 2014-11-11 282
a2d23edae drivers/tty/serial/8250/8250_of.c Franklin S Cooper Jr 2017-08-16 283 if (!uart_console(port) || console_suspend_enabled) {
a2d23edae drivers/tty/serial/8250/8250_of.c Franklin S Cooper Jr 2017-08-16 284 pm_runtime_get_sync(dev);
8ad3b1352 drivers/tty/serial/of_serial.c Jingchang Lu 2014-11-11 285 clk_prepare_enable(info->clk);
a2d23edae drivers/tty/serial/8250/8250_of.c Franklin S Cooper Jr 2017-08-16 286 }
8ad3b1352 drivers/tty/serial/of_serial.c Jingchang Lu 2014-11-11 287
8ad3b1352 drivers/tty/serial/of_serial.c Jingchang Lu 2014-11-11 288 serial8250_resume_port(info->line);
8ad3b1352 drivers/tty/serial/of_serial.c Jingchang Lu 2014-11-11 289
8ad3b1352 drivers/tty/serial/of_serial.c Jingchang Lu 2014-11-11 290 return 0;
8ad3b1352 drivers/tty/serial/of_serial.c Jingchang Lu 2014-11-11 291 }
8ad3b1352 drivers/tty/serial/of_serial.c Jingchang Lu 2014-11-11 292 #endif
8ad3b1352 drivers/tty/serial/of_serial.c Jingchang Lu 2014-11-11 @293 static SIMPLE_DEV_PM_OPS(of_serial_pm_ops, of_serial_suspend, of_serial_resume);
8ad3b1352 drivers/tty/serial/of_serial.c Jingchang Lu 2014-11-11 294
:::::: The code at line 293 was first introduced by commit
:::::: 8ad3b1352693972b737607c7b9c89b56d45fea9b serial: of-serial: add PM suspend/resume support
:::::: TO: Jingchang Lu <[email protected]>
:::::: CC: Greg Kroah-Hartman <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Andrzej,
I love your patch! Yet something to improve:
[auto build test ERROR on pm/linux-next]
[also build test ERROR on v4.17-rc5 next-20180511]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Andrzej-Hajda/PM-core-refactor-PM_OPS-initializers/20180514-172201
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: ia64-defconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64
All error/warnings (new ones prefixed by >>):
In file included from include/linux/device.h:23:0,
from include/linux/pci.h:31,
from drivers/net//ethernet/broadcom/tg3.c:33:
>> drivers/net//ethernet/broadcom/tg3.c:18150:38: error: 'tg3_suspend' undeclared here (not in a function); did you mean 'phy_suspend'?
static SIMPLE_DEV_PM_OPS(tg3_pm_ops, tg3_suspend, tg3_resume);
^
include/linux/pm.h:316:59: note: in definition of macro 'PM_SLEEP_PTR'
#define PM_SLEEP_PTR(ptr) (IS_ENABLED(CONFIG_PM_SLEEP) ? (ptr) : NULL)
^~~
include/linux/pm.h:354:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net//ethernet/broadcom/tg3.c:18150:8: note: in expansion of macro 'SIMPLE_DEV_PM_OPS'
static SIMPLE_DEV_PM_OPS(tg3_pm_ops, tg3_suspend, tg3_resume);
^~~~~~~~~~~~~~~~~
>> drivers/net//ethernet/broadcom/tg3.c:18150:51: error: 'tg3_resume' undeclared here (not in a function); did you mean 'phy_resume'?
static SIMPLE_DEV_PM_OPS(tg3_pm_ops, tg3_suspend, tg3_resume);
^
include/linux/pm.h:316:59: note: in definition of macro 'PM_SLEEP_PTR'
#define PM_SLEEP_PTR(ptr) (IS_ENABLED(CONFIG_PM_SLEEP) ? (ptr) : NULL)
^~~
include/linux/pm.h:354:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net//ethernet/broadcom/tg3.c:18150:8: note: in expansion of macro 'SIMPLE_DEV_PM_OPS'
static SIMPLE_DEV_PM_OPS(tg3_pm_ops, tg3_suspend, tg3_resume);
^~~~~~~~~~~~~~~~~
--
In file included from include/linux/device.h:23:0,
from drivers/video//backlight/backlight.c:12:
>> drivers/video//backlight/backlight.c:280:54: error: 'backlight_suspend' undeclared here (not in a function); did you mean 'backlight_types'?
static SIMPLE_DEV_PM_OPS(backlight_class_dev_pm_ops, backlight_suspend,
^
include/linux/pm.h:316:59: note: in definition of macro 'PM_SLEEP_PTR'
#define PM_SLEEP_PTR(ptr) (IS_ENABLED(CONFIG_PM_SLEEP) ? (ptr) : NULL)
^~~
include/linux/pm.h:354:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
^~~~~~~~~~~~~~~~~~~~~~~
drivers/video//backlight/backlight.c:280:8: note: in expansion of macro 'SIMPLE_DEV_PM_OPS'
static SIMPLE_DEV_PM_OPS(backlight_class_dev_pm_ops, backlight_suspend,
^~~~~~~~~~~~~~~~~
>> drivers/video//backlight/backlight.c:281:5: error: 'backlight_resume' undeclared here (not in a function); did you mean 'backlight_device'?
backlight_resume);
^
include/linux/pm.h:316:59: note: in definition of macro 'PM_SLEEP_PTR'
#define PM_SLEEP_PTR(ptr) (IS_ENABLED(CONFIG_PM_SLEEP) ? (ptr) : NULL)
^~~
include/linux/pm.h:354:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
^~~~~~~~~~~~~~~~~~~~~~~
drivers/video//backlight/backlight.c:280:8: note: in expansion of macro 'SIMPLE_DEV_PM_OPS'
static SIMPLE_DEV_PM_OPS(backlight_class_dev_pm_ops, backlight_suspend,
^~~~~~~~~~~~~~~~~
vim +18150 drivers/net//ethernet/broadcom/tg3.c
^1da177e drivers/net/tg3.c Linus Torvalds 2005-04-16 18149
c866b7ea drivers/net/tg3.c Rafael J. Wysocki 2010-12-25 @18150 static SIMPLE_DEV_PM_OPS(tg3_pm_ops, tg3_suspend, tg3_resume);
c866b7ea drivers/net/tg3.c Rafael J. Wysocki 2010-12-25 18151
:::::: The code at line 18150 was first introduced by commit
:::::: c866b7eac073198cef03ea6bac2dc978635a9f5c tg3: Do not use legacy PCI power management
:::::: TO: Rafael J. Wysocki <[email protected]>
:::::: CC: David S. Miller <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On 14.05.2018 10:21, Andrzej Hajda wrote:
> With current implementation of PM_OPS initializers users should annotate
> all PM callbacks with __maybe_unused attribute to prevent compiler from
> complaining in case respective option is not enabled. Using ternary
> operator with IS_ENABLED(symbol) as a condition allows to avoid marking
> these functions with __maybe_unused.
> Solution was tested successfully with multiple versions of gcc since
> 4.9.4 up to 7.2.1. No functional changes has been observed and callbacks
> were compiled out if not used, as before.
The kernel does not compile with the patch applied if the driver defines
PM callback inside "#ifdef CONFIG_PM(_SLEEP)" - initializers assume that
callbacks are defined, even if they are not used.
So if the idea is OK I should figure it out how to make proper
transition, any ideas welcome :)
Regards
Andrzej
>
> Signed-off-by: Andrzej Hajda <[email protected]>
> ---
> include/linux/pm.h | 61 ++++++++++++++++++----------------------------
> 1 file changed, 24 insertions(+), 37 deletions(-)
>
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index e723b78d8357..59f333922c15 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -313,50 +313,37 @@ struct dev_pm_ops {
> int (*runtime_idle)(struct device *dev);
> };
>
> -#ifdef CONFIG_PM_SLEEP
> +#define PM_SLEEP_PTR(ptr) (IS_ENABLED(CONFIG_PM_SLEEP) ? (ptr) : NULL)
> +#define PM_PTR(ptr) (IS_ENABLED(CONFIG_PM) ? (ptr) : NULL)
> +
> #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> - .suspend = suspend_fn, \
> - .resume = resume_fn, \
> - .freeze = suspend_fn, \
> - .thaw = resume_fn, \
> - .poweroff = suspend_fn, \
> - .restore = resume_fn,
> -#else
> -#define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
> -#endif
> + .suspend = PM_SLEEP_PTR(suspend_fn), \
> + .resume = PM_SLEEP_PTR(resume_fn), \
> + .freeze = PM_SLEEP_PTR(suspend_fn), \
> + .thaw = PM_SLEEP_PTR(resume_fn), \
> + .poweroff = PM_SLEEP_PTR(suspend_fn), \
> + .restore = PM_SLEEP_PTR(resume_fn),
>
> -#ifdef CONFIG_PM_SLEEP
> #define SET_LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> - .suspend_late = suspend_fn, \
> - .resume_early = resume_fn, \
> - .freeze_late = suspend_fn, \
> - .thaw_early = resume_fn, \
> - .poweroff_late = suspend_fn, \
> - .restore_early = resume_fn,
> -#else
> -#define SET_LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
> -#endif
> + .suspend_late = PM_SLEEP_PTR(suspend_fn), \
> + .resume_early = PM_SLEEP_PTR(resume_fn), \
> + .freeze_late = PM_SLEEP_PTR(suspend_fn), \
> + .thaw_early = PM_SLEEP_PTR(resume_fn), \
> + .poweroff_late = PM_SLEEP_PTR(suspend_fn), \
> + .restore_early = PM_SLEEP_PTR(resume_fn),
>
> -#ifdef CONFIG_PM_SLEEP
> #define SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> - .suspend_noirq = suspend_fn, \
> - .resume_noirq = resume_fn, \
> - .freeze_noirq = suspend_fn, \
> - .thaw_noirq = resume_fn, \
> - .poweroff_noirq = suspend_fn, \
> - .restore_noirq = resume_fn,
> -#else
> -#define SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
> -#endif
> + .suspend_noirq = PM_SLEEP_PTR(suspend_fn), \
> + .resume_noirq = PM_SLEEP_PTR(resume_fn), \
> + .freeze_noirq = PM_SLEEP_PTR(suspend_fn), \
> + .thaw_noirq = PM_SLEEP_PTR(resume_fn), \
> + .poweroff_noirq = PM_SLEEP_PTR(suspend_fn), \
> + .restore_noirq = PM_SLEEP_PTR(resume_fn),
>
> -#ifdef CONFIG_PM
> #define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
> - .runtime_suspend = suspend_fn, \
> - .runtime_resume = resume_fn, \
> - .runtime_idle = idle_fn,
> -#else
> -#define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn)
> -#endif
> + .runtime_suspend = PM_PTR(suspend_fn), \
> + .runtime_resume = PM_PTR(resume_fn), \
> + .runtime_idle = PM_PTR(idle_fn),
>
> /*
> * Use this if you want to use the same suspend and resume callbacks for suspend