| Message ID | 20180822090319.29167-1-johan@kernel.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | clk: ti: fix OF child-node lookup | expand |
Quoting Johan Hovold (2018-08-22 02:03:19) > 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.
On Tue, Aug 28, 2018 at 03:32:03PM -0700, Stephen Boyd wrote: > Quoting Johan Hovold (2018-08-22 02:03:19) > > 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
On 29/08/18 10:50, Johan Hovold wrote: > On Tue, Aug 28, 2018 at 03:32:03PM -0700, Stephen Boyd wrote: >> Quoting Johan Hovold (2018-08-22 02:03:19) >>> 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. 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. Acked-by: Tero Kristo <t-kristo@ti.com> -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Quoting Johan Hovold (2018-08-22 02:03:19) > 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") > Signed-off-by: Johan Hovold <johan@kernel.org> > --- Applied to clk-next
On Thu, Aug 30, 2018 at 10:28:28AM +0300, Tero Kristo wrote: > On 29/08/18 10:50, Johan Hovold wrote: > > On Tue, Aug 28, 2018 at 03:32:03PM -0700, Stephen Boyd wrote: > >> Quoting Johan Hovold (2018-08-22 02:03:19) > >>> 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. > > 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. > > Acked-by: Tero Kristo <t-kristo@ti.com> Thanks for reviewing. Johan
On 31/08/18 00:46, Stephen Boyd wrote: > Quoting Johan Hovold (2018-08-22 02:03:19) >> 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") >> Signed-off-by: Johan Hovold <johan@kernel.org> >> --- > > 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
Quoting Tero Kristo (2018-09-20 00:11:08) > On 31/08/18 00:46, Stephen Boyd wrote: > > Quoting Johan Hovold (2018-08-22 02:03:19) > >> 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") > >> Signed-off-by: Johan Hovold <johan@kernel.org> > >> --- > > > > 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. > clk-ti-of-node is immutable. You can base on top of that and send a PR. Does that work for you?
On 28/09/18 18:57, Stephen Boyd wrote: > Quoting Tero Kristo (2018-09-20 00:11:08) >> On 31/08/18 00:46, Stephen Boyd wrote: >>> Quoting Johan Hovold (2018-08-22 02:03:19) >>>> 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") >>>> Signed-off-by: Johan Hovold <johan@kernel.org> >>>> --- >>> >>> 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. >> > > 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
diff --git a/drivers/clk/ti/clk.c b/drivers/clk/ti/clk.c index 7d22e1af2247..27e0979b3158 100644 --- a/drivers/clk/ti/clk.c +++ b/drivers/clk/ti/clk.c @@ -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);
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") Signed-off-by: Johan Hovold <johan@kernel.org> --- drivers/clk/ti/clk.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)