Message ID | 1420659771-31401-3-git-send-email-jonathar@broadcom.com |
---|---|
State | Superseded |
Headers | show |
On Wed, Jan 07, 2015 at 11:42:51AM -0800, Jonathan Richardson wrote: > The pwm_enable function didn't clear the enabled bit if a call to a > clients enable function returned an error. The result was that the state > of the pwm core was wrong. Clearing the bit when enable returns an error > ensures the state is properly set. > > Tested-by: Jonathan Richardson <jonathar@broadcom.com> > Signed-off-by: Jonathan Richardson <jonathar@broadcom.com> > --- > drivers/pwm/core.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > index f28c4ce..c33e24f 100644 > --- a/drivers/pwm/core.c > +++ b/drivers/pwm/core.c > @@ -477,8 +477,14 @@ EXPORT_SYMBOL_GPL(pwm_set_polarity); > */ > int pwm_enable(struct pwm_device *pwm) > { > - if (pwm && !test_and_set_bit(PWMF_ENABLED, &pwm->flags)) > - return pwm->chip->ops->enable(pwm->chip, pwm); > + int err; > + > + if (pwm && !test_and_set_bit(PWMF_ENABLED, &pwm->flags)) { > + err = pwm->chip->ops->enable(pwm->chip, pwm); > + if (err) > + clear_bit(PWMF_ENABLED, &pwm->flags); > + return err; > + } > > return pwm ? 0 : -EINVAL; Seems fine in principle, but somewhat messy. Can we do the following: int pwm_enable(struct pwm_device *pwm) { int err; if (!pwm) return -EINVAL; if (!test_and_set_bit(PWMF_ENABLED, &pwm->flags)) { err = pwm->chip->ops->enable(pwm->chip, pwm); if (err) { clear_bit(PWMF_ENABLED, &pwm->flags); return err; } } return 0; } Otherwise: Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> Thanks.
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index f28c4ce..c33e24f 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -477,8 +477,14 @@ EXPORT_SYMBOL_GPL(pwm_set_polarity); */ int pwm_enable(struct pwm_device *pwm) { - if (pwm && !test_and_set_bit(PWMF_ENABLED, &pwm->flags)) - return pwm->chip->ops->enable(pwm->chip, pwm); + int err; + + if (pwm && !test_and_set_bit(PWMF_ENABLED, &pwm->flags)) { + err = pwm->chip->ops->enable(pwm->chip, pwm); + if (err) + clear_bit(PWMF_ENABLED, &pwm->flags); + return err; + } return pwm ? 0 : -EINVAL; }