Re: [RFC PATCH 1/4] clk: clk-conf: properly release of nodes

[Stephen Boyd <sboyd@kernel.org>]

Quoting Nuno Sá (2022-04-07 06:30:33)
> 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.
> 
> Fixes: 86be408bfbd8 ("clk: Support for clock parents and rates assigned from device tree")
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> (cherry picked from commit 69eb47a26e7f728a5c052687380993cd9a0dd643)

This line should be removed.

> ---
>  drivers/clk/clk-conf.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
> index 2ef819606c41..d6065cdf1540 100644
> --- a/drivers/clk/clk-conf.c
> +++ b/drivers/clk/clk-conf.c
> @@ -33,9 +33,12 @@ static int __set_clk_parents(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;
> +               }
>                 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",

I suspect it would be easier to follow and fix if the code was
reorganized to have most of the contents inside this function as a
sub-routine that is called for each index. Then all the node putting and
clk putting can be in one place in that other function and this is a
simple loop around that stops on the first error.

> @@ -49,7 +52,7 @@ static int __set_clk_parents(struct device_node *node, bool clk_supplier)
>                         goto err;
>                 if (clkspec.np == node && !clk_supplier) {
>                         rc = 0;
> -                       goto err;
> +                       goto err_of_put;
>                 }
>                 clk = of_clk_get_from_provider(&clkspec);
>                 if (IS_ERR(clk)) {