2015-05-20 15:17:35

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 0/20] leds: put hard limit on minimum blink period for slow leds


Currently the timer trigger allows to set blink period as small as
1mS. But in fact the minimum period is jiffy, which is usually 10mS.
The following mail says:
http://lkml.iu.edu/hypermail/linux/kernel/1504.3/03469.html
<<
As I mentioned before even with hr timer it wouldn't be possible
to assure that 1ms is achievable due to locking and bus latency
reasons.
>>
So the limit of 10mS should be enforced for drivers that use
waiting operations, like transfers across slow bus.
This allows to stay on safe side when CONFIG_HZ is set to eg 1000.

This patch set differentiates the "fast" drivers by marking them
with the new LED_BRIGHTNESS_FAST flag. For these drivers, setting
periods as small as 1mS is possible (provided CONFIG_HZ is set
appropriately), while for "slow" drivers the limit of 10mS in enforced.
In the future, the "fast" drivers may get used by a software PWM.


drivers/leds/dell-led.c | 2 +-
drivers/leds/led-core.c | 30 +++++++++++++++++++-----------
drivers/leds/led-triggers.c | 8 +++++---
drivers/leds/leds-asic3.c | 2 +-
drivers/leds/leds-bcm6328.c | 1 +
drivers/leds/leds-clevo-mail.c | 2 +-
drivers/leds/leds-cobalt-qube.c | 1 +
drivers/leds/leds-cobalt-raq.c | 2 ++
drivers/leds/leds-fsg.c | 12 ++++++------
drivers/leds/leds-gpio.c | 2 ++
drivers/leds/leds-hp6xx.c | 4 ++--
drivers/leds/leds-locomo.c | 2 ++
drivers/leds/leds-net48xx.c | 2 +-
drivers/leds/leds-netxbig.c | 2 +-
drivers/leds/leds-ns2.c | 2 +-
drivers/leds/leds-ot200.c | 1 +
drivers/leds/leds-pwm.c | 2 ++
drivers/leds/leds-s3c24xx.c | 2 +-
drivers/leds/leds-ss4200.c | 1 +
drivers/leds/leds-versatile.c | 1 +
drivers/leds/leds-wrap.c | 6 +++---
drivers/leds/trigger/ledtrig-oneshot.c | 5 ++++-
drivers/leds/trigger/ledtrig-timer.c | 18 +++++++++++++-----
include/linux/leds.h | 12 ++++++++++--
24 files changed, 82 insertions(+), 40 deletions(-)


2015-05-20 15:26:26

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 01/20] leds: implement LED_BRIGHTNESS_FAST flag


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 <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: Kyungmin Park <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
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);
--
1.7.9.5

2015-05-20 15:20:50

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 02/20] leds: mark dell-led driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/dell-led.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/dell-led.c b/drivers/leds/dell-led.c
index c36acaf..159231b 100644
--- a/drivers/leds/dell-led.c
+++ b/drivers/leds/dell-led.c
@@ -320,7 +320,7 @@ static struct led_classdev dell_led = {
.max_brightness = 1,
.brightness_set = dell_led_set,
.blink_set = dell_led_blink,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static int __init dell_led_init(void)
--
1.7.9.5

2015-05-20 15:21:54

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 03/20] leds: mark asic3 driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-asic3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-asic3.c b/drivers/leds/leds-asic3.c
index 1b71eac..427befa 100644
--- a/drivers/leds/leds-asic3.c
+++ b/drivers/leds/leds-asic3.c
@@ -108,7 +108,7 @@ static int asic3_led_probe(struct platform_device *pdev)
}

led->cdev->name = led->name;
- led->cdev->flags = LED_CORE_SUSPENDRESUME;
+ led->cdev->flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST;
led->cdev->brightness_set = brightness_set;
led->cdev->blink_set = blink_set;
led->cdev->default_trigger = led->default_trigger;
--
1.7.9.5

2015-05-20 15:22:52

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 04/20] leds: mark bcm6328 driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-bcm6328.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
index 986fe1e..5c5f5b4 100644
--- a/drivers/leds/leds-bcm6328.c
+++ b/drivers/leds/leds-bcm6328.c
@@ -313,6 +313,7 @@ static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,

led->cdev.brightness_set = bcm6328_led_set;
led->cdev.blink_set = bcm6328_blink_set;
+ led->cdev.flags = LED_BRIGHTNESS_FAST;

rc = led_classdev_register(dev, &led->cdev);
if (rc < 0)
--
1.7.9.5

2015-05-20 15:24:05

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 05/20] leds: mark clevo-mail driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-clevo-mail.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-clevo-mail.c b/drivers/leds/leds-clevo-mail.c
index 0f9ed1e..958e52c 100644
--- a/drivers/leds/leds-clevo-mail.c
+++ b/drivers/leds/leds-clevo-mail.c
@@ -150,7 +150,7 @@ static struct led_classdev clevo_mail_led = {
.name = "clevo::mail",
.brightness_set = clevo_mail_led_set,
.blink_set = clevo_mail_led_blink,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static int __init clevo_mail_led_probe(struct platform_device *pdev)
--
1.7.9.5

2015-05-20 15:24:53

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 06/20] leds: mark cobalt-qube driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-cobalt-qube.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/leds/leds-cobalt-qube.c b/drivers/leds/leds-cobalt-qube.c
index d975220..f16da2a 100644
--- a/drivers/leds/leds-cobalt-qube.c
+++ b/drivers/leds/leds-cobalt-qube.c
@@ -31,6 +31,7 @@ static struct led_classdev qube_front_led = {
.brightness = LED_FULL,
.brightness_set = qube_front_led_set,
.default_trigger = "default-on",
+ .flags = LED_BRIGHTNESS_FAST,
};

static int cobalt_qube_led_probe(struct platform_device *pdev)
--
1.7.9.5

2015-05-20 15:25:50

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 07/20] leds: mark cobalt-raq driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-cobalt-raq.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/leds/leds-cobalt-raq.c b/drivers/leds/leds-cobalt-raq.c
index 06dbe18..d0579ef 100644
--- a/drivers/leds/leds-cobalt-raq.c
+++ b/drivers/leds/leds-cobalt-raq.c
@@ -52,6 +52,7 @@ static void raq_web_led_set(struct led_classdev *led_cdev,
static struct led_classdev raq_web_led = {
.name = "raq::web",
.brightness_set = raq_web_led_set,
+ .flags = LED_BRIGHTNESS_FAST,
};

static void raq_power_off_led_set(struct led_classdev *led_cdev,
@@ -74,6 +75,7 @@ static struct led_classdev raq_power_off_led = {
.name = "raq::power-off",
.brightness_set = raq_power_off_led_set,
.default_trigger = "power-off",
+ .flags = LED_BRIGHTNESS_FAST,
};

static int cobalt_raq_led_probe(struct platform_device *pdev)
--
1.7.9.5

2015-05-20 15:26:35

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 08/20] leds: mark fsg driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-fsg.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/leds/leds-fsg.c b/drivers/leds/leds-fsg.c
index 2b4dc73..8b33269 100644
--- a/drivers/leds/leds-fsg.c
+++ b/drivers/leds/leds-fsg.c
@@ -109,37 +109,37 @@ static void fsg_led_ring_set(struct led_classdev *led_cdev,
static struct led_classdev fsg_wlan_led = {
.name = "fsg:blue:wlan",
.brightness_set = fsg_led_wlan_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static struct led_classdev fsg_wan_led = {
.name = "fsg:blue:wan",
.brightness_set = fsg_led_wan_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static struct led_classdev fsg_sata_led = {
.name = "fsg:blue:sata",
.brightness_set = fsg_led_sata_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static struct led_classdev fsg_usb_led = {
.name = "fsg:blue:usb",
.brightness_set = fsg_led_usb_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static struct led_classdev fsg_sync_led = {
.name = "fsg:blue:sync",
.brightness_set = fsg_led_sync_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static struct led_classdev fsg_ring_led = {
.name = "fsg:blue:ring",
.brightness_set = fsg_led_ring_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};


--
1.7.9.5

2015-05-20 15:27:44

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 09/20] leds: mark gpio driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method if can_sleep variable is not set. In that case it can be
marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-gpio.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 3af4f2b..0e1346c 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -138,6 +138,8 @@ static int create_gpio_led(const struct gpio_led *template,
led_dat->cdev.brightness = state ? LED_FULL : LED_OFF;
if (!template->retain_state_suspended)
led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
+ if (!led_dat->can_sleep)
+ led_dat->cdev.flags |= LED_BRIGHTNESS_FAST;

ret = gpiod_direction_output(led_dat->gpiod, state);
if (ret < 0)
--
1.7.9.5

2015-05-20 15:28:41

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 10/20] leds: mark hp6xx driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-hp6xx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-hp6xx.c b/drivers/leds/leds-hp6xx.c
index 0b84c01..8f369fb 100644
--- a/drivers/leds/leds-hp6xx.c
+++ b/drivers/leds/leds-hp6xx.c
@@ -45,14 +45,14 @@ static struct led_classdev hp6xx_red_led = {
.name = "hp6xx:red",
.default_trigger = "hp6xx-charge",
.brightness_set = hp6xxled_red_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static struct led_classdev hp6xx_green_led = {
.name = "hp6xx:green",
.default_trigger = "ide-disk",
.brightness_set = hp6xxled_green_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static int hp6xxled_probe(struct platform_device *pdev)
--
1.7.9.5

2015-05-20 15:31:11

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 11/20] leds: mark locomo driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-locomo.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/leds/leds-locomo.c b/drivers/leds/leds-locomo.c
index 80ba048..0eb69c3 100644
--- a/drivers/leds/leds-locomo.c
+++ b/drivers/leds/leds-locomo.c
@@ -47,12 +47,14 @@ static struct led_classdev locomo_led0 = {
.name = "locomo:amber:charge",
.default_trigger = "main-battery-charging",
.brightness_set = locomoled_brightness_set0,
+ .flags = LED_BRIGHTNESS_FAST,
};

static struct led_classdev locomo_led1 = {
.name = "locomo:green:mail",
.default_trigger = "nand-disk",
.brightness_set = locomoled_brightness_set1,
+ .flags = LED_BRIGHTNESS_FAST,
};

static int locomoled_probe(struct locomo_dev *ldev)
--
1.7.9.5

2015-05-20 15:30:45

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 12/20] leds: mark net48xx driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Chris Boot <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-net48xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-net48xx.c b/drivers/leds/leds-net48xx.c
index ec3a2e8..8644341 100644
--- a/drivers/leds/leds-net48xx.c
+++ b/drivers/leds/leds-net48xx.c
@@ -34,7 +34,7 @@ static void net48xx_error_led_set(struct led_classdev *led_cdev,
static struct led_classdev net48xx_error_led = {
.name = "net48xx::error",
.brightness_set = net48xx_error_led_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static int net48xx_led_probe(struct platform_device *pdev)
--
1.7.9.5

2015-05-20 15:35:57

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 13/20] leds: mark netxbig driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-netxbig.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-netxbig.c b/drivers/leds/leds-netxbig.c
index 25e4197..e99b233 100644
--- a/drivers/leds/leds-netxbig.c
+++ b/drivers/leds/leds-netxbig.c
@@ -329,7 +329,7 @@ create_netxbig_led(struct platform_device *pdev,
*/
led_dat->sata = 0;
led_dat->cdev.brightness = LED_OFF;
- led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
+ led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST;
led_dat->mode_addr = template->mode_addr;
led_dat->mode_val = template->mode_val;
led_dat->bright_addr = template->bright_addr;
--
1.7.9.5

2015-05-20 15:36:33

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 14/20] leds: mark ns2 driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-ns2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-ns2.c b/drivers/leds/leds-ns2.c
index 1fd6adb..9d872f5 100644
--- a/drivers/leds/leds-ns2.c
+++ b/drivers/leds/leds-ns2.c
@@ -224,7 +224,7 @@ create_ns2_led(struct platform_device *pdev, struct ns2_led_data *led_dat,
led_dat->cdev.default_trigger = template->default_trigger;
led_dat->cdev.blink_set = NULL;
led_dat->cdev.brightness_set = ns2_led_set;
- led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
+ led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST;
led_dat->cdev.groups = ns2_led_groups;
led_dat->cmd = template->cmd;
led_dat->slow = template->slow;
--
1.7.9.5

2015-05-20 15:33:38

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 15/20] leds: mark ot200 driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-ot200.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/leds/leds-ot200.c b/drivers/leds/leds-ot200.c
index 39870de..d357c14 100644
--- a/drivers/leds/leds-ot200.c
+++ b/drivers/leds/leds-ot200.c
@@ -123,6 +123,7 @@ static int ot200_led_probe(struct platform_device *pdev)

leds[i].cdev.name = leds[i].name;
leds[i].cdev.brightness_set = ot200_led_brightness_set;
+ leds[i].cdev.flags |= LED_BRIGHTNESS_FAST;

ret = led_classdev_register(&pdev->dev, &leds[i].cdev);
if (ret < 0)
--
1.7.9.5

2015-05-20 15:34:23

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 16/20] leds: mark pwm driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method if can_sleep variable is not set. In that case it can be
marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-pwm.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 1d07e3e..82d01e5 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -124,6 +124,8 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
led_data->can_sleep = pwm_can_sleep(led_data->pwm);
if (led_data->can_sleep)
INIT_WORK(&led_data->work, led_pwm_work);
+ else
+ led_data->cdev.flags |= LED_BRIGHTNESS_FAST;

led_data->period = pwm_get_period(led_data->pwm);
if (!led_data->period && (led->pwm_period_ns > 0))
--
1.7.9.5

2015-05-20 15:35:11

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 17/20] leds: mark s3c24xx driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-s3c24xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-s3c24xx.c b/drivers/leds/leds-s3c24xx.c
index 83641a7..ed61bf9 100644
--- a/drivers/leds/leds-s3c24xx.c
+++ b/drivers/leds/leds-s3c24xx.c
@@ -84,7 +84,7 @@ static int s3c24xx_led_probe(struct platform_device *dev)
led->cdev.brightness_set = s3c24xx_led_set;
led->cdev.default_trigger = pdata->def_trigger;
led->cdev.name = pdata->name;
- led->cdev.flags |= LED_CORE_SUSPENDRESUME;
+ led->cdev.flags |= LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST;

led->pdata = pdata;

--
1.7.9.5

2015-05-20 15:36:07

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 18/20] leds: mark ss4200 driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-ss4200.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/leds/leds-ss4200.c b/drivers/leds/leds-ss4200.c
index 046cb70..99e8348 100644
--- a/drivers/leds/leds-ss4200.c
+++ b/drivers/leds/leds-ss4200.c
@@ -487,6 +487,7 @@ static int register_nasgpio_led(int led_nr)
led->brightness = LED_FULL;
led->brightness_set = nasgpio_led_set_brightness;
led->blink_set = nasgpio_led_set_blink;
+ led->flags |= LED_BRIGHTNESS_FAST;
led->groups = nasgpio_led_groups;
ret = led_classdev_register(&nas_gpio_pci_dev->dev, led);
if (ret)
--
1.7.9.5

2015-05-20 15:37:05

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 19/20] leds: mark versatile driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-versatile.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/leds/leds-versatile.c b/drivers/leds/leds-versatile.c
index 8055302..4061257 100644
--- a/drivers/leds/leds-versatile.c
+++ b/drivers/leds/leds-versatile.c
@@ -84,6 +84,7 @@ static int versatile_leds_probe(struct platform_device *dev)
led->cdev.name = versatile_leds[i].name;
led->cdev.brightness_set = versatile_led_set;
led->cdev.brightness_get = versatile_led_get;
+ led->cdev.flags |= LED_BRIGHTNESS_FAST;
led->cdev.default_trigger = versatile_leds[i].trigger;
led->mask = BIT(i);

--
1.7.9.5

2015-05-20 15:38:01

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 20/20] leds: mark wrap driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-wrap.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/leds-wrap.c b/drivers/leds/leds-wrap.c
index 1ba3def..dfc9e73 100644
--- a/drivers/leds/leds-wrap.c
+++ b/drivers/leds/leds-wrap.c
@@ -57,19 +57,19 @@ static struct led_classdev wrap_power_led = {
.name = "wrap::power",
.brightness_set = wrap_power_led_set,
.default_trigger = "default-on",
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static struct led_classdev wrap_error_led = {
.name = "wrap::error",
.brightness_set = wrap_error_led_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static struct led_classdev wrap_extra_led = {
.name = "wrap::extra",
.brightness_set = wrap_extra_led_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static int wrap_led_probe(struct platform_device *pdev)
--
1.7.9.5

2015-05-21 13:23:41

by Jacek Anaszewski

[permalink] [raw]
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 <[email protected]>
> CC: Richard Purdie <[email protected]>
> CC: Jacek Anaszewski <[email protected]>
> CC: Kyungmin Park <[email protected]>
> CC: [email protected]
> CC: [email protected]
>
> Signed-off-by: Stas Sergeev <[email protected]>
> ---
> 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

2015-05-21 13:27:51

by Stas Sergeev

[permalink] [raw]
Subject: Re: [PATCH 01/20] leds: implement LED_BRIGHTNESS_FAST flag

21.05.2015 16:22, Jacek Anaszewski пишет:
> Hi Stas,
>
> My intention was to add a developer of each driver on
> cc in the commit message.
I did. You can see CC: Chris Boot <[email protected]> at patch 12.
Otherwise, this is all the get_maintainers.pl script gives me.
So unless you can suggest some other script, or get_maintainers.pl
is fixed, what do you expect me to do?

Anyway. I can reduce the patchset to just the initial patch and
the leds-gpio one. In a hope others will change more drivers later.
At least that would be trivial to review and apply.
Deal?

2015-05-21 13:37:51

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH 01/20] leds: implement LED_BRIGHTNESS_FAST flag

On 05/21/2015 03:27 PM, Stas Sergeev wrote:
> 21.05.2015 16:22, Jacek Anaszewski пишет:
>> Hi Stas,
>>
>> My intention was to add a developer of each driver on
>> cc in the commit message.
> I did. You can see CC: Chris Boot <[email protected]> at patch 12.
> Otherwise, this is all the get_maintainers.pl script gives me.
> So unless you can suggest some other script, or get_maintainers.pl
> is fixed, what do you expect me to do?

You don't have to rely on get_maintainers.pl script only, but
you can obtain the authors' names from driver headers or
MODULE_AUTHOR entries at the bottom.

> Anyway. I can reduce the patchset to just the initial patch and
> the leds-gpio one. In a hope others will change more drivers later.
> At least that would be trivial to review and apply.
> Deal?

Please don't reduce it. The greater number of people familiar with
related drivers/devices will get this, the greater chance that we
won't miss something crucial.

--
Best Regards,
Jacek Anaszewski

2015-05-21 15:12:06

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 02/20] leds: mark dell-led driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Louis Davis <[email protected]>
CC: Jim Dailey <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/dell-led.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/dell-led.c b/drivers/leds/dell-led.c
index c36acaf..159231b 100644
--- a/drivers/leds/dell-led.c
+++ b/drivers/leds/dell-led.c
@@ -320,7 +320,7 @@ static struct led_classdev dell_led = {
.max_brightness = 1,
.brightness_set = dell_led_set,
.blink_set = dell_led_blink,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static int __init dell_led_init(void)
--
1.7.9.5

2015-05-21 15:12:55

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 03/20] leds: mark asic3 driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Paul Parsons <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-asic3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-asic3.c b/drivers/leds/leds-asic3.c
index 1b71eac..427befa 100644
--- a/drivers/leds/leds-asic3.c
+++ b/drivers/leds/leds-asic3.c
@@ -108,7 +108,7 @@ static int asic3_led_probe(struct platform_device *pdev)
}

led->cdev->name = led->name;
- led->cdev->flags = LED_CORE_SUSPENDRESUME;
+ led->cdev->flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST;
led->cdev->brightness_set = brightness_set;
led->cdev->blink_set = blink_set;
led->cdev->default_trigger = led->default_trigger;
--
1.7.9.5

2015-05-21 15:13:50

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 04/20] leds: mark bcm6328 driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Álvaro Fernández Rojas <[email protected]>
CC: Jonas Gorski <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-bcm6328.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
index 986fe1e..5c5f5b4 100644
--- a/drivers/leds/leds-bcm6328.c
+++ b/drivers/leds/leds-bcm6328.c
@@ -313,6 +313,7 @@ static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,

led->cdev.brightness_set = bcm6328_led_set;
led->cdev.blink_set = bcm6328_blink_set;
+ led->cdev.flags = LED_BRIGHTNESS_FAST;

rc = led_classdev_register(dev, &led->cdev);
if (rc < 0)
--
1.7.9.5

2015-05-21 15:14:30

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 05/20] leds: mark clevo-mail driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Márton Németh <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-clevo-mail.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-clevo-mail.c b/drivers/leds/leds-clevo-mail.c
index 0f9ed1e..958e52c 100644
--- a/drivers/leds/leds-clevo-mail.c
+++ b/drivers/leds/leds-clevo-mail.c
@@ -150,7 +150,7 @@ static struct led_classdev clevo_mail_led = {
.name = "clevo::mail",
.brightness_set = clevo_mail_led_set,
.blink_set = clevo_mail_led_blink,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static int __init clevo_mail_led_probe(struct platform_device *pdev)
--
1.7.9.5

2015-05-21 15:15:15

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 06/20] leds: mark cobalt-qube driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Florian Fainelli <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-cobalt-qube.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/leds/leds-cobalt-qube.c b/drivers/leds/leds-cobalt-qube.c
index d975220..f16da2a 100644
--- a/drivers/leds/leds-cobalt-qube.c
+++ b/drivers/leds/leds-cobalt-qube.c
@@ -31,6 +31,7 @@ static struct led_classdev qube_front_led = {
.brightness = LED_FULL,
.brightness_set = qube_front_led_set,
.default_trigger = "default-on",
+ .flags = LED_BRIGHTNESS_FAST,
};

static int cobalt_qube_led_probe(struct platform_device *pdev)
--
1.7.9.5

2015-05-21 15:20:30

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 07/20] leds: mark cobalt-raq driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Yoichi Yuasa <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-cobalt-raq.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/leds/leds-cobalt-raq.c b/drivers/leds/leds-cobalt-raq.c
index 06dbe18..d0579ef 100644
--- a/drivers/leds/leds-cobalt-raq.c
+++ b/drivers/leds/leds-cobalt-raq.c
@@ -52,6 +52,7 @@ static void raq_web_led_set(struct led_classdev *led_cdev,
static struct led_classdev raq_web_led = {
.name = "raq::web",
.brightness_set = raq_web_led_set,
+ .flags = LED_BRIGHTNESS_FAST,
};

static void raq_power_off_led_set(struct led_classdev *led_cdev,
@@ -74,6 +75,7 @@ static struct led_classdev raq_power_off_led = {
.name = "raq::power-off",
.brightness_set = raq_power_off_led_set,
.default_trigger = "power-off",
+ .flags = LED_BRIGHTNESS_FAST,
};

static int cobalt_raq_led_probe(struct platform_device *pdev)
--
1.7.9.5

2015-05-21 15:16:25

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 08/20] leds: mark fsg driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Rod Whitby <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-fsg.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/leds/leds-fsg.c b/drivers/leds/leds-fsg.c
index 2b4dc73..8b33269 100644
--- a/drivers/leds/leds-fsg.c
+++ b/drivers/leds/leds-fsg.c
@@ -109,37 +109,37 @@ static void fsg_led_ring_set(struct led_classdev *led_cdev,
static struct led_classdev fsg_wlan_led = {
.name = "fsg:blue:wlan",
.brightness_set = fsg_led_wlan_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static struct led_classdev fsg_wan_led = {
.name = "fsg:blue:wan",
.brightness_set = fsg_led_wan_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static struct led_classdev fsg_sata_led = {
.name = "fsg:blue:sata",
.brightness_set = fsg_led_sata_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static struct led_classdev fsg_usb_led = {
.name = "fsg:blue:usb",
.brightness_set = fsg_led_usb_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static struct led_classdev fsg_sync_led = {
.name = "fsg:blue:sync",
.brightness_set = fsg_led_sync_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static struct led_classdev fsg_ring_led = {
.name = "fsg:blue:ring",
.brightness_set = fsg_led_ring_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};


--
1.7.9.5

2015-05-21 15:17:06

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 09/20] leds: mark gpio driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method if can_sleep variable is not set. In that case it can be
marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Raphael Assenat <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-gpio.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 3af4f2b..0e1346c 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -138,6 +138,8 @@ static int create_gpio_led(const struct gpio_led *template,
led_dat->cdev.brightness = state ? LED_FULL : LED_OFF;
if (!template->retain_state_suspended)
led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
+ if (!led_dat->can_sleep)
+ led_dat->cdev.flags |= LED_BRIGHTNESS_FAST;

ret = gpiod_direction_output(led_dat->gpiod, state);
if (ret < 0)
--
1.7.9.5

2015-05-21 15:17:49

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 10/20] leds: mark hp6xx driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Kristoffer Ericson <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-hp6xx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-hp6xx.c b/drivers/leds/leds-hp6xx.c
index 0b84c01..8f369fb 100644
--- a/drivers/leds/leds-hp6xx.c
+++ b/drivers/leds/leds-hp6xx.c
@@ -45,14 +45,14 @@ static struct led_classdev hp6xx_red_led = {
.name = "hp6xx:red",
.default_trigger = "hp6xx-charge",
.brightness_set = hp6xxled_red_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static struct led_classdev hp6xx_green_led = {
.name = "hp6xx:green",
.default_trigger = "ide-disk",
.brightness_set = hp6xxled_green_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static int hp6xxled_probe(struct platform_device *pdev)
--
1.7.9.5

2015-05-21 15:18:38

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 11/20] leds: mark locomo driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: John Lenz <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-locomo.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/leds/leds-locomo.c b/drivers/leds/leds-locomo.c
index 80ba048..0eb69c3 100644
--- a/drivers/leds/leds-locomo.c
+++ b/drivers/leds/leds-locomo.c
@@ -47,12 +47,14 @@ static struct led_classdev locomo_led0 = {
.name = "locomo:amber:charge",
.default_trigger = "main-battery-charging",
.brightness_set = locomoled_brightness_set0,
+ .flags = LED_BRIGHTNESS_FAST,
};

static struct led_classdev locomo_led1 = {
.name = "locomo:green:mail",
.default_trigger = "nand-disk",
.brightness_set = locomoled_brightness_set1,
+ .flags = LED_BRIGHTNESS_FAST,
};

static int locomoled_probe(struct locomo_dev *ldev)
--
1.7.9.5

2015-05-21 15:19:37

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 13/20] leds: mark netxbig driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Simon Guinot <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-netxbig.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-netxbig.c b/drivers/leds/leds-netxbig.c
index 25e4197..e99b233 100644
--- a/drivers/leds/leds-netxbig.c
+++ b/drivers/leds/leds-netxbig.c
@@ -329,7 +329,7 @@ create_netxbig_led(struct platform_device *pdev,
*/
led_dat->sata = 0;
led_dat->cdev.brightness = LED_OFF;
- led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
+ led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST;
led_dat->mode_addr = template->mode_addr;
led_dat->mode_val = template->mode_val;
led_dat->bright_addr = template->bright_addr;
--
1.7.9.5

2015-05-21 15:20:23

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 14/20] leds: mark ns2 driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Simon Guinot <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-ns2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-ns2.c b/drivers/leds/leds-ns2.c
index 1fd6adb..9d872f5 100644
--- a/drivers/leds/leds-ns2.c
+++ b/drivers/leds/leds-ns2.c
@@ -224,7 +224,7 @@ create_ns2_led(struct platform_device *pdev, struct ns2_led_data *led_dat,
led_dat->cdev.default_trigger = template->default_trigger;
led_dat->cdev.blink_set = NULL;
led_dat->cdev.brightness_set = ns2_led_set;
- led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
+ led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST;
led_dat->cdev.groups = ns2_led_groups;
led_dat->cmd = template->cmd;
led_dat->slow = template->slow;
--
1.7.9.5

2015-05-21 15:21:10

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 15/20] leds: mark ot200 driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Sebastian Andrzej Siewior <[email protected]>
CC: Christian Gmeiner <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-ot200.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/leds/leds-ot200.c b/drivers/leds/leds-ot200.c
index 39870de..d357c14 100644
--- a/drivers/leds/leds-ot200.c
+++ b/drivers/leds/leds-ot200.c
@@ -123,6 +123,7 @@ static int ot200_led_probe(struct platform_device *pdev)

leds[i].cdev.name = leds[i].name;
leds[i].cdev.brightness_set = ot200_led_brightness_set;
+ leds[i].cdev.flags |= LED_BRIGHTNESS_FAST;

ret = led_classdev_register(&pdev->dev, &leds[i].cdev);
if (ret < 0)
--
1.7.9.5

2015-05-21 15:21:54

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 16/20] leds: mark pwm driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method if can_sleep variable is not set. In that case it can be
marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Luotao Fu <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-pwm.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 1d07e3e..82d01e5 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -124,6 +124,8 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
led_data->can_sleep = pwm_can_sleep(led_data->pwm);
if (led_data->can_sleep)
INIT_WORK(&led_data->work, led_pwm_work);
+ else
+ led_data->cdev.flags |= LED_BRIGHTNESS_FAST;

led_data->period = pwm_get_period(led_data->pwm);
if (!led_data->period && (led->pwm_period_ns > 0))
--
1.7.9.5

2015-05-21 15:22:35

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 17/20] leds: mark s3c24xx driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Ben Dooks <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-s3c24xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-s3c24xx.c b/drivers/leds/leds-s3c24xx.c
index 83641a7..ed61bf9 100644
--- a/drivers/leds/leds-s3c24xx.c
+++ b/drivers/leds/leds-s3c24xx.c
@@ -84,7 +84,7 @@ static int s3c24xx_led_probe(struct platform_device *dev)
led->cdev.brightness_set = s3c24xx_led_set;
led->cdev.default_trigger = pdata->def_trigger;
led->cdev.name = pdata->name;
- led->cdev.flags |= LED_CORE_SUSPENDRESUME;
+ led->cdev.flags |= LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST;

led->pdata = pdata;

--
1.7.9.5

2015-05-21 15:23:13

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 18/20] leds: mark ss4200 driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Dave Hansen <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-ss4200.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/leds/leds-ss4200.c b/drivers/leds/leds-ss4200.c
index 046cb70..99e8348 100644
--- a/drivers/leds/leds-ss4200.c
+++ b/drivers/leds/leds-ss4200.c
@@ -487,6 +487,7 @@ static int register_nasgpio_led(int led_nr)
led->brightness = LED_FULL;
led->brightness_set = nasgpio_led_set_brightness;
led->blink_set = nasgpio_led_set_blink;
+ led->flags |= LED_BRIGHTNESS_FAST;
led->groups = nasgpio_led_groups;
ret = led_classdev_register(&nas_gpio_pci_dev->dev, led);
if (ret)
--
1.7.9.5

2015-05-21 15:25:26

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 19/20] leds: mark versatile driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Linus Walleij <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-versatile.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/leds/leds-versatile.c b/drivers/leds/leds-versatile.c
index 8055302..4061257 100644
--- a/drivers/leds/leds-versatile.c
+++ b/drivers/leds/leds-versatile.c
@@ -84,6 +84,7 @@ static int versatile_leds_probe(struct platform_device *dev)
led->cdev.name = versatile_leds[i].name;
led->cdev.brightness_set = versatile_led_set;
led->cdev.brightness_get = versatile_led_get;
+ led->cdev.flags |= LED_BRIGHTNESS_FAST;
led->cdev.default_trigger = versatile_leds[i].trigger;
led->mask = BIT(i);

--
1.7.9.5

2015-05-21 15:24:22

by Stas Sergeev

[permalink] [raw]
Subject: [PATCH 20/20] leds: mark wrap driver with LED_BRIGHTNESS_FAST flag


This driver doesn't use sleeping operations in .brightness_set
method, and can be marked with LED_BRIGHTNESS_FAST flag.
That flag allows changing brightness at high rates (over 100Hz).

CC: Bryan Wu <[email protected]>
CC: Richard Purdie <[email protected]>
CC: Jacek Anaszewski <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Kristian Kielhofner <[email protected]>

Signed-off-by: Stas Sergeev <[email protected]>
---
drivers/leds/leds-wrap.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/leds-wrap.c b/drivers/leds/leds-wrap.c
index 1ba3def..dfc9e73 100644
--- a/drivers/leds/leds-wrap.c
+++ b/drivers/leds/leds-wrap.c
@@ -57,19 +57,19 @@ static struct led_classdev wrap_power_led = {
.name = "wrap::power",
.brightness_set = wrap_power_led_set,
.default_trigger = "default-on",
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static struct led_classdev wrap_error_led = {
.name = "wrap::error",
.brightness_set = wrap_error_led_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static struct led_classdev wrap_extra_led = {
.name = "wrap::extra",
.brightness_set = wrap_extra_led_set,
- .flags = LED_CORE_SUSPENDRESUME,
+ .flags = LED_CORE_SUSPENDRESUME | LED_BRIGHTNESS_FAST,
};

static int wrap_led_probe(struct platform_device *pdev)
--
1.7.9.5

2015-05-21 15:26:21

by Stas Sergeev

[permalink] [raw]
Subject: Re: [PATCH 01/20] leds: implement LED_BRIGHTNESS_FAST flag

21.05.2015 16:37, Jacek Anaszewski пишет:
> On 05/21/2015 03:27 PM, Stas Sergeev wrote:
>> 21.05.2015 16:22, Jacek Anaszewski пишет:
>>> Hi Stas,
>>>
>>> My intention was to add a developer of each driver on
>>> cc in the commit message.
>> I did. You can see CC: Chris Boot <[email protected]> at patch 12.
>> Otherwise, this is all the get_maintainers.pl script gives me.
>> So unless you can suggest some other script, or get_maintainers.pl
>> is fixed, what do you expect me to do?
>
> You don't have to rely on get_maintainers.pl script only, but
> you can obtain the authors' names from driver headers or
> MODULE_AUTHOR entries at the bottom.

Re-sent with more CCs.

2015-05-22 07:14:49

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 19/20] leds: mark versatile driver with LED_BRIGHTNESS_FAST flag

On Thu, May 21, 2015 at 5:23 PM, Stas Sergeev <[email protected]> wrote:

> This driver doesn't use sleeping operations in .brightness_set
> method, and can be marked with LED_BRIGHTNESS_FAST flag.
> That flag allows changing brightness at high rates (over 100Hz).
>
> CC: Bryan Wu <[email protected]>
> CC: Richard Purdie <[email protected]>
> CC: Jacek Anaszewski <[email protected]>
> CC: [email protected]
> CC: [email protected]
> CC: Linus Walleij <[email protected]>
>
> Signed-off-by: Stas Sergeev <[email protected]>

Acked-by: Linus Walleij <[email protected]>

Interesting that you're mailing my student account from 1997.
Luckily, it still works.

Yours,
Linus Walleij

2015-05-22 10:06:32

by Stas Sergeev

[permalink] [raw]
Subject: Re: [PATCH 19/20] leds: mark versatile driver with LED_BRIGHTNESS_FAST flag

22.05.2015 10:14, Linus Walleij пишет:
> On Thu, May 21, 2015 at 5:23 PM, Stas Sergeev <[email protected]> wrote:
>
>> This driver doesn't use sleeping operations in .brightness_set
>> method, and can be marked with LED_BRIGHTNESS_FAST flag.
>> That flag allows changing brightness at high rates (over 100Hz).
>>
>> CC: Bryan Wu <[email protected]>
>> CC: Richard Purdie <[email protected]>
>> CC: Jacek Anaszewski <[email protected]>
>> CC: [email protected]
>> CC: [email protected]
>> CC: Linus Walleij <[email protected]>
>>
>> Signed-off-by: Stas Sergeev <[email protected]>
>
> Acked-by: Linus Walleij <[email protected]>
Thanks!

> Interesting that you're mailing my student account from 1997.
> Luckily, it still works.
For some reason, get_maintainer.pl script didn't give me
anything useful for most of the 20 files I modified. So
I simply took e-mails from copyright headings.
Script should be improved.

2015-05-26 13:22:19

by Christian Gmeiner

[permalink] [raw]
Subject: Re: [PATCH 15/20] leds: mark ot200 driver with LED_BRIGHTNESS_FAST flag

>
> This driver doesn't use sleeping operations in .brightness_set
> method, and can be marked with LED_BRIGHTNESS_FAST flag.
> That flag allows changing brightness at high rates (over 100Hz).
>
> CC: Bryan Wu <[email protected]>
> CC: Richard Purdie <[email protected]>
> CC: Jacek Anaszewski <[email protected]>
> CC: [email protected]
> CC: [email protected]
> CC: Sebastian Andrzej Siewior <[email protected]>
> CC: Christian Gmeiner <[email protected]>
>
> Signed-off-by: Stas Sergeev <[email protected]>
> ---
> drivers/leds/leds-ot200.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/leds/leds-ot200.c b/drivers/leds/leds-ot200.c
> index 39870de..d357c14 100644
> --- a/drivers/leds/leds-ot200.c
> +++ b/drivers/leds/leds-ot200.c
> @@ -123,6 +123,7 @@ static int ot200_led_probe(struct platform_device *pdev)
>
> leds[i].cdev.name = leds[i].name;
> leds[i].cdev.brightness_set = ot200_led_brightness_set;
> + leds[i].cdev.flags |= LED_BRIGHTNESS_FAST;
>
> ret = led_classdev_register(&pdev->dev, &leds[i].cdev);
> if (ret < 0)
> --
> 1.7.9.5

Reviewed-by: Christian Gmeiner <[email protected]>

2015-06-01 08:31:26

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH 01/20] leds: implement LED_BRIGHTNESS_FAST flag

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 <[email protected]>
> CC: Richard Purdie <[email protected]>
> CC: Jacek Anaszewski <[email protected]>
> CC: Kyungmin Park <[email protected]>
> CC: [email protected]
> CC: [email protected]
>
> Signed-off-by: Stas Sergeev <[email protected]>
> ---
> 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;
> + }
> +

With this approach, the LED will remain in its current blink state, in
case LED_BRIGHTNESS_FAST flag is not set and delay to be set is below
LED_SLOW_MIN_PERIOD. This is because timer is deleted at the beginning
of led_blink_set. Effectively this can result in the LED staying turned
on when e.g. "echo 1 > delay_on" fails.

I propose to change this part of code as follows:

if (!(led_cdev->flags & LED_BRIGHTNESS_FAST)) {
if (delay_on < LED_SLOW_MIN_PERIOD ||
delay_off < LED_SLOW_MIN_PERIOD)
led_set_brightness_async(led_cdev, LED_OFF);
return -ERANGE;
}
}

By this occasion it turned out that we have inconsistency:
led_set_brightness_async calls brightness_set op, which doesn't
set LED brightness in an asynchronous way in case of every driver.

Before introduction of SET_BRIGHTNESS_SYNC and SET_BRIGHTNESS_ASYNC
flags there was only brightness_set op. The flags and
brightness_set_sync op was added for flash LED controllers to avoid
delegating the job to work queue and assure that brightness is set
as quickly as possible.

I mistakenly assumed that all drivers set the brightness with use
of a work queue.

Now, to fix the issue all drivers that set brightness in the
asynchronous way need to set the SET_BRIGHTNESS_SYNC flag and rename
their brightness_set ops to brightness_set_sync.
It would be also nice to rename brightness_set op to
brightness_set_async.

Also, led_set_brightness_async shouldn't be called directly
anywhere, but led_set_brightness should be used, as it
selects the appropriate op basing on the SET_BRIGHTNESS_* flag.

I'd prefer to do these modifications before merging this patch
set. I can produce the patch set within a few days. Or, maybe you
would like to take care of it?

> 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

2015-06-01 11:56:31

by Stas Sergeev

[permalink] [raw]
Subject: Re: [PATCH 01/20] leds: implement LED_BRIGHTNESS_FAST flag

01.06.2015 11:31, Jacek Anaszewski пишет:
> With this approach, the LED will remain in its current blink state, in
> case LED_BRIGHTNESS_FAST flag is not set and delay to be set is below LED_SLOW_MIN_PERIOD. This is because timer is deleted at the beginning
> of led_blink_set. Effectively this can result in the LED staying turned
> on when e.g. "echo 1 > delay_on" fails.
>
> I propose to change this part of code as follows:
>
> if (!(led_cdev->flags & LED_BRIGHTNESS_FAST)) {
> if (delay_on < LED_SLOW_MIN_PERIOD ||
> delay_off < LED_SLOW_MIN_PERIOD)
> led_set_brightness_async(led_cdev, LED_OFF);
> return -ERANGE;
> }
> }
Sounds good.

> By this occasion it turned out that we have inconsistency:
> led_set_brightness_async calls brightness_set op, which doesn't
> set LED brightness in an asynchronous way in case of every driver.
>
> Before introduction of SET_BRIGHTNESS_SYNC and SET_BRIGHTNESS_ASYNC
> flags there was only brightness_set op. The flags and
> brightness_set_sync op was added for flash LED controllers to avoid
> delegating the job to work queue and assure that brightness is set
> as quickly as possible.
>
> I mistakenly assumed that all drivers set the brightness with use
> of a work queue.
Indeed.

> Now, to fix the issue all drivers that set brightness in the
> asynchronous way need to set the SET_BRIGHTNESS_SYNC flag and rename
> their brightness_set ops to brightness_set_sync.
> It would be also nice to rename brightness_set op to
> brightness_set_async.
>
> Also, led_set_brightness_async shouldn't be called directly
> anywhere, but led_set_brightness should be used, as it
> selects the appropriate op basing on the SET_BRIGHTNESS_* flag.
>
> I'd prefer to do these modifications before merging this patch
Sounds good: I wonder if the new flag for FAST drivers will then
be needed at all: maybe it would be enough for the driver to define
a SYNC method, and we assume the SYNC methods are always fast?
In fact, the things are more complicated: some drivers do small
udelay()'s but do not use a work-queue. I was not marking them as
FAST, although perhaps they could still be marked as SYNC?

> set. I can produce the patch set within a few days. Or, maybe you
> would like to take care of it?
I would appreciate if you do.
I very much hate to do any non-trivial changes to the code I can't
even properly test (sometimes not even compile-test!).

Since you are at it, I'd also suggest to kill the work-queue in
led-core.c. Now that we fixed a scenario where it was mistakenly
used, I again think it is not needed for what it was initially advertised.

2015-06-01 14:19:35

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH 01/20] leds: implement LED_BRIGHTNESS_FAST flag

On 06/01/2015 01:56 PM, Stas Sergeev wrote:
> 01.06.2015 11:31, Jacek Anaszewski пишет:
>> With this approach, the LED will remain in its current blink state, in
>> case LED_BRIGHTNESS_FAST flag is not set and delay to be set is below LED_SLOW_MIN_PERIOD. This is because timer is deleted at the beginning
>> of led_blink_set. Effectively this can result in the LED staying turned
>> on when e.g. "echo 1 > delay_on" fails.
>>
>> I propose to change this part of code as follows:
>>
>> if (!(led_cdev->flags & LED_BRIGHTNESS_FAST)) {
>> if (delay_on < LED_SLOW_MIN_PERIOD ||
>> delay_off < LED_SLOW_MIN_PERIOD)
>> led_set_brightness_async(led_cdev, LED_OFF);
>> return -ERANGE;
>> }
>> }
> Sounds good.
>
>> By this occasion it turned out that we have inconsistency:
>> led_set_brightness_async calls brightness_set op, which doesn't
>> set LED brightness in an asynchronous way in case of every driver.
>>
>> Before introduction of SET_BRIGHTNESS_SYNC and SET_BRIGHTNESS_ASYNC
>> flags there was only brightness_set op. The flags and
>> brightness_set_sync op was added for flash LED controllers to avoid
>> delegating the job to work queue and assure that brightness is set
>> as quickly as possible.
>>
>> I mistakenly assumed that all drivers set the brightness with use
>> of a work queue.
> Indeed.
>
>> Now, to fix the issue all drivers that set brightness in the
>> asynchronous way need to set the SET_BRIGHTNESS_SYNC flag and rename
>> their brightness_set ops to brightness_set_sync.
>> It would be also nice to rename brightness_set op to
>> brightness_set_async.
>>
>> Also, led_set_brightness_async shouldn't be called directly
>> anywhere, but led_set_brightness should be used, as it
>> selects the appropriate op basing on the SET_BRIGHTNESS_* flag.
>>
>> I'd prefer to do these modifications before merging this patch
> Sounds good: I wonder if the new flag for FAST drivers will then
> be needed at all: maybe it would be enough for the driver to define
> a SYNC method, and we assume the SYNC methods are always fast?

Flash LED controllers also set SYNC flag, but they are not fast.
They require to implement async method (current brightness_set),
for staying compatible with triggers.

> In fact, the things are more complicated: some drivers do small
> udelay()'s but do not use a work-queue. I was not marking them as
> FAST, although perhaps they could still be marked as SYNC?

This could be handled by adding a property to struct led_classdev
for defining minimum acceptable delay. Then FAST flag should not
be needed.

>> set. I can produce the patch set within a few days. Or, maybe you
>> would like to take care of it?
> I would appreciate if you do.
> I very much hate to do any non-trivial changes to the code I can't
> even properly test (sometimes not even compile-test!).

OK, I'll do that.

> Since you are at it, I'd also suggest to kill the work-queue in
> led-core.c. Now that we fixed a scenario where it was mistakenly
> used, I again think it is not needed for what it was initially advertised.
>

OK, I'll check this.

--
Best Regards,
Jacek Anaszewski

2015-06-01 14:37:59

by Stas Sergeev

[permalink] [raw]
Subject: Re: [PATCH 01/20] leds: implement LED_BRIGHTNESS_FAST flag

01.06.2015 17:19, Jacek Anaszewski пишет:
>> In fact, the things are more complicated: some drivers do small
>> udelay()'s but do not use a work-queue. I was not marking them as
>> FAST, although perhaps they could still be marked as SYNC?
> This could be handled by adding a property to struct led_classdev
> for defining minimum acceptable delay. Then FAST flag should not
> be needed.
Oh c'mon, that's too difficult!
Lets just have a flag whether we can do an SW PWM from hrtimer irq callback.
If we can't do from irq callback - simply do not do anything below 10mS.
IMHO a simple and practical solution.
Otherwise we'll not have anything implemented at all I guess.

2015-06-02 08:12:38

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH 01/20] leds: implement LED_BRIGHTNESS_FAST flag

On 06/01/2015 04:37 PM, Stas Sergeev wrote:
> 01.06.2015 17:19, Jacek Anaszewski пишет:
>>> In fact, the things are more complicated: some drivers do small
>>> udelay()'s but do not use a work-queue. I was not marking them as
>>> FAST, although perhaps they could still be marked as SYNC?
>> This could be handled by adding a property to struct led_classdev
>> for defining minimum acceptable delay. Then FAST flag should not
>> be needed.
> Oh c'mon, that's too difficult!
> Lets just have a flag whether we can do an SW PWM from hrtimer irq callback.
> If we can't do from irq callback - simply do not do anything below 10mS.
> IMHO a simple and practical solution.
> Otherwise we'll not have anything implemented at all I guess.
>

I agree if we are not going to mark the drivers using delays as FAST.
Otherwise the minimum acceptable value stemming from delay value
would be required. I prefer the former.

--
Best Regards,
Jacek Anaszewski