linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/25] Add generic support for composing LED class device name
@ 2019-03-10 18:28 Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 01/25] leds: class: Improve LED and LED flash class registration API Jacek Anaszewski
                   ` (25 more replies)
  0 siblings, 26 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds; +Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski

Changes from v1:

- improved led_parse_properties() to parse label property at first
  and return immediately after parsing succeeds
- added tool get_led_device_info.sh for retrieving LED class device's
  parent device related information
- extended LED naming section of Documentation/leds/leds-class.txt
- adjusted the list of LED_FUNCTION definitions according to the v1 review
  remarks
- added standard LED_COLOR_NAME definitions
- removed functions.h and moved both LED_FUNCTION and LED_COLOR_NAME
  definitions to include/dt-bindings/common.h
- rebased leds-as3645a changes on top of the patch switching to fwnode
  property API
- updated DT bindings to use new LED_COLOR_NAME definitions
- improved common LED bindings to not use address unit for sub-nodes
  without reg property

Generally I still insist on deprecating label property and devicename
section of LED name. The tool I added for obtaining LED device name
proves availability of the related information in other places in
the sysfs. Other discussed use cases are covered in the updated
Documentation/leds/leds-class.txt.

Beside that, I tried also to create macros for automatic composition
of "-N" suffixed LED functions, so that it would not be necessary
to pollute common.h with plenty repetitions of the same function,
differing only with the postfix. Unfortunately, the preprocessor
of the dtc compiler seems not to accept string concatenetation.
I.e. it is not possible to to the following assighment:

function = "hdd""-1"

If anyone knows how to obviate this shortocoming please let me know.

Original cover letter:

LED class device naming pattern included devicename section, which had
unpleasant effect of varying userspace interface dependent on underlaying
hardware. Moreover, this information was redundant in the LED name, since
the LED controller name could have been obtained from sysfs device group

This patch set introduces a led_compose_name() function in the LED core,
which unifies and simplifies LED class device name composition. This
change is accompanied by the improvements in the common LED DT bindings
where two new properties are introduced: "function" and "color" . The two
deprecate the old "label" property which was leaving too much room for
interpretation, leading to inconsistent LED naming.

There are also changes in LED DT node naming, which are in line with
DT maintainer's request from [0].

Since some DT LED naming unification, related to not including devicename
section in "label" DT property, is being requested during reviews of new
LED class drivers for almost a year now, then those drivers are the first
candidates for optimalization and the first users of the new
led_compose_name() API. The modifications were tested with Qemu,
by stubbing the driver internals where hardware interaction was needed
for proper probing.

Thanks,
Jacek Anaszewski

[0] https://lore.kernel.org/patchwork/patch/858993/

Jacek Anaszewski (25):
  leds: class: Improve LED and LED flash class registration API
  leds: core: Add support for composing LED class device names
  dt-bindings: leds: Add LED_FUNCTION definitions
  dt-bindings: leds: Add LED_COLOR_NAME definitions
  dt-bindings: leds: Add function and color properties
  dt-bindings: sc27xx-blt: Add function and color properties
  leds: sc27xx-blt: Use led_compose_name()
  dt-bindings: lt3593: Add function and color properties
  leds: lt3593: Use led_compose_name()
  dt-bindings: lp8860: Add function and color properties
  leds: lp8860: Use led_compose_name()
  dt-bindings: lm3692x: Add function and color properties
  leds: lm3692x: Use led_compose_name()
  dt-bindings: lm36010: Add function and color properties
  leds: lm3601x: Use led_compose_name()
  dt-bindings: cr0014114: Add function and color properties
  leds: cr0014114: Use led_compose_name()
  dt-bindings: aat1290: Add function and color properties
  leds: aat1290: Use led_compose_name()
  dt-bindings: as3645a: Add function and color properties
  leds: as3645a: Use led_compose_name()
  dt-bindings: leds-gpio: Add function and color properties
  leds: gpio: Use led_compose_name()
  dt-bindings: an30259a: Add function and color properties
  leds: an30259a: Use led_compose_name()

 .../devicetree/bindings/leds/ams,as3645a.txt       | 22 +++---
 Documentation/devicetree/bindings/leds/common.txt  | 55 ++++++++++++---
 .../devicetree/bindings/leds/leds-aat1290.txt      | 12 ++--
 .../devicetree/bindings/leds/leds-an30259a.txt     | 22 ++++--
 .../devicetree/bindings/leds/leds-cr0014114.txt    | 26 +++++--
 .../devicetree/bindings/leds/leds-gpio.txt         | 23 ++++--
 .../devicetree/bindings/leds/leds-lm3601x.txt      | 10 ++-
 .../devicetree/bindings/leds/leds-lm3692x.txt      |  9 ++-
 .../devicetree/bindings/leds/leds-lp8860.txt       |  9 ++-
 .../devicetree/bindings/leds/leds-lt3593.txt       | 11 ++-
 .../devicetree/bindings/leds/leds-sc27xx-bltc.txt  | 10 +--
 Documentation/leds/leds-class.txt                  | 20 +++++-
 drivers/leds/led-class-flash.c                     |  9 +--
 drivers/leds/led-class.c                           | 34 +++++----
 drivers/leds/led-core.c                            | 82 ++++++++++++++++++++++
 drivers/leds/leds-aat1290.c                        | 17 +++--
 drivers/leds/leds-an30259a.c                       | 26 +++----
 drivers/leds/leds-as3645a.c                        | 73 ++++++++-----------
 drivers/leds/leds-cr0014114.c                      | 30 +++-----
 drivers/leds/leds-gpio.c                           | 26 +++----
 drivers/leds/leds-lm3601x.c                        | 45 ++++++------
 drivers/leds/leds-lm3692x.c                        | 39 +++++-----
 drivers/leds/leds-lp8860.c                         | 38 +++++-----
 drivers/leds/leds-lt3593.c                         | 19 +++--
 drivers/leds/leds-sc27xx-bltc.c                    | 23 +++---
 include/dt-bindings/leds/common.h                  | 47 +++++++++++++
 include/linux/led-class-flash.h                    | 15 ++--
 include/linux/leds.h                               | 68 +++++++++++++++---
 tools/leds/get_led_device_info.sh                  | 81 +++++++++++++++++++++
 29 files changed, 639 insertions(+), 262 deletions(-)
 create mode 100755 tools/leds/get_led_device_info.sh

-- 
2.11.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH 01/25] leds: class: Improve LED and LED flash class registration API
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 20:18   ` Oleh Kravchenko
  2019-03-10 18:28 ` [PATCH 02/25] leds: core: Add support for composing LED class device names Jacek Anaszewski
                   ` (24 subsequent siblings)
  25 siblings, 1 reply; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski,
	Daniel Mack, Linus Walleij, Oleh Kravchenko, Sakari Ailus,
	Simon Shields

Replace of_led_classdev_register() with led_classdev_register_ext(), which
accepts easily extendable struct led_init_data, instead of the fixed
struct device_node argument. The latter can be now passed in a fwnode
property of the struct led_init_data.

The modification is driven by the need for passing another argument
to the LED class initialization function - a LED class device name.
Currently it is conveyed in the "name" char pointer property of
struct led_classdev, which is semantically and functionally incorrect
since it is needed only on LED class device registration, but persists
in system memory throughout the whole LED class device lifetime.

Whereas in case of the name strings coming from DT "label" property or
statically initialized ones the cost is only the pointer size, it grows
to LED_MAX_NAME_SIZE (64) with the advent of a new LED class device naming
scheme, where color and function properties come from separate board
firmware properties and the name needs to be constructed in a new buffer.

The change will not break any existing clients since the patch alters
also existing led_classdev{_flash}_register() macro wrappers, that pass
NULL in place of init_data, which leads to using legacy name
initialization path basing on the struct led_classdev's "name" property.

Two existing users of devm_of_led_classdev_registers() are modified
to use devm_led_classdev_register(), which will not impact their
operation since they in fact didn't need to pass struct device_node on
registration from the beginning.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Tested-by: Baolin Wang <baolin.wang@linaro.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
Acked-by: Dan Murphy <dmurphy@ti.com>
Cc: Daniel Mack <daniel@zonque.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Oleh Kravchenko <oleg@kaa.org.ua>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Simon Shields <simon@lineageos.org>
---
 drivers/leds/led-class-flash.c  |  9 +++++----
 drivers/leds/led-class.c        | 34 ++++++++++++++++++++++------------
 drivers/leds/leds-cr0014114.c   |  3 +--
 drivers/leds/leds-gpio.c        |  2 +-
 include/linux/led-class-flash.h | 15 ++++++++++-----
 include/linux/leds.h            | 37 +++++++++++++++++++++++++++----------
 6 files changed, 66 insertions(+), 34 deletions(-)

diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
index cf398275a53c..8d1c1177bb9a 100644
--- a/drivers/leds/led-class-flash.c
+++ b/drivers/leds/led-class-flash.c
@@ -285,8 +285,9 @@ static void led_flash_init_sysfs_groups(struct led_classdev_flash *fled_cdev)
 	led_cdev->groups = flash_groups;
 }
 
-int led_classdev_flash_register(struct device *parent,
-				struct led_classdev_flash *fled_cdev)
+int led_classdev_flash_register_ext(struct device *parent,
+				    struct led_classdev_flash *fled_cdev,
+				    struct led_init_data *init_data)
 {
 	struct led_classdev *led_cdev;
 	const struct led_flash_ops *ops;
@@ -312,13 +313,13 @@ int led_classdev_flash_register(struct device *parent,
 	}
 
 	/* Register led class device */
-	ret = led_classdev_register(parent, led_cdev);
+	ret = led_classdev_register_ext(parent, led_cdev, init_data);
 	if (ret < 0)
 		return ret;
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(led_classdev_flash_register);
+EXPORT_SYMBOL_GPL(led_classdev_flash_register_ext);
 
 void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev)
 {
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 3c7e3487b373..a8337e0e02ea 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -17,6 +17,7 @@
 #include <linux/leds.h>
 #include <linux/list.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/timer.h>
@@ -244,18 +245,25 @@ static int led_classdev_next_name(const char *init_name, char *name,
 }
 
 /**
- * of_led_classdev_register - register a new object of led_classdev class.
+ * led_classdev_register_ext - register a new object of led_classdev class
+ *			       with init data.
  *
  * @parent: parent of LED device
  * @led_cdev: the led_classdev structure for this device.
- * @np: DT node describing this LED
+ * @init_data: LED class device initialization data
  */
-int of_led_classdev_register(struct device *parent, struct device_node *np,
-			    struct led_classdev *led_cdev)
+int led_classdev_register_ext(struct device *parent,
+			      struct led_classdev *led_cdev,
+			      struct led_init_data *init_data)
 {
 	char name[LED_MAX_NAME_SIZE];
 	int ret;
 
+	if (init_data && init_data->name)
+		led_cdev->name = init_data->name;
+	else
+		dev_info(parent, "init_data not available");
+
 	ret = led_classdev_next_name(led_cdev->name, name, sizeof(name));
 	if (ret < 0)
 		return ret;
@@ -268,7 +276,8 @@ int of_led_classdev_register(struct device *parent, struct device_node *np,
 		mutex_unlock(&led_cdev->led_access);
 		return PTR_ERR(led_cdev->dev);
 	}
-	led_cdev->dev->of_node = np;
+	if (init_data && init_data->fwnode)
+		led_cdev->dev->of_node = to_of_node(init_data->fwnode);
 
 	if (ret)
 		dev_warn(parent, "Led %s renamed to %s due to name collision",
@@ -313,7 +322,7 @@ int of_led_classdev_register(struct device *parent, struct device_node *np,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(of_led_classdev_register);
+EXPORT_SYMBOL_GPL(led_classdev_register_ext);
 
 /**
  * led_classdev_unregister - unregisters a object of led_properties class.
@@ -358,14 +367,15 @@ static void devm_led_classdev_release(struct device *dev, void *res)
 }
 
 /**
- * devm_of_led_classdev_register - resource managed led_classdev_register()
+ * devm_led_classdev_register_ext - resource managed led_classdev_register_ext()
  *
  * @parent: parent of LED device
  * @led_cdev: the led_classdev structure for this device.
+ * @init_data: LED class device initialization data
  */
-int devm_of_led_classdev_register(struct device *parent,
-				  struct device_node *np,
-				  struct led_classdev *led_cdev)
+int devm_led_classdev_register_ext(struct device *parent,
+				   struct led_classdev *led_cdev,
+				   struct led_init_data *init_data)
 {
 	struct led_classdev **dr;
 	int rc;
@@ -374,7 +384,7 @@ int devm_of_led_classdev_register(struct device *parent,
 	if (!dr)
 		return -ENOMEM;
 
-	rc = of_led_classdev_register(parent, np, led_cdev);
+	rc = led_classdev_register_ext(parent, led_cdev, init_data);
 	if (rc) {
 		devres_free(dr);
 		return rc;
@@ -385,7 +395,7 @@ int devm_of_led_classdev_register(struct device *parent,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(devm_of_led_classdev_register);
+EXPORT_SYMBOL_GPL(devm_led_classdev_register_ext);
 
 static int devm_led_classdev_match(struct device *dev, void *res, void *data)
 {
diff --git a/drivers/leds/leds-cr0014114.c b/drivers/leds/leds-cr0014114.c
index 0e4262462cb9..1c82152764d2 100644
--- a/drivers/leds/leds-cr0014114.c
+++ b/drivers/leds/leds-cr0014114.c
@@ -207,8 +207,7 @@ static int cr0014114_probe_dt(struct cr0014114 *priv)
 		led->ldev.max_brightness	  = CR_MAX_BRIGHTNESS;
 		led->ldev.brightness_set_blocking = cr0014114_set_sync;
 
-		ret = devm_of_led_classdev_register(priv->dev, np,
-						    &led->ldev);
+		ret = devm_led_classdev_register(priv->dev, &led->ldev);
 		if (ret) {
 			dev_err(priv->dev,
 				"failed to register LED device %s, err %d",
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 998f2ff6914d..b26cf78993d1 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -112,7 +112,7 @@ static int create_gpio_led(const struct gpio_led *template,
 	if (ret < 0)
 		return ret;
 
-	return devm_of_led_classdev_register(parent, np, &led_dat->cdev);
+	return devm_led_classdev_register(parent, &led_dat->cdev);
 }
 
 struct gpio_leds_priv {
diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
index 700efaa9e115..ded8d9fa6149 100644
--- a/include/linux/led-class-flash.h
+++ b/include/linux/led-class-flash.h
@@ -90,15 +90,20 @@ static inline struct led_classdev_flash *lcdev_to_flcdev(
 }
 
 /**
- * led_classdev_flash_register - register a new object of led_classdev class
- *				 with support for flash LEDs
- * @parent: the flash LED to register
+ * led_classdev_flash_register_ext - register a new object of LED class with
+ *				     init data and with support for flash LEDs
+ * @parent: LED flash controller device this flash LED is driven by
  * @fled_cdev: the led_classdev_flash structure for this device
+ * @init_data: the LED class flash device initialization data
  *
  * Returns: 0 on success or negative error value on failure
  */
-extern int led_classdev_flash_register(struct device *parent,
-				struct led_classdev_flash *fled_cdev);
+extern int led_classdev_flash_register_ext(struct device *parent,
+					struct led_classdev_flash *fled_cdev,
+					struct led_init_data *init_data);
+
+#define led_classdev_flash_register(parent, fled_cdev)		\
+	led_classdev_flash_register_ext(parent, fled_cdev, NULL)
 
 /**
  * led_classdev_flash_unregister - unregisters an object of led_classdev class
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 5263f87e1d2c..bffb4315fd66 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -20,6 +20,7 @@
 #include <linux/spinlock.h>
 #include <linux/timer.h>
 #include <linux/workqueue.h>
+#include <uapi/linux/uleds.h>
 
 struct device;
 struct led_pattern;
@@ -34,6 +35,13 @@ enum led_brightness {
 	LED_FULL	= 255,
 };
 
+struct led_init_data {
+	/* Device fwnode handle */
+	struct fwnode_handle *fwnode;
+	/* Requested LED class device name */
+	char name[LED_MAX_NAME_SIZE];
+};
+
 struct led_classdev {
 	const char		*name;
 	enum led_brightness	 brightness;
@@ -129,16 +137,25 @@ struct led_classdev {
 	struct mutex		led_access;
 };
 
-extern int of_led_classdev_register(struct device *parent,
-				    struct device_node *np,
-				    struct led_classdev *led_cdev);
-#define led_classdev_register(parent, led_cdev)				\
-	of_led_classdev_register(parent, NULL, led_cdev)
-extern int devm_of_led_classdev_register(struct device *parent,
-					 struct device_node *np,
-					 struct led_classdev *led_cdev);
-#define devm_led_classdev_register(parent, led_cdev)			\
-	devm_of_led_classdev_register(parent, NULL, led_cdev)
+/**
+ * led_classdev_register_ext - register a new object of LED class with
+ *			       init data
+ * @parent: LED controller device this LED is driven by
+ * @led_cdev: the led_classdev structure for this device
+ * @init_data: the LED class device initialization data
+ *
+ * Returns: 0 on success or negative error value on failure
+ */
+extern int led_classdev_register_ext(struct device *parent,
+				     struct led_classdev *led_cdev,
+				     struct led_init_data *init_data);
+#define led_classdev_register(parent, led_cdev)			\
+	led_classdev_register_ext(parent, led_cdev, NULL)
+extern int devm_led_classdev_register_ext(struct device *parent,
+					  struct led_classdev *led_cdev,
+					  struct led_init_data *init_data);
+#define devm_led_classdev_register(parent, led_cdev)		\
+	devm_led_classdev_register_ext(parent, led_cdev, NULL)
 extern void led_classdev_unregister(struct led_classdev *led_cdev);
 extern void devm_led_classdev_unregister(struct device *parent,
 					 struct led_classdev *led_cdev);
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 02/25] leds: core: Add support for composing LED class device names
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 01/25] leds: class: Improve LED and LED flash class registration API Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-12 16:56   ` Jacek Anaszewski
  2019-03-12 17:15   ` Dan Murphy
  2019-03-10 18:28 ` [PATCH 03/25] dt-bindings: leds: Add LED_FUNCTION definitions Jacek Anaszewski
                   ` (23 subsequent siblings)
  25 siblings, 2 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski,
	Baolin Wang, Daniel Mack, Dan Murphy, Linus Walleij,
	Oleh Kravchenko, Sakari Ailus

Add public led_compose_name() API for composing LED class device
name basing on fwnode_handle data. The function composes device name
according to either a new <color:function> pattern or the legacy
<devicename:color:function> pattern. The decision on using the
particular pattern is made basing on whether fwnode contains new
"function" and "color" properties, or the legacy "label" proeprty.

Backwards compatibility with in-driver hard-coded LED class device
names is assured thanks to the default_desc argument.

In case none of the aforementioned properties was found, then, for OF
nodes, the node name is adopted for LED class device name.

Alongside these changes added is a new tool - tools/leds/get_led_device_info.sh.
The tool allows retrieving details of a LED class device's parent device,
which proves that getting rid of a devicename section from LED name pattern
is justified since this information is already available in sysfs.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Baolin Wang <baolin.wang@linaro.org>
Cc: Daniel Mack <daniel@zonque.org>
Cc: Dan Murphy <dmurphy@ti.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Oleh Kravchenko <oleg@kaa.org.ua>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/leds/leds-class.txt | 20 +++++++++-
 drivers/leds/led-core.c           | 82 +++++++++++++++++++++++++++++++++++++++
 include/linux/leds.h              | 31 +++++++++++++++
 tools/leds/get_led_device_info.sh | 81 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 213 insertions(+), 1 deletion(-)
 create mode 100755 tools/leds/get_led_device_info.sh

diff --git a/Documentation/leds/leds-class.txt b/Documentation/leds/leds-class.txt
index 8b39cc6b03ee..866fe87063d4 100644
--- a/Documentation/leds/leds-class.txt
+++ b/Documentation/leds/leds-class.txt
@@ -43,7 +43,22 @@ LED Device Naming
 
 Is currently of the form:
 
-"devicename:colour:function"
+"colour:function"
+
+There might be still LED class drivers around using "devicename:colour:function"
+naming pattern, but the "devicename" section is now deprecated since it used
+to convey information that was already available in the sysfs, like product
+name. There is a tool (tools/leds/get_led_device_info.sh) available for
+retrieving that information per a LED class device.
+
+Associations with other devices, like network ones, should be defined
+via LED triggr mechanism. This approach is applied by some of wireless
+network drivers that create triggers dynamically and incorporate phy
+name into its name. On the other hand input subsystem offers LED - input
+bridge (drivers/input/input-leds.c) for exposing keyboard LEDs as LED class
+devices. The get_led_device_info.sh script has support for retrieving related
+input device node name. Should it support discovery of associations between
+LEDs and other subsystems, please don't hesitate to submit a relevant patch.
 
 There have been calls for LED properties such as colour to be exported as
 individual led class attributes. As a solution which doesn't incur as much
@@ -51,6 +66,9 @@ overhead, I suggest these become part of the device name. The naming scheme
 above leaves scope for further attributes should they be needed. If sections
 of the name don't apply, just leave that section blank.
 
+Please also keep in mind that LED subsystem has a protection against LED name
+conflict. It adds numerical suffix (e.g. "_1", "_2", "_3" etc.) to the requested
+LED class device name in case it is already in use.
 
 Brightness setting API
 ======================
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index ede4fa0ac2cc..bad92250d1d5 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -16,6 +16,8 @@
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/property.h>
 #include <linux/rwsem.h>
 #include "leds.h"
 
@@ -327,3 +329,83 @@ void led_sysfs_enable(struct led_classdev *led_cdev)
 	led_cdev->flags &= ~LED_SYSFS_DISABLE;
 }
 EXPORT_SYMBOL_GPL(led_sysfs_enable);
+
+static void led_parse_properties(struct fwnode_handle *fwnode,
+				 struct led_properties *props)
+{
+	int ret;
+
+	if (!fwnode)
+		return;
+
+	if (fwnode_property_present(fwnode, "label")) {
+		ret = fwnode_property_read_string(fwnode, "label", &props->label);
+		if (ret)
+			pr_err("Error parsing \'label\' property (%d)\n", ret);
+		return;
+	}
+
+	if (fwnode_property_present(fwnode, "function")) {
+		ret = fwnode_property_read_string(fwnode, "function", &props->function);
+		if (ret)
+			pr_err("Error parsing \'function\' property (%d)\n", ret);
+	} else {
+		pr_info("\'function\' property not found\n");
+	}
+
+	if (fwnode_property_present(fwnode, "color")) {
+		ret = fwnode_property_read_string(fwnode, "color", &props->color);
+		if (ret)
+			pr_info("Error parsing \'color\' property (%d)\n", ret);
+	} else {
+		pr_info("\'color\' property not found\n");
+	}
+}
+
+int led_compose_name(struct fwnode_handle *fwnode, const char *led_hw_name,
+		     const char *default_desc, char *led_classdev_name)
+{
+	struct led_properties props = {};
+
+	if (!led_classdev_name)
+		return -EINVAL;
+
+	led_parse_properties(fwnode, &props);
+
+	if (props.label) {
+		/*
+		 * Presence of 'label' DT property implies legacy LED name,
+		 * formatted as <devicename:color:function>, with possible
+		 * section omission if doesn't apply to given device.
+		 *
+		 * If no led_hw_name has been passed, then it indicates that
+		 * DT label should be used as-is for LED class device name.
+		 * Otherwise the label is prepended with led_hw_name to compose
+		 * the final LED class device name.
+		 */
+		if (!led_hw_name) {
+			strncpy(led_classdev_name, props.label,
+				LED_MAX_NAME_SIZE);
+		} else {
+			snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
+				 led_hw_name, props.label);
+		}
+	} else if (props.function || props.color) {
+		snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
+			 props.color ?: "", props.function ?: "");
+	} else if (default_desc) {
+		if (!led_hw_name) {
+			pr_err("Legacy LED naming requires devicename segment");
+			return -EINVAL;
+		}
+		snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
+			 led_hw_name, default_desc);
+	} else if (is_of_node(fwnode)) {
+		strncpy(led_classdev_name, to_of_node(fwnode)->name,
+			LED_MAX_NAME_SIZE);
+	} else
+		return -EINVAL;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(led_compose_name);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index bffb4315fd66..c2936fc989d4 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -252,6 +252,31 @@ extern void led_sysfs_disable(struct led_classdev *led_cdev);
 extern void led_sysfs_enable(struct led_classdev *led_cdev);
 
 /**
+ * led_compose_name - compose LED class device name
+ * @child: child fwnode_handle describing a LED,
+ *	   or a group of synchronized LEDs.
+ * @led_hw_name: name of the LED controller, used when falling back to legacy
+ *		 LED naming; it should be set to NULL in new LED class drivers
+ * @default_desc: default <color:function> tuple, for backwards compatibility
+ *		  with in-driver hard-coded LED names used as a fallback when
+ *		  "label" DT property is absent; it should be set to NULL
+ *		  in new LED class drivers
+ * @led_classdev_name: composed LED class device name
+ *
+ * Create LED class device name basing on the configuration provided by the
+ * board firmware. The name can have a legacy form <devicename:color:function>,
+ * or a new form <color:function>. The latter is chosen if "label" property is
+ * absent and at least one of "color" or "function" is present in the fwnode,
+ * leaving the section blank if the related property is absent. In case none
+ * of the aforementioned properties is found, then, for OF nodes, the node name
+ * is adopted for LED class device name.
+ *
+ * Returns: 0 on success or negative error value on failure
+ */
+extern int led_compose_name(struct fwnode_handle *child, const char *led_hw_name,
+			    const char *default_desc, char *led_classdev_name);
+
+/**
  * led_sysfs_is_disabled - check if LED sysfs interface is disabled
  * @led_cdev: the LED to query
  *
@@ -428,6 +453,12 @@ struct led_platform_data {
 	struct led_info	*leds;
 };
 
+struct led_properties {
+	const char *color;
+	const char *function;
+	const char *label;
+};
+
 struct gpio_desc;
 typedef int (*gpio_blink_set_t)(struct gpio_desc *desc, int state,
 				unsigned long *delay_on,
diff --git a/tools/leds/get_led_device_info.sh b/tools/leds/get_led_device_info.sh
new file mode 100755
index 000000000000..4671aa690e4a
--- /dev/null
+++ b/tools/leds/get_led_device_info.sh
@@ -0,0 +1,81 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+if [ $# -ne 1 ]; then
+	echo "get_led_devicename.sh LED_CDEV_PATH"
+	exit 1
+fi
+
+led_cdev_path=`echo $1 | sed s'/\/$//'`
+
+ls "$led_cdev_path/brightness" > /dev/null 2>&1
+if [ $? -ne 0 ]; then
+	echo "Device \"$led_cdev_path\" does not exist."
+	exit 1
+fi
+
+bus=`readlink $led_cdev_path/device/subsystem | sed s'/.*\///'`
+usb_subdev=`readlink $led_cdev_path | grep usb | sed s'/\(.*usb[0-9]*\/[0-9]*-[0-9]*\)\/.*/\1/'`
+ls "$led_cdev_path/device/of_node/compatible" > /dev/null 2>&1
+of_node_missing=$?
+
+if [ "$bus" = "input" ]; then
+	input_node=`readlink $led_cdev_path/device | sed s'/.*\///'`
+	if [ ! -z $usb_subdev ]; then
+		bus="usb"
+	fi
+fi
+
+if [ "$bus" = "usb" ]; then
+	usb_interface=`readlink $led_cdev_path | sed s'/.*\(usb[0-9]*\)/\1/' | cut -d \/ -f 3`
+	driver=`readlink $usb_interface/driver | sed s'/.*\///'`
+	cd $led_cdev_path/../$usb_subdev
+	idVendor=`cat idVendor`
+	idProduct=`cat idProduct`
+	manufacturer=`cat manufacturer`
+	product=`cat product`
+elif [ "$bus" = "input" ]; then
+	cd $led_cdev_path
+	product=`cat device/name`
+	driver=`cat device/device/driver/description`
+elif [ $of_node_missing -eq 0 ]; then
+	cd $led_cdev_path
+	compatible=`cat device/of_node/compatible`
+	if [ "$compatible" = "gpio-leds" ]; then
+		driver="leds-gpio"
+	elif [ "$compatible" = "pwm-leds" ]; then
+		driver="leds-pwm"
+	else
+		manufacturer=`echo $compatible | cut -d, -f1`
+		product=`echo $compatible | cut -d, -f2`
+	fi
+else
+	echo "Unknown device type."
+	exit 1
+fi
+
+printf "bus:\t\t\t$bus\n"
+
+if [ ! -z "$idVendor" ]; then
+	printf "idVendor:\t\t$idVendor\n"
+fi
+
+if [ ! -z "$idProduct" ]; then
+	printf "idProduct:\t\t$idProduct\n"
+fi
+
+if [ ! -z "$manufacturer" ]; then
+	printf "manufacturer:\t\t$manufacturer\n"
+fi
+
+if [ ! -z "$product" ]; then
+	printf "product:\t\t$product\n"
+fi
+
+if [ ! -z "$driver" ]; then
+	printf "driver:\t\t\t$driver\n"
+fi
+
+if [ ! -z "$input_node" ]; then
+	printf "associated input node:\t$input_node\n"
+fi
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 03/25] dt-bindings: leds: Add LED_FUNCTION definitions
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 01/25] leds: class: Improve LED and LED flash class registration API Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 02/25] leds: core: Add support for composing LED class device names Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-11 12:22   ` Dan Murphy
  2019-03-28  0:03   ` Rob Herring
  2019-03-10 18:28 ` [PATCH 04/25] dt-bindings: leds: Add LED_COLOR_NAME definitions Jacek Anaszewski
                   ` (22 subsequent siblings)
  25 siblings, 2 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski,
	Baolin Wang, Daniel Mack, Dan Murphy, Linus Walleij,
	Oleh Kravchenko, Sakari Ailus, Simon Shields

Add common LED function definitions for use in Device Tree.
The function names were extracted from existing dts files
after eliminating oddities.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Baolin Wang <baolin.wang@linaro.org>
Cc: Daniel Mack <daniel@zonque.org>
Cc: Dan Murphy <dmurphy@ti.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Oleh Kravchenko <oleg@kaa.org.ua>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Simon Shields <simon@lineageos.org>
---
 include/dt-bindings/leds/common.h | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
index e171d0a6beb2..ffcd46317307 100644
--- a/include/dt-bindings/leds/common.h
+++ b/include/dt-bindings/leds/common.h
@@ -19,4 +19,42 @@
 #define LEDS_BOOST_ADAPTIVE	1
 #define LEDS_BOOST_FIXED	2
 
+/* Standard LED functions */
+#define LED_FUNCTION_ACTIVITY "activity"
+#define LED_FUNCTION_ADSL "adsl"
+#define LED_FUNCTION_ALARM "alarm"
+#define LED_FUNCTION_BACKLIGHT "backlight"
+#define LED_FUNCTION_BLUETOOTH "bluetooth"
+#define LED_FUNCTION_BOOT "boot"
+#define LED_FUNCTION_CHRG "chrg"
+#define LED_FUNCTION_DEBUG "debug"
+#define LED_FUNCTION_DISK "disk"
+#define LED_FUNCTION_DISK_READ "disk-read"
+#define LED_FUNCTION_DISK_WRITE "disk-write"
+#define LED_FUNCTION_FAULT "fault"
+#define LED_FUNCTION_FLASH "flash"
+#define LED_FUNCTION_HDDERR "hdderr"
+#define LED_FUNCTION_HEARTBEAT "heartbeat"
+#define LED_FUNCTION_INDICATOR "indicator"
+#define LED_FUNCTION_INFO "info"
+#define LED_FUNCTION_INTERNET "internet"
+#define LED_FUNCTION_LAN "lan"
+#define LED_FUNCTION_MMC "mmc"
+#define LED_FUNCTION_NAND "nand"
+#define LED_FUNCTION_ON "on"
+#define LED_FUNCTION_PROGRAMMING "programming"
+#define LED_FUNCTION_PWR "pwr"
+#define LED_FUNCTION_RX "rx"
+#define LED_FUNCTION_SD "sd"
+#define LED_FUNCTION_SLEEP "sleep"
+#define LED_FUNCTION_STANDBY "standby"
+#define LED_FUNCTION_STATUS "status"
+#define LED_FUNCTION_TORCH "torch"
+#define LED_FUNCTION_TV "tv"
+#define LED_FUNCTION_TX "tx"
+#define LED_FUNCTION_USB "usb"
+#define LED_FUNCTION_WAN "wan"
+#define LED_FUNCTION_WLAN "wlan"
+#define LED_FUNCTION_WPS "wps"
+
 #endif /* __DT_BINDINGS_LEDS_H */
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 04/25] dt-bindings: leds: Add LED_COLOR_NAME definitions
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (2 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 03/25] dt-bindings: leds: Add LED_FUNCTION definitions Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-11 12:23   ` Dan Murphy
                     ` (2 more replies)
  2019-03-10 18:28 ` [PATCH 05/25] dt-bindings: leds: Add function and color properties Jacek Anaszewski
                   ` (21 subsequent siblings)
  25 siblings, 3 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski,
	Baolin Wang, Daniel Mack, Dan Murphy, Linus Walleij,
	Oleh Kravchenko, Sakari Ailus, Simon Shields

Add common LED color name definitions for use in Device Tree.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Baolin Wang <baolin.wang@linaro.org>
Cc: Daniel Mack <daniel@zonque.org>
Cc: Dan Murphy <dmurphy@ti.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Oleh Kravchenko <oleg@kaa.org.ua>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Simon Shields <simon@lineageos.org>
---
 include/dt-bindings/leds/common.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
index ffcd46317307..0e986bb59391 100644
--- a/include/dt-bindings/leds/common.h
+++ b/include/dt-bindings/leds/common.h
@@ -57,4 +57,13 @@
 #define LED_FUNCTION_WLAN "wlan"
 #define LED_FUNCTION_WPS "wps"
 
+/* Standard LED colors */
+#define LED_COLOR_NAME_WHITE    "white"
+#define LED_COLOR_NAME_RED      "red"
+#define LED_COLOR_NAME_GREEN    "green"
+#define LED_COLOR_NAME_BLUE     "blue"
+#define LED_COLOR_NAME_AMBER    "amber"
+#define LED_COLOR_NAME_VIOLET   "violet"
+#define LED_COLOR_NAME_YELLOW   "yellow"
+
 #endif /* __DT_BINDINGS_LEDS_H */
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 05/25] dt-bindings: leds: Add function and color properties
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (3 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 04/25] dt-bindings: leds: Add LED_COLOR_NAME definitions Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-11 12:26   ` Dan Murphy
  2019-03-10 18:28 ` [PATCH 06/25] dt-bindings: sc27xx-blt: " Jacek Anaszewski
                   ` (20 subsequent siblings)
  25 siblings, 1 reply; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski,
	Baolin Wang, Daniel Mack, Dan Murphy, Linus Walleij,
	Oleh Kravchenko, Sakari Ailus, Simon Shields

Introduce dedicated properties for conveying information about
LED function and color. Mark old "label" property as deprecated.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Baolin Wang <baolin.wang@linaro.org>
Cc: Daniel Mack <daniel@zonque.org>
Cc: Dan Murphy <dmurphy@ti.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Oleh Kravchenko <oleg@kaa.org.ua>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Simon Shields <simon@lineageos.org>
---
 Documentation/devicetree/bindings/leds/common.txt | 55 +++++++++++++++++++----
 1 file changed, 47 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
index aa1399814a2a..3402b0e1cec9 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -10,14 +10,23 @@ can influence the way of the LED device initialization, the LED components
 have to be tightly coupled with the LED device binding. They are represented
 by child nodes of the parent LED device binding.
 
+
 Optional properties for child nodes:
 - led-sources : List of device current outputs the LED is connected to. The
 		outputs are identified by the numbers that must be defined
 		in the LED device binding documentation.
+- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
+	    from the header include/dt-bindings/leds/common.h.
+	    If there is no matching LED_FUNCTION available, add a new one.
+- color : Color of the LED. Use one of the LED_COLOR_NAME_* prefixed definitions
+	    from the header include/dt-bindings/leds/common.h.
+	    If there is no matching LED_COLOR_NAME available, add a new one.
+
 - label : The label for this LED. If omitted, the label is taken from the node
 	  name (excluding the unit address). It has to uniquely identify
 	  a device, i.e. no other LED class device can be assigned the same
-	  label.
+	  label. This property is deprecated - use 'function' and 'color'
+	  properties instead.
 
 - default-state : The initial state of the LED. Valid values are "on", "off",
   and "keep". If the LED is already on or off and the default-state property is
@@ -87,29 +96,59 @@ Required properties for trigger source:
 
 * Examples
 
-gpio-leds {
+#include <dt-bindings/leds/common.h>
+
+led-controller@0 {
 	compatible = "gpio-leds";
 
-	system-status {
-		label = "Status";
+	led0 {
+		function = LED_FUNCTION_STATUS;
 		linux,default-trigger = "heartbeat";
 		gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
 	};
 
-	usb {
+	led1 {
+		function = LED_FUNCTION_USB;
 		gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
 		trigger-sources = <&ohci_port1>, <&ehci_port1>;
 	};
 };
 
-max77693-led {
+led-controller@0 {
 	compatible = "maxim,max77693-led";
 
-	camera-flash {
-		label = "Flash";
+	led {
+		function = LED_FUNCTION_FLASH;
+		color = LED_COLOR_NAME_WHITE;
 		led-sources = <0>, <1>;
 		led-max-microamp = <50000>;
 		flash-max-microamp = <320000>;
 		flash-max-timeout-us = <500000>;
 	};
 };
+
+led-controller@30 {
+        compatible = "panasonic,an30259a";
+        reg = <0x30>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        led@1 {
+                reg = <1>;
+                linux,default-trigger = "heartbeat";
+                function = LED_FUNCTION_INDICATOR;
+                color = LED_COLOR_NAME_RED;
+        };
+
+        led@2 {
+                reg = <2>;
+                function = LED_FUNCTION_INDICATOR;
+                color = LED_COLOR_NAME_GREEN;
+        };
+
+        led@3 {
+                reg = <3>;
+                function = LED_FUNCTION_INDICATOR;
+                color = LED_COLOR_NAME_BLUE;
+        };
+};
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 06/25] dt-bindings: sc27xx-blt: Add function and color properties
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (4 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 05/25] dt-bindings: leds: Add function and color properties Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 07/25] leds: sc27xx-blt: Use led_compose_name() Jacek Anaszewski
                   ` (19 subsequent siblings)
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski, Baolin Wang

Refer to new "function" and "color" properties and mark "label"
as deprecated.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Baolin Wang <baolin.wang@linaro.org>
---
 Documentation/devicetree/bindings/leds/leds-sc27xx-bltc.txt | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-sc27xx-bltc.txt b/Documentation/devicetree/bindings/leds/leds-sc27xx-bltc.txt
index dddf84f9c7ea..0073dcea0761 100644
--- a/Documentation/devicetree/bindings/leds/leds-sc27xx-bltc.txt
+++ b/Documentation/devicetree/bindings/leds/leds-sc27xx-bltc.txt
@@ -14,7 +14,9 @@ Required child properties:
 - reg: Port this LED is connected to.
 
 Optional child properties:
-- label: See Documentation/devicetree/bindings/leds/common.txt.
+- function: See Documentation/devicetree/bindings/leds/common.txt.
+- color: See Documentation/devicetree/bindings/leds/common.txt.
+- label: See Documentation/devicetree/bindings/leds/common.txt (deprecated).
 
 Examples:
 
@@ -25,17 +27,17 @@ led-controller@200 {
 	reg = <0x200>;
 
 	led@0 {
-		label = "red";
+		color = LED_COLOR_NAME_RED;
 		reg = <0x0>;
 	};
 
 	led@1 {
-		label = "green";
+		color = LED_COLOR_NAME_GREEN;
 		reg = <0x1>;
 	};
 
 	led@2 {
-		label = "blue";
+		color = LED_COLOR_NAME_BLUE;
 		reg = <0x2>;
 	};
 };
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 07/25] leds: sc27xx-blt: Use led_compose_name()
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (5 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 06/25] dt-bindings: sc27xx-blt: " Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 08/25] dt-bindings: lt3593: Add function and color properties Jacek Anaszewski
                   ` (18 subsequent siblings)
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds; +Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski

Switch to using generic LED support for composing LED class
device name.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Tested-by: Baolin Wang <baolin.wang@linaro.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/leds-sc27xx-bltc.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/leds/leds-sc27xx-bltc.c b/drivers/leds/leds-sc27xx-bltc.c
index fecf27fb1cdc..cc93e3797a08 100644
--- a/drivers/leds/leds-sc27xx-bltc.c
+++ b/drivers/leds/leds-sc27xx-bltc.c
@@ -6,7 +6,6 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
-#include <uapi/linux/uleds.h>
 
 /* PMIC global control register definition */
 #define SC27XX_MODULE_EN0	0xc08
@@ -46,7 +45,7 @@
 #define SC27XX_DELTA_T_MAX	(SC27XX_LEDS_STEP * 255)
 
 struct sc27xx_led {
-	char name[LED_MAX_NAME_SIZE];
+	struct fwnode_handle *fwnode;
 	struct led_classdev ldev;
 	struct sc27xx_led_priv *priv;
 	u8 line;
@@ -249,19 +248,25 @@ static int sc27xx_led_register(struct device *dev, struct sc27xx_led_priv *priv)
 
 	for (i = 0; i < SC27XX_LEDS_MAX; i++) {
 		struct sc27xx_led *led = &priv->leds[i];
+		struct led_init_data init_data = { led->fwnode };
 
 		if (!led->active)
 			continue;
 
+		err = led_compose_name(led->fwnode, "sc27xx", ":",
+				       init_data.name);
+		if (err)
+			return err;
+
 		led->line = i;
 		led->priv = priv;
-		led->ldev.name = led->name;
 		led->ldev.brightness_set_blocking = sc27xx_led_set;
 		led->ldev.pattern_set = sc27xx_led_pattern_set;
 		led->ldev.pattern_clear = sc27xx_led_pattern_clear;
 		led->ldev.default_trigger = "pattern";
 
-		err = devm_led_classdev_register(dev, &led->ldev);
+		err = devm_led_classdev_register_ext(dev, &led->ldev,
+						     &init_data);
 		if (err)
 			return err;
 	}
@@ -274,7 +279,6 @@ static int sc27xx_led_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node, *child;
 	struct sc27xx_led_priv *priv;
-	const char *str;
 	u32 base, count, reg;
 	int err;
 
@@ -316,15 +320,8 @@ static int sc27xx_led_probe(struct platform_device *pdev)
 			return -EINVAL;
 		}
 
+		priv->leds[reg].fwnode = of_fwnode_handle(child);
 		priv->leds[reg].active = true;
-
-		err = of_property_read_string(child, "label", &str);
-		if (err)
-			snprintf(priv->leds[reg].name, LED_MAX_NAME_SIZE,
-				 "sc27xx::");
-		else
-			snprintf(priv->leds[reg].name, LED_MAX_NAME_SIZE,
-				 "sc27xx:%s", str);
 	}
 
 	err = sc27xx_led_register(dev, priv);
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 08/25] dt-bindings: lt3593: Add function and color properties
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (6 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 07/25] leds: sc27xx-blt: Use led_compose_name() Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 09/25] leds: lt3593: Use led_compose_name() Jacek Anaszewski
                   ` (17 subsequent siblings)
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski, Daniel Mack

Refer to new "function" and "color" properties and mark "label"
as deprecated.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Daniel Mack <daniel@zonque.org>
---
 Documentation/devicetree/bindings/leds/leds-lt3593.txt | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-lt3593.txt b/Documentation/devicetree/bindings/leds/leds-lt3593.txt
index 6b2cabc36c0c..63af05827447 100644
--- a/Documentation/devicetree/bindings/leds/leds-lt3593.txt
+++ b/Documentation/devicetree/bindings/leds/leds-lt3593.txt
@@ -9,8 +9,10 @@ The hardware supports only one LED. The properties of this LED are
 configured in a sub-node in the device node.
 
 Optional sub-node properties:
-- label:	A label for the LED. If none is given, the LED will be
-		named "lt3595::".
+- function:		See Documentation/devicetree/bindings/leds/common.txt
+- color:		See Documentation/devicetree/bindings/leds/common.txt
+- label:		A label for the LED. If none is given, the LED will be
+			named "lt3595::" (deprecated)
 - linux,default-trigger: The default trigger for the LED.
 			See Documentation/devicetree/bindings/leds/common.txt
 - default-state:	The initial state of the LED.
@@ -21,12 +23,15 @@ be handled by its own device node.
 
 Example:
 
+#include <dt-bindings/leds/common.h>
+
 led-controller {
 	compatible = "lltc,lt3593";
 	lltc,ctrl-gpios = <&gpio 0 GPIO_ACTIVE_HIGH>;
 
 	led {
-		label = "white:backlight";
+		function = LED_FUNCTION_BACKLIGHT;
+		color = LED_COLOR_NAME_WHITE;
 		default-state = "on";
 	};
 };
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 09/25] leds: lt3593: Use led_compose_name()
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (7 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 08/25] dt-bindings: lt3593: Add function and color properties Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 10/25] dt-bindings: lp8860: Add function and color properties Jacek Anaszewski
                   ` (16 subsequent siblings)
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds; +Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski

Switch to using generic LED support for composing LED class
device name.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Acked-by: Daniel Mack <daniel@zonque.org>
---
 drivers/leds/leds-lt3593.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/leds/leds-lt3593.c b/drivers/leds/leds-lt3593.c
index de3623e0d094..d005abcf4743 100644
--- a/drivers/leds/leds-lt3593.c
+++ b/drivers/leds/leds-lt3593.c
@@ -10,10 +10,10 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/of.h>
-#include <uapi/linux/uleds.h>
+
+#define LED_LT3593_NAME "lt3593"
 
 struct lt3593_led_data {
-	char name[LED_MAX_NAME_SIZE];
 	struct led_classdev cdev;
 	struct gpio_desc *gpiod;
 };
@@ -111,6 +111,7 @@ static int lt3593_led_probe(struct platform_device *pdev)
 	struct fwnode_handle *child;
 	int ret, state = LEDS_GPIO_DEFSTATE_OFF;
 	enum gpiod_flags flags = GPIOD_OUT_LOW;
+	struct led_init_data init_data;
 	const char *tmp;
 
 	if (dev_get_platdata(dev)) {
@@ -138,14 +139,11 @@ static int lt3593_led_probe(struct platform_device *pdev)
 		return PTR_ERR(led_data->gpiod);
 
 	child = device_get_next_child_node(dev, NULL);
+	init_data.fwnode = child;
 
-	ret = fwnode_property_read_string(child, "label", &tmp);
-	if (ret < 0)
-		snprintf(led_data->name, sizeof(led_data->name),
-			 "lt3593::");
-	else
-		snprintf(led_data->name, sizeof(led_data->name),
-			 "lt3593:%s", tmp);
+	ret = led_compose_name(child, LED_LT3593_NAME, ":", init_data.name);
+	if (ret)
+		return ret;
 
 	fwnode_property_read_string(child, "linux,default-trigger",
 				    &led_data->cdev.default_trigger);
@@ -160,11 +158,10 @@ static int lt3593_led_probe(struct platform_device *pdev)
 		}
 	}
 
-	led_data->cdev.name = led_data->name;
 	led_data->cdev.brightness_set_blocking = lt3593_led_set;
 	led_data->cdev.brightness = state ? LED_FULL : LED_OFF;
 
-	ret = devm_led_classdev_register(dev, &led_data->cdev);
+	ret = devm_led_classdev_register_ext(dev, &led_data->cdev, &init_data);
 	if (ret < 0) {
 		fwnode_handle_put(child);
 		return ret;
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 10/25] dt-bindings: lp8860: Add function and color properties
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (8 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 09/25] leds: lt3593: Use led_compose_name() Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 11/25] leds: lp8860: Use led_compose_name() Jacek Anaszewski
                   ` (15 subsequent siblings)
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds; +Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski

Refer to new "function" and "color" properties and mark "label"
as deprecated.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Acked-by: Dan Murphy <dmurphy@ti.com>
---
 Documentation/devicetree/bindings/leds/leds-lp8860.txt | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-lp8860.txt b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
index 5f0e892ad759..623065a55ed8 100644
--- a/Documentation/devicetree/bindings/leds/leds-lp8860.txt
+++ b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
@@ -20,12 +20,16 @@ Required child properties:
 	- reg : 0
 
 Optional child properties:
-	- label : see Documentation/devicetree/bindings/leds/common.txt
+	- function : see Documentation/devicetree/bindings/leds/common.txt
+	- color : see Documentation/devicetree/bindings/leds/common.txt
+	- label : see Documentation/devicetree/bindings/leds/common.txt (deprecated)
 	- linux,default-trigger :
 	   see Documentation/devicetree/bindings/leds/common.txt
 
 Example:
 
+#include <dt-bindings/leds/common.h>
+
 led-controller@2d {
 	compatible = "ti,lp8860";
 	#address-cells = <1>;
@@ -36,7 +40,8 @@ led-controller@2d {
 
 	led@0 {
 		reg = <0>;
-		label = "white:backlight";
+		function = LED_FUNCTION_BACKLIGHT;
+		color = LED_COLOR_NAME_WHITE;
 		linux,default-trigger = "backlight";
 	};
 }
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 11/25] leds: lp8860: Use led_compose_name()
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (9 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 10/25] dt-bindings: lp8860: Add function and color properties Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-11 12:28   ` Dan Murphy
  2019-03-10 18:28 ` [PATCH 12/25] dt-bindings: lm3692x: Add function and color properties Jacek Anaszewski
                   ` (14 subsequent siblings)
  25 siblings, 1 reply; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski, Dan Murphy

Switch to using generic LED support for composing LED class
device name.

While at it, avoid iterating through available child of nodes
in favor of obtaining single expected child node using single
call to of_get_next_available_child().

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Dan Murphy <dmurphy@ti.com>
---
 drivers/leds/leds-lp8860.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index 39c72a908f3b..7c12ccdc1f2f 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -22,7 +22,6 @@
 #include <linux/of_gpio.h>
 #include <linux/gpio/consumer.h>
 #include <linux/slab.h>
-#include <uapi/linux/uleds.h>
 
 #define LP8860_DISP_CL1_BRT_MSB		0x00
 #define LP8860_DISP_CL1_BRT_LSB		0x01
@@ -87,6 +86,8 @@
 
 #define LP8860_CLEAR_FAULTS		0x01
 
+#define LP8860_NAME			"lp8860"
+
 /**
  * struct lp8860_led -
  * @lock - Lock for reading/writing the device
@@ -96,7 +97,6 @@
  * @eeprom_regmap - EEPROM register map
  * @enable_gpio - VDDIO/EN gpio to enable communication interface
  * @regulator - LED supply regulator pointer
- * @label - LED label
  */
 struct lp8860_led {
 	struct mutex lock;
@@ -106,7 +106,6 @@ struct lp8860_led {
 	struct regmap *eeprom_regmap;
 	struct gpio_desc *enable_gpio;
 	struct regulator *regulator;
-	char label[LED_MAX_NAME_SIZE];
 };
 
 struct lp8860_eeprom_reg {
@@ -387,25 +386,26 @@ static int lp8860_probe(struct i2c_client *client,
 	struct lp8860_led *led;
 	struct device_node *np = client->dev.of_node;
 	struct device_node *child_node;
-	const char *name;
+	struct led_init_data init_data;
 
 	led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL);
 	if (!led)
 		return -ENOMEM;
 
-	for_each_available_child_of_node(np, child_node) {
-		led->led_dev.default_trigger = of_get_property(child_node,
-						    "linux,default-trigger",
-						    NULL);
-
-		ret = of_property_read_string(child_node, "label", &name);
-		if (!ret)
-			snprintf(led->label, sizeof(led->label), "%s:%s",
-				 id->name, name);
-		else
-			snprintf(led->label, sizeof(led->label),
-				"%s::display_cluster", id->name);
-	}
+	child_node = of_get_next_available_child(np, NULL);
+	if (!child_node)
+		return -EINVAL;
+
+	init_data.fwnode = of_fwnode_handle(child_node),
+
+	led->led_dev.default_trigger = of_get_property(child_node,
+					    "linux,default-trigger",
+					    NULL);
+
+	ret = led_compose_name(init_data.fwnode, LP8860_NAME,
+			       ":display_cluster", init_data.name);
+	if (ret)
+		return ret;
 
 	led->enable_gpio = devm_gpiod_get_optional(&client->dev,
 						   "enable", GPIOD_OUT_LOW);
@@ -420,7 +420,6 @@ static int lp8860_probe(struct i2c_client *client,
 		led->regulator = NULL;
 
 	led->client = client;
-	led->led_dev.name = led->label;
 	led->led_dev.brightness_set_blocking = lp8860_brightness_set;
 
 	mutex_init(&led->lock);
@@ -447,7 +446,8 @@ static int lp8860_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 
-	ret = devm_led_classdev_register(&client->dev, &led->led_dev);
+	ret = devm_led_classdev_register_ext(&client->dev, &led->led_dev,
+					     &init_data);
 	if (ret) {
 		dev_err(&client->dev, "led register err: %d\n", ret);
 		return ret;
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 12/25] dt-bindings: lm3692x: Add function and color properties
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (10 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 11/25] leds: lp8860: Use led_compose_name() Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 13/25] leds: lm3692x: Use led_compose_name() Jacek Anaszewski
                   ` (13 subsequent siblings)
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski, Dan Murphy

Refer to new "function" and "color" properties and mark "label"
as deprecated.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Dan Murphy <dmurphy@ti.com>
---
 Documentation/devicetree/bindings/leds/leds-lm3692x.txt | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
index 08b352840bd7..c15806235e85 100644
--- a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
+++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
@@ -26,12 +26,16 @@ Required child properties:
 		3 - Will enable the LED3 sync (LM36923 only)
 
 Optional child properties:
-	- label : see Documentation/devicetree/bindings/leds/common.txt
+	- function : see Documentation/devicetree/bindings/leds/common.txt
+	- color : see Documentation/devicetree/bindings/leds/common.txt
+	- label : see Documentation/devicetree/bindings/leds/common.txt (deprecated)
 	- linux,default-trigger :
 	   see Documentation/devicetree/bindings/leds/common.txt
 
 Example:
 
+#include <dt-bindings/leds/common.h>
+
 led-controller@36 {
 	compatible = "ti,lm3692x";
 	reg = <0x36>;
@@ -43,7 +47,8 @@ led-controller@36 {
 
 	led@0 {
 		reg = <0>;
-		label = "white:backlight_cluster";
+		function = LED_BACKLIGHT_CLUSTER;
+		color = LED_COLOR_NAME_WHITE;
 		linux,default-trigger = "backlight";
 	};
 }
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 13/25] leds: lm3692x: Use led_compose_name()
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (11 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 12/25] dt-bindings: lm3692x: Add function and color properties Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-11 12:38   ` Dan Murphy
  2019-03-10 18:28 ` [PATCH 14/25] dt-bindings: lm36010: Add function and color properties Jacek Anaszewski
                   ` (12 subsequent siblings)
  25 siblings, 1 reply; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski, Dan Murphy

Switch to using generic LED support for composing LED class
device name.

Since the same device strings would be used in two places,
then add macros LM36922_NAME and LM36922_NAME for use in
lm3692x_probe_dt(() and lm3692x_id array.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
Cc: Dan Murphy <dmurphy@ti.com>
---
 drivers/leds/leds-lm3692x.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index 4f413a7c5f05..9dfc0f28a9c8 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -13,7 +13,6 @@
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
-#include <uapi/linux/uleds.h>
 
 #define LM36922_MODEL	0
 #define LM36923_MODEL	1
@@ -95,6 +94,9 @@
 #define LM3692X_FAULT_FLAG_SHRT BIT(3)
 #define LM3692X_FAULT_FLAG_OPEN BIT(4)
 
+#define LM36922_NAME "lm36922"
+#define LM36923_NAME "lm36923"
+
 /**
  * struct lm3692x_led -
  * @lock - Lock for reading/writing the device
@@ -103,7 +105,6 @@
  * @regmap - Devices register map
  * @enable_gpio - VDDIO/EN gpio to enable communication interface
  * @regulator - LED supply regulator pointer
- * @label - LED label
  * @led_enable - LED sync to be enabled
  * @model_id - Current device model ID enumerated
  */
@@ -114,7 +115,6 @@ struct lm3692x_led {
 	struct regmap *regmap;
 	struct gpio_desc *enable_gpio;
 	struct regulator *regulator;
-	char label[LED_MAX_NAME_SIZE];
 	int led_enable;
 	int model_id;
 };
@@ -325,7 +325,8 @@ static int lm3692x_init(struct lm3692x_led *led)
 static int lm3692x_probe_dt(struct lm3692x_led *led)
 {
 	struct fwnode_handle *child = NULL;
-	const char *name;
+	struct led_init_data init_data;
+	char *model_name;
 	int ret;
 
 	led->enable_gpio = devm_gpiod_get_optional(&led->client->dev,
@@ -346,17 +347,20 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
 		dev_err(&led->client->dev, "No LED Child node\n");
 		return -ENODEV;
 	}
+	init_data.fwnode = child;
 
-	fwnode_property_read_string(child, "linux,default-trigger",
-				    &led->led_dev.default_trigger);
+	if (led->model_id == LM36922_MODEL)
+		model_name =  LM36922_NAME;
+	else
+		model_name =  LM36923_NAME;
 
-	ret = fwnode_property_read_string(child, "label", &name);
+	ret = led_compose_name(child, model_name, ":backlight_cluster",
+			       init_data.name);
 	if (ret)
-		snprintf(led->label, sizeof(led->label),
-			"%s::", led->client->name);
-	else
-		snprintf(led->label, sizeof(led->label),
-			 "%s:%s", led->client->name, name);
+		return ret;
+
+	fwnode_property_read_string(child, "linux,default-trigger",
+				    &led->led_dev.default_trigger);
 
 	ret = fwnode_property_read_u32(child, "reg", &led->led_enable);
 	if (ret) {
@@ -364,16 +368,13 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
 		return ret;
 	}
 
-	led->led_dev.name = led->label;
-
-	ret = devm_led_classdev_register(&led->client->dev, &led->led_dev);
+	ret = devm_led_classdev_register_ext(&led->client->dev, &led->led_dev,
+					     &init_data);
 	if (ret) {
 		dev_err(&led->client->dev, "led register err: %d\n", ret);
 		return ret;
 	}
 
-	led->led_dev.dev->of_node = to_of_node(child);
-
 	return 0;
 }
 
@@ -439,8 +440,8 @@ static int lm3692x_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id lm3692x_id[] = {
-	{ "lm36922", LM36922_MODEL },
-	{ "lm36923", LM36923_MODEL },
+	{ LM36922_NAME, LM36922_MODEL },
+	{ LM36923_NAME, LM36923_MODEL },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, lm3692x_id);
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 14/25] dt-bindings: lm36010: Add function and color properties
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (12 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 13/25] leds: lm3692x: Use led_compose_name() Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 15/25] leds: lm3601x: Use led_compose_name() Jacek Anaszewski
                   ` (11 subsequent siblings)
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski, Dan Murphy

Refer to new "function" and "color" properties and mark "label"
as deprecated.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Dan Murphy <dmurphy@ti.com>
---
 Documentation/devicetree/bindings/leds/leds-lm3601x.txt | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-lm3601x.txt b/Documentation/devicetree/bindings/leds/leds-lm3601x.txt
index a88b2c41e75b..157b68d3db89 100644
--- a/Documentation/devicetree/bindings/leds/leds-lm3601x.txt
+++ b/Documentation/devicetree/bindings/leds/leds-lm3601x.txt
@@ -22,9 +22,14 @@ Required properties for flash LED child nodes:
 	- led-max-microamp : Range from 2.4mA - 376mA
 
 Optional child properties:
-	- label : see Documentation/devicetree/bindings/leds/common.txt
+	- function : see Documentation/devicetree/bindings/leds/common.txt
+	- color : see Documentation/devicetree/bindings/leds/common.txt
+	- label : see Documentation/devicetree/bindings/leds/common.txt (deprecated)
 
 Example:
+
+#include <dt-bindings/leds/common.h>
+
 led-controller@64 {
 	compatible = "ti,lm36010";
 	#address-cells = <1>;
@@ -33,7 +38,8 @@ led-controller@64 {
 
 	led@0 {
 		reg = <1>;
-		label = "white:torch";
+		function = LED_FUNCTION_TORCH;
+		color = LED_COLOR_NAME_WHITE;
 		led-max-microamp = <376000>;
 		flash-max-microamp = <1500000>;
 		flash-max-timeout-us = <1600000>;
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 15/25] leds: lm3601x: Use led_compose_name()
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (13 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 14/25] dt-bindings: lm36010: Add function and color properties Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 16/25] dt-bindings: cr0014114: Add function and color properties Jacek Anaszewski
                   ` (10 subsequent siblings)
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds; +Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski

Switch to using generic LED support for composing LED class
device name.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Acked-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/leds/leds-lm3601x.c | 45 ++++++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/drivers/leds/leds-lm3601x.c b/drivers/leds/leds-lm3601x.c
index 081aa71e43a3..91ea65217d48 100644
--- a/drivers/leds/leds-lm3601x.c
+++ b/drivers/leds/leds-lm3601x.c
@@ -10,7 +10,6 @@
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
-#include <uapi/linux/uleds.h>
 
 #define LM3601X_LED_IR		0x0
 #define LM3601X_LED_TORCH	0x1
@@ -90,8 +89,6 @@ struct lm3601x_led {
 	struct regmap *regmap;
 	struct mutex lock;
 
-	char led_name[LED_MAX_NAME_SIZE];
-
 	unsigned int flash_timeout;
 	unsigned int last_flag;
 
@@ -322,10 +319,24 @@ static const struct led_flash_ops flash_ops = {
 	.fault_get		= lm3601x_flash_fault_get,
 };
 
-static int lm3601x_register_leds(struct lm3601x_led *led)
+static int lm3601x_register_leds(struct lm3601x_led *led,
+				 struct fwnode_handle *fwnode)
 {
 	struct led_classdev *led_cdev;
 	struct led_flash_setting *setting;
+	struct led_init_data init_data = { fwnode };
+	char *mode_name;
+	int ret;
+
+	if (led->led_mode == LM3601X_LED_TORCH)
+		mode_name = "torch";
+	else
+		mode_name = "infrared";
+
+	ret = led_compose_name(fwnode, led->client->name, mode_name,
+			       init_data.name);
+	if (ret < 0)
+		return ret;
 
 	led->fled_cdev.ops = &flash_ops;
 
@@ -342,20 +353,20 @@ static int lm3601x_register_leds(struct lm3601x_led *led)
 	setting->val = led->flash_current_max;
 
 	led_cdev = &led->fled_cdev.led_cdev;
-	led_cdev->name = led->led_name;
 	led_cdev->brightness_set_blocking = lm3601x_brightness_set;
 	led_cdev->max_brightness = DIV_ROUND_UP(led->torch_current_max,
 						LM3601X_TORCH_REG_DIV);
 	led_cdev->flags |= LED_DEV_CAP_FLASH;
 
-	return led_classdev_flash_register(&led->client->dev, &led->fled_cdev);
+	return led_classdev_flash_register_ext(&led->client->dev,
+						&led->fled_cdev, &init_data);
 }
 
-static int lm3601x_parse_node(struct lm3601x_led *led)
+static int lm3601x_parse_node(struct lm3601x_led *led,
+			      struct fwnode_handle **fwnode)
 {
 	struct fwnode_handle *child = NULL;
 	int ret = -ENODEV;
-	const char *name;
 
 	child = device_get_next_child_node(&led->client->dev, child);
 	if (!child) {
@@ -376,17 +387,6 @@ static int lm3601x_parse_node(struct lm3601x_led *led)
 		goto out_err;
 	}
 
-	ret = fwnode_property_read_string(child, "label", &name);
-	if (ret) {
-		if (led->led_mode == LM3601X_LED_TORCH)
-			name = "torch";
-		else
-			name = "infrared";
-	}
-
-	snprintf(led->led_name, sizeof(led->led_name),
-		"%s:%s", led->client->name, name);
-
 	ret = fwnode_property_read_u32(child, "led-max-microamp",
 					&led->torch_current_max);
 	if (ret) {
@@ -411,6 +411,8 @@ static int lm3601x_parse_node(struct lm3601x_led *led)
 		goto out_err;
 	}
 
+	*fwnode = child;
+
 out_err:
 	fwnode_handle_put(child);
 	return ret;
@@ -419,6 +421,7 @@ static int lm3601x_parse_node(struct lm3601x_led *led)
 static int lm3601x_probe(struct i2c_client *client)
 {
 	struct lm3601x_led *led;
+	struct fwnode_handle *fwnode;
 	int ret;
 
 	led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL);
@@ -428,7 +431,7 @@ static int lm3601x_probe(struct i2c_client *client)
 	led->client = client;
 	i2c_set_clientdata(client, led);
 
-	ret = lm3601x_parse_node(led);
+	ret = lm3601x_parse_node(led, &fwnode);
 	if (ret)
 		return -ENODEV;
 
@@ -442,7 +445,7 @@ static int lm3601x_probe(struct i2c_client *client)
 
 	mutex_init(&led->lock);
 
-	return lm3601x_register_leds(led);
+	return lm3601x_register_leds(led, fwnode);
 }
 
 static int lm3601x_remove(struct i2c_client *client)
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 16/25] dt-bindings: cr0014114: Add function and color properties
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (14 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 15/25] leds: lm3601x: Use led_compose_name() Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 17/25] leds: cr0014114: Use led_compose_name() Jacek Anaszewski
                   ` (9 subsequent siblings)
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski, Oleh Kravchenko

Refer to new "function" and "color" properties and mark "label"
as deprecated.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Oleh Kravchenko <oleg@kaa.org.ua>
---
 .../devicetree/bindings/leds/leds-cr0014114.txt    | 26 ++++++++++++++++------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-cr0014114.txt b/Documentation/devicetree/bindings/leds/leds-cr0014114.txt
index 4255b19ad25c..311b5dbbacac 100644
--- a/Documentation/devicetree/bindings/leds/leds-cr0014114.txt
+++ b/Documentation/devicetree/bindings/leds/leds-cr0014114.txt
@@ -11,14 +11,20 @@ Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt
 apply. In particular, "reg" and "spi-max-frequency" properties must be given.
 
 LED sub-node properties:
-- label :
+- function :
+	see Documentation/devicetree/bindings/leds/common.txt
+- color :
 	see Documentation/devicetree/bindings/leds/common.txt
+- label :
+	see Documentation/devicetree/bindings/leds/common.txt (deprecated)
 - linux,default-trigger : (optional)
 	see Documentation/devicetree/bindings/leds/common.txt
 
 Example
 -------
 
+#include <dt-bindings/leds/common.h>
+
 led-controller@0 {
 	compatible = "crane,cr0014114";
 	reg = <0>;
@@ -28,27 +34,33 @@ led-controller@0 {
 
 	led@0 {
 		reg = <0>;
-		label = "red:coin";
+		function = "coin";
+		color = LED_COLOR_NAME_RED;
 	};
 	led@1 {
 		reg = <1>;
-		label = "green:coin";
+		function = "coin";
+		color = LED_COLOR_NAME_GREEN;
 	};
 	led@2 {
 		reg = <2>;
-		label = "blue:coin";
+		function = "coin";
+		color = LED_COLOR_NAME_BLUE;
 	};
 	led@3 {
 		reg = <3>;
-		label = "red:bill";
+		function = "bill";
+		color = LED_COLOR_NAME_RED;
 	};
 	led@4 {
 		reg = <4>;
-		label = "green:bill";
+		function = "bill";
+		color = LED_COLOR_NAME_GREEN;
 	};
 	led@5 {
 		reg = <5>;
-		label = "blue:bill";
+		function = "bill";
+		color = LED_COLOR_NAME_BLUE;
 	};
 	...
 };
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 17/25] leds: cr0014114: Use led_compose_name()
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (15 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 16/25] dt-bindings: cr0014114: Add function and color properties Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 18/25] dt-bindings: aat1290: Add function and color properties Jacek Anaszewski
                   ` (8 subsequent siblings)
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski, Oleh Kravchenko

Switch to using generic LED support for composing LED class
device name.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Oleh Kravchenko <oleg@kaa.org.ua>
---
 drivers/leds/leds-cr0014114.c | 29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

diff --git a/drivers/leds/leds-cr0014114.c b/drivers/leds/leds-cr0014114.c
index 1c82152764d2..0aee979b048f 100644
--- a/drivers/leds/leds-cr0014114.c
+++ b/drivers/leds/leds-cr0014114.c
@@ -8,7 +8,6 @@
 #include <linux/of_device.h>
 #include <linux/spi/spi.h>
 #include <linux/workqueue.h>
-#include <uapi/linux/uleds.h>
 
 /*
  *  CR0014114 SPI protocol descrtiption:
@@ -40,8 +39,9 @@
 #define CR_FW_DELAY_MSEC	10
 #define CR_RECOUNT_DELAY	(HZ * 3600)
 
+#define CR_DEV_NAME		"cr0014114"
+
 struct cr0014114_led {
-	char			name[LED_MAX_NAME_SIZE];
 	struct cr0014114	*priv;
 	struct led_classdev	ldev;
 	u8			brightness;
@@ -167,8 +167,7 @@ static int cr0014114_set_sync(struct led_classdev *ldev,
 						    struct cr0014114_led,
 						    ldev);
 
-	dev_dbg(led->priv->dev, "Set brightness of %s to %d\n",
-		led->name, brightness);
+	dev_dbg(led->priv->dev, "Set brightness to %d\n", brightness);
 
 	mutex_lock(&led->priv->lock);
 	led->brightness = (u8)brightness;
@@ -183,41 +182,33 @@ static int cr0014114_probe_dt(struct cr0014114 *priv)
 	size_t			i = 0;
 	struct cr0014114_led	*led;
 	struct fwnode_handle	*child;
-	struct device_node	*np;
+	struct led_init_data	init_data;
 	int			ret;
-	const char		*str;
 
 	device_for_each_child_node(priv->dev, child) {
-		np = to_of_node(child);
 		led = &priv->leds[i];
+		init_data.fwnode = child;
 
-		ret = fwnode_property_read_string(child, "label", &str);
+		ret = led_compose_name(child, CR_DEV_NAME, ":", init_data.name);
 		if (ret)
-			snprintf(led->name, sizeof(led->name),
-				 "cr0014114::");
-		else
-			snprintf(led->name, sizeof(led->name),
-				 "cr0014114:%s", str);
+			return ret;
 
 		fwnode_property_read_string(child, "linux,default-trigger",
 					    &led->ldev.default_trigger);
 
 		led->priv			  = priv;
-		led->ldev.name			  = led->name;
 		led->ldev.max_brightness	  = CR_MAX_BRIGHTNESS;
 		led->ldev.brightness_set_blocking = cr0014114_set_sync;
 
-		ret = devm_led_classdev_register(priv->dev, &led->ldev);
+		ret = devm_led_classdev_register_ext(priv->dev, &led->ldev,
+						     &init_data);
 		if (ret) {
 			dev_err(priv->dev,
-				"failed to register LED device %s, err %d",
-				led->name, ret);
+				"failed to register LED device , err %d", ret);
 			fwnode_handle_put(child);
 			return ret;
 		}
 
-		led->ldev.dev->of_node = np;
-
 		i++;
 	}
 
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 18/25] dt-bindings: aat1290: Add function and color properties
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (16 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 17/25] leds: cr0014114: Use led_compose_name() Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 19/25] leds: aat1290: Use led_compose_name() Jacek Anaszewski
                   ` (7 subsequent siblings)
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds; +Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski

Refer to new "function" and "color" properties and mark "label"
as deprecated.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
---
 Documentation/devicetree/bindings/leds/leds-aat1290.txt | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-aat1290.txt b/Documentation/devicetree/bindings/leds/leds-aat1290.txt
index 85c0c58617f6..9192ff42c886 100644
--- a/Documentation/devicetree/bindings/leds/leds-aat1290.txt
+++ b/Documentation/devicetree/bindings/leds/leds-aat1290.txt
@@ -32,15 +32,18 @@ Required properties of the LED child node:
                          formula: T = 8.82 * 10^9 * Ct.
 
 Optional properties of the LED child node:
-- label : see Documentation/devicetree/bindings/leds/common.txt
+- function : see Documentation/devicetree/bindings/leds/common.txt
+- color : see Documentation/devicetree/bindings/leds/common.txt
+- label : see Documentation/devicetree/bindings/leds/common.txt (deprecated)
 
 Example (by Ct = 220nF, Rset = 160kohm and exynos4412-trats2 board with
 a switch that allows for routing strobe signal either from the host or from
 the camera sensor):
 
 #include "exynos4412.dtsi"
+#include <dt-bindings/leds/common.h>
 
-aat1290 {
+led-controller {
 	compatible = "skyworks,aat1290";
 	flen-gpios = <&gpj1 1 GPIO_ACTIVE_HIGH>;
 	enset-gpios = <&gpj1 2 GPIO_ACTIVE_HIGH>;
@@ -50,8 +53,9 @@ aat1290 {
 	pinctrl-1 = <&camera_flash_host>;
 	pinctrl-2 = <&camera_flash_isp>;
 
-	camera_flash: flash-led {
-		label = "aat1290-flash";
+	camera_flash: led {
+		function = LED_FUNCTION_FLASH;
+		color = LED_COLOR_NAME_WHITE;
 		led-max-microamp = <520833>;
 		flash-max-microamp = <1012500>;
 		flash-max-timeout-us = <1940000>;
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 19/25] leds: aat1290: Use led_compose_name()
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (17 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 18/25] dt-bindings: aat1290: Add function and color properties Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 20/25] dt-bindings: as3645a: Add function and color properties Jacek Anaszewski
                   ` (6 subsequent siblings)
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds; +Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski

Switch to using generic LED support for composing LED class
device name.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
---
 drivers/leds/leds-aat1290.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/leds/leds-aat1290.c b/drivers/leds/leds-aat1290.c
index 43bd8a43f36c..9132a6ee551e 100644
--- a/drivers/leds/leds-aat1290.c
+++ b/drivers/leds/leds-aat1290.c
@@ -45,6 +45,8 @@
 #define AAT1290_FLASH_TM_NUM_LEVELS	16
 #define AAT1290_MM_CURRENT_SCALE_SIZE	15
 
+#define AAT1290_NAME			"aat1290"
+
 
 struct aat1290_led_config_data {
 	/* maximum LED current in movie mode */
@@ -78,7 +80,6 @@ struct aat1290_led {
 	int *mm_current_scale;
 	/* device mode */
 	bool movie_mode;
-
 	/* brightness cache */
 	unsigned int torch_brightness;
 };
@@ -218,7 +219,6 @@ static int aat1290_led_parse_dt(struct aat1290_led *led,
 			struct aat1290_led_config_data *cfg,
 			struct device_node **sub_node)
 {
-	struct led_classdev *led_cdev = &led->fled_cdev.led_cdev;
 	struct device *dev = &led->pdev->dev;
 	struct device_node *child_node;
 #if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
@@ -257,9 +257,6 @@ static int aat1290_led_parse_dt(struct aat1290_led *led,
 		return -EINVAL;
 	}
 
-	led_cdev->name = of_get_property(child_node, "label", NULL) ? :
-						child_node->name;
-
 	ret = of_property_read_u32(child_node, "led-max-microamp",
 				&cfg->max_mm_current);
 	/*
@@ -469,6 +466,7 @@ static int aat1290_led_probe(struct platform_device *pdev)
 	struct aat1290_led *led;
 	struct led_classdev *led_cdev;
 	struct led_classdev_flash *fled_cdev;
+	struct led_init_data init_data;
 	struct aat1290_led_config_data led_cfg = {};
 	struct v4l2_flash_config v4l2_sd_cfg = {};
 	int ret;
@@ -488,6 +486,12 @@ static int aat1290_led_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
+	init_data.fwnode = of_fwnode_handle(sub_node);
+	ret = led_compose_name(init_data.fwnode, AAT1290_NAME, NULL,
+			       init_data.name);
+	if (ret < 0)
+		return ret;
+
 	mutex_init(&led->lock);
 
 	/* Initialize LED Flash class device */
@@ -498,7 +502,8 @@ static int aat1290_led_probe(struct platform_device *pdev)
 	aat1290_init_flash_timeout(led, &led_cfg);
 
 	/* Register LED Flash class device */
-	ret = led_classdev_flash_register(&pdev->dev, fled_cdev);
+	ret = led_classdev_flash_register_ext(&pdev->dev, fled_cdev,
+					      &init_data);
 	if (ret < 0)
 		goto err_flash_register;
 
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 20/25] dt-bindings: as3645a: Add function and color properties
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (18 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 19/25] leds: aat1290: Use led_compose_name() Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 21/25] leds: as3645a: Use led_compose_name() Jacek Anaszewski
                   ` (5 subsequent siblings)
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski, Sakari Ailus

Refer to new "function" and "color" properties and mark "label"
as deprecated.

Also, fix malformed syntax of address-cells and size-cells
in the example.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../devicetree/bindings/leds/ams,as3645a.txt       | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/ams,as3645a.txt b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
index fdc40e354a64..4af2987b25e9 100644
--- a/Documentation/devicetree/bindings/leds/ams,as3645a.txt
+++ b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
@@ -39,7 +39,9 @@ ams,input-max-microamp: Maximum flash controller input current. The
 Optional properties of the flash child node
 ===========================================
 
-label		: The label of the flash LED.
+function	:  See Documentation/devicetree/bindings/leds/common.txt.
+color		:  See Documentation/devicetree/bindings/leds/common.txt.
+label		:  See Documentation/devicetree/bindings/leds/common.txt (deprecated).
 
 
 Required properties of the indicator child node (1)
@@ -52,28 +54,32 @@ led-max-microamp: Maximum indicator current. The allowed values are
 Optional properties of the indicator child node
 ===============================================
 
-label		: The label of the indicator LED.
+function	:  See Documentation/devicetree/bindings/leds/common.txt.
+color		:  See Documentation/devicetree/bindings/leds/common.txt.
+label		:  See Documentation/devicetree/bindings/leds/common.txt (deprecated).
 
 
 Example
 =======
 
+#include <dt-bindings/leds/common.h>
+
 	as3645a@30 {
-		#address-cells: 1
-		#size-cells: 0
+		#address-cells = <1>;
+		#size-cells = <0>;
 		reg = <0x30>;
 		compatible = "ams,as3645a";
-		flash@0 {
+		led@0 {
 			reg = <0x0>;
 			flash-timeout-us = <150000>;
 			flash-max-microamp = <320000>;
 			led-max-microamp = <60000>;
 			ams,input-max-microamp = <1750000>;
-			label = "as3645a:flash";
+			function = LED_FUNCTION_FLASH;
 		};
-		indicator@1 {
+		led@1 {
 			reg = <0x1>;
 			led-max-microamp = <10000>;
-			label = "as3645a:indicator";
+			function = LED_FUNCTION_INDICATOR;
 		};
 	};
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 21/25] leds: as3645a: Use led_compose_name()
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (19 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 20/25] dt-bindings: as3645a: Add function and color properties Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 22/25] dt-bindings: leds-gpio: Add function and color properties Jacek Anaszewski
                   ` (4 subsequent siblings)
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski, Sakari Ailus

Switch to using generic LED support for composing LED class
device name.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/leds/leds-as3645a.c | 73 ++++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 44 deletions(-)

diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c
index b0df514992e1..a6c087e5491a 100644
--- a/drivers/leds/leds-as3645a.c
+++ b/drivers/leds/leds-as3645a.c
@@ -132,11 +132,6 @@ struct as3645a_config {
 	u32 peak;
 };
 
-struct as3645a_names {
-	char flash[32];
-	char indicator[32];
-};
-
 struct as3645a {
 	struct i2c_client *client;
 
@@ -492,12 +487,10 @@ static int as3645a_detect(struct as3645a *flash)
 }
 
 static int as3645a_parse_node(struct as3645a *flash,
-			      struct as3645a_names *names,
 			      struct fwnode_handle *fwnode)
 {
 	struct as3645a_config *cfg = &flash->cfg;
 	struct fwnode_handle *child;
-	const char *name;
 	int rval;
 
 	fwnode_for_each_child_node(fwnode, child) {
@@ -525,17 +518,6 @@ static int as3645a_parse_node(struct as3645a *flash,
 		return -ENODEV;
 	}
 
-	rval = fwnode_property_read_string(flash->flash_node, "label", &name);
-	if (!rval) {
-		strlcpy(names->flash, name, sizeof(names->flash));
-	} else if (is_of_node(fwnode)) {
-		snprintf(names->flash, sizeof(names->flash),
-			 "%pOFn:flash", to_of_node(fwnode));
-	} else {
-		dev_err(&flash->client->dev, "flash node has no label!\n");
-		return -EINVAL;
-	}
-
 	rval = fwnode_property_read_u32(flash->flash_node, "flash-timeout-us",
 					&cfg->flash_timeout_us);
 	if (rval < 0) {
@@ -573,17 +555,6 @@ static int as3645a_parse_node(struct as3645a *flash,
 		goto out_err;
 	}
 
-	rval = fwnode_property_read_string(flash->indicator_node, "label",
-					   &name);
-	if (!rval) {
-		strlcpy(names->indicator, name, sizeof(names->indicator));
-	} else if (is_of_node(fwnode)) {
-		snprintf(names->indicator, sizeof(names->indicator),
-			 "%pOFn:indicator", to_of_node(fwnode));
-	} else {
-		dev_err(&flash->client->dev, "indicator node has no label!\n");
-		return -EINVAL;
-	}
 
 	rval = fwnode_property_read_u32(flash->indicator_node,
 					"led-max-microamp",
@@ -603,21 +574,27 @@ static int as3645a_parse_node(struct as3645a *flash,
 	return rval;
 }
 
-static int as3645a_led_class_setup(struct as3645a *flash,
-				   struct as3645a_names *names)
+static int as3645a_led_class_setup(struct as3645a *flash)
 {
 	struct led_classdev *fled_cdev = &flash->fled.led_cdev;
 	struct led_classdev *iled_cdev = &flash->iled_cdev;
+	struct led_init_data init_data;
 	struct led_flash_setting *cfg;
 	int rval;
 
-	iled_cdev->name = names->indicator;
+	init_data.fwnode = flash->indicator_node;
+	rval = led_compose_name(init_data.fwnode, AS_NAME, "indicator",
+				init_data.name);
+	if (rval < 0)
+		return rval;
+
 	iled_cdev->brightness_set_blocking = as3645a_set_indicator_brightness;
 	iled_cdev->max_brightness =
 		flash->cfg.indicator_max_ua / AS_INDICATOR_INTENSITY_STEP;
 	iled_cdev->flags = LED_CORE_SUSPENDRESUME;
 
-	rval = led_classdev_register(&flash->client->dev, iled_cdev);
+	rval = led_classdev_register_ext(&flash->client->dev, iled_cdev,
+					 &init_data);
 	if (rval < 0)
 		return rval;
 
@@ -635,7 +612,12 @@ static int as3645a_led_class_setup(struct as3645a *flash,
 
 	flash->fled.ops = &as3645a_led_flash_ops;
 
-	fled_cdev->name = names->flash;
+	init_data.fwnode = flash->flash_node;
+	rval = led_compose_name(init_data.fwnode, AS_NAME, "flash",
+				init_data.name);
+	if (rval < 0)
+		goto out_err;
+
 	fled_cdev->brightness_set_blocking = as3645a_set_assist_brightness;
 	/* Value 0 is off in LED class. */
 	fled_cdev->max_brightness =
@@ -643,14 +625,18 @@ static int as3645a_led_class_setup(struct as3645a *flash,
 				       flash->cfg.assist_max_ua) + 1;
 	fled_cdev->flags = LED_DEV_CAP_FLASH | LED_CORE_SUSPENDRESUME;
 
-	rval = led_classdev_flash_register(&flash->client->dev, &flash->fled);
-	if (rval) {
-		led_classdev_unregister(iled_cdev);
-		dev_err(&flash->client->dev,
-			"led_classdev_flash_register() failed, error %d\n",
-			rval);
-	}
+	rval = led_classdev_flash_register_ext(&flash->client->dev,
+					       &flash->fled, &init_data);
+	if (rval)
+		goto out_err;
+
+	return rval;
 
+out_err:
+	led_classdev_unregister(iled_cdev);
+	dev_err(&flash->client->dev,
+		"led_classdev_flash_register() failed, error %d\n",
+		rval);
 	return rval;
 }
 
@@ -697,7 +683,6 @@ static int as3645a_v4l2_setup(struct as3645a *flash)
 
 static int as3645a_probe(struct i2c_client *client)
 {
-	struct as3645a_names names;
 	struct as3645a *flash;
 	int rval;
 
@@ -710,7 +695,7 @@ static int as3645a_probe(struct i2c_client *client)
 
 	flash->client = client;
 
-	rval = as3645a_parse_node(flash, &names, dev_fwnode(&client->dev));
+	rval = as3645a_parse_node(flash, dev_fwnode(&client->dev));
 	if (rval < 0)
 		return rval;
 
@@ -725,7 +710,7 @@ static int as3645a_probe(struct i2c_client *client)
 	if (rval)
 		goto out_mutex_destroy;
 
-	rval = as3645a_led_class_setup(flash, &names);
+	rval = as3645a_led_class_setup(flash);
 	if (rval)
 		goto out_mutex_destroy;
 
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 22/25] dt-bindings: leds-gpio: Add function and color properties
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (20 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 21/25] leds: as3645a: Use led_compose_name() Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 23/25] leds: gpio: Use led_compose_name() Jacek Anaszewski
                   ` (3 subsequent siblings)
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski, Linus Walleij

Refer to new "function" and "color" properties and mark "label"
as deprecated.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
---
 .../devicetree/bindings/leds/leds-gpio.txt         | 23 +++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-gpio.txt b/Documentation/devicetree/bindings/leds/leds-gpio.txt
index a48dda268f81..90fb24cb2864 100644
--- a/Documentation/devicetree/bindings/leds/leds-gpio.txt
+++ b/Documentation/devicetree/bindings/leds/leds-gpio.txt
@@ -10,8 +10,12 @@ LED sub-node properties:
 - gpios :  Should specify the LED's GPIO, see "gpios property" in
   Documentation/devicetree/bindings/gpio/gpio.txt.  Active low LEDs should be
   indicated using flags in the GPIO specifier.
-- label :  (optional)
+- function :  (optional)
+  see Documentation/devicetree/bindings/leds/common.txt
+- color :  (optional)
   see Documentation/devicetree/bindings/leds/common.txt
+- label :  (optional)
+  see Documentation/devicetree/bindings/leds/common.txt (deprecated)
 - linux,default-trigger :  (optional)
   see Documentation/devicetree/bindings/leds/common.txt
 - default-state:  (optional) The initial state of the LED.
@@ -27,30 +31,34 @@ LED sub-node properties:
 Examples:
 
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
 
 leds {
 	compatible = "gpio-leds";
-	hdd {
-		label = "Disk Activity";
+	led0 {
 		gpios = <&mcu_pio 0 GPIO_ACTIVE_LOW>;
 		linux,default-trigger = "disk-activity";
+		function = LED_FUNCTION_HDD;
 	};
 
-	fault {
+	led1 {
 		gpios = <&mcu_pio 1 GPIO_ACTIVE_HIGH>;
 		/* Keep LED on if BIOS detected hardware fault */
 		default-state = "keep";
+		function = LED_FUNCTION_FAULT;
 	};
 };
 
 run-control {
 	compatible = "gpio-leds";
-	red {
+	led0 {
 		gpios = <&mpc8572 6 GPIO_ACTIVE_HIGH>;
+		color = LED_COLOR_NAME_RED;
 		default-state = "off";
 	};
-	green {
+	led1 {
 		gpios = <&mpc8572 7 GPIO_ACTIVE_HIGH>;
+		color = LED_COLOR_NAME_GREEN;
 		default-state = "on";
 	};
 };
@@ -58,9 +66,10 @@ run-control {
 leds {
 	compatible = "gpio-leds";
 
-	charger-led {
+	led0 {
 		gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
 		linux,default-trigger = "max8903-charger-charging";
 		retain-state-suspended;
+		function = LED_FUNCTION_CHRG;
 	};
 };
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 23/25] leds: gpio: Use led_compose_name()
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (21 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 22/25] dt-bindings: leds-gpio: Add function and color properties Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-10 18:28 ` [PATCH 24/25] dt-bindings: an30259a: Add function and color properties Jacek Anaszewski
                   ` (2 subsequent siblings)
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski, Linus Walleij

Switch to using generic LED support for composing LED class
device name.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/leds/leds-gpio.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index b26cf78993d1..c2e551cd2fc7 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -77,11 +77,19 @@ static int gpio_blink_set(struct led_classdev *led_cdev,
 
 static int create_gpio_led(const struct gpio_led *template,
 	struct gpio_led_data *led_dat, struct device *parent,
-	struct device_node *np, gpio_blink_set_t blink_set)
+	struct fwnode_handle *fwnode, gpio_blink_set_t blink_set)
 {
+	struct led_init_data init_data = { fwnode };
 	int ret, state;
 
-	led_dat->cdev.name = template->name;
+	if (template->name) {
+		led_dat->cdev.name = template->name;
+	} else {
+		ret = led_compose_name(fwnode, NULL, NULL, init_data.name);
+		if (ret)
+			return ret;
+	}
+
 	led_dat->cdev.default_trigger = template->default_trigger;
 	led_dat->can_sleep = gpiod_cansleep(led_dat->gpiod);
 	if (!led_dat->can_sleep)
@@ -112,7 +120,8 @@ static int create_gpio_led(const struct gpio_led *template,
 	if (ret < 0)
 		return ret;
 
-	return devm_led_classdev_register(parent, &led_dat->cdev);
+	return devm_led_classdev_register_ext(parent, &led_dat->cdev,
+					      &init_data);
 }
 
 struct gpio_leds_priv {
@@ -145,15 +154,6 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
 		struct gpio_led_data *led_dat = &priv->leds[priv->num_leds];
 		struct gpio_led led = {};
 		const char *state = NULL;
-		struct device_node *np = to_of_node(child);
-
-		ret = fwnode_property_read_string(child, "label", &led.name);
-		if (ret && IS_ENABLED(CONFIG_OF) && np)
-			led.name = np->name;
-		if (!led.name) {
-			fwnode_handle_put(child);
-			return ERR_PTR(-EINVAL);
-		}
 
 		led.gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL, child,
 							     GPIOD_ASIS,
@@ -185,7 +185,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
 		if (fwnode_property_present(child, "panic-indicator"))
 			led.panic_indicator = 1;
 
-		ret = create_gpio_led(&led, led_dat, dev, np, NULL);
+		ret = create_gpio_led(&led, led_dat, dev, child, NULL);
 		if (ret < 0) {
 			fwnode_handle_put(child);
 			return ERR_PTR(ret);
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 24/25] dt-bindings: an30259a: Add function and color properties
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (22 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 23/25] leds: gpio: Use led_compose_name() Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-22  8:33   ` Pavel Machek
  2019-03-10 18:28 ` [PATCH 25/25] leds: an30259a: Use led_compose_name() Jacek Anaszewski
  2019-03-28  0:19 ` [PATCH v2 00/25] Add generic support for composing LED class device name Rob Herring
  25 siblings, 1 reply; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski, Simon Shields

Refer to new "function" and "color" properties and mark "label"
as deprecated.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Simon Shields <simon@lineageos.org>
---
 .../devicetree/bindings/leds/leds-an30259a.txt     | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-an30259a.txt b/Documentation/devicetree/bindings/leds/leds-an30259a.txt
index 6ffb861083c0..0e894c9b80ad 100644
--- a/Documentation/devicetree/bindings/leds/leds-an30259a.txt
+++ b/Documentation/devicetree/bindings/leds/leds-an30259a.txt
@@ -15,10 +15,19 @@ Required sub-node properties:
 	- reg: Pin that the LED is connected to. Must be 1, 2, or 3.
 
 Optional sub-node properties:
-	- label: see Documentation/devicetree/bindings/leds/common.txt
-	- linux,default-trigger: see Documentation/devicetree/bindings/leds/common.txt
+	- function :
+		see Documentation/devicetree/bindings/leds/common.txt
+	- color :
+		see Documentation/devicetree/bindings/leds/common.txt
+	- label :
+		see Documentation/devicetree/bindings/leds/common.txt (deprecated)
+	- linux,default-trigger :
+		see Documentation/devicetree/bindings/leds/common.txt
 
 Example:
+
+#include <dt-bindings/leds/common.h>
+
 led-controller@30 {
 	compatible = "panasonic,an30259a";
 	reg = <0x30>;
@@ -28,16 +37,19 @@ led-controller@30 {
 	led@1 {
 		reg = <1>;
 		linux,default-trigger = "heartbeat";
-		label = "red:indicator";
+		function = LED_FUNCTION_INDICATOR;
+		color = LED_COLOR_NAME_RED;
 	};
 
 	led@2 {
 		reg = <2>;
-		label = "green:indicator";
+		function = LED_FUNCTION_INDICATOR;
+		color = LED_COLOR_NAME_GREEN;
 	};
 
 	led@3 {
 		reg = <3>;
-		label = "blue:indicator";
+		function = LED_FUNCTION_INDICATOR;
+		color = LED_COLOR_NAME_BLUE;
 	};
 };
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH 25/25] leds: an30259a: Use led_compose_name()
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (23 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 24/25] dt-bindings: an30259a: Add function and color properties Jacek Anaszewski
@ 2019-03-10 18:28 ` Jacek Anaszewski
  2019-03-28  0:19 ` [PATCH v2 00/25] Add generic support for composing LED class device name Rob Herring
  25 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-10 18:28 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, jacek.anaszewski, Simon Shields

Switch to using generic LED support for composing LED class
device name.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Simon Shields <simon@lineageos.org>
---
 drivers/leds/leds-an30259a.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/leds/leds-an30259a.c b/drivers/leds/leds-an30259a.c
index 1c1f0c8c56f4..d714ee8322c0 100644
--- a/drivers/leds/leds-an30259a.c
+++ b/drivers/leds/leds-an30259a.c
@@ -13,7 +13,6 @@
 #include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/regmap.h>
-#include <uapi/linux/uleds.h>
 
 #define AN30259A_MAX_LEDS 3
 
@@ -54,6 +53,8 @@
 #define AN30259A_BLINK_MAX_TIME 7500 /* ms */
 #define AN30259A_SLOPE_RESOLUTION 500 /* ms */
 
+#define AN30259A_NAME "an30259a"
+
 #define STATE_OFF 0
 #define STATE_KEEP 1
 #define STATE_ON 2
@@ -62,11 +63,11 @@ struct an30259a;
 
 struct an30259a_led {
 	struct an30259a *chip;
+	struct fwnode_handle *fwnode;
 	struct led_classdev cdev;
 	u32 num;
 	u32 default_state;
 	bool sloping;
-	char label[LED_MAX_NAME_SIZE];
 };
 
 struct an30259a {
@@ -226,14 +227,7 @@ static int an30259a_dt_init(struct i2c_client *client,
 
 		led->num = source;
 		led->chip = chip;
-
-		if (of_property_read_string(child, "label", &str))
-			snprintf(led->label, sizeof(led->label), "an30259a::");
-		else
-			snprintf(led->label, sizeof(led->label), "an30259a:%s",
-				 str);
-
-		led->cdev.name = led->label;
+		led->fwnode = of_fwnode_handle(child);
 
 		if (!of_property_read_string(child, "default-state", &str)) {
 			if (!strcmp(str, "on"))
@@ -312,13 +306,21 @@ static int an30259a_probe(struct i2c_client *client)
 	chip->regmap = devm_regmap_init_i2c(client, &an30259a_regmap_config);
 
 	for (i = 0; i < chip->num_leds; i++) {
+		struct led_init_data init_data = { chip->leds[i].fwnode };
+
+		int ret = led_compose_name(init_data.fwnode, AN30259A_NAME, ":",
+					   init_data.name);
+		if (ret)
+			return ret;
+
 		an30259a_init_default_state(&chip->leds[i]);
 		chip->leds[i].cdev.brightness_set_blocking =
 			an30259a_brightness_set;
 		chip->leds[i].cdev.blink_set = an30259a_blink_set;
 
-		err = devm_led_classdev_register(&client->dev,
-						 &chip->leds[i].cdev);
+		err = devm_led_classdev_register_ext(&client->dev,
+						 &chip->leds[i].cdev,
+						 &init_data);
 		if (err < 0)
 			goto exit;
 	}
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 53+ messages in thread

* Re: [PATCH 01/25] leds: class: Improve LED and LED flash class registration API
  2019-03-10 18:28 ` [PATCH 01/25] leds: class: Improve LED and LED flash class registration API Jacek Anaszewski
@ 2019-03-10 20:18   ` Oleh Kravchenko
  0 siblings, 0 replies; 53+ messages in thread
From: Oleh Kravchenko @ 2019-03-10 20:18 UTC (permalink / raw)
  To: Jacek Anaszewski, linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, Daniel Mack,
	Linus Walleij, Sakari Ailus, Simon Shields

Thanks for patches :)

10.03.19 20:28, Jacek Anaszewski пише:
> Replace of_led_classdev_register() with led_classdev_register_ext(), which
> accepts easily extendable struct led_init_data, instead of the fixed
> struct device_node argument. The latter can be now passed in a fwnode
> property of the struct led_init_data.
> 
> The modification is driven by the need for passing another argument
> to the LED class initialization function - a LED class device name.
> Currently it is conveyed in the "name" char pointer property of
> struct led_classdev, which is semantically and functionally incorrect
> since it is needed only on LED class device registration, but persists
> in system memory throughout the whole LED class device lifetime.
> 
> Whereas in case of the name strings coming from DT "label" property or
> statically initialized ones the cost is only the pointer size, it grows
> to LED_MAX_NAME_SIZE (64) with the advent of a new LED class device naming
> scheme, where color and function properties come from separate board
> firmware properties and the name needs to be constructed in a new buffer.
> 
> The change will not break any existing clients since the patch alters
> also existing led_classdev{_flash}_register() macro wrappers, that pass
> NULL in place of init_data, which leads to using legacy name
> initialization path basing on the struct led_classdev's "name" property.
> 
> Two existing users of devm_of_led_classdev_registers() are modified
> to use devm_led_classdev_register(), which will not impact their
> operation since they in fact didn't need to pass struct device_node on
> registration from the beginning.
> 
> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Tested-by: Baolin Wang <baolin.wang@linaro.org>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> Acked-by: Dan Murphy <dmurphy@ti.com>
> Cc: Daniel Mack <daniel@zonque.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Simon Shields <simon@lineageos.org>
> ---
>  drivers/leds/led-class-flash.c  |  9 +++++----
>  drivers/leds/led-class.c        | 34 ++++++++++++++++++++++------------
>  drivers/leds/leds-cr0014114.c   |  3 +--
>  drivers/leds/leds-gpio.c        |  2 +-
>  include/linux/led-class-flash.h | 15 ++++++++++-----
>  include/linux/leds.h            | 37 +++++++++++++++++++++++++++----------
>  6 files changed, 66 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
> index cf398275a53c..8d1c1177bb9a 100644
> --- a/drivers/leds/led-class-flash.c
> +++ b/drivers/leds/led-class-flash.c
> @@ -285,8 +285,9 @@ static void led_flash_init_sysfs_groups(struct led_classdev_flash *fled_cdev)
>  	led_cdev->groups = flash_groups;
>  }
>  
> -int led_classdev_flash_register(struct device *parent,
> -				struct led_classdev_flash *fled_cdev)
> +int led_classdev_flash_register_ext(struct device *parent,
> +				    struct led_classdev_flash *fled_cdev,
> +				    struct led_init_data *init_data)
>  {
>  	struct led_classdev *led_cdev;
>  	const struct led_flash_ops *ops;
> @@ -312,13 +313,13 @@ int led_classdev_flash_register(struct device *parent,
>  	}
>  
>  	/* Register led class device */
> -	ret = led_classdev_register(parent, led_cdev);
> +	ret = led_classdev_register_ext(parent, led_cdev, init_data);
>  	if (ret < 0)
>  		return ret;
>  
>  	return 0;
>  }
> -EXPORT_SYMBOL_GPL(led_classdev_flash_register);
> +EXPORT_SYMBOL_GPL(led_classdev_flash_register_ext);
>  
>  void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev)
>  {
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 3c7e3487b373..a8337e0e02ea 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -17,6 +17,7 @@
>  #include <linux/leds.h>
>  #include <linux/list.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
>  #include <linux/timer.h>
> @@ -244,18 +245,25 @@ static int led_classdev_next_name(const char *init_name, char *name,
>  }
>  
>  /**
> - * of_led_classdev_register - register a new object of led_classdev class.
> + * led_classdev_register_ext - register a new object of led_classdev class
> + *			       with init data.
>   *
>   * @parent: parent of LED device
>   * @led_cdev: the led_classdev structure for this device.
> - * @np: DT node describing this LED
> + * @init_data: LED class device initialization data
>   */
> -int of_led_classdev_register(struct device *parent, struct device_node *np,
> -			    struct led_classdev *led_cdev)
> +int led_classdev_register_ext(struct device *parent,
> +			      struct led_classdev *led_cdev,
> +			      struct led_init_data *init_data)
>  {
>  	char name[LED_MAX_NAME_SIZE];
>  	int ret;
>  
> +	if (init_data && init_data->name)
> +		led_cdev->name = init_data->name;
> +	else
> +		dev_info(parent, "init_data not available");
> +
>  	ret = led_classdev_next_name(led_cdev->name, name, sizeof(name));
>  	if (ret < 0)
>  		return ret;
> @@ -268,7 +276,8 @@ int of_led_classdev_register(struct device *parent, struct device_node *np,
>  		mutex_unlock(&led_cdev->led_access);
>  		return PTR_ERR(led_cdev->dev);
>  	}
> -	led_cdev->dev->of_node = np;
> +	if (init_data && init_data->fwnode)
> +		led_cdev->dev->of_node = to_of_node(init_data->fwnode);
>  
>  	if (ret)
>  		dev_warn(parent, "Led %s renamed to %s due to name collision",
> @@ -313,7 +322,7 @@ int of_led_classdev_register(struct device *parent, struct device_node *np,
>  
>  	return 0;
>  }
> -EXPORT_SYMBOL_GPL(of_led_classdev_register);
> +EXPORT_SYMBOL_GPL(led_classdev_register_ext);
>  
>  /**
>   * led_classdev_unregister - unregisters a object of led_properties class.
> @@ -358,14 +367,15 @@ static void devm_led_classdev_release(struct device *dev, void *res)
>  }
>  
>  /**
> - * devm_of_led_classdev_register - resource managed led_classdev_register()
> + * devm_led_classdev_register_ext - resource managed led_classdev_register_ext()
>   *
>   * @parent: parent of LED device
>   * @led_cdev: the led_classdev structure for this device.
> + * @init_data: LED class device initialization data
>   */
> -int devm_of_led_classdev_register(struct device *parent,
> -				  struct device_node *np,
> -				  struct led_classdev *led_cdev)
> +int devm_led_classdev_register_ext(struct device *parent,
> +				   struct led_classdev *led_cdev,
> +				   struct led_init_data *init_data)
>  {
>  	struct led_classdev **dr;
>  	int rc;
> @@ -374,7 +384,7 @@ int devm_of_led_classdev_register(struct device *parent,
>  	if (!dr)
>  		return -ENOMEM;
>  
> -	rc = of_led_classdev_register(parent, np, led_cdev);
> +	rc = led_classdev_register_ext(parent, led_cdev, init_data);
>  	if (rc) {
>  		devres_free(dr);
>  		return rc;
> @@ -385,7 +395,7 @@ int devm_of_led_classdev_register(struct device *parent,
>  
>  	return 0;
>  }
> -EXPORT_SYMBOL_GPL(devm_of_led_classdev_register);
> +EXPORT_SYMBOL_GPL(devm_led_classdev_register_ext);
>  
>  static int devm_led_classdev_match(struct device *dev, void *res, void *data)
>  {
> diff --git a/drivers/leds/leds-cr0014114.c b/drivers/leds/leds-cr0014114.c
> index 0e4262462cb9..1c82152764d2 100644
> --- a/drivers/leds/leds-cr0014114.c
> +++ b/drivers/leds/leds-cr0014114.c
> @@ -207,8 +207,7 @@ static int cr0014114_probe_dt(struct cr0014114 *priv)
>  		led->ldev.max_brightness	  = CR_MAX_BRIGHTNESS;
>  		led->ldev.brightness_set_blocking = cr0014114_set_sync;
>  
> -		ret = devm_of_led_classdev_register(priv->dev, np,
> -						    &led->ldev);
> +		ret = devm_led_classdev_register(priv->dev, &led->ldev);
>  		if (ret) {
>  			dev_err(priv->dev,
>  				"failed to register LED device %s, err %d",
> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index 998f2ff6914d..b26cf78993d1 100644
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -112,7 +112,7 @@ static int create_gpio_led(const struct gpio_led *template,
>  	if (ret < 0)
>  		return ret;
>  
> -	return devm_of_led_classdev_register(parent, np, &led_dat->cdev);
> +	return devm_led_classdev_register(parent, &led_dat->cdev);
>  }
>  
>  struct gpio_leds_priv {
> diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
> index 700efaa9e115..ded8d9fa6149 100644
> --- a/include/linux/led-class-flash.h
> +++ b/include/linux/led-class-flash.h
> @@ -90,15 +90,20 @@ static inline struct led_classdev_flash *lcdev_to_flcdev(
>  }
>  
>  /**
> - * led_classdev_flash_register - register a new object of led_classdev class
> - *				 with support for flash LEDs
> - * @parent: the flash LED to register
> + * led_classdev_flash_register_ext - register a new object of LED class with
> + *				     init data and with support for flash LEDs
> + * @parent: LED flash controller device this flash LED is driven by
>   * @fled_cdev: the led_classdev_flash structure for this device
> + * @init_data: the LED class flash device initialization data
>   *
>   * Returns: 0 on success or negative error value on failure
>   */
> -extern int led_classdev_flash_register(struct device *parent,
> -				struct led_classdev_flash *fled_cdev);
> +extern int led_classdev_flash_register_ext(struct device *parent,
> +					struct led_classdev_flash *fled_cdev,
> +					struct led_init_data *init_data);
> +
> +#define led_classdev_flash_register(parent, fled_cdev)		\
> +	led_classdev_flash_register_ext(parent, fled_cdev, NULL)
>  
>  /**
>   * led_classdev_flash_unregister - unregisters an object of led_classdev class
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 5263f87e1d2c..bffb4315fd66 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -20,6 +20,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/timer.h>
>  #include <linux/workqueue.h>
> +#include <uapi/linux/uleds.h>
>  
>  struct device;
>  struct led_pattern;
> @@ -34,6 +35,13 @@ enum led_brightness {
>  	LED_FULL	= 255,
>  };
>  
> +struct led_init_data {
> +	/* Device fwnode handle */
> +	struct fwnode_handle *fwnode;
> +	/* Requested LED class device name */
> +	char name[LED_MAX_NAME_SIZE];
> +};
> +
>  struct led_classdev {
>  	const char		*name;
>  	enum led_brightness	 brightness;
> @@ -129,16 +137,25 @@ struct led_classdev {
>  	struct mutex		led_access;
>  };
>  
> -extern int of_led_classdev_register(struct device *parent,
> -				    struct device_node *np,
> -				    struct led_classdev *led_cdev);
> -#define led_classdev_register(parent, led_cdev)				\
> -	of_led_classdev_register(parent, NULL, led_cdev)
> -extern int devm_of_led_classdev_register(struct device *parent,
> -					 struct device_node *np,
> -					 struct led_classdev *led_cdev);
> -#define devm_led_classdev_register(parent, led_cdev)			\
> -	devm_of_led_classdev_register(parent, NULL, led_cdev)
> +/**
> + * led_classdev_register_ext - register a new object of LED class with
> + *			       init data
> + * @parent: LED controller device this LED is driven by
> + * @led_cdev: the led_classdev structure for this device
> + * @init_data: the LED class device initialization data
> + *
> + * Returns: 0 on success or negative error value on failure
> + */
> +extern int led_classdev_register_ext(struct device *parent,
> +				     struct led_classdev *led_cdev,
> +				     struct led_init_data *init_data);
> +#define led_classdev_register(parent, led_cdev)			\
> +	led_classdev_register_ext(parent, led_cdev, NULL)
> +extern int devm_led_classdev_register_ext(struct device *parent,
> +					  struct led_classdev *led_cdev,
> +					  struct led_init_data *init_data);
> +#define devm_led_classdev_register(parent, led_cdev)		\
> +	devm_led_classdev_register_ext(parent, led_cdev, NULL)
>  extern void led_classdev_unregister(struct led_classdev *led_cdev);
>  extern void devm_led_classdev_unregister(struct device *parent,
>  					 struct led_classdev *led_cdev);
> 

-- 
Best regards,
Oleh Kravchenko
Phone: +380972763224 | oleg@kaa.org.ua | Skype: oleg_krava

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 03/25] dt-bindings: leds: Add LED_FUNCTION definitions
  2019-03-10 18:28 ` [PATCH 03/25] dt-bindings: leds: Add LED_FUNCTION definitions Jacek Anaszewski
@ 2019-03-11 12:22   ` Dan Murphy
  2019-03-11 17:22     ` Jacek Anaszewski
  2019-03-28  0:03   ` Rob Herring
  1 sibling, 1 reply; 53+ messages in thread
From: Dan Murphy @ 2019-03-11 12:22 UTC (permalink / raw)
  To: Jacek Anaszewski, linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, Baolin Wang, Daniel Mack,
	Linus Walleij, Oleh Kravchenko, Sakari Ailus, Simon Shields

Jacek

On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
> Add common LED function definitions for use in Device Tree.
> The function names were extracted from existing dts files
> after eliminating oddities.
> 
> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Baolin Wang <baolin.wang@linaro.org>
> Cc: Daniel Mack <daniel@zonque.org>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Simon Shields <simon@lineageos.org>
> ---
>  include/dt-bindings/leds/common.h | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
> index e171d0a6beb2..ffcd46317307 100644
> --- a/include/dt-bindings/leds/common.h
> +++ b/include/dt-bindings/leds/common.h
> @@ -19,4 +19,42 @@
>  #define LEDS_BOOST_ADAPTIVE	1
>  #define LEDS_BOOST_FIXED	2
>  
> +/* Standard LED functions */
> +#define LED_FUNCTION_ACTIVITY "activity"
> +#define LED_FUNCTION_ADSL "adsl"
> +#define LED_FUNCTION_ALARM "alarm"
> +#define LED_FUNCTION_BACKLIGHT "backlight"
> +#define LED_FUNCTION_BLUETOOTH "bluetooth"
> +#define LED_FUNCTION_BOOT "boot"
> +#define LED_FUNCTION_CHRG "chrg"

Why not spell out charge?


> +#define LED_FUNCTION_DEBUG "debug"
> +#define LED_FUNCTION_DISK "disk"
> +#define LED_FUNCTION_DISK_READ "disk-read"
> +#define LED_FUNCTION_DISK_WRITE "disk-write"
> +#define LED_FUNCTION_FAULT "fault"
> +#define LED_FUNCTION_FLASH "flash"
> +#define LED_FUNCTION_HDDERR "hdderr"
> +#define LED_FUNCTION_HEARTBEAT "heartbeat"
> +#define LED_FUNCTION_INDICATOR "indicator"
> +#define LED_FUNCTION_INFO "info"
> +#define LED_FUNCTION_INTERNET "internet"

LEDs function can be for keyboards and keypads.

Keypad for hand held devices

#define LED_FUNCTION_KEYBOARD "keyboard"
#define LED_FUNCTION_KEYPAD "keypad"


> +#define LED_FUNCTION_LAN "lan"
> +#define LED_FUNCTION_MMC "mmc"
> +#define LED_FUNCTION_NAND "nand"
> +#define LED_FUNCTION_ON "on"
> +#define LED_FUNCTION_PROGRAMMING "programming"
> +#define LED_FUNCTION_PWR "pwr"

Same comment as above on chrg

> +#define LED_FUNCTION_RX "rx"
> +#define LED_FUNCTION_SD "sd"
> +#define LED_FUNCTION_SLEEP "sleep"
> +#define LED_FUNCTION_STANDBY "standby"
> +#define LED_FUNCTION_STATUS "status"
> +#define LED_FUNCTION_TORCH "torch"
> +#define LED_FUNCTION_TV "tv"
> +#define LED_FUNCTION_TX "tx"
> +#define LED_FUNCTION_USB "usb"
> +#define LED_FUNCTION_WAN "wan"
> +#define LED_FUNCTION_WLAN "wlan"
> +#define LED_FUNCTION_WPS "wps"
> +

Dan

>  #endif /* __DT_BINDINGS_LEDS_H */
> 


-- 
------------------
Dan Murphy

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 04/25] dt-bindings: leds: Add LED_COLOR_NAME definitions
  2019-03-10 18:28 ` [PATCH 04/25] dt-bindings: leds: Add LED_COLOR_NAME definitions Jacek Anaszewski
@ 2019-03-11 12:23   ` Dan Murphy
  2019-03-11 17:23     ` Jacek Anaszewski
  2019-03-22  8:35   ` Pavel Machek
  2019-03-28  0:08   ` Rob Herring
  2 siblings, 1 reply; 53+ messages in thread
From: Dan Murphy @ 2019-03-11 12:23 UTC (permalink / raw)
  To: Jacek Anaszewski, linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, Baolin Wang, Daniel Mack,
	Linus Walleij, Oleh Kravchenko, Sakari Ailus, Simon Shields

Jacek

On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
> Add common LED color name definitions for use in Device Tree.
> 
> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Baolin Wang <baolin.wang@linaro.org>
> Cc: Daniel Mack <daniel@zonque.org>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Simon Shields <simon@lineageos.org>
> ---
>  include/dt-bindings/leds/common.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
> index ffcd46317307..0e986bb59391 100644
> --- a/include/dt-bindings/leds/common.h
> +++ b/include/dt-bindings/leds/common.h
> @@ -57,4 +57,13 @@
>  #define LED_FUNCTION_WLAN "wlan"
>  #define LED_FUNCTION_WPS "wps"
>  
> +/* Standard LED colors */
> +#define LED_COLOR_NAME_WHITE    "white"
> +#define LED_COLOR_NAME_RED      "red"
> +#define LED_COLOR_NAME_GREEN    "green"
> +#define LED_COLOR_NAME_BLUE     "blue"
> +#define LED_COLOR_NAME_AMBER    "amber"
> +#define LED_COLOR_NAME_VIOLET   "violet"
> +#define LED_COLOR_NAME_YELLOW   "yellow"
> +

#define LED_COLOR_NAME_INFRARED	"ir"

Dan

>  #endif /* __DT_BINDINGS_LEDS_H */
> 


-- 
------------------
Dan Murphy

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 05/25] dt-bindings: leds: Add function and color properties
  2019-03-10 18:28 ` [PATCH 05/25] dt-bindings: leds: Add function and color properties Jacek Anaszewski
@ 2019-03-11 12:26   ` Dan Murphy
  2019-03-11 17:24     ` Jacek Anaszewski
  0 siblings, 1 reply; 53+ messages in thread
From: Dan Murphy @ 2019-03-11 12:26 UTC (permalink / raw)
  To: Jacek Anaszewski, linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, Baolin Wang, Daniel Mack,
	Linus Walleij, Oleh Kravchenko, Sakari Ailus, Simon Shields

On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
> Introduce dedicated properties for conveying information about
> LED function and color. Mark old "label" property as deprecated.
> 
> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Baolin Wang <baolin.wang@linaro.org>
> Cc: Daniel Mack <daniel@zonque.org>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Simon Shields <simon@lineageos.org>
> ---
>  Documentation/devicetree/bindings/leds/common.txt | 55 +++++++++++++++++++----
>  1 file changed, 47 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
> index aa1399814a2a..3402b0e1cec9 100644
> --- a/Documentation/devicetree/bindings/leds/common.txt
> +++ b/Documentation/devicetree/bindings/leds/common.txt
> @@ -10,14 +10,23 @@ can influence the way of the LED device initialization, the LED components
>  have to be tightly coupled with the LED device binding. They are represented
>  by child nodes of the parent LED device binding.
>  
> +
>  Optional properties for child nodes:
>  - led-sources : List of device current outputs the LED is connected to. The
>  		outputs are identified by the numbers that must be defined
>  		in the LED device binding documentation.
> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
> +	    from the header include/dt-bindings/leds/common.h.
> +	    If there is no matching LED_FUNCTION available, add a new one.
> +- color : Color of the LED. Use one of the LED_COLOR_NAME_* prefixed definitions
> +	    from the header include/dt-bindings/leds/common.h.
> +	    If there is no matching LED_COLOR_NAME available, add a new one.
> +

I am assuming multi color can re-use this property as well?

>  - label : The label for this LED. If omitted, the label is taken from the node
>  	  name (excluding the unit address). It has to uniquely identify
>  	  a device, i.e. no other LED class device can be assigned the same
> -	  label.
> +	  label. This property is deprecated - use 'function' and 'color'
> +	  properties instead.
>  
>  - default-state : The initial state of the LED. Valid values are "on", "off",
>    and "keep". If the LED is already on or off and the default-state property is
> @@ -87,29 +96,59 @@ Required properties for trigger source:
>  
>  * Examples
>  
> -gpio-leds {
> +#include <dt-bindings/leds/common.h>
> +
> +led-controller@0 {
>  	compatible = "gpio-leds";
>  
> -	system-status {
> -		label = "Status";
> +	led0 {
> +		function = LED_FUNCTION_STATUS;

Missing color for example unless there is a reason to omit it for GPIO LEDs

Also missing reg here

>  		linux,default-trigger = "heartbeat";
>  		gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
>  	};
>  
> -	usb {
> +	led1 {
> +		function = LED_FUNCTION_USB;

Same as above
Also missing reg here
>  		gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
>  		trigger-sources = <&ohci_port1>, <&ehci_port1>;
>  	};
>  };
>  
> -max77693-led {
> +led-controller@0 {
>  	compatible = "maxim,max77693-led";
>  
> -	camera-flash {
> -		label = "Flash";
> +	led {
> +		function = LED_FUNCTION_FLASH;
> +		color = LED_COLOR_NAME_WHITE;
>  		led-sources = <0>, <1>;
>  		led-max-microamp = <50000>;
>  		flash-max-microamp = <320000>;
>  		flash-max-timeout-us = <500000>;
>  	};
>  };
> +
> +led-controller@30 {
> +        compatible = "panasonic,an30259a";
> +        reg = <0x30>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        led@1 {
> +                reg = <1>;
> +                linux,default-trigger = "heartbeat";
> +                function = LED_FUNCTION_INDICATOR;
> +                color = LED_COLOR_NAME_RED;
> +        };
> +
> +        led@2 {
> +                reg = <2>;
> +                function = LED_FUNCTION_INDICATOR;
> +                color = LED_COLOR_NAME_GREEN;
> +        };
> +
> +        led@3 {
> +                reg = <3>;
> +                function = LED_FUNCTION_INDICATOR;
> +                color = LED_COLOR_NAME_BLUE;
> +        };
> +};
> 


-- 
------------------
Dan Murphy

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 11/25] leds: lp8860: Use led_compose_name()
  2019-03-10 18:28 ` [PATCH 11/25] leds: lp8860: Use led_compose_name() Jacek Anaszewski
@ 2019-03-11 12:28   ` Dan Murphy
  2019-03-11 17:24     ` Jacek Anaszewski
  0 siblings, 1 reply; 53+ messages in thread
From: Dan Murphy @ 2019-03-11 12:28 UTC (permalink / raw)
  To: Jacek Anaszewski, linux-leds; +Cc: devicetree, linux-kernel, pavel, robh

On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
> Switch to using generic LED support for composing LED class
> device name.
> 
> While at it, avoid iterating through available child of nodes
> in favor of obtaining single expected child node using single
> call to of_get_next_available_child().
> 
> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Dan Murphy <dmurphy@ti.com>
> ---
>  drivers/leds/leds-lp8860.c | 38 +++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
> index 39c72a908f3b..7c12ccdc1f2f 100644
> --- a/drivers/leds/leds-lp8860.c
> +++ b/drivers/leds/leds-lp8860.c
> @@ -22,7 +22,6 @@
>  #include <linux/of_gpio.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/slab.h>
> -#include <uapi/linux/uleds.h>
>  
>  #define LP8860_DISP_CL1_BRT_MSB		0x00
>  #define LP8860_DISP_CL1_BRT_LSB		0x01
> @@ -87,6 +86,8 @@
>  
>  #define LP8860_CLEAR_FAULTS		0x01
>  
> +#define LP8860_NAME			"lp8860"
> +
>  /**
>   * struct lp8860_led -
>   * @lock - Lock for reading/writing the device
> @@ -96,7 +97,6 @@
>   * @eeprom_regmap - EEPROM register map
>   * @enable_gpio - VDDIO/EN gpio to enable communication interface
>   * @regulator - LED supply regulator pointer
> - * @label - LED label
>   */
>  struct lp8860_led {
>  	struct mutex lock;
> @@ -106,7 +106,6 @@ struct lp8860_led {
>  	struct regmap *eeprom_regmap;
>  	struct gpio_desc *enable_gpio;
>  	struct regulator *regulator;
> -	char label[LED_MAX_NAME_SIZE];
>  };
>  
>  struct lp8860_eeprom_reg {
> @@ -387,25 +386,26 @@ static int lp8860_probe(struct i2c_client *client,
>  	struct lp8860_led *led;
>  	struct device_node *np = client->dev.of_node;
>  	struct device_node *child_node;
> -	const char *name;
> +	struct led_init_data init_data;
>  
>  	led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL);
>  	if (!led)
>  		return -ENOMEM;
>  
> -	for_each_available_child_of_node(np, child_node) {
> -		led->led_dev.default_trigger = of_get_property(child_node,
> -						    "linux,default-trigger",
> -						    NULL);
> -
> -		ret = of_property_read_string(child_node, "label", &name);
> -		if (!ret)
> -			snprintf(led->label, sizeof(led->label), "%s:%s",
> -				 id->name, name);
> -		else
> -			snprintf(led->label, sizeof(led->label),
> -				"%s::display_cluster", id->name);
> -	}
> +	child_node = of_get_next_available_child(np, NULL);

Again this device has multiple outputs that can be banked or separated.

I would prefer to leave the iteration and just change the name generation.

Dan

> +	if (!child_node)
> +		return -EINVAL;
> +
> +	init_data.fwnode = of_fwnode_handle(child_node),
> +
> +	led->led_dev.default_trigger = of_get_property(child_node,
> +					    "linux,default-trigger",
> +					    NULL);
> +
> +	ret = led_compose_name(init_data.fwnode, LP8860_NAME,
> +			       ":display_cluster", init_data.name);
> +	if (ret)
> +		return ret;
>  
>  	led->enable_gpio = devm_gpiod_get_optional(&client->dev,
>  						   "enable", GPIOD_OUT_LOW);
> @@ -420,7 +420,6 @@ static int lp8860_probe(struct i2c_client *client,
>  		led->regulator = NULL;
>  
>  	led->client = client;
> -	led->led_dev.name = led->label;
>  	led->led_dev.brightness_set_blocking = lp8860_brightness_set;
>  
>  	mutex_init(&led->lock);
> @@ -447,7 +446,8 @@ static int lp8860_probe(struct i2c_client *client,
>  	if (ret)
>  		return ret;
>  
> -	ret = devm_led_classdev_register(&client->dev, &led->led_dev);
> +	ret = devm_led_classdev_register_ext(&client->dev, &led->led_dev,
> +					     &init_data);
>  	if (ret) {
>  		dev_err(&client->dev, "led register err: %d\n", ret);
>  		return ret;
> 


-- 
------------------
Dan Murphy

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 13/25] leds: lm3692x: Use led_compose_name()
  2019-03-10 18:28 ` [PATCH 13/25] leds: lm3692x: Use led_compose_name() Jacek Anaszewski
@ 2019-03-11 12:38   ` Dan Murphy
  2019-03-11 17:24     ` Jacek Anaszewski
  0 siblings, 1 reply; 53+ messages in thread
From: Dan Murphy @ 2019-03-11 12:38 UTC (permalink / raw)
  To: Jacek Anaszewski, linux-leds; +Cc: devicetree, linux-kernel, pavel, robh

On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
> Switch to using generic LED support for composing LED class
> device name.
> 
> Since the same device strings would be used in two places,
> then add macros LM36922_NAME and LM36922_NAME for use in
> lm3692x_probe_dt(() and lm3692x_id array.
> 
> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> Cc: Dan Murphy <dmurphy@ti.com>
> ---
>  drivers/leds/leds-lm3692x.c | 39 ++++++++++++++++++++-------------------
>  1 file changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
> index 4f413a7c5f05..9dfc0f28a9c8 100644
> --- a/drivers/leds/leds-lm3692x.c
> +++ b/drivers/leds/leds-lm3692x.c
> @@ -13,7 +13,6 @@
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
> -#include <uapi/linux/uleds.h>
>  
>  #define LM36922_MODEL	0
>  #define LM36923_MODEL	1
> @@ -95,6 +94,9 @@
>  #define LM3692X_FAULT_FLAG_SHRT BIT(3)
>  #define LM3692X_FAULT_FLAG_OPEN BIT(4)
>  
> +#define LM36922_NAME "lm36922"
> +#define LM36923_NAME "lm36923"
> +
>  /**
>   * struct lm3692x_led -
>   * @lock - Lock for reading/writing the device
> @@ -103,7 +105,6 @@
>   * @regmap - Devices register map
>   * @enable_gpio - VDDIO/EN gpio to enable communication interface
>   * @regulator - LED supply regulator pointer
> - * @label - LED label
>   * @led_enable - LED sync to be enabled
>   * @model_id - Current device model ID enumerated
>   */
> @@ -114,7 +115,6 @@ struct lm3692x_led {
>  	struct regmap *regmap;
>  	struct gpio_desc *enable_gpio;
>  	struct regulator *regulator;
> -	char label[LED_MAX_NAME_SIZE];
>  	int led_enable;
>  	int model_id;
>  };
> @@ -325,7 +325,8 @@ static int lm3692x_init(struct lm3692x_led *led)
>  static int lm3692x_probe_dt(struct lm3692x_led *led)
>  {
>  	struct fwnode_handle *child = NULL;
> -	const char *name;
> +	struct led_init_data init_data;
> +	char *model_name;
>  	int ret;
>  
>  	led->enable_gpio = devm_gpiod_get_optional(&led->client->dev,
> @@ -346,17 +347,20 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
>  		dev_err(&led->client->dev, "No LED Child node\n");
>  		return -ENODEV;
>  	}
> +	init_data.fwnode = child;
>  
> -	fwnode_property_read_string(child, "linux,default-trigger",
> -				    &led->led_dev.default_trigger);
> +	if (led->model_id == LM36922_MODEL)
> +		model_name =  LM36922_NAME;
> +	else
> +		model_name =  LM36923_NAME;
>  


Why do we need this change?

led->client->name should suffice here and then we do not have to #define the name at all.


> -	ret = fwnode_property_read_string(child, "label", &name);
> +	ret = led_compose_name(child, model_name, ":backlight_cluster",
> +			       init_data.name);
>  	if (ret)
> -		snprintf(led->label, sizeof(led->label),
> -			"%s::", led->client->name);
> -	else
> -		snprintf(led->label, sizeof(led->label),
> -			 "%s:%s", led->client->name, name);
> +		return ret;
> +
> +	fwnode_property_read_string(child, "linux,default-trigger",
> +				    &led->led_dev.default_trigger);
>  
>  	ret = fwnode_property_read_u32(child, "reg", &led->led_enable);
>  	if (ret) {
> @@ -364,16 +368,13 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
>  		return ret;
>  	}
>  
> -	led->led_dev.name = led->label;
> -
> -	ret = devm_led_classdev_register(&led->client->dev, &led->led_dev);
> +	ret = devm_led_classdev_register_ext(&led->client->dev, &led->led_dev,
> +					     &init_data);
>  	if (ret) {
>  		dev_err(&led->client->dev, "led register err: %d\n", ret);
>  		return ret;
>  	}
>  
> -	led->led_dev.dev->of_node = to_of_node(child);
> -

Seems like an additional change here not documented.
If it is a bug fix or an enhancement probably need a separate patch.

I do know why this is removed just need tracability on the log.

Dan

>  	return 0;
>  }
>  
> @@ -439,8 +440,8 @@ static int lm3692x_remove(struct i2c_client *client)
>  }
>  
>  static const struct i2c_device_id lm3692x_id[] = {
> -	{ "lm36922", LM36922_MODEL },
> -	{ "lm36923", LM36923_MODEL },
> +	{ LM36922_NAME, LM36922_MODEL },
> +	{ LM36923_NAME, LM36923_MODEL },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, lm3692x_id);
> 


-- 
------------------
Dan Murphy

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 03/25] dt-bindings: leds: Add LED_FUNCTION definitions
  2019-03-11 12:22   ` Dan Murphy
@ 2019-03-11 17:22     ` Jacek Anaszewski
  0 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-11 17:22 UTC (permalink / raw)
  To: Dan Murphy, linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, Baolin Wang, Daniel Mack,
	Linus Walleij, Oleh Kravchenko, Sakari Ailus, Simon Shields

Hi Dan,

Thanks for the review.

On 3/11/19 1:22 PM, Dan Murphy wrote:
> Jacek
> 
> On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
>> Add common LED function definitions for use in Device Tree.
>> The function names were extracted from existing dts files
>> after eliminating oddities.
>>
>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>> Cc: Baolin Wang <baolin.wang@linaro.org>
>> Cc: Daniel Mack <daniel@zonque.org>
>> Cc: Dan Murphy <dmurphy@ti.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>> Cc: Simon Shields <simon@lineageos.org>
>> ---
>>   include/dt-bindings/leds/common.h | 38 ++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 38 insertions(+)
>>
>> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
>> index e171d0a6beb2..ffcd46317307 100644
>> --- a/include/dt-bindings/leds/common.h
>> +++ b/include/dt-bindings/leds/common.h
>> @@ -19,4 +19,42 @@
>>   #define LEDS_BOOST_ADAPTIVE	1
>>   #define LEDS_BOOST_FIXED	2
>>   
>> +/* Standard LED functions */
>> +#define LED_FUNCTION_ACTIVITY "activity"
>> +#define LED_FUNCTION_ADSL "adsl"
>> +#define LED_FUNCTION_ALARM "alarm"
>> +#define LED_FUNCTION_BACKLIGHT "backlight"
>> +#define LED_FUNCTION_BLUETOOTH "bluetooth"
>> +#define LED_FUNCTION_BOOT "boot"
>> +#define LED_FUNCTION_CHRG "chrg"
> 
> Why not spell out charge?

It was in one of existing dts files.
I have no issue with that, will change it to "charge".

> 
>> +#define LED_FUNCTION_DEBUG "debug"
>> +#define LED_FUNCTION_DISK "disk"
>> +#define LED_FUNCTION_DISK_READ "disk-read"
>> +#define LED_FUNCTION_DISK_WRITE "disk-write"
>> +#define LED_FUNCTION_FAULT "fault"
>> +#define LED_FUNCTION_FLASH "flash"
>> +#define LED_FUNCTION_HDDERR "hdderr"
>> +#define LED_FUNCTION_HEARTBEAT "heartbeat"
>> +#define LED_FUNCTION_INDICATOR "indicator"
>> +#define LED_FUNCTION_INFO "info"
>> +#define LED_FUNCTION_INTERNET "internet"
> 
> LEDs function can be for keyboards and keypads.
> 
> Keypad for hand held devices
> 
> #define LED_FUNCTION_KEYBOARD "keyboard"
> #define LED_FUNCTION_KEYPAD "keypad"

Ack.

> 
>> +#define LED_FUNCTION_LAN "lan"
>> +#define LED_FUNCTION_MMC "mmc"
>> +#define LED_FUNCTION_NAND "nand"
>> +#define LED_FUNCTION_ON "on"
>> +#define LED_FUNCTION_PROGRAMMING "programming"
>> +#define LED_FUNCTION_PWR "pwr"
> 
> Same comment as above on chrg

Ack.

>> +#define LED_FUNCTION_RX "rx"
>> +#define LED_FUNCTION_SD "sd"
>> +#define LED_FUNCTION_SLEEP "sleep"
>> +#define LED_FUNCTION_STANDBY "standby"
>> +#define LED_FUNCTION_STATUS "status"
>> +#define LED_FUNCTION_TORCH "torch"
>> +#define LED_FUNCTION_TV "tv"
>> +#define LED_FUNCTION_TX "tx"
>> +#define LED_FUNCTION_USB "usb"
>> +#define LED_FUNCTION_WAN "wan"
>> +#define LED_FUNCTION_WLAN "wlan"
>> +#define LED_FUNCTION_WPS "wps"
>> +
> 
> Dan
> 
>>   #endif /* __DT_BINDINGS_LEDS_H */
>>
> 
> 

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 04/25] dt-bindings: leds: Add LED_COLOR_NAME definitions
  2019-03-11 12:23   ` Dan Murphy
@ 2019-03-11 17:23     ` Jacek Anaszewski
  0 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-11 17:23 UTC (permalink / raw)
  To: Dan Murphy, linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, Baolin Wang, Daniel Mack,
	Linus Walleij, Oleh Kravchenko, Sakari Ailus, Simon Shields

Dan,

On 3/11/19 1:23 PM, Dan Murphy wrote:
> Jacek
> 
> On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
>> Add common LED color name definitions for use in Device Tree.
>>
>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>> Cc: Baolin Wang <baolin.wang@linaro.org>
>> Cc: Daniel Mack <daniel@zonque.org>
>> Cc: Dan Murphy <dmurphy@ti.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>> Cc: Simon Shields <simon@lineageos.org>
>> ---
>>   include/dt-bindings/leds/common.h | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
>> index ffcd46317307..0e986bb59391 100644
>> --- a/include/dt-bindings/leds/common.h
>> +++ b/include/dt-bindings/leds/common.h
>> @@ -57,4 +57,13 @@
>>   #define LED_FUNCTION_WLAN "wlan"
>>   #define LED_FUNCTION_WPS "wps"
>>   
>> +/* Standard LED colors */
>> +#define LED_COLOR_NAME_WHITE    "white"
>> +#define LED_COLOR_NAME_RED      "red"
>> +#define LED_COLOR_NAME_GREEN    "green"
>> +#define LED_COLOR_NAME_BLUE     "blue"
>> +#define LED_COLOR_NAME_AMBER    "amber"
>> +#define LED_COLOR_NAME_VIOLET   "violet"
>> +#define LED_COLOR_NAME_YELLOW   "yellow"
>> +
> 
> #define LED_COLOR_NAME_INFRARED	"ir"

Ack.

> Dan
> 
>>   #endif /* __DT_BINDINGS_LEDS_H */
>>
> 
> 

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 05/25] dt-bindings: leds: Add function and color properties
  2019-03-11 12:26   ` Dan Murphy
@ 2019-03-11 17:24     ` Jacek Anaszewski
  2019-03-12 16:43       ` Jacek Anaszewski
  0 siblings, 1 reply; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-11 17:24 UTC (permalink / raw)
  To: Dan Murphy, linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, Baolin Wang, Daniel Mack,
	Linus Walleij, Oleh Kravchenko, Sakari Ailus, Simon Shields

Dan,

On 3/11/19 1:26 PM, Dan Murphy wrote:
> On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
>> Introduce dedicated properties for conveying information about
>> LED function and color. Mark old "label" property as deprecated.
>>
>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>> Cc: Baolin Wang <baolin.wang@linaro.org>
>> Cc: Daniel Mack <daniel@zonque.org>
>> Cc: Dan Murphy <dmurphy@ti.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>> Cc: Simon Shields <simon@lineageos.org>
>> ---
>>   Documentation/devicetree/bindings/leds/common.txt | 55 +++++++++++++++++++----
>>   1 file changed, 47 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
>> index aa1399814a2a..3402b0e1cec9 100644
>> --- a/Documentation/devicetree/bindings/leds/common.txt
>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>> @@ -10,14 +10,23 @@ can influence the way of the LED device initialization, the LED components
>>   have to be tightly coupled with the LED device binding. They are represented
>>   by child nodes of the parent LED device binding.
>>   
>> +
>>   Optional properties for child nodes:
>>   - led-sources : List of device current outputs the LED is connected to. The
>>   		outputs are identified by the numbers that must be defined
>>   		in the LED device binding documentation.
>> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed definitions
>> +	    from the header include/dt-bindings/leds/common.h.
>> +	    If there is no matching LED_FUNCTION available, add a new one.
>> +- color : Color of the LED. Use one of the LED_COLOR_NAME_* prefixed definitions
>> +	    from the header include/dt-bindings/leds/common.h.
>> +	    If there is no matching LED_COLOR_NAME available, add a new one.
>> +
> 
> I am assuming multi color can re-use this property as well?

I intended it to be a string, but indeed it would be better if we will
make it an integer to be consistent with the discussed LED multi color
design.


>>   - label : The label for this LED. If omitted, the label is taken from the node
>>   	  name (excluding the unit address). It has to uniquely identify
>>   	  a device, i.e. no other LED class device can be assigned the same
>> -	  label.
>> +	  label. This property is deprecated - use 'function' and 'color'
>> +	  properties instead.
>>   
>>   - default-state : The initial state of the LED. Valid values are "on", "off",
>>     and "keep". If the LED is already on or off and the default-state property is
>> @@ -87,29 +96,59 @@ Required properties for trigger source:
>>   
>>   * Examples
>>   
>> -gpio-leds {
>> +#include <dt-bindings/leds/common.h>
>> +
>> +led-controller@0 {
>>   	compatible = "gpio-leds";
>>   
>> -	system-status {
>> -		label = "Status";
>> +	led0 {
>> +		function = LED_FUNCTION_STATUS;
> 
> Missing color for example unless there is a reason to omit it for GPIO LEDs

It is on purpose - to show that it is an optional property for
monochrome LEDs.

> 
> Also missing reg here

Also on purpose. leds-gpio bindings don't require reg here.
And when reg is absent the unit address in the node name should
be omitted as Rob stated in one of recent reviews.

>>   		linux,default-trigger = "heartbeat";
>>   		gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
>>   	};
>>   
>> -	usb {
>> +	led1 {
>> +		function = LED_FUNCTION_USB;
> 
> Same as above
> Also missing reg here
>>   		gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
>>   		trigger-sources = <&ohci_port1>, <&ehci_port1>;
>>   	};
>>   };
>>   
>> -max77693-led {
>> +led-controller@0 {
>>   	compatible = "maxim,max77693-led";
>>   
>> -	camera-flash {
>> -		label = "Flash";
>> +	led {
>> +		function = LED_FUNCTION_FLASH;
>> +		color = LED_COLOR_NAME_WHITE;
>>   		led-sources = <0>, <1>;
>>   		led-max-microamp = <50000>;
>>   		flash-max-microamp = <320000>;
>>   		flash-max-timeout-us = <500000>;
>>   	};
>>   };
>> +
>> +led-controller@30 {
>> +        compatible = "panasonic,an30259a";
>> +        reg = <0x30>;
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        led@1 {
>> +                reg = <1>;
>> +                linux,default-trigger = "heartbeat";
>> +                function = LED_FUNCTION_INDICATOR;
>> +                color = LED_COLOR_NAME_RED;
>> +        };
>> +
>> +        led@2 {
>> +                reg = <2>;
>> +                function = LED_FUNCTION_INDICATOR;
>> +                color = LED_COLOR_NAME_GREEN;
>> +        };
>> +
>> +        led@3 {
>> +                reg = <3>;
>> +                function = LED_FUNCTION_INDICATOR;
>> +                color = LED_COLOR_NAME_BLUE;
>> +        };
>> +};
>>
> 
> 

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 11/25] leds: lp8860: Use led_compose_name()
  2019-03-11 12:28   ` Dan Murphy
@ 2019-03-11 17:24     ` Jacek Anaszewski
  2019-03-11 17:26       ` Dan Murphy
  0 siblings, 1 reply; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-11 17:24 UTC (permalink / raw)
  To: Dan Murphy, linux-leds; +Cc: devicetree, linux-kernel, pavel, robh

Dan,

On 3/11/19 1:28 PM, Dan Murphy wrote:
> On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
>> Switch to using generic LED support for composing LED class
>> device name.
>>
>> While at it, avoid iterating through available child of nodes
>> in favor of obtaining single expected child node using single
>> call to of_get_next_available_child().
>>
>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>> Cc: Dan Murphy <dmurphy@ti.com>
>> ---
>>   drivers/leds/leds-lp8860.c | 38 +++++++++++++++++++-------------------
>>   1 file changed, 19 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
>> index 39c72a908f3b..7c12ccdc1f2f 100644
>> --- a/drivers/leds/leds-lp8860.c
>> +++ b/drivers/leds/leds-lp8860.c
>> @@ -22,7 +22,6 @@
>>   #include <linux/of_gpio.h>
>>   #include <linux/gpio/consumer.h>
>>   #include <linux/slab.h>
>> -#include <uapi/linux/uleds.h>
>>   
>>   #define LP8860_DISP_CL1_BRT_MSB		0x00
>>   #define LP8860_DISP_CL1_BRT_LSB		0x01
>> @@ -87,6 +86,8 @@
>>   
>>   #define LP8860_CLEAR_FAULTS		0x01
>>   
>> +#define LP8860_NAME			"lp8860"
>> +
>>   /**
>>    * struct lp8860_led -
>>    * @lock - Lock for reading/writing the device
>> @@ -96,7 +97,6 @@
>>    * @eeprom_regmap - EEPROM register map
>>    * @enable_gpio - VDDIO/EN gpio to enable communication interface
>>    * @regulator - LED supply regulator pointer
>> - * @label - LED label
>>    */
>>   struct lp8860_led {
>>   	struct mutex lock;
>> @@ -106,7 +106,6 @@ struct lp8860_led {
>>   	struct regmap *eeprom_regmap;
>>   	struct gpio_desc *enable_gpio;
>>   	struct regulator *regulator;
>> -	char label[LED_MAX_NAME_SIZE];
>>   };
>>   
>>   struct lp8860_eeprom_reg {
>> @@ -387,25 +386,26 @@ static int lp8860_probe(struct i2c_client *client,
>>   	struct lp8860_led *led;
>>   	struct device_node *np = client->dev.of_node;
>>   	struct device_node *child_node;
>> -	const char *name;
>> +	struct led_init_data init_data;
>>   
>>   	led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL);
>>   	if (!led)
>>   		return -ENOMEM;
>>   
>> -	for_each_available_child_of_node(np, child_node) {
>> -		led->led_dev.default_trigger = of_get_property(child_node,
>> -						    "linux,default-trigger",
>> -						    NULL);
>> -
>> -		ret = of_property_read_string(child_node, "label", &name);
>> -		if (!ret)
>> -			snprintf(led->label, sizeof(led->label), "%s:%s",
>> -				 id->name, name);
>> -		else
>> -			snprintf(led->label, sizeof(led->label),
>> -				"%s::display_cluster", id->name);
>> -	}
>> +	child_node = of_get_next_available_child(np, NULL);
> 
> Again this device has multiple outputs that can be banked or separated.
> 
> I would prefer to leave the iteration and just change the name generation.

In [0] you agreed on removing that. This code is simply broken since
there is only one instance of struct lp8860_led allocated.

>> +	if (!child_node)
>> +		return -EINVAL;
>> +
>> +	init_data.fwnode = of_fwnode_handle(child_node),
>> +
>> +	led->led_dev.default_trigger = of_get_property(child_node,
>> +					    "linux,default-trigger",
>> +					    NULL);
>> +
>> +	ret = led_compose_name(init_data.fwnode, LP8860_NAME,
>> +			       ":display_cluster", init_data.name);
>> +	if (ret)
>> +		return ret;
>>   
>>   	led->enable_gpio = devm_gpiod_get_optional(&client->dev,
>>   						   "enable", GPIOD_OUT_LOW);
>> @@ -420,7 +420,6 @@ static int lp8860_probe(struct i2c_client *client,
>>   		led->regulator = NULL;
>>   
>>   	led->client = client;
>> -	led->led_dev.name = led->label;
>>   	led->led_dev.brightness_set_blocking = lp8860_brightness_set;
>>   
>>   	mutex_init(&led->lock);
>> @@ -447,7 +446,8 @@ static int lp8860_probe(struct i2c_client *client,
>>   	if (ret)
>>   		return ret;
>>   
>> -	ret = devm_led_classdev_register(&client->dev, &led->led_dev);
>> +	ret = devm_led_classdev_register_ext(&client->dev, &led->led_dev,
>> +					     &init_data);
>>   	if (ret) {
>>   		dev_err(&client->dev, "led register err: %d\n", ret);
>>   		return ret;
>>
> 
> 

[0] https://lkml.org/lkml/2018/11/12/2231

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 13/25] leds: lm3692x: Use led_compose_name()
  2019-03-11 12:38   ` Dan Murphy
@ 2019-03-11 17:24     ` Jacek Anaszewski
  0 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-11 17:24 UTC (permalink / raw)
  To: Dan Murphy, linux-leds; +Cc: devicetree, linux-kernel, pavel, robh

Dan,

On 3/11/19 1:38 PM, Dan Murphy wrote:
> On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
>> Switch to using generic LED support for composing LED class
>> device name.
>>
>> Since the same device strings would be used in two places,
>> then add macros LM36922_NAME and LM36922_NAME for use in
>> lm3692x_probe_dt(() and lm3692x_id array.
>>
>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>> Acked-by: Pavel Machek <pavel@ucw.cz>
>> Cc: Dan Murphy <dmurphy@ti.com>
>> ---
>>   drivers/leds/leds-lm3692x.c | 39 ++++++++++++++++++++-------------------
>>   1 file changed, 20 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
>> index 4f413a7c5f05..9dfc0f28a9c8 100644
>> --- a/drivers/leds/leds-lm3692x.c
>> +++ b/drivers/leds/leds-lm3692x.c
>> @@ -13,7 +13,6 @@
>>   #include <linux/regmap.h>
>>   #include <linux/regulator/consumer.h>
>>   #include <linux/slab.h>
>> -#include <uapi/linux/uleds.h>
>>   
>>   #define LM36922_MODEL	0
>>   #define LM36923_MODEL	1
>> @@ -95,6 +94,9 @@
>>   #define LM3692X_FAULT_FLAG_SHRT BIT(3)
>>   #define LM3692X_FAULT_FLAG_OPEN BIT(4)
>>   
>> +#define LM36922_NAME "lm36922"
>> +#define LM36923_NAME "lm36923"
>> +
>>   /**
>>    * struct lm3692x_led -
>>    * @lock - Lock for reading/writing the device
>> @@ -103,7 +105,6 @@
>>    * @regmap - Devices register map
>>    * @enable_gpio - VDDIO/EN gpio to enable communication interface
>>    * @regulator - LED supply regulator pointer
>> - * @label - LED label
>>    * @led_enable - LED sync to be enabled
>>    * @model_id - Current device model ID enumerated
>>    */
>> @@ -114,7 +115,6 @@ struct lm3692x_led {
>>   	struct regmap *regmap;
>>   	struct gpio_desc *enable_gpio;
>>   	struct regulator *regulator;
>> -	char label[LED_MAX_NAME_SIZE];
>>   	int led_enable;
>>   	int model_id;
>>   };
>> @@ -325,7 +325,8 @@ static int lm3692x_init(struct lm3692x_led *led)
>>   static int lm3692x_probe_dt(struct lm3692x_led *led)
>>   {
>>   	struct fwnode_handle *child = NULL;
>> -	const char *name;
>> +	struct led_init_data init_data;
>> +	char *model_name;
>>   	int ret;
>>   
>>   	led->enable_gpio = devm_gpiod_get_optional(&led->client->dev,
>> @@ -346,17 +347,20 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
>>   		dev_err(&led->client->dev, "No LED Child node\n");
>>   		return -ENODEV;
>>   	}
>> +	init_data.fwnode = child;
>>   
>> -	fwnode_property_read_string(child, "linux,default-trigger",
>> -				    &led->led_dev.default_trigger);
>> +	if (led->model_id == LM36922_MODEL)
>> +		model_name =  LM36922_NAME;
>> +	else
>> +		model_name =  LM36923_NAME;
>>   
> 
> 
> Why do we need this change?
> 
> led->client->name should suffice here and then we do not have to #define the name at all.

Indeed, at that time I was not aware that I2C subsystem creates client
name using product name from DT compatible property.

> 
>> -	ret = fwnode_property_read_string(child, "label", &name);
>> +	ret = led_compose_name(child, model_name, ":backlight_cluster",
>> +			       init_data.name);
>>   	if (ret)
>> -		snprintf(led->label, sizeof(led->label),
>> -			"%s::", led->client->name);
>> -	else
>> -		snprintf(led->label, sizeof(led->label),
>> -			 "%s:%s", led->client->name, name);
>> +		return ret;
>> +
>> +	fwnode_property_read_string(child, "linux,default-trigger",
>> +				    &led->led_dev.default_trigger);
>>   
>>   	ret = fwnode_property_read_u32(child, "reg", &led->led_enable);
>>   	if (ret) {
>> @@ -364,16 +368,13 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
>>   		return ret;
>>   	}
>>   
>> -	led->led_dev.name = led->label;
>> -
>> -	ret = devm_led_classdev_register(&led->client->dev, &led->led_dev);
>> +	ret = devm_led_classdev_register_ext(&led->client->dev, &led->led_dev,
>> +					     &init_data);
>>   	if (ret) {
>>   		dev_err(&led->client->dev, "led register err: %d\n", ret);
>>   		return ret;
>>   	}
>>   
>> -	led->led_dev.dev->of_node = to_of_node(child);
>> -
> 
> Seems like an additional change here not documented.
> If it is a bug fix or an enhancement probably need a separate patch.
> 
> I do know why this is removed just need tracability on the log.

OK, I will add the comment.

>>   	return 0;
>>   }
>>   
>> @@ -439,8 +440,8 @@ static int lm3692x_remove(struct i2c_client *client)
>>   }
>>   
>>   static const struct i2c_device_id lm3692x_id[] = {
>> -	{ "lm36922", LM36922_MODEL },
>> -	{ "lm36923", LM36923_MODEL },
>> +	{ LM36922_NAME, LM36922_MODEL },
>> +	{ LM36923_NAME, LM36923_MODEL },
>>   	{ }
>>   };
>>   MODULE_DEVICE_TABLE(i2c, lm3692x_id);
>>
> 
> 

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 11/25] leds: lp8860: Use led_compose_name()
  2019-03-11 17:24     ` Jacek Anaszewski
@ 2019-03-11 17:26       ` Dan Murphy
  0 siblings, 0 replies; 53+ messages in thread
From: Dan Murphy @ 2019-03-11 17:26 UTC (permalink / raw)
  To: Jacek Anaszewski, linux-leds; +Cc: devicetree, linux-kernel, pavel, robh

Jacek

On 3/11/19 12:24 PM, Jacek Anaszewski wrote:
> Dan,
> 
> On 3/11/19 1:28 PM, Dan Murphy wrote:
>> On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
>>> Switch to using generic LED support for composing LED class
>>> device name.
>>>
>>> While at it, avoid iterating through available child of nodes
>>> in favor of obtaining single expected child node using single
>>> call to of_get_next_available_child().
>>>
>>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>>> Cc: Dan Murphy <dmurphy@ti.com>
>>> ---
>>>   drivers/leds/leds-lp8860.c | 38 +++++++++++++++++++-------------------
>>>   1 file changed, 19 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
>>> index 39c72a908f3b..7c12ccdc1f2f 100644
>>> --- a/drivers/leds/leds-lp8860.c
>>> +++ b/drivers/leds/leds-lp8860.c
>>> @@ -22,7 +22,6 @@
>>>   #include <linux/of_gpio.h>
>>>   #include <linux/gpio/consumer.h>
>>>   #include <linux/slab.h>
>>> -#include <uapi/linux/uleds.h>
>>>     #define LP8860_DISP_CL1_BRT_MSB        0x00
>>>   #define LP8860_DISP_CL1_BRT_LSB        0x01
>>> @@ -87,6 +86,8 @@
>>>     #define LP8860_CLEAR_FAULTS        0x01
>>>   +#define LP8860_NAME            "lp8860"
>>> +
>>>   /**
>>>    * struct lp8860_led -
>>>    * @lock - Lock for reading/writing the device
>>> @@ -96,7 +97,6 @@
>>>    * @eeprom_regmap - EEPROM register map
>>>    * @enable_gpio - VDDIO/EN gpio to enable communication interface
>>>    * @regulator - LED supply regulator pointer
>>> - * @label - LED label
>>>    */
>>>   struct lp8860_led {
>>>       struct mutex lock;
>>> @@ -106,7 +106,6 @@ struct lp8860_led {
>>>       struct regmap *eeprom_regmap;
>>>       struct gpio_desc *enable_gpio;
>>>       struct regulator *regulator;
>>> -    char label[LED_MAX_NAME_SIZE];
>>>   };
>>>     struct lp8860_eeprom_reg {
>>> @@ -387,25 +386,26 @@ static int lp8860_probe(struct i2c_client *client,
>>>       struct lp8860_led *led;
>>>       struct device_node *np = client->dev.of_node;
>>>       struct device_node *child_node;
>>> -    const char *name;
>>> +    struct led_init_data init_data;
>>>         led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL);
>>>       if (!led)
>>>           return -ENOMEM;
>>>   -    for_each_available_child_of_node(np, child_node) {
>>> -        led->led_dev.default_trigger = of_get_property(child_node,
>>> -                            "linux,default-trigger",
>>> -                            NULL);
>>> -
>>> -        ret = of_property_read_string(child_node, "label", &name);
>>> -        if (!ret)
>>> -            snprintf(led->label, sizeof(led->label), "%s:%s",
>>> -                 id->name, name);
>>> -        else
>>> -            snprintf(led->label, sizeof(led->label),
>>> -                "%s::display_cluster", id->name);
>>> -    }
>>> +    child_node = of_get_next_available_child(np, NULL);
>>
>> Again this device has multiple outputs that can be banked or separated.
>>
>> I would prefer to leave the iteration and just change the name generation.
> 
> In [0] you agreed on removing that. This code is simply broken since
> there is only one instance of struct lp8860_led allocated.
> 

Thanks for the reminder on my ack

Dan

>>> +    if (!child_node)
>>> +        return -EINVAL;
>>> +
>>> +    init_data.fwnode = of_fwnode_handle(child_node),
>>> +
>>> +    led->led_dev.default_trigger = of_get_property(child_node,
>>> +                        "linux,default-trigger",
>>> +                        NULL);
>>> +
>>> +    ret = led_compose_name(init_data.fwnode, LP8860_NAME,
>>> +                   ":display_cluster", init_data.name);
>>> +    if (ret)
>>> +        return ret;
>>>         led->enable_gpio = devm_gpiod_get_optional(&client->dev,
>>>                              "enable", GPIOD_OUT_LOW);
>>> @@ -420,7 +420,6 @@ static int lp8860_probe(struct i2c_client *client,
>>>           led->regulator = NULL;
>>>         led->client = client;
>>> -    led->led_dev.name = led->label;
>>>       led->led_dev.brightness_set_blocking = lp8860_brightness_set;
>>>         mutex_init(&led->lock);
>>> @@ -447,7 +446,8 @@ static int lp8860_probe(struct i2c_client *client,
>>>       if (ret)
>>>           return ret;
>>>   -    ret = devm_led_classdev_register(&client->dev, &led->led_dev);
>>> +    ret = devm_led_classdev_register_ext(&client->dev, &led->led_dev,
>>> +                         &init_data);
>>>       if (ret) {
>>>           dev_err(&client->dev, "led register err: %d\n", ret);
>>>           return ret;
>>>
>>
>>
> 
> [0] https://lkml.org/lkml/2018/11/12/2231
> 


-- 
------------------
Dan Murphy

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 05/25] dt-bindings: leds: Add function and color properties
  2019-03-11 17:24     ` Jacek Anaszewski
@ 2019-03-12 16:43       ` Jacek Anaszewski
  2019-03-28  0:23         ` Rob Herring
  0 siblings, 1 reply; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-12 16:43 UTC (permalink / raw)
  To: Dan Murphy, linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, Baolin Wang, Daniel Mack,
	Linus Walleij, Oleh Kravchenko, Sakari Ailus, Simon Shields

On 3/11/19 6:24 PM, Jacek Anaszewski wrote:
> Dan,
> 
> On 3/11/19 1:26 PM, Dan Murphy wrote:
>> On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
>>> Introduce dedicated properties for conveying information about
>>> LED function and color. Mark old "label" property as deprecated.
>>>
>>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>>> Cc: Baolin Wang <baolin.wang@linaro.org>
>>> Cc: Daniel Mack <daniel@zonque.org>
>>> Cc: Dan Murphy <dmurphy@ti.com>
>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
>>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>>> Cc: Simon Shields <simon@lineageos.org>
>>> ---
>>>   Documentation/devicetree/bindings/leds/common.txt | 55 
>>> +++++++++++++++++++----
>>>   1 file changed, 47 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/common.txt 
>>> b/Documentation/devicetree/bindings/leds/common.txt
>>> index aa1399814a2a..3402b0e1cec9 100644
>>> --- a/Documentation/devicetree/bindings/leds/common.txt
>>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>>> @@ -10,14 +10,23 @@ can influence the way of the LED device 
>>> initialization, the LED components
>>>   have to be tightly coupled with the LED device binding. They are 
>>> represented
>>>   by child nodes of the parent LED device binding.
>>> +
>>>   Optional properties for child nodes:
>>>   - led-sources : List of device current outputs the LED is connected 
>>> to. The
>>>           outputs are identified by the numbers that must be defined
>>>           in the LED device binding documentation.
>>> +- function: LED functon. Use one of the LED_FUNCTION_* prefixed 
>>> definitions
>>> +        from the header include/dt-bindings/leds/common.h.
>>> +        If there is no matching LED_FUNCTION available, add a new one.
>>> +- color : Color of the LED. Use one of the LED_COLOR_NAME_* prefixed 
>>> definitions
>>> +        from the header include/dt-bindings/leds/common.h.
>>> +        If there is no matching LED_COLOR_NAME available, add a new 
>>> one.
>>> +
>>
>> I am assuming multi color can re-use this property as well?
> 
> I intended it to be a string, but indeed it would be better if we will
> make it an integer to be consistent with the discussed LED multi color
> design.

Going further, I wonder if it would make sense to make function also
an integer. This way we could enforce use of LED functions known
to kernel.

Additionally, if we introduced a property like function-enum-val
(better name suggestions are welcome), then we could easily solve the
problem of string concatenation limitation in dtc compiler mentioned
in the cover letter, since the concatenation would be done in the LED core.

>>>   - label : The label for this LED. If omitted, the label is taken 
>>> from the node
>>>         name (excluding the unit address). It has to uniquely identify
>>>         a device, i.e. no other LED class device can be assigned the 
>>> same
>>> -      label.
>>> +      label. This property is deprecated - use 'function' and 'color'
>>> +      properties instead.
>>>   - default-state : The initial state of the LED. Valid values are 
>>> "on", "off",
>>>     and "keep". If the LED is already on or off and the default-state 
>>> property is
>>> @@ -87,29 +96,59 @@ Required properties for trigger source:
>>>   * Examples
>>> -gpio-leds {
>>> +#include <dt-bindings/leds/common.h>
>>> +
>>> +led-controller@0 {
>>>       compatible = "gpio-leds";
>>> -    system-status {
>>> -        label = "Status";
>>> +    led0 {
>>> +        function = LED_FUNCTION_STATUS;
>>
>> Missing color for example unless there is a reason to omit it for GPIO 
>> LEDs
> 
> It is on purpose - to show that it is an optional property for
> monochrome LEDs.
> 
>>
>> Also missing reg here
> 
> Also on purpose. leds-gpio bindings don't require reg here.
> And when reg is absent the unit address in the node name should
> be omitted as Rob stated in one of recent reviews.
> 
>>>           linux,default-trigger = "heartbeat";
>>>           gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
>>>       };
>>> -    usb {
>>> +    led1 {
>>> +        function = LED_FUNCTION_USB;
>>
>> Same as above
>> Also missing reg here
>>>           gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
>>>           trigger-sources = <&ohci_port1>, <&ehci_port1>;
>>>       };
>>>   };
>>> -max77693-led {
>>> +led-controller@0 {
>>>       compatible = "maxim,max77693-led";
>>> -    camera-flash {
>>> -        label = "Flash";
>>> +    led {
>>> +        function = LED_FUNCTION_FLASH;
>>> +        color = LED_COLOR_NAME_WHITE;
>>>           led-sources = <0>, <1>;
>>>           led-max-microamp = <50000>;
>>>           flash-max-microamp = <320000>;
>>>           flash-max-timeout-us = <500000>;
>>>       };
>>>   };
>>> +
>>> +led-controller@30 {
>>> +        compatible = "panasonic,an30259a";
>>> +        reg = <0x30>;
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +
>>> +        led@1 {
>>> +                reg = <1>;
>>> +                linux,default-trigger = "heartbeat";
>>> +                function = LED_FUNCTION_INDICATOR;
>>> +                color = LED_COLOR_NAME_RED;
>>> +        };
>>> +
>>> +        led@2 {
>>> +                reg = <2>;
>>> +                function = LED_FUNCTION_INDICATOR;
>>> +                color = LED_COLOR_NAME_GREEN;
>>> +        };
>>> +
>>> +        led@3 {
>>> +                reg = <3>;
>>> +                function = LED_FUNCTION_INDICATOR;
>>> +                color = LED_COLOR_NAME_BLUE;
>>> +        };
>>> +};
>>>
>>
>>
> 

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 02/25] leds: core: Add support for composing LED class device names
  2019-03-10 18:28 ` [PATCH 02/25] leds: core: Add support for composing LED class device names Jacek Anaszewski
@ 2019-03-12 16:56   ` Jacek Anaszewski
  2019-03-12 17:15   ` Dan Murphy
  1 sibling, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-12 16:56 UTC (permalink / raw)
  To: linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, Baolin Wang, Daniel Mack,
	Dan Murphy, Linus Walleij, Oleh Kravchenko, Sakari Ailus

Two auto corrections:

On 3/10/19 7:28 PM, Jacek Anaszewski wrote:
> Add public led_compose_name() API for composing LED class device
> name basing on fwnode_handle data. The function composes device name
> according to either a new <color:function> pattern or the legacy
> <devicename:color:function> pattern. The decision on using the
> particular pattern is made basing on whether fwnode contains new
> "function" and "color" properties, or the legacy "label" proeprty.
> 
> Backwards compatibility with in-driver hard-coded LED class device
> names is assured thanks to the default_desc argument.
> 
> In case none of the aforementioned properties was found, then, for OF
> nodes, the node name is adopted for LED class device name.
> 
> Alongside these changes added is a new tool - tools/leds/get_led_device_info.sh.
> The tool allows retrieving details of a LED class device's parent device,
> which proves that getting rid of a devicename section from LED name pattern
> is justified since this information is already available in sysfs.
> 
> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Baolin Wang <baolin.wang@linaro.org>
> Cc: Daniel Mack <daniel@zonque.org>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>   Documentation/leds/leds-class.txt | 20 +++++++++-
>   drivers/leds/led-core.c           | 82 +++++++++++++++++++++++++++++++++++++++
>   include/linux/leds.h              | 31 +++++++++++++++
>   tools/leds/get_led_device_info.sh | 81 ++++++++++++++++++++++++++++++++++++++
>   4 files changed, 213 insertions(+), 1 deletion(-)
>   create mode 100755 tools/leds/get_led_device_info.sh
> 
> diff --git a/Documentation/leds/leds-class.txt b/Documentation/leds/leds-class.txt
> index 8b39cc6b03ee..866fe87063d4 100644
> --- a/Documentation/leds/leds-class.txt
> +++ b/Documentation/leds/leds-class.txt
> @@ -43,7 +43,22 @@ LED Device Naming
>   
>   Is currently of the form:
>   
> -"devicename:colour:function"
> +"colour:function"
> +
> +There might be still LED class drivers around using "devicename:colour:function"
> +naming pattern, but the "devicename" section is now deprecated since it used
> +to convey information that was already available in the sysfs, like product
> +name. There is a tool (tools/leds/get_led_device_info.sh) available for
> +retrieving that information per a LED class device.
> +
> +Associations with other devices, like network ones, should be defined
> +via LED triggr mechanism. This approach is applied by some of wireless

s/triggr/trigger/

> +network drivers that create triggers dynamically and incorporate phy
> +name into its name. On the other hand input subsystem offers LED - input

s/its name/the trigger name/

> +bridge (drivers/input/input-leds.c) for exposing keyboard LEDs as LED class
> +devices. The get_led_device_info.sh script has support for retrieving related
> +input device node name. Should it support discovery of associations between
> +LEDs and other subsystems, please don't hesitate to submit a relevant patch.
>   
>   There have been calls for LED properties such as colour to be exported as
>   individual led class attributes. As a solution which doesn't incur as much
> @@ -51,6 +66,9 @@ overhead, I suggest these become part of the device name. The naming scheme
>   above leaves scope for further attributes should they be needed. If sections
>   of the name don't apply, just leave that section blank.
>   
> +Please also keep in mind that LED subsystem has a protection against LED name
> +conflict. It adds numerical suffix (e.g. "_1", "_2", "_3" etc.) to the requested
> +LED class device name in case it is already in use.


-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 02/25] leds: core: Add support for composing LED class device names
  2019-03-10 18:28 ` [PATCH 02/25] leds: core: Add support for composing LED class device names Jacek Anaszewski
  2019-03-12 16:56   ` Jacek Anaszewski
@ 2019-03-12 17:15   ` Dan Murphy
  2019-03-12 17:28     ` Jacek Anaszewski
  1 sibling, 1 reply; 53+ messages in thread
From: Dan Murphy @ 2019-03-12 17:15 UTC (permalink / raw)
  To: Jacek Anaszewski, linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, Baolin Wang, Daniel Mack,
	Linus Walleij, Oleh Kravchenko, Sakari Ailus

On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
> Add public led_compose_name() API for composing LED class device
> name basing on fwnode_handle data. The function composes device name
> according to either a new <color:function> pattern or the legacy
> <devicename:color:function> pattern. The decision on using the
> particular pattern is made basing on whether fwnode contains new
> "function" and "color" properties, or the legacy "label" proeprty.
> 
> Backwards compatibility with in-driver hard-coded LED class device
> names is assured thanks to the default_desc argument.
> 
> In case none of the aforementioned properties was found, then, for OF
> nodes, the node name is adopted for LED class device name.
> 
> Alongside these changes added is a new tool - tools/leds/get_led_device_info.sh.
> The tool allows retrieving details of a LED class device's parent device,
> which proves that getting rid of a devicename section from LED name pattern
> is justified since this information is already available in sysfs.
> 
> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Baolin Wang <baolin.wang@linaro.org>
> Cc: Daniel Mack <daniel@zonque.org>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  Documentation/leds/leds-class.txt | 20 +++++++++-
>  drivers/leds/led-core.c           | 82 +++++++++++++++++++++++++++++++++++++++
>  include/linux/leds.h              | 31 +++++++++++++++
>  tools/leds/get_led_device_info.sh | 81 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 213 insertions(+), 1 deletion(-)
>  create mode 100755 tools/leds/get_led_device_info.sh
> 
> diff --git a/Documentation/leds/leds-class.txt b/Documentation/leds/leds-class.txt
> index 8b39cc6b03ee..866fe87063d4 100644
> --- a/Documentation/leds/leds-class.txt
> +++ b/Documentation/leds/leds-class.txt
> @@ -43,7 +43,22 @@ LED Device Naming
>  
>  Is currently of the form:
>  
> -"devicename:colour:function"
> +"colour:function"
> +
> +There might be still LED class drivers around using "devicename:colour:function"
> +naming pattern, but the "devicename" section is now deprecated since it used
> +to convey information that was already available in the sysfs, like product
> +name. There is a tool (tools/leds/get_led_device_info.sh) available for
> +retrieving that information per a LED class device.
> +
> +Associations with other devices, like network ones, should be defined
> +via LED triggr mechanism. This approach is applied by some of wireless
> +network drivers that create triggers dynamically and incorporate phy
> +name into its name. On the other hand input subsystem offers LED - input
> +bridge (drivers/input/input-leds.c) for exposing keyboard LEDs as LED class
> +devices. The get_led_device_info.sh script has support for retrieving related
> +input device node name. Should it support discovery of associations between
> +LEDs and other subsystems, please don't hesitate to submit a relevant patch.
>  
>  There have been calls for LED properties such as colour to be exported as
>  individual led class attributes. As a solution which doesn't incur as much
> @@ -51,6 +66,9 @@ overhead, I suggest these become part of the device name. The naming scheme
>  above leaves scope for further attributes should they be needed. If sections
>  of the name don't apply, just leave that section blank.
>  
> +Please also keep in mind that LED subsystem has a protection against LED name
> +conflict. It adds numerical suffix (e.g. "_1", "_2", "_3" etc.) to the requested
> +LED class device name in case it is already in use.
>  
>  Brightness setting API
>  ======================
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index ede4fa0ac2cc..bad92250d1d5 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -16,6 +16,8 @@
>  #include <linux/list.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/property.h>
>  #include <linux/rwsem.h>
>  #include "leds.h"
>  
> @@ -327,3 +329,83 @@ void led_sysfs_enable(struct led_classdev *led_cdev)
>  	led_cdev->flags &= ~LED_SYSFS_DISABLE;
>  }
>  EXPORT_SYMBOL_GPL(led_sysfs_enable);
> +
> +static void led_parse_properties(struct fwnode_handle *fwnode,
> +				 struct led_properties *props)
> +{
> +	int ret;
> +
> +	if (!fwnode)
> +		return;
> +
> +	if (fwnode_property_present(fwnode, "label")) {
> +		ret = fwnode_property_read_string(fwnode, "label", &props->label);
> +		if (ret)
> +			pr_err("Error parsing \'label\' property (%d)\n", ret);
> +		return;
> +	}
> +
> +	if (fwnode_property_present(fwnode, "function")) {
> +		ret = fwnode_property_read_string(fwnode, "function", &props->function);
> +		if (ret)
> +			pr_err("Error parsing \'function\' property (%d)\n", ret);
> +	} else {
> +		pr_info("\'function\' property not found\n");
> +	}
> +
> +	if (fwnode_property_present(fwnode, "color")) {
> +		ret = fwnode_property_read_string(fwnode, "color", &props->color);
> +		if (ret)
> +			pr_info("Error parsing \'color\' property (%d)\n", ret);
> +	} else {
> +		pr_info("\'color\' property not found\n");
> +	}
> +}
> +
> +int led_compose_name(struct fwnode_handle *fwnode, const char *led_hw_name,
> +		     const char *default_desc, char *led_classdev_name)
> +{
> +	struct led_properties props = {};
> +
> +	if (!led_classdev_name)
> +		return -EINVAL;
> +
> +	led_parse_properties(fwnode, &props);
> +
> +	if (props.label) {
> +		/*
> +		 * Presence of 'label' DT property implies legacy LED name,
> +		 * formatted as <devicename:color:function>, with possible
> +		 * section omission if doesn't apply to given device.
> +		 *
> +		 * If no led_hw_name has been passed, then it indicates that
> +		 * DT label should be used as-is for LED class device name.
> +		 * Otherwise the label is prepended with led_hw_name to compose
> +		 * the final LED class device name.
> +		 */
> +		if (!led_hw_name) {
> +			strncpy(led_classdev_name, props.label,
> +				LED_MAX_NAME_SIZE);
> +		} else {
> +			snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
> +				 led_hw_name, props.label);
> +		}
> +	} else if (props.function || props.color) {
> +		snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
> +			 props.color ?: "", props.function ?: "");
> +	} else if (default_desc) {
> +		if (!led_hw_name) {
> +			pr_err("Legacy LED naming requires devicename segment");
> +			return -EINVAL;
> +		}
> +		snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
> +			 led_hw_name, default_desc);
> +	} else if (is_of_node(fwnode)) {
> +		strncpy(led_classdev_name, to_of_node(fwnode)->name,
> +			LED_MAX_NAME_SIZE);
> +	} else
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(led_compose_name);
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index bffb4315fd66..c2936fc989d4 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -252,6 +252,31 @@ extern void led_sysfs_disable(struct led_classdev *led_cdev);
>  extern void led_sysfs_enable(struct led_classdev *led_cdev);
>  
>  /**
> + * led_compose_name - compose LED class device name
> + * @child: child fwnode_handle describing a LED,
> + *	   or a group of synchronized LEDs.
> + * @led_hw_name: name of the LED controller, used when falling back to legacy
> + *		 LED naming; it should be set to NULL in new LED class drivers
> + * @default_desc: default <color:function> tuple, for backwards compatibility
> + *		  with in-driver hard-coded LED names used as a fallback when
> + *		  "label" DT property is absent; it should be set to NULL
> + *		  in new LED class drivers
> + * @led_classdev_name: composed LED class device name
> + *
> + * Create LED class device name basing on the configuration provided by the
> + * board firmware. The name can have a legacy form <devicename:color:function>,
> + * or a new form <color:function>. The latter is chosen if "label" property is
> + * absent and at least one of "color" or "function" is present in the fwnode,
> + * leaving the section blank if the related property is absent. In case none
> + * of the aforementioned properties is found, then, for OF nodes, the node name
> + * is adopted for LED class device name.
> + *
> + * Returns: 0 on success or negative error value on failure
> + */
> +extern int led_compose_name(struct fwnode_handle *child, const char *led_hw_name,
> +			    const char *default_desc, char *led_classdev_name);
> +
> +/**
>   * led_sysfs_is_disabled - check if LED sysfs interface is disabled
>   * @led_cdev: the LED to query
>   *
> @@ -428,6 +453,12 @@ struct led_platform_data {
>  	struct led_info	*leds;
>  };
>  
> +struct led_properties {
> +	const char *color;
> +	const char *function;
> +	const char *label;
> +};
> +
>  struct gpio_desc;
>  typedef int (*gpio_blink_set_t)(struct gpio_desc *desc, int state,
>  				unsigned long *delay_on,
> diff --git a/tools/leds/get_led_device_info.sh b/tools/leds/get_led_device_info.sh
> new file mode 100755
> index 000000000000..4671aa690e4a
> --- /dev/null
> +++ b/tools/leds/get_led_device_info.sh
> @@ -0,0 +1,81 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +

Is there a way to give usage or help here?  It's not entirely clear what the argument to pass in is.

maybe if $1 = "?" then print usage

Dan


> +if [ $# -ne 1 ]; then
> +	echo "get_led_devicename.sh LED_CDEV_PATH"
> +	exit 1
> +fi
> +
> +led_cdev_path=`echo $1 | sed s'/\/$//'`
> +
> +ls "$led_cdev_path/brightness" > /dev/null 2>&1
> +if [ $? -ne 0 ]; then
> +	echo "Device \"$led_cdev_path\" does not exist."
> +	exit 1
> +fi
> +
> +bus=`readlink $led_cdev_path/device/subsystem | sed s'/.*\///'`
> +usb_subdev=`readlink $led_cdev_path | grep usb | sed s'/\(.*usb[0-9]*\/[0-9]*-[0-9]*\)\/.*/\1/'`
> +ls "$led_cdev_path/device/of_node/compatible" > /dev/null 2>&1
> +of_node_missing=$?
> +
> +if [ "$bus" = "input" ]; then
> +	input_node=`readlink $led_cdev_path/device | sed s'/.*\///'`
> +	if [ ! -z $usb_subdev ]; then
> +		bus="usb"
> +	fi
> +fi
> +
> +if [ "$bus" = "usb" ]; then
> +	usb_interface=`readlink $led_cdev_path | sed s'/.*\(usb[0-9]*\)/\1/' | cut -d \/ -f 3`
> +	driver=`readlink $usb_interface/driver | sed s'/.*\///'`
> +	cd $led_cdev_path/../$usb_subdev
> +	idVendor=`cat idVendor`
> +	idProduct=`cat idProduct`
> +	manufacturer=`cat manufacturer`
> +	product=`cat product`
> +elif [ "$bus" = "input" ]; then
> +	cd $led_cdev_path
> +	product=`cat device/name`
> +	driver=`cat device/device/driver/description`
> +elif [ $of_node_missing -eq 0 ]; then
> +	cd $led_cdev_path
> +	compatible=`cat device/of_node/compatible`
> +	if [ "$compatible" = "gpio-leds" ]; then
> +		driver="leds-gpio"
> +	elif [ "$compatible" = "pwm-leds" ]; then
> +		driver="leds-pwm"
> +	else
> +		manufacturer=`echo $compatible | cut -d, -f1`
> +		product=`echo $compatible | cut -d, -f2`
> +	fi
> +else
> +	echo "Unknown device type."
> +	exit 1
> +fi
> +
> +printf "bus:\t\t\t$bus\n"
> +
> +if [ ! -z "$idVendor" ]; then
> +	printf "idVendor:\t\t$idVendor\n"
> +fi
> +
> +if [ ! -z "$idProduct" ]; then
> +	printf "idProduct:\t\t$idProduct\n"
> +fi
> +
> +if [ ! -z "$manufacturer" ]; then
> +	printf "manufacturer:\t\t$manufacturer\n"
> +fi
> +
> +if [ ! -z "$product" ]; then
> +	printf "product:\t\t$product\n"
> +fi
> +
> +if [ ! -z "$driver" ]; then
> +	printf "driver:\t\t\t$driver\n"
> +fi
> +
> +if [ ! -z "$input_node" ]; then
> +	printf "associated input node:\t$input_node\n"
> +fi
> 


-- 
------------------
Dan Murphy

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 02/25] leds: core: Add support for composing LED class device names
  2019-03-12 17:15   ` Dan Murphy
@ 2019-03-12 17:28     ` Jacek Anaszewski
  2019-03-12 17:46       ` Dan Murphy
  0 siblings, 1 reply; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-12 17:28 UTC (permalink / raw)
  To: Dan Murphy, linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, Baolin Wang, Daniel Mack,
	Linus Walleij, Oleh Kravchenko, Sakari Ailus

Hi Dan,

On 3/12/19 6:15 PM, Dan Murphy wrote:
> On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
>> Add public led_compose_name() API for composing LED class device
>> name basing on fwnode_handle data. The function composes device name
>> according to either a new <color:function> pattern or the legacy
>> <devicename:color:function> pattern. The decision on using the
>> particular pattern is made basing on whether fwnode contains new
>> "function" and "color" properties, or the legacy "label" proeprty.
>>
>> Backwards compatibility with in-driver hard-coded LED class device
>> names is assured thanks to the default_desc argument.
>>
>> In case none of the aforementioned properties was found, then, for OF
>> nodes, the node name is adopted for LED class device name.
>>
>> Alongside these changes added is a new tool - tools/leds/get_led_device_info.sh.
>> The tool allows retrieving details of a LED class device's parent device,
>> which proves that getting rid of a devicename section from LED name pattern
>> is justified since this information is already available in sysfs.
>>
>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>> Cc: Baolin Wang <baolin.wang@linaro.org>
>> Cc: Daniel Mack <daniel@zonque.org>
>> Cc: Dan Murphy <dmurphy@ti.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>> ---
>>   Documentation/leds/leds-class.txt | 20 +++++++++-
>>   drivers/leds/led-core.c           | 82 +++++++++++++++++++++++++++++++++++++++
>>   include/linux/leds.h              | 31 +++++++++++++++
>>   tools/leds/get_led_device_info.sh | 81 ++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 213 insertions(+), 1 deletion(-)
>>   create mode 100755 tools/leds/get_led_device_info.sh
>>
>> diff --git a/Documentation/leds/leds-class.txt b/Documentation/leds/leds-class.txt
>> index 8b39cc6b03ee..866fe87063d4 100644
>> --- a/Documentation/leds/leds-class.txt
>> +++ b/Documentation/leds/leds-class.txt
>> @@ -43,7 +43,22 @@ LED Device Naming
>>   
>>   Is currently of the form:
>>   
>> -"devicename:colour:function"
>> +"colour:function"
>> +
>> +There might be still LED class drivers around using "devicename:colour:function"
>> +naming pattern, but the "devicename" section is now deprecated since it used
>> +to convey information that was already available in the sysfs, like product
>> +name. There is a tool (tools/leds/get_led_device_info.sh) available for
>> +retrieving that information per a LED class device.
>> +
>> +Associations with other devices, like network ones, should be defined
>> +via LED triggr mechanism. This approach is applied by some of wireless
>> +network drivers that create triggers dynamically and incorporate phy
>> +name into its name. On the other hand input subsystem offers LED - input
>> +bridge (drivers/input/input-leds.c) for exposing keyboard LEDs as LED class
>> +devices. The get_led_device_info.sh script has support for retrieving related
>> +input device node name. Should it support discovery of associations between
>> +LEDs and other subsystems, please don't hesitate to submit a relevant patch.
>>   
>>   There have been calls for LED properties such as colour to be exported as
>>   individual led class attributes. As a solution which doesn't incur as much
>> @@ -51,6 +66,9 @@ overhead, I suggest these become part of the device name. The naming scheme
>>   above leaves scope for further attributes should they be needed. If sections
>>   of the name don't apply, just leave that section blank.
>>   
>> +Please also keep in mind that LED subsystem has a protection against LED name
>> +conflict. It adds numerical suffix (e.g. "_1", "_2", "_3" etc.) to the requested
>> +LED class device name in case it is already in use.
>>   
>>   Brightness setting API
>>   ======================
>> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
>> index ede4fa0ac2cc..bad92250d1d5 100644
>> --- a/drivers/leds/led-core.c
>> +++ b/drivers/leds/led-core.c
>> @@ -16,6 +16,8 @@
>>   #include <linux/list.h>
>>   #include <linux/module.h>
>>   #include <linux/mutex.h>
>> +#include <linux/of.h>
>> +#include <linux/property.h>
>>   #include <linux/rwsem.h>
>>   #include "leds.h"
>>   
>> @@ -327,3 +329,83 @@ void led_sysfs_enable(struct led_classdev *led_cdev)
>>   	led_cdev->flags &= ~LED_SYSFS_DISABLE;
>>   }
>>   EXPORT_SYMBOL_GPL(led_sysfs_enable);
>> +
>> +static void led_parse_properties(struct fwnode_handle *fwnode,
>> +				 struct led_properties *props)
>> +{
>> +	int ret;
>> +
>> +	if (!fwnode)
>> +		return;
>> +
>> +	if (fwnode_property_present(fwnode, "label")) {
>> +		ret = fwnode_property_read_string(fwnode, "label", &props->label);
>> +		if (ret)
>> +			pr_err("Error parsing \'label\' property (%d)\n", ret);
>> +		return;
>> +	}
>> +
>> +	if (fwnode_property_present(fwnode, "function")) {
>> +		ret = fwnode_property_read_string(fwnode, "function", &props->function);
>> +		if (ret)
>> +			pr_err("Error parsing \'function\' property (%d)\n", ret);
>> +	} else {
>> +		pr_info("\'function\' property not found\n");
>> +	}
>> +
>> +	if (fwnode_property_present(fwnode, "color")) {
>> +		ret = fwnode_property_read_string(fwnode, "color", &props->color);
>> +		if (ret)
>> +			pr_info("Error parsing \'color\' property (%d)\n", ret);
>> +	} else {
>> +		pr_info("\'color\' property not found\n");
>> +	}
>> +}
>> +
>> +int led_compose_name(struct fwnode_handle *fwnode, const char *led_hw_name,
>> +		     const char *default_desc, char *led_classdev_name)
>> +{
>> +	struct led_properties props = {};
>> +
>> +	if (!led_classdev_name)
>> +		return -EINVAL;
>> +
>> +	led_parse_properties(fwnode, &props);
>> +
>> +	if (props.label) {
>> +		/*
>> +		 * Presence of 'label' DT property implies legacy LED name,
>> +		 * formatted as <devicename:color:function>, with possible
>> +		 * section omission if doesn't apply to given device.
>> +		 *
>> +		 * If no led_hw_name has been passed, then it indicates that
>> +		 * DT label should be used as-is for LED class device name.
>> +		 * Otherwise the label is prepended with led_hw_name to compose
>> +		 * the final LED class device name.
>> +		 */
>> +		if (!led_hw_name) {
>> +			strncpy(led_classdev_name, props.label,
>> +				LED_MAX_NAME_SIZE);
>> +		} else {
>> +			snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
>> +				 led_hw_name, props.label);
>> +		}
>> +	} else if (props.function || props.color) {
>> +		snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
>> +			 props.color ?: "", props.function ?: "");
>> +	} else if (default_desc) {
>> +		if (!led_hw_name) {
>> +			pr_err("Legacy LED naming requires devicename segment");
>> +			return -EINVAL;
>> +		}
>> +		snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
>> +			 led_hw_name, default_desc);
>> +	} else if (is_of_node(fwnode)) {
>> +		strncpy(led_classdev_name, to_of_node(fwnode)->name,
>> +			LED_MAX_NAME_SIZE);
>> +	} else
>> +		return -EINVAL;
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(led_compose_name);
>> diff --git a/include/linux/leds.h b/include/linux/leds.h
>> index bffb4315fd66..c2936fc989d4 100644
>> --- a/include/linux/leds.h
>> +++ b/include/linux/leds.h
>> @@ -252,6 +252,31 @@ extern void led_sysfs_disable(struct led_classdev *led_cdev);
>>   extern void led_sysfs_enable(struct led_classdev *led_cdev);
>>   
>>   /**
>> + * led_compose_name - compose LED class device name
>> + * @child: child fwnode_handle describing a LED,
>> + *	   or a group of synchronized LEDs.
>> + * @led_hw_name: name of the LED controller, used when falling back to legacy
>> + *		 LED naming; it should be set to NULL in new LED class drivers
>> + * @default_desc: default <color:function> tuple, for backwards compatibility
>> + *		  with in-driver hard-coded LED names used as a fallback when
>> + *		  "label" DT property is absent; it should be set to NULL
>> + *		  in new LED class drivers
>> + * @led_classdev_name: composed LED class device name
>> + *
>> + * Create LED class device name basing on the configuration provided by the
>> + * board firmware. The name can have a legacy form <devicename:color:function>,
>> + * or a new form <color:function>. The latter is chosen if "label" property is
>> + * absent and at least one of "color" or "function" is present in the fwnode,
>> + * leaving the section blank if the related property is absent. In case none
>> + * of the aforementioned properties is found, then, for OF nodes, the node name
>> + * is adopted for LED class device name.
>> + *
>> + * Returns: 0 on success or negative error value on failure
>> + */
>> +extern int led_compose_name(struct fwnode_handle *child, const char *led_hw_name,
>> +			    const char *default_desc, char *led_classdev_name);
>> +
>> +/**
>>    * led_sysfs_is_disabled - check if LED sysfs interface is disabled
>>    * @led_cdev: the LED to query
>>    *
>> @@ -428,6 +453,12 @@ struct led_platform_data {
>>   	struct led_info	*leds;
>>   };
>>   
>> +struct led_properties {
>> +	const char *color;
>> +	const char *function;
>> +	const char *label;
>> +};
>> +
>>   struct gpio_desc;
>>   typedef int (*gpio_blink_set_t)(struct gpio_desc *desc, int state,
>>   				unsigned long *delay_on,
>> diff --git a/tools/leds/get_led_device_info.sh b/tools/leds/get_led_device_info.sh
>> new file mode 100755
>> index 000000000000..4671aa690e4a
>> --- /dev/null
>> +++ b/tools/leds/get_led_device_info.sh
>> @@ -0,0 +1,81 @@
>> +#!/bin/sh
>> +# SPDX-License-Identifier: GPL-2.0
>> +
> 
> Is there a way to give usage or help here?  It's not entirely clear what the argument to pass in is.

It is in the first statement of this script - see below.
It is customary to print help when unexpected arguments or a number
thereof is given.

I can print help also when "--help" is passed.

> maybe if $1 = "?" then print usage
> 
> Dan
> 
> 
>> +if [ $# -ne 1 ]; then
>> +	echo "get_led_devicename.sh LED_CDEV_PATH"

s/get_led_devicename/get_led_device_info/

It is a leftover from earlier stage of development.

>> +	exit 1
>> +fi
>> +
>> +led_cdev_path=`echo $1 | sed s'/\/$//'`
>> +
>> +ls "$led_cdev_path/brightness" > /dev/null 2>&1
>> +if [ $? -ne 0 ]; then
>> +	echo "Device \"$led_cdev_path\" does not exist."
>> +	exit 1
>> +fi
>> +
>> +bus=`readlink $led_cdev_path/device/subsystem | sed s'/.*\///'`
>> +usb_subdev=`readlink $led_cdev_path | grep usb | sed s'/\(.*usb[0-9]*\/[0-9]*-[0-9]*\)\/.*/\1/'`
>> +ls "$led_cdev_path/device/of_node/compatible" > /dev/null 2>&1
>> +of_node_missing=$?
>> +
>> +if [ "$bus" = "input" ]; then
>> +	input_node=`readlink $led_cdev_path/device | sed s'/.*\///'`
>> +	if [ ! -z $usb_subdev ]; then
>> +		bus="usb"
>> +	fi
>> +fi
>> +
>> +if [ "$bus" = "usb" ]; then
>> +	usb_interface=`readlink $led_cdev_path | sed s'/.*\(usb[0-9]*\)/\1/' | cut -d \/ -f 3`
>> +	driver=`readlink $usb_interface/driver | sed s'/.*\///'`
>> +	cd $led_cdev_path/../$usb_subdev
>> +	idVendor=`cat idVendor`
>> +	idProduct=`cat idProduct`
>> +	manufacturer=`cat manufacturer`
>> +	product=`cat product`
>> +elif [ "$bus" = "input" ]; then
>> +	cd $led_cdev_path
>> +	product=`cat device/name`
>> +	driver=`cat device/device/driver/description`
>> +elif [ $of_node_missing -eq 0 ]; then
>> +	cd $led_cdev_path
>> +	compatible=`cat device/of_node/compatible`
>> +	if [ "$compatible" = "gpio-leds" ]; then
>> +		driver="leds-gpio"
>> +	elif [ "$compatible" = "pwm-leds" ]; then
>> +		driver="leds-pwm"
>> +	else
>> +		manufacturer=`echo $compatible | cut -d, -f1`
>> +		product=`echo $compatible | cut -d, -f2`
>> +	fi
>> +else
>> +	echo "Unknown device type."
>> +	exit 1
>> +fi
>> +
>> +printf "bus:\t\t\t$bus\n"
>> +
>> +if [ ! -z "$idVendor" ]; then
>> +	printf "idVendor:\t\t$idVendor\n"
>> +fi
>> +
>> +if [ ! -z "$idProduct" ]; then
>> +	printf "idProduct:\t\t$idProduct\n"
>> +fi
>> +
>> +if [ ! -z "$manufacturer" ]; then
>> +	printf "manufacturer:\t\t$manufacturer\n"
>> +fi
>> +
>> +if [ ! -z "$product" ]; then
>> +	printf "product:\t\t$product\n"
>> +fi
>> +
>> +if [ ! -z "$driver" ]; then
>> +	printf "driver:\t\t\t$driver\n"
>> +fi
>> +
>> +if [ ! -z "$input_node" ]; then
>> +	printf "associated input node:\t$input_node\n"
>> +fi
>>
> 
> 

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 02/25] leds: core: Add support for composing LED class device names
  2019-03-12 17:28     ` Jacek Anaszewski
@ 2019-03-12 17:46       ` Dan Murphy
  2019-03-12 18:19         ` Jacek Anaszewski
  0 siblings, 1 reply; 53+ messages in thread
From: Dan Murphy @ 2019-03-12 17:46 UTC (permalink / raw)
  To: Jacek Anaszewski, linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, Baolin Wang, Daniel Mack,
	Linus Walleij, Oleh Kravchenko, Sakari Ailus

On 3/12/19 12:28 PM, Jacek Anaszewski wrote:
> Hi Dan,
> 
> On 3/12/19 6:15 PM, Dan Murphy wrote:
>> On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
>>> Add public led_compose_name() API for composing LED class device
>>> name basing on fwnode_handle data. The function composes device name
>>> according to either a new <color:function> pattern or the legacy
>>> <devicename:color:function> pattern. The decision on using the
>>> particular pattern is made basing on whether fwnode contains new
>>> "function" and "color" properties, or the legacy "label" proeprty.
>>>
>>> Backwards compatibility with in-driver hard-coded LED class device
>>> names is assured thanks to the default_desc argument.
>>>
>>> In case none of the aforementioned properties was found, then, for OF
>>> nodes, the node name is adopted for LED class device name.
>>>
>>> Alongside these changes added is a new tool - tools/leds/get_led_device_info.sh.
>>> The tool allows retrieving details of a LED class device's parent device,
>>> which proves that getting rid of a devicename section from LED name pattern
>>> is justified since this information is already available in sysfs.
>>>
>>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>>> Cc: Baolin Wang <baolin.wang@linaro.org>
>>> Cc: Daniel Mack <daniel@zonque.org>
>>> Cc: Dan Murphy <dmurphy@ti.com>
>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
>>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>>> ---
>>>   Documentation/leds/leds-class.txt | 20 +++++++++-
>>>   drivers/leds/led-core.c           | 82 +++++++++++++++++++++++++++++++++++++++
>>>   include/linux/leds.h              | 31 +++++++++++++++
>>>   tools/leds/get_led_device_info.sh | 81 ++++++++++++++++++++++++++++++++++++++
>>>   4 files changed, 213 insertions(+), 1 deletion(-)
>>>   create mode 100755 tools/leds/get_led_device_info.sh
>>>
>>> diff --git a/Documentation/leds/leds-class.txt b/Documentation/leds/leds-class.txt
>>> index 8b39cc6b03ee..866fe87063d4 100644
>>> --- a/Documentation/leds/leds-class.txt
>>> +++ b/Documentation/leds/leds-class.txt
>>> @@ -43,7 +43,22 @@ LED Device Naming
>>>     Is currently of the form:
>>>   -"devicename:colour:function"
>>> +"colour:function"
>>> +
>>> +There might be still LED class drivers around using "devicename:colour:function"
>>> +naming pattern, but the "devicename" section is now deprecated since it used
>>> +to convey information that was already available in the sysfs, like product
>>> +name. There is a tool (tools/leds/get_led_device_info.sh) available for
>>> +retrieving that information per a LED class device.
>>> +
>>> +Associations with other devices, like network ones, should be defined
>>> +via LED triggr mechanism. This approach is applied by some of wireless
>>> +network drivers that create triggers dynamically and incorporate phy
>>> +name into its name. On the other hand input subsystem offers LED - input
>>> +bridge (drivers/input/input-leds.c) for exposing keyboard LEDs as LED class
>>> +devices. The get_led_device_info.sh script has support for retrieving related
>>> +input device node name. Should it support discovery of associations between
>>> +LEDs and other subsystems, please don't hesitate to submit a relevant patch.
>>>     There have been calls for LED properties such as colour to be exported as
>>>   individual led class attributes. As a solution which doesn't incur as much
>>> @@ -51,6 +66,9 @@ overhead, I suggest these become part of the device name. The naming scheme
>>>   above leaves scope for further attributes should they be needed. If sections
>>>   of the name don't apply, just leave that section blank.
>>>   +Please also keep in mind that LED subsystem has a protection against LED name
>>> +conflict. It adds numerical suffix (e.g. "_1", "_2", "_3" etc.) to the requested
>>> +LED class device name in case it is already in use.
>>>     Brightness setting API
>>>   ======================
>>> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
>>> index ede4fa0ac2cc..bad92250d1d5 100644
>>> --- a/drivers/leds/led-core.c
>>> +++ b/drivers/leds/led-core.c
>>> @@ -16,6 +16,8 @@
>>>   #include <linux/list.h>
>>>   #include <linux/module.h>
>>>   #include <linux/mutex.h>
>>> +#include <linux/of.h>
>>> +#include <linux/property.h>
>>>   #include <linux/rwsem.h>
>>>   #include "leds.h"
>>>   @@ -327,3 +329,83 @@ void led_sysfs_enable(struct led_classdev *led_cdev)
>>>       led_cdev->flags &= ~LED_SYSFS_DISABLE;
>>>   }
>>>   EXPORT_SYMBOL_GPL(led_sysfs_enable);
>>> +
>>> +static void led_parse_properties(struct fwnode_handle *fwnode,
>>> +                 struct led_properties *props)
>>> +{
>>> +    int ret;
>>> +
>>> +    if (!fwnode)
>>> +        return;
>>> +
>>> +    if (fwnode_property_present(fwnode, "label")) {
>>> +        ret = fwnode_property_read_string(fwnode, "label", &props->label);
>>> +        if (ret)
>>> +            pr_err("Error parsing \'label\' property (%d)\n", ret);
>>> +        return;
>>> +    }
>>> +
>>> +    if (fwnode_property_present(fwnode, "function")) {
>>> +        ret = fwnode_property_read_string(fwnode, "function", &props->function);
>>> +        if (ret)
>>> +            pr_err("Error parsing \'function\' property (%d)\n", ret);
>>> +    } else {
>>> +        pr_info("\'function\' property not found\n");
>>> +    }
>>> +
>>> +    if (fwnode_property_present(fwnode, "color")) {
>>> +        ret = fwnode_property_read_string(fwnode, "color", &props->color);
>>> +        if (ret)
>>> +            pr_info("Error parsing \'color\' property (%d)\n", ret);
>>> +    } else {
>>> +        pr_info("\'color\' property not found\n");
>>> +    }
>>> +}
>>> +
>>> +int led_compose_name(struct fwnode_handle *fwnode, const char *led_hw_name,
>>> +             const char *default_desc, char *led_classdev_name)
>>> +{
>>> +    struct led_properties props = {};
>>> +
>>> +    if (!led_classdev_name)
>>> +        return -EINVAL;
>>> +
>>> +    led_parse_properties(fwnode, &props);
>>> +
>>> +    if (props.label) {
>>> +        /*
>>> +         * Presence of 'label' DT property implies legacy LED name,
>>> +         * formatted as <devicename:color:function>, with possible
>>> +         * section omission if doesn't apply to given device.
>>> +         *
>>> +         * If no led_hw_name has been passed, then it indicates that
>>> +         * DT label should be used as-is for LED class device name.
>>> +         * Otherwise the label is prepended with led_hw_name to compose
>>> +         * the final LED class device name.
>>> +         */
>>> +        if (!led_hw_name) {
>>> +            strncpy(led_classdev_name, props.label,
>>> +                LED_MAX_NAME_SIZE);
>>> +        } else {
>>> +            snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
>>> +                 led_hw_name, props.label);
>>> +        }
>>> +    } else if (props.function || props.color) {
>>> +        snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
>>> +             props.color ?: "", props.function ?: "");
>>> +    } else if (default_desc) {
>>> +        if (!led_hw_name) {
>>> +            pr_err("Legacy LED naming requires devicename segment");
>>> +            return -EINVAL;
>>> +        }
>>> +        snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
>>> +             led_hw_name, default_desc);
>>> +    } else if (is_of_node(fwnode)) {
>>> +        strncpy(led_classdev_name, to_of_node(fwnode)->name,
>>> +            LED_MAX_NAME_SIZE);
>>> +    } else
>>> +        return -EINVAL;
>>> +
>>> +    return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(led_compose_name);

I was thinking a bit more about this.
Why do we want to export this function to have the drivers call it?
Can't we just set the required parameters in the led_class struct?

struct led_properties can be extended to set the needed strings in the LED driver.
The pointer to this struct can be set in the LED driver for the class

Then we can just call compose_name during class registration.

If we add the fwnode to the struct this may help in the multi-color registration as we could pick off
the parent node and look for the specific labels/handles.


>>> diff --git a/include/linux/leds.h b/include/linux/leds.h
>>> index bffb4315fd66..c2936fc989d4 100644
>>> --- a/include/linux/leds.h
>>> +++ b/include/linux/leds.h
>>> @@ -252,6 +252,31 @@ extern void led_sysfs_disable(struct led_classdev *led_cdev);
>>>   extern void led_sysfs_enable(struct led_classdev *led_cdev);
>>>     /**
>>> + * led_compose_name - compose LED class device name
>>> + * @child: child fwnode_handle describing a LED,
>>> + *       or a group of synchronized LEDs.
>>> + * @led_hw_name: name of the LED controller, used when falling back to legacy
>>> + *         LED naming; it should be set to NULL in new LED class drivers
>>> + * @default_desc: default <color:function> tuple, for backwards compatibility
>>> + *          with in-driver hard-coded LED names used as a fallback when
>>> + *          "label" DT property is absent; it should be set to NULL
>>> + *          in new LED class drivers
>>> + * @led_classdev_name: composed LED class device name
>>> + *
>>> + * Create LED class device name basing on the configuration provided by the
>>> + * board firmware. The name can have a legacy form <devicename:color:function>,
>>> + * or a new form <color:function>. The latter is chosen if "label" property is
>>> + * absent and at least one of "color" or "function" is present in the fwnode,
>>> + * leaving the section blank if the related property is absent. In case none
>>> + * of the aforementioned properties is found, then, for OF nodes, the node name
>>> + * is adopted for LED class device name.
>>> + *
>>> + * Returns: 0 on success or negative error value on failure
>>> + */
>>> +extern int led_compose_name(struct fwnode_handle *child, const char *led_hw_name,
>>> +                const char *default_desc, char *led_classdev_name);
>>> +
>>> +/**
>>>    * led_sysfs_is_disabled - check if LED sysfs interface is disabled
>>>    * @led_cdev: the LED to query
>>>    *
>>> @@ -428,6 +453,12 @@ struct led_platform_data {
>>>       struct led_info    *leds;
>>>   };
>>>   +struct led_properties {
>>> +    const char *color;
>>> +    const char *function;
>>> +    const char *label;
>>> +};
>>> +
>>>   struct gpio_desc;
>>>   typedef int (*gpio_blink_set_t)(struct gpio_desc *desc, int state,
>>>                   unsigned long *delay_on,
>>> diff --git a/tools/leds/get_led_device_info.sh b/tools/leds/get_led_device_info.sh
>>> new file mode 100755
>>> index 000000000000..4671aa690e4a
>>> --- /dev/null
>>> +++ b/tools/leds/get_led_device_info.sh
>>> @@ -0,0 +1,81 @@
>>> +#!/bin/sh
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +
>>
>> Is there a way to give usage or help here?  It's not entirely clear what the argument to pass in is.
> 
> It is in the first statement of this script - see below.
> It is customary to print help when unexpected arguments or a number
> thereof is given.
> 
> I can print help also when "--help" is passed.
> 

OK.  Maybe doing --help or --? would be to much.  Maybe a bit better help message I could not tell that
was an error message.

maybe

echo "usage: get_led_device_info.sh LED_CDEV_PATH"

>> maybe if $1 = "?" then print usage
>>
>> Dan
>>
>>
>>> +if [ $# -ne 1 ]; then
>>> +    echo "get_led_devicename.sh LED_CDEV_PATH"
> 
> s/get_led_devicename/get_led_device_info/
> 
> It is a leftover from earlier stage of development.
> 
>>> +    exit 1
>>> +fi
>>> +
>>> +led_cdev_path=`echo $1 | sed s'/\/$//'`
>>> +
>>> +ls "$led_cdev_path/brightness" > /dev/null 2>&1
>>> +if [ $? -ne 0 ]; then
>>> +    echo "Device \"$led_cdev_path\" does not exist."
>>> +    exit 1
>>> +fi
>>> +
>>> +bus=`readlink $led_cdev_path/device/subsystem | sed s'/.*\///'`
>>> +usb_subdev=`readlink $led_cdev_path | grep usb | sed s'/\(.*usb[0-9]*\/[0-9]*-[0-9]*\)\/.*/\1/'`
>>> +ls "$led_cdev_path/device/of_node/compatible" > /dev/null 2>&1
>>> +of_node_missing=$?
>>> +
>>> +if [ "$bus" = "input" ]; then
>>> +    input_node=`readlink $led_cdev_path/device | sed s'/.*\///'`
>>> +    if [ ! -z $usb_subdev ]; then
>>> +        bus="usb"
>>> +    fi
>>> +fi
>>> +
>>> +if [ "$bus" = "usb" ]; then
>>> +    usb_interface=`readlink $led_cdev_path | sed s'/.*\(usb[0-9]*\)/\1/' | cut -d \/ -f 3`
>>> +    driver=`readlink $usb_interface/driver | sed s'/.*\///'`
>>> +    cd $led_cdev_path/../$usb_subdev
>>> +    idVendor=`cat idVendor`
>>> +    idProduct=`cat idProduct`
>>> +    manufacturer=`cat manufacturer`
>>> +    product=`cat product`
>>> +elif [ "$bus" = "input" ]; then
>>> +    cd $led_cdev_path
>>> +    product=`cat device/name`
>>> +    driver=`cat device/device/driver/description`
>>> +elif [ $of_node_missing -eq 0 ]; then
>>> +    cd $led_cdev_path
>>> +    compatible=`cat device/of_node/compatible`
>>> +    if [ "$compatible" = "gpio-leds" ]; then
>>> +        driver="leds-gpio"
>>> +    elif [ "$compatible" = "pwm-leds" ]; then
>>> +        driver="leds-pwm"
>>> +    else
>>> +        manufacturer=`echo $compatible | cut -d, -f1`
>>> +        product=`echo $compatible | cut -d, -f2`
>>> +    fi
>>> +else
>>> +    echo "Unknown device type."
>>> +    exit 1
>>> +fi
>>> +
>>> +printf "bus:\t\t\t$bus\n"
>>> +
>>> +if [ ! -z "$idVendor" ]; then
>>> +    printf "idVendor:\t\t$idVendor\n"
>>> +fi
>>> +
>>> +if [ ! -z "$idProduct" ]; then
>>> +    printf "idProduct:\t\t$idProduct\n"
>>> +fi
>>> +
>>> +if [ ! -z "$manufacturer" ]; then
>>> +    printf "manufacturer:\t\t$manufacturer\n"
>>> +fi
>>> +
>>> +if [ ! -z "$product" ]; then
>>> +    printf "product:\t\t$product\n"
>>> +fi
>>> +
>>> +if [ ! -z "$driver" ]; then
>>> +    printf "driver:\t\t\t$driver\n"
>>> +fi
>>> +
>>> +if [ ! -z "$input_node" ]; then
>>> +    printf "associated input node:\t$input_node\n"
>>> +fi
>>>
>>
>>
> 


-- 
------------------
Dan Murphy

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 02/25] leds: core: Add support for composing LED class device names
  2019-03-12 17:46       ` Dan Murphy
@ 2019-03-12 18:19         ` Jacek Anaszewski
  0 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-12 18:19 UTC (permalink / raw)
  To: Dan Murphy, linux-leds
  Cc: devicetree, linux-kernel, pavel, robh, Baolin Wang, Daniel Mack,
	Linus Walleij, Oleh Kravchenko, Sakari Ailus

On 3/12/19 6:46 PM, Dan Murphy wrote:
> On 3/12/19 12:28 PM, Jacek Anaszewski wrote:
>> Hi Dan,
>>
>> On 3/12/19 6:15 PM, Dan Murphy wrote:
>>> On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
>>>> Add public led_compose_name() API for composing LED class device
>>>> name basing on fwnode_handle data. The function composes device name
>>>> according to either a new <color:function> pattern or the legacy
>>>> <devicename:color:function> pattern. The decision on using the
>>>> particular pattern is made basing on whether fwnode contains new
>>>> "function" and "color" properties, or the legacy "label" proeprty.
>>>>
>>>> Backwards compatibility with in-driver hard-coded LED class device
>>>> names is assured thanks to the default_desc argument.
>>>>
>>>> In case none of the aforementioned properties was found, then, for OF
>>>> nodes, the node name is adopted for LED class device name.
>>>>
>>>> Alongside these changes added is a new tool - tools/leds/get_led_device_info.sh.
>>>> The tool allows retrieving details of a LED class device's parent device,
>>>> which proves that getting rid of a devicename section from LED name pattern
>>>> is justified since this information is already available in sysfs.
>>>>
>>>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>>>> Cc: Baolin Wang <baolin.wang@linaro.org>
>>>> Cc: Daniel Mack <daniel@zonque.org>
>>>> Cc: Dan Murphy <dmurphy@ti.com>
>>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>>> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
>>>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>>>> ---
>>>>    Documentation/leds/leds-class.txt | 20 +++++++++-
>>>>    drivers/leds/led-core.c           | 82 +++++++++++++++++++++++++++++++++++++++
>>>>    include/linux/leds.h              | 31 +++++++++++++++
>>>>    tools/leds/get_led_device_info.sh | 81 ++++++++++++++++++++++++++++++++++++++
>>>>    4 files changed, 213 insertions(+), 1 deletion(-)
>>>>    create mode 100755 tools/leds/get_led_device_info.sh
>>>>
>>>> diff --git a/Documentation/leds/leds-class.txt b/Documentation/leds/leds-class.txt
>>>> index 8b39cc6b03ee..866fe87063d4 100644
>>>> --- a/Documentation/leds/leds-class.txt
>>>> +++ b/Documentation/leds/leds-class.txt
>>>> @@ -43,7 +43,22 @@ LED Device Naming
>>>>      Is currently of the form:
>>>>    -"devicename:colour:function"
>>>> +"colour:function"
>>>> +
>>>> +There might be still LED class drivers around using "devicename:colour:function"
>>>> +naming pattern, but the "devicename" section is now deprecated since it used
>>>> +to convey information that was already available in the sysfs, like product
>>>> +name. There is a tool (tools/leds/get_led_device_info.sh) available for
>>>> +retrieving that information per a LED class device.
>>>> +
>>>> +Associations with other devices, like network ones, should be defined
>>>> +via LED triggr mechanism. This approach is applied by some of wireless
>>>> +network drivers that create triggers dynamically and incorporate phy
>>>> +name into its name. On the other hand input subsystem offers LED - input
>>>> +bridge (drivers/input/input-leds.c) for exposing keyboard LEDs as LED class
>>>> +devices. The get_led_device_info.sh script has support for retrieving related
>>>> +input device node name. Should it support discovery of associations between
>>>> +LEDs and other subsystems, please don't hesitate to submit a relevant patch.
>>>>      There have been calls for LED properties such as colour to be exported as
>>>>    individual led class attributes. As a solution which doesn't incur as much
>>>> @@ -51,6 +66,9 @@ overhead, I suggest these become part of the device name. The naming scheme
>>>>    above leaves scope for further attributes should they be needed. If sections
>>>>    of the name don't apply, just leave that section blank.
>>>>    +Please also keep in mind that LED subsystem has a protection against LED name
>>>> +conflict. It adds numerical suffix (e.g. "_1", "_2", "_3" etc.) to the requested
>>>> +LED class device name in case it is already in use.
>>>>      Brightness setting API
>>>>    ======================
>>>> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
>>>> index ede4fa0ac2cc..bad92250d1d5 100644
>>>> --- a/drivers/leds/led-core.c
>>>> +++ b/drivers/leds/led-core.c
>>>> @@ -16,6 +16,8 @@
>>>>    #include <linux/list.h>
>>>>    #include <linux/module.h>
>>>>    #include <linux/mutex.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/property.h>
>>>>    #include <linux/rwsem.h>
>>>>    #include "leds.h"
>>>>    @@ -327,3 +329,83 @@ void led_sysfs_enable(struct led_classdev *led_cdev)
>>>>        led_cdev->flags &= ~LED_SYSFS_DISABLE;
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(led_sysfs_enable);
>>>> +
>>>> +static void led_parse_properties(struct fwnode_handle *fwnode,
>>>> +                 struct led_properties *props)
>>>> +{
>>>> +    int ret;
>>>> +
>>>> +    if (!fwnode)
>>>> +        return;
>>>> +
>>>> +    if (fwnode_property_present(fwnode, "label")) {
>>>> +        ret = fwnode_property_read_string(fwnode, "label", &props->label);
>>>> +        if (ret)
>>>> +            pr_err("Error parsing \'label\' property (%d)\n", ret);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    if (fwnode_property_present(fwnode, "function")) {
>>>> +        ret = fwnode_property_read_string(fwnode, "function", &props->function);
>>>> +        if (ret)
>>>> +            pr_err("Error parsing \'function\' property (%d)\n", ret);
>>>> +    } else {
>>>> +        pr_info("\'function\' property not found\n");
>>>> +    }
>>>> +
>>>> +    if (fwnode_property_present(fwnode, "color")) {
>>>> +        ret = fwnode_property_read_string(fwnode, "color", &props->color);
>>>> +        if (ret)
>>>> +            pr_info("Error parsing \'color\' property (%d)\n", ret);
>>>> +    } else {
>>>> +        pr_info("\'color\' property not found\n");
>>>> +    }
>>>> +}
>>>> +
>>>> +int led_compose_name(struct fwnode_handle *fwnode, const char *led_hw_name,
>>>> +             const char *default_desc, char *led_classdev_name)
>>>> +{
>>>> +    struct led_properties props = {};
>>>> +
>>>> +    if (!led_classdev_name)
>>>> +        return -EINVAL;
>>>> +
>>>> +    led_parse_properties(fwnode, &props);
>>>> +
>>>> +    if (props.label) {
>>>> +        /*
>>>> +         * Presence of 'label' DT property implies legacy LED name,
>>>> +         * formatted as <devicename:color:function>, with possible
>>>> +         * section omission if doesn't apply to given device.
>>>> +         *
>>>> +         * If no led_hw_name has been passed, then it indicates that
>>>> +         * DT label should be used as-is for LED class device name.
>>>> +         * Otherwise the label is prepended with led_hw_name to compose
>>>> +         * the final LED class device name.
>>>> +         */
>>>> +        if (!led_hw_name) {
>>>> +            strncpy(led_classdev_name, props.label,
>>>> +                LED_MAX_NAME_SIZE);
>>>> +        } else {
>>>> +            snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
>>>> +                 led_hw_name, props.label);
>>>> +        }
>>>> +    } else if (props.function || props.color) {
>>>> +        snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
>>>> +             props.color ?: "", props.function ?: "");
>>>> +    } else if (default_desc) {
>>>> +        if (!led_hw_name) {
>>>> +            pr_err("Legacy LED naming requires devicename segment");
>>>> +            return -EINVAL;
>>>> +        }
>>>> +        snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
>>>> +             led_hw_name, default_desc);
>>>> +    } else if (is_of_node(fwnode)) {
>>>> +        strncpy(led_classdev_name, to_of_node(fwnode)->name,
>>>> +            LED_MAX_NAME_SIZE);
>>>> +    } else
>>>> +        return -EINVAL;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(led_compose_name);
> 
> I was thinking a bit more about this.
> Why do we want to export this function to have the drivers call it?
> Can't we just set the required parameters in the led_class struct?
> 
> struct led_properties can be extended to set the needed strings in the LED driver.
> The pointer to this struct can be set in the LED driver for the class
> 
> Then we can just call compose_name during class registration.

Adding to the struct led_classdev anything needed only in
led_classdev_register() would be waste of memory, but we can
add required properties to the new struct led_init_data and
then call led_compose_name() inside led_classdev_register().

I will change it in the v3.

> If we add the fwnode to the struct this may help in the multi-color registration as we could pick off
> the parent node and look for the specific labels/handles.

struct led_init_data already has fwnode property in this set.

> 
>>>> diff --git a/include/linux/leds.h b/include/linux/leds.h
>>>> index bffb4315fd66..c2936fc989d4 100644
>>>> --- a/include/linux/leds.h
>>>> +++ b/include/linux/leds.h
>>>> @@ -252,6 +252,31 @@ extern void led_sysfs_disable(struct led_classdev *led_cdev);
>>>>    extern void led_sysfs_enable(struct led_classdev *led_cdev);
>>>>      /**
>>>> + * led_compose_name - compose LED class device name
>>>> + * @child: child fwnode_handle describing a LED,
>>>> + *       or a group of synchronized LEDs.
>>>> + * @led_hw_name: name of the LED controller, used when falling back to legacy
>>>> + *         LED naming; it should be set to NULL in new LED class drivers
>>>> + * @default_desc: default <color:function> tuple, for backwards compatibility
>>>> + *          with in-driver hard-coded LED names used as a fallback when
>>>> + *          "label" DT property is absent; it should be set to NULL
>>>> + *          in new LED class drivers
>>>> + * @led_classdev_name: composed LED class device name
>>>> + *
>>>> + * Create LED class device name basing on the configuration provided by the
>>>> + * board firmware. The name can have a legacy form <devicename:color:function>,
>>>> + * or a new form <color:function>. The latter is chosen if "label" property is
>>>> + * absent and at least one of "color" or "function" is present in the fwnode,
>>>> + * leaving the section blank if the related property is absent. In case none
>>>> + * of the aforementioned properties is found, then, for OF nodes, the node name
>>>> + * is adopted for LED class device name.
>>>> + *
>>>> + * Returns: 0 on success or negative error value on failure
>>>> + */
>>>> +extern int led_compose_name(struct fwnode_handle *child, const char *led_hw_name,
>>>> +                const char *default_desc, char *led_classdev_name);
>>>> +
>>>> +/**
>>>>     * led_sysfs_is_disabled - check if LED sysfs interface is disabled
>>>>     * @led_cdev: the LED to query
>>>>     *
>>>> @@ -428,6 +453,12 @@ struct led_platform_data {
>>>>        struct led_info    *leds;
>>>>    };
>>>>    +struct led_properties {
>>>> +    const char *color;
>>>> +    const char *function;
>>>> +    const char *label;
>>>> +};
>>>> +
>>>>    struct gpio_desc;
>>>>    typedef int (*gpio_blink_set_t)(struct gpio_desc *desc, int state,
>>>>                    unsigned long *delay_on,
>>>> diff --git a/tools/leds/get_led_device_info.sh b/tools/leds/get_led_device_info.sh
>>>> new file mode 100755
>>>> index 000000000000..4671aa690e4a
>>>> --- /dev/null
>>>> +++ b/tools/leds/get_led_device_info.sh
>>>> @@ -0,0 +1,81 @@
>>>> +#!/bin/sh
>>>> +# SPDX-License-Identifier: GPL-2.0
>>>> +
>>>
>>> Is there a way to give usage or help here?  It's not entirely clear what the argument to pass in is.
>>
>> It is in the first statement of this script - see below.
>> It is customary to print help when unexpected arguments or a number
>> thereof is given.
>>
>> I can print help also when "--help" is passed.
>>
> 
> OK.  Maybe doing --help or --? would be to much.  Maybe a bit better help message I could not tell that
> was an error message.
> 
> maybe
> 
> echo "usage: get_led_device_info.sh LED_CDEV_PATH"

Ack.

>>> maybe if $1 = "?" then print usage
>>>
>>> Dan
>>>
>>>
>>>> +if [ $# -ne 1 ]; then
>>>> +    echo "get_led_devicename.sh LED_CDEV_PATH"
>>
>> s/get_led_devicename/get_led_device_info/
>>
>> It is a leftover from earlier stage of development.
>>
>>>> +    exit 1
>>>> +fi
>>>> +
>>>> +led_cdev_path=`echo $1 | sed s'/\/$//'`
>>>> +
>>>> +ls "$led_cdev_path/brightness" > /dev/null 2>&1
>>>> +if [ $? -ne 0 ]; then
>>>> +    echo "Device \"$led_cdev_path\" does not exist."
>>>> +    exit 1
>>>> +fi
>>>> +
>>>> +bus=`readlink $led_cdev_path/device/subsystem | sed s'/.*\///'`
>>>> +usb_subdev=`readlink $led_cdev_path | grep usb | sed s'/\(.*usb[0-9]*\/[0-9]*-[0-9]*\)\/.*/\1/'`
>>>> +ls "$led_cdev_path/device/of_node/compatible" > /dev/null 2>&1
>>>> +of_node_missing=$?
>>>> +
>>>> +if [ "$bus" = "input" ]; then
>>>> +    input_node=`readlink $led_cdev_path/device | sed s'/.*\///'`
>>>> +    if [ ! -z $usb_subdev ]; then
>>>> +        bus="usb"
>>>> +    fi
>>>> +fi
>>>> +
>>>> +if [ "$bus" = "usb" ]; then
>>>> +    usb_interface=`readlink $led_cdev_path | sed s'/.*\(usb[0-9]*\)/\1/' | cut -d \/ -f 3`
>>>> +    driver=`readlink $usb_interface/driver | sed s'/.*\///'`
>>>> +    cd $led_cdev_path/../$usb_subdev
>>>> +    idVendor=`cat idVendor`
>>>> +    idProduct=`cat idProduct`
>>>> +    manufacturer=`cat manufacturer`
>>>> +    product=`cat product`
>>>> +elif [ "$bus" = "input" ]; then
>>>> +    cd $led_cdev_path
>>>> +    product=`cat device/name`
>>>> +    driver=`cat device/device/driver/description`
>>>> +elif [ $of_node_missing -eq 0 ]; then
>>>> +    cd $led_cdev_path
>>>> +    compatible=`cat device/of_node/compatible`
>>>> +    if [ "$compatible" = "gpio-leds" ]; then
>>>> +        driver="leds-gpio"
>>>> +    elif [ "$compatible" = "pwm-leds" ]; then
>>>> +        driver="leds-pwm"
>>>> +    else
>>>> +        manufacturer=`echo $compatible | cut -d, -f1`
>>>> +        product=`echo $compatible | cut -d, -f2`
>>>> +    fi
>>>> +else
>>>> +    echo "Unknown device type."
>>>> +    exit 1
>>>> +fi
>>>> +
>>>> +printf "bus:\t\t\t$bus\n"
>>>> +
>>>> +if [ ! -z "$idVendor" ]; then
>>>> +    printf "idVendor:\t\t$idVendor\n"
>>>> +fi
>>>> +
>>>> +if [ ! -z "$idProduct" ]; then
>>>> +    printf "idProduct:\t\t$idProduct\n"
>>>> +fi
>>>> +
>>>> +if [ ! -z "$manufacturer" ]; then
>>>> +    printf "manufacturer:\t\t$manufacturer\n"
>>>> +fi
>>>> +
>>>> +if [ ! -z "$product" ]; then
>>>> +    printf "product:\t\t$product\n"
>>>> +fi
>>>> +
>>>> +if [ ! -z "$driver" ]; then
>>>> +    printf "driver:\t\t\t$driver\n"
>>>> +fi
>>>> +
>>>> +if [ ! -z "$input_node" ]; then
>>>> +    printf "associated input node:\t$input_node\n"
>>>> +fi
>>>>
>>>
>>>
>>
> 
> 

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 24/25] dt-bindings: an30259a: Add function and color properties
  2019-03-10 18:28 ` [PATCH 24/25] dt-bindings: an30259a: Add function and color properties Jacek Anaszewski
@ 2019-03-22  8:33   ` Pavel Machek
  0 siblings, 0 replies; 53+ messages in thread
From: Pavel Machek @ 2019-03-22  8:33 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-leds, devicetree, linux-kernel, robh, Simon Shields

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

On Sun 2019-03-10 19:28:35, Jacek Anaszewski wrote:
> Refer to new "function" and "color" properties and mark "label"
> as deprecated.
> 
> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Simon Shields <simon@lineageos.org>

Patches 6,8,10,14,16,18,20,22:

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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 04/25] dt-bindings: leds: Add LED_COLOR_NAME definitions
  2019-03-10 18:28 ` [PATCH 04/25] dt-bindings: leds: Add LED_COLOR_NAME definitions Jacek Anaszewski
  2019-03-11 12:23   ` Dan Murphy
@ 2019-03-22  8:35   ` Pavel Machek
  2019-03-28  0:08   ` Rob Herring
  2 siblings, 0 replies; 53+ messages in thread
From: Pavel Machek @ 2019-03-22  8:35 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-leds, devicetree, linux-kernel, robh, Baolin Wang,
	Daniel Mack, Dan Murphy, Linus Walleij, Oleh Kravchenko,
	Sakari Ailus, Simon Shields

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

On Sun 2019-03-10 19:28:15, Jacek Anaszewski wrote:
> Add common LED color name definitions for use in Device Tree.

Could we do "LED_COLOR_NAME_" => "LED_COLOR_"?

								Pavel

> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Baolin Wang <baolin.wang@linaro.org>
> Cc: Daniel Mack <daniel@zonque.org>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Simon Shields <simon@lineageos.org>
> ---
>  include/dt-bindings/leds/common.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
> index ffcd46317307..0e986bb59391 100644
> --- a/include/dt-bindings/leds/common.h
> +++ b/include/dt-bindings/leds/common.h
> @@ -57,4 +57,13 @@
>  #define LED_FUNCTION_WLAN "wlan"
>  #define LED_FUNCTION_WPS "wps"
>  
> +/* Standard LED colors */
> +#define LED_COLOR_NAME_WHITE    "white"
> +#define LED_COLOR_NAME_RED      "red"
> +#define LED_COLOR_NAME_GREEN    "green"
> +#define LED_COLOR_NAME_BLUE     "blue"
> +#define LED_COLOR_NAME_AMBER    "amber"
> +#define LED_COLOR_NAME_VIOLET   "violet"
> +#define LED_COLOR_NAME_YELLOW   "yellow"
> +
>  #endif /* __DT_BINDINGS_LEDS_H */

-- 
(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 --]

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 03/25] dt-bindings: leds: Add LED_FUNCTION definitions
  2019-03-10 18:28 ` [PATCH 03/25] dt-bindings: leds: Add LED_FUNCTION definitions Jacek Anaszewski
  2019-03-11 12:22   ` Dan Murphy
@ 2019-03-28  0:03   ` Rob Herring
  2019-03-28 20:01     ` Jacek Anaszewski
  1 sibling, 1 reply; 53+ messages in thread
From: Rob Herring @ 2019-03-28  0:03 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-leds, devicetree, linux-kernel, pavel, Baolin Wang,
	Daniel Mack, Dan Murphy, Linus Walleij, Oleh Kravchenko,
	Sakari Ailus, Simon Shields

On Sun, Mar 10, 2019 at 07:28:14PM +0100, Jacek Anaszewski wrote:
> Add common LED function definitions for use in Device Tree.
> The function names were extracted from existing dts files
> after eliminating oddities.

I'd like this to be suggestions of what to use rather than what's 
already out there. So my comments are in that context.

> 
> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Baolin Wang <baolin.wang@linaro.org>
> Cc: Daniel Mack <daniel@zonque.org>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Simon Shields <simon@lineageos.org>
> ---
>  include/dt-bindings/leds/common.h | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
> index e171d0a6beb2..ffcd46317307 100644
> --- a/include/dt-bindings/leds/common.h
> +++ b/include/dt-bindings/leds/common.h
> @@ -19,4 +19,42 @@
>  #define LEDS_BOOST_ADAPTIVE	1
>  #define LEDS_BOOST_FIXED	2
>  
> +/* Standard LED functions */
> +#define LED_FUNCTION_ACTIVITY "activity"

of what?

> +#define LED_FUNCTION_ADSL "adsl"

wan?

> +#define LED_FUNCTION_ALARM "alarm"
> +#define LED_FUNCTION_BACKLIGHT "backlight"
> +#define LED_FUNCTION_BLUETOOTH "bluetooth"
> +#define LED_FUNCTION_BOOT "boot"

What about boot?

> +#define LED_FUNCTION_CHRG "chrg"
> +#define LED_FUNCTION_DEBUG "debug"
> +#define LED_FUNCTION_DISK "disk"
> +#define LED_FUNCTION_DISK_READ "disk-read"
> +#define LED_FUNCTION_DISK_WRITE "disk-write"
> +#define LED_FUNCTION_FAULT "fault"
> +#define LED_FUNCTION_FLASH "flash"
> +#define LED_FUNCTION_HDDERR "hdderr"

disk-err for consistency?

> +#define LED_FUNCTION_HEARTBEAT "heartbeat"
> +#define LED_FUNCTION_INDICATOR "indicator"
> +#define LED_FUNCTION_INFO "info"

of what?

> +#define LED_FUNCTION_INTERNET "internet"

Same as wan? 

> +#define LED_FUNCTION_LAN "lan"
> +#define LED_FUNCTION_MMC "mmc"

Same as disk?

> +#define LED_FUNCTION_NAND "nand"
> +#define LED_FUNCTION_ON "on"
> +#define LED_FUNCTION_PROGRAMMING "programming"
> +#define LED_FUNCTION_PWR "pwr"
> +#define LED_FUNCTION_RX "rx"
> +#define LED_FUNCTION_SD "sd"

> +#define LED_FUNCTION_SLEEP "sleep"
> +#define LED_FUNCTION_STANDBY "standby"

Same things?

> +#define LED_FUNCTION_STATUS "status"
> +#define LED_FUNCTION_TORCH "torch"
> +#define LED_FUNCTION_TV "tv"
> +#define LED_FUNCTION_TX "tx"
> +#define LED_FUNCTION_USB "usb"
> +#define LED_FUNCTION_WAN "wan"
> +#define LED_FUNCTION_WLAN "wlan"
> +#define LED_FUNCTION_WPS "wps"
> +
>  #endif /* __DT_BINDINGS_LEDS_H */
> -- 
> 2.11.0
> 

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 04/25] dt-bindings: leds: Add LED_COLOR_NAME definitions
  2019-03-10 18:28 ` [PATCH 04/25] dt-bindings: leds: Add LED_COLOR_NAME definitions Jacek Anaszewski
  2019-03-11 12:23   ` Dan Murphy
  2019-03-22  8:35   ` Pavel Machek
@ 2019-03-28  0:08   ` Rob Herring
  2 siblings, 0 replies; 53+ messages in thread
From: Rob Herring @ 2019-03-28  0:08 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-leds, devicetree, linux-kernel, pavel, Baolin Wang,
	Daniel Mack, Dan Murphy, Linus Walleij, Oleh Kravchenko,
	Sakari Ailus, Simon Shields

On Sun, Mar 10, 2019 at 07:28:15PM +0100, Jacek Anaszewski wrote:
> Add common LED color name definitions for use in Device Tree.

Do we actually have variations in color strings? Maybe someone uses 
"RED" or something. If not, I think this adds less value compared to 
function names. Just my 2 cents, either way is fine.

> 
> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Baolin Wang <baolin.wang@linaro.org>
> Cc: Daniel Mack <daniel@zonque.org>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Simon Shields <simon@lineageos.org>
> ---
>  include/dt-bindings/leds/common.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
> index ffcd46317307..0e986bb59391 100644
> --- a/include/dt-bindings/leds/common.h
> +++ b/include/dt-bindings/leds/common.h
> @@ -57,4 +57,13 @@
>  #define LED_FUNCTION_WLAN "wlan"
>  #define LED_FUNCTION_WPS "wps"
>  
> +/* Standard LED colors */
> +#define LED_COLOR_NAME_WHITE    "white"
> +#define LED_COLOR_NAME_RED      "red"
> +#define LED_COLOR_NAME_GREEN    "green"
> +#define LED_COLOR_NAME_BLUE     "blue"
> +#define LED_COLOR_NAME_AMBER    "amber"
> +#define LED_COLOR_NAME_VIOLET   "violet"
> +#define LED_COLOR_NAME_YELLOW   "yellow"
> +
>  #endif /* __DT_BINDINGS_LEDS_H */
> -- 
> 2.11.0
> 

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 00/25] Add generic support for composing LED class device name
  2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
                   ` (24 preceding siblings ...)
  2019-03-10 18:28 ` [PATCH 25/25] leds: an30259a: Use led_compose_name() Jacek Anaszewski
@ 2019-03-28  0:19 ` Rob Herring
  2019-03-28 20:54   ` Jacek Anaszewski
  25 siblings, 1 reply; 53+ messages in thread
From: Rob Herring @ 2019-03-28  0:19 UTC (permalink / raw)
  To: Jacek Anaszewski; +Cc: linux-leds, devicetree, linux-kernel, pavel

On Sun, Mar 10, 2019 at 07:28:11PM +0100, Jacek Anaszewski wrote:
> Changes from v1:
> 
> - improved led_parse_properties() to parse label property at first
>   and return immediately after parsing succeeds
> - added tool get_led_device_info.sh for retrieving LED class device's
>   parent device related information
> - extended LED naming section of Documentation/leds/leds-class.txt
> - adjusted the list of LED_FUNCTION definitions according to the v1 review
>   remarks
> - added standard LED_COLOR_NAME definitions
> - removed functions.h and moved both LED_FUNCTION and LED_COLOR_NAME
>   definitions to include/dt-bindings/common.h
> - rebased leds-as3645a changes on top of the patch switching to fwnode
>   property API
> - updated DT bindings to use new LED_COLOR_NAME definitions
> - improved common LED bindings to not use address unit for sub-nodes
>   without reg property
> 
> Generally I still insist on deprecating label property and devicename
> section of LED name. The tool I added for obtaining LED device name
> proves availability of the related information in other places in
> the sysfs. Other discussed use cases are covered in the updated
> Documentation/leds/leds-class.txt.
> 
> Beside that, I tried also to create macros for automatic composition
> of "-N" suffixed LED functions, so that it would not be necessary
> to pollute common.h with plenty repetitions of the same function,
> differing only with the postfix. Unfortunately, the preprocessor
> of the dtc compiler seems not to accept string concatenetation.
> I.e. it is not possible to to the following assighment:
> 
> function = "hdd""-1"
> 
> If anyone knows how to obviate this shortocoming please let me know.

Raise the question on devicetree-compiler list. I've done my share of 
dtc patches, but the parsing side is generally not an area I touch.

Rob

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 05/25] dt-bindings: leds: Add function and color properties
  2019-03-12 16:43       ` Jacek Anaszewski
@ 2019-03-28  0:23         ` Rob Herring
  2019-04-04 13:21           ` Pavel Machek
  0 siblings, 1 reply; 53+ messages in thread
From: Rob Herring @ 2019-03-28  0:23 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Dan Murphy, linux-leds, devicetree, linux-kernel, pavel,
	Baolin Wang, Daniel Mack, Linus Walleij, Oleh Kravchenko,
	Sakari Ailus, Simon Shields

On Tue, Mar 12, 2019 at 05:43:11PM +0100, Jacek Anaszewski wrote:
> On 3/11/19 6:24 PM, Jacek Anaszewski wrote:
> > Dan,
> > 
> > On 3/11/19 1:26 PM, Dan Murphy wrote:
> > > On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
> > > > Introduce dedicated properties for conveying information about
> > > > LED function and color. Mark old "label" property as deprecated.
> > > > 
> > > > Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> > > > Cc: Baolin Wang <baolin.wang@linaro.org>
> > > > Cc: Daniel Mack <daniel@zonque.org>
> > > > Cc: Dan Murphy <dmurphy@ti.com>
> > > > Cc: Linus Walleij <linus.walleij@linaro.org>
> > > > Cc: Oleh Kravchenko <oleg@kaa.org.ua>
> > > > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > Cc: Simon Shields <simon@lineageos.org>
> > > > ---
> > > >   Documentation/devicetree/bindings/leds/common.txt | 55
> > > > +++++++++++++++++++----
> > > >   1 file changed, 47 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/leds/common.txt
> > > > b/Documentation/devicetree/bindings/leds/common.txt
> > > > index aa1399814a2a..3402b0e1cec9 100644
> > > > --- a/Documentation/devicetree/bindings/leds/common.txt
> > > > +++ b/Documentation/devicetree/bindings/leds/common.txt
> > > > @@ -10,14 +10,23 @@ can influence the way of the LED device
> > > > initialization, the LED components
> > > >   have to be tightly coupled with the LED device binding. They
> > > > are represented
> > > >   by child nodes of the parent LED device binding.
> > > > +
> > > >   Optional properties for child nodes:
> > > >   - led-sources : List of device current outputs the LED is
> > > > connected to. The
> > > >           outputs are identified by the numbers that must be defined
> > > >           in the LED device binding documentation.
> > > > +- function: LED functon. Use one of the LED_FUNCTION_* prefixed
> > > > definitions
> > > > +        from the header include/dt-bindings/leds/common.h.
> > > > +        If there is no matching LED_FUNCTION available, add a new one.
> > > > +- color : Color of the LED. Use one of the LED_COLOR_NAME_*
> > > > prefixed definitions
> > > > +        from the header include/dt-bindings/leds/common.h.
> > > > +        If there is no matching LED_COLOR_NAME available, add a
> > > > new one.
> > > > +
> > > 
> > > I am assuming multi color can re-use this property as well?
> > 
> > I intended it to be a string, but indeed it would be better if we will
> > make it an integer to be consistent with the discussed LED multi color
> > design.
> 
> Going further, I wonder if it would make sense to make function also
> an integer. This way we could enforce use of LED functions known
> to kernel.

I think if we did that, then we'd just need to keep label. I think we 
need to allow for populating a string in DT matching a sticker next to 
an LED on a device.

Rob

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 03/25] dt-bindings: leds: Add LED_FUNCTION definitions
  2019-03-28  0:03   ` Rob Herring
@ 2019-03-28 20:01     ` Jacek Anaszewski
  0 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-28 20:01 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-leds, devicetree, linux-kernel, pavel, Baolin Wang,
	Daniel Mack, Dan Murphy, Linus Walleij, Oleh Kravchenko,
	Sakari Ailus, Simon Shields

Hi Rob,

Thanks for the review.

On 3/28/19 1:03 AM, Rob Herring wrote:
> On Sun, Mar 10, 2019 at 07:28:14PM +0100, Jacek Anaszewski wrote:
>> Add common LED function definitions for use in Device Tree.
>> The function names were extracted from existing dts files
>> after eliminating oddities.
> 
> I'd like this to be suggestions of what to use rather than what's
> already out there. So my comments are in that context.
> 
>>
>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>> Cc: Baolin Wang <baolin.wang@linaro.org>
>> Cc: Daniel Mack <daniel@zonque.org>
>> Cc: Dan Murphy <dmurphy@ti.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>> Cc: Simon Shields <simon@lineageos.org>
>> ---
>>   include/dt-bindings/leds/common.h | 38 ++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 38 insertions(+)
>>
>> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
>> index e171d0a6beb2..ffcd46317307 100644
>> --- a/include/dt-bindings/leds/common.h
>> +++ b/include/dt-bindings/leds/common.h
>> @@ -19,4 +19,42 @@
>>   #define LEDS_BOOST_ADAPTIVE	1
>>   #define LEDS_BOOST_FIXED	2
>>   
>> +/* Standard LED functions */
>> +#define LED_FUNCTION_ACTIVITY "activity"
> 
> of what?

We have activity trigger, which provides instant feedback
on the cpu activity.

This would nicely map to that.

>> +#define LED_FUNCTION_ADSL "adsl"
> 
> wan?

Good point.

>> +#define LED_FUNCTION_ALARM "alarm"
>> +#define LED_FUNCTION_BACKLIGHT "backlight"
>> +#define LED_FUNCTION_BLUETOOTH "bluetooth"
>> +#define LED_FUNCTION_BOOT "boot"
> 
> What about boot?

Is lit when bootloader code is executed?
Turned off just before starting the kernel.

>> +#define LED_FUNCTION_CHRG "chrg"
>> +#define LED_FUNCTION_DEBUG "debug"
>> +#define LED_FUNCTION_DISK "disk"
>> +#define LED_FUNCTION_DISK_READ "disk-read"
>> +#define LED_FUNCTION_DISK_WRITE "disk-write"
>> +#define LED_FUNCTION_FAULT "fault"
>> +#define LED_FUNCTION_FLASH "flash"
>> +#define LED_FUNCTION_HDDERR "hdderr"
> 
> disk-err for consistency?

Ack.

>> +#define LED_FUNCTION_HEARTBEAT "heartbeat"
>> +#define LED_FUNCTION_INDICATOR "indicator"
>> +#define LED_FUNCTION_INFO "info"
> 
> of what?

Right, doesn't make sense alone.

>> +#define LED_FUNCTION_INTERNET "internet"
> 
> Same as wan?

OK, to be removed.

>> +#define LED_FUNCTION_LAN "lan"
>> +#define LED_FUNCTION_MMC "mmc"
> 
> Same as disk?

Not sure. The possibility to have separate LEDs for different types of
mass storage devices could be useful.

>> +#define LED_FUNCTION_NAND "nand"
>> +#define LED_FUNCTION_ON "on"
>> +#define LED_FUNCTION_PROGRAMMING "programming"
>> +#define LED_FUNCTION_PWR "pwr"
>> +#define LED_FUNCTION_RX "rx"
>> +#define LED_FUNCTION_SD "sd"
> 
>> +#define LED_FUNCTION_SLEEP "sleep"
>> +#define LED_FUNCTION_STANDBY "standby"
> 
> Same things?

Ack. Standby feels more technical so I'd remove "sleep".

>> +#define LED_FUNCTION_STATUS "status"
>> +#define LED_FUNCTION_TORCH "torch"
>> +#define LED_FUNCTION_TV "tv"
>> +#define LED_FUNCTION_TX "tx"
>> +#define LED_FUNCTION_USB "usb"
>> +#define LED_FUNCTION_WAN "wan"
>> +#define LED_FUNCTION_WLAN "wlan"
>> +#define LED_FUNCTION_WPS "wps"
>> +
>>   #endif /* __DT_BINDINGS_LEDS_H */
>> -- 
>> 2.11.0
>>
> 

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 00/25] Add generic support for composing LED class device name
  2019-03-28  0:19 ` [PATCH v2 00/25] Add generic support for composing LED class device name Rob Herring
@ 2019-03-28 20:54   ` Jacek Anaszewski
  0 siblings, 0 replies; 53+ messages in thread
From: Jacek Anaszewski @ 2019-03-28 20:54 UTC (permalink / raw)
  To: Rob Herring; +Cc: linux-leds, devicetree, linux-kernel, pavel

On 3/28/19 1:19 AM, Rob Herring wrote:
> On Sun, Mar 10, 2019 at 07:28:11PM +0100, Jacek Anaszewski wrote:
>> Changes from v1:
>>
>> - improved led_parse_properties() to parse label property at first
>>    and return immediately after parsing succeeds
>> - added tool get_led_device_info.sh for retrieving LED class device's
>>    parent device related information
>> - extended LED naming section of Documentation/leds/leds-class.txt
>> - adjusted the list of LED_FUNCTION definitions according to the v1 review
>>    remarks
>> - added standard LED_COLOR_NAME definitions
>> - removed functions.h and moved both LED_FUNCTION and LED_COLOR_NAME
>>    definitions to include/dt-bindings/common.h
>> - rebased leds-as3645a changes on top of the patch switching to fwnode
>>    property API
>> - updated DT bindings to use new LED_COLOR_NAME definitions
>> - improved common LED bindings to not use address unit for sub-nodes
>>    without reg property
>>
>> Generally I still insist on deprecating label property and devicename
>> section of LED name. The tool I added for obtaining LED device name
>> proves availability of the related information in other places in
>> the sysfs. Other discussed use cases are covered in the updated
>> Documentation/leds/leds-class.txt.
>>
>> Beside that, I tried also to create macros for automatic composition
>> of "-N" suffixed LED functions, so that it would not be necessary
>> to pollute common.h with plenty repetitions of the same function,
>> differing only with the postfix. Unfortunately, the preprocessor
>> of the dtc compiler seems not to accept string concatenetation.
>> I.e. it is not possible to to the following assighment:
>>
>> function = "hdd""-1"
>>
>> If anyone knows how to obviate this shortocoming please let me know.
> 
> Raise the question on devicetree-compiler list. I've done my share of
> dtc patches, but the parsing side is generally not an area I touch.

I'll probably come up with additional property function-enumerator.

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH 05/25] dt-bindings: leds: Add function and color properties
  2019-03-28  0:23         ` Rob Herring
@ 2019-04-04 13:21           ` Pavel Machek
  0 siblings, 0 replies; 53+ messages in thread
From: Pavel Machek @ 2019-04-04 13:21 UTC (permalink / raw)
  To: Rob Herring
  Cc: Jacek Anaszewski, Dan Murphy, linux-leds, devicetree,
	linux-kernel, Baolin Wang, Daniel Mack, Linus Walleij,
	Oleh Kravchenko, Sakari Ailus, Simon Shields


> > > > > +- function: LED functon. Use one of the LED_FUNCTION_* prefixed
> > > > > definitions
> > > > > +        from the header include/dt-bindings/leds/common.h.
> > > > > +        If there is no matching LED_FUNCTION available, add a new one.
> > > > > +- color : Color of the LED. Use one of the LED_COLOR_NAME_*
> > > > > prefixed definitions
> > > > > +        from the header include/dt-bindings/leds/common.h.
> > > > > +        If there is no matching LED_COLOR_NAME available, add a
> > > > > new one.
> > > > > +
> > > > 
> > > > I am assuming multi color can re-use this property as well?
> > > 
> > > I intended it to be a string, but indeed it would be better if we will
> > > make it an integer to be consistent with the discussed LED multi color
> > > design.
> > 
> > Going further, I wonder if it would make sense to make function also
> > an integer. This way we could enforce use of LED functions known
> > to kernel.
> 
> I think if we did that, then we'd just need to keep label. I think we 
> need to allow for populating a string in DT matching a sticker next to 
> an LED on a device.

Well, there's often icon next to the LED... and we already use "label" for
something else. New property might make sense, but again, icons will make it tricky.

									Pavel

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

^ permalink raw reply	[flat|nested] 53+ messages in thread

end of thread, other threads:[~2019-04-04 13:29 UTC | newest]

Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-10 18:28 [PATCH v2 00/25] Add generic support for composing LED class device name Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 01/25] leds: class: Improve LED and LED flash class registration API Jacek Anaszewski
2019-03-10 20:18   ` Oleh Kravchenko
2019-03-10 18:28 ` [PATCH 02/25] leds: core: Add support for composing LED class device names Jacek Anaszewski
2019-03-12 16:56   ` Jacek Anaszewski
2019-03-12 17:15   ` Dan Murphy
2019-03-12 17:28     ` Jacek Anaszewski
2019-03-12 17:46       ` Dan Murphy
2019-03-12 18:19         ` Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 03/25] dt-bindings: leds: Add LED_FUNCTION definitions Jacek Anaszewski
2019-03-11 12:22   ` Dan Murphy
2019-03-11 17:22     ` Jacek Anaszewski
2019-03-28  0:03   ` Rob Herring
2019-03-28 20:01     ` Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 04/25] dt-bindings: leds: Add LED_COLOR_NAME definitions Jacek Anaszewski
2019-03-11 12:23   ` Dan Murphy
2019-03-11 17:23     ` Jacek Anaszewski
2019-03-22  8:35   ` Pavel Machek
2019-03-28  0:08   ` Rob Herring
2019-03-10 18:28 ` [PATCH 05/25] dt-bindings: leds: Add function and color properties Jacek Anaszewski
2019-03-11 12:26   ` Dan Murphy
2019-03-11 17:24     ` Jacek Anaszewski
2019-03-12 16:43       ` Jacek Anaszewski
2019-03-28  0:23         ` Rob Herring
2019-04-04 13:21           ` Pavel Machek
2019-03-10 18:28 ` [PATCH 06/25] dt-bindings: sc27xx-blt: " Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 07/25] leds: sc27xx-blt: Use led_compose_name() Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 08/25] dt-bindings: lt3593: Add function and color properties Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 09/25] leds: lt3593: Use led_compose_name() Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 10/25] dt-bindings: lp8860: Add function and color properties Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 11/25] leds: lp8860: Use led_compose_name() Jacek Anaszewski
2019-03-11 12:28   ` Dan Murphy
2019-03-11 17:24     ` Jacek Anaszewski
2019-03-11 17:26       ` Dan Murphy
2019-03-10 18:28 ` [PATCH 12/25] dt-bindings: lm3692x: Add function and color properties Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 13/25] leds: lm3692x: Use led_compose_name() Jacek Anaszewski
2019-03-11 12:38   ` Dan Murphy
2019-03-11 17:24     ` Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 14/25] dt-bindings: lm36010: Add function and color properties Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 15/25] leds: lm3601x: Use led_compose_name() Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 16/25] dt-bindings: cr0014114: Add function and color properties Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 17/25] leds: cr0014114: Use led_compose_name() Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 18/25] dt-bindings: aat1290: Add function and color properties Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 19/25] leds: aat1290: Use led_compose_name() Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 20/25] dt-bindings: as3645a: Add function and color properties Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 21/25] leds: as3645a: Use led_compose_name() Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 22/25] dt-bindings: leds-gpio: Add function and color properties Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 23/25] leds: gpio: Use led_compose_name() Jacek Anaszewski
2019-03-10 18:28 ` [PATCH 24/25] dt-bindings: an30259a: Add function and color properties Jacek Anaszewski
2019-03-22  8:33   ` Pavel Machek
2019-03-10 18:28 ` [PATCH 25/25] leds: an30259a: Use led_compose_name() Jacek Anaszewski
2019-03-28  0:19 ` [PATCH v2 00/25] Add generic support for composing LED class device name Rob Herring
2019-03-28 20:54   ` Jacek Anaszewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).