| Message ID | 20210408205858.51751-1-masahiroy@kernel.org |
|---|---|
| Headers | show |
| Series | linux/kconfig.h: move IF_ENABLED() out of <linux/kconfig.h> | expand |
On Thu, Apr 8, 2021 at 11:00 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > Drivers typically surround suspend and resume callbacks with #ifdef > CONFIG_PM(_SLEEP) or mark them as __maybe_unused in order to avoid > -Wunused-const-variable warnings. > > With this commit, drivers will be able to remove #ifdef CONFIG_PM(_SLEEP) > and __maybe_unsed because unused functions are dropped by the compiler > instead of the preprocessor. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> I tried this before and could not get it to work right. > > -#ifdef CONFIG_PM_SLEEP > +#define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), _ptr) > +#define pm_sleep_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), _ptr) > + > #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), The problem that I think you inevitably hit is that you run into a missing declaration for any driver that still uses an #ifdef around a static function. The only way I can see us doing this is to create a new set of macros that behave like the version you propose here but leave the old macros in place until the last such #ifdef has been removed. Arnd
On Fri, Apr 9, 2021 at 6:30 AM Arnd Bergmann <arnd@arndb.de> wrote: > > On Thu, Apr 8, 2021 at 11:00 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > Drivers typically surround suspend and resume callbacks with #ifdef > > CONFIG_PM(_SLEEP) or mark them as __maybe_unused in order to avoid > > -Wunused-const-variable warnings. > > > > With this commit, drivers will be able to remove #ifdef CONFIG_PM(_SLEEP) > > and __maybe_unsed because unused functions are dropped by the compiler > > instead of the preprocessor. > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > I tried this before and could not get it to work right. > > > > > -#ifdef CONFIG_PM_SLEEP > > +#define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), _ptr) > > +#define pm_sleep_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), _ptr) > > + > > #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), > > The problem that I think you inevitably hit is that you run into a missing > declaration for any driver that still uses an #ifdef around a static > function. > > The only way I can see us doing this is to create a new set of > macros that behave like the version you propose here but leave > the old macros in place until the last such #ifdef has been removed. > > Arnd Agh, you are right. We cannot change all the callsites atomically due to the huge amount of users.
I insist on <linux/kconfig.h> having only minimal set of macros that are needed to evaluate CONFIG options. Everytime somebody added an alien to <linux/kconfig.h>, I needed to kick it out. I did not notice 1b399bb04837183cecdc1b32ef1cfc7fcfa75d32 because I was not addressed by [1]. [1]: https://lore.kernel.org/lkml/?q=kconfig.h%3A+Add+IF_ENABLED%28%29+macro I like Paul's idea, but if I had noticed the patch in time, I would have tried my best to persuade to implement it outside of <linux/kconfig.h> (Paul's initial patch was adding it to a new header instead of <linux/kconfig.h>) Before it is widely used, I want to fix it. In 2/2, I converted pm.h to allow driver cleanups. Masahiro Yamada (2): linux/kconfig.h: replace IF_ENABLED() with PTR_IF() in <linux/kernel.h> pm: allow drivers to drop #ifdef and __maybe_unused from pm callbacks drivers/pinctrl/pinctrl-ingenic.c | 20 ++++----- include/linux/kconfig.h | 6 --- include/linux/kernel.h | 2 + include/linux/pm.h | 67 +++++++++++-------------------- 4 files changed, 36 insertions(+), 59 deletions(-)