Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758523Ab2EaVGx (ORCPT ); Thu, 31 May 2012 17:06:53 -0400 Received: from mail-wi0-f178.google.com ([209.85.212.178]:57428 "EHLO mail-wi0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752502Ab2EaVGv (ORCPT ); Thu, 31 May 2012 17:06:51 -0400 Date: Thu, 31 May 2012 23:08:00 +0200 From: Fabio Baltieri To: Shuah Khan Cc: Bryan Wu , linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org, Richard Purdie Subject: Re: [PATCH v2 1/3] leds: add oneshot blink functions Message-ID: <20120531210800.GB2400@gmail.com> References: <1338074364-1829-1-git-send-email-fabio.baltieri@gmail.com> <1338135226.2968.22.camel@lorien2> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1338135226.2968.22.camel@lorien2> X-Operating-System: Linux balto-eee 3.4.0-balto-eee-00005-ga209f76 GNU/Linux User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 10470 Lines: 294 Hi Shuah, On Sun, May 27, 2012 at 10:13:46AM -0600, Shuah Khan wrote: > On Sun, 2012-05-27 at 01:19 +0200, Fabio Baltieri wrote: > > Add two new functions, led_blink_set_oneshot and > > led_trigger_blink_oneshot, to be used by triggers for one-shot blink of > > led devices. > > > > This is implemented extending the existing software-blink code, and uses > > the same timer and handler function. > > > > The behavior of the code is to do a blink-on, blink-off sequence when > > the function is called, ignoring other calls until the sequence is > > completed so that the leds keep blinking at constant rate if the > > functions are called repeatedly. > > > > This is meant to be used by drivers which needs to trigger on sporadic > > event, but doesn't have clear busy/idle trigger points. > > > > After the blink sequence the led remains off. This behavior can be > > inverted setting the "invert" argument, which blink the led off, than on > > and leave the led on after the sequence. > > > > Signed-off-by: Fabio Baltieri > > Cc: Bryan Wu > > --- > > > > Same patch as v1 but rebased on Bryan's linux-leds tree. > > Fabio/Bryan, > > Please take a look at linux-next. I am concerned that this work > conflicts with LEDS_TRIGGER_TIMER implementation. Anyways using There should be no compatibity issues with current timer-trigger calls, can you be more specific? > linux-next is more appropriate here, as there were a few patches that > went in while Andrew Morton was acting as a maintainer for leds > sub-system. Ok, anyway v1 of the patch should apply cleanly on current mainline until linus pulls from Bryan. Fabio > -- Shuah > > > > More info in original thread: > > http://marc.info/?l=linux-kernel&m=133763247601381&w=2 > > > > Tested on an x86 system with a custom usb-led device (http://wp.me/p1M1k7-fZ > > for the curious). > > > > Fabio > > > > drivers/leds/led-class.c | 19 +++++++++++++++++ > > drivers/leds/led-core.c | 50 ++++++++++++++++++++++++++++++++++----------- > > drivers/leds/led-triggers.c | 30 +++++++++++++++++++++++---- > > include/linux/leds.h | 25 +++++++++++++++++++++++ > > 4 files changed, 108 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c > > index 5bff843..42d9359 100644 > > --- a/drivers/leds/led-class.c > > +++ b/drivers/leds/led-class.c > > @@ -91,6 +91,11 @@ static void led_timer_function(unsigned long data) > > return; > > } > > > > + if (led_cdev->flags & LED_BLINK_ONESHOT_STOP) { > > + led_cdev->flags &= ~LED_BLINK_ONESHOT_STOP; > > + return; > > + } > > + > > brightness = led_get_brightness(led_cdev); > > if (!brightness) { > > /* Time to switch the LED on. */ > > @@ -107,6 +112,20 @@ static void led_timer_function(unsigned long data) > > > > led_set_brightness(led_cdev, brightness); > > > > + /* Return in next iteration if led is in one-shot mode and we are in > > + * the final blink state so that the led is toggled each delay_on + > > + * delay_off milliseconds in worst case. > > + */ > > + if (led_cdev->flags & LED_BLINK_ONESHOT) { > > + if (led_cdev->flags & LED_BLINK_INVERT) { > > + if (brightness) > > + led_cdev->flags |= LED_BLINK_ONESHOT_STOP; > > + } else { > > + if (!brightness) > > + led_cdev->flags |= LED_BLINK_ONESHOT_STOP; > > + } > > + } > > + > > mod_timer(&led_cdev->blink_timer, jiffies + msecs_to_jiffies(delay)); > > } > > > > diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c > > index d686004..579eb78 100644 > > --- a/drivers/leds/led-core.c > > +++ b/drivers/leds/led-core.c > > @@ -27,7 +27,6 @@ EXPORT_SYMBOL_GPL(leds_list); > > static void led_stop_software_blink(struct led_classdev *led_cdev) > > { > > /* deactivate previous settings */ > > - del_timer_sync(&led_cdev->blink_timer); > > led_cdev->blink_delay_on = 0; > > led_cdev->blink_delay_off = 0; > > } > > @@ -44,11 +43,6 @@ static void led_set_software_blink(struct led_classdev *led_cdev, > > if (!led_cdev->blink_brightness) > > led_cdev->blink_brightness = led_cdev->max_brightness; > > > > - if (led_get_trigger_data(led_cdev) && > > - delay_on == led_cdev->blink_delay_on && > > - delay_off == led_cdev->blink_delay_off) > > - return; > > - > > led_stop_software_blink(led_cdev); > > > > led_cdev->blink_delay_on = delay_on; > > @@ -68,13 +62,12 @@ static void led_set_software_blink(struct led_classdev *led_cdev, > > } > > > > > > -void led_blink_set(struct led_classdev *led_cdev, > > - unsigned long *delay_on, > > - unsigned long *delay_off) > > +void led_blink_setup(struct led_classdev *led_cdev, > > + unsigned long *delay_on, > > + unsigned long *delay_off) > > { > > - del_timer_sync(&led_cdev->blink_timer); > > - > > - if (led_cdev->blink_set && > > + if (!(led_cdev->flags & LED_BLINK_ONESHOT) && > > + led_cdev->blink_set && > > !led_cdev->blink_set(led_cdev, delay_on, delay_off)) > > return; > > > > @@ -84,8 +77,41 @@ void led_blink_set(struct led_classdev *led_cdev, > > > > led_set_software_blink(led_cdev, *delay_on, *delay_off); > > } > > + > > +void led_blink_set(struct led_classdev *led_cdev, > > + unsigned long *delay_on, > > + unsigned long *delay_off) > > +{ > > + del_timer_sync(&led_cdev->blink_timer); > > + > > + led_cdev->flags &= ~LED_BLINK_ONESHOT; > > + led_cdev->flags &= ~LED_BLINK_ONESHOT_STOP; > > + > > + led_blink_setup(led_cdev, delay_on, delay_off); > > +} > > EXPORT_SYMBOL(led_blink_set); > > > > +void led_blink_set_oneshot(struct led_classdev *led_cdev, > > + unsigned long *delay_on, > > + unsigned long *delay_off, > > + int invert) > > +{ > > + if ((led_cdev->flags & LED_BLINK_ONESHOT) && > > + timer_pending(&led_cdev->blink_timer)) > > + return; > > + > > + led_cdev->flags |= LED_BLINK_ONESHOT; > > + led_cdev->flags &= ~LED_BLINK_ONESHOT_STOP; > > + > > + if (invert) > > + led_cdev->flags |= LED_BLINK_INVERT; > > + else > > + led_cdev->flags &= ~LED_BLINK_INVERT; > > + > > + led_blink_setup(led_cdev, delay_on, delay_off); > > +} > > +EXPORT_SYMBOL(led_blink_set_oneshot); > > + > > void led_brightness_set(struct led_classdev *led_cdev, > > enum led_brightness brightness) > > { > > diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c > > index b449ed8..fa0b9be 100644 > > --- a/drivers/leds/led-triggers.c > > +++ b/drivers/leds/led-triggers.c > > @@ -230,9 +230,11 @@ void led_trigger_event(struct led_trigger *trig, > > } > > EXPORT_SYMBOL_GPL(led_trigger_event); > > > > -void led_trigger_blink(struct led_trigger *trig, > > - unsigned long *delay_on, > > - unsigned long *delay_off) > > +void led_trigger_blink_setup(struct led_trigger *trig, > > + unsigned long *delay_on, > > + unsigned long *delay_off, > > + int oneshot, > > + int invert) > > { > > struct list_head *entry; > > > > @@ -244,12 +246,32 @@ void led_trigger_blink(struct led_trigger *trig, > > struct led_classdev *led_cdev; > > > > led_cdev = list_entry(entry, struct led_classdev, trig_list); > > - led_blink_set(led_cdev, delay_on, delay_off); > > + if (oneshot) > > + led_blink_set_oneshot(led_cdev, delay_on, delay_off, > > + invert); > > + else > > + led_blink_set(led_cdev, delay_on, delay_off); > > } > > read_unlock(&trig->leddev_list_lock); > > } > > + > > +void led_trigger_blink(struct led_trigger *trig, > > + unsigned long *delay_on, > > + unsigned long *delay_off) > > +{ > > + led_trigger_blink_setup(trig, delay_on, delay_off, 0, 0); > > +} > > EXPORT_SYMBOL_GPL(led_trigger_blink); > > > > +void led_trigger_blink_oneshot(struct led_trigger *trig, > > + unsigned long *delay_on, > > + unsigned long *delay_off, > > + int invert) > > +{ > > + led_trigger_blink_setup(trig, delay_on, delay_off, 1, invert); > > +} > > +EXPORT_SYMBOL_GPL(led_trigger_blink_oneshot); > > + > > void led_trigger_register_simple(const char *name, struct led_trigger **tp) > > { > > struct led_trigger *trig; > > diff --git a/include/linux/leds.h b/include/linux/leds.h > > index 5884def..f252438 100644 > > --- a/include/linux/leds.h > > +++ b/include/linux/leds.h > > @@ -38,6 +38,9 @@ struct led_classdev { > > #define LED_SUSPENDED (1 << 0) > > /* Upper 16 bits reflect control information */ > > #define LED_CORE_SUSPENDRESUME (1 << 16) > > +#define LED_BLINK_ONESHOT (1 << 17) > > +#define LED_BLINK_ONESHOT_STOP (1 << 18) > > +#define LED_BLINK_INVERT (1 << 19) > > > > /* Set LED brightness level */ > > /* Must not sleep, use a workqueue if needed */ > > @@ -101,6 +104,24 @@ extern void led_blink_set(struct led_classdev *led_cdev, > > unsigned long *delay_on, > > unsigned long *delay_off); > > /** > > + * led_blink_set_oneshot - do a oneshot software blink > > + * @led_cdev: the LED to start blinking > > + * @delay_on: the time it should be on (in ms) > > + * @delay_off: the time it should ble off (in ms) > > + * @invert: blink off, then on, leaving the led on > > + * > > + * This function makes the LED blink one time for delay_on + > > + * delay_off time, ignoring the request if another one-shot > > + * blink is already in progress. > > + * > > + * If invert is set, led blinks for delay_off first, then for > > + * delay_on and leave the led on after the on-off cycle. > > + */ > > +extern void led_blink_set_oneshot(struct led_classdev *led_cdev, > > + unsigned long *delay_on, > > + unsigned long *delay_off, > > + int invert); > > +/** > > * led_brightness_set - set LED brightness > > * @led_cdev: the LED to set > > * @brightness: the brightness to set it to > > @@ -148,6 +169,10 @@ extern void led_trigger_event(struct led_trigger *trigger, > > extern void led_trigger_blink(struct led_trigger *trigger, > > unsigned long *delay_on, > > unsigned long *delay_off); > > +extern void led_trigger_blink_oneshot(struct led_trigger *trigger, > > + unsigned long *delay_on, > > + unsigned long *delay_off, > > + int invert); > > > > #else > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/