* [PATCH 0/2] linux/kconfig.h: move IF_ENABLED() out of <linux/kconfig.h>
@ 2021-04-08 20:58 Masahiro Yamada
2021-04-08 20:58 ` [PATCH 1/2] linux/kconfig.h: replace IF_ENABLED() with PTR_IF() in <linux/kernel.h> Masahiro Yamada
2021-04-08 20:58 ` [PATCH 2/2] pm: allow drivers to drop #ifdef and __maybe_unused from pm callbacks Masahiro Yamada
0 siblings, 2 replies; 14+ messages in thread
From: Masahiro Yamada @ 2021-04-08 20:58 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-gpio, Arnd Bergmann, Paul Cercueil, Masahiro Yamada,
Len Brown, Linus Walleij, Pavel Machek, Rafael J. Wysocki,
linux-kernel, linux-mips, linux-pm
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(-)
--
2.27.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] linux/kconfig.h: replace IF_ENABLED() with PTR_IF() in <linux/kernel.h>
2021-04-08 20:58 [PATCH 0/2] linux/kconfig.h: move IF_ENABLED() out of <linux/kconfig.h> Masahiro Yamada
@ 2021-04-08 20:58 ` Masahiro Yamada
2021-04-09 8:15 ` Paul Cercueil
2021-04-09 9:24 ` Andy Shevchenko
2021-04-08 20:58 ` [PATCH 2/2] pm: allow drivers to drop #ifdef and __maybe_unused from pm callbacks Masahiro Yamada
1 sibling, 2 replies; 14+ messages in thread
From: Masahiro Yamada @ 2021-04-08 20:58 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-gpio, Arnd Bergmann, Paul Cercueil, Masahiro Yamada,
Linus Walleij, linux-kernel, linux-mips
<linux/kconfig.h> is included from all the kernel-space source files,
including C, assembly, linker scripts. It is intended to contain minimal
set of macros to evaluate CONFIG options.
IF_ENABLED() is an intruder here because (x ? y : z) is C code, which
should not be included from assembly files or linker scripts.
Also, <linux/kconfig.h> is no longer self-contained because NULL is
defined in <linux/stddef.h>.
Move IF_ENABLED() out to <linux/kernel.h> as PTR_IF().
PTR_IF(IS_ENABLED(CONFIG_FOO), ...) is slightly longer than
IF_ENABLED(CONFIG_FOO, ...), but it is not a big deal because
sub-systems often define dedicated macros such as of_match_ptr(),
pm_ptr() etc. for common use-cases.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
drivers/pinctrl/pinctrl-ingenic.c | 20 ++++++++++----------
include/linux/kconfig.h | 6 ------
include/linux/kernel.h | 2 ++
3 files changed, 12 insertions(+), 16 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-ingenic.c b/drivers/pinctrl/pinctrl-ingenic.c
index f2746125b077..b21e2ae4528d 100644
--- a/drivers/pinctrl/pinctrl-ingenic.c
+++ b/drivers/pinctrl/pinctrl-ingenic.c
@@ -2496,43 +2496,43 @@ static int __init ingenic_pinctrl_probe(struct platform_device *pdev)
static const struct of_device_id ingenic_pinctrl_of_match[] = {
{
.compatible = "ingenic,jz4740-pinctrl",
- .data = IF_ENABLED(CONFIG_MACH_JZ4740, &jz4740_chip_info)
+ .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4740), &jz4740_chip_info)
},
{
.compatible = "ingenic,jz4725b-pinctrl",
- .data = IF_ENABLED(CONFIG_MACH_JZ4725B, &jz4725b_chip_info)
+ .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4725B), &jz4725b_chip_info)
},
{
.compatible = "ingenic,jz4760-pinctrl",
- .data = IF_ENABLED(CONFIG_MACH_JZ4760, &jz4760_chip_info)
+ .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4760), &jz4760_chip_info)
},
{
.compatible = "ingenic,jz4760b-pinctrl",
- .data = IF_ENABLED(CONFIG_MACH_JZ4760, &jz4760_chip_info)
+ .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4760), &jz4760_chip_info)
},
{
.compatible = "ingenic,jz4770-pinctrl",
- .data = IF_ENABLED(CONFIG_MACH_JZ4770, &jz4770_chip_info)
+ .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4770), &jz4770_chip_info)
},
{
.compatible = "ingenic,jz4780-pinctrl",
- .data = IF_ENABLED(CONFIG_MACH_JZ4780, &jz4780_chip_info)
+ .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4780), &jz4780_chip_info)
},
{
.compatible = "ingenic,x1000-pinctrl",
- .data = IF_ENABLED(CONFIG_MACH_X1000, &x1000_chip_info)
+ .data = PTR_IF(IS_ENABLED(CONFIG_MACH_X1000), &x1000_chip_info)
},
{
.compatible = "ingenic,x1000e-pinctrl",
- .data = IF_ENABLED(CONFIG_MACH_X1000, &x1000_chip_info)
+ .data = PTR_IF(IS_ENABLED(CONFIG_MACH_X1000), &x1000_chip_info)
},
{
.compatible = "ingenic,x1500-pinctrl",
- .data = IF_ENABLED(CONFIG_MACH_X1500, &x1500_chip_info)
+ .data = PTR_IF(IS_ENABLED(CONFIG_MACH_X1500), &x1500_chip_info)
},
{
.compatible = "ingenic,x1830-pinctrl",
- .data = IF_ENABLED(CONFIG_MACH_X1830, &x1830_chip_info)
+ .data = PTR_IF(IS_ENABLED(CONFIG_MACH_X1830), &x1830_chip_info)
},
{ /* sentinel */ },
};
diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index 24a59cb06963..cc8fa109cfa3 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -70,10 +70,4 @@
*/
#define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
-/*
- * IF_ENABLED(CONFIG_FOO, ptr) evaluates to (ptr) if CONFIG_FOO is set to 'y'
- * or 'm', NULL otherwise.
- */
-#define IF_ENABLED(option, ptr) (IS_ENABLED(option) ? (ptr) : NULL)
-
#endif /* __LINUX_KCONFIG_H */
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 5b7ed6dc99ac..8685ca4cf287 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -38,6 +38,8 @@
#define PTR_ALIGN_DOWN(p, a) ((typeof(p))ALIGN_DOWN((unsigned long)(p), (a)))
#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
+#define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
+
/* generic data direction definitions */
#define READ 0
#define WRITE 1
--
2.27.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] pm: allow drivers to drop #ifdef and __maybe_unused from pm callbacks
2021-04-08 20:58 [PATCH 0/2] linux/kconfig.h: move IF_ENABLED() out of <linux/kconfig.h> Masahiro Yamada
2021-04-08 20:58 ` [PATCH 1/2] linux/kconfig.h: replace IF_ENABLED() with PTR_IF() in <linux/kernel.h> Masahiro Yamada
@ 2021-04-08 20:58 ` Masahiro Yamada
2021-04-08 21:29 ` Arnd Bergmann
` (2 more replies)
1 sibling, 3 replies; 14+ messages in thread
From: Masahiro Yamada @ 2021-04-08 20:58 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-gpio, Arnd Bergmann, Paul Cercueil, Masahiro Yamada,
Len Brown, Pavel Machek, Rafael J. Wysocki, linux-kernel,
linux-pm
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>
---
include/linux/pm.h | 67 +++++++++++++++++-----------------------------
1 file changed, 24 insertions(+), 43 deletions(-)
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 482313a8ccfc..ca764566692a 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -301,50 +301,37 @@ struct dev_pm_ops {
int (*runtime_idle)(struct device *dev);
};
-#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),
-#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
@@ -374,12 +361,6 @@ const struct dev_pm_ops __maybe_unused name = { \
SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
}
-#ifdef CONFIG_PM
-#define pm_ptr(_ptr) (_ptr)
-#else
-#define pm_ptr(_ptr) NULL
-#endif
-
/*
* PM_EVENT_ messages
*
--
2.27.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] pm: allow drivers to drop #ifdef and __maybe_unused from pm callbacks
2021-04-08 20:58 ` [PATCH 2/2] pm: allow drivers to drop #ifdef and __maybe_unused from pm callbacks Masahiro Yamada
@ 2021-04-08 21:29 ` Arnd Bergmann
2021-04-09 3:17 ` Masahiro Yamada
2021-04-08 23:49 ` kernel test robot
2021-04-08 23:55 ` kernel test robot
2 siblings, 1 reply; 14+ messages in thread
From: Arnd Bergmann @ 2021-04-08 21:29 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Linux Kbuild mailing list, open list:GPIO SUBSYSTEM,
Paul Cercueil, Len Brown, Pavel Machek, Rafael J. Wysocki,
Linux Kernel Mailing List, Linux PM list
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] pm: allow drivers to drop #ifdef and __maybe_unused from pm callbacks
2021-04-08 20:58 ` [PATCH 2/2] pm: allow drivers to drop #ifdef and __maybe_unused from pm callbacks Masahiro Yamada
@ 2021-04-08 23:49 ` kernel test robot
2021-04-08 23:49 ` kernel test robot
2021-04-08 23:55 ` kernel test robot
2 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-04-08 23:49 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild
Cc: kbuild-all, linux-gpio, Arnd Bergmann, Paul Cercueil,
Masahiro Yamada, Len Brown, Pavel Machek, Rafael J. Wysocki,
linux-kernel, linux-pm
[-- Attachment #1: Type: text/plain, Size: 10188 bytes --]
Hi Masahiro,
I love your patch! Yet something to improve:
[auto build test ERROR on pinctrl/devel]
[also build test ERROR on pm/linux-next soc/for-next linus/master v5.12-rc6]
[cannot apply to next-20210408]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Masahiro-Yamada/linux-kconfig-h-move-IF_ENABLED-out-of-linux-kconfig-h/20210409-050128
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: nds32-randconfig-r004-20210408 (attached as .config)
compiler: nds32le-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/01dfb9e1a54c14b3f491c3a5e93f1e8756042567
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Masahiro-Yamada/linux-kconfig-h-move-IF_ENABLED-out-of-linux-kconfig-h/20210409-050128
git checkout 01dfb9e1a54c14b3f491c3a5e93f1e8756042567
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nds32
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from drivers/input/rmi4/rmi_spi.c:7:
drivers/input/rmi4/rmi_spi.c:507:26: error: 'rmi_spi_suspend' undeclared here (not in a function); did you mean 'rmi_driver_suspend'?
507 | SET_SYSTEM_SLEEP_PM_OPS(rmi_spi_suspend, rmi_spi_resume)
| ^~~~~~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:308:14: note: in expansion of macro 'pm_sleep_ptr'
308 | .suspend = pm_sleep_ptr(suspend_fn), \
| ^~~~~~~~~~~~
drivers/input/rmi4/rmi_spi.c:507:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
507 | SET_SYSTEM_SLEEP_PM_OPS(rmi_spi_suspend, rmi_spi_resume)
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/input/rmi4/rmi_spi.c:507:43: error: 'rmi_spi_resume' undeclared here (not in a function); did you mean 'rmi_spi_pm'?
507 | SET_SYSTEM_SLEEP_PM_OPS(rmi_spi_suspend, rmi_spi_resume)
| ^~~~~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:309:14: note: in expansion of macro 'pm_sleep_ptr'
309 | .resume = pm_sleep_ptr(resume_fn), \
| ^~~~~~~~~~~~
drivers/input/rmi4/rmi_spi.c:507:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
507 | SET_SYSTEM_SLEEP_PM_OPS(rmi_spi_suspend, rmi_spi_resume)
| ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/input/rmi4/rmi_spi.c:508:21: error: 'rmi_spi_runtime_suspend' undeclared here (not in a function)
508 | SET_RUNTIME_PM_OPS(rmi_spi_runtime_suspend, rmi_spi_runtime_resume,
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:332:21: note: in expansion of macro 'pm_ptr'
332 | .runtime_suspend = pm_ptr(suspend_fn), \
| ^~~~~~
drivers/input/rmi4/rmi_spi.c:508:2: note: in expansion of macro 'SET_RUNTIME_PM_OPS'
508 | SET_RUNTIME_PM_OPS(rmi_spi_runtime_suspend, rmi_spi_runtime_resume,
| ^~~~~~~~~~~~~~~~~~
>> drivers/input/rmi4/rmi_spi.c:508:46: error: 'rmi_spi_runtime_resume' undeclared here (not in a function)
508 | SET_RUNTIME_PM_OPS(rmi_spi_runtime_suspend, rmi_spi_runtime_resume,
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:333:21: note: in expansion of macro 'pm_ptr'
333 | .runtime_resume = pm_ptr(resume_fn), \
| ^~~~~~
drivers/input/rmi4/rmi_spi.c:508:2: note: in expansion of macro 'SET_RUNTIME_PM_OPS'
508 | SET_RUNTIME_PM_OPS(rmi_spi_runtime_suspend, rmi_spi_runtime_resume,
| ^~~~~~~~~~~~~~~~~~
--
In file included from include/linux/list.h:9,
from include/linux/rculist.h:10,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from include/linux/backlight.h:12,
from drivers/video/backlight/ams369fg06.c:11:
>> drivers/video/backlight/ams369fg06.c:541:45: error: 'ams369fg06_suspend' undeclared here (not in a function); did you mean 'ams369fg06_probe'?
541 | static SIMPLE_DEV_PM_OPS(ams369fg06_pm_ops, ams369fg06_suspend,
| ^~~~~~~~~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:308:14: note: in expansion of macro 'pm_sleep_ptr'
308 | .suspend = pm_sleep_ptr(suspend_fn), \
| ^~~~~~~~~~~~
include/linux/pm.h:342:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
342 | SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/video/backlight/ams369fg06.c:541:8: note: in expansion of macro 'SIMPLE_DEV_PM_OPS'
541 | static SIMPLE_DEV_PM_OPS(ams369fg06_pm_ops, ams369fg06_suspend,
| ^~~~~~~~~~~~~~~~~
>> drivers/video/backlight/ams369fg06.c:542:4: error: 'ams369fg06_resume' undeclared here (not in a function); did you mean 'ams369fg06_remove'?
542 | ams369fg06_resume);
| ^~~~~~~~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:309:14: note: in expansion of macro 'pm_sleep_ptr'
309 | .resume = pm_sleep_ptr(resume_fn), \
| ^~~~~~~~~~~~
include/linux/pm.h:342:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
342 | SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/video/backlight/ams369fg06.c:541:8: note: in expansion of macro 'SIMPLE_DEV_PM_OPS'
541 | static SIMPLE_DEV_PM_OPS(ams369fg06_pm_ops, ams369fg06_suspend,
| ^~~~~~~~~~~~~~~~~
--
In file included from include/linux/delay.h:22,
from drivers/video/backlight/vgg2432a4.c:11:
>> drivers/video/backlight/vgg2432a4.c:246:44: error: 'vgg2432a4_suspend' undeclared here (not in a function); did you mean 'vgg2432a4_client'?
246 | static SIMPLE_DEV_PM_OPS(vgg2432a4_pm_ops, vgg2432a4_suspend, vgg2432a4_resume);
| ^~~~~~~~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:308:14: note: in expansion of macro 'pm_sleep_ptr'
308 | .suspend = pm_sleep_ptr(suspend_fn), \
| ^~~~~~~~~~~~
include/linux/pm.h:342:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
342 | SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/video/backlight/vgg2432a4.c:246:8: note: in expansion of macro 'SIMPLE_DEV_PM_OPS'
246 | static SIMPLE_DEV_PM_OPS(vgg2432a4_pm_ops, vgg2432a4_suspend, vgg2432a4_resume);
| ^~~~~~~~~~~~~~~~~
>> drivers/video/backlight/vgg2432a4.c:246:63: error: 'vgg2432a4_resume' undeclared here (not in a function); did you mean 'vgg2432a4_remove'?
246 | static SIMPLE_DEV_PM_OPS(vgg2432a4_pm_ops, vgg2432a4_suspend, vgg2432a4_resume);
| ^~~~~~~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:309:14: note: in expansion of macro 'pm_sleep_ptr'
309 | .resume = pm_sleep_ptr(resume_fn), \
| ^~~~~~~~~~~~
include/linux/pm.h:342:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
342 | SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/video/backlight/vgg2432a4.c:246:8: note: in expansion of macro 'SIMPLE_DEV_PM_OPS'
246 | static SIMPLE_DEV_PM_OPS(vgg2432a4_pm_ops, vgg2432a4_suspend, vgg2432a4_resume);
| ^~~~~~~~~~~~~~~~~
vim +/rmi_spi_runtime_suspend +508 drivers/input/rmi4/rmi_spi.c
8d99758dee31ff Andrew Duggan 2016-03-10 505
8d99758dee31ff Andrew Duggan 2016-03-10 506 static const struct dev_pm_ops rmi_spi_pm = {
8d99758dee31ff Andrew Duggan 2016-03-10 @507 SET_SYSTEM_SLEEP_PM_OPS(rmi_spi_suspend, rmi_spi_resume)
8d99758dee31ff Andrew Duggan 2016-03-10 @508 SET_RUNTIME_PM_OPS(rmi_spi_runtime_suspend, rmi_spi_runtime_resume,
8d99758dee31ff Andrew Duggan 2016-03-10 509 NULL)
8d99758dee31ff Andrew Duggan 2016-03-10 510 };
8d99758dee31ff Andrew Duggan 2016-03-10 511
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31298 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] pm: allow drivers to drop #ifdef and __maybe_unused from pm callbacks
@ 2021-04-08 23:49 ` kernel test robot
0 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-04-08 23:49 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 10357 bytes --]
Hi Masahiro,
I love your patch! Yet something to improve:
[auto build test ERROR on pinctrl/devel]
[also build test ERROR on pm/linux-next soc/for-next linus/master v5.12-rc6]
[cannot apply to next-20210408]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Masahiro-Yamada/linux-kconfig-h-move-IF_ENABLED-out-of-linux-kconfig-h/20210409-050128
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: nds32-randconfig-r004-20210408 (attached as .config)
compiler: nds32le-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/01dfb9e1a54c14b3f491c3a5e93f1e8756042567
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Masahiro-Yamada/linux-kconfig-h-move-IF_ENABLED-out-of-linux-kconfig-h/20210409-050128
git checkout 01dfb9e1a54c14b3f491c3a5e93f1e8756042567
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nds32
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from drivers/input/rmi4/rmi_spi.c:7:
drivers/input/rmi4/rmi_spi.c:507:26: error: 'rmi_spi_suspend' undeclared here (not in a function); did you mean 'rmi_driver_suspend'?
507 | SET_SYSTEM_SLEEP_PM_OPS(rmi_spi_suspend, rmi_spi_resume)
| ^~~~~~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:308:14: note: in expansion of macro 'pm_sleep_ptr'
308 | .suspend = pm_sleep_ptr(suspend_fn), \
| ^~~~~~~~~~~~
drivers/input/rmi4/rmi_spi.c:507:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
507 | SET_SYSTEM_SLEEP_PM_OPS(rmi_spi_suspend, rmi_spi_resume)
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/input/rmi4/rmi_spi.c:507:43: error: 'rmi_spi_resume' undeclared here (not in a function); did you mean 'rmi_spi_pm'?
507 | SET_SYSTEM_SLEEP_PM_OPS(rmi_spi_suspend, rmi_spi_resume)
| ^~~~~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:309:14: note: in expansion of macro 'pm_sleep_ptr'
309 | .resume = pm_sleep_ptr(resume_fn), \
| ^~~~~~~~~~~~
drivers/input/rmi4/rmi_spi.c:507:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
507 | SET_SYSTEM_SLEEP_PM_OPS(rmi_spi_suspend, rmi_spi_resume)
| ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/input/rmi4/rmi_spi.c:508:21: error: 'rmi_spi_runtime_suspend' undeclared here (not in a function)
508 | SET_RUNTIME_PM_OPS(rmi_spi_runtime_suspend, rmi_spi_runtime_resume,
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:332:21: note: in expansion of macro 'pm_ptr'
332 | .runtime_suspend = pm_ptr(suspend_fn), \
| ^~~~~~
drivers/input/rmi4/rmi_spi.c:508:2: note: in expansion of macro 'SET_RUNTIME_PM_OPS'
508 | SET_RUNTIME_PM_OPS(rmi_spi_runtime_suspend, rmi_spi_runtime_resume,
| ^~~~~~~~~~~~~~~~~~
>> drivers/input/rmi4/rmi_spi.c:508:46: error: 'rmi_spi_runtime_resume' undeclared here (not in a function)
508 | SET_RUNTIME_PM_OPS(rmi_spi_runtime_suspend, rmi_spi_runtime_resume,
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:333:21: note: in expansion of macro 'pm_ptr'
333 | .runtime_resume = pm_ptr(resume_fn), \
| ^~~~~~
drivers/input/rmi4/rmi_spi.c:508:2: note: in expansion of macro 'SET_RUNTIME_PM_OPS'
508 | SET_RUNTIME_PM_OPS(rmi_spi_runtime_suspend, rmi_spi_runtime_resume,
| ^~~~~~~~~~~~~~~~~~
--
In file included from include/linux/list.h:9,
from include/linux/rculist.h:10,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from include/linux/backlight.h:12,
from drivers/video/backlight/ams369fg06.c:11:
>> drivers/video/backlight/ams369fg06.c:541:45: error: 'ams369fg06_suspend' undeclared here (not in a function); did you mean 'ams369fg06_probe'?
541 | static SIMPLE_DEV_PM_OPS(ams369fg06_pm_ops, ams369fg06_suspend,
| ^~~~~~~~~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:308:14: note: in expansion of macro 'pm_sleep_ptr'
308 | .suspend = pm_sleep_ptr(suspend_fn), \
| ^~~~~~~~~~~~
include/linux/pm.h:342:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
342 | SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/video/backlight/ams369fg06.c:541:8: note: in expansion of macro 'SIMPLE_DEV_PM_OPS'
541 | static SIMPLE_DEV_PM_OPS(ams369fg06_pm_ops, ams369fg06_suspend,
| ^~~~~~~~~~~~~~~~~
>> drivers/video/backlight/ams369fg06.c:542:4: error: 'ams369fg06_resume' undeclared here (not in a function); did you mean 'ams369fg06_remove'?
542 | ams369fg06_resume);
| ^~~~~~~~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:309:14: note: in expansion of macro 'pm_sleep_ptr'
309 | .resume = pm_sleep_ptr(resume_fn), \
| ^~~~~~~~~~~~
include/linux/pm.h:342:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
342 | SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/video/backlight/ams369fg06.c:541:8: note: in expansion of macro 'SIMPLE_DEV_PM_OPS'
541 | static SIMPLE_DEV_PM_OPS(ams369fg06_pm_ops, ams369fg06_suspend,
| ^~~~~~~~~~~~~~~~~
--
In file included from include/linux/delay.h:22,
from drivers/video/backlight/vgg2432a4.c:11:
>> drivers/video/backlight/vgg2432a4.c:246:44: error: 'vgg2432a4_suspend' undeclared here (not in a function); did you mean 'vgg2432a4_client'?
246 | static SIMPLE_DEV_PM_OPS(vgg2432a4_pm_ops, vgg2432a4_suspend, vgg2432a4_resume);
| ^~~~~~~~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:308:14: note: in expansion of macro 'pm_sleep_ptr'
308 | .suspend = pm_sleep_ptr(suspend_fn), \
| ^~~~~~~~~~~~
include/linux/pm.h:342:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
342 | SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/video/backlight/vgg2432a4.c:246:8: note: in expansion of macro 'SIMPLE_DEV_PM_OPS'
246 | static SIMPLE_DEV_PM_OPS(vgg2432a4_pm_ops, vgg2432a4_suspend, vgg2432a4_resume);
| ^~~~~~~~~~~~~~~~~
>> drivers/video/backlight/vgg2432a4.c:246:63: error: 'vgg2432a4_resume' undeclared here (not in a function); did you mean 'vgg2432a4_remove'?
246 | static SIMPLE_DEV_PM_OPS(vgg2432a4_pm_ops, vgg2432a4_suspend, vgg2432a4_resume);
| ^~~~~~~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:309:14: note: in expansion of macro 'pm_sleep_ptr'
309 | .resume = pm_sleep_ptr(resume_fn), \
| ^~~~~~~~~~~~
include/linux/pm.h:342:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
342 | SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/video/backlight/vgg2432a4.c:246:8: note: in expansion of macro 'SIMPLE_DEV_PM_OPS'
246 | static SIMPLE_DEV_PM_OPS(vgg2432a4_pm_ops, vgg2432a4_suspend, vgg2432a4_resume);
| ^~~~~~~~~~~~~~~~~
vim +/rmi_spi_runtime_suspend +508 drivers/input/rmi4/rmi_spi.c
8d99758dee31ff Andrew Duggan 2016-03-10 505
8d99758dee31ff Andrew Duggan 2016-03-10 506 static const struct dev_pm_ops rmi_spi_pm = {
8d99758dee31ff Andrew Duggan 2016-03-10 @507 SET_SYSTEM_SLEEP_PM_OPS(rmi_spi_suspend, rmi_spi_resume)
8d99758dee31ff Andrew Duggan 2016-03-10 @508 SET_RUNTIME_PM_OPS(rmi_spi_runtime_suspend, rmi_spi_runtime_resume,
8d99758dee31ff Andrew Duggan 2016-03-10 509 NULL)
8d99758dee31ff Andrew Duggan 2016-03-10 510 };
8d99758dee31ff Andrew Duggan 2016-03-10 511
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 31298 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] pm: allow drivers to drop #ifdef and __maybe_unused from pm callbacks
2021-04-08 20:58 ` [PATCH 2/2] pm: allow drivers to drop #ifdef and __maybe_unused from pm callbacks Masahiro Yamada
@ 2021-04-08 23:55 ` kernel test robot
2021-04-08 23:49 ` kernel test robot
2021-04-08 23:55 ` kernel test robot
2 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-04-08 23:55 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild
Cc: kbuild-all, linux-gpio, Arnd Bergmann, Paul Cercueil,
Masahiro Yamada, Len Brown, Pavel Machek, Rafael J. Wysocki,
linux-kernel, linux-pm
[-- Attachment #1: Type: text/plain, Size: 4181 bytes --]
Hi Masahiro,
I love your patch! Yet something to improve:
[auto build test ERROR on pinctrl/devel]
[also build test ERROR on pm/linux-next soc/for-next linus/master v5.12-rc6]
[cannot apply to next-20210408]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Masahiro-Yamada/linux-kconfig-h-move-IF_ENABLED-out-of-linux-kconfig-h/20210409-050128
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: xtensa-randconfig-r023-20210408 (attached as .config)
compiler: xtensa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/01dfb9e1a54c14b3f491c3a5e93f1e8756042567
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Masahiro-Yamada/linux-kconfig-h-move-IF_ENABLED-out-of-linux-kconfig-h/20210409-050128
git checkout 01dfb9e1a54c14b3f491c3a5e93f1e8756042567
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from include/linux/delay.h:22,
from sound/pci/hda/hda_intel.c:23:
>> sound/pci/hda/hda_intel.c:1182:26: error: 'azx_suspend' undeclared here (not in a function)
1182 | SET_SYSTEM_SLEEP_PM_OPS(azx_suspend, azx_resume)
| ^~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:308:14: note: in expansion of macro 'pm_sleep_ptr'
308 | .suspend = pm_sleep_ptr(suspend_fn), \
| ^~~~~~~~~~~~
sound/pci/hda/hda_intel.c:1182:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
1182 | SET_SYSTEM_SLEEP_PM_OPS(azx_suspend, azx_resume)
| ^~~~~~~~~~~~~~~~~~~~~~~
>> sound/pci/hda/hda_intel.c:1182:39: error: 'azx_resume' undeclared here (not in a function)
1182 | SET_SYSTEM_SLEEP_PM_OPS(azx_suspend, azx_resume)
| ^~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:309:14: note: in expansion of macro 'pm_sleep_ptr'
309 | .resume = pm_sleep_ptr(resume_fn), \
| ^~~~~~~~~~~~
sound/pci/hda/hda_intel.c:1182:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
1182 | SET_SYSTEM_SLEEP_PM_OPS(azx_suspend, azx_resume)
| ^~~~~~~~~~~~~~~~~~~~~~~
vim +/azx_suspend +1182 sound/pci/hda/hda_intel.c
6eb827d23577a4 Takashi Iwai 2012-12-12 1180
b8dfc4624162c0 Mengdong Lin 2012-08-23 1181 static const struct dev_pm_ops azx_pm = {
b8dfc4624162c0 Mengdong Lin 2012-08-23 @1182 SET_SYSTEM_SLEEP_PM_OPS(azx_suspend, azx_resume)
3e6db33aaf1d42 Xiong Zhang 2015-12-18 1183 #ifdef CONFIG_PM_SLEEP
f5dac54d9d9382 Kai-Heng Feng 2020-10-27 1184 .prepare = azx_prepare,
f5dac54d9d9382 Kai-Heng Feng 2020-10-27 1185 .complete = azx_complete,
3e6db33aaf1d42 Xiong Zhang 2015-12-18 1186 .freeze_noirq = azx_freeze_noirq,
3e6db33aaf1d42 Xiong Zhang 2015-12-18 1187 .thaw_noirq = azx_thaw_noirq,
3e6db33aaf1d42 Xiong Zhang 2015-12-18 1188 #endif
6eb827d23577a4 Takashi Iwai 2012-12-12 1189 SET_RUNTIME_PM_OPS(azx_runtime_suspend, azx_runtime_resume, azx_runtime_idle)
b8dfc4624162c0 Mengdong Lin 2012-08-23 1190 };
b8dfc4624162c0 Mengdong Lin 2012-08-23 1191
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26505 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] pm: allow drivers to drop #ifdef and __maybe_unused from pm callbacks
@ 2021-04-08 23:55 ` kernel test robot
0 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-04-08 23:55 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4259 bytes --]
Hi Masahiro,
I love your patch! Yet something to improve:
[auto build test ERROR on pinctrl/devel]
[also build test ERROR on pm/linux-next soc/for-next linus/master v5.12-rc6]
[cannot apply to next-20210408]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Masahiro-Yamada/linux-kconfig-h-move-IF_ENABLED-out-of-linux-kconfig-h/20210409-050128
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: xtensa-randconfig-r023-20210408 (attached as .config)
compiler: xtensa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/01dfb9e1a54c14b3f491c3a5e93f1e8756042567
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Masahiro-Yamada/linux-kconfig-h-move-IF_ENABLED-out-of-linux-kconfig-h/20210409-050128
git checkout 01dfb9e1a54c14b3f491c3a5e93f1e8756042567
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from include/linux/delay.h:22,
from sound/pci/hda/hda_intel.c:23:
>> sound/pci/hda/hda_intel.c:1182:26: error: 'azx_suspend' undeclared here (not in a function)
1182 | SET_SYSTEM_SLEEP_PM_OPS(azx_suspend, azx_resume)
| ^~~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:308:14: note: in expansion of macro 'pm_sleep_ptr'
308 | .suspend = pm_sleep_ptr(suspend_fn), \
| ^~~~~~~~~~~~
sound/pci/hda/hda_intel.c:1182:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
1182 | SET_SYSTEM_SLEEP_PM_OPS(azx_suspend, azx_resume)
| ^~~~~~~~~~~~~~~~~~~~~~~
>> sound/pci/hda/hda_intel.c:1182:39: error: 'azx_resume' undeclared here (not in a function)
1182 | SET_SYSTEM_SLEEP_PM_OPS(azx_suspend, azx_resume)
| ^~~~~~~~~~
include/linux/kernel.h:41:38: note: in definition of macro 'PTR_IF'
41 | #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
| ^~~
include/linux/pm.h:309:14: note: in expansion of macro 'pm_sleep_ptr'
309 | .resume = pm_sleep_ptr(resume_fn), \
| ^~~~~~~~~~~~
sound/pci/hda/hda_intel.c:1182:2: note: in expansion of macro 'SET_SYSTEM_SLEEP_PM_OPS'
1182 | SET_SYSTEM_SLEEP_PM_OPS(azx_suspend, azx_resume)
| ^~~~~~~~~~~~~~~~~~~~~~~
vim +/azx_suspend +1182 sound/pci/hda/hda_intel.c
6eb827d23577a4 Takashi Iwai 2012-12-12 1180
b8dfc4624162c0 Mengdong Lin 2012-08-23 1181 static const struct dev_pm_ops azx_pm = {
b8dfc4624162c0 Mengdong Lin 2012-08-23 @1182 SET_SYSTEM_SLEEP_PM_OPS(azx_suspend, azx_resume)
3e6db33aaf1d42 Xiong Zhang 2015-12-18 1183 #ifdef CONFIG_PM_SLEEP
f5dac54d9d9382 Kai-Heng Feng 2020-10-27 1184 .prepare = azx_prepare,
f5dac54d9d9382 Kai-Heng Feng 2020-10-27 1185 .complete = azx_complete,
3e6db33aaf1d42 Xiong Zhang 2015-12-18 1186 .freeze_noirq = azx_freeze_noirq,
3e6db33aaf1d42 Xiong Zhang 2015-12-18 1187 .thaw_noirq = azx_thaw_noirq,
3e6db33aaf1d42 Xiong Zhang 2015-12-18 1188 #endif
6eb827d23577a4 Takashi Iwai 2012-12-12 1189 SET_RUNTIME_PM_OPS(azx_runtime_suspend, azx_runtime_resume, azx_runtime_idle)
b8dfc4624162c0 Mengdong Lin 2012-08-23 1190 };
b8dfc4624162c0 Mengdong Lin 2012-08-23 1191
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 26505 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] pm: allow drivers to drop #ifdef and __maybe_unused from pm callbacks
2021-04-08 21:29 ` Arnd Bergmann
@ 2021-04-09 3:17 ` Masahiro Yamada
0 siblings, 0 replies; 14+ messages in thread
From: Masahiro Yamada @ 2021-04-09 3:17 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linux Kbuild mailing list, open list:GPIO SUBSYSTEM,
Paul Cercueil, Len Brown, Pavel Machek, Rafael J. Wysocki,
Linux Kernel Mailing List, Linux PM list
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.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] linux/kconfig.h: replace IF_ENABLED() with PTR_IF() in <linux/kernel.h>
2021-04-08 20:58 ` [PATCH 1/2] linux/kconfig.h: replace IF_ENABLED() with PTR_IF() in <linux/kernel.h> Masahiro Yamada
@ 2021-04-09 8:15 ` Paul Cercueil
2021-04-09 8:30 ` Masahiro Yamada
2021-04-09 9:24 ` Andy Shevchenko
1 sibling, 1 reply; 14+ messages in thread
From: Paul Cercueil @ 2021-04-09 8:15 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-gpio, Arnd Bergmann, Linus Walleij,
linux-kernel, linux-mips
Hi Masahiro,
Le ven. 9 avril 2021 à 5:58, Masahiro Yamada <masahiroy@kernel.org> a
écrit :
> <linux/kconfig.h> is included from all the kernel-space source files,
> including C, assembly, linker scripts. It is intended to contain
> minimal
> set of macros to evaluate CONFIG options.
>
> IF_ENABLED() is an intruder here because (x ? y : z) is C code, which
> should not be included from assembly files or linker scripts.
>
> Also, <linux/kconfig.h> is no longer self-contained because NULL is
> defined in <linux/stddef.h>.
>
> Move IF_ENABLED() out to <linux/kernel.h> as PTR_IF().
>
> PTR_IF(IS_ENABLED(CONFIG_FOO), ...) is slightly longer than
> IF_ENABLED(CONFIG_FOO, ...), but it is not a big deal because
> sub-systems often define dedicated macros such as of_match_ptr(),
> pm_ptr() etc. for common use-cases.
What's the idea behind changing IF_ENABLED() to PTR_IF()? You didn't
explain that. What's wrong with IF_ENABLED()?
Cheers,
-Paul
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> drivers/pinctrl/pinctrl-ingenic.c | 20 ++++++++++----------
> include/linux/kconfig.h | 6 ------
> include/linux/kernel.h | 2 ++
> 3 files changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-ingenic.c
> b/drivers/pinctrl/pinctrl-ingenic.c
> index f2746125b077..b21e2ae4528d 100644
> --- a/drivers/pinctrl/pinctrl-ingenic.c
> +++ b/drivers/pinctrl/pinctrl-ingenic.c
> @@ -2496,43 +2496,43 @@ static int __init
> ingenic_pinctrl_probe(struct platform_device *pdev)
> static const struct of_device_id ingenic_pinctrl_of_match[] = {
> {
> .compatible = "ingenic,jz4740-pinctrl",
> - .data = IF_ENABLED(CONFIG_MACH_JZ4740, &jz4740_chip_info)
> + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4740), &jz4740_chip_info)
> },
> {
> .compatible = "ingenic,jz4725b-pinctrl",
> - .data = IF_ENABLED(CONFIG_MACH_JZ4725B, &jz4725b_chip_info)
> + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4725B), &jz4725b_chip_info)
> },
> {
> .compatible = "ingenic,jz4760-pinctrl",
> - .data = IF_ENABLED(CONFIG_MACH_JZ4760, &jz4760_chip_info)
> + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4760), &jz4760_chip_info)
> },
> {
> .compatible = "ingenic,jz4760b-pinctrl",
> - .data = IF_ENABLED(CONFIG_MACH_JZ4760, &jz4760_chip_info)
> + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4760), &jz4760_chip_info)
> },
> {
> .compatible = "ingenic,jz4770-pinctrl",
> - .data = IF_ENABLED(CONFIG_MACH_JZ4770, &jz4770_chip_info)
> + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4770), &jz4770_chip_info)
> },
> {
> .compatible = "ingenic,jz4780-pinctrl",
> - .data = IF_ENABLED(CONFIG_MACH_JZ4780, &jz4780_chip_info)
> + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4780), &jz4780_chip_info)
> },
> {
> .compatible = "ingenic,x1000-pinctrl",
> - .data = IF_ENABLED(CONFIG_MACH_X1000, &x1000_chip_info)
> + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_X1000), &x1000_chip_info)
> },
> {
> .compatible = "ingenic,x1000e-pinctrl",
> - .data = IF_ENABLED(CONFIG_MACH_X1000, &x1000_chip_info)
> + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_X1000), &x1000_chip_info)
> },
> {
> .compatible = "ingenic,x1500-pinctrl",
> - .data = IF_ENABLED(CONFIG_MACH_X1500, &x1500_chip_info)
> + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_X1500), &x1500_chip_info)
> },
> {
> .compatible = "ingenic,x1830-pinctrl",
> - .data = IF_ENABLED(CONFIG_MACH_X1830, &x1830_chip_info)
> + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_X1830), &x1830_chip_info)
> },
> { /* sentinel */ },
> };
> diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
> index 24a59cb06963..cc8fa109cfa3 100644
> --- a/include/linux/kconfig.h
> +++ b/include/linux/kconfig.h
> @@ -70,10 +70,4 @@
> */
> #define IS_ENABLED(option) __or(IS_BUILTIN(option),
> IS_MODULE(option))
>
> -/*
> - * IF_ENABLED(CONFIG_FOO, ptr) evaluates to (ptr) if CONFIG_FOO is
> set to 'y'
> - * or 'm', NULL otherwise.
> - */
> -#define IF_ENABLED(option, ptr) (IS_ENABLED(option) ? (ptr) : NULL)
> -
> #endif /* __LINUX_KCONFIG_H */
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 5b7ed6dc99ac..8685ca4cf287 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -38,6 +38,8 @@
> #define PTR_ALIGN_DOWN(p, a) ((typeof(p))ALIGN_DOWN((unsigned
> long)(p), (a)))
> #define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
>
> +#define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
> +
> /* generic data direction definitions */
> #define READ 0
> #define WRITE 1
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] linux/kconfig.h: replace IF_ENABLED() with PTR_IF() in <linux/kernel.h>
2021-04-09 8:15 ` Paul Cercueil
@ 2021-04-09 8:30 ` Masahiro Yamada
2021-04-09 9:38 ` Paul Cercueil
0 siblings, 1 reply; 14+ messages in thread
From: Masahiro Yamada @ 2021-04-09 8:30 UTC (permalink / raw)
To: Paul Cercueil
Cc: Linux Kbuild mailing list, open list:GPIO SUBSYSTEM,
Arnd Bergmann, Linus Walleij, Linux Kernel Mailing List,
linux-mips
On Fri, Apr 9, 2021 at 5:15 PM Paul Cercueil <paul@crapouillou.net> wrote:
>
> Hi Masahiro,
>
> Le ven. 9 avril 2021 à 5:58, Masahiro Yamada <masahiroy@kernel.org> a
> écrit :
> > <linux/kconfig.h> is included from all the kernel-space source files,
> > including C, assembly, linker scripts. It is intended to contain
> > minimal
> > set of macros to evaluate CONFIG options.
> >
> > IF_ENABLED() is an intruder here because (x ? y : z) is C code, which
> > should not be included from assembly files or linker scripts.
> >
> > Also, <linux/kconfig.h> is no longer self-contained because NULL is
> > defined in <linux/stddef.h>.
> >
> > Move IF_ENABLED() out to <linux/kernel.h> as PTR_IF().
> >
> > PTR_IF(IS_ENABLED(CONFIG_FOO), ...) is slightly longer than
> > IF_ENABLED(CONFIG_FOO, ...), but it is not a big deal because
> > sub-systems often define dedicated macros such as of_match_ptr(),
> > pm_ptr() etc. for common use-cases.
>
> What's the idea behind changing IF_ENABLED() to PTR_IF()? You didn't
> explain that. What's wrong with IF_ENABLED()?
PTR_IF() is a more generalized variant, which I believe is
a better fit in <linux/kernel.h>
The first parameter does not need to be a CONFIG option,
but any expression.
> Cheers,
> -Paul
>
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> > drivers/pinctrl/pinctrl-ingenic.c | 20 ++++++++++----------
> > include/linux/kconfig.h | 6 ------
> > include/linux/kernel.h | 2 ++
> > 3 files changed, 12 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/pinctrl/pinctrl-ingenic.c
> > b/drivers/pinctrl/pinctrl-ingenic.c
> > index f2746125b077..b21e2ae4528d 100644
> > --- a/drivers/pinctrl/pinctrl-ingenic.c
> > +++ b/drivers/pinctrl/pinctrl-ingenic.c
> > @@ -2496,43 +2496,43 @@ static int __init
> > ingenic_pinctrl_probe(struct platform_device *pdev)
> > static const struct of_device_id ingenic_pinctrl_of_match[] = {
> > {
> > .compatible = "ingenic,jz4740-pinctrl",
> > - .data = IF_ENABLED(CONFIG_MACH_JZ4740, &jz4740_chip_info)
> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4740), &jz4740_chip_info)
> > },
> > {
> > .compatible = "ingenic,jz4725b-pinctrl",
> > - .data = IF_ENABLED(CONFIG_MACH_JZ4725B, &jz4725b_chip_info)
> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4725B), &jz4725b_chip_info)
> > },
> > {
> > .compatible = "ingenic,jz4760-pinctrl",
> > - .data = IF_ENABLED(CONFIG_MACH_JZ4760, &jz4760_chip_info)
> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4760), &jz4760_chip_info)
> > },
> > {
> > .compatible = "ingenic,jz4760b-pinctrl",
> > - .data = IF_ENABLED(CONFIG_MACH_JZ4760, &jz4760_chip_info)
> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4760), &jz4760_chip_info)
> > },
> > {
> > .compatible = "ingenic,jz4770-pinctrl",
> > - .data = IF_ENABLED(CONFIG_MACH_JZ4770, &jz4770_chip_info)
> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4770), &jz4770_chip_info)
> > },
> > {
> > .compatible = "ingenic,jz4780-pinctrl",
> > - .data = IF_ENABLED(CONFIG_MACH_JZ4780, &jz4780_chip_info)
> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4780), &jz4780_chip_info)
> > },
> > {
> > .compatible = "ingenic,x1000-pinctrl",
> > - .data = IF_ENABLED(CONFIG_MACH_X1000, &x1000_chip_info)
> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_X1000), &x1000_chip_info)
> > },
> > {
> > .compatible = "ingenic,x1000e-pinctrl",
> > - .data = IF_ENABLED(CONFIG_MACH_X1000, &x1000_chip_info)
> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_X1000), &x1000_chip_info)
> > },
> > {
> > .compatible = "ingenic,x1500-pinctrl",
> > - .data = IF_ENABLED(CONFIG_MACH_X1500, &x1500_chip_info)
> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_X1500), &x1500_chip_info)
> > },
> > {
> > .compatible = "ingenic,x1830-pinctrl",
> > - .data = IF_ENABLED(CONFIG_MACH_X1830, &x1830_chip_info)
> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_X1830), &x1830_chip_info)
> > },
> > { /* sentinel */ },
> > };
> > diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
> > index 24a59cb06963..cc8fa109cfa3 100644
> > --- a/include/linux/kconfig.h
> > +++ b/include/linux/kconfig.h
> > @@ -70,10 +70,4 @@
> > */
> > #define IS_ENABLED(option) __or(IS_BUILTIN(option),
> > IS_MODULE(option))
> >
> > -/*
> > - * IF_ENABLED(CONFIG_FOO, ptr) evaluates to (ptr) if CONFIG_FOO is
> > set to 'y'
> > - * or 'm', NULL otherwise.
> > - */
> > -#define IF_ENABLED(option, ptr) (IS_ENABLED(option) ? (ptr) : NULL)
> > -
> > #endif /* __LINUX_KCONFIG_H */
> > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> > index 5b7ed6dc99ac..8685ca4cf287 100644
> > --- a/include/linux/kernel.h
> > +++ b/include/linux/kernel.h
> > @@ -38,6 +38,8 @@
> > #define PTR_ALIGN_DOWN(p, a) ((typeof(p))ALIGN_DOWN((unsigned
> > long)(p), (a)))
> > #define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
> >
> > +#define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
> > +
> > /* generic data direction definitions */
> > #define READ 0
> > #define WRITE 1
> > --
> > 2.27.0
> >
>
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] linux/kconfig.h: replace IF_ENABLED() with PTR_IF() in <linux/kernel.h>
2021-04-08 20:58 ` [PATCH 1/2] linux/kconfig.h: replace IF_ENABLED() with PTR_IF() in <linux/kernel.h> Masahiro Yamada
2021-04-09 8:15 ` Paul Cercueil
@ 2021-04-09 9:24 ` Andy Shevchenko
2021-04-10 3:52 ` Masahiro Yamada
1 sibling, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2021-04-09 9:24 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Linux Kbuild mailing list, open list:GPIO SUBSYSTEM,
Arnd Bergmann, Paul Cercueil, Linus Walleij,
Linux Kernel Mailing List, linux-mips
On Fri, Apr 9, 2021 at 12:00 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> <linux/kconfig.h> is included from all the kernel-space source files,
> including C, assembly, linker scripts. It is intended to contain minimal
a minimal
> set of macros to evaluate CONFIG options.
>
> IF_ENABLED() is an intruder here because (x ? y : z) is C code, which
> should not be included from assembly files or linker scripts.
>
> Also, <linux/kconfig.h> is no longer self-contained because NULL is
> defined in <linux/stddef.h>.
>
> Move IF_ENABLED() out to <linux/kernel.h> as PTR_IF().
>
> PTR_IF(IS_ENABLED(CONFIG_FOO), ...) is slightly longer than
> IF_ENABLED(CONFIG_FOO, ...), but it is not a big deal because
> sub-systems often define dedicated macros such as of_match_ptr(),
> pm_ptr() etc. for common use-cases.
> include/linux/kernel.h | 2 ++
Why kernel.h? Shouldn't it belong to a particular domain with a
respective header file?
Really what we have in the kernel.h right now is a complete train
wreck of something.
We have to define what exactly is kernel.h for?
Arnd? Others? Shall we start a wider discussion on the topic?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] linux/kconfig.h: replace IF_ENABLED() with PTR_IF() in <linux/kernel.h>
2021-04-09 8:30 ` Masahiro Yamada
@ 2021-04-09 9:38 ` Paul Cercueil
0 siblings, 0 replies; 14+ messages in thread
From: Paul Cercueil @ 2021-04-09 9:38 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Linux Kbuild mailing list, GPIO SUBSYSTEM, Arnd Bergmann,
Linus Walleij, Linux Kernel Mailing List, linux-mips
Le ven. 9 avril 2021 à 17:30, Masahiro Yamada <masahiroy@kernel.org> a
écrit :
> On Fri, Apr 9, 2021 at 5:15 PM Paul Cercueil <paul@crapouillou.net>
> wrote:
>>
>> Hi Masahiro,
>>
>> Le ven. 9 avril 2021 à 5:58, Masahiro Yamada
>> <masahiroy@kernel.org> a
>> écrit :
>> > <linux/kconfig.h> is included from all the kernel-space source
>> files,
>> > including C, assembly, linker scripts. It is intended to contain
>> > minimal
>> > set of macros to evaluate CONFIG options.
>> >
>> > IF_ENABLED() is an intruder here because (x ? y : z) is C code,
>> which
>> > should not be included from assembly files or linker scripts.
>> >
>> > Also, <linux/kconfig.h> is no longer self-contained because NULL
>> is
>> > defined in <linux/stddef.h>.
>> >
>> > Move IF_ENABLED() out to <linux/kernel.h> as PTR_IF().
>> >
>> > PTR_IF(IS_ENABLED(CONFIG_FOO), ...) is slightly longer than
>> > IF_ENABLED(CONFIG_FOO, ...), but it is not a big deal because
>> > sub-systems often define dedicated macros such as of_match_ptr(),
>> > pm_ptr() etc. for common use-cases.
>>
>> What's the idea behind changing IF_ENABLED() to PTR_IF()? You didn't
>> explain that. What's wrong with IF_ENABLED()?
>
>
> PTR_IF() is a more generalized variant, which I believe is
> a better fit in <linux/kernel.h>
> The first parameter does not need to be a CONFIG option,
> but any expression.
Fair enough, but we could still have IF_ENABLED as a specialized
variant of PTR_IF:
#define IF_ENABLED(cfg, ptr) PTR_IF(IS_ENABLED(cfg), (ptr))
-Paul
>
>
>> Cheers,
>> -Paul
>>
>> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>> > ---
>> >
>> > drivers/pinctrl/pinctrl-ingenic.c | 20 ++++++++++----------
>> > include/linux/kconfig.h | 6 ------
>> > include/linux/kernel.h | 2 ++
>> > 3 files changed, 12 insertions(+), 16 deletions(-)
>> >
>> > diff --git a/drivers/pinctrl/pinctrl-ingenic.c
>> > b/drivers/pinctrl/pinctrl-ingenic.c
>> > index f2746125b077..b21e2ae4528d 100644
>> > --- a/drivers/pinctrl/pinctrl-ingenic.c
>> > +++ b/drivers/pinctrl/pinctrl-ingenic.c
>> > @@ -2496,43 +2496,43 @@ static int __init
>> > ingenic_pinctrl_probe(struct platform_device *pdev)
>> > static const struct of_device_id ingenic_pinctrl_of_match[] = {
>> > {
>> > .compatible = "ingenic,jz4740-pinctrl",
>> > - .data = IF_ENABLED(CONFIG_MACH_JZ4740,
>> &jz4740_chip_info)
>> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4740),
>> &jz4740_chip_info)
>> > },
>> > {
>> > .compatible = "ingenic,jz4725b-pinctrl",
>> > - .data = IF_ENABLED(CONFIG_MACH_JZ4725B,
>> &jz4725b_chip_info)
>> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4725B),
>> &jz4725b_chip_info)
>> > },
>> > {
>> > .compatible = "ingenic,jz4760-pinctrl",
>> > - .data = IF_ENABLED(CONFIG_MACH_JZ4760,
>> &jz4760_chip_info)
>> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4760),
>> &jz4760_chip_info)
>> > },
>> > {
>> > .compatible = "ingenic,jz4760b-pinctrl",
>> > - .data = IF_ENABLED(CONFIG_MACH_JZ4760,
>> &jz4760_chip_info)
>> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4760),
>> &jz4760_chip_info)
>> > },
>> > {
>> > .compatible = "ingenic,jz4770-pinctrl",
>> > - .data = IF_ENABLED(CONFIG_MACH_JZ4770,
>> &jz4770_chip_info)
>> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4770),
>> &jz4770_chip_info)
>> > },
>> > {
>> > .compatible = "ingenic,jz4780-pinctrl",
>> > - .data = IF_ENABLED(CONFIG_MACH_JZ4780,
>> &jz4780_chip_info)
>> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_JZ4780),
>> &jz4780_chip_info)
>> > },
>> > {
>> > .compatible = "ingenic,x1000-pinctrl",
>> > - .data = IF_ENABLED(CONFIG_MACH_X1000,
>> &x1000_chip_info)
>> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_X1000),
>> &x1000_chip_info)
>> > },
>> > {
>> > .compatible = "ingenic,x1000e-pinctrl",
>> > - .data = IF_ENABLED(CONFIG_MACH_X1000,
>> &x1000_chip_info)
>> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_X1000),
>> &x1000_chip_info)
>> > },
>> > {
>> > .compatible = "ingenic,x1500-pinctrl",
>> > - .data = IF_ENABLED(CONFIG_MACH_X1500,
>> &x1500_chip_info)
>> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_X1500),
>> &x1500_chip_info)
>> > },
>> > {
>> > .compatible = "ingenic,x1830-pinctrl",
>> > - .data = IF_ENABLED(CONFIG_MACH_X1830,
>> &x1830_chip_info)
>> > + .data = PTR_IF(IS_ENABLED(CONFIG_MACH_X1830),
>> &x1830_chip_info)
>> > },
>> > { /* sentinel */ },
>> > };
>> > diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
>> > index 24a59cb06963..cc8fa109cfa3 100644
>> > --- a/include/linux/kconfig.h
>> > +++ b/include/linux/kconfig.h
>> > @@ -70,10 +70,4 @@
>> > */
>> > #define IS_ENABLED(option) __or(IS_BUILTIN(option),
>> > IS_MODULE(option))
>> >
>> > -/*
>> > - * IF_ENABLED(CONFIG_FOO, ptr) evaluates to (ptr) if CONFIG_FOO
>> is
>> > set to 'y'
>> > - * or 'm', NULL otherwise.
>> > - */
>> > -#define IF_ENABLED(option, ptr) (IS_ENABLED(option) ? (ptr) :
>> NULL)
>> > -
>> > #endif /* __LINUX_KCONFIG_H */
>> > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>> > index 5b7ed6dc99ac..8685ca4cf287 100644
>> > --- a/include/linux/kernel.h
>> > +++ b/include/linux/kernel.h
>> > @@ -38,6 +38,8 @@
>> > #define PTR_ALIGN_DOWN(p, a) ((typeof(p))ALIGN_DOWN((unsigned
>> > long)(p), (a)))
>> > #define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) -
>> 1)) == 0)
>> >
>> > +#define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
>> > +
>> > /* generic data direction definitions */
>> > #define READ 0
>> > #define WRITE 1
>> > --
>> > 2.27.0
>> >
>>
>>
>
>
> --
> Best Regards
> Masahiro Yamada
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] linux/kconfig.h: replace IF_ENABLED() with PTR_IF() in <linux/kernel.h>
2021-04-09 9:24 ` Andy Shevchenko
@ 2021-04-10 3:52 ` Masahiro Yamada
0 siblings, 0 replies; 14+ messages in thread
From: Masahiro Yamada @ 2021-04-10 3:52 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linux Kbuild mailing list, open list:GPIO SUBSYSTEM,
Arnd Bergmann, Paul Cercueil, Linus Walleij,
Linux Kernel Mailing List, linux-mips
On Fri, Apr 9, 2021 at 6:24 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Fri, Apr 9, 2021 at 12:00 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > <linux/kconfig.h> is included from all the kernel-space source files,
> > including C, assembly, linker scripts. It is intended to contain minimal
>
> a minimal
>
> > set of macros to evaluate CONFIG options.
> >
> > IF_ENABLED() is an intruder here because (x ? y : z) is C code, which
> > should not be included from assembly files or linker scripts.
> >
> > Also, <linux/kconfig.h> is no longer self-contained because NULL is
> > defined in <linux/stddef.h>.
> >
> > Move IF_ENABLED() out to <linux/kernel.h> as PTR_IF().
> >
> > PTR_IF(IS_ENABLED(CONFIG_FOO), ...) is slightly longer than
> > IF_ENABLED(CONFIG_FOO, ...), but it is not a big deal because
> > sub-systems often define dedicated macros such as of_match_ptr(),
> > pm_ptr() etc. for common use-cases.
>
> > include/linux/kernel.h | 2 ++
>
> Why kernel.h? Shouldn't it belong to a particular domain with a
> respective header file?
>
> Really what we have in the kernel.h right now is a complete train
> wreck of something.
> We have to define what exactly is kernel.h for?
<linux/kernel.h> contains random utility macros.
I did not find a good header to put it in otherwise.
>
> Arnd? Others? Shall we start a wider discussion on the topic?
>
> --
> With Best Regards,
> Andy Shevchenko
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-04-10 3:53 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-08 20:58 [PATCH 0/2] linux/kconfig.h: move IF_ENABLED() out of <linux/kconfig.h> Masahiro Yamada
2021-04-08 20:58 ` [PATCH 1/2] linux/kconfig.h: replace IF_ENABLED() with PTR_IF() in <linux/kernel.h> Masahiro Yamada
2021-04-09 8:15 ` Paul Cercueil
2021-04-09 8:30 ` Masahiro Yamada
2021-04-09 9:38 ` Paul Cercueil
2021-04-09 9:24 ` Andy Shevchenko
2021-04-10 3:52 ` Masahiro Yamada
2021-04-08 20:58 ` [PATCH 2/2] pm: allow drivers to drop #ifdef and __maybe_unused from pm callbacks Masahiro Yamada
2021-04-08 21:29 ` Arnd Bergmann
2021-04-09 3:17 ` Masahiro Yamada
2021-04-08 23:49 ` kernel test robot
2021-04-08 23:49 ` kernel test robot
2021-04-08 23:55 ` kernel test robot
2021-04-08 23:55 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.