On Wed, Apr 22, 2015 at 04:45:04PM +0930, Rusty Russell wrote: > "Luis R. Rodriguez" <mcgrof@xxxxxxxxxxxxxxxx> writes: > > From: "Luis R. Rodriguez" <mcgrof@xxxxxxxx> > > > > This adds a couple of bool module_param_config_*() helpers > > which are designed to let us easily associate a booloean > > module parameter with an associated kernel configuration > > option, and to help us remove #ifdef'ery eyesores. > > But they don't. And I had to read the descriptions twice to understand > what you're doing. > > eg you use it like this: > > -#ifdef CONFIG_WQ_POWER_EFFICIENT_DEFAULT > -static bool wq_power_efficient = true; > -#else > -static bool wq_power_efficient; > -#endif > - > -module_param_named(power_efficient, wq_power_efficient, bool, 0444); > +module_param_config_on_off(power_efficient, wq_power_efficient, 0444, > CONFIG_WQ_POWER_EFFICIENT_DEFAULT); > > It would be much clearer to do this: > > -#ifdef CONFIG_WQ_POWER_EFFICIENT_DEFAULT > -static bool wq_power_efficient = true; > -#else > -static bool wq_power_efficient; > -#endif > +static bool wq_power_efficient = IS_ENABLED(CONFIG_WQ_POWER_EFFICIENT_DEFAULT); > > I know exactly what that does without having to notice the difference > between module_param_config_on_off() and module_param_config_on(). You're right, I forgot a small step patch in between to make the change clearer. I can add that in my next respin, anything else or do the other changes look OK? Luis -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
