At 2022-06-22 14:56:32, "Geert Uytterhoeven" <geert@xxxxxxxxxxxxxx> wrote: >Hi Liang, > >CC devicetree > >On Wed, Jun 22, 2022 at 8:08 AM Liang He <windhl@xxxxxxx> 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@xxxxxxx> > >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@xxxxxxxxxxxxxxxxxxx/ >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@xxxxxxxxxxxxxx > >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
