Hi Shreshtha, > -----Original Message----- > From: Shreshtha Kumar SAHU [mailto:shreshthakumar.sahu@xxxxxxxxxxxxxx] > Sent: Thursday, December 20, 2012 2:28 PM > To: Axel Lin > Cc: Kim, Milo; Richard Purdie; linux-leds@xxxxxxxxxxxxxxx; Bryan Wu > Subject: RE: [PATCH] leds: lm3530: Ensure drvdata->enable has correct > status if regulator_disable fails > > 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: > If private data, 'enable' is used for the status of 'vin', I would suggest using regulator_is_enabled() rather than private data. Best Regards, Milo -- 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