Re: [PATCH] Revert "leds: save the delay values after a successful call to blink_set()"

[Johan Hovold <jhovold@gmail.com>]

On Wed, Nov 16, 2011 at 10:21:00PM +0100, Antonio Ospite wrote:
> On Mon,  7 Nov 2011 11:36:37 +0100
> Johan Hovold <jhovold@gmail.com> wrote:
> 
> > This reverts commit 6123b0e274503a0d3588e84fbe07c9aa01bfaf5d.
> > 
> > The problem this patch intends to solve has already been fixed by commit
> > 7a5caabd090b8f7 (drivers/leds/ledtrig-timer.c: fix broken sysfs delay
> > handling).
> > 
> > Signed-off-by: Johan Hovold <jhovold@gmail.com>
> 
> Sorry for not replying earlier, I was on vacation.
> 
> Thanks Johan, I've just verified the timer trigger works fine on 3.1
> indeed even without my changes. But keep reading, please.
> 
> > ---
> > 
> > Hi, 
> > 
> > A fix for the problem with the delay parameters not being stored is already in
> > 3.1 (and stable). Rather than storing both values at every call to
> > led_blink_set only the updated value is stored in led_delay_{on,off}_store in
> > ledtrig-timer.c.
> >
> 
> I thought that this was better fixed in the led_blink_set() directly,
> as delay_on and delay_off are led_cdev fields after all, not
> necessarily specific to ledtrig-timer, what about other users of them?
> For example I can see led_blink_set() used in: net/mac80211/led.c
> and led_trigger_blink(), which uses led_blink_set() in:
> drivers/power/power_supply_leds.c
> 
> Maybe I am missing something.

When ledtrig-timer was generalised to led_set_blink() with fallback to
software blinking, the blink_delay_{on,off} parameters were moved to
led_classdev and now have two, partly overlapping uses: to store the
ledtrig-timer delay parameters and to implement generic software
blinking.

Note that both uses are internal -- drivers implementing hardware
blinking through blink_set() receives both parameters at each call and
need not access these fields directly. And apart from the ledtrig-timer
sysfs interface there is no other way to read them back either.

In particular, there is currently no need to update them at each call to
led_blink_set() as far as I can see.

Thanks,
Johan