mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 1/2] kconfig.h: Add IF_ENABLED() macro
@ 2020-12-13 23:54 Paul Cercueil
  2020-12-13 23:54 ` [PATCH v2 2/2] pinctrl: ingenic: Only support SoCs enabled in config Paul Cercueil
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Paul Cercueil @ 2020-12-13 23:54 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Arnd Bergmann, od, linux-gpio, linux-kernel, Zhou Yanjie, Paul Cercueil

IF_ENABLED(CONFIG_FOO, ptr) evaluates to (ptr) if CONFIG_FOO is set to 'y'
or 'm', NULL otherwise. The (ptr) argument must be a pointer.

The IF_ENABLED() macro can be very useful to help GCC drop dead code.

For instance, consider the following:

    #ifdef CONFIG_FOO_SUSPEND
    static int foo_suspend(struct device *dev)
    {
       ...
    }
    #endif

    static struct pm_ops foo_ops = {
	#ifdef CONFIG_FOO_SUSPEND
        .suspend = foo_suspend,
	#endif
    };

While this works, the foo_suspend() macro is compiled conditionally,
only when CONFIG_FOO_SUSPEND is set. This is problematic, as there could
be a build bug in this function, we wouldn't have a way to know unless
the config option is set.

An alternative is to declare foo_suspend() always, but mark it as maybe
unused:

    static int __maybe_unused foo_suspend(struct device *dev)
    {
       ...
    }

    static struct pm_ops foo_ops = {
	#ifdef CONFIG_FOO_SUSPEND
        .suspend = foo_suspend,
	#endif
    };

Again, this works, but the __maybe_unused attribute is required to
instruct the compiler that the function may not be referenced anywhere,
and is safe to remove without making a fuss about it. This makes the
programmer responsible for tagging the functions that can be
garbage-collected.

With this patch, it is now possible to write the following:

    static int foo_suspend(struct device *dev)
    {
       ...
    }

    static struct pm_ops foo_ops = {
        .suspend = IF_ENABLED(CONFIG_FOO_SUSPEND, foo_suspend),
    };

The foo_suspend() function will now be automatically dropped by the
compiler, and it does not require any specific attribute.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Notes:
    v2: Only add IF_ENABLED() and don't verify that the argument is a
        pointer; since the second argument of the ?: ternary operator
    	is NULL, the compiler should issue an error if the other
    	argument is not a pointer.

 include/linux/kconfig.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index 9d12c970f18f..e78e17a76dc9 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -72,4 +72,10 @@
  */
 #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 */
-- 
2.29.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] pinctrl: ingenic: Only support SoCs enabled in config
  2020-12-13 23:54 [PATCH v2 1/2] kconfig.h: Add IF_ENABLED() macro Paul Cercueil
@ 2020-12-13 23:54 ` Paul Cercueil
  2020-12-14  9:05 ` [PATCH v2 1/2] kconfig.h: Add IF_ENABLED() macro Linus Walleij
  2021-01-18 15:14 ` Linus Walleij
  2 siblings, 0 replies; 6+ messages in thread
From: Paul Cercueil @ 2020-12-13 23:54 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Arnd Bergmann, od, linux-gpio, linux-kernel, Zhou Yanjie, Paul Cercueil

Tested on a JZ4740 system (ARCH=mips make qi_lb60_defconfig), this saves
about 14 KiB, by allowing the compiler to garbage-collect all the
functions and tables that correspond to SoCs that were disabled in the
config.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Notes:
    v2: Drop <linux/if_enabled.h> include

 drivers/pinctrl/pinctrl-ingenic.c | 60 ++++++++++++++++++++++++-------
 1 file changed, 48 insertions(+), 12 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-ingenic.c b/drivers/pinctrl/pinctrl-ingenic.c
index a14938a7cc30..cb8fa261110a 100644
--- a/drivers/pinctrl/pinctrl-ingenic.c
+++ b/drivers/pinctrl/pinctrl-ingenic.c
@@ -2384,6 +2384,12 @@ static int __init ingenic_pinctrl_probe(struct platform_device *pdev)
 	unsigned int i;
 	int err;
 
+	chip_info = of_device_get_match_data(dev);
+	if (!chip_info) {
+		dev_err(dev, "Unsupported SoC\n");
+		return -EINVAL;
+	}
+
 	jzpc = devm_kzalloc(dev, sizeof(*jzpc), GFP_KERNEL);
 	if (!jzpc)
 		return -ENOMEM;
@@ -2400,7 +2406,7 @@ static int __init ingenic_pinctrl_probe(struct platform_device *pdev)
 	}
 
 	jzpc->dev = dev;
-	jzpc->info = chip_info = of_device_get_match_data(dev);
+	jzpc->info = chip_info;
 
 	pctl_desc = devm_kzalloc(&pdev->dev, sizeof(*pctl_desc), GFP_KERNEL);
 	if (!pctl_desc)
@@ -2470,17 +2476,47 @@ 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 = &jz4740_chip_info },
-	{ .compatible = "ingenic,jz4725b-pinctrl", .data = &jz4725b_chip_info },
-	{ .compatible = "ingenic,jz4760-pinctrl", .data = &jz4760_chip_info },
-	{ .compatible = "ingenic,jz4760b-pinctrl", .data = &jz4760_chip_info },
-	{ .compatible = "ingenic,jz4770-pinctrl", .data = &jz4770_chip_info },
-	{ .compatible = "ingenic,jz4780-pinctrl", .data = &jz4780_chip_info },
-	{ .compatible = "ingenic,x1000-pinctrl", .data = &x1000_chip_info },
-	{ .compatible = "ingenic,x1000e-pinctrl", .data = &x1000_chip_info },
-	{ .compatible = "ingenic,x1500-pinctrl", .data = &x1500_chip_info },
-	{ .compatible = "ingenic,x1830-pinctrl", .data = &x1830_chip_info },
-	{},
+	{
+		.compatible = "ingenic,jz4740-pinctrl",
+		.data = IF_ENABLED(CONFIG_MACH_JZ4740, &jz4740_chip_info)
+	},
+	{
+		.compatible = "ingenic,jz4725b-pinctrl",
+		.data = IF_ENABLED(CONFIG_MACH_JZ4725B, &jz4725b_chip_info)
+	},
+	{
+		.compatible = "ingenic,jz4760-pinctrl",
+		.data = IF_ENABLED(CONFIG_MACH_JZ4760, &jz4760_chip_info)
+	},
+	{
+		.compatible = "ingenic,jz4760b-pinctrl",
+		.data = IF_ENABLED(CONFIG_MACH_JZ4760, &jz4760_chip_info)
+	},
+	{
+		.compatible = "ingenic,jz4770-pinctrl",
+		.data = IF_ENABLED(CONFIG_MACH_JZ4770, &jz4770_chip_info)
+	},
+	{
+		.compatible = "ingenic,jz4780-pinctrl",
+		.data = IF_ENABLED(CONFIG_MACH_JZ4780, &jz4780_chip_info)
+	},
+	{
+		.compatible = "ingenic,x1000-pinctrl",
+		.data = IF_ENABLED(CONFIG_MACH_X1000, &x1000_chip_info)
+	},
+	{
+		.compatible = "ingenic,x1000e-pinctrl",
+		.data = IF_ENABLED(CONFIG_MACH_X1000, &x1000_chip_info)
+	},
+	{
+		.compatible = "ingenic,x1500-pinctrl",
+		.data = IF_ENABLED(CONFIG_MACH_X1500, &x1500_chip_info)
+	},
+	{
+		.compatible = "ingenic,x1830-pinctrl",
+		.data = IF_ENABLED(CONFIG_MACH_X1830, &x1830_chip_info)
+	},
+	{ /* sentinel */ },
 };
 
 static struct platform_driver ingenic_pinctrl_driver = {
-- 
2.29.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] kconfig.h: Add IF_ENABLED() macro
  2020-12-13 23:54 [PATCH v2 1/2] kconfig.h: Add IF_ENABLED() macro Paul Cercueil
  2020-12-13 23:54 ` [PATCH v2 2/2] pinctrl: ingenic: Only support SoCs enabled in config Paul Cercueil
@ 2020-12-14  9:05 ` Linus Walleij
  2021-01-11  8:45   ` Paul Cercueil
  2021-01-18 15:14 ` Linus Walleij
  2 siblings, 1 reply; 6+ messages in thread
From: Linus Walleij @ 2020-12-14  9:05 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Arnd Bergmann, od, open list:GPIO SUBSYSTEM, linux-kernel, Zhou Yanjie

On Mon, Dec 14, 2020 at 12:54 AM Paul Cercueil <paul@crapouillou.net> wrote:

> IF_ENABLED(CONFIG_FOO, ptr) evaluates to (ptr) if CONFIG_FOO is set to 'y'
> or 'm', NULL otherwise. The (ptr) argument must be a pointer.
>
> The IF_ENABLED() macro can be very useful to help GCC drop dead code.

I can apply this with the other patch to the pinctrl subsystem if Arnd or
someone else who is good at KConfig provides an ACK.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] kconfig.h: Add IF_ENABLED() macro
  2020-12-14  9:05 ` [PATCH v2 1/2] kconfig.h: Add IF_ENABLED() macro Linus Walleij
@ 2021-01-11  8:45   ` Paul Cercueil
  2021-01-15 11:47     ` Arnd Bergmann
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Cercueil @ 2021-01-11  8:45 UTC (permalink / raw)
  To: Linus Walleij, Arnd Bergmann
  Cc: od, GPIO SUBSYSTEM, linux-kernel, Zhou Yanjie

Hi,

Le lun. 14 déc. 2020 à 10:05, Linus Walleij 
<linus.walleij@linaro.org> a écrit :
> On Mon, Dec 14, 2020 at 12:54 AM Paul Cercueil <paul@crapouillou.net> 
> wrote:
> 
>>  IF_ENABLED(CONFIG_FOO, ptr) evaluates to (ptr) if CONFIG_FOO is set 
>> to 'y'
>>  or 'm', NULL otherwise. The (ptr) argument must be a pointer.
>> 
>>  The IF_ENABLED() macro can be very useful to help GCC drop dead 
>> code.
> 
> I can apply this with the other patch to the pinctrl subsystem if 
> Arnd or
> someone else who is good at KConfig provides an ACK.

Arnd? Any feedback?

Cheers,
-Paul



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] kconfig.h: Add IF_ENABLED() macro
  2021-01-11  8:45   ` Paul Cercueil
@ 2021-01-15 11:47     ` Arnd Bergmann
  0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2021-01-15 11:47 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Linus Walleij, od, GPIO SUBSYSTEM, linux-kernel, Zhou Yanjie

On Mon, Jan 11, 2021 at 9:45 AM Paul Cercueil <paul@crapouillou.net> wrote:
>
> Hi,
>
> Le lun. 14 déc. 2020 à 10:05, Linus Walleij
> <linus.walleij@linaro.org> a écrit :
> > On Mon, Dec 14, 2020 at 12:54 AM Paul Cercueil <paul@crapouillou.net>
> > wrote:
> >
> >>  IF_ENABLED(CONFIG_FOO, ptr) evaluates to (ptr) if CONFIG_FOO is set
> >> to 'y'
> >>  or 'm', NULL otherwise. The (ptr) argument must be a pointer.
> >>
> >>  The IF_ENABLED() macro can be very useful to help GCC drop dead
> >> code.

Looks great! Thanks a lot for getting this sorted.

> > I can apply this with the other patch to the pinctrl subsystem if
> > Arnd or someone else who is good at KConfig provides an ACK.

Yes, please do.

Acked-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] kconfig.h: Add IF_ENABLED() macro
  2020-12-13 23:54 [PATCH v2 1/2] kconfig.h: Add IF_ENABLED() macro Paul Cercueil
  2020-12-13 23:54 ` [PATCH v2 2/2] pinctrl: ingenic: Only support SoCs enabled in config Paul Cercueil
  2020-12-14  9:05 ` [PATCH v2 1/2] kconfig.h: Add IF_ENABLED() macro Linus Walleij
@ 2021-01-18 15:14 ` Linus Walleij
  2 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2021-01-18 15:14 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Arnd Bergmann, od, open list:GPIO SUBSYSTEM, linux-kernel, Zhou Yanjie

Both patches applied!

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-01-18 15:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-13 23:54 [PATCH v2 1/2] kconfig.h: Add IF_ENABLED() macro Paul Cercueil
2020-12-13 23:54 ` [PATCH v2 2/2] pinctrl: ingenic: Only support SoCs enabled in config Paul Cercueil
2020-12-14  9:05 ` [PATCH v2 1/2] kconfig.h: Add IF_ENABLED() macro Linus Walleij
2021-01-11  8:45   ` Paul Cercueil
2021-01-15 11:47     ` Arnd Bergmann
2021-01-18 15:14 ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®