* [PATCH v3 1/5] leds: lm3692x: Print error value on dev_err
2019-09-21 21:12 [PATCH v3 0/5] leds: lm3692x: Probing and flag fixes Guido Günther
@ 2019-09-21 21:12 ` Guido Günther
2019-09-21 21:12 ` [PATCH v3 2/5] leds: lm3692x: Don't overwrite return value in error path Guido Günther
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Guido Günther @ 2019-09-21 21:12 UTC (permalink / raw)
To: Jacek Anaszewski, Pavel Machek, Dan Murphy, linux-leds, linux-kernel
This gives a way better idea what is going on.
Signed-off-by: Guido Günther <agx@sigxcpu.org>
Reviewed-by: Dan Murphy <dmurphy@ti.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
drivers/leds/leds-lm3692x.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index 3d381f2f73d0..487228c2bed2 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -174,19 +174,20 @@ static int lm3692x_brightness_set(struct led_classdev *led_cdev,
ret = lm3692x_fault_check(led);
if (ret) {
- dev_err(&led->client->dev, "Cannot read/clear faults\n");
+ 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\n");
+ 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\n");
+ dev_err(&led->client->dev, "Cannot write LSB: %d\n", ret);
goto out;
}
out:
@@ -203,7 +204,7 @@ static int lm3692x_init(struct lm3692x_led *led)
ret = regulator_enable(led->regulator);
if (ret) {
dev_err(&led->client->dev,
- "Failed to enable regulator\n");
+ "Failed to enable regulator: %d\n", ret);
return ret;
}
}
@@ -213,7 +214,8 @@ static int lm3692x_init(struct lm3692x_led *led)
ret = lm3692x_fault_check(led);
if (ret) {
- dev_err(&led->client->dev, "Cannot read/clear faults\n");
+ dev_err(&led->client->dev, "Cannot read/clear faults: %d\n",
+ ret);
goto out;
}
@@ -409,7 +411,8 @@ static int lm3692x_remove(struct i2c_client *client)
ret = regmap_update_bits(led->regmap, LM3692X_EN, LM3692X_DEVICE_EN, 0);
if (ret) {
- dev_err(&led->client->dev, "Failed to disable regulator\n");
+ dev_err(&led->client->dev, "Failed to disable regulator: %d\n",
+ ret);
return ret;
}
@@ -420,7 +423,7 @@ static int lm3692x_remove(struct i2c_client *client)
ret = regulator_disable(led->regulator);
if (ret)
dev_err(&led->client->dev,
- "Failed to disable regulator\n");
+ "Failed to disable regulator: %d\n", ret);
}
mutex_destroy(&led->lock);
--
2.23.0.rc1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 2/5] leds: lm3692x: Don't overwrite return value in error path
2019-09-21 21:12 [PATCH v3 0/5] leds: lm3692x: Probing and flag fixes Guido Günther
2019-09-21 21:12 ` [PATCH v3 1/5] leds: lm3692x: Print error value on dev_err Guido Günther
@ 2019-09-21 21:12 ` Guido Günther
2019-09-23 14:16 ` Dan Murphy
2019-09-21 21:12 ` [PATCH v3 3/5] leds: lm3692x: Handle failure to probe the regulator Guido Günther
` (3 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Guido Günther @ 2019-09-21 21:12 UTC (permalink / raw)
To: Jacek Anaszewski, Pavel Machek, Dan Murphy, linux-leds, linux-kernel
The driver currently reports successful initialization on every failure
as long as it's able to power off the regulator. Don't check the return
value of regulator_disable to avoid that.
Signed-off-by: Guido Günther <agx@sigxcpu.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
drivers/leds/leds-lm3692x.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index 487228c2bed2..ad76e34455ff 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -198,7 +198,7 @@ static int lm3692x_brightness_set(struct led_classdev *led_cdev,
static int lm3692x_init(struct lm3692x_led *led)
{
int enable_state;
- int ret;
+ int ret, reg_ret;
if (led->regulator) {
ret = regulator_enable(led->regulator);
@@ -313,14 +313,15 @@ static int lm3692x_init(struct lm3692x_led *led)
gpiod_direction_output(led->enable_gpio, 0);
if (led->regulator) {
- ret = regulator_disable(led->regulator);
- if (ret)
+ reg_ret = regulator_disable(led->regulator);
+ if (reg_ret)
dev_err(&led->client->dev,
- "Failed to disable regulator\n");
+ "Failed to disable regulator: %d\n", reg_ret);
}
return ret;
}
+
static int lm3692x_probe_dt(struct lm3692x_led *led)
{
struct fwnode_handle *child = NULL;
--
2.23.0.rc1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 2/5] leds: lm3692x: Don't overwrite return value in error path
2019-09-21 21:12 ` [PATCH v3 2/5] leds: lm3692x: Don't overwrite return value in error path Guido Günther
@ 2019-09-23 14:16 ` Dan Murphy
0 siblings, 0 replies; 9+ messages in thread
From: Dan Murphy @ 2019-09-23 14:16 UTC (permalink / raw)
To: Guido Günther, Jacek Anaszewski, Pavel Machek, linux-leds,
linux-kernel
Guido
Thanks for the update
Reviewed-by: Dan Murphy <dmurphy@ti.com>
On 9/21/19 4:12 PM, Guido Günther wrote:
> The driver currently reports successful initialization on every failure
> as long as it's able to power off the regulator. Don't check the return
> value of regulator_disable to avoid that.
>
> Signed-off-by: Guido Günther <agx@sigxcpu.org>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
> drivers/leds/leds-lm3692x.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
> index 487228c2bed2..ad76e34455ff 100644
> --- a/drivers/leds/leds-lm3692x.c
> +++ b/drivers/leds/leds-lm3692x.c
> @@ -198,7 +198,7 @@ static int lm3692x_brightness_set(struct led_classdev *led_cdev,
> static int lm3692x_init(struct lm3692x_led *led)
> {
> int enable_state;
> - int ret;
> + int ret, reg_ret;
>
> if (led->regulator) {
> ret = regulator_enable(led->regulator);
> @@ -313,14 +313,15 @@ static int lm3692x_init(struct lm3692x_led *led)
> gpiod_direction_output(led->enable_gpio, 0);
>
> if (led->regulator) {
> - ret = regulator_disable(led->regulator);
> - if (ret)
> + reg_ret = regulator_disable(led->regulator);
> + if (reg_ret)
> dev_err(&led->client->dev,
> - "Failed to disable regulator\n");
> + "Failed to disable regulator: %d\n", reg_ret);
> }
>
> return ret;
> }
> +
> static int lm3692x_probe_dt(struct lm3692x_led *led)
> {
> struct fwnode_handle *child = NULL;
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 3/5] leds: lm3692x: Handle failure to probe the regulator
2019-09-21 21:12 [PATCH v3 0/5] leds: lm3692x: Probing and flag fixes Guido Günther
2019-09-21 21:12 ` [PATCH v3 1/5] leds: lm3692x: Print error value on dev_err Guido Günther
2019-09-21 21:12 ` [PATCH v3 2/5] leds: lm3692x: Don't overwrite return value in error path Guido Günther
@ 2019-09-21 21:12 ` Guido Günther
2019-09-23 14:17 ` Dan Murphy
2019-09-21 21:12 ` [PATCH v3 4/5] leds: lm3692x: Use flags from LM3692X_BOOST_CTRL Guido Günther
` (2 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Guido Günther @ 2019-09-21 21:12 UTC (permalink / raw)
To: Jacek Anaszewski, Pavel Machek, Dan Murphy, linux-leds, linux-kernel
Instead use devm_regulator_get_optional since the regulator
is optional and check for errors.
Signed-off-by: Guido Günther <agx@sigxcpu.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
drivers/leds/leds-lm3692x.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index ad76e34455ff..54e9bd2d288b 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -337,9 +337,18 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
return ret;
}
- led->regulator = devm_regulator_get(&led->client->dev, "vled");
- if (IS_ERR(led->regulator))
+ led->regulator = devm_regulator_get_optional(&led->client->dev, "vled");
+ if (IS_ERR(led->regulator)) {
+ ret = PTR_ERR(led->regulator);
+ if (ret != -ENODEV) {
+ if (ret != -EPROBE_DEFER)
+ dev_err(&led->client->dev,
+ "Failed to get vled regulator: %d\n",
+ ret);
+ return ret;
+ }
led->regulator = NULL;
+ }
child = device_get_next_child_node(&led->client->dev, child);
if (!child) {
--
2.23.0.rc1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 3/5] leds: lm3692x: Handle failure to probe the regulator
2019-09-21 21:12 ` [PATCH v3 3/5] leds: lm3692x: Handle failure to probe the regulator Guido Günther
@ 2019-09-23 14:17 ` Dan Murphy
0 siblings, 0 replies; 9+ messages in thread
From: Dan Murphy @ 2019-09-23 14:17 UTC (permalink / raw)
To: Guido Günther, Jacek Anaszewski, Pavel Machek, linux-leds,
linux-kernel
Guido
Thanks for the update
Reviewed-by: Dan Murphy <dmurphy@ti.com>
On 9/21/19 4:12 PM, Guido Günther wrote:
> Instead use devm_regulator_get_optional since the regulator
> is optional and check for errors.
>
> Signed-off-by: Guido Günther <agx@sigxcpu.org>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
> drivers/leds/leds-lm3692x.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
> index ad76e34455ff..54e9bd2d288b 100644
> --- a/drivers/leds/leds-lm3692x.c
> +++ b/drivers/leds/leds-lm3692x.c
> @@ -337,9 +337,18 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
> return ret;
> }
>
> - led->regulator = devm_regulator_get(&led->client->dev, "vled");
> - if (IS_ERR(led->regulator))
> + led->regulator = devm_regulator_get_optional(&led->client->dev, "vled");
> + if (IS_ERR(led->regulator)) {
> + ret = PTR_ERR(led->regulator);
> + if (ret != -ENODEV) {
> + if (ret != -EPROBE_DEFER)
> + dev_err(&led->client->dev,
> + "Failed to get vled regulator: %d\n",
> + ret);
> + return ret;
> + }
> led->regulator = NULL;
> + }
>
> child = device_get_next_child_node(&led->client->dev, child);
> if (!child) {
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 4/5] leds: lm3692x: Use flags from LM3692X_BOOST_CTRL
2019-09-21 21:12 [PATCH v3 0/5] leds: lm3692x: Probing and flag fixes Guido Günther
` (2 preceding siblings ...)
2019-09-21 21:12 ` [PATCH v3 3/5] leds: lm3692x: Handle failure to probe the regulator Guido Günther
@ 2019-09-21 21:12 ` Guido Günther
2019-09-21 21:12 ` [PATCH v3 5/5] leds: lm3692x: Use flags from LM3692X_BRT_CTRL Guido Günther
2019-09-24 18:20 ` [PATCH v3 0/5] leds: lm3692x: Probing and flag fixes Jacek Anaszewski
5 siblings, 0 replies; 9+ messages in thread
From: Guido Günther @ 2019-09-21 21:12 UTC (permalink / raw)
To: Jacek Anaszewski, Pavel Machek, Dan Murphy, linux-leds, linux-kernel
The current setup of LM3692X_BOOST_CTRL uses flags from LM3692X_BRT_CTRL.
Use flags from LM3692X_BOOST_CTRL but leave the resulting register value
unchanged.
Signed-off-by: Guido Günther <agx@sigxcpu.org>
Reviewed-by: Dan Murphy <dmurphy@ti.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
drivers/leds/leds-lm3692x.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index 54e9bd2d288b..a57b1571e359 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -250,9 +250,9 @@ static int lm3692x_init(struct lm3692x_led *led)
goto out;
ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL,
- LM3692X_BRHT_MODE_RAMP_MULTI |
- LM3692X_BL_ADJ_POL |
- LM3692X_RAMP_RATE_250us);
+ LM3692X_BOOST_SW_1MHZ |
+ LM3692X_BOOST_SW_NO_SHIFT |
+ LM3692X_OCP_PROT_1_5A);
if (ret)
goto out;
--
2.23.0.rc1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 5/5] leds: lm3692x: Use flags from LM3692X_BRT_CTRL
2019-09-21 21:12 [PATCH v3 0/5] leds: lm3692x: Probing and flag fixes Guido Günther
` (3 preceding siblings ...)
2019-09-21 21:12 ` [PATCH v3 4/5] leds: lm3692x: Use flags from LM3692X_BOOST_CTRL Guido Günther
@ 2019-09-21 21:12 ` Guido Günther
2019-09-24 18:20 ` [PATCH v3 0/5] leds: lm3692x: Probing and flag fixes Jacek Anaszewski
5 siblings, 0 replies; 9+ messages in thread
From: Guido Günther @ 2019-09-21 21:12 UTC (permalink / raw)
To: Jacek Anaszewski, Pavel Machek, Dan Murphy, linux-leds, linux-kernel
Use LM3692X_RAMP_EN instead of LM3692X_PWM_HYSTER_4LSB
since the later is a flag for the PWM register. The
actual register value remains unchanged.
Signed-off-by: Guido Günther <agx@sigxcpu.org>
Reviewed-by: Dan Murphy <dmurphy@ti.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
drivers/leds/leds-lm3692x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index a57b1571e359..8b408102e138 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -269,7 +269,7 @@ static int lm3692x_init(struct lm3692x_led *led)
goto out;
ret = regmap_write(led->regmap, LM3692X_BRT_CTRL,
- LM3692X_BL_ADJ_POL | LM3692X_PWM_HYSTER_4LSB);
+ LM3692X_BL_ADJ_POL | LM3692X_RAMP_EN);
if (ret)
goto out;
--
2.23.0.rc1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 0/5] leds: lm3692x: Probing and flag fixes
2019-09-21 21:12 [PATCH v3 0/5] leds: lm3692x: Probing and flag fixes Guido Günther
` (4 preceding siblings ...)
2019-09-21 21:12 ` [PATCH v3 5/5] leds: lm3692x: Use flags from LM3692X_BRT_CTRL Guido Günther
@ 2019-09-24 18:20 ` Jacek Anaszewski
5 siblings, 0 replies; 9+ messages in thread
From: Jacek Anaszewski @ 2019-09-24 18:20 UTC (permalink / raw)
To: Guido Günther, Pavel Machek, Dan Murphy, linux-leds, linux-kernel
Hi Guido,
Thank you for the update.
On 9/21/19 11:12 PM, Guido Günther wrote:
> The driver currently returns success on init although probing fails and
> register setup uses flag values from other registers which is confusing when
> reading the driver. This series cleans this up.
>
> Changes from v2:
> - Add Acked-by from Pavel Machek, thanks!
> https://lore.kernel.org/linux-leds/20190920114743.GA21835@amd/
> - As per review comment from Dan Murphy
> https://lore.kernel.org/linux-leds/2bde2870-08a3-38b9-9cd7-fee0e2107743@ti.com/
> - rename return value from ret2 to reg_ret
> - print error code
>
> Changes from v1:
> - Add reviewed by's from Dan Murphy, thanks!
> https://lore.kernel.org/linux-leds/cover.1568772964.git.agx@sigxcpu.org/T/#mc183f3f65931371fa9f9ca2e0e83e0b85010f24b
> https://lore.kernel.org/linux-leds/cover.1568772964.git.agx@sigxcpu.org/T/#mb845fac36327a5d5dd03fe7e988eef0eb5626f82
> https://lore.kernel.org/linux-leds/cover.1568772964.git.agx@sigxcpu.org/T/#m995bce73dda3e3bd4f0c2e8f98cbd04a39c13832
> - As per review comment from Dan Murphy
> - Don't drop error message when disabling the regulator fails
> https://lore.kernel.org/linux-leds/cover.1568772964.git.agx@sigxcpu.org/T/#m2ab6dc33b7277b71a197c3747847f1c4d9d9c1d8
> - Handle -ENODEV (when the regulator is not set)
> https://lore.kernel.org/linux-leds/cover.1568772964.git.agx@sigxcpu.org/T/#mf6212c29bbfa37b43200ea2c9744074de4f068ee
> - Add Acked-by from Pavel Machek, thanks!
> https://lore.kernel.org/linux-leds/20190919095415.GA29939@amd/
>
> Guido Günther (5):
> leds: lm3692x: Print error value on dev_err
> leds: lm3692x: Don't overwrite return value in error path
> leds: lm3692x: Handle failure to probe the regulator
> leds: lm3692x: Use flags from LM3692X_BOOST_CTRL
> leds: lm3692x: Use flags from LM3692X_BRT_CTRL
>
> drivers/leds/leds-lm3692x.c | 47 +++++++++++++++++++++++--------------
> 1 file changed, 30 insertions(+), 17 deletions(-)
>
Patch set applied to the for-5.5 branch of linux-leds.git.
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 9+ messages in thread