linux-kernel - Re: [PATCH ] leds: add new transient trigger for one shot timer support

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 for Android: free password hash cracker in your pocket

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

Date:	Mon, 23 Apr 2012 11:56:10 +1000
From:	NeilBrown <neilb@...e.de>
To:	shuahkhan@...il.com
Cc:	Jonas Bonn <jonas@...thpole.se>,
	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Richard Purdie <richard.purdie@...uxfoundation.org>
Subject: Re: [PATCH ] leds: add new transient trigger for one shot timer
 support

On Sun, 22 Apr 2012 17:51:27 -0600 Shuah Khan <shuahkhan@...il.com> wrote:

> Hi Jonas,
> 
> activate does sound better and having units in the property name is a
> good idea. Will make the changes including the 0200 change for activate.
> Will change the transient_time to transient_time_msec (no problem using
> duration, just that time is shorter :)

I'm not sure I agree.

I think there are two possible correct choices:

 1/ follow the established pattern:  use "delay_off" and assume milliseconds.
    Consistency is good.

 2/ Do the "right" thing which is to use seconds.  "seconds" are the SI 
    unit for time and we should always use them (says Linus - I could
    probably hunt up a reference if you were interested).
    so "0.2" is 200ms.  In that case don't use "delay_off" - probably
    "duration" would be good.
    This doesn't mean you need to use floating point.  See
    "safe_delay_show" and "safe_delay_store" in drivers/md/md.c

But that is just my opinion.

> 
> > A couple of questions:
> > 
> > i)  Why is it important for you to maintain the LED state?
> One use-case that would be easier for user to comprehend would be to
> have a constant timer that runs periodically. time value doesn't change
> and by simply writing 1 or 0, timer with a value specified by the time
> file can be started. The following use-case is a good example for what I
> am talking about:

I agree that "delay" and "activate" should be separate interfaces.

I wonder if we should allow control of the brightness during the "on" time as
well.
You could set the brightness after enabling the timer, but awkward pauses or
races could then leave the "led" permanently on.

Possibly we could hook into led_set_brightness() and restart the timer
whenever the brightness was set - and remember the setting.

So
  echo 100 > brightness

set the current brightness, sets the future brightness, starts the timer.
Or maybe if we have to hook into led_set_brightness anyway, then we could
cause "echo 100 > brightness" *not* to set the current brightness but just
to record a value for later usage...

But these are just minor things.  I think you are close to something useful.

NeilBrown

Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)