[PATCH v7 0/5] leds: trigger: Add pattern initialization from Device Tree

[PATCH v7 0/5] leds: trigger: Add pattern initialization from Device Tree

[Krzysztof Kozlowski]

Hi,

Changes since v6:
1. Drop goto from led_get_default_pattern() and rename "DeviceTree"
   into "device tree".
2. Add Pavel's ack to patch 1/5.

Changes since v5:
1. Drop the "classdev" prefix from helper for getting default pattern
   and move the kerneldoc to header file (this was not fixed in v5 by
   mistake).

Changes since v4:
1. Move helper to get default pattern from led-class.c to led-core.c and
   rename it (build issue pointed by kbuild robot).

Changes since v3:
1. Add missing EXPORT_SYMBOL_GPL.
2. Put pattern trigger format into common file - shared between
   dt-bindings and sysfs.
3. Use array of integers as led-pattern property (since it is bigger change
   I did not add Pavel's ack to patch 2/5).
4. Mention ms units for led-pattern bindings.

Changes since v2:
1. Drop Jacek's patches and my "led: triggers: Initialize
   LED_INIT_DEFAULT_TRIGGER if trigger is brought after class".
2. Follow Rob's advices about bindings - use "led-pattern" property
   and generalize usage of it into to three triggers.
3. New patches (2/5, 4/5 and 5/5).

Changes since v1:
1. Rebase on Jacek's patches.
2. Add patch 3/5 for fixup of Jacek's solution.
3. Drop first two patches from the series (applied).
4. Patch 5/5: Use LED_INIT_DEFAULT_TRIGGER (suggested by Jacek Anaszewski).
5. Patch 5/5: Return-on-error and log warning (suggested by Pavel Machek).


Best regards,
Krzysztof

Krzysztof Kozlowski (5):
  dt-bindings: leds: Add pattern initialization from Device Tree
  leds: Add helper for getting default pattern from Device Tree
  leds: trigger: pattern: Add pattern initialization from Device Tree
  leds: trigger: oneshot: Add initialization from Device Tree
  leds: trigger: timer: Add initialization from Device Tree

 .../ABI/testing/sysfs-class-led-trigger-pattern    | 51 +----------
 Documentation/devicetree/bindings/leds/common.txt  | 12 +++
 .../bindings/leds/leds-trigger-pattern.txt         | 49 +++++++++++
 drivers/leds/led-core.c                            | 30 +++++++
 drivers/leds/trigger/ledtrig-oneshot.c             | 38 ++++++++-
 drivers/leds/trigger/ledtrig-pattern.c             | 99 +++++++++++++++++-----
 drivers/leds/trigger/ledtrig-timer.c               | 34 ++++++++
 include/linux/leds.h                               | 13 +++
 8 files changed, 257 insertions(+), 69 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt

-- 
2.7.4

[PATCH v7 1/5] dt-bindings: leds: Add pattern initialization from Device Tree

[Krzysztof Kozlowski]

Document new led-pattern property for initialization of LED triggers.
The property format is trigger-specific (except being array of
integers).  For pattern trigger, the explanation of pattern format was
moved to a common file shared with sysfs ABI.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 .../ABI/testing/sysfs-class-led-trigger-pattern    | 51 ++--------------------
 Documentation/devicetree/bindings/leds/common.txt  | 12 +++++
 .../bindings/leds/leds-trigger-pattern.txt         | 49 +++++++++++++++++++++
 3 files changed, 64 insertions(+), 48 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt

diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-pattern b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern
index 1e5d172e0646..bd92ef9d6faa 100644
--- a/Documentation/ABI/testing/sysfs-class-led-trigger-pattern
+++ b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern
@@ -7,55 +7,10 @@ Description:
 		timer. It can do gradual dimming and step change of brightness.
 
 		The pattern is given by a series of tuples, of brightness and
-		duration (ms). The LED is expected to traverse the series and
-		each brightness value for the specified duration. Duration of
-		0 means brightness should immediately change to new value, and
-		writing malformed pattern deactivates any active one.
+		duration (ms).
 
-		1. For gradual dimming, the dimming interval now is set as 50
-		milliseconds. So the tuple with duration less than dimming
-		interval (50ms) is treated as a step change of brightness,
-		i.e. the subsequent brightness will be applied without adding
-		intervening dimming intervals.
-
-		The gradual dimming format of the software pattern values should be:
-		"brightness_1 duration_1 brightness_2 duration_2 brightness_3
-		duration_3 ...". For example:
-
-		echo 0 1000 255 2000 > pattern
-
-		It will make the LED go gradually from zero-intensity to max (255)
-		intensity in 1000 milliseconds, then back to zero intensity in 2000
-		milliseconds:
-
-		LED brightness
-		    ^
-		255-|       / \            / \            /
-		    |      /    \         /    \         /
-		    |     /       \      /       \      /
-		    |    /          \   /          \   /
-		  0-|   /             \/             \/
-		    +---0----1----2----3----4----5----6------------> time (s)
-
-		2. To make the LED go instantly from one brightness value to another,
-		we should use zero-time lengths (the brightness must be same as
-		the previous tuple's). So the format should be:
-		"brightness_1 duration_1 brightness_1 0 brightness_2 duration_2
-		brightness_2 0 ...". For example:
-
-		echo 0 1000 0 0 255 2000 255 0 > pattern
-
-		It will make the LED stay off for one second, then stay at max brightness
-		for two seconds:
-
-		LED brightness
-		    ^
-		255-|        +---------+    +---------+
-		    |        |         |    |         |
-		    |        |         |    |         |
-		    |        |         |    |         |
-		  0-|   -----+         +----+         +----
-		    +---0----1----2----3----4----5----6------------> time (s)
+		The exact format is described in:
+		Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt
 
 What:		/sys/class/leds/<led>/hw_pattern
 Date:		September 2018
diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
index aa1399814a2a..70876ac11367 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -37,6 +37,18 @@ Optional properties for child nodes:
      "ide-disk" - LED indicates IDE disk activity (deprecated),
                   in new implementations use "disk-activity"
      "timer" - LED flashes at a fixed, configurable rate
+     "pattern" - LED alters the brightness for the specified duration with one
+                 software timer (requires "led-pattern" property)
+
+- led-pattern : Array of integers with default pattern for certain triggers.
+                Each trigger may parse this property differently:
+                - one-shot : two numbers specifying delay on and delay off (in ms),
+                - timer : two numbers specifying delay on and delay off (in ms),
+                - pattern : the pattern is given by a series of tuples, of
+                  brightness and duration (in ms).  The exact format is
+                  described in:
+                  Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt
+
 
 - led-max-microamp : Maximum LED supply current in microamperes. This property
                      can be made mandatory for the board configurations
diff --git a/Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt b/Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt
new file mode 100644
index 000000000000..d3696680bfc8
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt
@@ -0,0 +1,49 @@
+* Pattern format for LED pattern trigger
+
+The pattern is given by a series of tuples, of brightness and duration (ms).
+The LED is expected to traverse the series and each brightness value for the
+specified duration. Duration of 0 means brightness should immediately change to
+new value, and writing malformed pattern deactivates any active one.
+
+1. For gradual dimming, the dimming interval now is set as 50 milliseconds. So
+the tuple with duration less than dimming interval (50ms) is treated as a step
+change of brightness, i.e. the subsequent brightness will be applied without
+adding intervening dimming intervals.
+
+The gradual dimming format of the software pattern values should be:
+"brightness_1 duration_1 brightness_2 duration_2 brightness_3 duration_3 ...".
+For example (using sysfs interface):
+
+echo 0 1000 255 2000 > pattern
+
+It will make the LED go gradually from zero-intensity to max (255) intensity in
+1000 milliseconds, then back to zero intensity in 2000 milliseconds:
+
+LED brightness
+    ^
+255-|       / \            / \            /
+    |      /    \         /    \         /
+    |     /       \      /       \      /
+    |    /          \   /          \   /
+  0-|   /             \/             \/
+    +---0----1----2----3----4----5----6------------> time (s)
+
+2. To make the LED go instantly from one brightness value to another, we should
+use zero-time lengths (the brightness must be same as the previous tuple's). So
+the format should be: "brightness_1 duration_1 brightness_1 0 brightness_2
+duration_2 brightness_2 0 ...".
+For example (using sysfs interface):
+
+echo 0 1000 0 0 255 2000 255 0 > pattern
+
+It will make the LED stay off for one second, then stay at max brightness for
+two seconds:
+
+LED brightness
+    ^
+255-|        +---------+    +---------+
+    |        |         |    |         |
+    |        |         |    |         |
+    |        |         |    |         |
+  0-|   -----+         +----+         +----
+    +---0----1----2----3----4----5----6------------> time (s)
-- 
2.7.4

[PATCH v7 2/5] leds: Add helper for getting default pattern from Device Tree

[Krzysztof Kozlowski]

Multiple LED triggers might need to access default pattern so add a
helper for that.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/leds/led-core.c | 30 ++++++++++++++++++++++++++++++
 include/linux/leds.h    | 13 +++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index ede4fa0ac2cc..e3da7c03da1b 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -16,7 +16,9 @@
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
 #include <linux/rwsem.h>
+#include <linux/slab.h>
 #include "leds.h"
 
 DECLARE_RWSEM(leds_list_lock);
@@ -310,6 +312,34 @@ int led_update_brightness(struct led_classdev *led_cdev)
 }
 EXPORT_SYMBOL_GPL(led_update_brightness);
 
+u32 *led_get_default_pattern(struct led_classdev *led_cdev, unsigned int *size)
+{
+	struct device_node *np = dev_of_node(led_cdev->dev);
+	u32 *pattern;
+	int count;
+
+	if (!np)
+		return NULL;
+
+	count = of_property_count_u32_elems(np, "led-pattern");
+	if (count < 0)
+		return NULL;
+
+	pattern = kcalloc(count, sizeof(*pattern), GFP_KERNEL);
+	if (!pattern)
+		return NULL;
+
+	if (of_property_read_u32_array(np, "led-pattern", pattern, count)) {
+		kfree(pattern);
+		return NULL;
+	}
+
+	*size = count;
+
+	return pattern;
+}
+EXPORT_SYMBOL_GPL(led_get_default_pattern);
+
 /* Caller must ensure led_cdev->led_access held */
 void led_sysfs_disable(struct led_classdev *led_cdev)
 {
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 5263f87e1d2c..78204650fe2a 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -219,6 +219,19 @@ extern int led_set_brightness_sync(struct led_classdev *led_cdev,
 extern int led_update_brightness(struct led_classdev *led_cdev);
 
 /**
+ * led_get_default_pattern - return default pattern
+ *
+ * @led_cdev: the LED to get default pattern for
+ * @size:     pointer for storing the number of elements in returned array,
+ *            modified only if return != NULL
+ *
+ * Return:    Allocated array of integers with default pattern from device tree
+ *            or NULL.  Caller is responsible for kfree().
+ */
+extern u32 *led_get_default_pattern(struct led_classdev *led_cdev,
+				    unsigned int *size);
+
+/**
  * led_sysfs_disable - disable LED sysfs interface
  * @led_cdev: the LED to set
  *
-- 
2.7.4

[PATCH v7 3/5] leds: trigger: pattern: Add pattern initialization from Device Tree

[Krzysztof Kozlowski]

Allow initialization of pattern used in pattern trigger from Device Tree
property.

This is especially useful for embedded systems where the pattern trigger
would be used to indicate the process of boot status in a nice,
user-friendly blinking way.  This initialization pattern will be used
till user-space is brought up and sets its own pattern, indicating the
boot status is for example finished.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/leds/trigger/ledtrig-pattern.c | 99 +++++++++++++++++++++++++++-------
 1 file changed, 80 insertions(+), 19 deletions(-)

diff --git a/drivers/leds/trigger/ledtrig-pattern.c b/drivers/leds/trigger/ledtrig-pattern.c
index 1870cf87afe1..718729c89440 100644
--- a/drivers/leds/trigger/ledtrig-pattern.c
+++ b/drivers/leds/trigger/ledtrig-pattern.c
@@ -220,22 +220,10 @@ static ssize_t pattern_trig_show_patterns(struct pattern_trig_data *data,
 	return count;
 }
 
-static ssize_t pattern_trig_store_patterns(struct led_classdev *led_cdev,
-					   const char *buf, size_t count,
-					   bool hw_pattern)
+static int pattern_trig_store_patterns_string(struct pattern_trig_data *data,
+					      const char *buf, size_t count)
 {
-	struct pattern_trig_data *data = led_cdev->trigger_data;
-	int ccount, cr, offset = 0, err = 0;
-
-	mutex_lock(&data->lock);
-
-	del_timer_sync(&data->timer);
-
-	if (data->is_hw_pattern)
-		led_cdev->pattern_clear(led_cdev);
-
-	data->is_hw_pattern = hw_pattern;
-	data->npatterns = 0;
+	int ccount, cr, offset = 0;
 
 	while (offset < count - 1 && data->npatterns < MAX_PATTERNS) {
 		cr = 0;
@@ -244,14 +232,54 @@ static ssize_t pattern_trig_store_patterns(struct led_classdev *led_cdev,
 				&data->patterns[data->npatterns].delta_t, &cr);
 		if (ccount != 2) {
 			data->npatterns = 0;
-			err = -EINVAL;
-			goto out;
+			return -EINVAL;
 		}
 
 		offset += cr;
 		data->npatterns++;
 	}
 
+	return 0;
+}
+
+static int pattern_trig_store_patterns_int(struct pattern_trig_data *data,
+					   const u32 *buf, size_t count)
+{
+	unsigned int i;
+
+	for (i = 0; i < count; i += 2) {
+		data->patterns[data->npatterns].brightness = buf[i];
+		data->patterns[data->npatterns].delta_t = buf[i + 1];
+		data->npatterns++;
+	}
+
+	return 0;
+}
+
+static ssize_t pattern_trig_store_patterns(struct led_classdev *led_cdev,
+					   const char *buf, const u32 *buf_int,
+					   size_t count, bool hw_pattern)
+{
+	struct pattern_trig_data *data = led_cdev->trigger_data;
+	int err = 0;
+
+	mutex_lock(&data->lock);
+
+	del_timer_sync(&data->timer);
+
+	if (data->is_hw_pattern)
+		led_cdev->pattern_clear(led_cdev);
+
+	data->is_hw_pattern = hw_pattern;
+	data->npatterns = 0;
+
+	if (buf)
+		err = pattern_trig_store_patterns_string(data, buf, count);
+	else
+		err = pattern_trig_store_patterns_int(data, buf_int, count);
+	if (err)
+		goto out;
+
 	err = pattern_trig_start_pattern(led_cdev);
 	if (err)
 		data->npatterns = 0;
@@ -275,7 +303,7 @@ static ssize_t pattern_store(struct device *dev, struct device_attribute *attr,
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 
-	return pattern_trig_store_patterns(led_cdev, buf, count, false);
+	return pattern_trig_store_patterns(led_cdev, buf, NULL, count, false);
 }
 
 static DEVICE_ATTR_RW(pattern);
@@ -295,7 +323,7 @@ static ssize_t hw_pattern_store(struct device *dev,
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 
-	return pattern_trig_store_patterns(led_cdev, buf, count, true);
+	return pattern_trig_store_patterns(led_cdev, buf, NULL, count, true);
 }
 
 static DEVICE_ATTR_RW(hw_pattern);
@@ -331,6 +359,30 @@ static const struct attribute_group *pattern_trig_groups[] = {
 	NULL,
 };
 
+static void pattern_init(struct led_classdev *led_cdev)
+{
+	unsigned int size = 0;
+	u32 *pattern;
+	int err;
+
+	pattern = led_get_default_pattern(led_cdev, &size);
+	if (!pattern)
+		return;
+
+	if (size % 2) {
+		dev_warn(led_cdev->dev, "Expected pattern of tuples\n");
+		goto out;
+	}
+
+	err = pattern_trig_store_patterns(led_cdev, NULL, pattern, size, false);
+	if (err < 0)
+		dev_warn(led_cdev->dev,
+			 "Pattern initialization failed with error %d\n", err);
+
+out:
+	kfree(pattern);
+}
+
 static int pattern_trig_activate(struct led_classdev *led_cdev)
 {
 	struct pattern_trig_data *data;
@@ -354,6 +406,15 @@ static int pattern_trig_activate(struct led_classdev *led_cdev)
 	timer_setup(&data->timer, pattern_trig_timer_function, 0);
 	led_cdev->activated = true;
 
+	if (led_cdev->flags & LED_INIT_DEFAULT_TRIGGER) {
+		pattern_init(led_cdev);
+		/*
+		 * Mark as initialized even on pattern_init() error because
+		 * any consecutive call to it would produce the same error.
+		 */
+		led_cdev->flags &= ~LED_INIT_DEFAULT_TRIGGER;
+	}
+
 	return 0;
 }
 
-- 
2.7.4

[PATCH v7 4/5] leds: trigger: oneshot: Add initialization from Device Tree

[Krzysztof Kozlowski]

Allow initialization of delays used in oneshot trigger from Device
Tree property.

This is especially useful for embedded systems where the trigger might
be used early, before bringing up user-space.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/leds/trigger/ledtrig-oneshot.c | 38 ++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/trigger/ledtrig-oneshot.c b/drivers/leds/trigger/ledtrig-oneshot.c
index 95c9be4b6e7e..8808f0ad7339 100644
--- a/drivers/leds/trigger/ledtrig-oneshot.c
+++ b/drivers/leds/trigger/ledtrig-oneshot.c
@@ -130,6 +130,34 @@ static struct attribute *oneshot_trig_attrs[] = {
 };
 ATTRIBUTE_GROUPS(oneshot_trig);
 
+static void pattern_init(struct led_classdev *led_cdev)
+{
+	u32 *pattern;
+	unsigned int size = 0;
+
+	pattern = led_get_default_pattern(led_cdev, &size);
+	if (!pattern)
+		goto out_default;
+
+	if (size != 2) {
+		dev_warn(led_cdev->dev,
+			 "Expected 2 but got %u values for delays pattern\n",
+			 size);
+		goto out_default;
+	}
+
+	led_cdev->blink_delay_on = pattern[0];
+	led_cdev->blink_delay_off = pattern[1];
+	kfree(pattern);
+
+	return;
+
+out_default:
+	kfree(pattern);
+	led_cdev->blink_delay_on = DEFAULT_DELAY;
+	led_cdev->blink_delay_off = DEFAULT_DELAY;
+}
+
 static int oneshot_trig_activate(struct led_classdev *led_cdev)
 {
 	struct oneshot_trig_data *oneshot_data;
@@ -140,8 +168,14 @@ static int oneshot_trig_activate(struct led_classdev *led_cdev)
 
 	led_set_trigger_data(led_cdev, oneshot_data);
 
-	led_cdev->blink_delay_on = DEFAULT_DELAY;
-	led_cdev->blink_delay_off = DEFAULT_DELAY;
+	if (led_cdev->flags & LED_INIT_DEFAULT_TRIGGER) {
+		pattern_init(led_cdev);
+		/*
+		 * Mark as initialized even on pattern_init() error because
+		 * any consecutive call to it would produce the same error.
+		 */
+		led_cdev->flags &= ~LED_INIT_DEFAULT_TRIGGER;
+	}
 
 	return 0;
 }
-- 
2.7.4

[PATCH v7 5/5] leds: trigger: timer: Add initialization from Device Tree

[Krzysztof Kozlowski]

Allow initialization of delays used in timer trigger from Device
Tree property.

This is especially useful for embedded systems where the trigger might
be used early, before bringing up user-space.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/leds/trigger/ledtrig-timer.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/leds/trigger/ledtrig-timer.c b/drivers/leds/trigger/ledtrig-timer.c
index 7c14983781ee..ca898c1383be 100644
--- a/drivers/leds/trigger/ledtrig-timer.c
+++ b/drivers/leds/trigger/ledtrig-timer.c
@@ -15,6 +15,7 @@
 #include <linux/init.h>
 #include <linux/device.h>
 #include <linux/ctype.h>
+#include <linux/slab.h>
 #include <linux/leds.h>
 
 static ssize_t led_delay_on_show(struct device *dev,
@@ -77,8 +78,41 @@ static struct attribute *timer_trig_attrs[] = {
 };
 ATTRIBUTE_GROUPS(timer_trig);
 
+static void pattern_init(struct led_classdev *led_cdev)
+{
+	u32 *pattern;
+	unsigned int size = 0;
+
+	pattern = led_get_default_pattern(led_cdev, &size);
+	if (!pattern)
+		return;
+
+	if (size != 2) {
+		dev_warn(led_cdev->dev,
+			 "Expected 2 but got %u values for delays pattern\n",
+			 size);
+		goto out;
+	}
+
+	led_cdev->blink_delay_on = pattern[0];
+	led_cdev->blink_delay_off = pattern[1];
+	/* led_blink_set() called by caller */
+
+out:
+	kfree(pattern);
+}
+
 static int timer_trig_activate(struct led_classdev *led_cdev)
 {
+	if (led_cdev->flags & LED_INIT_DEFAULT_TRIGGER) {
+		pattern_init(led_cdev);
+		/*
+		 * Mark as initialized even on pattern_init() error because
+		 * any consecutive call to it would produce the same error.
+		 */
+		led_cdev->flags &= ~LED_INIT_DEFAULT_TRIGGER;
+	}
+
 	led_blink_set(led_cdev, &led_cdev->blink_delay_on,
 		      &led_cdev->blink_delay_off);
 
-- 
2.7.4

Re: [PATCH v7 2/5] leds: Add helper for getting default pattern from Device Tree

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 389 bytes --]

On Wed 2019-01-09 15:44:46, Krzysztof Kozlowski wrote:
> Multiple LED triggers might need to access default pattern so add a
> helper for that.
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

Acked-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v7 3/5] leds: trigger: pattern: Add pattern initialization from Device Tree

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 721 bytes --]

On Wed 2019-01-09 15:44:47, Krzysztof Kozlowski wrote:
> Allow initialization of pattern used in pattern trigger from Device Tree
> property.
> 
> This is especially useful for embedded systems where the pattern trigger
> would be used to indicate the process of boot status in a nice,
> user-friendly blinking way.  This initialization pattern will be used
> till user-space is brought up and sets its own pattern, indicating the
> boot status is for example finished.
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

Acked-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v7 1/5] dt-bindings: leds: Add pattern initialization from Device Tree

[Jacek Anaszewski]

Hi Krzysztof,

Thank you for the update.

Patch set looks good to me.

Now we're missing only ack from Rob for this patch.

Best regards,
Jacek Anaszewski

On 1/9/19 3:44 PM, Krzysztof Kozlowski wrote:
> Document new led-pattern property for initialization of LED triggers.
> The property format is trigger-specific (except being array of
> integers).  For pattern trigger, the explanation of pattern format was
> moved to a common file shared with sysfs ABI.
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
>   .../ABI/testing/sysfs-class-led-trigger-pattern    | 51 ++--------------------
>   Documentation/devicetree/bindings/leds/common.txt  | 12 +++++
>   .../bindings/leds/leds-trigger-pattern.txt         | 49 +++++++++++++++++++++
>   3 files changed, 64 insertions(+), 48 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-pattern b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern
> index 1e5d172e0646..bd92ef9d6faa 100644
> --- a/Documentation/ABI/testing/sysfs-class-led-trigger-pattern
> +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern
> @@ -7,55 +7,10 @@ Description:
>   		timer. It can do gradual dimming and step change of brightness.
>   
>   		The pattern is given by a series of tuples, of brightness and
> -		duration (ms). The LED is expected to traverse the series and
> -		each brightness value for the specified duration. Duration of
> -		0 means brightness should immediately change to new value, and
> -		writing malformed pattern deactivates any active one.
> +		duration (ms).
>   
> -		1. For gradual dimming, the dimming interval now is set as 50
> -		milliseconds. So the tuple with duration less than dimming
> -		interval (50ms) is treated as a step change of brightness,
> -		i.e. the subsequent brightness will be applied without adding
> -		intervening dimming intervals.
> -
> -		The gradual dimming format of the software pattern values should be:
> -		"brightness_1 duration_1 brightness_2 duration_2 brightness_3
> -		duration_3 ...". For example:
> -
> -		echo 0 1000 255 2000 > pattern
> -
> -		It will make the LED go gradually from zero-intensity to max (255)
> -		intensity in 1000 milliseconds, then back to zero intensity in 2000
> -		milliseconds:
> -
> -		LED brightness
> -		    ^
> -		255-|       / \            / \            /
> -		    |      /    \         /    \         /
> -		    |     /       \      /       \      /
> -		    |    /          \   /          \   /
> -		  0-|   /             \/             \/
> -		    +---0----1----2----3----4----5----6------------> time (s)
> -
> -		2. To make the LED go instantly from one brightness value to another,
> -		we should use zero-time lengths (the brightness must be same as
> -		the previous tuple's). So the format should be:
> -		"brightness_1 duration_1 brightness_1 0 brightness_2 duration_2
> -		brightness_2 0 ...". For example:
> -
> -		echo 0 1000 0 0 255 2000 255 0 > pattern
> -
> -		It will make the LED stay off for one second, then stay at max brightness
> -		for two seconds:
> -
> -		LED brightness
> -		    ^
> -		255-|        +---------+    +---------+
> -		    |        |         |    |         |
> -		    |        |         |    |         |
> -		    |        |         |    |         |
> -		  0-|   -----+         +----+         +----
> -		    +---0----1----2----3----4----5----6------------> time (s)
> +		The exact format is described in:
> +		Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt
>   
>   What:		/sys/class/leds/<led>/hw_pattern
>   Date:		September 2018
> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
> index aa1399814a2a..70876ac11367 100644
> --- a/Documentation/devicetree/bindings/leds/common.txt
> +++ b/Documentation/devicetree/bindings/leds/common.txt
> @@ -37,6 +37,18 @@ Optional properties for child nodes:
>        "ide-disk" - LED indicates IDE disk activity (deprecated),
>                     in new implementations use "disk-activity"
>        "timer" - LED flashes at a fixed, configurable rate
> +     "pattern" - LED alters the brightness for the specified duration with one
> +                 software timer (requires "led-pattern" property)
> +
> +- led-pattern : Array of integers with default pattern for certain triggers.
> +                Each trigger may parse this property differently:
> +                - one-shot : two numbers specifying delay on and delay off (in ms),
> +                - timer : two numbers specifying delay on and delay off (in ms),
> +                - pattern : the pattern is given by a series of tuples, of
> +                  brightness and duration (in ms).  The exact format is
> +                  described in:
> +                  Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt
> +
>   
>   - led-max-microamp : Maximum LED supply current in microamperes. This property
>                        can be made mandatory for the board configurations
> diff --git a/Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt b/Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt
> new file mode 100644
> index 000000000000..d3696680bfc8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt
> @@ -0,0 +1,49 @@
> +* Pattern format for LED pattern trigger
> +
> +The pattern is given by a series of tuples, of brightness and duration (ms).
> +The LED is expected to traverse the series and each brightness value for the
> +specified duration. Duration of 0 means brightness should immediately change to
> +new value, and writing malformed pattern deactivates any active one.
> +
> +1. For gradual dimming, the dimming interval now is set as 50 milliseconds. So
> +the tuple with duration less than dimming interval (50ms) is treated as a step
> +change of brightness, i.e. the subsequent brightness will be applied without
> +adding intervening dimming intervals.
> +
> +The gradual dimming format of the software pattern values should be:
> +"brightness_1 duration_1 brightness_2 duration_2 brightness_3 duration_3 ...".
> +For example (using sysfs interface):
> +
> +echo 0 1000 255 2000 > pattern
> +
> +It will make the LED go gradually from zero-intensity to max (255) intensity in
> +1000 milliseconds, then back to zero intensity in 2000 milliseconds:
> +
> +LED brightness
> +    ^
> +255-|       / \            / \            /
> +    |      /    \         /    \         /
> +    |     /       \      /       \      /
> +    |    /          \   /          \   /
> +  0-|   /             \/             \/
> +    +---0----1----2----3----4----5----6------------> time (s)
> +
> +2. To make the LED go instantly from one brightness value to another, we should
> +use zero-time lengths (the brightness must be same as the previous tuple's). So
> +the format should be: "brightness_1 duration_1 brightness_1 0 brightness_2
> +duration_2 brightness_2 0 ...".
> +For example (using sysfs interface):
> +
> +echo 0 1000 0 0 255 2000 255 0 > pattern
> +
> +It will make the LED stay off for one second, then stay at max brightness for
> +two seconds:
> +
> +LED brightness
> +    ^
> +255-|        +---------+    +---------+
> +    |        |         |    |         |
> +    |        |         |    |         |
> +    |        |         |    |         |
> +  0-|   -----+         +----+         +----
> +    +---0----1----2----3----4----5----6------------> time (s)
> 

Re: [PATCH v7 1/5] dt-bindings: leds: Add pattern initialization from Device Tree

[Rob Herring]

On Wed,  9 Jan 2019 15:44:45 +0100, Krzysztof Kozlowski wrote:
> Document new led-pattern property for initialization of LED triggers.
> The property format is trigger-specific (except being array of
> integers).  For pattern trigger, the explanation of pattern format was
> moved to a common file shared with sysfs ABI.
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
>  .../ABI/testing/sysfs-class-led-trigger-pattern    | 51 ++--------------------
>  Documentation/devicetree/bindings/leds/common.txt  | 12 +++++
>  .../bindings/leds/leds-trigger-pattern.txt         | 49 +++++++++++++++++++++
>  3 files changed, 64 insertions(+), 48 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v7 0/5] leds: trigger: Add pattern initialization from Device Tree

[Jacek Anaszewski]

Hi Krzysztof.

On 1/9/19 3:44 PM, Krzysztof Kozlowski wrote:
> Hi,
> 
> Changes since v6:
> 1. Drop goto from led_get_default_pattern() and rename "DeviceTree"
>     into "device tree".
> 2. Add Pavel's ack to patch 1/5.
> 
> Changes since v5:
> 1. Drop the "classdev" prefix from helper for getting default pattern
>     and move the kerneldoc to header file (this was not fixed in v5 by
>     mistake).
> 
> Changes since v4:
> 1. Move helper to get default pattern from led-class.c to led-core.c and
>     rename it (build issue pointed by kbuild robot).
> 
> Changes since v3:
> 1. Add missing EXPORT_SYMBOL_GPL.
> 2. Put pattern trigger format into common file - shared between
>     dt-bindings and sysfs.
> 3. Use array of integers as led-pattern property (since it is bigger change
>     I did not add Pavel's ack to patch 2/5).
> 4. Mention ms units for led-pattern bindings.
> 
> Changes since v2:
> 1. Drop Jacek's patches and my "led: triggers: Initialize
>     LED_INIT_DEFAULT_TRIGGER if trigger is brought after class".
> 2. Follow Rob's advices about bindings - use "led-pattern" property
>     and generalize usage of it into to three triggers.
> 3. New patches (2/5, 4/5 and 5/5).
> 
> Changes since v1:
> 1. Rebase on Jacek's patches.
> 2. Add patch 3/5 for fixup of Jacek's solution.
> 3. Drop first two patches from the series (applied).
> 4. Patch 5/5: Use LED_INIT_DEFAULT_TRIGGER (suggested by Jacek Anaszewski).
> 5. Patch 5/5: Return-on-error and log warning (suggested by Pavel Machek).
> 
> 
> Best regards,
> Krzysztof
> 
> Krzysztof Kozlowski (5):
>    dt-bindings: leds: Add pattern initialization from Device Tree
>    leds: Add helper for getting default pattern from Device Tree
>    leds: trigger: pattern: Add pattern initialization from Device Tree
>    leds: trigger: oneshot: Add initialization from Device Tree
>    leds: trigger: timer: Add initialization from Device Tree
> 
>   .../ABI/testing/sysfs-class-led-trigger-pattern    | 51 +----------
>   Documentation/devicetree/bindings/leds/common.txt  | 12 +++
>   .../bindings/leds/leds-trigger-pattern.txt         | 49 +++++++++++
>   drivers/leds/led-core.c                            | 30 +++++++
>   drivers/leds/trigger/ledtrig-oneshot.c             | 38 ++++++++-
>   drivers/leds/trigger/ledtrig-pattern.c             | 99 +++++++++++++++++-----
>   drivers/leds/trigger/ledtrig-timer.c               | 34 ++++++++
>   include/linux/leds.h                               | 13 +++
>   8 files changed, 257 insertions(+), 69 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/leds/leds-trigger-pattern.txt
> 

Patch set applied to the for-next branch of linux-leds.git.

Thank you for your effort.

-- 
Best regards,
Jacek Anaszewski