| Message ID | 20220617060827.4004795-1-windhl@126.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000 | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 10 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 10 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 7 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 23 jobs. |
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 ? > + } > > err = gpio_request(gpio, "gpio-halt"); > if (err) { > printk(KERN_ERR "gpio-halt: error requesting GPIO %d.\n", > gpio); > - halt_node = NULL; > - return err; > + goto err_put; > } > > trigger = (flags == OF_GPIO_ACTIVE_LOW); > @@ -105,15 +109,14 @@ static int gpio_halt_probe(struct platform_device *pdev) > gpio_direction_output(gpio, !trigger); > > /* Now get the IRQ which tells us when the power button is hit */ > - irq = irq_of_parse_and_map(halt_node, 0); > + irq = irq_of_parse_and_map(child_node, 0); > err = request_irq(irq, gpio_halt_irq, IRQF_TRIGGER_RISING | > - IRQF_TRIGGER_FALLING, "gpio-halt", halt_node); > + IRQF_TRIGGER_FALLING, "gpio-halt", child_node); > if (err) { > printk(KERN_ERR "gpio-halt: error requesting IRQ %d for " > "GPIO %d.\n", irq, gpio); > gpio_free(gpio); > - halt_node = NULL; > - return err; > + goto err_put; > } > > /* Register our halt function */ > @@ -123,7 +126,12 @@ static int gpio_halt_probe(struct platform_device *pdev) > printk(KERN_INFO "gpio-halt: registered GPIO %d (%d trigger, %d" > " irq).\n", gpio, trigger, irq); > > + halt_node = child_node; > return 0; > + > +err_put: > + of_node_put(child_node); > + return err; > } > > static int gpio_halt_remove(struct platform_device *pdev) > @@ -139,6 +147,7 @@ static int gpio_halt_remove(struct platform_device *pdev) > > gpio_free(gpio); > > + of_node_put(halt_node); > halt_node = NULL; > } >
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. > >> + } >> >> err = gpio_request(gpio, "gpio-halt"); >> if (err) { >> printk(KERN_ERR "gpio-halt: error requesting GPIO %d.\n", >> gpio); >> - halt_node = NULL; >> - return err; >> + goto err_put; >> } >> >> trigger = (flags == OF_GPIO_ACTIVE_LOW); >> @@ -105,15 +109,14 @@ static int gpio_halt_probe(struct platform_device *pdev) >> gpio_direction_output(gpio, !trigger); >> >> /* Now get the IRQ which tells us when the power button is hit */ >> - irq = irq_of_parse_and_map(halt_node, 0); >> + irq = irq_of_parse_and_map(child_node, 0); >> err = request_irq(irq, gpio_halt_irq, IRQF_TRIGGER_RISING | >> - IRQF_TRIGGER_FALLING, "gpio-halt", halt_node); >> + IRQF_TRIGGER_FALLING, "gpio-halt", child_node); >> if (err) { >> printk(KERN_ERR "gpio-halt: error requesting IRQ %d for " >> "GPIO %d.\n", irq, gpio); >> gpio_free(gpio); >> - halt_node = NULL; >> - return err; >> + goto err_put; >> } >> >> /* Register our halt function */ >> @@ -123,7 +126,12 @@ static int gpio_halt_probe(struct platform_device *pdev) >> printk(KERN_INFO "gpio-halt: registered GPIO %d (%d trigger, %d" >> " irq).\n", gpio, trigger, irq); >> >> + halt_node = child_node; >> return 0; >> + >> +err_put: >> + of_node_put(child_node); >> + return err; >> } >> >> static int gpio_halt_remove(struct platform_device *pdev) >> @@ -139,6 +147,7 @@ static int gpio_halt_remove(struct platform_device *pdev) >> >> gpio_free(gpio); >> >> + of_node_put(halt_node); >> halt_node = NULL; >> } >>
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
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
On 17/06/2022 09:17, Liang He wrote: > > > > 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. 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-lkp@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
At 2022-06-17 16:27:03, Conor.Dooley@microchip.com wrote: >On 17/06/2022 09:17, Liang He wrote: >> >> >> >> 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. > >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-lkp@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.
At 2022-06-17 16:27:03, Conor.Dooley@microchip.com wrote: >On 17/06/2022 09:17, Liang He wrote: >> >> >> >> 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. > >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-lkp@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, Conor and Christophe. I finally figure out an efficient way in which I can use cross-compiler to check my single patched file as follow: powerpc64le-linux-gnu-gcc -Wp,-MMD,arch/powerpc/kernel/.io.o.d -nostdinc -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -I ./arch/powerpc -fmacro-prefix-map=./= -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Werror=return-type -Wno-format-security -std=gnu11 -mcpu=powerpc -mcpu=powerpc -m32 -msoft-float -pipe -ffixed-r2 -mno-readonly-in-sdata -mno-altivec -mno-vsx -fno-asynchronous-unwind-tables -mno-string -mbig-endian -mstack-protector-guard=tls -mstack-protector-guard-reg=r2 -fno-delete-null-pointer-checks -Wno-frame-address -Wno-format-truncation -Wno-format-overflow -Wno-address-of-packed-member -O2 --param=allow-store-data-races=0 -Wframe-larger-than=1024 -fstack-protector-strong -Wimplicit-fallthrough=5 -Wno-main -Wno-unused-but-set-variable -Wno-unused-const-variable -fomit-frame-pointer -fno-stack-clash-protection -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wcast-function-type -Wno-stringop-truncation -Wno-stringop-overflow -Wno-restrict -Wno-maybe-uninitialized -Wno-alloc-size-larger-than -fno-strict-overflow -fno-stack-check -fconserve-stack -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init -Wno-packed-not-aligned -mstack-protector-guard-offset=576 -Werror -DKBUILD_MODNAME='"85xx"' -DKBUILD_MODFILE='"arch/powerpc/platforms/85xx/sgy_cts1000.c"' -fsyntax-only ./arch/powerpc/platforms/85xx/sgy_cts1000.c Thanks again. Liang
Le 17/06/2022 à 12:47, Liang He a écrit : > > > > At 2022-06-17 16:27:03, Conor.Dooley@microchip.com wrote: >> On 17/06/2022 09:17, Liang He wrote: >>> >>> >>> >>> 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. >> >> 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-lkp@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, Conor and Christophe. > > I finally figure out an efficient way in which I can use cross-compiler to check my single patched file as follow: > > powerpc64le-linux-gnu-gcc -Wp,-MMD,arch/powerpc/kernel/.io.o.d -nostdinc -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -I ./arch/powerpc -fmacro-prefix-map=./= -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Werror=return-type -Wno-format-security -std=gnu11 -mcpu=powerpc -mcpu=powerpc -m32 -msoft-float -pipe -ffixed-r2 -mno-readonly-in-sdata -mno-altivec -mno-vsx -fno-asynchronous-unwind-tables -mno-string -mbig-endian -mstack-protector-guard=tls -mstack-protector-guard-reg=r2 -fno-delete-null-pointer-checks -Wno-frame-address -Wno-format-truncation -Wno-format-overflow -Wno-address-of-packed-member -O2 --param=allow-store-data-races=0 -Wframe-larger-than=1024 -fstack-protector-strong -Wimplicit-fallthrough=5 -Wno-main -Wno-unused-but-set-variable -Wno-unused-const-variable -fomit-frame-pointer -fno-stack-clash-protection -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wcast-function-type -Wno-stringop-truncation -Wno-stringop-overflow -Wno-restrict -Wno-maybe-uninitialized -Wno-alloc-size-larger-than -fno-strict-overflow -fno-stack-check -fconserve-stack -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init -Wno-packed-not-aligned -mstack-protector-guard-offset=576 -Werror -DKBUILD_MODNAME='"85xx"' -DKBUILD_MODFILE='"arch/powerpc/platforms/85xx/sgy_cts1000.c"' -fsyntax-only ./arch/powerpc/platforms/85xx/sgy_cts1000.c > Easiest way is: - Download cross compiler and extract it somewhere on your PC and ensure it is in the PATH. - Then: run "make CROSS_COMPILE=powerpc64-linux-gnu ARCH=powerpc corenet32_smp_defconfig" modify .config to activate CONFIG_SGY_CTS1000 run "make CROSS_COMPILE=powerpc64-linux-gnu ARCH=powerpc arch/powerpc/platforms/85xx/sgy_cts1000.o"
在 2022-06-17 18:57:36，"Christophe Leroy" <christophe.leroy@csgroup.eu> 写道： > > >Le 17/06/2022 à 12:47, Liang He a écrit : >> >> >> >> At 2022-06-17 16:27:03, Conor.Dooley@microchip.com wrote: >>> On 17/06/2022 09:17, Liang He wrote: >>>> >>>> >>>> >>>> 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. >>> >>> 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-lkp@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, Conor and Christophe. >> >> I finally figure out an efficient way in which I can use cross-compiler to check my single patched file as follow: >> >> powerpc64le-linux-gnu-gcc -Wp,-MMD,arch/powerpc/kernel/.io.o.d -nostdinc -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -I ./arch/powerpc -fmacro-prefix-map=./= -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Werror=return-type -Wno-format-security -std=gnu11 -mcpu=powerpc -mcpu=powerpc -m32 -msoft-float -pipe -ffixed-r2 -mno-readonly-in-sdata -mno-altivec -mno-vsx -fno-asynchronous-unwind-tables -mno-string -mbig-endian -mstack-protector-guard=tls -mstack-protector-guard-reg=r2 -fno-delete-null-pointer-checks -Wno-frame-address -Wno-format-truncation -Wno-format-overflow -Wno-address-of-packed-member -O2 --param=allow-store-data-races=0 -Wframe-larger-than=1024 -fstack-protector-strong -Wimplicit-fallthrough=5 -Wno-main -Wno-unused-but-set-variable -Wno-unused-const-variable -fomit-frame-pointer -fno-stack-clash-protection -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wcast-function-type -Wno-stringop-truncation -Wno-stringop-overflow -Wno-restrict -Wno-maybe-uninitialized -Wno-alloc-size-larger-than -fno-strict-overflow -fno-stack-check -fconserve-stack -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init -Wno-packed-not-aligned -mstack-protector-guard-offset=576 -Werror -DKBUILD_MODNAME='"85xx"' -DKBUILD_MODFILE='"arch/powerpc/platforms/85xx/sgy_cts1000.c"' -fsyntax-only ./arch/powerpc/platforms/85xx/sgy_cts1000.c >> > >Easiest way is: >- Download cross compiler and extract it somewhere on your PC and ensure >it is in the PATH. >- Then: > >run "make CROSS_COMPILE=powerpc64-linux-gnu ARCH=powerpc >corenet32_smp_defconfig" > >modify .config to activate CONFIG_SGY_CTS1000 > >run "make CROSS_COMPILE=powerpc64-linux-gnu ARCH=powerpc >arch/powerpc/platforms/85xx/sgy_cts1000.o" > Thanks for your valuable experience, Christophe. It is great and I have used it to successfully compile my patched code without any error: " CC arch/powerpc/platforms/85xx/sgy_cts1000.o " BYW, it should be 'make CROSS_COMPILE=powerpc64-linux-gnu- ...' Thanks again! Liang
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; + } err = gpio_request(gpio, "gpio-halt"); if (err) { printk(KERN_ERR "gpio-halt: error requesting GPIO %d.\n", gpio); - halt_node = NULL; - return err; + goto err_put; } trigger = (flags == OF_GPIO_ACTIVE_LOW); @@ -105,15 +109,14 @@ static int gpio_halt_probe(struct platform_device *pdev) gpio_direction_output(gpio, !trigger); /* Now get the IRQ which tells us when the power button is hit */ - irq = irq_of_parse_and_map(halt_node, 0); + irq = irq_of_parse_and_map(child_node, 0); err = request_irq(irq, gpio_halt_irq, IRQF_TRIGGER_RISING | - IRQF_TRIGGER_FALLING, "gpio-halt", halt_node); + IRQF_TRIGGER_FALLING, "gpio-halt", child_node); if (err) { printk(KERN_ERR "gpio-halt: error requesting IRQ %d for " "GPIO %d.\n", irq, gpio); gpio_free(gpio); - halt_node = NULL; - return err; + goto err_put; } /* Register our halt function */ @@ -123,7 +126,12 @@ static int gpio_halt_probe(struct platform_device *pdev) printk(KERN_INFO "gpio-halt: registered GPIO %d (%d trigger, %d" " irq).\n", gpio, trigger, irq); + halt_node = child_node; return 0; + +err_put: + of_node_put(child_node); + return err; } static int gpio_halt_remove(struct platform_device *pdev) @@ -139,6 +147,7 @@ static int gpio_halt_remove(struct platform_device *pdev) gpio_free(gpio); + of_node_put(halt_node); halt_node = NULL; }
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(-)