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

["Liang He" <windhl@126.com>]

At 2022-06-22 14:56:32, "Geert Uytterhoeven" <geert@linux-m68k.org> wrote:
>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...
>

Hi, this is also strange to me.

In fact, I also don't like add a special of_node_get() before I call of_find_xx() each time.

In fact, I have learned this error-pattern based on the following bug-fix:

https://lore.kernel.org/all/20200720152806.443262648@linuxfoundation.org/


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

Thanks.

>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);
>


Sorry, Geert.

As I don't know anyother details of the code except the error-pattern code,
I cannot do anyother (more efficient) changes.

If it is a bug, please fix it in your professional way if my patch is not so well.

Thanks.

Liang.

>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