At 2022-06-17 16:27:03, conor.doo...@microchip.com wrote: >On 17/06/2022 09:17, Liang He wrote: >> >> >> >> At 2022-06-17 14:53:13, "Christophe Leroy" <christophe.le...@csgroup.eu> >> wrote: >>> >>> >>> Le 17/06/2022 à 08:45, Liang He a écrit : >>>> >>>> >>>> >>>> At 2022-06-17 14:28:56, "Christophe Leroy" <christophe.le...@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 <win...@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. > >idk anything about powerpc, but what I find is a nice way to get a compiler >for an arch I don't use is to search on lore.kernel.org for a 0day robot >build error since it gives instructions for building on that arch. >For example: >https://lore.kernel.org/linuxppc-dev/202206060910.ryntfqdi-...@intel.com/ > > >In this case, your bug seems obvious? You typed "gotot" instead of "goto". > >Hope that helps, >Conor. > >> >> Thanks again, Christophe and Conor. >> >> Liang
Thanks so much, I will try it.