[PATCH v2 0/6] leds: lm3692x: Allow to set ovp and brigthness mode

[PATCH v2 0/6] leds: lm3692x: Allow to set ovp and brigthness mode

[Guido Günther]

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

To: Jacek Anaszewski <jacek.anaszewski@gmail.com>,Pavel Machek <pavel@ucw.cz>,Dan Murphy <dmurphy@ti.com>,Rob Herring <robh+dt@kernel.org>,Mark Rutland <mark.rutland@arm.com>,linux-leds@vger.kernel.org,devicetree@vger.kernel.org,linux-kernel@vger.kernel.org


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

-- 
2.23.0

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

[Guido Günther]

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

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
 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..95f6d8c19f20 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 = <25000000>;
 
 	led@0 {
 		reg = <0>;
-- 
2.23.0

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

[Pavel Machek]

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

On Wed 2019-12-25 12:07:14, Guido Günther 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>

I will need Rob's Ack.

> @@ -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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

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

[Guido Günther]

Hi,
On Thu, Dec 26, 2019 at 11:06:15AM +0100, Pavel Machek wrote:
> On Wed 2019-12-25 12:07:14, Guido Günther 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>

Thanks!

> I will need Rob's Ack.
> 
> > @@ -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

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

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

[Guido Günther]

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

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
 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

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

[Pavel Machek]

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

On Wed 2019-12-25 12:07:15, Guido Günther wrote:
> 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>

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

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

[Guido Günther]

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 95f6d8c19f20..73f41249ecd3 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 = <25000000>;
+	ti,brightness-mapping-exponential;
 
 	led@0 {
 		reg = <0>;
-- 
2.23.0

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

[Guido Günther]

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

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

[Guido Günther]

This can be used to limit the current per led strip

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 73f41249ecd3..e0d7b3030c01 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

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

[Pavel Machek]

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

On Wed 2019-12-25 12:07:18, Guido Günther wrote:
> This can be used to limit the current per led strip

"per LED strip."

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

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

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

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

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

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
 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..d7e5de8fe8db 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;
 	}
 
+	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

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

[Pavel Machek]

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

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

>  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);
> +

Umm. Should ret come from this fwnode_property_read_u32()?

With that fixed,

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

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

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

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

[Guido Günther]

Hi,
On Thu, Dec 26, 2019 at 11:13:36AM +0100, Pavel Machek wrote:
> 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
> > 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?
> 
> >  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);
> > +
> 
> 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

> 
> With that fixed,
> 
> Acked-by: Pavel Machek <pavel@ucw.cz>
> 
> (Feel free to wait for Rob before resending the series, and I guess
> you can merge it with the next one).
> 
> Best regards,
> 									Pavel
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html