All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nuno Sá" <nuno.sa@analog.com>
To: <linux-clk@vger.kernel.org>
Cc: Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>
Subject: [RESEND RFC PATCH v2 1/4] clk: clk-conf: properly release of nodes
Date: Thu, 23 Jun 2022 14:18:54 +0200	[thread overview]
Message-ID: <20220623121857.886-2-nuno.sa@analog.com> (raw)
In-Reply-To: <20220623121857.886-1-nuno.sa@analog.com>

We need to call 'of_node_put()' when we are done with the node or on
error paths. Otherwise this can leak memory in DYNAMIC_OF setups.

In order to make things easier to follow, an helper function was added
to set each parent in it's own function.

Fixes: 86be408bfbd8 ("clk: Support for clock parents and rates assigned from device tree")
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/clk/clk-conf.c | 126 +++++++++++++++++++++++++----------------
 1 file changed, 78 insertions(+), 48 deletions(-)

diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
index 2ef819606c41..b8ea4f93f35e 100644
--- a/drivers/clk/clk-conf.c
+++ b/drivers/clk/clk-conf.c
@@ -11,11 +11,78 @@
 #include <linux/of.h>
 #include <linux/printk.h>
 
-static int __set_clk_parents(struct device_node *node, bool clk_supplier)
+static int __set_clk_parent(struct device_node *node, bool clk_supplier,
+			    int index, bool *stop)
 {
 	struct of_phandle_args clkspec;
-	int index, rc, num_parents;
 	struct clk *clk, *pclk;
+	int rc;
+
+	rc = of_parse_phandle_with_args(node, "assigned-clock-parents",
+					"#clock-cells",	index, &clkspec);
+	if (rc) {
+		/* skip empty (null) phandles */
+		if (rc == -ENOENT)
+			return 0;
+
+		return rc;
+	}
+
+	if (clkspec.np == node && !clk_supplier) {
+		*stop = true;
+		goto out_of_put;
+	}
+
+	pclk = of_clk_get_from_provider(&clkspec);
+	of_node_put(clkspec.np);
+	if (IS_ERR(pclk)) {
+		if (PTR_ERR(pclk) != -EPROBE_DEFER)
+			pr_warn("clk: couldn't get parent clock %d for %pOF\n",
+				index, node);
+
+		return PTR_ERR(pclk);
+	}
+
+	rc = of_parse_phandle_with_args(node, "assigned-clocks",
+					"#clock-cells", index, &clkspec);
+	if (rc) {
+		clk_put(pclk);
+		return rc;
+	}
+
+	if (clkspec.np == node && !clk_supplier) {
+		*stop = true;
+		goto out_clk_put;
+	}
+
+	clk = of_clk_get_from_provider(&clkspec);
+	if (IS_ERR(clk)) {
+		if (PTR_ERR(clk) != -EPROBE_DEFER)
+			pr_warn("clk: couldn't get assigned clock %d for %pOF\n",
+				index, node);
+
+		rc = PTR_ERR(clk);
+		goto out_clk_put;
+	}
+
+	rc = clk_set_parent(clk, pclk);
+	if (rc)
+		pr_err("clk: failed to reparent %s to %s: %d\n",
+		       __clk_get_name(clk), __clk_get_name(pclk), rc);
+
+	clk_put(clk);
+
+out_clk_put:
+	clk_put(pclk);
+out_of_put:
+	of_node_put(clkspec.np);
+	return rc;
+}
+
+static int __set_clk_parents(struct device_node *node, bool clk_supplier)
+{
+	int index, rc, num_parents;
+	bool stop = false;
 
 	num_parents = of_count_phandle_with_args(node, "assigned-clock-parents",
 						 "#clock-cells");
@@ -24,53 +91,12 @@ static int __set_clk_parents(struct device_node *node, bool clk_supplier)
 		       node);
 
 	for (index = 0; index < num_parents; index++) {
-		rc = of_parse_phandle_with_args(node, "assigned-clock-parents",
-					"#clock-cells",	index, &clkspec);
-		if (rc < 0) {
-			/* skip empty (null) phandles */
-			if (rc == -ENOENT)
-				continue;
-			else
-				return rc;
-		}
-		if (clkspec.np == node && !clk_supplier)
-			return 0;
-		pclk = of_clk_get_from_provider(&clkspec);
-		if (IS_ERR(pclk)) {
-			if (PTR_ERR(pclk) != -EPROBE_DEFER)
-				pr_warn("clk: couldn't get parent clock %d for %pOF\n",
-					index, node);
-			return PTR_ERR(pclk);
-		}
-
-		rc = of_parse_phandle_with_args(node, "assigned-clocks",
-					"#clock-cells", index, &clkspec);
-		if (rc < 0)
-			goto err;
-		if (clkspec.np == node && !clk_supplier) {
-			rc = 0;
-			goto err;
-		}
-		clk = of_clk_get_from_provider(&clkspec);
-		if (IS_ERR(clk)) {
-			if (PTR_ERR(clk) != -EPROBE_DEFER)
-				pr_warn("clk: couldn't get assigned clock %d for %pOF\n",
-					index, node);
-			rc = PTR_ERR(clk);
-			goto err;
-		}
-
-		rc = clk_set_parent(clk, pclk);
-		if (rc < 0)
-			pr_err("clk: failed to reparent %s to %s: %d\n",
-			       __clk_get_name(clk), __clk_get_name(pclk), rc);
-		clk_put(clk);
-		clk_put(pclk);
+		rc = __set_clk_parent(node, clk_supplier, index, &stop);
+		if (rc || stop)
+			return rc;
 	}
+
 	return 0;
-err:
-	clk_put(pclk);
-	return rc;
 }
 
 static int __set_clk_rates(struct device_node *node, bool clk_supplier)
@@ -93,14 +119,17 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier)
 				else
 					return rc;
 			}
-			if (clkspec.np == node && !clk_supplier)
+			if (clkspec.np == node && !clk_supplier) {
+				of_node_put(clkspec.np);
 				return 0;
+			}
 
 			clk = of_clk_get_from_provider(&clkspec);
 			if (IS_ERR(clk)) {
 				if (PTR_ERR(clk) != -EPROBE_DEFER)
 					pr_warn("clk: couldn't get clock %d for %pOF\n",
 						index, node);
+				of_node_put(clkspec.np);
 				return PTR_ERR(clk);
 			}
 
@@ -110,6 +139,7 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier)
 				       __clk_get_name(clk), rate, rc,
 				       clk_get_rate(clk));
 			clk_put(clk);
+			of_node_put(clkspec.np);
 		}
 		index++;
 	}
-- 
2.17.1


  reply	other threads:[~2022-06-23 12:19 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-23 12:18 [RESEND RFC PATCH v2 0/4] Dynamic OF and use after free related fixes Nuno Sá
2022-06-23 12:18 ` Nuno Sá [this message]
2022-06-23 12:18 ` [RESEND RFC PATCH v2 2/4] clk: fix clk not being unlinked from consumers list Nuno Sá
2022-06-23 12:18 ` [RESEND RFC PATCH v2 3/4] clk: refcount the active parent clk_core Nuno Sá
2022-06-23 12:18 ` [RESEND RFC PATCH v2 4/4] clk: use clk_core_unlink_consumer() helper Nuno Sá
2022-07-13 13:24 ` [RESEND RFC PATCH v2 0/4] Dynamic OF and use after free related fixes Nuno Sá
2022-09-12  7:12   ` Nuno Sá

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220623121857.886-2-nuno.sa@analog.com \
    --to=nuno.sa@analog.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.