linux-leds.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode
@ 2020-01-04 10:54 Guido Günther
  2020-01-04 10:54 ` [PATCH v3 1/9] dt: bindings: lm3692x: Add ti,ovp-microvolt property Guido Günther
                   ` (9 more replies)
  0 siblings, 10 replies; 16+ messages in thread
From: Guido Günther @ 2020-01-04 10:54 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

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

Besides addressing review comments v3 folds in the patches to
disable the chip and turn of the regulator on brightness 0 from

  https://lore.kernel.org/linux-leds/20191226101419.GE4033@amd/T/#t

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

Patches are against next-20191220.

Changes from v2
- As per review comment from Pavel Machek
  https://lore.kernel.org/linux-leds/20191226100615.GA4033@amd/T/#u
  - Use default value in DT example
  https://lore.kernel.org/linux-leds/20191226100842.GC4033@amd/T/#u
  - Use uppercase LED in commit message
  https://lore.kernel.org/linux-leds/20191226101336.GD4033@amd/T/#u
  - Fix typo in commit message
  - Use correct return value when checking if property is present
  - Fold in
    https://lore.kernel.org/linux-leds/20191226101419.GE4033@amd/T/#t
- Add Acked-By's from Pavel Machek, thanks!

Changes from v1
- As per review comments by Dan Murphy
  https://lore.kernel.org/linux-leds/3d66b07d-b4c5-43e6-4378-d63cc84b8d43@ti.com/
  - 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

Guido Günther (9):
  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
  leds: lm3692x: Move lm3692x_init and rename to lm3692x_leds_enable
  leds: lm3692x: Split out lm3692x_leds_disable
  leds: lm3692x: Disable chip on brightness 0

 .../devicetree/bindings/leds/leds-lm3692x.txt |  11 +
 drivers/leds/leds-lm3692x.c                   | 195 +++++++++++++-----
 2 files changed, 149 insertions(+), 57 deletions(-)

-- 
2.23.0


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

* [PATCH v3 1/9] dt: bindings: lm3692x: Add ti,ovp-microvolt property
  2020-01-04 10:54 [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode Guido Günther
@ 2020-01-04 10:54 ` Guido Günther
  2020-01-04 21:11   ` Rob Herring
  2020-01-04 10:54 ` [PATCH v3 2/9] leds: lm3692x: Allow to configure over voltage protection Guido Günther
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Guido Günther @ 2020-01-04 10:54 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

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

Signed-off-by: Guido Günther <agx@sigxcpu.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 Documentation/devicetree/bindings/leds/leds-lm3692x.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
index 4c2d923f8758..9b334695c410 100644
--- a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
+++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
@@ -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 = <29000000>;
 
 	led@0 {
 		reg = <0>;
-- 
2.23.0


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

* [PATCH v3 2/9] leds: lm3692x: Allow to configure over voltage protection
  2020-01-04 10:54 [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode Guido Günther
  2020-01-04 10:54 ` [PATCH v3 1/9] dt: bindings: lm3692x: Add ti,ovp-microvolt property Guido Günther
@ 2020-01-04 10:54 ` Guido Günther
  2020-01-04 10:54 ` [PATCH v3 3/9] dt: bindings: lm3692x: Add ti,brightness-mapping-exponential property Guido Günther
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Guido Günther @ 2020-01-04 10:54 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

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

Signed-off-by: Guido Günther <agx@sigxcpu.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/leds-lm3692x.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index 8b408102e138..28973cc5a6cc 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -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");
-- 
2.23.0


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

* [PATCH v3 3/9] dt: bindings: lm3692x: Add ti,brightness-mapping-exponential property
  2020-01-04 10:54 [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode Guido Günther
  2020-01-04 10:54 ` [PATCH v3 1/9] dt: bindings: lm3692x: Add ti,ovp-microvolt property Guido Günther
  2020-01-04 10:54 ` [PATCH v3 2/9] leds: lm3692x: Allow to configure over voltage protection Guido Günther
@ 2020-01-04 10:54 ` Guido Günther
  2020-01-04 21:11   ` Rob Herring
  2020-01-04 10:54 ` [PATCH v3 4/9] leds: lm3692x: Allow to configure brigthness mode Guido Günther
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Guido Günther @ 2020-01-04 10:54 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

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

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
 Documentation/devicetree/bindings/leds/leds-lm3692x.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
index 9b334695c410..197e3fd2ae87 100644
--- a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
+++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
@@ -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 = <29000000>;
+	ti,brightness-mapping-exponential;
 
 	led@0 {
 		reg = <0>;
-- 
2.23.0


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

* [PATCH v3 4/9] leds: lm3692x: Allow to configure brigthness mode
  2020-01-04 10:54 [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode Guido Günther
                   ` (2 preceding siblings ...)
  2020-01-04 10:54 ` [PATCH v3 3/9] dt: bindings: lm3692x: Add ti,brightness-mapping-exponential property Guido Günther
@ 2020-01-04 10:54 ` Guido Günther
  2020-01-04 10:54 ` [PATCH v3 5/9] dt: bindings: lm3692x: Add led-max-microamp property Guido Günther
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Guido Günther @ 2020-01-04 10:54 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

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

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
 drivers/leds/leds-lm3692x.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index 28973cc5a6cc..ff20560a8263 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -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");
-- 
2.23.0


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

* [PATCH v3 5/9] dt: bindings: lm3692x: Add led-max-microamp property
  2020-01-04 10:54 [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode Guido Günther
                   ` (3 preceding siblings ...)
  2020-01-04 10:54 ` [PATCH v3 4/9] leds: lm3692x: Allow to configure brigthness mode Guido Günther
@ 2020-01-04 10:54 ` Guido Günther
  2020-01-04 21:11   ` Rob Herring
  2020-01-04 10:54 ` [PATCH v3 6/9] leds: lm3692x: Make sure we don't exceed the maximum led current Guido Günther
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Guido Günther @ 2020-01-04 10:54 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

This can be used to limit the current per LED strip.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 Documentation/devicetree/bindings/leds/leds-lm3692x.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
index 197e3fd2ae87..b8939fdd19d6 100644
--- a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
+++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
@@ -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>;
 	};
 }
 
-- 
2.23.0


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

* [PATCH v3 6/9] leds: lm3692x: Make sure we don't exceed the maximum led current
  2020-01-04 10:54 [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode Guido Günther
                   ` (4 preceding siblings ...)
  2020-01-04 10:54 ` [PATCH v3 5/9] dt: bindings: lm3692x: Add led-max-microamp property Guido Günther
@ 2020-01-04 10:54 ` Guido Günther
  2020-01-04 10:54 ` [PATCH v3 7/9] leds: lm3692x: Move lm3692x_init and rename to lm3692x_leds_enable Guido Günther
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Guido Günther @ 2020-01-04 10:54 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

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 formula for the lm36923 is identical according to the data sheet.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/leds-lm3692x.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index ff20560a8263..2e24316566f5 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -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;
 	}
 
+	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 = ":";
-- 
2.23.0


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

* [PATCH v3 7/9] leds: lm3692x: Move lm3692x_init and rename to lm3692x_leds_enable
  2020-01-04 10:54 [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode Guido Günther
                   ` (5 preceding siblings ...)
  2020-01-04 10:54 ` [PATCH v3 6/9] leds: lm3692x: Make sure we don't exceed the maximum led current Guido Günther
@ 2020-01-04 10:54 ` Guido Günther
  2020-01-04 10:54 ` [PATCH v3 8/9] leds: lm3692x: Split out lm3692x_leds_disable Guido Günther
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Guido Günther @ 2020-01-04 10:54 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

This moves lm3692x_init so it can be used from
lm3692x_brightness_set. Rename to lm3692_leds_enable to pair up
with lm3692x_leds_disable. No functional change.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/leds-lm3692x.c | 70 ++++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index 2e24316566f5..2b8f87b829ae 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -165,40 +165,7 @@ static int lm3692x_fault_check(struct lm3692x_led *led)
 	return read_buf;
 }
 
-static int lm3692x_brightness_set(struct led_classdev *led_cdev,
-				enum led_brightness brt_val)
-{
-	struct lm3692x_led *led =
-			container_of(led_cdev, struct lm3692x_led, led_dev);
-	int ret;
-	int led_brightness_lsb = (brt_val >> 5);
-
-	mutex_lock(&led->lock);
-
-	ret = lm3692x_fault_check(led);
-	if (ret) {
-		dev_err(&led->client->dev, "Cannot read/clear faults: %d\n",
-			ret);
-		goto out;
-	}
-
-	ret = regmap_write(led->regmap, LM3692X_BRT_MSB, brt_val);
-	if (ret) {
-		dev_err(&led->client->dev, "Cannot write MSB: %d\n", ret);
-		goto out;
-	}
-
-	ret = regmap_write(led->regmap, LM3692X_BRT_LSB, led_brightness_lsb);
-	if (ret) {
-		dev_err(&led->client->dev, "Cannot write LSB: %d\n", ret);
-		goto out;
-	}
-out:
-	mutex_unlock(&led->lock);
-	return ret;
-}
-
-static int lm3692x_init(struct lm3692x_led *led)
+static int lm3692x_leds_enable(struct lm3692x_led *led)
 {
 	int enable_state;
 	int ret, reg_ret;
@@ -321,6 +288,39 @@ static int lm3692x_init(struct lm3692x_led *led)
 	return ret;
 }
 
+static int lm3692x_brightness_set(struct led_classdev *led_cdev,
+				enum led_brightness brt_val)
+{
+	struct lm3692x_led *led =
+			container_of(led_cdev, struct lm3692x_led, led_dev);
+	int ret;
+	int led_brightness_lsb = (brt_val >> 5);
+
+	mutex_lock(&led->lock);
+
+	ret = lm3692x_fault_check(led);
+	if (ret) {
+		dev_err(&led->client->dev, "Cannot read/clear faults: %d\n",
+			ret);
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_BRT_MSB, brt_val);
+	if (ret) {
+		dev_err(&led->client->dev, "Cannot write MSB: %d\n", ret);
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_BRT_LSB, led_brightness_lsb);
+	if (ret) {
+		dev_err(&led->client->dev, "Cannot write LSB: %d\n", ret);
+		goto out;
+	}
+out:
+	mutex_unlock(&led->lock);
+	return ret;
+}
+
 static enum led_brightness lm3692x_max_brightness(struct lm3692x_led *led,
 						  u32 max_cur)
 {
@@ -462,7 +462,7 @@ static int lm3692x_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 
-	ret = lm3692x_init(led);
+	ret = lm3692x_leds_enable(led);
 	if (ret)
 		return ret;
 
-- 
2.23.0


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

* [PATCH v3 8/9] leds: lm3692x: Split out lm3692x_leds_disable
  2020-01-04 10:54 [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode Guido Günther
                   ` (6 preceding siblings ...)
  2020-01-04 10:54 ` [PATCH v3 7/9] leds: lm3692x: Move lm3692x_init and rename to lm3692x_leds_enable Guido Günther
@ 2020-01-04 10:54 ` Guido Günther
  2020-01-04 10:54 ` [PATCH v3 9/9] leds: lm3692x: Disable chip on brightness 0 Guido Günther
  2020-01-05 23:47 ` [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode Pavel Machek
  9 siblings, 0 replies; 16+ messages in thread
From: Guido Günther @ 2020-01-04 10:54 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

Move the relevant parts out of lm3692x_remove() and
call it from there. No functional change.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/leds-lm3692x.c | 42 +++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index 2b8f87b829ae..075dc4b6b3b6 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -288,6 +288,30 @@ static int lm3692x_leds_enable(struct lm3692x_led *led)
 	return ret;
 }
 
+static int lm3692x_leds_disable(struct lm3692x_led *led)
+{
+	int ret;
+
+	ret = regmap_update_bits(led->regmap, LM3692X_EN, LM3692X_DEVICE_EN, 0);
+	if (ret) {
+		dev_err(&led->client->dev, "Failed to disable regulator: %d\n",
+			ret);
+		return ret;
+	}
+
+	if (led->enable_gpio)
+		gpiod_direction_output(led->enable_gpio, 0);
+
+	if (led->regulator) {
+		ret = regulator_disable(led->regulator);
+		if (ret)
+			dev_err(&led->client->dev,
+				"Failed to disable regulator: %d\n", ret);
+	}
+
+	return ret;
+}
+
 static int lm3692x_brightness_set(struct led_classdev *led_cdev,
 				enum led_brightness brt_val)
 {
@@ -474,23 +498,9 @@ static int lm3692x_remove(struct i2c_client *client)
 	struct lm3692x_led *led = i2c_get_clientdata(client);
 	int ret;
 
-	ret = regmap_update_bits(led->regmap, LM3692X_EN, LM3692X_DEVICE_EN, 0);
-	if (ret) {
-		dev_err(&led->client->dev, "Failed to disable regulator: %d\n",
-			ret);
+	ret = lm3692x_leds_disable(led);
+	if (ret)
 		return ret;
-	}
-
-	if (led->enable_gpio)
-		gpiod_direction_output(led->enable_gpio, 0);
-
-	if (led->regulator) {
-		ret = regulator_disable(led->regulator);
-		if (ret)
-			dev_err(&led->client->dev,
-				"Failed to disable regulator: %d\n", ret);
-	}
-
 	mutex_destroy(&led->lock);
 
 	return 0;
-- 
2.23.0


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

* [PATCH v3 9/9] leds: lm3692x: Disable chip on brightness 0
  2020-01-04 10:54 [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode Guido Günther
                   ` (7 preceding siblings ...)
  2020-01-04 10:54 ` [PATCH v3 8/9] leds: lm3692x: Split out lm3692x_leds_disable Guido Günther
@ 2020-01-04 10:54 ` Guido Günther
  2020-01-05 23:47 ` [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode Pavel Machek
  9 siblings, 0 replies; 16+ messages in thread
From: Guido Günther @ 2020-01-04 10:54 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, Rob Herring,
	Mark Rutland, linux-leds, devicetree, linux-kernel

Otherwise there's a noticeable glow even with brightness 0. Also
turning off the regulator can save additional power.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
 drivers/leds/leds-lm3692x.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index 075dc4b6b3b6..933b786cfaec 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -117,6 +117,7 @@ struct lm3692x_led {
 	int model_id;
 
 	u8 boost_ctrl, brightness_ctrl;
+	bool enabled;
 };
 
 static const struct reg_default lm3692x_reg_defs[] = {
@@ -170,6 +171,9 @@ static int lm3692x_leds_enable(struct lm3692x_led *led)
 	int enable_state;
 	int ret, reg_ret;
 
+	if (led->enabled)
+		return 0;
+
 	if (led->regulator) {
 		ret = regulator_enable(led->regulator);
 		if (ret) {
@@ -271,6 +275,7 @@ static int lm3692x_leds_enable(struct lm3692x_led *led)
 	ret = regmap_update_bits(led->regmap, LM3692X_EN, LM3692X_ENABLE_MASK,
 				 enable_state | LM3692X_DEVICE_EN);
 
+	led->enabled = true;
 	return ret;
 out:
 	dev_err(&led->client->dev, "Fail writing initialization values\n");
@@ -292,6 +297,9 @@ static int lm3692x_leds_disable(struct lm3692x_led *led)
 {
 	int ret;
 
+	if (!led->enabled)
+		return 0;
+
 	ret = regmap_update_bits(led->regmap, LM3692X_EN, LM3692X_DEVICE_EN, 0);
 	if (ret) {
 		dev_err(&led->client->dev, "Failed to disable regulator: %d\n",
@@ -309,6 +317,7 @@ static int lm3692x_leds_disable(struct lm3692x_led *led)
 				"Failed to disable regulator: %d\n", ret);
 	}
 
+	led->enabled = false;
 	return ret;
 }
 
@@ -322,6 +331,13 @@ static int lm3692x_brightness_set(struct led_classdev *led_cdev,
 
 	mutex_lock(&led->lock);
 
+	if (brt_val == 0) {
+		ret = lm3692x_leds_disable(led);
+		goto out;
+	} else {
+		lm3692x_leds_enable(led);
+	}
+
 	ret = lm3692x_fault_check(led);
 	if (ret) {
 		dev_err(&led->client->dev, "Cannot read/clear faults: %d\n",
-- 
2.23.0


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

* Re: [PATCH v3 1/9] dt: bindings: lm3692x: Add ti,ovp-microvolt property
  2020-01-04 10:54 ` [PATCH v3 1/9] dt: bindings: lm3692x: Add ti,ovp-microvolt property Guido Günther
@ 2020-01-04 21:11   ` Rob Herring
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2020-01-04 21:11 UTC (permalink / raw)
  To: Guido Günther
  Cc: Jacek Anaszewski, Pavel Machek, Dan Murphy, Mark Rutland,
	linux-leds, devicetree, linux-kernel

On Sat,  4 Jan 2020 11:54:17 +0100, =?UTF-8?q?Guido=20G=C3=BCnther?= wrote:
> This allows to set the overvoltage protection to 17V, 21V, 25V or 29V.
> 
> Signed-off-by: Guido Günther <agx@sigxcpu.org>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
>  Documentation/devicetree/bindings/leds/leds-lm3692x.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 

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

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

* Re: [PATCH v3 3/9] dt: bindings: lm3692x: Add ti,brightness-mapping-exponential property
  2020-01-04 10:54 ` [PATCH v3 3/9] dt: bindings: lm3692x: Add ti,brightness-mapping-exponential property Guido Günther
@ 2020-01-04 21:11   ` Rob Herring
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2020-01-04 21:11 UTC (permalink / raw)
  To: Guido Günther
  Cc: Jacek Anaszewski, Pavel Machek, Dan Murphy, Mark Rutland,
	linux-leds, devicetree, linux-kernel

On Sat,  4 Jan 2020 11:54:19 +0100, =?UTF-8?q?Guido=20G=C3=BCnther?= wrote:
> This allows to select exponential brightness mode instead of the default
> linear mode.
> 
> Signed-off-by: Guido Günther <agx@sigxcpu.org>
> ---
>  Documentation/devicetree/bindings/leds/leds-lm3692x.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 

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

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

* Re: [PATCH v3 5/9] dt: bindings: lm3692x: Add led-max-microamp property
  2020-01-04 10:54 ` [PATCH v3 5/9] dt: bindings: lm3692x: Add led-max-microamp property Guido Günther
@ 2020-01-04 21:11   ` Rob Herring
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2020-01-04 21:11 UTC (permalink / raw)
  To: Guido Günther
  Cc: Jacek Anaszewski, Pavel Machek, Dan Murphy, Mark Rutland,
	linux-leds, devicetree, linux-kernel

On Sat,  4 Jan 2020 11:54:21 +0100, =?UTF-8?q?Guido=20G=C3=BCnther?= wrote:
> This can be used to limit the current per LED strip.
> 
> Signed-off-by: Guido Günther <agx@sigxcpu.org>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
>  Documentation/devicetree/bindings/leds/leds-lm3692x.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 

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

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

* Re: [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode
  2020-01-04 10:54 [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode Guido Günther
                   ` (8 preceding siblings ...)
  2020-01-04 10:54 ` [PATCH v3 9/9] leds: lm3692x: Disable chip on brightness 0 Guido Günther
@ 2020-01-05 23:47 ` Pavel Machek
  2020-01-06  9:44   ` Guido Günther
  9 siblings, 1 reply; 16+ messages in thread
From: Pavel Machek @ 2020-01-05 23:47 UTC (permalink / raw)
  To: Guido Günther
  Cc: Jacek Anaszewski, Dan Murphy, Rob Herring, Mark Rutland,
	linux-leds, devicetree, linux-kernel

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

Hi!

> Overvoltage protection and brightness mode are currently hardcoded
> as 29V and disabled in the driver. Make these configurable via DT.
> 
> Besides addressing review comments v3 folds in the patches to
> disable the chip and turn of the regulator on brightness 0 from
> 
>   https://lore.kernel.org/linux-leds/20191226101419.GE4033@amd/T/#t
> 
> Besides addressing review comments v2 also allows to limit the maximum led
> current.

> Patches are against next-20191220.

I applied everything but the "exponential" changes and the last
one. I'll apply the last one if I get version that applies on top of
leds tree.

Best regards,
									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] 16+ messages in thread

* Re: [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode
  2020-01-05 23:47 ` [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode Pavel Machek
@ 2020-01-06  9:44   ` Guido Günther
  2020-02-25 10:19     ` Pavel Machek
  0 siblings, 1 reply; 16+ messages in thread
From: Guido Günther @ 2020-01-06  9:44 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Jacek Anaszewski, Dan Murphy, Rob Herring, Mark Rutland,
	linux-leds, devicetree, linux-kernel

Hi,
On Mon, Jan 06, 2020 at 12:47:08AM +0100, Pavel Machek wrote:
> Hi!
> 
> > Overvoltage protection and brightness mode are currently hardcoded
> > as 29V and disabled in the driver. Make these configurable via DT.
> > 
> > Besides addressing review comments v3 folds in the patches to
> > disable the chip and turn of the regulator on brightness 0 from
> > 
> >   https://lore.kernel.org/linux-leds/20191226101419.GE4033@amd/T/#t
> > 
> > Besides addressing review comments v2 also allows to limit the maximum led
> > current.
> 
> > Patches are against next-20191220.
> 
> I applied everything but the "exponential" changes and the last
> one. I'll apply the last one if I get version that applies on top of
> leds tree.

Thanks! Can I do anything to get the exponential part in? Is it because
you want the exponential mode to move to the backlight binding?

Cheers,
 -- Guido

> 
> Best regards,
> 									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] 16+ messages in thread

* Re: [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode
  2020-01-06  9:44   ` Guido Günther
@ 2020-02-25 10:19     ` Pavel Machek
  0 siblings, 0 replies; 16+ messages in thread
From: Pavel Machek @ 2020-02-25 10:19 UTC (permalink / raw)
  To: Guido Günther
  Cc: Jacek Anaszewski, Dan Murphy, Rob Herring, Mark Rutland,
	linux-leds, devicetree, linux-kernel

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

Hi!

> > > Patches are against next-20191220.
> > 
> > I applied everything but the "exponential" changes and the last
> > one. I'll apply the last one if I get version that applies on top of
> > leds tree.
> 
> Thanks! Can I do anything to get the exponential part in? Is it because
> you want the exponential mode to move to the backlight binding?

You'd have to do some serious convincing, explaining why we absolutely
need the exponential stuff.

Most devices today use linear brightness, and userspace needs to know
the relation, especially for RGB stuff.

You can set bigger max-brightness, and then do in-driver conversion to
use full dynamic brightness range...?

Best regards,

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

end of thread, other threads:[~2020-02-25 10:19 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-04 10:54 [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode Guido Günther
2020-01-04 10:54 ` [PATCH v3 1/9] dt: bindings: lm3692x: Add ti,ovp-microvolt property Guido Günther
2020-01-04 21:11   ` Rob Herring
2020-01-04 10:54 ` [PATCH v3 2/9] leds: lm3692x: Allow to configure over voltage protection Guido Günther
2020-01-04 10:54 ` [PATCH v3 3/9] dt: bindings: lm3692x: Add ti,brightness-mapping-exponential property Guido Günther
2020-01-04 21:11   ` Rob Herring
2020-01-04 10:54 ` [PATCH v3 4/9] leds: lm3692x: Allow to configure brigthness mode Guido Günther
2020-01-04 10:54 ` [PATCH v3 5/9] dt: bindings: lm3692x: Add led-max-microamp property Guido Günther
2020-01-04 21:11   ` Rob Herring
2020-01-04 10:54 ` [PATCH v3 6/9] leds: lm3692x: Make sure we don't exceed the maximum led current Guido Günther
2020-01-04 10:54 ` [PATCH v3 7/9] leds: lm3692x: Move lm3692x_init and rename to lm3692x_leds_enable Guido Günther
2020-01-04 10:54 ` [PATCH v3 8/9] leds: lm3692x: Split out lm3692x_leds_disable Guido Günther
2020-01-04 10:54 ` [PATCH v3 9/9] leds: lm3692x: Disable chip on brightness 0 Guido Günther
2020-01-05 23:47 ` [PATCH v3 0/9] leds: lm3692x: Allow to set ovp and brigthness mode Pavel Machek
2020-01-06  9:44   ` Guido Günther
2020-02-25 10:19     ` Pavel Machek

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).