All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Bottomley <jejb@linux.vnet.ibm.com>
To: LEROY Christophe <christophe.leroy@c-s.fr>,
	Arnd Bergmann <arnd@arndb.de>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	linux-m68k <linux-m68k@lists.linux-m68k.org>,
	Finn Thain <fthain@telegraphics.com.au>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	linux-scsi <linux-scsi@vger.kernel.org>,
	Michael Schmitz <schmitzmic@gmail.com>
Subject: Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
Date: Sun, 30 Dec 2018 10:06:43 -0800	[thread overview]
Message-ID: <1546193203.2844.9.camel@linux.vnet.ibm.com> (raw)
In-Reply-To: <20181230185002.Horde.AdSknvcs4hP1Kh_T_Cos6w8@messagerie.si.c-s.fr>

On Sun, 2018-12-30 at 18:50 +0100, LEROY Christophe wrote:
> Arnd Bergmann <arnd@arndb.de> a écrit :
> > On Sat, Dec 29, 2018 at 3:51 AM Michael Schmitz
> > <schmitzmic@gmail.com> wrote:
[...]
> > > (On second thought - I don't want to speculate whether there's
> > > weird compiler options that could result in the
> > > nvram_check_checksum and nvram_read_bytes symbols to still be
> > > referenced in the final link, even though
> > > IS_BUILTIN(CONFIG_NVRAM) always evaluates to false. Best
> > > leave this as-is.)
> > 
> > As far as I know, it's totally reliable with the supported
> > compilers  (gcc-4.6+). In the older compilers (e.g. 4.1), there was
> > a corner case, where it could have failed to eliminate a function
> > that was only referenced through a pointer from a discarded
> > variable, but a plain IS_ENABLED() check like the one here
> > was still ok, and lots of code relies on that.
> > 
> > Other than that, I agree either way is totally fine here, so no
> > objections to using the #ifdef.
> 
> As far as I know, kernel codying style promotes the use of  
> IS_ENABLED() etc. instead of #ifdefs when possible.

It's a preference, as with a lot of coding style stuff, which we leave
up to the maintainer.

That said, as has been pointed out, the current #ifdef has a failing
corner case when both are modular (because the code should then be
included).  The runtime macro that correctly expresses this is
IS_REACHABLE(CONFIG_NVRAM).

James

WARNING: multiple messages have this Message-ID (diff)
From: James Bottomley <jejb@linux.vnet.ibm.com>
To: LEROY Christophe <christophe.leroy@c-s.fr>,
	Arnd Bergmann <arnd@arndb.de>
Cc: Michael Schmitz <schmitzmic@gmail.com>,
	linux-scsi <linux-scsi@vger.kernel.org>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Finn Thain <fthain@telegraphics.com.au>,
	linux-m68k <linux-m68k@lists.linux-m68k.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
Date: Sun, 30 Dec 2018 10:06:43 -0800	[thread overview]
Message-ID: <1546193203.2844.9.camel@linux.vnet.ibm.com> (raw)
In-Reply-To: <20181230185002.Horde.AdSknvcs4hP1Kh_T_Cos6w8@messagerie.si.c-s.fr>

On Sun, 2018-12-30 at 18:50 +0100, LEROY Christophe wrote:
> Arnd Bergmann <arnd@arndb.de> a écrit :
> > On Sat, Dec 29, 2018 at 3:51 AM Michael Schmitz
> > <schmitzmic@gmail.com> wrote:
[...]
> > > (On second thought - I don't want to speculate whether there's
> > > weird compiler options that could result in the
> > > nvram_check_checksum and nvram_read_bytes symbols to still be
> > > referenced in the final link, even though
> > > IS_BUILTIN(CONFIG_NVRAM) always evaluates to false. Best
> > > leave this as-is.)
> > 
> > As far as I know, it's totally reliable with the supported
> > compilers  (gcc-4.6+). In the older compilers (e.g. 4.1), there was
> > a corner case, where it could have failed to eliminate a function
> > that was only referenced through a pointer from a discarded
> > variable, but a plain IS_ENABLED() check like the one here
> > was still ok, and lots of code relies on that.
> > 
> > Other than that, I agree either way is totally fine here, so no
> > objections to using the #ifdef.
> 
> As far as I know, kernel codying style promotes the use of  
> IS_ENABLED() etc. instead of #ifdefs when possible.

It's a preference, as with a lot of coding style stuff, which we leave
up to the maintainer.

That said, as has been pointed out, the current #ifdef has a failing
corner case when both are modular (because the code should then be
included).  The runtime macro that correctly expresses this is
IS_REACHABLE(CONFIG_NVRAM).

James


  reply	other threads:[~2018-12-30 18:06 UTC|newest]

Thread overview: 167+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-26  0:37 [PATCH v8 00/25] Re-use nvram module Finn Thain
2018-12-26  0:37 ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 10/25] m68k/mac: Adopt naming and calling conventions for PRAM routines Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 14/25] char/nvram: Add "devname:nvram" module alias Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 05/25] char/nvram: Adopt arch_nvram_ops Finn Thain
2018-12-26  0:37   ` Finn Thain
2019-01-03  8:02   ` Christoph Hellwig
2019-01-03  8:02     ` Christoph Hellwig
2019-01-03 22:08     ` Finn Thain
2019-01-03 22:08       ` Finn Thain
2019-01-04 17:56       ` Christoph Hellwig
2019-01-04 17:56         ` Christoph Hellwig
2019-01-04 22:05         ` Finn Thain
2019-01-04 22:05           ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 15/25] powerpc: Clean up nvram includes Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 06/25] x86/thinkpad_acpi: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_write_byte() Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 13/25] m68k: Dispatch nvram_ops calls to Atari or Mac functions Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-29 22:27   ` Arnd Bergmann
2018-12-29 22:27     ` Arnd Bergmann
2018-12-30  7:26     ` Finn Thain
2018-12-30  7:26       ` Finn Thain
2018-12-30 17:53       ` LEROY Christophe
2018-12-30 17:53         ` LEROY Christophe
2018-12-30 22:12         ` Finn Thain
2018-12-30 22:12           ` Finn Thain
2018-12-31 12:19           ` Arnd Bergmann
2018-12-31 12:19             ` Arnd Bergmann
2018-12-26  0:37 ` [PATCH v8 22/25] powerpc: Remove CONFIG_GENERIC_NVRAM and adopt CONFIG_HAVE_ARCH_NVRAM_OPS Finn Thain
2018-12-26  0:37   ` Finn Thain
2019-01-03  8:05   ` Christoph Hellwig
2019-01-03  8:05     ` Christoph Hellwig
2019-01-03 22:11     ` Finn Thain
2019-01-03 22:11       ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 02/25] m68k/atari: Move Atari-specific code out of drivers/char/nvram.c Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-28 16:51   ` LEROY Christophe
2018-12-28 16:51     ` LEROY Christophe
2018-12-29  1:16     ` Finn Thain
2018-12-29  1:16       ` Finn Thain
2019-01-02 22:29       ` Finn Thain
2019-01-02 22:29         ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 11/25] m68k/mac: Use macros for RTC accesses not magic numbers Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 12/25] m68k/mac: Fix PRAM accessors Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 20/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_w Finn Thain
2018-12-26  0:37   ` [PATCH v8 20/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_write_byte() Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-29 17:02   ` [PATCH v8 20/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvr LEROY Christophe
2018-12-29 17:02     ` [PATCH v8 20/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_write_byte() LEROY Christophe
2018-12-29 17:02     ` LEROY Christophe
2018-12-29 23:43     ` [PATCH v8 20/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvr Finn Thain
2018-12-29 23:43       ` [PATCH v8 20/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_write_byte() Finn Thain
2018-12-29 23:43       ` Finn Thain
2018-12-31 12:29       ` [PATCH v8 20/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvr Arnd Bergmann
2018-12-31 12:29         ` [PATCH v8 20/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_write_byte() Arnd Bergmann
2018-12-31 12:29         ` Arnd Bergmann
2018-12-31 12:29         ` Arnd Bergmann
2019-01-01  1:10         ` [PATCH v8 20/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvr Finn Thain
2019-01-01  1:10           ` [PATCH v8 20/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_write_byte() Finn Thain
2019-01-01  1:10           ` Finn Thain
2019-01-05 23:06       ` [PATCH v8 20/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvr Finn Thain
2019-01-05 23:06         ` [PATCH v8 20/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_write_byte() Finn Thain
2019-01-05 23:06         ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 25/25] powerpc: Remove pmac_xpram_{read,write} functions Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 18/25] powerpc: Implement nvram sync ioctl Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-29 22:19   ` Arnd Bergmann
2018-12-29 22:19     ` Arnd Bergmann
2018-12-30  7:25     ` Finn Thain
2018-12-30  7:25       ` Finn Thain
2018-12-30 23:13       ` Finn Thain
2018-12-30 23:13         ` Finn Thain
2018-12-31 12:17         ` Arnd Bergmann
2018-12-31 12:17           ` Arnd Bergmann
2018-12-31 12:16       ` Arnd Bergmann
2018-12-31 12:16         ` Arnd Bergmann
2019-01-01  1:06         ` Finn Thain
2019-01-01  1:06           ` Finn Thain
2019-01-02 22:25           ` Finn Thain
2019-01-02 22:25             ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 24/25] powerpc: Adopt nvram module for PPC64 Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-29 22:36   ` Arnd Bergmann
2018-12-29 22:36     ` Arnd Bergmann
2018-12-30  3:28     ` Finn Thain
2018-12-30  3:28       ` Finn Thain
2018-12-31 12:32       ` Arnd Bergmann
2018-12-31 12:32         ` Arnd Bergmann
2019-01-01  1:38         ` Finn Thain
2019-01-01  1:38           ` Finn Thain
2019-01-04  8:45         ` Finn Thain
2019-01-04  8:45           ` Finn Thain
2019-01-06 23:36     ` Michael Ellerman
2019-01-06 23:36       ` Michael Ellerman
2019-01-07  4:52       ` Finn Thain
2019-01-07  4:52         ` Finn Thain
2019-01-08  9:27         ` Michael Ellerman
2019-01-08  9:27           ` Michael Ellerman
2019-01-08 22:51           ` Finn Thain
2019-01-08 22:51             ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 04/25] char/nvram: Re-order functions to remove forward declarations and #ifdefs Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 23/25] char/generic_nvram: Remove as unused Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 08/25] char/nvram: Implement NVRAM read/write methods Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 19/25] powerpc, fbdev: Use NV_CMODE and NV_VMODE only when CONFIG_PPC32 && CONFIG_PPC_PMAC Finn Thain
2018-12-26  0:37   ` [PATCH v8 19/25] powerpc, fbdev: Use NV_CMODE and NV_VMODE only when CONFIG_PPC32 && CONFIG_PPC_PMAC && CONFIG_NVRAM Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 21/25] nvram: Drop nvram_* symbol exports and prototypes Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 07/25] char/nvram: Allow the set_checksum and initialize ioctls to be omitted Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 16/25] powerpc: Add missing ppc_md.nvram_size for CHRP and PowerMac Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-28 16:38   ` LEROY Christophe
2018-12-28 16:38     ` LEROY Christophe
2018-12-29  1:06     ` Finn Thain
2018-12-29  1:06       ` Finn Thain
2018-12-29  1:33       ` Michael Schmitz
2018-12-29  1:33         ` Michael Schmitz
2018-12-29  2:34         ` Finn Thain
2018-12-29  2:34           ` Finn Thain
2018-12-29  2:50           ` Michael Schmitz
2018-12-29  2:50             ` Michael Schmitz
2018-12-29 21:37             ` Arnd Bergmann
2018-12-29 21:37               ` Arnd Bergmann
2018-12-30 17:50               ` LEROY Christophe
2018-12-30 17:50                 ` LEROY Christophe
2018-12-30 18:06                 ` James Bottomley [this message]
2018-12-30 18:06                   ` James Bottomley
2018-12-30 21:44                   ` Finn Thain
2018-12-30 21:44                     ` Finn Thain
2018-12-30 22:45                     ` Finn Thain
2018-12-30 22:45                       ` Finn Thain
2018-12-29 16:55         ` LEROY Christophe
2018-12-29 16:55           ` LEROY Christophe
2018-12-29 18:54           ` Michael Schmitz
2018-12-29 18:54             ` Michael Schmitz
2018-12-29  2:54   ` Michael Schmitz
2018-12-29  2:54     ` Michael Schmitz
2018-12-26  0:37 ` [PATCH v8 09/25] m68k/atari: Implement arch_nvram_ops methods and enable CONFIG_HAVE_ARCH_NVRAM_OPS Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 03/25] m68k/atari: Replace nvram_{read,write}_byte with arch_nvram_ops Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-26  0:37 ` [PATCH v8 17/25] powerpc: Implement arch_nvram_ops.get_size() and remove old nvram_* exports Finn Thain
2018-12-26  0:37   ` Finn Thain
2018-12-29 22:45 ` [PATCH v8 00/25] Re-use nvram module Arnd Bergmann
2018-12-29 22:45   ` Arnd Bergmann
2018-12-30  0:09   ` Finn Thain
2018-12-30  0:09     ` Finn Thain
2018-12-30  4:05     ` Finn Thain
2018-12-30  4:05       ` Finn Thain
2018-12-31 12:22       ` Arnd Bergmann
2018-12-31 12:22         ` Arnd Bergmann
2018-12-31 22:54       ` Finn Thain
2018-12-31 22:54         ` Finn Thain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1546193203.2844.9.camel@linux.vnet.ibm.com \
    --to=jejb@linux.vnet.ibm.com \
    --cc=arnd@arndb.de \
    --cc=christophe.leroy@c-s.fr \
    --cc=fthain@telegraphics.com.au \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=martin.petersen@oracle.com \
    --cc=schmitzmic@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.