| 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: <555DDC01.9050502@samsung.com>
Date: Thu, 21 May 2015 15:22:09 +0200
From: Jacek Anaszewski <j.anaszewski@...sung.com>
To: Stas Sergeev <stsp@...t.ru>
Cc: linux-leds@...r.kernel.org,
Linux kernel <linux-kernel@...r.kernel.org>,
Stas Sergeev <stsp@...rs.sourceforge.net>,
Bryan Wu <cooloney@...il.com>,
Richard Purdie <rpurdie@...ys.net>,
Kyungmin Park <kyungmin.park@...sung.com>
Subject: Re: [PATCH 01/20] leds: implement LED_BRIGHTNESS_FAST flag
Hi Stas,
My intention was to add a developer of each driver on
cc in the commit message. This way authors of drivers that
you classified as the fast ones could provide us with the
feedback in case there were some non-obvious obstacles
preventing the drivers from setting the brightness with
high frequency.
On 05/20/2015 05:19 PM, Stas Sergeev wrote:
>
> Add LED_BRIGHTNESS_FAST flag. This flag is used to mark the led drivers
> that do not use waiting operations when setting led brightness and do not
> use work-queue in .brightness_set op.
> When this flag is not set, disallow the blink periods smaller than 10mS
> (LED_SLOW_MIN_PERIOD define).
>
> CC: Bryan Wu <cooloney@...il.com>
> CC: Richard Purdie <rpurdie@...ys.net>
> CC: Jacek Anaszewski <j.anaszewski@...sung.com>
> CC: Kyungmin Park <kyungmin.park@...sung.com>
> CC: linux-leds@...r.kernel.org
> CC: linux-kernel@...r.kernel.org
>
> Signed-off-by: Stas Sergeev <stsp@...rs.sourceforge.net>
> ---
> drivers/leds/led-core.c | 30 +++++++++++++++++++-----------
> drivers/leds/led-triggers.c | 8 +++++---
> drivers/leds/trigger/ledtrig-oneshot.c | 5 ++++-
> drivers/leds/trigger/ledtrig-timer.c | 18 +++++++++++++-----
> include/linux/leds.h | 12 ++++++++++--
> 5 files changed, 51 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index 9886dac..89241aa 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -25,12 +25,19 @@ EXPORT_SYMBOL_GPL(leds_list_lock);
> LIST_HEAD(leds_list);
> EXPORT_SYMBOL_GPL(leds_list);
>
> -static void led_set_software_blink(struct led_classdev *led_cdev,
> +static int led_set_software_blink(struct led_classdev *led_cdev,
> unsigned long delay_on,
> unsigned long delay_off)
> {
> int current_brightness;
>
> + if (!(led_cdev->flags & LED_BRIGHTNESS_FAST)) {
> + if (delay_on < LED_SLOW_MIN_PERIOD)
> + return -ERANGE;
> + if (delay_off < LED_SLOW_MIN_PERIOD)
> + return -ERANGE;
> + }
> +
> current_brightness = led_get_brightness(led_cdev);
> if (current_brightness)
> led_cdev->blink_brightness = current_brightness;
> @@ -43,36 +50,37 @@ static void led_set_software_blink(struct led_classdev *led_cdev,
> /* never on - just set to off */
> if (!delay_on) {
> led_set_brightness_async(led_cdev, LED_OFF);
> - return;
> + return 0;
> }
>
> /* never off - just set to brightness */
> if (!delay_off) {
> led_set_brightness_async(led_cdev, led_cdev->blink_brightness);
> - return;
> + return 0;
> }
>
> mod_timer(&led_cdev->blink_timer, jiffies + 1);
> + return 0;
> }
>
>
> -static void led_blink_setup(struct led_classdev *led_cdev,
> +static int led_blink_setup(struct led_classdev *led_cdev,
> unsigned long *delay_on,
> unsigned long *delay_off)
> {
> if (!(led_cdev->flags & LED_BLINK_ONESHOT) &&
> led_cdev->blink_set &&
> !led_cdev->blink_set(led_cdev, delay_on, delay_off))
> - return;
> + return 0;
>
> /* blink with 1 Hz as default if nothing specified */
> if (!*delay_on && !*delay_off)
> *delay_on = *delay_off = 500;
>
> - led_set_software_blink(led_cdev, *delay_on, *delay_off);
> + return led_set_software_blink(led_cdev, *delay_on, *delay_off);
> }
>
> -void led_blink_set(struct led_classdev *led_cdev,
> +int led_blink_set(struct led_classdev *led_cdev,
> unsigned long *delay_on,
> unsigned long *delay_off)
> {
> @@ -81,18 +89,18 @@ void led_blink_set(struct led_classdev *led_cdev,
> led_cdev->flags &= ~LED_BLINK_ONESHOT;
> led_cdev->flags &= ~LED_BLINK_ONESHOT_STOP;
>
> - led_blink_setup(led_cdev, delay_on, delay_off);
> + return led_blink_setup(led_cdev, delay_on, delay_off);
> }
> EXPORT_SYMBOL(led_blink_set);
>
> -void led_blink_set_oneshot(struct led_classdev *led_cdev,
> +int 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;
> + return -EBUSY;
>
> led_cdev->flags |= LED_BLINK_ONESHOT;
> led_cdev->flags &= ~LED_BLINK_ONESHOT_STOP;
> @@ -102,7 +110,7 @@ void led_blink_set_oneshot(struct led_classdev *led_cdev,
> else
> led_cdev->flags &= ~LED_BLINK_INVERT;
>
> - led_blink_setup(led_cdev, delay_on, delay_off);
> + return led_blink_setup(led_cdev, delay_on, delay_off);
> }
> EXPORT_SYMBOL(led_blink_set_oneshot);
>
> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
> index e8b1120..63305b1 100644
> --- a/drivers/leds/led-triggers.c
> +++ b/drivers/leds/led-triggers.c
> @@ -272,6 +272,7 @@ static void led_trigger_blink_setup(struct led_trigger *trig,
> int oneshot,
> int invert)
> {
> + int ret = 0;
> struct led_classdev *led_cdev;
>
> if (!trig)
> @@ -280,12 +281,13 @@ static void led_trigger_blink_setup(struct led_trigger *trig,
> read_lock(&trig->leddev_list_lock);
> list_for_each_entry(led_cdev, &trig->led_cdevs, trig_list) {
> if (oneshot)
> - led_blink_set_oneshot(led_cdev, delay_on, delay_off,
> - invert);
> + ret = led_blink_set_oneshot(led_cdev, delay_on,
> + delay_off, invert);
> else
> - led_blink_set(led_cdev, delay_on, delay_off);
> + ret = led_blink_set(led_cdev, delay_on, delay_off);
> }
> read_unlock(&trig->leddev_list_lock);
> + WARN_ON(ret);
> }
>
> void led_trigger_blink(struct led_trigger *trig,
> diff --git a/drivers/leds/trigger/ledtrig-oneshot.c b/drivers/leds/trigger/ledtrig-oneshot.c
> index fbd02cd..ba91838 100644
> --- a/drivers/leds/trigger/ledtrig-oneshot.c
> +++ b/drivers/leds/trigger/ledtrig-oneshot.c
> @@ -29,12 +29,15 @@ struct oneshot_trig_data {
> static ssize_t led_shot(struct device *dev,
> struct device_attribute *attr, const char *buf, size_t size)
> {
> + ssize_t ret;
> struct led_classdev *led_cdev = dev_get_drvdata(dev);
> struct oneshot_trig_data *oneshot_data = led_cdev->trigger_data;
>
> - led_blink_set_oneshot(led_cdev,
> + ret = led_blink_set_oneshot(led_cdev,
> &led_cdev->blink_delay_on, &led_cdev->blink_delay_off,
> oneshot_data->invert);
> + if (ret)
> + return ret;
>
> /* content is ignored */
> return size;
> diff --git a/drivers/leds/trigger/ledtrig-timer.c b/drivers/leds/trigger/ledtrig-timer.c
> index 8d09327..edc0c7f 100644
> --- a/drivers/leds/trigger/ledtrig-timer.c
> +++ b/drivers/leds/trigger/ledtrig-timer.c
> @@ -31,13 +31,15 @@ static ssize_t led_delay_on_store(struct device *dev,
> {
> struct led_classdev *led_cdev = dev_get_drvdata(dev);
> unsigned long state;
> - ssize_t ret = -EINVAL;
> + ssize_t ret;
>
> ret = kstrtoul(buf, 10, &state);
> if (ret)
> return ret;
>
> - led_blink_set(led_cdev, &state, &led_cdev->blink_delay_off);
> + ret = led_blink_set(led_cdev, &state, &led_cdev->blink_delay_off);
> + if (ret)
> + return ret;
> led_cdev->blink_delay_on = state;
>
> return size;
> @@ -56,13 +58,15 @@ static ssize_t led_delay_off_store(struct device *dev,
> {
> struct led_classdev *led_cdev = dev_get_drvdata(dev);
> unsigned long state;
> - ssize_t ret = -EINVAL;
> + ssize_t ret;
>
> ret = kstrtoul(buf, 10, &state);
> if (ret)
> return ret;
>
> - led_blink_set(led_cdev, &led_cdev->blink_delay_on, &state);
> + ret = led_blink_set(led_cdev, &led_cdev->blink_delay_on, &state);
> + if (ret)
> + return ret;
> led_cdev->blink_delay_off = state;
>
> return size;
> @@ -84,12 +88,16 @@ static void timer_trig_activate(struct led_classdev *led_cdev)
> if (rc)
> goto err_out_delayon;
>
> - led_blink_set(led_cdev, &led_cdev->blink_delay_on,
> + rc = led_blink_set(led_cdev, &led_cdev->blink_delay_on,
> &led_cdev->blink_delay_off);
> + if (rc)
> + goto err_out_delayoff;
> led_cdev->activated = true;
>
> return;
>
> +err_out_delayoff:
> + device_remove_file(led_cdev->dev, &dev_attr_delay_off);
> err_out_delayon:
> device_remove_file(led_cdev->dev, &dev_attr_delay_on);
> }
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index b122eea..d3c6c2e 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -48,6 +48,10 @@ struct led_classdev {
> #define SET_BRIGHTNESS_ASYNC (1 << 21)
> #define SET_BRIGHTNESS_SYNC (1 << 22)
> #define LED_DEV_CAP_FLASH (1 << 23)
> +#define LED_BRIGHTNESS_FAST (1 << 24)
> +
> +/* safe period value for slow leds is 10mS */
> +#define LED_SLOW_MIN_PERIOD 10
>
> /* Set LED brightness level */
> /* Must not sleep, use a workqueue if needed */
> @@ -127,8 +131,10 @@ extern void led_classdev_resume(struct led_classdev *led_cdev);
> * Note that if software blinking is active, simply calling
> * led_cdev->brightness_set() will not stop the blinking,
> * use led_classdev_brightness_set() instead.
> + *
> + * Returns: 0 on success or negative error value on failure
> */
> -extern void led_blink_set(struct led_classdev *led_cdev,
> +extern int led_blink_set(struct led_classdev *led_cdev,
> unsigned long *delay_on,
> unsigned long *delay_off);
> /**
> @@ -144,8 +150,10 @@ extern void led_blink_set(struct led_classdev *led_cdev,
> *
> * If invert is set, led blinks for delay_off first, then for
> * delay_on and leave the led on after the on-off cycle.
> + *
> + * Returns: 0 on success or negative error value on failure
> */
> -extern void led_blink_set_oneshot(struct led_classdev *led_cdev,
> +extern int led_blink_set_oneshot(struct led_classdev *led_cdev,
> unsigned long *delay_on,
> unsigned long *delay_off,
> int invert);
>
--
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