All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/6] Updated LP8860 driver series
@ 2017-12-12 22:01 ` Dan Murphy
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-12 22:01 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	rpurdie-Fm38FmjxZ/leoWH0uzbU5w,
	jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w, pavel-+ZI9xUNit7I
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-leds-u79uwXL29TY76Z2rM5mHXA, Dan Murphy

All

v4 - Fix checkpatch warning for code indentation on the
leds: lp8860: Update the dt parsing for LED labeling patch.

v3 - Made changes to the patch set to address concerns on DT node naming conventions
based on discussion in the RFC patch https://patchwork.kernel.org/patch/10089047/
also made requested changes to the DT and driver based on feedback.  Patchworks
links in each patch.

v2 - Added an initial patch to bring the DT binding up to standard prior to adding
the changes for the label and triggers.

v1 Cover letter repeat below

After creating a new LED driver for the LM3692x device I went back to the
LP8860 driver that I authored and found some updates that need to be applied.

First the way the LP8860 retrieved the label from the DT was incorrect as the
label should have been from a child node as opposed to the parent.  This is now
fixed with this series.

Second, since that device can be used to as either a backlight driver or as a
string agnostic driver a trigger to the backlight needed to be added.

Finally there are changes to the driver that need to be made as either
unnoticed bugs or updates to the driver to align with the current LED
framework.  For instance moving to the devm LED class registration, destroying
the mutex upon driver removal and removing the in driver dependency on CONFIG_OF
and moving it to the Kconfig.

With these changes this should at least bring the driver into a better shape.

There are additional changes coming for this driver but I wanted to get the
driver up to snuff before adding a feature to it.

Dan

Dan Murphy (6):
  dt: bindings: lp8860: Update bindings for lp8860
  dt: bindings: lp8860: Update DT label binding
  leds: lp8860: Update the dt parsing for LED labeling
  dt: bindings: lp8860: Add trigger binding to the lp8860
  leds: lp8860: Add DT parsing to retrieve the trigger node
  leds: lp8860: Various fixes to align with LED framework

 .../devicetree/bindings/leds/leds-lp8860.txt       | 32 ++++++++++++-----
 drivers/leds/Kconfig                               |  2 +-
 drivers/leds/leds-lp8860.c                         | 40 ++++++++++++----------
 3 files changed, 46 insertions(+), 28 deletions(-)

-- 
2.15.0.124.g7668cbc60

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH v4 0/6] Updated LP8860 driver series
@ 2017-12-12 22:01 ` Dan Murphy
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-12 22:01 UTC (permalink / raw)
  To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, linux-leds, Dan Murphy

All

v4 - Fix checkpatch warning for code indentation on the
leds: lp8860: Update the dt parsing for LED labeling patch.

v3 - Made changes to the patch set to address concerns on DT node naming conventions
based on discussion in the RFC patch https://patchwork.kernel.org/patch/10089047/
also made requested changes to the DT and driver based on feedback.  Patchworks
links in each patch.

v2 - Added an initial patch to bring the DT binding up to standard prior to adding
the changes for the label and triggers.

v1 Cover letter repeat below

After creating a new LED driver for the LM3692x device I went back to the
LP8860 driver that I authored and found some updates that need to be applied.

First the way the LP8860 retrieved the label from the DT was incorrect as the
label should have been from a child node as opposed to the parent.  This is now
fixed with this series.

Second, since that device can be used to as either a backlight driver or as a
string agnostic driver a trigger to the backlight needed to be added.

Finally there are changes to the driver that need to be made as either
unnoticed bugs or updates to the driver to align with the current LED
framework.  For instance moving to the devm LED class registration, destroying
the mutex upon driver removal and removing the in driver dependency on CONFIG_OF
and moving it to the Kconfig.

With these changes this should at least bring the driver into a better shape.

There are additional changes coming for this driver but I wanted to get the
driver up to snuff before adding a feature to it.

Dan

Dan Murphy (6):
  dt: bindings: lp8860: Update bindings for lp8860
  dt: bindings: lp8860: Update DT label binding
  leds: lp8860: Update the dt parsing for LED labeling
  dt: bindings: lp8860: Add trigger binding to the lp8860
  leds: lp8860: Add DT parsing to retrieve the trigger node
  leds: lp8860: Various fixes to align with LED framework

 .../devicetree/bindings/leds/leds-lp8860.txt       | 32 ++++++++++++-----
 drivers/leds/Kconfig                               |  2 +-
 drivers/leds/leds-lp8860.c                         | 40 ++++++++++++----------
 3 files changed, 46 insertions(+), 28 deletions(-)

-- 
2.15.0.124.g7668cbc60

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

* [PATCH v4 1/6] dt: bindings: lp8860: Update bindings for lp8860
  2017-12-12 22:01 ` Dan Murphy
@ 2017-12-12 22:01   ` Dan Murphy
  -1 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-12 22:01 UTC (permalink / raw)
  To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, linux-leds, Dan Murphy

Update the lp8860 bindings to fix various issues
found.  Add address-cells and size-cells, rename
enable-gpio to enable-gpios, update the node name
to the device name and indent the node example.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v4 - No changes

v3 - Indicatd enable-gpios is active high, moved address and size cells to child
node patch and updated parent DT node name - https://patchwork.kernel.org/patch/10093745/
v2 - New patch

 Documentation/devicetree/bindings/leds/leds-lp8860.txt | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-lp8860.txt b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
index aad38dd94d4b..b9d09acbaa73 100644
--- a/Documentation/devicetree/bindings/leds/leds-lp8860.txt
+++ b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
@@ -6,22 +6,22 @@ current sinks that can be controlled by a PWM input
 signal, a SPI/I2C master, or both.
 
 Required properties:
-	- compatible:
+	- compatible :
 		"ti,lp8860"
-	- reg -  I2C slave address
-	- label - Used for naming LEDs
+	- reg : I2C slave address
+	- label : Used for naming LEDs
 
 Optional properties:
-	- enable-gpio - gpio pin to enable/disable the device.
-	- supply - "vled" - LED supply
+	- enable-gpios : gpio pin to enable (active high)/disable the device.
+	- vled-supply : LED supply
 
 Example:
 
-leds: leds@6 {
+led-controller@2d {
 	compatible = "ti,lp8860";
 	reg = <0x2d>;
 	label = "display_cluster";
-	enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
 	vled-supply = <&vbatt>;
 }
 
-- 
2.15.0.124.g7668cbc60

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

* [PATCH v4 1/6] dt: bindings: lp8860: Update bindings for lp8860
@ 2017-12-12 22:01   ` Dan Murphy
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-12 22:01 UTC (permalink / raw)
  To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, linux-leds, Dan Murphy

Update the lp8860 bindings to fix various issues
found.  Add address-cells and size-cells, rename
enable-gpio to enable-gpios, update the node name
to the device name and indent the node example.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v4 - No changes

v3 - Indicatd enable-gpios is active high, moved address and size cells to child
node patch and updated parent DT node name - https://patchwork.kernel.org/patch/10093745/
v2 - New patch

 Documentation/devicetree/bindings/leds/leds-lp8860.txt | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-lp8860.txt b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
index aad38dd94d4b..b9d09acbaa73 100644
--- a/Documentation/devicetree/bindings/leds/leds-lp8860.txt
+++ b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
@@ -6,22 +6,22 @@ current sinks that can be controlled by a PWM input
 signal, a SPI/I2C master, or both.
 
 Required properties:
-	- compatible:
+	- compatible :
 		"ti,lp8860"
-	- reg -  I2C slave address
-	- label - Used for naming LEDs
+	- reg : I2C slave address
+	- label : Used for naming LEDs
 
 Optional properties:
-	- enable-gpio - gpio pin to enable/disable the device.
-	- supply - "vled" - LED supply
+	- enable-gpios : gpio pin to enable (active high)/disable the device.
+	- vled-supply : LED supply
 
 Example:
 
-leds: leds@6 {
+led-controller@2d {
 	compatible = "ti,lp8860";
 	reg = <0x2d>;
 	label = "display_cluster";
-	enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
 	vled-supply = <&vbatt>;
 }
 
-- 
2.15.0.124.g7668cbc60

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

* [PATCH v4 2/6] dt: bindings: lp8860: Update DT label binding
  2017-12-12 22:01 ` Dan Murphy
@ 2017-12-12 22:01   ` Dan Murphy
  -1 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-12 22:01 UTC (permalink / raw)
  To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, linux-leds, Dan Murphy

Update the lp8860 label binding to the LED
standard as documented in

Documentation/devicetree/bindings/leds/common.txt

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v4 - No changes

v3 - Added address and size cells, updated label with color and inserted spaces
around the reg node - https://patchwork.kernel.org/patch/10093749/
v2 - Added reg to child node and made it required

 Documentation/devicetree/bindings/leds/leds-lp8860.txt | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-lp8860.txt b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
index b9d09acbaa73..c3d64ade1e26 100644
--- a/Documentation/devicetree/bindings/leds/leds-lp8860.txt
+++ b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
@@ -9,20 +9,33 @@ Required properties:
 	- compatible :
 		"ti,lp8860"
 	- reg : I2C slave address
-	- label : Used for naming LEDs
+	- #address-cells : 1
+	- #size-cells : 0
 
 Optional properties:
 	- enable-gpios : gpio pin to enable (active high)/disable the device.
 	- vled-supply : LED supply
 
+Required child properties:
+	- reg : 0
+
+Optional child properties:
+	- label : see Documentation/devicetree/bindings/leds/common.txt
+
 Example:
 
 led-controller@2d {
 	compatible = "ti,lp8860";
+	#address-cells = <1>;
+	#size-cells = <0>;
 	reg = <0x2d>;
-	label = "display_cluster";
 	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
 	vled-supply = <&vbatt>;
+
+	led@0 {
+		reg = <0>;
+		label = "white:display_cluster";
+	};
 }
 
 For more product information please see the link below:
-- 
2.15.0.124.g7668cbc60

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

* [PATCH v4 2/6] dt: bindings: lp8860: Update DT label binding
@ 2017-12-12 22:01   ` Dan Murphy
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-12 22:01 UTC (permalink / raw)
  To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, linux-leds, Dan Murphy

Update the lp8860 label binding to the LED
standard as documented in

Documentation/devicetree/bindings/leds/common.txt

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v4 - No changes

v3 - Added address and size cells, updated label with color and inserted spaces
around the reg node - https://patchwork.kernel.org/patch/10093749/
v2 - Added reg to child node and made it required

 Documentation/devicetree/bindings/leds/leds-lp8860.txt | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/leds-lp8860.txt b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
index b9d09acbaa73..c3d64ade1e26 100644
--- a/Documentation/devicetree/bindings/leds/leds-lp8860.txt
+++ b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
@@ -9,20 +9,33 @@ Required properties:
 	- compatible :
 		"ti,lp8860"
 	- reg : I2C slave address
-	- label : Used for naming LEDs
+	- #address-cells : 1
+	- #size-cells : 0
 
 Optional properties:
 	- enable-gpios : gpio pin to enable (active high)/disable the device.
 	- vled-supply : LED supply
 
+Required child properties:
+	- reg : 0
+
+Optional child properties:
+	- label : see Documentation/devicetree/bindings/leds/common.txt
+
 Example:
 
 led-controller@2d {
 	compatible = "ti,lp8860";
+	#address-cells = <1>;
+	#size-cells = <0>;
 	reg = <0x2d>;
-	label = "display_cluster";
 	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
 	vled-supply = <&vbatt>;
+
+	led@0 {
+		reg = <0>;
+		label = "white:display_cluster";
+	};
 }
 
 For more product information please see the link below:
-- 
2.15.0.124.g7668cbc60

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

* [PATCH v4 3/6] leds: lp8860: Update the dt parsing for LED labeling
  2017-12-12 22:01 ` Dan Murphy
@ 2017-12-12 22:01   ` Dan Murphy
  -1 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-12 22:01 UTC (permalink / raw)
  To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, linux-leds, Dan Murphy

Update the DT parsing for the label node so that
the label is retrieved from the device child as
opposed to being part of the parent.

This will align this driver with the LED
binding documentation

Documentation/devicetree/bindings/leds/common.txt

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v4 - Fix checkpatch warning for code indentation - https://patchwork.kernel.org/patch/10108157/

v3 - Changed the label generation to pull the name from the i2c device id 
as opposed to pulling the id from the parent dt node since that will just be 
led-controller - https://patchwork.kernel.org/patch/10093753/
v2 - no changes

 drivers/leds/leds-lp8860.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index 3e70775a2d54..46578fbc36be 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -22,6 +22,7 @@
 #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
@@ -86,8 +87,6 @@
 
 #define LP8860_CLEAR_FAULTS		0x01
 
-#define LP8860_DISP_LED_NAME		"display_cluster"
-
 /**
  * struct lp8860_led -
  * @lock - Lock for reading/writing the device
@@ -107,7 +106,7 @@ struct lp8860_led {
 	struct regmap *eeprom_regmap;
 	struct gpio_desc *enable_gpio;
 	struct regulator *regulator;
-	const char *label;
+	char label[LED_MAX_NAME_SIZE];
 };
 
 struct lp8860_eeprom_reg {
@@ -365,19 +364,21 @@ static int lp8860_probe(struct i2c_client *client,
 	int ret;
 	struct lp8860_led *led;
 	struct device_node *np = client->dev.of_node;
+	struct device_node *child_node;
+	const char *name;
 
 	led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL);
 	if (!led)
 		return -ENOMEM;
 
-	led->label = LP8860_DISP_LED_NAME;
-
-	if (client->dev.of_node) {
-		ret = of_property_read_string(np, "label", &led->label);
-		if (ret) {
-			dev_err(&client->dev, "Missing label in dt\n");
-			return -EINVAL;
-		}
+	for_each_available_child_of_node(np, child_node) {
+		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);
 	}
 
 	led->enable_gpio = devm_gpiod_get_optional(&client->dev,
-- 
2.15.0.124.g7668cbc60

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

* [PATCH v4 3/6] leds: lp8860: Update the dt parsing for LED labeling
@ 2017-12-12 22:01   ` Dan Murphy
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-12 22:01 UTC (permalink / raw)
  To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, linux-leds, Dan Murphy

Update the DT parsing for the label node so that
the label is retrieved from the device child as
opposed to being part of the parent.

This will align this driver with the LED
binding documentation

Documentation/devicetree/bindings/leds/common.txt

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v4 - Fix checkpatch warning for code indentation - https://patchwork.kernel.org/patch/10108157/

v3 - Changed the label generation to pull the name from the i2c device id 
as opposed to pulling the id from the parent dt node since that will just be 
led-controller - https://patchwork.kernel.org/patch/10093753/
v2 - no changes

 drivers/leds/leds-lp8860.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index 3e70775a2d54..46578fbc36be 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -22,6 +22,7 @@
 #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
@@ -86,8 +87,6 @@
 
 #define LP8860_CLEAR_FAULTS		0x01
 
-#define LP8860_DISP_LED_NAME		"display_cluster"
-
 /**
  * struct lp8860_led -
  * @lock - Lock for reading/writing the device
@@ -107,7 +106,7 @@ struct lp8860_led {
 	struct regmap *eeprom_regmap;
 	struct gpio_desc *enable_gpio;
 	struct regulator *regulator;
-	const char *label;
+	char label[LED_MAX_NAME_SIZE];
 };
 
 struct lp8860_eeprom_reg {
@@ -365,19 +364,21 @@ static int lp8860_probe(struct i2c_client *client,
 	int ret;
 	struct lp8860_led *led;
 	struct device_node *np = client->dev.of_node;
+	struct device_node *child_node;
+	const char *name;
 
 	led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL);
 	if (!led)
 		return -ENOMEM;
 
-	led->label = LP8860_DISP_LED_NAME;
-
-	if (client->dev.of_node) {
-		ret = of_property_read_string(np, "label", &led->label);
-		if (ret) {
-			dev_err(&client->dev, "Missing label in dt\n");
-			return -EINVAL;
-		}
+	for_each_available_child_of_node(np, child_node) {
+		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);
 	}
 
 	led->enable_gpio = devm_gpiod_get_optional(&client->dev,
-- 
2.15.0.124.g7668cbc60

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

* [PATCH v4 4/6] dt: bindings: lp8860: Add trigger binding to the lp8860
  2017-12-12 22:01 ` Dan Murphy
@ 2017-12-12 22:01     ` Dan Murphy
  -1 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-12 22:01 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	rpurdie-Fm38FmjxZ/leoWH0uzbU5w,
	jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w, pavel-+ZI9xUNit7I
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-leds-u79uwXL29TY76Z2rM5mHXA, Dan Murphy

Add a default trigger optional node to the child node.
This will allow the driver to set the trigger for a backlight.

Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>
---

v4 - No changes

v3 - Removed optional and rebased - https://patchwork.kernel.org/patch/10093755/
v2 - Moved binding changes to first patch in the series.

 Documentation/devicetree/bindings/leds/leds-lp8860.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/leds-lp8860.txt b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
index c3d64ade1e26..cdb9e12bd0aa 100644
--- a/Documentation/devicetree/bindings/leds/leds-lp8860.txt
+++ b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
@@ -21,6 +21,8 @@ Required child properties:
 
 Optional child properties:
 	- label : see Documentation/devicetree/bindings/leds/common.txt
+	- linux,default-trigger :
+	   see Documentation/devicetree/bindings/leds/common.txt
 
 Example:
 
@@ -35,6 +37,7 @@ led-controller@2d {
 	led@0 {
 		reg = <0>;
 		label = "white:display_cluster";
+		linux,default-trigger = "backlight";
 	};
 }
 
-- 
2.15.0.124.g7668cbc60

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH v4 4/6] dt: bindings: lp8860: Add trigger binding to the lp8860
@ 2017-12-12 22:01     ` Dan Murphy
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-12 22:01 UTC (permalink / raw)
  To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, linux-leds, Dan Murphy

Add a default trigger optional node to the child node.
This will allow the driver to set the trigger for a backlight.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v4 - No changes

v3 - Removed optional and rebased - https://patchwork.kernel.org/patch/10093755/
v2 - Moved binding changes to first patch in the series.

 Documentation/devicetree/bindings/leds/leds-lp8860.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/leds-lp8860.txt b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
index c3d64ade1e26..cdb9e12bd0aa 100644
--- a/Documentation/devicetree/bindings/leds/leds-lp8860.txt
+++ b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
@@ -21,6 +21,8 @@ Required child properties:
 
 Optional child properties:
 	- label : see Documentation/devicetree/bindings/leds/common.txt
+	- linux,default-trigger :
+	   see Documentation/devicetree/bindings/leds/common.txt
 
 Example:
 
@@ -35,6 +37,7 @@ led-controller@2d {
 	led@0 {
 		reg = <0>;
 		label = "white:display_cluster";
+		linux,default-trigger = "backlight";
 	};
 }
 
-- 
2.15.0.124.g7668cbc60

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

* [PATCH v4 5/6] leds: lp8860: Add DT parsing to retrieve the trigger node
  2017-12-12 22:01 ` Dan Murphy
@ 2017-12-12 22:01   ` Dan Murphy
  -1 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-12 22:01 UTC (permalink / raw)
  To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, linux-leds, Dan Murphy

Add the ability to parse the DT and set the default
trigger mode for the LED.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v4 - No changes

v3 - no changes - https://patchwork.kernel.org/patch/10093751/
v2 - no changes

 drivers/leds/leds-lp8860.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index 46578fbc36be..bb1d7bbc928c 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -372,6 +372,10 @@ static int lp8860_probe(struct i2c_client *client,
 		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",
-- 
2.15.0.124.g7668cbc60

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

* [PATCH v4 5/6] leds: lp8860: Add DT parsing to retrieve the trigger node
@ 2017-12-12 22:01   ` Dan Murphy
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-12 22:01 UTC (permalink / raw)
  To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, linux-leds, Dan Murphy

Add the ability to parse the DT and set the default
trigger mode for the LED.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v4 - No changes

v3 - no changes - https://patchwork.kernel.org/patch/10093751/
v2 - no changes

 drivers/leds/leds-lp8860.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index 46578fbc36be..bb1d7bbc928c 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -372,6 +372,10 @@ static int lp8860_probe(struct i2c_client *client,
 		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",
-- 
2.15.0.124.g7668cbc60

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

* [PATCH v4 6/6] leds: lp8860: Various fixes to align with LED framework
  2017-12-12 22:01 ` Dan Murphy
@ 2017-12-12 22:01   ` Dan Murphy
  -1 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-12 22:01 UTC (permalink / raw)
  To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, linux-leds, Dan Murphy

Update the driver to conform with the LED framework.
Use devm_led_classdev_register
Destroy mutex on exit
Remove dependency on CONFIG_OF in the driver and move
to the Kconfig
Update the MODULE_LICENSE to GPL v2
Remove setting of MAX brightness as the LED framework
does this.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v4 - No changes

v3 - no changes - https://patchwork.kernel.org/patch/10093747/
v2 - no changes

 drivers/leds/Kconfig       |  2 +-
 drivers/leds/leds-lp8860.c | 13 +++++--------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 318a28fd58fe..ac4d9d8bf96b 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -347,7 +347,7 @@ config LEDS_LP8788
 
 config LEDS_LP8860
 	tristate "LED support for the TI LP8860 4 channel LED driver"
-	depends on LEDS_CLASS && I2C
+	depends on LEDS_CLASS && I2C && OF
 	select REGMAP_I2C
 	help
 	  If you say yes here you get support for the TI LP8860 4 channel
diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index bb1d7bbc928c..c7716f774431 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -399,7 +399,6 @@ static int lp8860_probe(struct i2c_client *client,
 
 	led->client = client;
 	led->led_dev.name = led->label;
-	led->led_dev.max_brightness = LED_FULL;
 	led->led_dev.brightness_set_blocking = lp8860_brightness_set;
 
 	mutex_init(&led->lock);
@@ -426,7 +425,7 @@ static int lp8860_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 
-	ret = led_classdev_register(&client->dev, &led->led_dev);
+	ret = devm_led_classdev_register(&client->dev, &led->led_dev);
 	if (ret) {
 		dev_err(&client->dev, "led register err: %d\n", ret);
 		return ret;
@@ -440,8 +439,6 @@ static int lp8860_remove(struct i2c_client *client)
 	struct lp8860_led *led = i2c_get_clientdata(client);
 	int ret;
 
-	led_classdev_unregister(&led->led_dev);
-
 	if (led->enable_gpio)
 		gpiod_direction_output(led->enable_gpio, 0);
 
@@ -452,6 +449,8 @@ static int lp8860_remove(struct i2c_client *client)
 				"Failed to disable regulator\n");
 	}
 
+	mutex_destroy(&led->lock);
+
 	return 0;
 }
 
@@ -461,18 +460,16 @@ static const struct i2c_device_id lp8860_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, lp8860_id);
 
-#ifdef CONFIG_OF
 static const struct of_device_id of_lp8860_leds_match[] = {
 	{ .compatible = "ti,lp8860", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, of_lp8860_leds_match);
-#endif
 
 static struct i2c_driver lp8860_driver = {
 	.driver = {
 		.name	= "lp8860",
-		.of_match_table = of_match_ptr(of_lp8860_leds_match),
+		.of_match_table = of_lp8860_leds_match,
 	},
 	.probe		= lp8860_probe,
 	.remove		= lp8860_remove,
@@ -482,4 +479,4 @@ module_i2c_driver(lp8860_driver);
 
 MODULE_DESCRIPTION("Texas Instruments LP8860 LED driver");
 MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
-MODULE_LICENSE("GPL");
+MODULE_LICENSE("GPL v2");
-- 
2.15.0.124.g7668cbc60

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

* [PATCH v4 6/6] leds: lp8860: Various fixes to align with LED framework
@ 2017-12-12 22:01   ` Dan Murphy
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-12 22:01 UTC (permalink / raw)
  To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, linux-leds, Dan Murphy

Update the driver to conform with the LED framework.
Use devm_led_classdev_register
Destroy mutex on exit
Remove dependency on CONFIG_OF in the driver and move
to the Kconfig
Update the MODULE_LICENSE to GPL v2
Remove setting of MAX brightness as the LED framework
does this.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v4 - No changes

v3 - no changes - https://patchwork.kernel.org/patch/10093747/
v2 - no changes

 drivers/leds/Kconfig       |  2 +-
 drivers/leds/leds-lp8860.c | 13 +++++--------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 318a28fd58fe..ac4d9d8bf96b 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -347,7 +347,7 @@ config LEDS_LP8788
 
 config LEDS_LP8860
 	tristate "LED support for the TI LP8860 4 channel LED driver"
-	depends on LEDS_CLASS && I2C
+	depends on LEDS_CLASS && I2C && OF
 	select REGMAP_I2C
 	help
 	  If you say yes here you get support for the TI LP8860 4 channel
diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index bb1d7bbc928c..c7716f774431 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -399,7 +399,6 @@ static int lp8860_probe(struct i2c_client *client,
 
 	led->client = client;
 	led->led_dev.name = led->label;
-	led->led_dev.max_brightness = LED_FULL;
 	led->led_dev.brightness_set_blocking = lp8860_brightness_set;
 
 	mutex_init(&led->lock);
@@ -426,7 +425,7 @@ static int lp8860_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 
-	ret = led_classdev_register(&client->dev, &led->led_dev);
+	ret = devm_led_classdev_register(&client->dev, &led->led_dev);
 	if (ret) {
 		dev_err(&client->dev, "led register err: %d\n", ret);
 		return ret;
@@ -440,8 +439,6 @@ static int lp8860_remove(struct i2c_client *client)
 	struct lp8860_led *led = i2c_get_clientdata(client);
 	int ret;
 
-	led_classdev_unregister(&led->led_dev);
-
 	if (led->enable_gpio)
 		gpiod_direction_output(led->enable_gpio, 0);
 
@@ -452,6 +449,8 @@ static int lp8860_remove(struct i2c_client *client)
 				"Failed to disable regulator\n");
 	}
 
+	mutex_destroy(&led->lock);
+
 	return 0;
 }
 
@@ -461,18 +460,16 @@ static const struct i2c_device_id lp8860_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, lp8860_id);
 
-#ifdef CONFIG_OF
 static const struct of_device_id of_lp8860_leds_match[] = {
 	{ .compatible = "ti,lp8860", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, of_lp8860_leds_match);
-#endif
 
 static struct i2c_driver lp8860_driver = {
 	.driver = {
 		.name	= "lp8860",
-		.of_match_table = of_match_ptr(of_lp8860_leds_match),
+		.of_match_table = of_lp8860_leds_match,
 	},
 	.probe		= lp8860_probe,
 	.remove		= lp8860_remove,
@@ -482,4 +479,4 @@ module_i2c_driver(lp8860_driver);
 
 MODULE_DESCRIPTION("Texas Instruments LP8860 LED driver");
 MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
-MODULE_LICENSE("GPL");
+MODULE_LICENSE("GPL v2");
-- 
2.15.0.124.g7668cbc60

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

* Re: [PATCH v4 0/6] Updated LP8860 driver series
  2017-12-12 22:01 ` Dan Murphy
                   ` (6 preceding siblings ...)
  (?)
@ 2017-12-13 19:55 ` Jacek Anaszewski
  -1 siblings, 0 replies; 27+ messages in thread
From: Jacek Anaszewski @ 2017-12-13 19:55 UTC (permalink / raw)
  To: Dan Murphy, robh+dt, mark.rutland, rpurdie, pavel
  Cc: devicetree, linux-kernel, linux-leds

Dan,

Patch set applied, thanks.

-- 
Best regards,
Jacek Anaszewski

On 12/12/2017 11:01 PM, Dan Murphy wrote:
> All
> 
> v4 - Fix checkpatch warning for code indentation on the
> leds: lp8860: Update the dt parsing for LED labeling patch.
> 
> v3 - Made changes to the patch set to address concerns on DT node naming conventions
> based on discussion in the RFC patch https://patchwork.kernel.org/patch/10089047/
> also made requested changes to the DT and driver based on feedback.  Patchworks
> links in each patch.
> 
> v2 - Added an initial patch to bring the DT binding up to standard prior to adding
> the changes for the label and triggers.
> 
> v1 Cover letter repeat below
> 
> After creating a new LED driver for the LM3692x device I went back to the
> LP8860 driver that I authored and found some updates that need to be applied.
> 
> First the way the LP8860 retrieved the label from the DT was incorrect as the
> label should have been from a child node as opposed to the parent.  This is now
> fixed with this series.
> 
> Second, since that device can be used to as either a backlight driver or as a
> string agnostic driver a trigger to the backlight needed to be added.
> 
> Finally there are changes to the driver that need to be made as either
> unnoticed bugs or updates to the driver to align with the current LED
> framework.  For instance moving to the devm LED class registration, destroying
> the mutex upon driver removal and removing the in driver dependency on CONFIG_OF
> and moving it to the Kconfig.
> 
> With these changes this should at least bring the driver into a better shape.
> 
> There are additional changes coming for this driver but I wanted to get the
> driver up to snuff before adding a feature to it.
> 
> Dan
> 
> Dan Murphy (6):
>   dt: bindings: lp8860: Update bindings for lp8860
>   dt: bindings: lp8860: Update DT label binding
>   leds: lp8860: Update the dt parsing for LED labeling
>   dt: bindings: lp8860: Add trigger binding to the lp8860
>   leds: lp8860: Add DT parsing to retrieve the trigger node
>   leds: lp8860: Various fixes to align with LED framework
> 
>  .../devicetree/bindings/leds/leds-lp8860.txt       | 32 ++++++++++++-----
>  drivers/leds/Kconfig                               |  2 +-
>  drivers/leds/leds-lp8860.c                         | 40 ++++++++++++----------
>  3 files changed, 46 insertions(+), 28 deletions(-)
> 

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

* Re: [PATCH v4 4/6] dt: bindings: lp8860: Add trigger binding to the lp8860
  2017-12-12 22:01     ` Dan Murphy
@ 2017-12-15  9:09         ` Pavel Machek
  -1 siblings, 0 replies; 27+ messages in thread
From: Pavel Machek @ 2017-12-15  9:09 UTC (permalink / raw)
  To: Dan Murphy
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	rpurdie-Fm38FmjxZ/leoWH0uzbU5w,
	jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-leds-u79uwXL29TY76Z2rM5mHXA

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

Hi!

> @@ -35,6 +37,7 @@ led-controller@2d {
>  	led@0 {
>  		reg = <0>;
>  		label = "white:display_cluster";
> +		linux,default-trigger = "backlight";
>  	};
>  }

What is "display_cluster"? Is it display backlight?

Could we use ":backlight" and perhaps start documentation file listing
"common" LED descriptions so they are shared across machines?

For the record, I'd prefer this to be
"/sys/class/leds/display:white:backlight" in the end... so that we
have nice naming for userspace, consistent between different machines.

									Pavel
-- 
(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] 27+ messages in thread

* Re: [PATCH v4 4/6] dt: bindings: lp8860: Add trigger binding to the lp8860
@ 2017-12-15  9:09         ` Pavel Machek
  0 siblings, 0 replies; 27+ messages in thread
From: Pavel Machek @ 2017-12-15  9:09 UTC (permalink / raw)
  To: Dan Murphy
  Cc: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, devicetree,
	linux-kernel, linux-leds

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

Hi!

> @@ -35,6 +37,7 @@ led-controller@2d {
>  	led@0 {
>  		reg = <0>;
>  		label = "white:display_cluster";
> +		linux,default-trigger = "backlight";
>  	};
>  }

What is "display_cluster"? Is it display backlight?

Could we use ":backlight" and perhaps start documentation file listing
"common" LED descriptions so they are shared across machines?

For the record, I'd prefer this to be
"/sys/class/leds/display:white:backlight" in the end... so that we
have nice naming for userspace, consistent between different machines.

									Pavel
-- 
(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] 27+ messages in thread

* Re: [PATCH v4 4/6] dt: bindings: lp8860: Add trigger binding to the lp8860
  2017-12-15  9:09         ` Pavel Machek
@ 2017-12-15 17:22           ` Dan Murphy
  -1 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-15 17:22 UTC (permalink / raw)
  To: Pavel Machek
  Cc: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, devicetree,
	linux-kernel, linux-leds

Pavel

On 12/15/2017 03:09 AM, Pavel Machek wrote:
> Hi!
> 
>> @@ -35,6 +37,7 @@ led-controller@2d {
>>  	led@0 {
>>  		reg = <0>;
>>  		label = "white:display_cluster";
>> +		linux,default-trigger = "backlight";
>>  	};
>>  }
> 
> What is "display_cluster"? Is it display backlight?
> 

This device can be configured to drive all 4 LED strings in display mode
so that all 4 strings have the same brightness controlled either through PWM or
through the brightness register 

or

As cluster string where the brightness of individual strings or clustered strings
can be controlled via independent brightness control registers.  I am currently working
on adding this support to the driver.

Dan

> Could we use ":backlight" and perhaps start documentation file listing
> "common" LED descriptions so they are shared across machines?
> 
> For the record, I'd prefer this to be
> "/sys/class/leds/display:white:backlight" in the end... so that we
> have nice naming for userspace, consistent between different machines.
> 
> 									Pavel
> 


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

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

* Re: [PATCH v4 4/6] dt: bindings: lp8860: Add trigger binding to the lp8860
@ 2017-12-15 17:22           ` Dan Murphy
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-15 17:22 UTC (permalink / raw)
  To: Pavel Machek
  Cc: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, devicetree,
	linux-kernel, linux-leds

Pavel

On 12/15/2017 03:09 AM, Pavel Machek wrote:
> Hi!
> 
>> @@ -35,6 +37,7 @@ led-controller@2d {
>>  	led@0 {
>>  		reg = <0>;
>>  		label = "white:display_cluster";
>> +		linux,default-trigger = "backlight";
>>  	};
>>  }
> 
> What is "display_cluster"? Is it display backlight?
> 

This device can be configured to drive all 4 LED strings in display mode
so that all 4 strings have the same brightness controlled either through PWM or
through the brightness register 

or

As cluster string where the brightness of individual strings or clustered strings
can be controlled via independent brightness control registers.  I am currently working
on adding this support to the driver.

Dan

> Could we use ":backlight" and perhaps start documentation file listing
> "common" LED descriptions so they are shared across machines?
> 
> For the record, I'd prefer this to be
> "/sys/class/leds/display:white:backlight" in the end... so that we
> have nice naming for userspace, consistent between different machines.
> 
> 									Pavel
> 


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

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

* Re: [PATCH v4 4/6] dt: bindings: lp8860: Add trigger binding to the lp8860
  2017-12-15 17:22           ` Dan Murphy
@ 2017-12-15 21:00               ` Pavel Machek
  -1 siblings, 0 replies; 27+ messages in thread
From: Pavel Machek @ 2017-12-15 21:00 UTC (permalink / raw)
  To: Dan Murphy
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	rpurdie-Fm38FmjxZ/leoWH0uzbU5w,
	jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-leds-u79uwXL29TY76Z2rM5mHXA

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

On Fri 2017-12-15 11:22:24, Dan Murphy wrote:
> Pavel
> 
> On 12/15/2017 03:09 AM, Pavel Machek wrote:
> > Hi!
> > 
> >> @@ -35,6 +37,7 @@ led-controller@2d {
> >>  	led@0 {
> >>  		reg = <0>;
> >>  		label = "white:display_cluster";
> >> +		linux,default-trigger = "backlight";
> >>  	};
> >>  }
> > 
> > What is "display_cluster"? Is it display backlight?
> > 
> 
> This device can be configured to drive all 4 LED strings in display mode
> so that all 4 strings have the same brightness controlled either through PWM or
> through the brightness register 
> 
> or
> 
> As cluster string where the brightness of individual strings or clustered strings
> can be controlled via independent brightness control registers.  I am currently working
> on adding this support to the driver.

Ok; but I guess I'd like to keep this implementation detail away from
kernel, and just expose as display:white:backlight or something like that.

-- 
(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] 27+ messages in thread

* Re: [PATCH v4 4/6] dt: bindings: lp8860: Add trigger binding to the lp8860
@ 2017-12-15 21:00               ` Pavel Machek
  0 siblings, 0 replies; 27+ messages in thread
From: Pavel Machek @ 2017-12-15 21:00 UTC (permalink / raw)
  To: Dan Murphy
  Cc: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, devicetree,
	linux-kernel, linux-leds

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

On Fri 2017-12-15 11:22:24, Dan Murphy wrote:
> Pavel
> 
> On 12/15/2017 03:09 AM, Pavel Machek wrote:
> > Hi!
> > 
> >> @@ -35,6 +37,7 @@ led-controller@2d {
> >>  	led@0 {
> >>  		reg = <0>;
> >>  		label = "white:display_cluster";
> >> +		linux,default-trigger = "backlight";
> >>  	};
> >>  }
> > 
> > What is "display_cluster"? Is it display backlight?
> > 
> 
> This device can be configured to drive all 4 LED strings in display mode
> so that all 4 strings have the same brightness controlled either through PWM or
> through the brightness register 
> 
> or
> 
> As cluster string where the brightness of individual strings or clustered strings
> can be controlled via independent brightness control registers.  I am currently working
> on adding this support to the driver.

Ok; but I guess I'd like to keep this implementation detail away from
kernel, and just expose as display:white:backlight or something like that.

-- 
(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] 27+ messages in thread

* Re: [PATCH v4 1/6] dt: bindings: lp8860: Update bindings for lp8860
  2017-12-12 22:01   ` Dan Murphy
  (?)
@ 2017-12-15 22:56   ` Rob Herring
  -1 siblings, 0 replies; 27+ messages in thread
From: Rob Herring @ 2017-12-15 22:56 UTC (permalink / raw)
  To: Dan Murphy
  Cc: mark.rutland, rpurdie, jacek.anaszewski, pavel, devicetree,
	linux-kernel, linux-leds

On Tue, Dec 12, 2017 at 04:01:38PM -0600, Dan Murphy wrote:
> Update the lp8860 bindings to fix various issues
> found.  Add address-cells and size-cells, rename
> enable-gpio to enable-gpios, update the node name
> to the device name and indent the node example.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v4 - No changes
> 
> v3 - Indicatd enable-gpios is active high, moved address and size cells to child
> node patch and updated parent DT node name - https://patchwork.kernel.org/patch/10093745/
> v2 - New patch
> 
>  Documentation/devicetree/bindings/leds/leds-lp8860.txt | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

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

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

* Re: [PATCH v4 1/6] dt: bindings: lp8860: Update bindings for lp8860
  2017-12-12 22:01   ` Dan Murphy
  (?)
  (?)
@ 2017-12-15 22:57   ` Rob Herring
  -1 siblings, 0 replies; 27+ messages in thread
From: Rob Herring @ 2017-12-15 22:57 UTC (permalink / raw)
  To: Dan Murphy
  Cc: mark.rutland, rpurdie, jacek.anaszewski, pavel, devicetree,
	linux-kernel, linux-leds

On Tue, Dec 12, 2017 at 04:01:38PM -0600, Dan Murphy wrote:
> Update the lp8860 bindings to fix various issues
> found.  Add address-cells and size-cells, rename

You are doing this in the next patch...

> enable-gpio to enable-gpios, update the node name
> to the device name and indent the node example.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v4 - No changes
> 
> v3 - Indicatd enable-gpios is active high, moved address and size cells to child
> node patch and updated parent DT node name - https://patchwork.kernel.org/patch/10093745/
> v2 - New patch
> 
>  Documentation/devicetree/bindings/leds/leds-lp8860.txt | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-lp8860.txt b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
> index aad38dd94d4b..b9d09acbaa73 100644
> --- a/Documentation/devicetree/bindings/leds/leds-lp8860.txt
> +++ b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
> @@ -6,22 +6,22 @@ current sinks that can be controlled by a PWM input
>  signal, a SPI/I2C master, or both.
>  
>  Required properties:
> -	- compatible:
> +	- compatible :
>  		"ti,lp8860"
> -	- reg -  I2C slave address
> -	- label - Used for naming LEDs
> +	- reg : I2C slave address
> +	- label : Used for naming LEDs
>  
>  Optional properties:
> -	- enable-gpio - gpio pin to enable/disable the device.
> -	- supply - "vled" - LED supply
> +	- enable-gpios : gpio pin to enable (active high)/disable the device.
> +	- vled-supply : LED supply
>  
>  Example:
>  
> -leds: leds@6 {
> +led-controller@2d {
>  	compatible = "ti,lp8860";
>  	reg = <0x2d>;
>  	label = "display_cluster";
> -	enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>;
> +	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
>  	vled-supply = <&vbatt>;
>  }
>  
> -- 
> 2.15.0.124.g7668cbc60
> 

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

* Re: [PATCH v4 2/6] dt: bindings: lp8860: Update DT label binding
  2017-12-12 22:01   ` Dan Murphy
@ 2017-12-15 22:59       ` Rob Herring
  -1 siblings, 0 replies; 27+ messages in thread
From: Rob Herring @ 2017-12-15 22:59 UTC (permalink / raw)
  To: Dan Murphy
  Cc: mark.rutland-5wv7dgnIgG8, rpurdie-Fm38FmjxZ/leoWH0uzbU5w,
	jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w, pavel-+ZI9xUNit7I,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-leds-u79uwXL29TY76Z2rM5mHXA

On Tue, Dec 12, 2017 at 04:01:39PM -0600, Dan Murphy wrote:
> Update the lp8860 label binding to the LED
> standard as documented in
> 
> Documentation/devicetree/bindings/leds/common.txt
> 
> Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>
> ---
> 
> v4 - No changes
> 
> v3 - Added address and size cells, updated label with color and inserted spaces
> around the reg node - https://patchwork.kernel.org/patch/10093749/
> v2 - Added reg to child node and made it required
> 
>  Documentation/devicetree/bindings/leds/leds-lp8860.txt | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-lp8860.txt b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
> index b9d09acbaa73..c3d64ade1e26 100644
> --- a/Documentation/devicetree/bindings/leds/leds-lp8860.txt
> +++ b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
> @@ -9,20 +9,33 @@ Required properties:
>  	- compatible :
>  		"ti,lp8860"
>  	- reg : I2C slave address
> -	- label : Used for naming LEDs
> +	- #address-cells : 1
> +	- #size-cells : 0
>  
>  Optional properties:
>  	- enable-gpios : gpio pin to enable (active high)/disable the device.
>  	- vled-supply : LED supply
>  
> +Required child properties:
> +	- reg : 0
> +

This should be in previous patch?

> +Optional child properties:
> +	- label : see Documentation/devicetree/bindings/leds/common.txt
> +
>  Example:
>  
>  led-controller@2d {
>  	compatible = "ti,lp8860";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
>  	reg = <0x2d>;
> -	label = "display_cluster";
>  	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
>  	vled-supply = <&vbatt>;
> +
> +	led@0 {
> +		reg = <0>;
> +		label = "white:display_cluster";
> +	};
>  }
>  
>  For more product information please see the link below:
> -- 
> 2.15.0.124.g7668cbc60
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v4 2/6] dt: bindings: lp8860: Update DT label binding
@ 2017-12-15 22:59       ` Rob Herring
  0 siblings, 0 replies; 27+ messages in thread
From: Rob Herring @ 2017-12-15 22:59 UTC (permalink / raw)
  To: Dan Murphy
  Cc: mark.rutland, rpurdie, jacek.anaszewski, pavel, devicetree,
	linux-kernel, linux-leds

On Tue, Dec 12, 2017 at 04:01:39PM -0600, Dan Murphy wrote:
> Update the lp8860 label binding to the LED
> standard as documented in
> 
> Documentation/devicetree/bindings/leds/common.txt
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v4 - No changes
> 
> v3 - Added address and size cells, updated label with color and inserted spaces
> around the reg node - https://patchwork.kernel.org/patch/10093749/
> v2 - Added reg to child node and made it required
> 
>  Documentation/devicetree/bindings/leds/leds-lp8860.txt | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-lp8860.txt b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
> index b9d09acbaa73..c3d64ade1e26 100644
> --- a/Documentation/devicetree/bindings/leds/leds-lp8860.txt
> +++ b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
> @@ -9,20 +9,33 @@ Required properties:
>  	- compatible :
>  		"ti,lp8860"
>  	- reg : I2C slave address
> -	- label : Used for naming LEDs
> +	- #address-cells : 1
> +	- #size-cells : 0
>  
>  Optional properties:
>  	- enable-gpios : gpio pin to enable (active high)/disable the device.
>  	- vled-supply : LED supply
>  
> +Required child properties:
> +	- reg : 0
> +

This should be in previous patch?

> +Optional child properties:
> +	- label : see Documentation/devicetree/bindings/leds/common.txt
> +
>  Example:
>  
>  led-controller@2d {
>  	compatible = "ti,lp8860";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
>  	reg = <0x2d>;
> -	label = "display_cluster";
>  	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
>  	vled-supply = <&vbatt>;
> +
> +	led@0 {
> +		reg = <0>;
> +		label = "white:display_cluster";
> +	};
>  }
>  
>  For more product information please see the link below:
> -- 
> 2.15.0.124.g7668cbc60
> 

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

* Re: [PATCH v4 2/6] dt: bindings: lp8860: Update DT label binding
  2017-12-15 22:59       ` Rob Herring
@ 2017-12-18 13:08         ` Dan Murphy
  -1 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-18 13:08 UTC (permalink / raw)
  To: Rob Herring
  Cc: mark.rutland, rpurdie, jacek.anaszewski, pavel, devicetree,
	linux-kernel, linux-leds

Rob

On 12/15/2017 04:59 PM, Rob Herring wrote:
> On Tue, Dec 12, 2017 at 04:01:39PM -0600, Dan Murphy wrote:
>> Update the lp8860 label binding to the LED
>> standard as documented in
>>
>> Documentation/devicetree/bindings/leds/common.txt
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>>
>> v4 - No changes
>>
>> v3 - Added address and size cells, updated label with color and inserted spaces
>> around the reg node - https://patchwork.kernel.org/patch/10093749/
>> v2 - Added reg to child node and made it required
>>
>>  Documentation/devicetree/bindings/leds/leds-lp8860.txt | 17 +++++++++++++++--
>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/leds-lp8860.txt b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
>> index b9d09acbaa73..c3d64ade1e26 100644
>> --- a/Documentation/devicetree/bindings/leds/leds-lp8860.txt
>> +++ b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
>> @@ -9,20 +9,33 @@ Required properties:
>>  	- compatible :
>>  		"ti,lp8860"
>>  	- reg : I2C slave address
>> -	- label : Used for naming LEDs
>> +	- #address-cells : 1
>> +	- #size-cells : 0
>>  
>>  Optional properties:
>>  	- enable-gpios : gpio pin to enable (active high)/disable the device.
>>  	- vled-supply : LED supply
>>  
>> +Required child properties:
>> +	- reg : 0
>> +
> 
> This should be in previous patch?


No this is the reg node for the child property.  The child property is
introduced in this patch.  So this should be correct

Dan

> 
>> +Optional child properties:
>> +	- label : see Documentation/devicetree/bindings/leds/common.txt
>> +
>>  Example:
>>  
>>  led-controller@2d {
>>  	compatible = "ti,lp8860";
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>>  	reg = <0x2d>;
>> -	label = "display_cluster";
>>  	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
>>  	vled-supply = <&vbatt>;
>> +
>> +	led@0 {
>> +		reg = <0>;
>> +		label = "white:display_cluster";
>> +	};
>>  }
>>  
>>  For more product information please see the link below:
>> -- 
>> 2.15.0.124.g7668cbc60
>>


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

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

* Re: [PATCH v4 2/6] dt: bindings: lp8860: Update DT label binding
@ 2017-12-18 13:08         ` Dan Murphy
  0 siblings, 0 replies; 27+ messages in thread
From: Dan Murphy @ 2017-12-18 13:08 UTC (permalink / raw)
  To: Rob Herring
  Cc: mark.rutland, rpurdie, jacek.anaszewski, pavel, devicetree,
	linux-kernel, linux-leds

Rob

On 12/15/2017 04:59 PM, Rob Herring wrote:
> On Tue, Dec 12, 2017 at 04:01:39PM -0600, Dan Murphy wrote:
>> Update the lp8860 label binding to the LED
>> standard as documented in
>>
>> Documentation/devicetree/bindings/leds/common.txt
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>>
>> v4 - No changes
>>
>> v3 - Added address and size cells, updated label with color and inserted spaces
>> around the reg node - https://patchwork.kernel.org/patch/10093749/
>> v2 - Added reg to child node and made it required
>>
>>  Documentation/devicetree/bindings/leds/leds-lp8860.txt | 17 +++++++++++++++--
>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/leds-lp8860.txt b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
>> index b9d09acbaa73..c3d64ade1e26 100644
>> --- a/Documentation/devicetree/bindings/leds/leds-lp8860.txt
>> +++ b/Documentation/devicetree/bindings/leds/leds-lp8860.txt
>> @@ -9,20 +9,33 @@ Required properties:
>>  	- compatible :
>>  		"ti,lp8860"
>>  	- reg : I2C slave address
>> -	- label : Used for naming LEDs
>> +	- #address-cells : 1
>> +	- #size-cells : 0
>>  
>>  Optional properties:
>>  	- enable-gpios : gpio pin to enable (active high)/disable the device.
>>  	- vled-supply : LED supply
>>  
>> +Required child properties:
>> +	- reg : 0
>> +
> 
> This should be in previous patch?


No this is the reg node for the child property.  The child property is
introduced in this patch.  So this should be correct

Dan

> 
>> +Optional child properties:
>> +	- label : see Documentation/devicetree/bindings/leds/common.txt
>> +
>>  Example:
>>  
>>  led-controller@2d {
>>  	compatible = "ti,lp8860";
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>>  	reg = <0x2d>;
>> -	label = "display_cluster";
>>  	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
>>  	vled-supply = <&vbatt>;
>> +
>> +	led@0 {
>> +		reg = <0>;
>> +		label = "white:display_cluster";
>> +	};
>>  }
>>  
>>  For more product information please see the link below:
>> -- 
>> 2.15.0.124.g7668cbc60
>>


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

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

end of thread, other threads:[~2017-12-18 13:08 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-12 22:01 [PATCH v4 0/6] Updated LP8860 driver series Dan Murphy
2017-12-12 22:01 ` Dan Murphy
2017-12-12 22:01 ` [PATCH v4 1/6] dt: bindings: lp8860: Update bindings for lp8860 Dan Murphy
2017-12-12 22:01   ` Dan Murphy
2017-12-15 22:56   ` Rob Herring
2017-12-15 22:57   ` Rob Herring
2017-12-12 22:01 ` [PATCH v4 2/6] dt: bindings: lp8860: Update DT label binding Dan Murphy
2017-12-12 22:01   ` Dan Murphy
     [not found]   ` <20171212220143.31210-3-dmurphy-l0cyMroinI0@public.gmane.org>
2017-12-15 22:59     ` Rob Herring
2017-12-15 22:59       ` Rob Herring
2017-12-18 13:08       ` Dan Murphy
2017-12-18 13:08         ` Dan Murphy
2017-12-12 22:01 ` [PATCH v4 3/6] leds: lp8860: Update the dt parsing for LED labeling Dan Murphy
2017-12-12 22:01   ` Dan Murphy
     [not found] ` <20171212220143.31210-1-dmurphy-l0cyMroinI0@public.gmane.org>
2017-12-12 22:01   ` [PATCH v4 4/6] dt: bindings: lp8860: Add trigger binding to the lp8860 Dan Murphy
2017-12-12 22:01     ` Dan Murphy
     [not found]     ` <20171212220143.31210-5-dmurphy-l0cyMroinI0@public.gmane.org>
2017-12-15  9:09       ` Pavel Machek
2017-12-15  9:09         ` Pavel Machek
2017-12-15 17:22         ` Dan Murphy
2017-12-15 17:22           ` Dan Murphy
     [not found]           ` <76329432-f96e-93ab-ed9b-ade22c8735c9-l0cyMroinI0@public.gmane.org>
2017-12-15 21:00             ` Pavel Machek
2017-12-15 21:00               ` Pavel Machek
2017-12-12 22:01 ` [PATCH v4 5/6] leds: lp8860: Add DT parsing to retrieve the trigger node Dan Murphy
2017-12-12 22:01   ` Dan Murphy
2017-12-12 22:01 ` [PATCH v4 6/6] leds: lp8860: Various fixes to align with LED framework Dan Murphy
2017-12-12 22:01   ` Dan Murphy
2017-12-13 19:55 ` [PATCH v4 0/6] Updated LP8860 driver series Jacek Anaszewski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.