Skip to contentSkip to conversation

clk: h8300: fix error handling in h8s2678_pll_clk_setup()

PATCH In review Dan Carpenter 2 participants 2 messages May 14, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 clk: h8300: fix error handling in h8s2678_pll_clk_setup()

The error handling was a bit messy and buggy. It freed "pll_clock" then dereferenced it, and then freed it again. I've re-written it in normal kernel style.

Fixes 42ff8e8008c8 ('h8300: clock driver')
drivers/clk/h8300/clk-h8s2678.c +16 -17
index 4de7ee5..4701b093 100644
@@ -107,13 +107,13 @@ static void __init h8s2678_pll_clk_setup(struct device_node *node)
 	pll_clock->sckcr = of_iomap(node, 0);
 	if (pll_clock->sckcr = NULL) {
 		pr_err("%s: failed to map divide register", clk_name);
-		goto error;
+		goto free_clock;
 	}
 
 	pll_clock->pllcr = of_iomap(node, 1);
 	if (pll_clock->pllcr = NULL) {
 		pr_err("%s: failed to map multiply register", clk_name);
-		goto error;
+		goto unmap_sckcr;
 	}
 
 	parent_name = of_clk_get_parent_name(node, 0);
@@ -125,22 +125,21 @@ static void __init h8s2678_pll_clk_setup(struct device_node *node)
 	pll_clock->hw.init = &init;
 
 	clk = clk_register(NULL, &pll_clock->hw);
-	if (IS_ERR(clk))
-		kfree(pll_clock);
-	if (!IS_ERR(clk)) {
-		of_clk_add_provider(node, of_clk_src_simple_get, clk);
-		return;
-	}
-	pr_err("%s: failed to register %s div clock (%ld)\n",
-	       __func__, clk_name, PTR_ERR(clk));
-error:
-	if (pll_clock) {
-		if (pll_clock->sckcr)
-			iounmap(pll_clock->sckcr);
-		if (pll_clock->pllcr)
-			iounmap(pll_clock->pllcr);
-		kfree(pll_clock);
+	if (IS_ERR(clk)) {
+		pr_err("%s: failed to register %s div clock (%ld)\n",
+		       __func__, clk_name, PTR_ERR(clk));
+		goto unmap_pllcr;
 	}
+
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+	return;
+
+unmap_pllcr:
+	iounmap(pll_clock->pllcr);
+unmap_sckcr:
+	iounmap(pll_clock->sckcr);
+free_clock:
+	kfree(pll_clock);
 }
 
 CLK_OF_DECLARE(h8s2678_div_clk, "renesas,h8s2678-pll-clock",
#1 lore raw

At Thu, 14 May 2015 13:05:00 +0300,

Quoted 55 lines · Dan Carpenter

The error handling was a bit messy and buggy. It freed "pll_clock" then dereferenced it, and then freed it again. I've re-written it in normal kernel style.

Fixes 42ff8e8008c8 ('h8300: clock driver')
drivers/clk/h8300/clk-h8s2678.c +9 -17
index 4de7ee5..4701b093 100644
@@ -107,13 +107,13 @@ static void __init h8s2678_pll_clk_setup(struct device_node *node)
 	pll_clock->sckcr = of_iomap(node, 0);
 	if (pll_clock->sckcr = NULL) {
 		pr_err("%s: failed to map divide register", clk_name);
-		goto error;
+		goto free_clock;
 	}
 
 	pll_clock->pllcr = of_iomap(node, 1);
 	if (pll_clock->pllcr = NULL) {
 		pr_err("%s: failed to map multiply register", clk_name);
-		goto error;
+		goto unmap_sckcr;
 	}
 
 	parent_name = of_clk_get_parent_name(node, 0);
@@ -125,22 +125,21 @@ static void __init h8s2678_pll_clk_setup(struct device_node *node)
 	pll_clock->hw.init = &init;
 
 	clk = clk_register(NULL, &pll_clock->hw);
-	if (IS_ERR(clk))
-		kfree(pll_clock);
-	if (!IS_ERR(clk)) {
-		of_clk_add_provider(node, of_clk_src_simple_get, clk);
-		return;
-	}
-	pr_err("%s: failed to register %s div clock (%ld)\n",
-	       __func__, clk_name, PTR_ERR(clk));
-error:
-	if (pll_clock) {
-		if (pll_clock->sckcr)
-			iounmap(pll_clock->sckcr);
-		if (pll_clock->pllcr)
-			iounmap(pll_clock->pllcr);
-		kfree(pll_clock);
+	if (IS_ERR(clk)) {
+		pr_err("%s: failed to register %s div clock (%ld)\n",
+		       __func__, clk_name, PTR_ERR(clk));
+		goto unmap_pllcr;
 	}
+
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+	return;

drivers/clk/h8300/clk-h8s2678.c static void __init h8s2678_pll_clk_setup(struct device_node *node) · line 145

Quoted 10 lines
drivers/clk/h8300/clk-h8s2678.c +7 -0
+
+unmap_pllcr:
+	iounmap(pll_clock->pllcr);
+unmap_sckcr:
+	iounmap(pll_clock->sckcr);
+free_clock:
+	kfree(pll_clock);
 }
 
 CLK_OF_DECLARE(h8s2678_div_clk, "renesas,h8s2678-pll-clock",

Looks good.
Applied in my tree.
Thanks.

Signature · 2 lines
Yoshinori Sato
<[email protected]>