Agree!
So wouldn't this be enough instead of moving regulator_enable/disable to a separate helper function
--- a/drivers/leds/leds-lm3530.c
+++ b/drivers/leds/leds-lm3530.c
@@ -295,13 +295,15 @@ static void lm3530_brightness_set(struct led_classdev *led_cdev,
drvdata->brightness = brt_val;
if (brt_val == 0) {
+ drvdata->enable = false;
err = regulator_disable(drvdata->regulator);
- if (err)
+ if (err) {
dev_err(&drvdata->client->dev,
"Disable regulator failed\n");
+ drvdata->enable = true;
+ }
- drvdata->enable = false;
}
break;
case LM3530_BL_MODE_ALS:
BR
Shreshtha
From: Axel Lin [mailto:axel.lin@xxxxxxxxxx]
Sent: Thursday, December 20, 2012 10:34 AM
To: Shreshtha Kumar SAHU
Cc: Milo(Woogyom) Kim; Richard Purdie; linux-leds@xxxxxxxxxxxxxxx; Bryan Wu
Subject: Re: [PATCH] leds: lm3530: Ensure drvdata->enable has correct status if regulator_disable fails
2012/12/20 Shreshtha Kumar SAHU <shreshthakumar.sahu@xxxxxxxxxxxxxx>
Hi Axel,
In which case "drvdata->enable" is getting incorrect status, and how adding these helper functions correcting it.
In lm3530_brightness_set(), in case LM3530_BL_MODE_MANUAL:
if (brt_val == 0) {
err = regulator_disable(drvdata->regulator);
if (err)
dev_err(&drvdata->client->dev,
"Disable regulator failed\n");
drvdata->enable = false;
}
It shows dev_err if regulator_disable fails, but drvdata->enable becomes is still set to false.
Regards,
Axel
--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
