lkml.org 
[lkml]   [2007]   [May]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 1/2] leds:arch/sh/boards/landisk LEDs supports
    From
    Date
    On Tue, 2007-05-15 at 00:12 +0400, Anton Vorontsov wrote:
    > This change needed for two purposes:
    >
    > 1. When somebody sets trigger, and that trigger would setup
    > brightness in its activate() function, and led driver would check
    > if that trigger supported (used by hwtimer trigger and drivers
    > supporting hw blinking LEDs, not in mainline yet).

    Why does led_cdev->trigger need to be set to do this? Any well written
    LED drivers shouldn't need to look at it as the LED drivers shouldn't
    have or need any knowledge about specific triggers. If they need this,
    you hw blinking abstraction isn't generic.

    I had reservations about the hw blinking support last time we discussed
    it but I can't remember the specifics of that discussion now without
    looking it up. I'm going to hold this patch until we're about the merge
    something that needs it, if it really needs it.

    > 2. If trigger would access itself through led_cdev in its activate()
    > function.

    I can't see why you'd need to do this...

    > Pros of that patch:
    >
    > 1. Just sane to do;
    > 2. Trivial;
    > 3. Can't break anything;
    > 4. No new code, just one line movement.
    >
    > Cons of applying that patch:
    >
    > 1. No mainline kernel user, but offshores.

    2. Encourages bad code ;-).

    --
    Richard


    -
    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: 2007-05-14 22:37    [W:0.032 / U:0.276 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site