[cocci] [PATCH 00/10] clk: ti: Adjustments for eight function implementations

[cocci] [PATCH 00/10] clk: ti: Adjustments for eight function implementations

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 24 Dec 2023 17:03:21 +0100

Several update suggestions were taken into account
from static source code analysis.

Markus Elfring (10):
  Less function calls in of_omap2_apll_setup() after error detection
  Less function calls in of_dra7_apll_setup() after error detection
  Use common code in omap_clk_register_apll()
  Less function calls in ti_fapll_setup() after error detection
  One function call less in ti_fapll_synth_setup() after error detection
  Return directly after a failed kzalloc() in of_mux_clk_setup()
  Less function calls in _ti_omap4_clkctrl_setup() after error detection
  Use common error handling code in _ti_omap4_clkctrl_setup()
  Less function calls in _ti_clkctrl_clk_register() after error detection
  Delete an unnecessary initialisation in _ti_clkctrl_clk_register()

 drivers/clk/ti/apll.c    | 58 ++++++++++++++++++++++++----------------
 drivers/clk/ti/clkctrl.c | 44 ++++++++++++++++--------------
 drivers/clk/ti/fapll.c   | 29 +++++++++++---------
 drivers/clk/ti/mux.c     |  2 +-
 4 files changed, 76 insertions(+), 57 deletions(-)

--
2.43.0

[cocci] [PATCH 01/10] clk: ti: Less function calls in of_omap2_apll_setup() after error detection

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 24 Dec 2023 11:15:27 +0100

The kfree() function was called in up to three cases by
the of_omap2_apll_setup() function during error handling
even if the passed variable contained a null pointer.
This issue was detected by using the Coccinelle software.

* Split a condition check.

* Adjust jump targets.

* Delete three initialisations which became unnecessary
  with this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/clk/ti/apll.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/ti/apll.c b/drivers/clk/ti/apll.c
index 93183287c58d..929c021f0cdb 100644
--- a/drivers/clk/ti/apll.c
+++ b/drivers/clk/ti/apll.c
@@ -338,21 +338,25 @@ static const struct clk_hw_omap_ops omap2_apll_hwops = {

 static void __init of_omap2_apll_setup(struct device_node *node)
 {
-	struct dpll_data *ad = NULL;
-	struct clk_hw_omap *clk_hw = NULL;
-	struct clk_init_data *init = NULL;
+	struct dpll_data *ad;
+	struct clk_hw_omap *clk_hw;
+	struct clk_init_data *init = kzalloc(sizeof(*init), GFP_KERNEL);
 	const char *name;
 	struct clk *clk;
 	const char *parent_name;
 	u32 val;
 	int ret;

-	ad = kzalloc(sizeof(*ad), GFP_KERNEL);
+	if (!init)
+		return;
+
 	clk_hw = kzalloc(sizeof(*clk_hw), GFP_KERNEL);
-	init = kzalloc(sizeof(*init), GFP_KERNEL);
+	if (!clk_hw)
+		goto free_init;

-	if (!ad || !clk_hw || !init)
-		goto cleanup;
+	ad = kzalloc(sizeof(*ad), GFP_KERNEL);
+	if (!ad)
+		goto free_clk_hw;

 	clk_hw->dpll_data = ad;
 	clk_hw->hw.init = init;
@@ -403,12 +407,13 @@ static void __init of_omap2_apll_setup(struct device_node *node)
 	clk = of_ti_clk_register_omap_hw(node, &clk_hw->hw, name);
 	if (!IS_ERR(clk)) {
 		of_clk_add_provider(node, of_clk_src_simple_get, clk);
-		kfree(init);
-		return;
+		goto free_init;
 	}
 cleanup:
 	kfree(ad);
+free_clk_hw:
 	kfree(clk_hw);
+free_init:
 	kfree(init);
 }
 CLK_OF_DECLARE(omap2_apll_clock, "ti,omap2-apll-clock",
--
2.43.0

[cocci] [PATCH 02/10] clk: ti: Less function calls in of_dra7_apll_setup() after error detection

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 24 Dec 2023 12:27:09 +0100

The kfree() function was called in up to four cases by
the of_dra7_apll_setup() function during error handling
even if the passed variable contained a null pointer.
This issue was detected by using the Coccinelle software.

* Split a condition check.

* Adjust jump targets.

* Delete four initialisations which became unnecessary
  with this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/clk/ti/apll.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/ti/apll.c b/drivers/clk/ti/apll.c
index 929c021f0cdb..d2be672521a3 100644
--- a/drivers/clk/ti/apll.c
+++ b/drivers/clk/ti/apll.c
@@ -177,17 +177,22 @@ static void __init omap_clk_register_apll(void *user,

 static void __init of_dra7_apll_setup(struct device_node *node)
 {
-	struct dpll_data *ad = NULL;
-	struct clk_hw_omap *clk_hw = NULL;
-	struct clk_init_data *init = NULL;
-	const char **parent_names = NULL;
+	struct dpll_data *ad;
+	struct clk_hw_omap *clk_hw;
+	struct clk_init_data *init = kzalloc(sizeof(*init), GFP_KERNEL);
+	const char **parent_names;
 	int ret;

-	ad = kzalloc(sizeof(*ad), GFP_KERNEL);
+	if (!init)
+		return;
+
 	clk_hw = kzalloc(sizeof(*clk_hw), GFP_KERNEL);
-	init = kzalloc(sizeof(*init), GFP_KERNEL);
-	if (!ad || !clk_hw || !init)
-		goto cleanup;
+	if (!clk_hw)
+		goto free_init;
+
+	ad = kzalloc(sizeof(*ad), GFP_KERNEL);
+	if (!ad)
+		goto free_clk_hw;

 	clk_hw->dpll_data = ad;
 	clk_hw->hw.init = init;
@@ -198,12 +203,12 @@ static void __init of_dra7_apll_setup(struct device_node *node)
 	init->num_parents = of_clk_get_parent_count(node);
 	if (init->num_parents < 1) {
 		pr_err("dra7 apll %pOFn must have parent(s)\n", node);
-		goto cleanup;
+		goto free_ad;
 	}

 	parent_names = kcalloc(init->num_parents, sizeof(char *), GFP_KERNEL);
 	if (!parent_names)
-		goto cleanup;
+		goto free_ad;

 	of_clk_parent_fill(node, parent_names, init->num_parents);

@@ -223,8 +228,11 @@ static void __init of_dra7_apll_setup(struct device_node *node)

 cleanup:
 	kfree(parent_names);
+free_ad:
 	kfree(ad);
+free_clk_hw:
 	kfree(clk_hw);
+free_init:
 	kfree(init);
 }
 CLK_OF_DECLARE(dra7_apll_clock, "ti,dra7-apll-clock", of_dra7_apll_setup);
--
2.43.0

[cocci] [PATCH 03/10] clk: ti: Use common code in omap_clk_register_apll()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 24 Dec 2023 12:40:59 +0100

Use another label so that two function calls can be better reused
at the end of this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/clk/ti/apll.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/ti/apll.c b/drivers/clk/ti/apll.c
index d2be672521a3..406326883741 100644
--- a/drivers/clk/ti/apll.c
+++ b/drivers/clk/ti/apll.c
@@ -163,16 +163,15 @@ static void __init omap_clk_register_apll(void *user,
 	clk = of_ti_clk_register_omap_hw(node, &clk_hw->hw, name);
 	if (!IS_ERR(clk)) {
 		of_clk_add_provider(node, of_clk_src_simple_get, clk);
-		kfree(init->parent_names);
-		kfree(init);
-		return;
+		goto free_names;
 	}

 cleanup:
 	kfree(clk_hw->dpll_data);
+	kfree(clk_hw);
+free_names:
 	kfree(init->parent_names);
 	kfree(init);
-	kfree(clk_hw);
 }

 static void __init of_dra7_apll_setup(struct device_node *node)
--
2.43.0

[cocci] [PATCH 04/10] clk: ti: Less function calls in ti_fapll_setup() after error detection

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 24 Dec 2023 13:55:44 +0100

The kfree() function was called in some cases by
the ti_fapll_setup() function during error handling
even if the passed variable contained a null pointer.

* Adjust jump targets.

* Delete two condition checks which became unnecessary
  with this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/clk/ti/fapll.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/ti/fapll.c b/drivers/clk/ti/fapll.c
index 2db3fc4a443e..e9956e3ccd65 100644
--- a/drivers/clk/ti/fapll.c
+++ b/drivers/clk/ti/fapll.c
@@ -546,11 +546,11 @@ static void __init ti_fapll_setup(struct device_node *node)
 				   MAX_FAPLL_OUTPUTS + 1,
 				   GFP_KERNEL);
 	if (!fd->outputs.clks)
-		goto free;
+		goto free_fd;

 	init = kzalloc(sizeof(*init), GFP_KERNEL);
 	if (!init)
-		goto free;
+		goto free_clks;

 	init->ops = &ti_fapll_ops;
 	name = ti_dt_clk_name(node);
@@ -559,7 +559,7 @@ static void __init ti_fapll_setup(struct device_node *node)
 	init->num_parents = of_clk_get_parent_count(node);
 	if (init->num_parents != 2) {
 		pr_err("%pOFn must have two parents\n", node);
-		goto free;
+		goto free_init;
 	}

 	of_clk_parent_fill(node, parent_name, 2);
@@ -568,19 +568,19 @@ static void __init ti_fapll_setup(struct device_node *node)
 	fd->clk_ref = of_clk_get(node, 0);
 	if (IS_ERR(fd->clk_ref)) {
 		pr_err("%pOFn could not get clk_ref\n", node);
-		goto free;
+		goto free_init;
 	}

 	fd->clk_bypass = of_clk_get(node, 1);
 	if (IS_ERR(fd->clk_bypass)) {
 		pr_err("%pOFn could not get clk_bypass\n", node);
-		goto free;
+		goto put_clk_ref;
 	}

 	fd->base = of_iomap(node, 0);
 	if (!fd->base) {
 		pr_err("%pOFn could not get IO base\n", node);
-		goto free;
+		goto put_clk_bypass;
 	}

 	if (fapll_is_ddr_pll(fd->base))
@@ -653,14 +653,16 @@ static void __init ti_fapll_setup(struct device_node *node)

 unmap:
 	iounmap(fd->base);
-free:
-	if (fd->clk_bypass)
-		clk_put(fd->clk_bypass);
-	if (fd->clk_ref)
-		clk_put(fd->clk_ref);
+put_clk_bypass:
+	clk_put(fd->clk_bypass);
+put_clk_ref:
+	clk_put(fd->clk_ref);
+free_init:
+	kfree(init);
+free_clks:
 	kfree(fd->outputs.clks);
+free_fd:
 	kfree(fd);
-	kfree(init);
 }

 CLK_OF_DECLARE(ti_fapll_clock, "ti,dm816-fapll-clock", ti_fapll_setup);
--
2.43.0

[cocci] [PATCH 05/10] clk: ti: One function call less in ti_fapll_synth_setup() after error detection

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 24 Dec 2023 14:05:41 +0100

The kfree() function was called in one case by
the ti_fapll_synth_setup() function during error handling
even if the passed variable contained a null pointer.
This issue was detected by using the Coccinelle software.

Thus use another label.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/clk/ti/fapll.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/ti/fapll.c b/drivers/clk/ti/fapll.c
index e9956e3ccd65..d4674ec3d7e9 100644
--- a/drivers/clk/ti/fapll.c
+++ b/drivers/clk/ti/fapll.c
@@ -504,7 +504,7 @@ static struct clk * __init ti_fapll_synth_setup(struct fapll_data *fd,

 	synth = kzalloc(sizeof(*synth), GFP_KERNEL);
 	if (!synth)
-		goto free;
+		goto free_init;

 	synth->fd = fd;
 	synth->index = index;
@@ -524,6 +524,7 @@ static struct clk * __init ti_fapll_synth_setup(struct fapll_data *fd,

 free:
 	kfree(synth);
+free_init:
 	kfree(init);

 	return clk;
--
2.43.0

[cocci] [PATCH 06/10] clk: ti: Return directly after a failed kzalloc() in of_mux_clk_setup()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 24 Dec 2023 14:20:18 +0100

The kfree() function was called in one case by
the of_mux_clk_setup() function during error handling
even if the passed variable contained a null pointer.
This issue was detected by using the Coccinelle software.

Thus return directly after a call of the function “kzalloc” failed
at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/clk/ti/mux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/ti/mux.c b/drivers/clk/ti/mux.c
index 1ebafa386be6..ab1205fa40d6 100644
--- a/drivers/clk/ti/mux.c
+++ b/drivers/clk/ti/mux.c
@@ -182,7 +182,7 @@ static void of_mux_clk_setup(struct device_node *node)
 	}
 	parent_names = kzalloc((sizeof(char *) * num_parents), GFP_KERNEL);
 	if (!parent_names)
-		goto cleanup;
+		return;

 	of_clk_parent_fill(node, parent_names, num_parents);

--
2.43.0

[cocci] [PATCH 07/10] clk: ti: Less function calls in _ti_omap4_clkctrl_setup() after error detection

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 24 Dec 2023 15:46:04 +0100

The kfree() function was called in a few cases by
the _ti_omap4_clkctrl_setup() function during error handling
even if the passed variable contained a null pointer.
Thus adjust jump targets.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/clk/ti/clkctrl.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/ti/clkctrl.c b/drivers/clk/ti/clkctrl.c
index 607e34d8e289..82b48548818b 100644
--- a/drivers/clk/ti/clkctrl.c
+++ b/drivers/clk/ti/clkctrl.c
@@ -681,11 +681,11 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)
 						   reg_data->offset, 0,
 						   legacy_naming);
 		if (!init.name)
-			goto cleanup;
+			goto free_clkctrl_name;

 		clkctrl_clk = kzalloc(sizeof(*clkctrl_clk), GFP_KERNEL);
 		if (!clkctrl_clk)
-			goto cleanup;
+			goto free_init_name;

 		init.ops = &omap4_clkctrl_clk_ops;
 		hw->hw.init = &init;
@@ -711,10 +711,12 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)
 	return;

 cleanup:
-	kfree(hw);
+	kfree(clkctrl_clk);
+free_init_name:
 	kfree(init.name);
+free_clkctrl_name:
 	kfree(clkctrl_name);
-	kfree(clkctrl_clk);
+	kfree(hw);
 }
 CLK_OF_DECLARE(ti_omap4_clkctrl_clock, "ti,clkctrl",
 	       _ti_omap4_clkctrl_setup);
--
2.43.0

[cocci] [PATCH 08/10] clk: ti: Use common error handling code in _ti_omap4_clkctrl_setup()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 24 Dec 2023 15:56:08 +0100

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/clk/ti/clkctrl.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/ti/clkctrl.c b/drivers/clk/ti/clkctrl.c
index 82b48548818b..5a1bd176160c 100644
--- a/drivers/clk/ti/clkctrl.c
+++ b/drivers/clk/ti/clkctrl.c
@@ -590,10 +590,9 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)
 	if (clkctrl_name) {
 		provider->clkdm_name = kasprintf(GFP_KERNEL,
 						 "%s_clkdm", clkctrl_name);
-		if (!provider->clkdm_name) {
-			kfree(provider);
-			return;
-		}
+		if (!provider->clkdm_name)
+			goto free_provider;
+
 		goto clkdm_found;
 	}

@@ -603,10 +602,8 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)
 	 */
 	if (legacy_naming) {
 		provider->clkdm_name = kasprintf(GFP_KERNEL, "%pOFnxxx", node->parent);
-		if (!provider->clkdm_name) {
-			kfree(provider);
-			return;
-		}
+		if (!provider->clkdm_name)
+			goto free_provider;

 		/*
 		 * Create default clkdm name, replace _cm from end of parent
@@ -615,10 +612,8 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)
 		provider->clkdm_name[strlen(provider->clkdm_name) - 2] = 0;
 	} else {
 		provider->clkdm_name = kasprintf(GFP_KERNEL, "%pOFn", node);
-		if (!provider->clkdm_name) {
-			kfree(provider);
-			return;
-		}
+		if (!provider->clkdm_name)
+			goto free_provider;

 		/*
 		 * Create default clkdm name, replace _clkctrl from end of
@@ -710,4 +705,8 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)

 	return;

+free_provider:
+	kfree(provider);
+	return;
+
 cleanup:
--
2.43.0

[cocci] [PATCH 09/10] clk: ti: Less function calls in _ti_clkctrl_clk_register() after error detection

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 24 Dec 2023 16:06:34 +0100

The kfree() function was called in a few cases by
the of_omap2_apll_setup() function during error handling
even if the passed variable contained a null pointer.

* Split a condition check.

* Adjust jump targets.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/clk/ti/clkctrl.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/ti/clkctrl.c b/drivers/clk/ti/clkctrl.c
index 5a1bd176160c..cdc3cf1ddddf 100644
--- a/drivers/clk/ti/clkctrl.c
+++ b/drivers/clk/ti/clkctrl.c
@@ -296,11 +296,13 @@ _ti_clkctrl_clk_register(struct omap_clkctrl_provider *provider,
 	init.name = clkctrl_get_clock_name(node, clkctrl_name, offset, bit,
 					   ti_clk_get_features()->flags &
 					   TI_CLK_CLKCTRL_COMPAT);
+	if (!init.name)
+		return -ENOMEM;

 	clkctrl_clk = kzalloc(sizeof(*clkctrl_clk), GFP_KERNEL);
-	if (!init.name || !clkctrl_clk) {
+	if (!clkctrl_clk) {
 		ret = -ENOMEM;
-		goto cleanup;
+		goto free_init_name;
 	}

 	clk_hw->init = &init;
@@ -324,8 +326,9 @@ _ti_clkctrl_clk_register(struct omap_clkctrl_provider *provider,
 	return 0;

 cleanup:
-	kfree(init.name);
 	kfree(clkctrl_clk);
+free_init_name;
+	kfree(init.name);
 	return ret;
 }

--
2.43.0

[cocci] [PATCH 10/10] clk: ti: Delete an unnecessary initialisation in _ti_clkctrl_clk_register()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 24 Dec 2023 16:27:33 +0100

The variable “ret” will eventually be set to an appropriate value
a bit later. Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/clk/ti/clkctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/ti/clkctrl.c b/drivers/clk/ti/clkctrl.c
index cdc3cf1ddddf..b9df75c6cc50 100644
--- a/drivers/clk/ti/clkctrl.c
+++ b/drivers/clk/ti/clkctrl.c
@@ -291,7 +291,7 @@ _ti_clkctrl_clk_register(struct omap_clkctrl_provider *provider,
 	struct clk_init_data init = { NULL };
 	struct clk *clk;
 	struct omap_clkctrl_clk *clkctrl_clk;
-	int ret = 0;
+	int ret;

 	init.name = clkctrl_get_clock_name(node, clkctrl_name, offset, bit,
 					   ti_clk_get_features()->flags &
--
2.43.0

Re: [cocci] [09/10] clk: ti: Less function calls in _ti_clkctrl_clk_register() after error detection

[Markus Elfring]

…
>    drivers/clk/ti/clkctrl.c: In function '_ti_clkctrl_clk_register':
>>> drivers/clk/ti/clkctrl.c:330:1: error: 'free_init_name' undeclared (first use in this function)
…
>    328	cleanup:
>    329		kfree(clkctrl_clk);
>  > 330	free_init_name;
>    331		kfree(init.name);
>    332		return ret;
…

Will it become helpful to fix a typo for the delimiter of the shown label?

   Or:

Are you looking for further adjustments according to software design options
which would be supported by the file “cleanup.h”?
https://elixir.bootlin.com/linux/v6.7-rc7/source/include/linux/cleanup.h

Regards,
Markus