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
| ||
|
Date: Mon, 16 Apr 2012 17:05:24 -0600 From: Shuah Khan <shuahkhan@...il.com> To: Jonas Bonn <jonas@...thpole.se> Cc: shuahkhan@...il.com, akpm@...ux-foundation.org, neilb@...e.de, linux-kernel@...r.kernel.org, richard.purdie@...uxfoundation.org Subject: Re: [PATCH 1/1] leds: add "kickable" LED trigger On Tue, 2012-04-17 at 00:33 +0200, Jonas Bonn wrote: > Hi Shuah, > > Nice work! If I may comment on this briefly: > > i) The semantics of this patch make me cringe. It's bad enough that > this is a "LED" trigger; having a "vibrating LED" is even worse. > Really, this is a GPIO trigger (which is reasonably logical); secondly, > this trigger is about maintaining a transient state given some criteria. > > ii) Why force the trigger to a fixed duration? The point, if I > understand correctly, is to return to the "rest state" after a given > time in case the triggering entity dies; but if the triggering entity > (userspace application) is still alive, why not let it prolong the > "active state" by triggering again. This is along the lines of the > 'kickable' trigger patch I posted earlier. > > iii) I don't see the point of maintaining the state. The state is > maintained by the fact that you have running timer; if the timer is not > running, the state is the "rest state". > > I'd suggest: > > i) Calling this ledtrig-transient > ii) Having a property 'duration' that determines how long the > GPIO/LED/signal is 'active' > iii) Having a property 'activate' (or 'tickle'!) that just sets the > 'active' state if it isn't already active; writing to this properly > while active just resets the timer to 0 so that another full 'duration' > can pass before the LED deactivates. > iv) Maybe having a property 'active-state' to decide whether the LED is > on or off when active; this isn't strictly necessary as the LED class > already has the active-low property to invert the meaning of the ON > state. > > This would allow this trigger to be used for a bunch of different use > cases: > > i) Control of vibrator by userspace app. > ii) Use of LED by userspace app as activity indicator. > iii) Use of LED by userspace app as a kind of watchdog indicator -- as > long as the app is alive, it can keep the LED illuminated, if it dies > the LED will be extinguished automatically. > iv) Use by any userspace app that needs a transient GPIO output. > > I hope that makes sense. It mostly comes down to semantics... the > implementation is mostly fine, with the exception of maintaining the > illumination state that I don't see the need for. > > /Jonas Jonas, Thanks for the feedback. I better change the name before too long :) Thanks for summarizing the use-cases. I like your duration and activate idea. I am testing the patch now and will incorporate your ideas. I would like to include you in the thread when I send it out if you don't mind. Thanks again, -- Shuah -- 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/
Powered by blists - more mailing lists