On Wed, Apr 19, 2017 at 08:52:12AM -0600, Mathieu Poirier wrote: [...] > >> > +static bool debug_enable; > >> > +module_param_named(enable, debug_enable, bool, 0600); > >> > +MODULE_PARM_DESC(enable, "Knob to enable debug functionality " > >> > + "(default is 0, which means is disabled by default)"); > >> > >> For this driver we have a debugFS interface so I question the validity of a > >> kernel module parameter. Other than adding complexity to the code it offers no > >> real added value. If a user is to insmod a module, it is just as easy to switch > >> on the functionality using debugFS in a second step. > > > > This module parameter can be used for kernel command line, so > > it's useful when user wants to dynamically turn on/off the > > functionality at boot up time. > > > > Does this make sense for you? Removing this parameter is okay for > > me, but this means users need to decide if use it by Kernel config > > with static building in. This is a bit contradictory with before's > > discussion. > > My hope was to use the kernel command line and the debugFS interface, > avoiding the module parameter. Look at what the baycom_par and > blacklist drivers are doing with the "__setup()" function and see if > we can void a module parameter. If not then let it be, unless someone > else has a better idea. > > [1]. drivers/net/hamradio/baycom_par.c > [2]. drivers/s390/cio/blacklist.c This driver supports module mode. So we can choose to use either module parameter or __setup(). But as described in the file Documentation/admin-guide/kernel-parameters.rst, the module parameter is more flexible to be used at boot time or insmod the module: "Parameters for modules which are built into the kernel need to be specified on the kernel command line. modprobe looks through the kernel command line (/proc/cmdline) and collects module parameters when it loads a module, so the kernel command line can be used for loadable modules too." __setup() cannot support module mode, and when use __setup(), we need register one callback function for it. The callback function is friendly to parse complex parameters rather than module parameter. but it's not necessary for this case. So I'm bias to use module parameter :) Thanks, Leo Yan -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
