Messages in this thread | | | From | <> | Subject | Re: [PATCH] pwm: atmel: add missing clk_disable_unprepare() | Date | Fri, 25 Aug 2023 03:04:07 +0000 |
| |
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);
| |