public inbox for linux-omap@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] clk: ti: Adjustments for eight function implementations
@ 2023-12-24 16:33 Markus Elfring
  2023-12-24 16:36 ` [PATCH 01/10] clk: ti: Less function calls in of_omap2_apll_setup() after error detection Markus Elfring
                   ` (10 more replies)
  0 siblings, 11 replies; 16+ messages in thread
From: Markus Elfring @ 2023-12-24 16:33 UTC (permalink / raw)
  To: linux-omap, linux-clk, kernel-janitors, Andy Shevchenko,
	Claudiu Beznea, Michael Turquette, Rob Herring, Stephen Boyd,
	Tero Kristo, Tony Lindgren
  Cc: LKML, cocci

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 01/10] clk: ti: Less function calls in of_omap2_apll_setup() after error detection
  2023-12-24 16:33 [PATCH 00/10] clk: ti: Adjustments for eight function implementations Markus Elfring
@ 2023-12-24 16:36 ` Markus Elfring
  2023-12-27 16:38   ` Andy Shevchenko
  2023-12-24 16:38 ` [PATCH 02/10] clk: ti: Less function calls in of_dra7_apll_setup() " Markus Elfring
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Markus Elfring @ 2023-12-24 16:36 UTC (permalink / raw)
  To: linux-omap, linux-clk, kernel-janitors, Andy Shevchenko,
	Claudiu Beznea, Michael Turquette, Rob Herring, Stephen Boyd,
	Tero Kristo, Tony Lindgren
  Cc: LKML, cocci

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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 02/10] clk: ti: Less function calls in of_dra7_apll_setup() after error detection
  2023-12-24 16:33 [PATCH 00/10] clk: ti: Adjustments for eight function implementations Markus Elfring
  2023-12-24 16:36 ` [PATCH 01/10] clk: ti: Less function calls in of_omap2_apll_setup() after error detection Markus Elfring
@ 2023-12-24 16:38 ` Markus Elfring
  2023-12-24 16:40 ` [PATCH 03/10] clk: ti: Use common code in omap_clk_register_apll() Markus Elfring
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Markus Elfring @ 2023-12-24 16:38 UTC (permalink / raw)
  To: linux-omap, linux-clk, kernel-janitors, Andy Shevchenko,
	Claudiu Beznea, Michael Turquette, Rob Herring, Stephen Boyd,
	Tero Kristo, Tony Lindgren
  Cc: LKML, cocci

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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 03/10] clk: ti: Use common code in omap_clk_register_apll()
  2023-12-24 16:33 [PATCH 00/10] clk: ti: Adjustments for eight function implementations Markus Elfring
  2023-12-24 16:36 ` [PATCH 01/10] clk: ti: Less function calls in of_omap2_apll_setup() after error detection Markus Elfring
  2023-12-24 16:38 ` [PATCH 02/10] clk: ti: Less function calls in of_dra7_apll_setup() " Markus Elfring
@ 2023-12-24 16:40 ` Markus Elfring
  2023-12-24 16:42 ` [PATCH 04/10] clk: ti: Less function calls in ti_fapll_setup() after error detection Markus Elfring
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Markus Elfring @ 2023-12-24 16:40 UTC (permalink / raw)
  To: linux-omap, linux-clk, kernel-janitors, Andy Shevchenko,
	Claudiu Beznea, Michael Turquette, Rob Herring, Stephen Boyd,
	Tero Kristo, Tony Lindgren
  Cc: LKML, cocci

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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 04/10] clk: ti: Less function calls in ti_fapll_setup() after error detection
  2023-12-24 16:33 [PATCH 00/10] clk: ti: Adjustments for eight function implementations Markus Elfring
                   ` (2 preceding siblings ...)
  2023-12-24 16:40 ` [PATCH 03/10] clk: ti: Use common code in omap_clk_register_apll() Markus Elfring
@ 2023-12-24 16:42 ` Markus Elfring
  2023-12-24 16:44 ` [PATCH 05/10] clk: ti: One function call less in ti_fapll_synth_setup() " Markus Elfring
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Markus Elfring @ 2023-12-24 16:42 UTC (permalink / raw)
  To: linux-omap, linux-clk, kernel-janitors, Andy Shevchenko,
	Claudiu Beznea, Michael Turquette, Rob Herring, Stephen Boyd,
	Tero Kristo, Tony Lindgren
  Cc: LKML, cocci

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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 05/10] clk: ti: One function call less in ti_fapll_synth_setup() after error detection
  2023-12-24 16:33 [PATCH 00/10] clk: ti: Adjustments for eight function implementations Markus Elfring
                   ` (3 preceding siblings ...)
  2023-12-24 16:42 ` [PATCH 04/10] clk: ti: Less function calls in ti_fapll_setup() after error detection Markus Elfring
@ 2023-12-24 16:44 ` Markus Elfring
  2023-12-24 16:45 ` [PATCH 06/10] clk: ti: Return directly after a failed kzalloc() in of_mux_clk_setup() Markus Elfring
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Markus Elfring @ 2023-12-24 16:44 UTC (permalink / raw)
  To: linux-omap, linux-clk, kernel-janitors, Andy Shevchenko,
	Claudiu Beznea, Michael Turquette, Rob Herring, Stephen Boyd,
	Tero Kristo, Tony Lindgren
  Cc: LKML, cocci

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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 06/10] clk: ti: Return directly after a failed kzalloc() in of_mux_clk_setup()
  2023-12-24 16:33 [PATCH 00/10] clk: ti: Adjustments for eight function implementations Markus Elfring
                   ` (4 preceding siblings ...)
  2023-12-24 16:44 ` [PATCH 05/10] clk: ti: One function call less in ti_fapll_synth_setup() " Markus Elfring
@ 2023-12-24 16:45 ` Markus Elfring
  2023-12-24 16:47 ` [PATCH 07/10] clk: ti: Less function calls in _ti_omap4_clkctrl_setup() after error detection Markus Elfring
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Markus Elfring @ 2023-12-24 16:45 UTC (permalink / raw)
  To: linux-omap, linux-clk, kernel-janitors, Andy Shevchenko,
	Claudiu Beznea, Michael Turquette, Rob Herring, Stephen Boyd,
	Tero Kristo, Tony Lindgren
  Cc: LKML, cocci

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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 07/10] clk: ti: Less function calls in _ti_omap4_clkctrl_setup() after error detection
  2023-12-24 16:33 [PATCH 00/10] clk: ti: Adjustments for eight function implementations Markus Elfring
                   ` (5 preceding siblings ...)
  2023-12-24 16:45 ` [PATCH 06/10] clk: ti: Return directly after a failed kzalloc() in of_mux_clk_setup() Markus Elfring
@ 2023-12-24 16:47 ` Markus Elfring
  2023-12-24 16:48 ` [PATCH 08/10] clk: ti: Use common error handling code in _ti_omap4_clkctrl_setup() Markus Elfring
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Markus Elfring @ 2023-12-24 16:47 UTC (permalink / raw)
  To: linux-omap, linux-clk, kernel-janitors, Andy Shevchenko,
	Claudiu Beznea, Michael Turquette, Rob Herring, Stephen Boyd,
	Tero Kristo, Tony Lindgren
  Cc: LKML, cocci

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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 08/10] clk: ti: Use common error handling code in _ti_omap4_clkctrl_setup()
  2023-12-24 16:33 [PATCH 00/10] clk: ti: Adjustments for eight function implementations Markus Elfring
                   ` (6 preceding siblings ...)
  2023-12-24 16:47 ` [PATCH 07/10] clk: ti: Less function calls in _ti_omap4_clkctrl_setup() after error detection Markus Elfring
@ 2023-12-24 16:48 ` Markus Elfring
  2023-12-24 16:50 ` [PATCH 09/10] clk: ti: Less function calls in _ti_clkctrl_clk_register() after error detection Markus Elfring
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Markus Elfring @ 2023-12-24 16:48 UTC (permalink / raw)
  To: linux-omap, linux-clk, kernel-janitors, Andy Shevchenko,
	Claudiu Beznea, Michael Turquette, Rob Herring, Stephen Boyd,
	Tero Kristo, Tony Lindgren
  Cc: LKML, cocci

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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 09/10] clk: ti: Less function calls in _ti_clkctrl_clk_register() after error detection
  2023-12-24 16:33 [PATCH 00/10] clk: ti: Adjustments for eight function implementations Markus Elfring
                   ` (7 preceding siblings ...)
  2023-12-24 16:48 ` [PATCH 08/10] clk: ti: Use common error handling code in _ti_omap4_clkctrl_setup() Markus Elfring
@ 2023-12-24 16:50 ` Markus Elfring
  2023-12-28  5:18   ` kernel test robot
  2023-12-24 16:52 ` [PATCH 10/10] clk: ti: Delete an unnecessary initialisation in _ti_clkctrl_clk_register() Markus Elfring
  2023-12-27 16:39 ` [PATCH 00/10] clk: ti: Adjustments for eight function implementations Andy Shevchenko
  10 siblings, 1 reply; 16+ messages in thread
From: Markus Elfring @ 2023-12-24 16:50 UTC (permalink / raw)
  To: linux-omap, linux-clk, kernel-janitors, Andy Shevchenko,
	Claudiu Beznea, Michael Turquette, Rob Herring, Stephen Boyd,
	Tero Kristo, Tony Lindgren
  Cc: LKML, cocci

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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 10/10] clk: ti: Delete an unnecessary initialisation in _ti_clkctrl_clk_register()
  2023-12-24 16:33 [PATCH 00/10] clk: ti: Adjustments for eight function implementations Markus Elfring
                   ` (8 preceding siblings ...)
  2023-12-24 16:50 ` [PATCH 09/10] clk: ti: Less function calls in _ti_clkctrl_clk_register() after error detection Markus Elfring
@ 2023-12-24 16:52 ` Markus Elfring
  2023-12-27 16:39 ` [PATCH 00/10] clk: ti: Adjustments for eight function implementations Andy Shevchenko
  10 siblings, 0 replies; 16+ messages in thread
From: Markus Elfring @ 2023-12-24 16:52 UTC (permalink / raw)
  To: linux-omap, linux-clk, kernel-janitors, Andy Shevchenko,
	Claudiu Beznea, Michael Turquette, Rob Herring, Stephen Boyd,
	Tero Kristo, Tony Lindgren
  Cc: LKML, cocci

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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 01/10] clk: ti: Less function calls in of_omap2_apll_setup() after error detection
  2023-12-24 16:36 ` [PATCH 01/10] clk: ti: Less function calls in of_omap2_apll_setup() after error detection Markus Elfring
@ 2023-12-27 16:38   ` Andy Shevchenko
  0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2023-12-27 16:38 UTC (permalink / raw)
  To: Markus Elfring
  Cc: linux-omap, linux-clk, kernel-janitors, Claudiu Beznea,
	Michael Turquette, Rob Herring, Stephen Boyd, Tero Kristo,
	Tony Lindgren, LKML, cocci

On Sun, Dec 24, 2023 at 05:36:57PM +0100, Markus Elfring wrote:
> 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.

Instead, make use of cleanup.h.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 00/10] clk: ti: Adjustments for eight function implementations
  2023-12-24 16:33 [PATCH 00/10] clk: ti: Adjustments for eight function implementations Markus Elfring
                   ` (9 preceding siblings ...)
  2023-12-24 16:52 ` [PATCH 10/10] clk: ti: Delete an unnecessary initialisation in _ti_clkctrl_clk_register() Markus Elfring
@ 2023-12-27 16:39 ` Andy Shevchenko
  2024-02-20  9:27   ` Tony Lindgren
  10 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2023-12-27 16:39 UTC (permalink / raw)
  To: Markus Elfring
  Cc: linux-omap, linux-clk, kernel-janitors, Claudiu Beznea,
	Michael Turquette, Rob Herring, Stephen Boyd, Tero Kristo,
	Tony Lindgren, LKML, cocci

On Sun, Dec 24, 2023 at 05:33:53PM +0100, Markus Elfring wrote:
> 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.

Unneeded churn, if you want to make it better, switch the code to use
cleanup.h.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 09/10] clk: ti: Less function calls in _ti_clkctrl_clk_register() after error detection
  2023-12-24 16:50 ` [PATCH 09/10] clk: ti: Less function calls in _ti_clkctrl_clk_register() after error detection Markus Elfring
@ 2023-12-28  5:18   ` kernel test robot
  2023-12-28  6:39     ` [09/10] " Markus Elfring
  0 siblings, 1 reply; 16+ messages in thread
From: kernel test robot @ 2023-12-28  5:18 UTC (permalink / raw)
  To: Markus Elfring, linux-omap, linux-clk, kernel-janitors,
	Andy Shevchenko, Claudiu Beznea, Michael Turquette, Rob Herring,
	Stephen Boyd, Tero Kristo, Tony Lindgren
  Cc: oe-kbuild-all, LKML, cocci

Hi Markus,

kernel test robot noticed the following build errors:

[auto build test ERROR on clk/clk-next]
[also build test ERROR on linus/master v6.7-rc7 next-20231222]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Markus-Elfring/clk-ti-Less-function-calls-in-of_omap2_apll_setup-after-error-detection/20231225-152410
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link:    https://lore.kernel.org/r/b11039e4-69c6-4247-b4ba-c442b9427231%40web.de
patch subject: [PATCH 09/10] clk: ti: Less function calls in _ti_clkctrl_clk_register() after error detection
config: arm-allmodconfig (https://download.01.org/0day-ci/archive/20231228/202312281350.5H2Rhh67-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231228/202312281350.5H2Rhh67-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312281350.5H2Rhh67-lkp@intel.com/

All errors (new ones prefixed by >>):

   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)
     330 | free_init_name;
         | ^~~~~~~~~~~~~~
   drivers/clk/ti/clkctrl.c:330:1: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/clk/ti/clkctrl.c:305:17: error: label 'free_init_name' used but not defined
     305 |                 goto free_init_name;
         |                 ^~~~


vim +/free_init_name +330 drivers/clk/ti/clkctrl.c

   283	
   284	static int __init
   285	_ti_clkctrl_clk_register(struct omap_clkctrl_provider *provider,
   286				 struct device_node *node, struct clk_hw *clk_hw,
   287				 u16 offset, u8 bit, const char * const *parents,
   288				 int num_parents, const struct clk_ops *ops,
   289				 const char *clkctrl_name)
   290	{
   291		struct clk_init_data init = { NULL };
   292		struct clk *clk;
   293		struct omap_clkctrl_clk *clkctrl_clk;
   294		int ret = 0;
   295	
   296		init.name = clkctrl_get_clock_name(node, clkctrl_name, offset, bit,
   297						   ti_clk_get_features()->flags &
   298						   TI_CLK_CLKCTRL_COMPAT);
   299		if (!init.name)
   300			return -ENOMEM;
   301	
   302		clkctrl_clk = kzalloc(sizeof(*clkctrl_clk), GFP_KERNEL);
   303		if (!clkctrl_clk) {
   304			ret = -ENOMEM;
 > 305			goto free_init_name;
   306		}
   307	
   308		clk_hw->init = &init;
   309		init.parent_names = parents;
   310		init.num_parents = num_parents;
   311		init.ops = ops;
   312		init.flags = 0;
   313	
   314		clk = of_ti_clk_register(node, clk_hw, init.name);
   315		if (IS_ERR_OR_NULL(clk)) {
   316			ret = -EINVAL;
   317			goto cleanup;
   318		}
   319	
   320		clkctrl_clk->reg_offset = offset;
   321		clkctrl_clk->bit_offset = bit;
   322		clkctrl_clk->clk = clk_hw;
   323	
   324		list_add(&clkctrl_clk->node, &provider->clocks);
   325	
   326		return 0;
   327	
   328	cleanup:
   329		kfree(clkctrl_clk);
 > 330	free_init_name;
   331		kfree(init.name);
   332		return ret;
   333	}
   334	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [09/10] clk: ti: Less function calls in _ti_clkctrl_clk_register() after error detection
  2023-12-28  5:18   ` kernel test robot
@ 2023-12-28  6:39     ` Markus Elfring
  0 siblings, 0 replies; 16+ messages in thread
From: Markus Elfring @ 2023-12-28  6:39 UTC (permalink / raw)
  To: kernel test robot, linux-omap, linux-clk, kernel-janitors,
	Andy Shevchenko, Claudiu Beznea, Michael Turquette, Rob Herring,
	Stephen Boyd, Tero Kristo, Tony Lindgren
  Cc: oe-kbuild-all, LKML, cocci

…
>    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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 00/10] clk: ti: Adjustments for eight function implementations
  2023-12-27 16:39 ` [PATCH 00/10] clk: ti: Adjustments for eight function implementations Andy Shevchenko
@ 2024-02-20  9:27   ` Tony Lindgren
  0 siblings, 0 replies; 16+ messages in thread
From: Tony Lindgren @ 2024-02-20  9:27 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Markus Elfring, linux-omap, linux-clk, kernel-janitors,
	Claudiu Beznea, Michael Turquette, Rob Herring, Stephen Boyd,
	Tero Kristo, LKML, cocci

* Andy Shevchenko <andriy.shevchenko@linux.intel.com> [231227 16:39]:
> On Sun, Dec 24, 2023 at 05:33:53PM +0100, Markus Elfring wrote:
> > 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.
> 
> Unneeded churn, if you want to make it better, switch the code to use
> cleanup.h.

Yes cleanup.h sounds good to me too.

Regards,

Tony

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-02-20  9:27 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-24 16:33 [PATCH 00/10] clk: ti: Adjustments for eight function implementations Markus Elfring
2023-12-24 16:36 ` [PATCH 01/10] clk: ti: Less function calls in of_omap2_apll_setup() after error detection Markus Elfring
2023-12-27 16:38   ` Andy Shevchenko
2023-12-24 16:38 ` [PATCH 02/10] clk: ti: Less function calls in of_dra7_apll_setup() " Markus Elfring
2023-12-24 16:40 ` [PATCH 03/10] clk: ti: Use common code in omap_clk_register_apll() Markus Elfring
2023-12-24 16:42 ` [PATCH 04/10] clk: ti: Less function calls in ti_fapll_setup() after error detection Markus Elfring
2023-12-24 16:44 ` [PATCH 05/10] clk: ti: One function call less in ti_fapll_synth_setup() " Markus Elfring
2023-12-24 16:45 ` [PATCH 06/10] clk: ti: Return directly after a failed kzalloc() in of_mux_clk_setup() Markus Elfring
2023-12-24 16:47 ` [PATCH 07/10] clk: ti: Less function calls in _ti_omap4_clkctrl_setup() after error detection Markus Elfring
2023-12-24 16:48 ` [PATCH 08/10] clk: ti: Use common error handling code in _ti_omap4_clkctrl_setup() Markus Elfring
2023-12-24 16:50 ` [PATCH 09/10] clk: ti: Less function calls in _ti_clkctrl_clk_register() after error detection Markus Elfring
2023-12-28  5:18   ` kernel test robot
2023-12-28  6:39     ` [09/10] " Markus Elfring
2023-12-24 16:52 ` [PATCH 10/10] clk: ti: Delete an unnecessary initialisation in _ti_clkctrl_clk_register() Markus Elfring
2023-12-27 16:39 ` [PATCH 00/10] clk: ti: Adjustments for eight function implementations Andy Shevchenko
2024-02-20  9:27   ` Tony Lindgren

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox