Skip to contentSkip to conversation

clk: ti: fix OF child-node lookup

PATCH Applied Johan Hovold 3 participants 9 messages Aug 22, 2018 – Oct 1, 2018

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

PATCH 1/1 clk: ti: fix OF child-node lookup

Fix child-node lookup which by using the wrong OF helper was searching the whole tree depth-first, something which could end up matching an unrelated node.

Also fix the related node-reference leaks.

Fixes 5b385a45e001 ("clk: ti: add support for clkctrl aliases")
---
 drivers/clk/ti/clk.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
drivers/clk/ti/clk.c +9 -4
index 7d22e1af2247..27e0979b3158 100644
@@ -129,7 +129,7 @@ int ti_clk_setup_ll_ops(struct ti_clk_ll_ops *ops)
 void __init ti_dt_clocks_register(struct ti_dt_clk oclks[])
 {
 	struct ti_dt_clk *c;
-	struct device_node *node;
+	struct device_node *node, *parent;
 	struct clk *clk;
 	struct of_phandle_args clkspec;
 	char buf[64];
@@ -164,8 +164,12 @@ void __init ti_dt_clocks_register(struct ti_dt_clk oclks[])
 			continue;
 
 		node = of_find_node_by_name(NULL, buf);
-		if (num_args)
-			node = of_find_node_by_name(node, "clk");
+		if (num_args) {
+			parent = node;
+			node = of_get_child_by_name(parent, "clk");
+			of_node_put(parent);
+		}
+
 		clkspec.np = node;
 		clkspec.args_count = num_args;
 		for (i = 0; i < num_args; i++) {
@@ -173,11 +177,12 @@ void __init ti_dt_clocks_register(struct ti_dt_clk oclks[])
 			if (ret) {
 				pr_warn("Bad tag in %s at %d: %s\n",
 					c->node_name, i, tags[i]);
+				of_node_put(node);
 				return;
 			}
 		}
 		clk = of_clk_get_from_provider(&clkspec);
-
+		of_node_put(node);
 		if (!IS_ERR(clk)) {
 			c->lk.clk = clk;
 			clkdev_add(&c->lk);
Signature · 1 line
2.18.0
maintainer
#1 lore raw

Quoting Johan Hovold (2018-08-22 02:03:19)

Quoted 9 lines

Fix child-node lookup which by using the wrong OF helper was searching the whole tree depth-first, something which could end up matching an unrelated node.
=

Also fix the related node-reference leaks.
=

Fixes 5b385a45e001 ("clk: ti: add support for clkctrl aliases")

Found by inspection? Or it's actively causing problems? I'm thinking it's the former so this can bake in clk-next for a while.

#2 lore raw
Quoted 11 lines · Stephen Boyd

Quoting Johan Hovold (2018-08-22 02:03:19)

Quoted 7 lines

Fix child-node lookup which by using the wrong OF helper was searching the whole tree depth-first, something which could end up matching an unrelated node.

Also fix the related node-reference leaks.

Fixes 5b385a45e001 ("clk: ti: add support for clkctrl aliases")

Found by inspection? Or it's actively causing problems? I'm thinking it's the former so this can bake in clk-next for a while.

Right, through inspection. I fixed up most of these last year, but this one managed to sneak in since then.

clk-next should be fine.

Thanks,
Johan

Acked-by
#3 lore raw
Quoted 10 lines · Johan Hovold
Quoted 9 lines · Stephen Boyd

Quoting Johan Hovold (2018-08-22 02:03:19)

Quoted 7 lines

Fix child-node lookup which by using the wrong OF helper was searching the whole tree depth-first, something which could end up matching an unrelated node.

Also fix the related node-reference leaks.

Fixes 5b385a45e001 ("clk: ti: add support for clkctrl aliases")
Quoted 7 lines
Quoted 2 lines

Found by inspection? Or it's actively causing problems? I'm thinking it's the former so this can bake in clk-next for a while.

Right, through inspection. I fixed up most of these last year, but this one managed to sneak in since then.

clk-next should be fine.

The patch looks fine to me also, just had to test this out with my latest development branch as it conflicts with that one a bit.

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

#4 lore raw
Quoted 17 lines · Tero Kristo
Quoted 16 lines · Johan Hovold
Quoted 11 lines · Stephen Boyd

Quoting Johan Hovold (2018-08-22 02:03:19)

Quoted 7 lines

Fix child-node lookup which by using the wrong OF helper was searching the whole tree depth-first, something which could end up matching an unrelated node.

Also fix the related node-reference leaks.

Fixes 5b385a45e001 ("clk: ti: add support for clkctrl aliases")

Found by inspection? Or it's actively causing problems? I'm thinking it's the former so this can bake in clk-next for a while.

Right, through inspection. I fixed up most of these last year, but this one managed to sneak in since then.

Quoted 6 lines
Quoted 1 line

clk-next should be fine.

The patch looks fine to me also, just had to test this out with my latest development branch as it conflicts with that one a bit.

Thanks for reviewing.

Johan

maintainer Applied
#5 lore raw
In reply to Johan Hovold

Quoting Johan Hovold (2018-08-22 02:03:19)

Quoted 11 lines

Fix child-node lookup which by using the wrong OF helper was searching the whole tree depth-first, something which could end up matching an unrelated node.
=

Also fix the related node-reference leaks.
=

Fixes 5b385a45e001 ("clk: ti: add support for clkctrl aliases")

---

Applied to clk-next

#6 lore raw
Quoted 5 lines · Stephen Boyd

Quoting Johan Hovold (2018-08-22 02:03:19)

Quoted 4 lines

Fix child-node lookup which by using the wrong OF helper was searching the whole tree depth-first, something which could end up matching an unrelated node.

Quoted 8 lines
Quoted 5 lines

Also fix the related node-reference leaks.

Fixes 5b385a45e001 ("clk: ti: add support for clkctrl aliases")

---

Applied to clk-next

Stephen, this patch causes a merge conflict with some of the code I am planning to send a pull-req against (the clkctrl work series: https://patchwork.kernel.org/project/linux-clk/list/?series=13841)

Would it be possible to drop this from clk-next, and for me to pick-it-up via my own branch? I can get this setup either today or tomorrow.

Alternatively I need an immutable branch against which I can work.

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

maintainer
#7 lore raw

Quoting Tero Kristo (2018-09-20 00:11:08)

Quoted 15 lines · Stephen Boyd
Quoted 5 lines

Quoting Johan Hovold (2018-08-22 02:03:19)

Quoted 4 lines

Fix child-node lookup which by using the wrong OF helper was searching the whole tree depth-first, something which could end up matching an unrelated node.

Quoted 8 lines
Quoted 5 lines

Also fix the related node-reference leaks.

Fixes 5b385a45e001 ("clk: ti: add support for clkctrl aliases")

---

Applied to clk-next

Quoted 9 lines

Stephen, this patch causes a merge conflict with some of the code I am planning to send a pull-req against (the clkctrl work series:
https://patchwork.kernel.org/project/linux-clk/list/?series=13841)

Would it be possible to drop this from clk-next, and for me to pick-it-up via my own branch? I can get this setup either today or tomorrow.

Alternatively I need an immutable branch against which I can work.

clk-ti-of-node is immutable. You can base on top of that and send a PR. Does that work for you?

#8 lore raw
Quoted 20 lines · Stephen Boyd

Quoting Tero Kristo (2018-09-20 00:11:08)

Quoted 19 lines · Stephen Boyd
Quoted 5 lines

Quoting Johan Hovold (2018-08-22 02:03:19)

Quoted 4 lines

Fix child-node lookup which by using the wrong OF helper was searching the whole tree depth-first, something which could end up matching an unrelated node.

Quoted 8 lines
Quoted 5 lines

Also fix the related node-reference leaks.

Fixes 5b385a45e001 ("clk: ti: add support for clkctrl aliases")

---

Applied to clk-next

Stephen, this patch causes a merge conflict with some of the code I am planning to send a pull-req against (the clkctrl work series:
https://patchwork.kernel.org/project/linux-clk/list/?series=13841)

Quoted 8 lines
Quoted 5 lines

Would it be possible to drop this from clk-next, and for me to pick-it-up via my own branch? I can get this setup either today or tomorrow.

Alternatively I need an immutable branch against which I can work.

clk-ti-of-node is immutable. You can base on top of that and send a PR. Does that work for you?

That should work just fine for me.

I'll work on top of this and send a pull-req, thanks.

-Tero

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki