Re: [PATCH] i2c: avoid ifdeffery in I2C drivers with optional slave support

[Sascha Hauer <s.hauer@pengutronix.de>]

On Fri, Apr 17, 2020 at 04:00:14PM +0200, Jean Delvare wrote:
> One last thing I meant to mention but forgot...
> 
> On Wed, 15 Apr 2020 07:16:19 +0200, Sascha Hauer wrote:
> > static int i2c_imx_reg_slave(struct i2c_client *client)
> > {
> > 	if (!IS_ENABLED(CONFIG_I2C_SLAVE))
> > 		return -ESOMETHING;
> > 	...
> > }
> > 
> > The code is gone without CONFIG_I2C_SLAVE enabled, yet the compile coverage
> > is there.
> 
> Compile coverage is nice but it comes at a cost. With the approach
> above, the code will be built, then discarded. When the code is
> #ifdef'd out, it isn't built at all. This means that your approach,
> although it has advantages, increases the build time.
> 
> And don't tell me "you only build once", we live at the time of
> continuous integration so we keep building kernels. As a support
> engineer, I build kernels daily, and even though I have access to a
> powerful build farm for that purpose, it still takes 30 to 60 minutes
> to get my kernel built each time. Obviously most of that time isn't
> spent on the i2c-imx driver ;-) but if every piece of code does the
> same, build time will inevitably increase.

Well often enough I spend more time building Kernels than actually
running them, so don't tell me.

Yes, build time increases when we do things in C rather than in CPP,
but each #ifdef doubles the number of builds necessary to let the
compiler go though all paths. I've sent and received more than enough
patches that fix #ifdefs for the more unlikely cases which were not
compile tested.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |