Messages in this thread | | | From | Jim Cromie <> | Date | Sun, 4 Dec 2011 22:42:42 -0700 | Subject | Re: [PATCH 19/25] pnp: if CONFIG_DYNAMIC_DEBUG, use pnp.ddebug instead of pnp.debug |
| |
2011/12/1 Thomas Renninger <trenn@suse.de>: > On Wednesday 30 November 2011 20:56:48 jim.cromie@gmail.com wrote: >> From: Jim Cromie <jim.cromie@gmail.com> >> >> resubmit of https://lkml.org/lkml/2010/9/15/398 >> >> This allows usage of generic pnp.ddebug debug parameter instead of >> pnp.debug PNP specific parameter.
> It depends on what you compile in which pnp debug parameter one has to use > and both are doing more or less the same? > > We could add two pnp parameters in !defined(CONFIG_DYNAMIC_DEBUG) case: > - deprecate pnp.debug by a message: > "pnp.debug deprecated, use pnp.ddebug" instead
Just to be clear, this patch (yours) does this deprecation.
> - pnp.ddebug doing what pnp.debug is doing currently
FWIW, the patch after this changes the name .ddebug to .dyndbg.
Why is this better than just fixing kernel-parameters to advise using dyndbg directly, and skipping the indirection ?
With the newer unknown-parameter approach that Jason, Rusty recommended (now done), it is possible for a module to implement its own .dyndbg option handler (using __setup only, not with nicer module_param_named() macro, at least with patch 25 included), but that doesnt seem wise:
modname.dyndbg is a fake option, it doesnt show up in /sys/module/pnp/parameters/debug. Adding pnp.dyndbg using __setup would add the sys file, giving an entirely different interface than the one implemented in /dbg/dynamic_debug/control. Explaining this special case sounds difficult to do clearly, a sign of trouble.
> The only misleading would be that pnp.ddebug has nothing to do with > dynamic debug if not compiled in, but user would have one parameter > to rely on. > > In Documentation/kernel-parameters: > pnp.debug [PNP] > Enable PNP debug messages. This depends on the > CONFIG_PNP_DEBUG_MESSAGES option. > > Would be wrong in defined(CONFIG_DYNAMIC_DEBUG) case with your patch, > but would always work with: > pnp.ddebug > with my above suggestions.
how about a something like this ?
pnp.debug=1 [PNP] Enable PNP debug messages (depends on the CONFIG_PNP_DEBUG_MESSAGES option and !CONFIG_DYNAMIC_DEBUG). If CONFIG_DYNAMIC_DEBUG use pnp.dyndbg instead. ...
This approach doesnt add any new failures; if ! CONFIG_DYNAMIC_DEBUG, pnp.dyndbg will fail/warn just like any.dyndbg would else pnp.debug will warn, yndbg will fail/warn just like any.dyndbg
> > It's not a big deal and not a perfect solution, just looks a bit confusing > to have 2 different parameters for the same thing.
I think this is covered adequately by a doc update, and less confusing than different behavior/usage of 1 parameter under 2 different configs.
> > Bjorn should have a look and acknowledge or sign the pnp parts off. > > Thomas >
thanks Jim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |