Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM

[Finn Thain]

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.

--