linux-kernel - Re: [PATCH] pwm: atmel: add missing clk_disable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5663e3e6-de16-e9ba-0745-b9a40676b109@tuxon.dev>
Date:   Wed, 23 Aug 2023 07:59:08 +0300
From:   claudiu beznea <claudiu.beznea@...on.dev>
To:     Hari Prasath Gujulan Elango <Hari.PrasathGE@...rochip.com>,
        thierry.reding@...il.com, u.kleine-koenig@...gutronix.de,
        nicolas.ferre@...rochip.com, alexandre.belloni@...tlin.com
Cc:     linux-pwm@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] pwm: atmel: add missing clk_disable_unprepare()



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@...rochip.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);