Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1230784 > unrolled thread
| Started by | Andrew Lunn <andrew@lunn.ch> |
|---|---|
| First post | 2015-09-22 21:00 +0200 |
| Last post | 2015-09-23 10:10 +0200 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH 2/5] leds: core: Add LED_BLINK_CHANGE and LED_BLINK_DISABLE flags Andrew Lunn <andrew@lunn.ch> - 2015-09-22 21:00 +0200
Re: [PATCH 2/5] leds: core: Add LED_BLINK_CHANGE and LED_BLINK_DISABLE flags Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-09-23 10:10 +0200
| From | Andrew Lunn <andrew@lunn.ch> |
|---|---|
| Date | 2015-09-22 21:00 +0200 |
| Subject | Re: [PATCH 2/5] leds: core: Add LED_BLINK_CHANGE and LED_BLINK_DISABLE flags |
| Message-ID | <qbAXh-5tC-63@gated-at.bofh.it> |
On Wed, Sep 16, 2015 at 12:47:41PM +0200, Jacek Anaszewski wrote:
> This patch adds LED_BLINK_CHANGE flag to indicate that blink brightness
> has changed
Hi Jacek
The name LED_BLINK_CHANGE does not make me think of changing
brightness. Maybe a better name would be LED_BLINK_BRIGHTNESS_CHANGE,
although that it getting a bit long.
, and LED_BLINK_DISABLE flag to indicate that blinking
> deactivation has been requested. In order to use the flags
> led_timer_function and set_brightness_delayed callbacks as well as
> led_set_brightness function are being modified. The main goal of these
> modifications is to prepare set_brightness_work for extension of the
> scope of its responsibilities.
>
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> ---
> drivers/leds/led-core.c | 36 ++++++++++++++++++++++++++----------
> include/linux/leds.h | 10 ++++++----
> 2 files changed, 32 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index 2cb4e37..1968e24 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -44,18 +44,18 @@ static void led_timer_function(unsigned long data)
> brightness = led_get_brightness(led_cdev);
> if (!brightness) {
> /* Time to switch the LED on. */
> - if (led_cdev->delayed_set_value) {
> - led_cdev->blink_brightness =
> - led_cdev->delayed_set_value;
> - led_cdev->delayed_set_value = 0;
> - }
> brightness = led_cdev->blink_brightness;
> delay = led_cdev->blink_delay_on;
> } else {
> /* Store the current brightness value to be able
> * to restore it when the delay_off period is over.
> + * Do it only if there is no pending blink brightness
> + * change, to avoid overwriting the new value.
> */
> - led_cdev->blink_brightness = brightness;
> + if (!(led_cdev->flags & LED_BLINK_CHANGE))
> + led_cdev->blink_brightness = brightness;
> + else
> + led_cdev->flags &= ~LED_BLINK_CHANGE;
> brightness = LED_OFF;
> delay = led_cdev->blink_delay_off;
> }
> @@ -84,7 +84,11 @@ static void set_brightness_delayed(struct work_struct *ws)
> struct led_classdev *led_cdev =
> container_of(ws, struct led_classdev, set_brightness_work);
>
> - led_stop_software_blink(led_cdev);
> + if (led_cdev->flags & LED_BLINK_DISABLE) {
> + led_cdev->delayed_set_value = LED_OFF;
> + led_stop_software_blink(led_cdev);
> + led_cdev->flags &= ~LED_BLINK_DISABLE;
> + }
>
> led_set_brightness_async(led_cdev, led_cdev->delayed_set_value);
> }
> @@ -192,11 +196,23 @@ void led_set_brightness(struct led_classdev *led_cdev,
> {
> int ret = 0;
>
> - /* delay brightness if soft-blink is active */
> + /*
> + * In case blinking is on delay brightness setting
> + * until the next timer tick.
> + */
> if (led_cdev->blink_delay_on || led_cdev->blink_delay_off) {
> - led_cdev->delayed_set_value = brightness;
> - if (brightness == LED_OFF)
> + /*
> + * If need to disable soft blinking delegate this to the
If _we_ need to...
> + * work queue task to avoid problems in case we are
> + * called from hard irq context.
> + */
> + if (brightness == LED_OFF) {
> + led_cdev->flags |= LED_BLINK_DISABLE;
> schedule_work(&led_cdev->set_brightness_work);
> + } else {
> + led_cdev->flags |= LED_BLINK_CHANGE;
> + led_cdev->blink_brightness = brightness;
> + }
> return;
> }
>
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index b122eea..188352c 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -44,10 +44,12 @@ struct led_classdev {
> #define LED_BLINK_ONESHOT (1 << 17)
> #define LED_BLINK_ONESHOT_STOP (1 << 18)
> #define LED_BLINK_INVERT (1 << 19)
> -#define LED_SYSFS_DISABLE (1 << 20)
> -#define SET_BRIGHTNESS_ASYNC (1 << 21)
> -#define SET_BRIGHTNESS_SYNC (1 << 22)
> -#define LED_DEV_CAP_FLASH (1 << 23)
> +#define LED_BLINK_CHANGE (1 << 20)
> +#define LED_BLINK_DISABLE (1 << 21)
> +#define LED_SYSFS_DISABLE (1 << 22)
> +#define SET_BRIGHTNESS_ASYNC (1 << 23)
> +#define SET_BRIGHTNESS_SYNC (1 << 24)
> +#define LED_DEV_CAP_FLASH (1 << 25)
>
> /* Set LED brightness level */
> /* Must not sleep, use a workqueue if needed */
> --
> 1.7.9.5
>
--
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/
[toc] | [next] | [standalone]
| From | Jacek Anaszewski <j.anaszewski@samsung.com> |
|---|---|
| Date | 2015-09-23 10:10 +0200 |
| Subject | Re: [PATCH 2/5] leds: core: Add LED_BLINK_CHANGE and LED_BLINK_DISABLE flags |
| Message-ID | <qbNhM-6K3-11@gated-at.bofh.it> |
| In reply to | #1230784 |
Hi Andrew,
Thanks for the review.
On 09/22/2015 08:44 PM, Andrew Lunn wrote:
> On Wed, Sep 16, 2015 at 12:47:41PM +0200, Jacek Anaszewski wrote:
>> This patch adds LED_BLINK_CHANGE flag to indicate that blink brightness
>> has changed
>
> Hi Jacek
>
> The name LED_BLINK_CHANGE does not make me think of changing
> brightness. Maybe a better name would be LED_BLINK_BRIGHTNESS_CHANGE,
> although that it getting a bit long.
I had the same dilemma. We could go for shortcut like
LED_BLINK_BR_CHANGE, but is is not too meaningful either.
I will rename LED_BLINK_CHANGE to LED_BLINK_BRIGHTNESS_CHANGE, unless
better option appears.
> , and LED_BLINK_DISABLE flag to indicate that blinking
>> deactivation has been requested. In order to use the flags
>> led_timer_function and set_brightness_delayed callbacks as well as
>> led_set_brightness function are being modified. The main goal of these
>> modifications is to prepare set_brightness_work for extension of the
>> scope of its responsibilities.
>>
>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>> ---
>> drivers/leds/led-core.c | 36 ++++++++++++++++++++++++++----------
>> include/linux/leds.h | 10 ++++++----
>> 2 files changed, 32 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
>> index 2cb4e37..1968e24 100644
>> --- a/drivers/leds/led-core.c
>> +++ b/drivers/leds/led-core.c
>> @@ -44,18 +44,18 @@ static void led_timer_function(unsigned long data)
>> brightness = led_get_brightness(led_cdev);
>> if (!brightness) {
>> /* Time to switch the LED on. */
>> - if (led_cdev->delayed_set_value) {
>> - led_cdev->blink_brightness =
>> - led_cdev->delayed_set_value;
>> - led_cdev->delayed_set_value = 0;
>> - }
>> brightness = led_cdev->blink_brightness;
>> delay = led_cdev->blink_delay_on;
>> } else {
>> /* Store the current brightness value to be able
>> * to restore it when the delay_off period is over.
>> + * Do it only if there is no pending blink brightness
>> + * change, to avoid overwriting the new value.
>> */
>> - led_cdev->blink_brightness = brightness;
>> + if (!(led_cdev->flags & LED_BLINK_CHANGE))
>> + led_cdev->blink_brightness = brightness;
>> + else
>> + led_cdev->flags &= ~LED_BLINK_CHANGE;
>> brightness = LED_OFF;
>> delay = led_cdev->blink_delay_off;
>> }
>> @@ -84,7 +84,11 @@ static void set_brightness_delayed(struct work_struct *ws)
>> struct led_classdev *led_cdev =
>> container_of(ws, struct led_classdev, set_brightness_work);
>>
>> - led_stop_software_blink(led_cdev);
>> + if (led_cdev->flags & LED_BLINK_DISABLE) {
>> + led_cdev->delayed_set_value = LED_OFF;
>> + led_stop_software_blink(led_cdev);
>> + led_cdev->flags &= ~LED_BLINK_DISABLE;
>> + }
>>
>> led_set_brightness_async(led_cdev, led_cdev->delayed_set_value);
>> }
>> @@ -192,11 +196,23 @@ void led_set_brightness(struct led_classdev *led_cdev,
>> {
>> int ret = 0;
>>
>> - /* delay brightness if soft-blink is active */
>> + /*
>> + * In case blinking is on delay brightness setting
>> + * until the next timer tick.
>> + */
>> if (led_cdev->blink_delay_on || led_cdev->blink_delay_off) {
>> - led_cdev->delayed_set_value = brightness;
>> - if (brightness == LED_OFF)
>> + /*
>> + * If need to disable soft blinking delegate this to the
>
> If _we_ need to...
OK.
>> + * work queue task to avoid problems in case we are
>> + * called from hard irq context.
>> + */
>> + if (brightness == LED_OFF) {
>> + led_cdev->flags |= LED_BLINK_DISABLE;
>> schedule_work(&led_cdev->set_brightness_work);
>> + } else {
>> + led_cdev->flags |= LED_BLINK_CHANGE;
>> + led_cdev->blink_brightness = brightness;
>> + }
>> return;
>> }
>>
>> diff --git a/include/linux/leds.h b/include/linux/leds.h
>> index b122eea..188352c 100644
>> --- a/include/linux/leds.h
>> +++ b/include/linux/leds.h
>> @@ -44,10 +44,12 @@ struct led_classdev {
>> #define LED_BLINK_ONESHOT (1 << 17)
>> #define LED_BLINK_ONESHOT_STOP (1 << 18)
>> #define LED_BLINK_INVERT (1 << 19)
>> -#define LED_SYSFS_DISABLE (1 << 20)
>> -#define SET_BRIGHTNESS_ASYNC (1 << 21)
>> -#define SET_BRIGHTNESS_SYNC (1 << 22)
>> -#define LED_DEV_CAP_FLASH (1 << 23)
>> +#define LED_BLINK_CHANGE (1 << 20)
>> +#define LED_BLINK_DISABLE (1 << 21)
>> +#define LED_SYSFS_DISABLE (1 << 22)
>> +#define SET_BRIGHTNESS_ASYNC (1 << 23)
>> +#define SET_BRIGHTNESS_SYNC (1 << 24)
>> +#define LED_DEV_CAP_FLASH (1 << 25)
>>
>> /* Set LED brightness level */
>> /* Must not sleep, use a workqueue if needed */
>> --
>> 1.7.9.5
>>
>
--
Best Regards,
Jacek Anaszewski
--
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/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web