Re: [PATCH v2] leds: fix led_brightness_set when soft-blinking

[Shuah Khan]

> I realize now that my last patch broke ledtrig-timer updates as that
> works by passing delay_{on,off} values by pointer, and moving
> led_stop_software_blink() earlier destroy the other value.

Fabio,

This type of interaction is what I am concerned about, and came up
during the transient trigger discussion as well. In the case of
transient trigger it was an easy decision to use separate namespace, but
that doesn't make sense in this case.

> 
> That's a bit of a pitfall, as the old code was working because values
> were copied when entering led_set_software_blink.
> 
> I see three options here:
> - reverting back my leds: "fix led_brightness_set when soft-blinking"
>   (9b05cd0) to the first version I posted, wich should work as before.
> - modify ledtrig-timer to use two internal variables to store delay_on
>   and delay_off instead of the led_cdev ones.

Don't this this is a viable option without restructuring the leds code.
These two variables are common to led_cdev and used by other drivers as
well. ledtrig-timer will still have to store it as these are used in the
led_timer_function() in led-class.c

> - moving the two led_cdev->blink_delay_xx = 0 only into
>   led_brightness_set, as that's the only place when they are needed.

Sounds reasonable. I would like to pull your patches in and do some
testing, but very likely I won't be able to get to it until this
weekend. This is where use-cases will help as well for us go through
various transitions and see if it all comes together.

-- Shuah

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/