On Mon, 31 Dec 2018, Finn Thain wrote: > On Sun, 30 Dec 2018, James Bottomley wrote: > > > > > 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). > > > > No, in the case of CONFIG_NVRAM=m, the conditional code is deliberately > excluded. This is discussed in the patch description. The convention on > PPC32 is that device drivers drop support for NVRAM in this situation. > > I've adopted the PPC32 convention here because M68K drivers and PPC32 > drivers have to co-exist (I'm thinking of valkyriefb, but there are other > examples). > I agree with your comment in principle, that these drivers should be using IS_REACHABLE(CONFIG_NVRAM), not defined(CONFIG_NVRAM). $ grep -lrw CONFIG_NVRAM drivers/ drivers/video/fbdev/matrox/matroxfb_base.c drivers/video/fbdev/platinumfb.c drivers/video/fbdev/controlfb.c drivers/video/fbdev/valkyriefb.c drivers/video/fbdev/imsttfb.c drivers/scsi/atari_scsi.c $ But I think this is not the fault of this patch; it's just a historical accident. Having said that, I will rework this series to convert all of the above drivers to IS_REACHABLE(CONFIG_NVRAM). I think it would be an improvement. Besides, it seems likely that some rework involving ppc_md will be needed too because as Arnd pointed out, it would be good to avoid two sets of nvram accessor methods. --