From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EB6F0C433ED for ; Fri, 9 Apr 2021 09:38:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BDB0861181 for ; Fri, 9 Apr 2021 09:38:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231772AbhDIJjG convert rfc822-to-8bit (ORCPT ); Fri, 9 Apr 2021 05:39:06 -0400 Received: from aposti.net ([89.234.176.197]:38162 "EHLO aposti.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230181AbhDIJjG (ORCPT ); Fri, 9 Apr 2021 05:39:06 -0400 Date: Fri, 09 Apr 2021 10:38:39 +0100 From: Paul Cercueil Subject: Re: [PATCH 1/2] linux/kconfig.h: replace IF_ENABLED() with PTR_IF() in To: Masahiro Yamada Cc: Linux Kbuild mailing list , GPIO SUBSYSTEM , Arnd Bergmann , Linus Walleij , Linux Kernel Mailing List , linux-mips@vger.kernel.org Message-Id: In-Reply-To: References: <20210408205858.51751-1-masahiroy@kernel.org> <20210408205858.51751-2-masahiroy@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1; format=flowed Content-Transfer-Encoding: 8BIT Precedence: bulk List-ID: X-Mailing-List: linux-mips@vger.kernel.org Le ven. 9 avril 2021 à 17:30, Masahiro Yamada a écrit : > On Fri, Apr 9, 2021 at 5:15 PM Paul Cercueil > wrote: >> >> Hi Masahiro, >> >> Le ven. 9 avril 2021 à 5:58, Masahiro Yamada >> a >> écrit : >> > 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, is no longer self-contained because NULL >> is >> > defined in . >> > >> > Move IF_ENABLED() out to 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 > 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 >> > --- >> > >> > 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