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.026 / U:124.360 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site