Linux-LEDs Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/3] leds: lm3692x: Disable chip on brightness 0
@ 2019-12-25 11:16 Guido Günther
  2019-12-25 11:16 ` [PATCH 1/3] leds: lm3692x: Move lm3692x_init and rename to lm3692x_leds_enable Guido Günther
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Guido Günther @ 2019-12-25 11:16 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, linux-leds, linux-kernel

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

This is on top of the "leds: lm3692x: Allow to set ovp and brigthness mode".
Without that applied there's a minor merge conflict since this introduces a new
variable in struct lm3692x_led as well. I did not want to lump it in there as
well since it's not related to any DT properties at all.

Guido Günther (3):
  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

 drivers/leds/leds-lm3692x.c | 128 ++++++++++++++++++++++--------------
 1 file changed, 77 insertions(+), 51 deletions(-)

-- 
2.23.0


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

* [PATCH 1/3] leds: lm3692x: Move lm3692x_init and rename to lm3692x_leds_enable
  2019-12-25 11:16 [PATCH 0/3] leds: lm3692x: Disable chip on brightness 0 Guido Günther
@ 2019-12-25 11:16 ` Guido Günther
  2019-12-26 10:15   ` Pavel Machek
  2019-12-25 11:16 ` [PATCH 2/3] leds: lm3692x: Split out lm3692x_leds_disable Guido Günther
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Guido Günther @ 2019-12-25 11:16 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, linux-leds, 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>
---
 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 d7e5de8fe8db..7bf97ee6aa92 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	[flat|nested] 9+ messages in thread

* [PATCH 2/3] leds: lm3692x: Split out lm3692x_leds_disable
  2019-12-25 11:16 [PATCH 0/3] leds: lm3692x: Disable chip on brightness 0 Guido Günther
  2019-12-25 11:16 ` [PATCH 1/3] leds: lm3692x: Move lm3692x_init and rename to lm3692x_leds_enable Guido Günther
@ 2019-12-25 11:16 ` Guido Günther
  2019-12-26 10:16   ` Pavel Machek
  2019-12-25 11:16 ` [PATCH 3/3] leds: lm3692x: Disable chip on brightness 0 Guido Günther
  2019-12-26 10:14 ` [PATCH 0/3] " Pavel Machek
  3 siblings, 1 reply; 9+ messages in thread
From: Guido Günther @ 2019-12-25 11:16 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, linux-leds, 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>
---
 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 7bf97ee6aa92..d1bd9ae4e7ab 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	[flat|nested] 9+ messages in thread

* [PATCH 3/3] leds: lm3692x: Disable chip on brightness 0
  2019-12-25 11:16 [PATCH 0/3] leds: lm3692x: Disable chip on brightness 0 Guido Günther
  2019-12-25 11:16 ` [PATCH 1/3] leds: lm3692x: Move lm3692x_init and rename to lm3692x_leds_enable Guido Günther
  2019-12-25 11:16 ` [PATCH 2/3] leds: lm3692x: Split out lm3692x_leds_disable Guido Günther
@ 2019-12-25 11:16 ` Guido Günther
  2019-12-26 10:29   ` Pavel Machek
  2019-12-26 10:14 ` [PATCH 0/3] " Pavel Machek
  3 siblings, 1 reply; 9+ messages in thread
From: Guido Günther @ 2019-12-25 11:16 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, linux-leds, 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 d1bd9ae4e7ab..183614744b92 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	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] leds: lm3692x: Disable chip on brightness 0
  2019-12-25 11:16 [PATCH 0/3] leds: lm3692x: Disable chip on brightness 0 Guido Günther
                   ` (2 preceding siblings ...)
  2019-12-25 11:16 ` [PATCH 3/3] leds: lm3692x: Disable chip on brightness 0 Guido Günther
@ 2019-12-26 10:14 ` " Pavel Machek
  3 siblings, 0 replies; 9+ messages in thread
From: Pavel Machek @ 2019-12-26 10:14 UTC (permalink / raw)
  To: Guido Günther; +Cc: Jacek Anaszewski, Dan Murphy, linux-leds, linux-kernel

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

On Wed 2019-12-25 12:16:36, Guido Günther wrote:
> Otherwise there's a hardly noticeable glow even with brightness 0. Also turning
> off the regulator can save additional power.

I guess it is of too low intensity to be used, for example in
completely dark room?

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

* Re: [PATCH 1/3] leds: lm3692x: Move lm3692x_init and rename to lm3692x_leds_enable
  2019-12-25 11:16 ` [PATCH 1/3] leds: lm3692x: Move lm3692x_init and rename to lm3692x_leds_enable Guido Günther
@ 2019-12-26 10:15   ` Pavel Machek
  0 siblings, 0 replies; 9+ messages in thread
From: Pavel Machek @ 2019-12-26 10:15 UTC (permalink / raw)
  To: Guido Günther; +Cc: Jacek Anaszewski, Dan Murphy, linux-leds, linux-kernel

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

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

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

* Re: [PATCH 2/3] leds: lm3692x: Split out lm3692x_leds_disable
  2019-12-25 11:16 ` [PATCH 2/3] leds: lm3692x: Split out lm3692x_leds_disable Guido Günther
@ 2019-12-26 10:16   ` Pavel Machek
  0 siblings, 0 replies; 9+ messages in thread
From: Pavel Machek @ 2019-12-26 10:16 UTC (permalink / raw)
  To: Guido Günther; +Cc: Jacek Anaszewski, Dan Murphy, linux-leds, linux-kernel

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

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

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

* Re: [PATCH 3/3] leds: lm3692x: Disable chip on brightness 0
  2019-12-25 11:16 ` [PATCH 3/3] leds: lm3692x: Disable chip on brightness 0 Guido Günther
@ 2019-12-26 10:29   ` Pavel Machek
  2019-12-27 12:56     ` Guido Günther
  0 siblings, 1 reply; 9+ messages in thread
From: Pavel Machek @ 2019-12-26 10:29 UTC (permalink / raw)
  To: Guido Günther; +Cc: Jacek Anaszewski, Dan Murphy, linux-leds, linux-kernel

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

On Wed 2019-12-25 12:16:39, Guido Günther wrote:
> Otherwise there's a noticeable glow even with brightness 0. Also
> turning off the regulator can save additional power.

Ok, this will make set brightness slower, but I guess it makes sense.

Can you try to toggling brightness quickly from userspace, maybe even
from two threads, to ensure nothing really goes wrong there?

Hmm. And if this is independend of the other series, tell me and I can
apply v2.

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

* Re: [PATCH 3/3] leds: lm3692x: Disable chip on brightness 0
  2019-12-26 10:29   ` Pavel Machek
@ 2019-12-27 12:56     ` Guido Günther
  0 siblings, 0 replies; 9+ messages in thread
From: Guido Günther @ 2019-12-27 12:56 UTC (permalink / raw)
  To: Pavel Machek; +Cc: Jacek Anaszewski, Dan Murphy, linux-leds, linux-kernel

Hi Pavel,
On Thu, Dec 26, 2019 at 11:29:56AM +0100, Pavel Machek wrote:
> On Wed 2019-12-25 12:16:39, Guido Günther wrote:
> > Otherwise there's a noticeable glow even with brightness 0. Also
> > turning off the regulator can save additional power.
> 
> Ok, this will make set brightness slower, but I guess it makes sense.
> 
> Can you try to toggling brightness quickly from userspace, maybe even
> from two threads, to ensure nothing really goes wrong there?

I ran brightness setting in a tight loop from 0 to max from two threads
100 times without issues.

> 
> Hmm. And if this is independend of the other series, tell me and I can
> apply v2.

There's minor merge conflict so to make sure this does not get dependent
on the order i've folded this to the end of the

    "leds: lm3692x: Allow to set ovp and brigthness mode"

series with your Acked-by's folded in. I'll wait a couple of days and
then send out a v3.

Cheers and thanks for having a look,
 -- 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] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-25 11:16 [PATCH 0/3] leds: lm3692x: Disable chip on brightness 0 Guido Günther
2019-12-25 11:16 ` [PATCH 1/3] leds: lm3692x: Move lm3692x_init and rename to lm3692x_leds_enable Guido Günther
2019-12-26 10:15   ` Pavel Machek
2019-12-25 11:16 ` [PATCH 2/3] leds: lm3692x: Split out lm3692x_leds_disable Guido Günther
2019-12-26 10:16   ` Pavel Machek
2019-12-25 11:16 ` [PATCH 3/3] leds: lm3692x: Disable chip on brightness 0 Guido Günther
2019-12-26 10:29   ` Pavel Machek
2019-12-27 12:56     ` Guido Günther
2019-12-26 10:14 ` [PATCH 0/3] " Pavel Machek

Linux-LEDs Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-leds/0 linux-leds/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-leds linux-leds/ https://lore.kernel.org/linux-leds \
		linux-leds@vger.kernel.org
	public-inbox-index linux-leds

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-leds


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git