Messages in this thread | | | Date | Fri, 17 Jun 2022 16:17:04 +0800 (CST) | From | "Liang He" <> | Subject | Re:Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000 |
| |
At 2022-06-17 14:53:13, "Christophe Leroy" <christophe.leroy@csgroup.eu> wrote: > > >Le 17/06/2022 à 08:45, Liang He a écrit : >> >> >> >> At 2022-06-17 14:28:56, "Christophe Leroy" <christophe.leroy@csgroup.eu> wrote: >>> >>> >>> Le 17/06/2022 à 08:08, Liang He a écrit : >>>> In gpio_halt_probe(), of_find_matching_node() will return a node >>>> pointer with refcount incremented. We should use of_node_put() in >>>> fail path or when it is not used anymore. >>>> >>>> Signed-off-by: Liang He <windhl@126.com> >>>> --- >>>> changelog: >>>> v4: reuse exist 'err' and use a simple code style, advised by CJ >>>> v3: use local 'child_node' advised by Michael. >>>> v2: use goto-label patch style advised by Christophe Leroy. >>>> v1: add of_node_put() before each exit. >>>> >>>> arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 ++++++++++++++--------- >>>> 1 file changed, 22 insertions(+), 13 deletions(-) >>>> >>>> diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c b/arch/powerpc/platforms/85xx/sgy_cts1000.c >>>> index 98ae64075193..e4588943fe7e 100644 >>>> --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c >>>> +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c >>>> @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device *pdev) >>>> { >>>> enum of_gpio_flags flags; >>>> struct device_node *node = pdev->dev.of_node; >>>> + struct device_node *child_node; >>>> int gpio, err, irq; >>>> int trigger; >>>> >>>> @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device *pdev) >>>> return -ENODEV; >>>> >>>> /* If there's no matching child, this isn't really an error */ >>>> - halt_node = of_find_matching_node(node, child_match); >>>> - if (!halt_node) >>>> + child_node = of_find_matching_node(node, child_match); >>>> + if (!child_node) >>>> return 0; >>>> >>>> /* Technically we could just read the first one, but punish >>>> * DT writers for invalid form. */ >>>> - if (of_gpio_count(halt_node) != 1) >>>> - return -EINVAL; >>>> + if (of_gpio_count(child_node) != 1) { >>>> + err = -EINVAL; >>>> + goto err_put; >>>> + } >>>> >>>> /* Get the gpio number relative to the dynamic base. */ >>>> - gpio = of_get_gpio_flags(halt_node, 0, &flags); >>>> - if (!gpio_is_valid(gpio)) >>>> - return -EINVAL; >>>> + gpio = of_get_gpio_flags(child_node, 0, &flags); >>>> + if (!gpio_is_valid(gpio)) { >>>> + err = -EINVAL; >>>> + gotot err_put; >>> >>> Did you test the build ? >> >> Sorry for this fault. >> >> In fact, I am still finding an efficient way to building different arch source code as I only have x86-64. >> >> Now I am try using QEMU. >> >> Anyway, sorry for this fault. > >You can find cross compilers for most architectures for x86-64 here : >https://mirrors.edge.kernel.org/pub/tools/crosstool/ > >Christophe
Hi, Christophe and Conor.
Sorry to trouble you again.
Now I only know how to quickly identify the refcounting bugs, but I cannot efficiently give a build test.
For example, I use the cross compilers 'powerpc-linux-gnu-gcc' to compile 'arch/powerpc/platforms/85xx/sgy_cts1000.c' with -fsyntax-only flag. But I meet too many header file missing errors. Even if I add some 'include' pathes, e.g., ./arch/powerpc/include, ./include, there are still too many other errors.
So if there is any efficient way to check my patch code to avoid 'gotot' error again.
Thanks again, Christophe and Conor.
Liang
| |