Re: [PATCH] mips/kernel: Add missing of_node_get()

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Liang,

CC devicetree

On Wed, Jun 22, 2022 at 8:08 AM Liang He <windhl@126.com> wrote:
> In mips_cpc_default_phys_base(), we need to add of_node_get() before
> of_find_compatible_node() which will decrease the refcount of its
> first arg.
>
> Signed-off-by: Liang He <windhl@126.com>

Thanks for your patch!

> --- a/arch/mips/kernel/mips-cpc.c
> +++ b/arch/mips/kernel/mips-cpc.c
> @@ -25,6 +25,7 @@ phys_addr_t __weak mips_cpc_default_phys_base(void)
>         struct resource res;
>         int err;
>
> +       of_node_get(of_root);

Adding this looks strange to me...

However, of_find_compatible_node() indeed calls of_node_put(from),
so your patch is correct.

However, when passed NULL as the from pointer, __of_find_all_nodes()
(expanded from for_each_of_allnodes_from()) turns this into of_root.
As of_find_compatible_node() still has the original (NULL) from
pointer, of_node_put(from) becomes a no-op.

>         cpc_node = of_find_compatible_node(of_root, NULL, "mti,mips-cpc");

Hence I think it would be better to change the above to

    cpc_node = of_find_compatible_node(NULL, NULL, "mti,mips-cpc");

instead, i.e. get rid of the explicit of_root handling?

>         if (cpc_node) {
>                 err = of_address_to_resource(cpc_node, 0, &res);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds