Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip

[Paul Walmsley]

On Mon, 25 Mar 2019, Borislav Petkov wrote:

> On Sun, Mar 24, 2019 at 05:23:27PM -0700, Paul Walmsley wrote:
> > > +	if (!IS_ENABLED(CONFIG_EDAC_DEBUG))
> > > +		return;
> > 
> > Can all of these debugfs functions be wrapped with an #if ... #endif such 
> > that, if CONFIG_EDAC_DEBUG is not set, they will all be stripped out by 
> > the preprocessor?
> 
> Why would you make the code more ugly with ifdeffery?
> 
> Do you have any serious code size constraints so that you absolutely
> need to remove a couple of KBs?

We'll definitely take the RAM savings that a few #ifdefs will deliver to 
us.  They add up.  We're selling chips for embedded use cases, not just 
big-iron x86 systems.

Other EDAC drivers have far more #ifdef lines than the single set that I'm 
proposing, so I don't understand why you're singling this driver out for 
criticism.  Consider:

./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_L2C
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_OCRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_OCRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_L2C
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_ETHERNET
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_NAND
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_DMA
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_USB
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_QSPI
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDMMC
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_L2C
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_OCRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_ETHERNET
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_NAND
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_DMA
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_USB
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_QSPI
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDMMC
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDRAM
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG


- Paul