Re:Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
在 2022-06-17 18:57:36，"Christophe Leroy" 写道：
>
>
>Le 17/06/2022 à 12:47, Liang He a écrit :
>>
>>
>>
>> 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"
wrote:
>
>
> Le 17/06/2022 à 08:45, Liang He a écrit :
>>
>>
>>
>> At 2022-06-17 14:28:56, "Christophe Leroy"
>> 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
---
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, );
- if (!gpio_is_valid(gpio))
- return -EINVAL;
+ gpio = of_get_gpio_flags(child_node, 0, );
+ 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, 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
Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
Le 17/06/2022 à 12:47, Liang He a écrit :
>
>
>
> 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"
>>> wrote:
Le 17/06/2022 à 08:45, Liang He a écrit :
>
>
>
> At 2022-06-17 14:28:56, "Christophe Leroy"
> 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
>>> ---
>>> 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, );
>>> - if (!gpio_is_valid(gpio))
>>> - return -EINVAL;
>>> + gpio = of_get_gpio_flags(child_node, 0, );
>>> + 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, 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
Re:Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
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"
>> wrote:
>>>
>>>
>>> Le 17/06/2022 à 08:45, Liang He a écrit :
At 2022-06-17 14:28:56, "Christophe Leroy"
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
>> ---
>> 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, );
>> -if (!gpio_is_valid(gpio))
>> -return -EINVAL;
>> +gpio = of_get_gpio_flags(child_node, 0, );
>> +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, 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=./=
Re:Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
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"
>> wrote:
>>>
>>>
>>> Le 17/06/2022 à 08:45, Liang He a écrit :
At 2022-06-17 14:28:56, "Christophe Leroy"
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
>> ---
>> 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, );
>> -if (!gpio_is_valid(gpio))
>> -return -EINVAL;
>> +gpio = of_get_gpio_flags(child_node, 0, );
>> +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.
Re:Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
At 2022-06-17 14:53:13, "Christophe Leroy" wrote:
>
>
>Le 17/06/2022 à 08:45, Liang He a écrit :
>>
>>
>>
>> At 2022-06-17 14:28:56, "Christophe Leroy"
>> 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
---
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, );
- if (!gpio_is_valid(gpio))
- return -EINVAL;
+ gpio = of_get_gpio_flags(child_node, 0, );
+ 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
Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
Le 17/06/2022 à 08:45, Liang He a écrit :
>
>
>
> At 2022-06-17 14:28:56, "Christophe Leroy"
> 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
>>> ---
>>>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, );
>>> - if (!gpio_is_valid(gpio))
>>> - return -EINVAL;
>>> + gpio = of_get_gpio_flags(child_node, 0, );
>>> + 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
Re:Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
At 2022-06-17 14:28:56, "Christophe Leroy" 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
>> ---
>> 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, );
>> -if (!gpio_is_valid(gpio))
>> -return -EINVAL;
>> +gpio = of_get_gpio_flags(child_node, 0, );
>> +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;
>> }
>>
Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
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
> ---
> 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, );
> - if (!gpio_is_valid(gpio))
> - return -EINVAL;
> + gpio = of_get_gpio_flags(child_node, 0, );
> + 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;
> }
>
[PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
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
---
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, );
- if (!gpio_is_valid(gpio))
- return -EINVAL;
+ gpio = of_get_gpio_flags(child_node, 0, );
+ 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;
}
--
2.25.1
