lkml.org 
[lkml]   [2011]   [Dec]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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/

\
 
 \ /
  Last update: 2011-12-05 06:45    [W:0.138 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site