On Sat, Jun 14, 2014 at 2:21 AM, Vincent Donnefort <vdonnefort@xxxxxxxxx> wrote:
> From: Vincent Donnefort <vdonnefort@xxxxxxxxx>
>
> This patch converts the blink timer from led-core to workqueue which is more
> suitable for this kind of non-priority operations. Moreover, timer may lead to
> errors when a LED setting function use a scheduling function such as pinctrl
> which is using mutex.
>
Actually I prefer to use workqueue here, too. But I'd like to know
more details about the error you mentioned here.
Any test case?
Thanks,
-Bryan
> Signed-off-by: Vincent Donnefort <vdonnefort@xxxxxxxxx>
>
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index f37d63c..0567a24 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -15,10 +15,10 @@
> #include <linux/list.h>
> #include <linux/spinlock.h>
> #include <linux/device.h>
> -#include <linux/timer.h>
> #include <linux/err.h>
> #include <linux/ctype.h>
> #include <linux/leds.h>
> +#include <linux/workqueue.h>
> #include "leds.h"
>
> static struct class *leds_class;
> @@ -97,9 +97,10 @@ static const struct attribute_group *led_groups[] = {
> NULL,
> };
>
> -static void led_timer_function(unsigned long data)
> +static void led_work_function(struct work_struct *ws)
> {
> - struct led_classdev *led_cdev = (void *)data;
> + struct led_classdev *led_cdev =
> + container_of(ws, struct led_classdev, blink_work.work);
> unsigned long brightness;
> unsigned long delay;
>
> @@ -143,7 +144,8 @@ static void led_timer_function(unsigned long data)
> }
> }
>
> - mod_timer(&led_cdev->blink_timer, jiffies + msecs_to_jiffies(delay));
> + queue_delayed_work(system_wq, &led_cdev->blink_work,
> + msecs_to_jiffies(delay));
> }
>
> static void set_brightness_delayed(struct work_struct *ws)
> @@ -230,9 +232,7 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
>
> INIT_WORK(&led_cdev->set_brightness_work, set_brightness_delayed);
>
> - init_timer(&led_cdev->blink_timer);
> - led_cdev->blink_timer.function = led_timer_function;
> - led_cdev->blink_timer.data = (unsigned long)led_cdev;
> + INIT_DELAYED_WORK(&led_cdev->blink_work, led_work_function);
>
> #ifdef CONFIG_LEDS_TRIGGERS
> led_trigger_set_default(led_cdev);
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index 71b40d3..4bb1168 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -16,6 +16,7 @@
> #include <linux/module.h>
> #include <linux/rwsem.h>
> #include <linux/leds.h>
> +#include <linux/workqueue.h>
> #include "leds.h"
>
> DECLARE_RWSEM(leds_list_lock);
> @@ -51,7 +52,7 @@ static void led_set_software_blink(struct led_classdev *led_cdev,
> return;
> }
>
> - mod_timer(&led_cdev->blink_timer, jiffies + 1);
> + queue_delayed_work(system_wq, &led_cdev->blink_work, 1);
> }
>
>
> @@ -75,7 +76,7 @@ void led_blink_set(struct led_classdev *led_cdev,
> unsigned long *delay_on,
> unsigned long *delay_off)
> {
> - del_timer_sync(&led_cdev->blink_timer);
> + cancel_delayed_work_sync(&led_cdev->blink_work);
>
> led_cdev->flags &= ~LED_BLINK_ONESHOT;
> led_cdev->flags &= ~LED_BLINK_ONESHOT_STOP;
> @@ -90,7 +91,7 @@ void led_blink_set_oneshot(struct led_classdev *led_cdev,
> int invert)
> {
> if ((led_cdev->flags & LED_BLINK_ONESHOT) &&
> - timer_pending(&led_cdev->blink_timer))
> + delayed_work_pending(&led_cdev->blink_work))
> return;
>
> led_cdev->flags |= LED_BLINK_ONESHOT;
> @@ -107,7 +108,7 @@ EXPORT_SYMBOL(led_blink_set_oneshot);
>
> void led_stop_software_blink(struct led_classdev *led_cdev)
> {
> - del_timer_sync(&led_cdev->blink_timer);
> + cancel_delayed_work_sync(&led_cdev->blink_work);
> led_cdev->blink_delay_on = 0;
> led_cdev->blink_delay_off = 0;
> }
> @@ -116,7 +117,7 @@ EXPORT_SYMBOL_GPL(led_stop_software_blink);
> void led_set_brightness(struct led_classdev *led_cdev,
> enum led_brightness brightness)
> {
> - /* delay brightness setting if need to stop soft-blink timer */
> + /* delay brightness setting if need to stop soft-blink work */
> if (led_cdev->blink_delay_on || led_cdev->blink_delay_off) {
> led_cdev->delayed_set_value = brightness;
> schedule_work(&led_cdev->set_brightness_work);
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 0287ab2..89af7b8 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -15,7 +15,6 @@
> #include <linux/list.h>
> #include <linux/spinlock.h>
> #include <linux/rwsem.h>
> -#include <linux/timer.h>
> #include <linux/workqueue.h>
>
> struct device;
> @@ -67,7 +66,7 @@ struct led_classdev {
> const char *default_trigger; /* Trigger to use */
>
> unsigned long blink_delay_on, blink_delay_off;
> - struct timer_list blink_timer;
> + struct delayed_work blink_work;
> int blink_brightness;
>
> struct work_struct set_brightness_work;
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
