* [PATCH] mips/kernel: Add missing of_node_get()
@ 2022-06-22 6:08 Liang He
2022-06-22 6:56 ` Geert Uytterhoeven
0 siblings, 1 reply; 3+ messages in thread
From: Liang He @ 2022-06-22 6:08 UTC (permalink / raw)
To: tsbogend, geert+renesas, linux-mips, windhl
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>
---
arch/mips/kernel/mips-cpc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/mips/kernel/mips-cpc.c b/arch/mips/kernel/mips-cpc.c
index 3e386f7e1545..7ba0ae5df882 100644
--- 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);
cpc_node = of_find_compatible_node(of_root, NULL, "mti,mips-cpc");
if (cpc_node) {
err = of_address_to_resource(cpc_node, 0, &res);
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mips/kernel: Add missing of_node_get()
2022-06-22 6:08 [PATCH] mips/kernel: Add missing of_node_get() Liang He
@ 2022-06-22 6:56 ` Geert Uytterhoeven
2022-06-22 7:12 ` Liang He
0 siblings, 1 reply; 3+ messages in thread
From: Geert Uytterhoeven @ 2022-06-22 6:56 UTC (permalink / raw)
To: Liang He
Cc: Thomas Bogendoerfer, open list:BROADCOM NVRAM DRIVER,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re:Re: [PATCH] mips/kernel: Add missing of_node_get()
2022-06-22 6:56 ` Geert Uytterhoeven
@ 2022-06-22 7:12 ` Liang He
0 siblings, 0 replies; 3+ messages in thread
From: Liang He @ 2022-06-22 7:12 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Thomas Bogendoerfer, open list:BROADCOM NVRAM DRIVER,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-06-22 7:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-22 6:08 [PATCH] mips/kernel: Add missing of_node_get() Liang He
2022-06-22 6:56 ` Geert Uytterhoeven
2022-06-22 7:12 ` Liang He
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.