| Message ID | 20230822070441.22170-1-Hari.PrasathGE@microchip.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pwm: atmel: add missing clk_disable_unprepare() | expand |
On 8/22/23 10:04, Hari Prasath Gujulan Elango wrote: > Fix the below smatch warning: > > drivers/pwm/pwm-atmel-hlcdc.c:167 atmel_hlcdc_pwm_apply() warn: 'new_clk' from clk_prepare_enable() not released on lines: 112,137,142,149. > Can you add a fixes tag? > Signed-off-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com> > --- > drivers/pwm/pwm-atmel-hlcdc.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c > index 96a709a9d49a..ce46f6c74a14 100644 > --- a/drivers/pwm/pwm-atmel-hlcdc.c > +++ b/drivers/pwm/pwm-atmel-hlcdc.c > @@ -108,8 +108,10 @@ static int atmel_hlcdc_pwm_apply(struct pwm_chip *c, struct pwm_device *pwm, > ATMEL_HLCDC_CFG(0), > ATMEL_HLCDC_CLKPWMSEL, > gencfg); > - if (ret) > + if (ret) { > + clk_disable_unprepare(new_clk); > return ret; > + } > } > > do_div(pwmcval, state->period); > @@ -133,20 +135,27 @@ static int atmel_hlcdc_pwm_apply(struct pwm_chip *c, struct pwm_device *pwm, > ATMEL_HLCDC_PWMPS_MASK | > ATMEL_HLCDC_PWMPOL, > pwmcfg); > - if (ret) > + if (ret) { > + clk_disable_unprepare(new_clk); > return ret; > + } > > ret = regmap_write(hlcdc->regmap, ATMEL_HLCDC_EN, > ATMEL_HLCDC_PWM); > - if (ret) > + if (ret) { > + clk_disable_unprepare(new_clk); > return ret; > + } > > ret = regmap_read_poll_timeout(hlcdc->regmap, ATMEL_HLCDC_SR, > status, > status & ATMEL_HLCDC_PWM, > 10, 0); > - if (ret) > + if (ret) { > + clk_disable_unprepare(new_clk); Can you keep a single failure point for all these? Also, you have to set chip->cur_clk = NULL otherwise next time your apply will get executed the new_clk will not be enabled. Thank you, Claudiu Beznea > return ret; > + } > + > } else { > ret = regmap_write(hlcdc->regmap, ATMEL_HLCDC_DIS, > ATMEL_HLCDC_PWM);
Hello Claudiu, On 23/08/23 10:29 am, claudiu beznea wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 8/22/23 10:04, Hari Prasath Gujulan Elango wrote: >> Fix the below smatch warning: >> >> drivers/pwm/pwm-atmel-hlcdc.c:167 atmel_hlcdc_pwm_apply() warn: 'new_clk' from clk_prepare_enable() not released on lines: 112,137,142,149. >> > > Can you add a fixes tag? > yes I will add it. >> Signed-off-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com> >> --- >> drivers/pwm/pwm-atmel-hlcdc.c | 17 +++++++++++++---- >> 1 file changed, 13 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c >> index 96a709a9d49a..ce46f6c74a14 100644 >> --- a/drivers/pwm/pwm-atmel-hlcdc.c >> +++ b/drivers/pwm/pwm-atmel-hlcdc.c >> @@ -108,8 +108,10 @@ static int atmel_hlcdc_pwm_apply(struct pwm_chip *c, struct pwm_device *pwm, >> ATMEL_HLCDC_CFG(0), >> ATMEL_HLCDC_CLKPWMSEL, >> gencfg); >> - if (ret) >> + if (ret) { >> + clk_disable_unprepare(new_clk); >> return ret; >> + } >> } >> >> do_div(pwmcval, state->period); >> @@ -133,20 +135,27 @@ static int atmel_hlcdc_pwm_apply(struct pwm_chip *c, struct pwm_device *pwm, >> ATMEL_HLCDC_PWMPS_MASK | >> ATMEL_HLCDC_PWMPOL, >> pwmcfg); >> - if (ret) >> + if (ret) { >> + clk_disable_unprepare(new_clk); >> return ret; >> + } >> >> ret = regmap_write(hlcdc->regmap, ATMEL_HLCDC_EN, >> ATMEL_HLCDC_PWM); >> - if (ret) >> + if (ret) { >> + clk_disable_unprepare(new_clk); >> return ret; >> + } >> >> ret = regmap_read_poll_timeout(hlcdc->regmap, ATMEL_HLCDC_SR, >> status, >> status & ATMEL_HLCDC_PWM, >> 10, 0); >> - if (ret) >> + if (ret) { >> + clk_disable_unprepare(new_clk); > > Can you keep a single failure point for all these? > > Also, you have to set chip->cur_clk = NULL otherwise next time your apply > will get executed the new_clk will not be enabled. > I see that new_clk is assigned to cur_clk in the if (state->enabled) block and clk_disable_unprepare() is invoked only in the else block for cur_clk and its made NULL. I will cleanup all of this at a single point and resend v2. Thanks, Hari > Thank you, > Claudiu Beznea > >> return ret; >> + } >> + >> } else { >> ret = regmap_write(hlcdc->regmap, ATMEL_HLCDC_DIS, >> ATMEL_HLCDC_PWM);
diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c index 96a709a9d49a..ce46f6c74a14 100644 --- a/drivers/pwm/pwm-atmel-hlcdc.c +++ b/drivers/pwm/pwm-atmel-hlcdc.c @@ -108,8 +108,10 @@ static int atmel_hlcdc_pwm_apply(struct pwm_chip *c, struct pwm_device *pwm, ATMEL_HLCDC_CFG(0), ATMEL_HLCDC_CLKPWMSEL, gencfg); - if (ret) + if (ret) { + clk_disable_unprepare(new_clk); return ret; + } } do_div(pwmcval, state->period); @@ -133,20 +135,27 @@ static int atmel_hlcdc_pwm_apply(struct pwm_chip *c, struct pwm_device *pwm, ATMEL_HLCDC_PWMPS_MASK | ATMEL_HLCDC_PWMPOL, pwmcfg); - if (ret) + if (ret) { + clk_disable_unprepare(new_clk); return ret; + } ret = regmap_write(hlcdc->regmap, ATMEL_HLCDC_EN, ATMEL_HLCDC_PWM); - if (ret) + if (ret) { + clk_disable_unprepare(new_clk); return ret; + } ret = regmap_read_poll_timeout(hlcdc->regmap, ATMEL_HLCDC_SR, status, status & ATMEL_HLCDC_PWM, 10, 0); - if (ret) + if (ret) { + clk_disable_unprepare(new_clk); return ret; + } + } else { ret = regmap_write(hlcdc->regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_PWM);
Fix the below smatch warning: drivers/pwm/pwm-atmel-hlcdc.c:167 atmel_hlcdc_pwm_apply() warn: 'new_clk' from clk_prepare_enable() not released on lines: 112,137,142,149. Signed-off-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com> --- drivers/pwm/pwm-atmel-hlcdc.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)