linux-kernel - Re: [PATCH 1/2] leds:arch/sh/boards/landisk LEDs supports

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 14 May 2007 21:33:36 +0100
From:	Richard Purdie <rpurdie@...ys.net>
To:	cbou@...l.ru
Cc:	kogiidena@...plant.ddo.jp, linux-kernel@...r.kernel.org,
	lethal@...ux-sh.org
Subject: Re: [PATCH 1/2] leds:arch/sh/boards/landisk LEDs supports

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/