Re: [PATCHv2] LEDS-One-Shot-Timer-Trigger-implementation

[Shuah Khan]

Neil,

I also added documentation file in this second version. Thanks again for
brainstorming ideas and reviews. Leaving the subject line unchanged to
avoid confusion, and the documentation and code are updated to change
one-shot-timer to timer-no-default.

> 
> A separate patch is certainly a good idea.
> I would do the clean-up patches first, and then have the big change at the
> end of the series.  That way checkpatch won't complain about anything in your
> change.
> But you should do what you are most comfortable with.

Decided to do the simple_strtoul() cleanup patch later - after this one
goes through.
> 
> > 
> > > 
> > > > +	
> > > 
> > > Do you need to led_trigger_unregister(&timer_led_trigger) if the registration
> > > of one_shot_timer_led_trigger fails?
> > 
> > Yes, considered that while I was changing this routine, and thought
> > might be better to leave the registered timer alone when the second
> > registration fails. I can go either way.
> 
> The important point is that if timer_trig_init fails, then timer_trig_exit
> will never be called.
> So when timer_trig_init fails, it must ensure that it has un-done any bits
> that it successfully did.

Fixed.

-- Shuah

LED infrastructure lacks support for timer-no-default trigger and activation.
The current support allows for setting two timers, one for specifying how 
long a state to be on, and the second for how long the state to be off. For
example, delay_on value specifies the time period an LED should stay in on
state, followed by a delay_off value that specifies how long the LED should
stay in off state. The on and off cycle repeats until the trigger gets 
deactivated. There is no provision for one time activation to implement
features that require an on or off state to be held just once and then stay
in the original state forever.

This feature will help implement vibrate functionality which requires one
time activation of vibrate mode without a continuous vibrate on/off cycles.

This patch implements the timer-no-default trigger support by enhancing the
current led-class, led-core, and ledtrig-timer drivers to

1. Add support for forever timer case. forever tag can be written to delay_on
or delay_off files. Internally forever is mapped to ULONG_MAX with no timer
associated with it.

2. The led_blink_set() which takes two pointers to times one each for delay_on
and delay_off has been extended so that a NULL instead of a pointer means 
"forever".

3. Add a new timer-no-default trigger to ledtrig-timer


The above enhancements support the following use-cases:

use-case 1:
echo timer-no-default > /sys/class/leds/SOMELED/trigger
echo forever > /sys/class/leds/SOMELED/delay_off
echo 2000 > /sys/class/leds/SOMELED/delay_on

When timer-no-default is activated in step1, unlike the timer trigger case,
timer-no-default activate routine activates the trigger without starting
any timers. The default 1 HZ delay_on and delay_off timers won't be started
like in the case of timer trigger activation. Not starting timers ensures 
that the one time state isn't stuck if some error occurs before actual timer
periods are specified. delay_on and delay_off files get created with 0
values. Please note that it is important to set delay_off to forever prior
to setting delay_on value. If the order is reversed, the LED will be turned
on, with no timer set to turn it off.

When delay_off value is specified in step 2, delay_off_store recognizes the
special forever tag and records it and returns without starting any timer.
Internally forever maps to ULONG_MAX. The led_blink_set() which takes
two pointers to times one each for delay_on and delay_off has been extended so
that a NULL instead of a pointer means "forever".

When delay_on value is specified in step 3, a timer gets started for delay_on
period, and delay_off stays at ULONG_MAX with no timer associated with it. 

use-case 2:
echo timer-no-default > /sys/class/leds/SOMELED/trigger
echo forever > /sys/class/leds/SOMELED/delay_on
echo 2000 > /sys/class/leds/SOMELED/delay_off

When timer-no-default is activated in step1, unlike the timer trigger case,
timer-no-default activate routine activates the trigger without starting
any timers. The default 1 HZ delay_on and delay_off timers won't be started
like in the case of timer trigger activation. Not starting timers ensures 
that the one time state isn't stuck if some error occurs before actual timer
periods are specified. delay_on and delay_off files get created with 0
values. Please note that it is important to set delay_on to forever prior
to setting delay_off value. If the order is reversed, the LED will be turned
off, with no timer set to turn it back on.

When delay_on value is specified in step 2, delay_on_store recognizes the
special forever tag and records it and returns without starting any timer. 
Internally forever maps to ULONG_MAX.

When delay_off value is specified in step 3, a timer gets started for delay_off
period, and delay_on stays at ULONG_MAX with no timer associated with it.