Skip to contentSkip to conversation

leds: lm3692x: Allow to set ovp and brigthness mode

PATCH Superseded Guido Günther 2 participants 13 messages Dec 25, 2019 – Dec 27, 2019

Subsystems (2)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v2 cover leds: lm3692x: Allow to set ovp and brigthness mode

Overvoltage protection and brightness mode are currently hardcoded as 29V and disabled in the driver. Make these configurable via DT.

Besides addressing review comments v2 also allows to limit the maximum led current.

Changes from v1
- As per review comments by Dan Murphy
https://lore.kernel.org/linux-leds/[email protected]/
- Split commits per propoerty
- Add new properties to DT example too
- Drop dev_dbg() statements
- ovp: fix 21V value parsing
- ovp: Set correct default value if DT parsing fails
- As per review comments by Pavel Machek
https://lore.kernel.org/linux-leds/20191221191515.GF32732@amd/
- Fix defaults (which is 29V)
- Use uV as Unit for ovp property
- Change property name to 'ti,ovp-microvolt' to make it shorter
- Honor led-max-microamp to not exceed the maximum led current

To: Jacek Anaszewski <[email protected]>,Pavel Machek <[email protected]>,Dan Murphy <[email protected]>,Rob Herring <[email protected]>,Mark Rutland <[email protected]>,[email protected],[email protected],[email protected]

Guido Günther (6):
dt: bindings: lm3692x: Add ti,ovp-microvolt property
leds: lm3692x: Allow to configure over voltage protection
dt: bindings: lm3692x: Add ti,brightness-mapping-exponential property
leds: lm3692x: Allow to configure brigthness mode
dt: bindings: lm3692x: Add led-max-microamp property
leds: lm3692x: Make sure we don't exceed the maximum led current

 .../devicetree/bindings/leds/leds-lm3692x.txt | 11 +++
 drivers/leds/leds-lm3692x.c                   | 67 +++++++++++++++++--
 2 files changed, 72 insertions(+), 6 deletions(-)
Signature · 1 line
2.23.0
#1 lore raw

PATCH v2 1/6 dt: bindings: lm3692x: Add ti,ovp-microvolt property

This allows to set the overvoltage protection to 17V, 21V, 25V or 29V.

---
 Documentation/devicetree/bindings/leds/leds-lm3692x.txt | 5 +++++
 1 file changed, 5 insertions(+)
Documentation/devicetree/bindings/leds/leds-lm3692x.txt +5 -0
index 4c2d923f8758..95f6d8c19f20 100644
@@ -18,6 +18,10 @@ Required properties:
 Optional properties:
 	- enable-gpios : gpio pin to enable/disable the device.
 	- vled-supply : LED supply
+	- ti,ovp-microvolt: Overvoltage protection in
+	    micro-volt, can be 17000000, 21000000, 25000000 or
+	    29000000. If ti,ovp-microvolt is not specified it
+	    defaults to 29000000.
 
 Required child properties:
 	- reg : 0 - Will enable all LED sync paths
@@ -44,6 +48,7 @@ led-controller@36 {
 
 	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
 	vled-supply = <&vbatt>;
+	ti,ovp-microvolt = <25000000>;
 
 	led@0 {
 		reg = <0>;
Signature · 1 line
2.23.0
Acked-by on v2 1/6
#2 lore raw

Re: [PATCH v2 1/6] dt: bindings: lm3692x: Add ti,ovp-microvolt property

Quoted 3 lines · Guido Günther

This allows to set the overvoltage protection to 17V, 21V, 25V or 29V.

I will need Rob's Ack.

Documentation/devicetree/bindings/leds/leds-lm3692x.txt led-controller@36 { · line 51

Quoted 5 lines
Documentation/devicetree/bindings/leds/leds-lm3692x.txt +1 -0
@@ -44,6 +48,7 @@ led-controller@36 {
 
 	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
 	vled-supply = <&vbatt>;
+	ti,ovp-microvolt = <25000000>;

Wed often put the default values here.
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#3 lore raw

Re: [PATCH v2 1/6] dt: bindings: lm3692x: Add ti,ovp-microvolt property

Hi,

Quoted 6 lines · Pavel Machek
Quoted 3 lines · Guido Günther

This allows to set the overvoltage protection to 17V, 21V, 25V or 29V.

Thanks!

Quoted 9 lines

I will need Rob's Ack.

Documentation/devicetree/bindings/leds/leds-lm3692x.txt led-controller@36 { · line 51

Quoted 5 lines
Documentation/devicetree/bindings/leds/leds-lm3692x.txt +1 -0
@@ -44,6 +48,7 @@ led-controller@36 {
 
 	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
 	vled-supply = <&vbatt>;
+	ti,ovp-microvolt = <25000000>;

Wed often put the default values here.

Makes sense, thanks.
-- Guido

Quoted 4 lines

Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#4 lore raw

PATCH v2 2/6 leds: lm3692x: Allow to configure over voltage protection

In reply to Guido Günther

Overvoltage protection is currently using the default of 29V. Make it configurable via DT.

---
 drivers/leds/leds-lm3692x.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)
drivers/leds/leds-lm3692x.c +30 -4
index 8b408102e138..28973cc5a6cc 100644
@@ -114,6 +114,8 @@ struct lm3692x_led {
 	struct regulator *regulator;
 	int led_enable;
 	int model_id;
+
+	u8 boost_ctrl;
 };
 
 static const struct reg_default lm3692x_reg_defs[] = {
@@ -249,10 +251,7 @@ static int lm3692x_init(struct lm3692x_led *led)
 	if (ret)
 		goto out;
 
-	ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL,
-			LM3692X_BOOST_SW_1MHZ |
-			LM3692X_BOOST_SW_NO_SHIFT |
-			LM3692X_OCP_PROT_1_5A);
+	ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL, led->boost_ctrl);
 	if (ret)
 		goto out;
 
@@ -326,6 +325,7 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
 {
 	struct fwnode_handle *child = NULL;
 	struct led_init_data init_data = {};
+	u32 ovp;
 	int ret;
 
 	led->enable_gpio = devm_gpiod_get_optional(&led->client->dev,
@@ -350,6 +350,32 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
 		led->regulator = NULL;
 	}
 
+	led->boost_ctrl = LM3692X_BOOST_SW_1MHZ |
+		LM3692X_BOOST_SW_NO_SHIFT |
+		LM3692X_OCP_PROT_1_5A;
+	ret = device_property_read_u32(&led->client->dev,
+				       "ti,ovp-microvolt", &ovp);
+	if (ret) {
+		led->boost_ctrl |= LM3692X_OVP_29V;
+	} else {
+		switch (ovp) {
+		case 17000000:
+			break;
+		case 21000000:
+			led->boost_ctrl |= LM3692X_OVP_21V;
+			break;
+		case 25000000:
+			led->boost_ctrl |= LM3692X_OVP_25V;
+			break;
+		case 29000000:
+			led->boost_ctrl |= LM3692X_OVP_29V;
+			break;
+		default:
+			dev_err(&led->client->dev, "Invalid OVP %d\n", ovp);
+			return -EINVAL;
+		}
+	}
+
 	child = device_get_next_child_node(&led->client->dev, child);
 	if (!child) {
 		dev_err(&led->client->dev, "No LED Child node\n");
Signature · 1 line
2.23.0
Acked-by on v2 2/6
#5 lore raw

Re: [PATCH v2 2/6] leds: lm3692x: Allow to configure over voltage protection

Quoted 4 lines · Guido Günther

Overvoltage protection is currently using the default of 29V. Make it configurable via DT.

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#6 lore raw

PATCH v2 3/6 dt: bindings: lm3692x: Add ti,brightness-mapping-exponential property

In reply to Guido Günther

This allows to select exponential brightness mode instead of the default linear mode.

---
 Documentation/devicetree/bindings/leds/leds-lm3692x.txt | 3 +++
 1 file changed, 3 insertions(+)
Documentation/devicetree/bindings/leds/leds-lm3692x.txt +3 -0
index 95f6d8c19f20..73f41249ecd3 100644
@@ -18,6 +18,8 @@ Required properties:
 Optional properties:
 	- enable-gpios : gpio pin to enable/disable the device.
 	- vled-supply : LED supply
+	- ti,brightness-mapping-exponential: Whether to use exponential
+	    brightness mapping
 	- ti,ovp-microvolt: Overvoltage protection in
 	    micro-volt, can be 17000000, 21000000, 25000000 or
 	    29000000. If ti,ovp-microvolt is not specified it
@@ -49,6 +51,7 @@ led-controller@36 {
 	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
 	vled-supply = <&vbatt>;
 	ti,ovp-microvolt = <25000000>;
+	ti,brightness-mapping-exponential;
 
 	led@0 {
 		reg = <0>;
Signature · 1 line
2.23.0
#7 lore raw

PATCH v2 4/6 leds: lm3692x: Allow to configure brigthness mode

In reply to Guido Günther

Brightness mode is currently hardcoded as linear in the driver. Make exponential mode configurable via DT.

---
 drivers/leds/leds-lm3692x.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
drivers/leds/leds-lm3692x.c +9 -3
index 28973cc5a6cc..ff20560a8263 100644
@@ -115,7 +115,7 @@ struct lm3692x_led {
 	int led_enable;
 	int model_id;
 
-	u8 boost_ctrl;
+	u8 boost_ctrl, brightness_ctrl;
 };
 
 static const struct reg_default lm3692x_reg_defs[] = {
@@ -267,8 +267,7 @@ static int lm3692x_init(struct lm3692x_led *led)
 	if (ret)
 		goto out;
 
-	ret = regmap_write(led->regmap, LM3692X_BRT_CTRL,
-			LM3692X_BL_ADJ_POL | LM3692X_RAMP_EN);
+	ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, led->brightness_ctrl);
 	if (ret)
 		goto out;
 
@@ -326,6 +325,7 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
 	struct fwnode_handle *child = NULL;
 	struct led_init_data init_data = {};
 	u32 ovp;
+	bool exp_mode;
 	int ret;
 
 	led->enable_gpio = devm_gpiod_get_optional(&led->client->dev,
@@ -376,6 +376,12 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
 		}
 	}
 
+	led->brightness_ctrl = LM3692X_BL_ADJ_POL | LM3692X_RAMP_EN;
+	exp_mode = device_property_read_bool(&led->client->dev,
+				     "ti,brightness-mapping-exponential");
+	if (exp_mode)
+		led->brightness_ctrl |= LM3692X_MAP_MODE_EXP;
+
 	child = device_get_next_child_node(&led->client->dev, child);
 	if (!child) {
 		dev_err(&led->client->dev, "No LED Child node\n");
Signature · 1 line
2.23.0
#8 lore raw

PATCH v2 5/6 dt: bindings: lm3692x: Add led-max-microamp property

In reply to Guido Günther

This can be used to limit the current per led strip

---
 Documentation/devicetree/bindings/leds/leds-lm3692x.txt | 3 +++
 1 file changed, 3 insertions(+)
Documentation/devicetree/bindings/leds/leds-lm3692x.txt +3 -0
index 73f41249ecd3..e0d7b3030c01 100644
@@ -37,6 +37,8 @@ Optional child properties:
 	- label : see Documentation/devicetree/bindings/leds/common.txt (deprecated)
 	- linux,default-trigger :
 	   see Documentation/devicetree/bindings/leds/common.txt
+	- led-max-microamp :
+	   see Documentation/devicetree/bindings/leds/common.txt
 
 Example:
 
@@ -58,6 +60,7 @@ led-controller@36 {
 		function = LED_FUNCTION_BACKLIGHT;
 		color = <LED_COLOR_ID_WHITE>;
 		linux,default-trigger = "backlight";
+		led-max-microamp = <20000>;
 	};
 }
 
Signature · 1 line
2.23.0
Acked-by on v2 5/6
#9 lore raw

Re: [PATCH v2 5/6] dt: bindings: lm3692x: Add led-max-microamp property

Quoted 1 line · Guido Günther

This can be used to limit the current per led strip

"per LED strip."

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#10 lore raw

PATCH v2 6/6 leds: lm3692x: Make sure we don't exceed the maximum led current

In reply to Guido Günther

The current is given by the formular from page 12 of
https://www.ti.com/lit/ds/symlink/lm36922.pdf. We use this to limit the led's max_brightness using the led-max-microamp DT property.

The formular for the lm36923 is identical according to the data sheet.

---
 drivers/leds/leds-lm3692x.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)
drivers/leds/leds-lm3692x.c +24 -1
index ff20560a8263..d7e5de8fe8db 100644
@@ -6,6 +6,7 @@
 #include <linux/i2c.h>
 #include <linux/init.h>
 #include <linux/leds.h>
+#include <linux/log2.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
@@ -320,11 +321,29 @@ static int lm3692x_init(struct lm3692x_led *led)
 	return ret;
 }
 
+static enum led_brightness lm3692x_max_brightness(struct lm3692x_led *led,
+						  u32 max_cur)
+{
+	u32 max_code;
+
+	/* see p.12 of LM36922 data sheet for brightness formula */
+	if (led->brightness_ctrl & LM3692X_MAP_MODE_EXP) {
+		/*  228 =~ 1.0 / log2(1.003040572) */
+		max_code = ilog2(max_cur/50) * 228;
+	} else {
+		max_code = ((max_cur * 1000) - 37806) / 12195;
+	}
+	if (max_code > 0x7FF)
+		max_code = 0x7FF;
+
+	return max_code >> 3;
+}
+
 static int lm3692x_probe_dt(struct lm3692x_led *led)
 {
 	struct fwnode_handle *child = NULL;
 	struct led_init_data init_data = {};
-	u32 ovp;
+	u32 ovp, max_cur;
 	bool exp_mode;
 	int ret;
 
@@ -397,6 +416,10 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
 		return ret;
 	}
 
+	fwnode_property_read_u32(child, "led-max-microamp", &max_cur);
+	led->led_dev.max_brightness = ret ? LED_FULL :
+		lm3692x_max_brightness(led, max_cur);
+
 	init_data.fwnode = child;
 	init_data.devicename = led->client->name;
 	init_data.default_label = ":";
Signature · 1 line
2.23.0
Acked-by on v2 6/6
#11 lore raw

Re: [PATCH v2 6/6] leds: lm3692x: Make sure we don't exceed the maximum led current

On Wed 2019-12-25 12:07:19, Guido Günther wrote:
1;2802;0c> The current is given by the formular from page 12 of

Quoted 4 lines

https://www.ti.com/lit/ds/symlink/lm36922.pdf. We use this to limit the led's max_brightness using the led-max-microamp DT property.

The formular for the lm36923 is identical according to the data

sheet.

formula?

Quoted 10 lines

static int lm3692x_probe_dt(struct lm3692x_led *led) {

drivers/leds/leds-lm3692x.c +1 -1
 	struct fwnode_handle *child = NULL;
 	struct led_init_data init_data = {};
-	u32 ovp;
+	u32 ovp, max_cur;
 	bool exp_mode;
 	int ret;

drivers/leds/leds-lm3692x.c static int lm3692x_probe_dt(struct lm3692x_led *led) · line 422

Quoted 7 lines
drivers/leds/leds-lm3692x.c +4 -0
@@ -397,6 +416,10 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
 		return ret;
 	}
 
+	fwnode_property_read_u32(child, "led-max-microamp", &max_cur);
+	led->led_dev.max_brightness = ret ? LED_FULL :
+		lm3692x_max_brightness(led, max_cur);
+

Umm. Should ret come from this fwnode_property_read_u32()?

With that fixed,

(Feel free to wait for Rob before resending the series, and I guess you can merge it with the next one).

Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#12 lore raw

Re: [PATCH v2 6/6] leds: lm3692x: Make sure we don't exceed the maximum led current

Hi,

Quoted 23 lines · Pavel Machek

On Wed 2019-12-25 12:07:19, Guido Günther wrote:
1;2802;0c> The current is given by the formular from page 12 of

Quoted 4 lines

https://www.ti.com/lit/ds/symlink/lm36922.pdf. We use this to limit the led's max_brightness using the led-max-microamp DT property.

The formular for the lm36923 is identical according to the data

sheet.

formula?

Quoted 13 lines

static int lm3692x_probe_dt(struct lm3692x_led *led) {

drivers/leds/leds-lm3692x.c +1 -1
 	struct fwnode_handle *child = NULL;
 	struct led_init_data init_data = {};
-	u32 ovp;
+	u32 ovp, max_cur;
 	bool exp_mode;
 	int ret;
@@ -397,6 +416,10 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
 		return ret;
 	}
 
Quoted 6 lines

drivers/leds/leds-lm3692x.c static int lm3692x_probe_dt(struct lm3692x_led *led) · line 422

Quoted 4 lines
drivers/leds/leds-lm3692x.c +4 -0
+	fwnode_property_read_u32(child, "led-max-microamp", &max_cur);
+	led->led_dev.max_brightness = ret ? LED_FULL :
+		lm3692x_max_brightness(led, max_cur);
+

Umm. Should ret come from this fwnode_property_read_u32()?

Argh...i was sure i had that fixed (and tested without setting led-max-microamp) but it was sitting on another branch. Thanks a lot for catching that!
-- Guido

Quoted 13 lines

With that fixed,

(Feel free to wait for Rob before resending the series, and I guess you can merge it with the next one).

Best regards,
Pavel

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