[PATCH] clk: devres: make clock cleanup explicit on failure

[PATCH] clk: devres: make clock cleanup explicit on failure

[Onur Özkan]

Use devm_add_action() and explicitly disable and unprepare the clock if
registering the action fails to avoid smatch warning.

Link: https://lore.kernel.org/all/apmUPdYtdIM0dQbF@redhat.com
Closes: https://lore.kernel.org/r/202608090025.dwxdfqFr-lkp@intel.com
Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
 drivers/clk/clk-devres.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
index 4cc3863bd7da..9ed9a7b7b23f 100644
--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c
@@ -123,9 +123,11 @@ struct clk *devm_clk_get_optional_enabled_with_rate(struct device *dev,
 	if (ret)
 		goto out_put_clk;
 
-	ret = devm_add_action_or_reset(dev, devm_clk_disable_unprepare, clk);
-	if (ret)
+	ret = devm_add_action(dev, devm_clk_disable_unprepare, clk);
+	if (ret) {
+		clk_disable_unprepare(clk);
 		goto out_put_clk;
+	}
 
 	return clk;
 
-- 
2.51.2

Re: [PATCH] clk: devres: make clock cleanup explicit on failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Replaces devm_add_action_or_reset() with open-coded error handling to pacify a static analyzer.
--

commit f91fcabb46a07cc1fa1a03a8bac1f236304fd974
Author: Onur Özkan <work@onurozkan.dev>
Subject: clk: devres: make clock cleanup explicit on failure

This commit replaces the standard devm_add_action_or_reset() helper with
devm_add_action() and an explicitly open-coded clock disable and unprepare
sequence on failure. This change was made to address a smatch warning.

Link: https://lore.kernel.org/all/apmUPdYtdIM0dQbF@redhat.com
Closes: https://lore.kernel.org/r/202608090025.dwxdfqFr-lkp@intel.com

> diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
> index 4cc3863bd7da9..9ed9a7b7b23f4 100644
> --- a/drivers/clk/clk-devres.c
> +++ b/drivers/clk/clk-devres.c
> @@ -123,9 +123,11 @@ struct clk *devm_clk_get_optional_enabled_with_rate(struct device *dev,
>  	if (ret)
>  		goto out_put_clk;
>  
> -	ret = devm_add_action_or_reset(dev, devm_clk_disable_unprepare, clk);
> -	if (ret)
> +	ret = devm_add_action(dev, devm_clk_disable_unprepare, clk);
> +	if (ret) {
> +		clk_disable_unprepare(clk);
>  		goto out_put_clk;
> +	}

[Severity: Low]
Is it necessary to open-code this cleanup logic in
devm_clk_get_optional_enabled_with_rate() just to satisfy a static analyzer?

Replacing standard kernel abstractions like devm_add_action_or_reset() with
manual error handling to pacify a false positive degrades code structure and
reverts the benefits of the devres pattern.

Should the analyzer be fixed instead of open-coding the manual cleanup
sequence here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904075026.60793-1-work@onurozkan.dev?part=1

Re: [PATCH] clk: devres: make clock cleanup explicit on failure

[Jerome Brunet]

On ven. 04 sept. 2026 at 08:01, sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Replaces devm_add_action_or_reset() with open-coded error handling to pacify a static analyzer.
> --
>
> commit f91fcabb46a07cc1fa1a03a8bac1f236304fd974
> Author: Onur Özkan <work@onurozkan.dev>
> Subject: clk: devres: make clock cleanup explicit on failure
>
> This commit replaces the standard devm_add_action_or_reset() helper with
> devm_add_action() and an explicitly open-coded clock disable and unprepare
> sequence on failure. This change was made to address a smatch warning.
>
> Link: https://lore.kernel.org/all/apmUPdYtdIM0dQbF@redhat.com
> Closes: https://lore.kernel.org/r/202608090025.dwxdfqFr-lkp@intel.com

Note that when you are fixing something, it usually needs a Fixes tag too.

>
>> diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
>> index 4cc3863bd7da9..9ed9a7b7b23f4 100644
>> --- a/drivers/clk/clk-devres.c
>> +++ b/drivers/clk/clk-devres.c
>> @@ -123,9 +123,11 @@ struct clk *devm_clk_get_optional_enabled_with_rate(struct device *dev,
>>  	if (ret)
>>  		goto out_put_clk;
>>  
>> -	ret = devm_add_action_or_reset(dev, devm_clk_disable_unprepare, clk);
>> -	if (ret)
>> +	ret = devm_add_action(dev, devm_clk_disable_unprepare, clk);
>> +	if (ret) {
>> +		clk_disable_unprepare(clk);
>>  		goto out_put_clk;
>> +	}
>
> [Severity: Low]
> Is it necessary to open-code this cleanup logic in
> devm_clk_get_optional_enabled_with_rate() just to satisfy a static analyzer?
>
> Replacing standard kernel abstractions like devm_add_action_or_reset() with
> manual error handling to pacify a false positive degrades code structure and
> reverts the benefits of the devres pattern.
>
> Should the analyzer be fixed instead of open-coding the manual cleanup
> sequence here?

I tend to agree. If we are missing something, please ellaborate.

>
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260904075026.60793-1-work@onurozkan.dev?part=1

-- 
Jerome