| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-id: <56025DCA.2000301@samsung.com>
Date: Wed, 23 Sep 2015 10:07:38 +0200
From: Jacek Anaszewski <j.anaszewski@...sung.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: linux-leds@...r.kernel.org, linux-kernel@...r.kernel.org,
sakari.ailus@...ux.intel.com, stsp@...rs.sourceforge.net,
pavel@....cz, ospite@...denti.unina.it, davem@...emloft.net,
linus.walleij@...aro.org, ricardo.ribalda@...il.com,
p.meerwald@...-electronic.com
Subject: Re: [PATCH 2/5] leds: core: Add LED_BLINK_CHANGE and LED_BLINK_DISABLE
flags
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@...sung.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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists