1. Patchew
  2. linux
  3. [PATCH 0/7] clk: st: clkgen-pll: Add cleanup in clkgen_c32_pll_setup() and clkgen_odf_register()
  4. View patch

[PATCH 7/7] clk: st: clkgen-pll: Add clk_unregister for odf_clk in clkgen_c32_pll_setup()

Haoxiang Li posted 7 patches 3 weeks, 3 days ago
There is a newer version of this series
[PATCH 7/7] clk: st: clkgen-pll: Add clk_unregister for odf_clk in clkgen_c32_pll_setup()
Posted by Haoxiang Li 3 weeks, 3 days ago 
In clkgen_c32_pll_setup(), clkgen_odf_register() allocated
clk_gate and clk_divider memory and registered a clk. Add
clk_unregister() and kfree() to release the memory if
error occurs. Initialize odf to zero for safe.

Fixes: b9b8e614b580 ("clk: st: Support for PLLs inside ClockGenA(s)")
Cc: stable@vger.kernel.org
Signed-off-by: Haoxiang Li <lihaoxiang@isrc.iscas.ac.cn>
---
 drivers/clk/st/clkgen-pll.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/st/clkgen-pll.c b/drivers/clk/st/clkgen-pll.c
index 89f0454fa72e..3fc0af4b77c6 100644
--- a/drivers/clk/st/clkgen-pll.c
+++ b/drivers/clk/st/clkgen-pll.c
@@ -761,10 +761,12 @@ static void __init clkgen_c32_pll_setup(struct device_node *np,
 	struct clk *pll_clk;
 	const char *parent_name, *pll_name;
 	void __iomem *pll_base;
-	int num_odfs, odf;
+	int num_odfs, odf = 0;
 	struct clk_onecell_data *clk_data;
 	unsigned long pll_flags = 0;
 	struct clkgen_pll *pll;
+	struct clk_gate *gate;
+	struct clk_divider *div;
 
 	parent_name = of_clk_get_parent_name(np, 0);
 	if (!parent_name)
@@ -808,7 +810,7 @@ static void __init clkgen_c32_pll_setup(struct device_node *np,
 			if (of_property_read_string_index(np,
 							  "clock-output-names",
 							  odf, &clk_name))
-				return;
+				goto err_odf_unregister;
 
 			of_clk_detect_critical(np, odf, &odf_flags);
 		}
@@ -816,8 +818,8 @@ static void __init clkgen_c32_pll_setup(struct device_node *np,
 		odf_clk = clkgen_odf_register(pll_name, pll_base, datac->data,
 				odf_flags, odf, &clkgena_c32_odf_lock,
 				clk_name);
-			goto err;
 		if (IS_ERR(odf_clk))
+			goto err_odf_unregister;
 
 		clk_data->clks[odf] = odf_clk;
 	}
@@ -825,6 +827,14 @@ static void __init clkgen_c32_pll_setup(struct device_node *np,
 	of_clk_add_provider(np, of_clk_src_onecell_get, clk_data);
 	return;
 
+err_odf_unregister:
+	while (--odf >= 0) {
+		gate = to_clk_gate(__clk_get_hw(clk_data->clks[odf]));
+		div = to_clk_divider(__clk_get_hw(clk_data->clks[odf]));
+		clk_unregister_composite(clk_data->clks[odf]);
+		kfree(div);
+		kfree(gate);
+	}
 err:
 	kfree(clk_data->clks);
 	kfree(clk_data);
-- 
2.25.1
Re: [PATCH 7/7] clk: st: clkgen-pll: Add clk_unregister for odf_clk in clkgen_c32_pll_setup()
Posted by Markus Elfring 3 weeks, 2 days ago 
> In clkgen_c32_pll_setup(), clkgen_odf_register() allocated
> clk_gate and clk_divider memory and registered a clk. Add
> clk_unregister() and kfree() to release the memory if

You may occasionally put more than 58 characters into text lines
of such a change description.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.19-rc5#n659


> error occurs. Initialize odf to zero for safe.

                                                data processing?

Regards,
Markus
Re: [PATCH 7/7] clk: st: clkgen-pll: Add clk_unregister for odf_clk in clkgen_c32_pll_setup()
Posted by Brian Masney 3 weeks, 3 days ago 
Hi Haoxiang,

On Fri, Jan 16, 2026 at 07:38:47PM +0800, Haoxiang Li wrote:
> In clkgen_c32_pll_setup(), clkgen_odf_register() allocated
> clk_gate and clk_divider memory and registered a clk. Add
> clk_unregister() and kfree() to release the memory if
> error occurs. Initialize odf to zero for safe.
> 
> Fixes: b9b8e614b580 ("clk: st: Support for PLLs inside ClockGenA(s)")
> Cc: stable@vger.kernel.org
> Signed-off-by: Haoxiang Li <lihaoxiang@isrc.iscas.ac.cn>
> ---
>  drivers/clk/st/clkgen-pll.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/st/clkgen-pll.c b/drivers/clk/st/clkgen-pll.c
> index 89f0454fa72e..3fc0af4b77c6 100644
> --- a/drivers/clk/st/clkgen-pll.c
> +++ b/drivers/clk/st/clkgen-pll.c
> @@ -761,10 +761,12 @@ static void __init clkgen_c32_pll_setup(struct device_node *np,
>  	struct clk *pll_clk;
>  	const char *parent_name, *pll_name;
>  	void __iomem *pll_base;
> -	int num_odfs, odf;
> +	int num_odfs, odf = 0;
>  	struct clk_onecell_data *clk_data;
>  	unsigned long pll_flags = 0;
>  	struct clkgen_pll *pll;
> +	struct clk_gate *gate;
> +	struct clk_divider *div;
>  
>  	parent_name = of_clk_get_parent_name(np, 0);
>  	if (!parent_name)
> @@ -808,7 +810,7 @@ static void __init clkgen_c32_pll_setup(struct device_node *np,
>  			if (of_property_read_string_index(np,
>  							  "clock-output-names",
>  							  odf, &clk_name))
> -				return;
> +				goto err_odf_unregister;
>  
>  			of_clk_detect_critical(np, odf, &odf_flags);
>  		}
> @@ -816,8 +818,8 @@ static void __init clkgen_c32_pll_setup(struct device_node *np,
>  		odf_clk = clkgen_odf_register(pll_name, pll_base, datac->data,
>  				odf_flags, odf, &clkgena_c32_odf_lock,
>  				clk_name);
> -			goto err;
>  		if (IS_ERR(odf_clk))
> +			goto err_odf_unregister;
>  
>  		clk_data->clks[odf] = odf_clk;
>  	}
> @@ -825,6 +827,14 @@ static void __init clkgen_c32_pll_setup(struct device_node *np,
>  	of_clk_add_provider(np, of_clk_src_onecell_get, clk_data);
>  	return;
>  
> +err_odf_unregister:
> +	while (--odf >= 0) {

I think the prefix -- is not appropriate here. If clkgen_odf_register()
fails for the first odf (ie odf=0), then when we jump to
err_odf_unregister, odf will still be set to 0, --odf will set it to -1,
the while loop will not run, and won't free anything.

What do you think about using the postfix operator instead?

	while (odf-- >= 0)

Brian

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.