* [PATCH] drivers: hwmon: Add missing of_node_put() in gsc-hwmon.c
@ 2022-06-15 15:18 Liang He
2022-06-15 17:57 ` Guenter Roeck
0 siblings, 1 reply; 4+ messages in thread
From: Liang He @ 2022-06-15 15:18 UTC (permalink / raw)
To: tharvey, rjones, jdelvare, linux; +Cc: linux-hwmon, linux-kernel, windhl
In gsc_hwmon_get_devtree_pdata(), of_find_compatible_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>
---
drivers/hwmon/gsc-hwmon.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/gsc-hwmon.c b/drivers/hwmon/gsc-hwmon.c
index 1fe37418ff46..34c20d13627a 100644
--- a/drivers/hwmon/gsc-hwmon.c
+++ b/drivers/hwmon/gsc-hwmon.c
@@ -268,10 +268,14 @@ gsc_hwmon_get_devtree_pdata(struct device *dev)
/* fan controller base address */
fan = of_find_compatible_node(dev->parent->of_node, NULL, "gw,gsc-fan");
- if (fan && of_property_read_u32(fan, "reg", &pdata->fan_base)) {
+ if (fan && of_property_read_u32(fan, "reg", &pdata->fan_base)) {
+ of_node_put(fan);
dev_err(dev, "fan node without base\n");
return ERR_PTR(-EINVAL);
}
+
+ /* if fan&&!of_property_read_u32 fail */
+ of_node_put(fan);
/* allocate structures for channels and count instances of each type */
device_for_each_child_node(dev, child) {
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drivers: hwmon: Add missing of_node_put() in gsc-hwmon.c
2022-06-15 15:18 [PATCH] drivers: hwmon: Add missing of_node_put() in gsc-hwmon.c Liang He
@ 2022-06-15 17:57 ` Guenter Roeck
2022-06-16 2:37 ` 和亮
0 siblings, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2022-06-15 17:57 UTC (permalink / raw)
To: Liang He, tharvey, rjones, jdelvare; +Cc: linux-hwmon, linux-kernel
On 6/15/22 08:18, Liang He wrote:
> In gsc_hwmon_get_devtree_pdata(), of_find_compatible_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>
> ---
Please use proper subject lines. Here it should have been
hwmon: (gsc-hwmon) Add missing of_node_put()
> drivers/hwmon/gsc-hwmon.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/gsc-hwmon.c b/drivers/hwmon/gsc-hwmon.c
> index 1fe37418ff46..34c20d13627a 100644
> --- a/drivers/hwmon/gsc-hwmon.c
> +++ b/drivers/hwmon/gsc-hwmon.c
> @@ -268,10 +268,14 @@ gsc_hwmon_get_devtree_pdata(struct device *dev)
>
> /* fan controller base address */
> fan = of_find_compatible_node(dev->parent->of_node, NULL, "gw,gsc-fan");
A single of_node_put(fan) here would have been be sufficient.
> - if (fan && of_property_read_u32(fan, "reg", &pdata->fan_base)) {
> + if (fan && of_property_read_u32(fan, "reg", &pdata->fan_base)) {
> + of_node_put(fan);
> dev_err(dev, "fan node without base\n");
> return ERR_PTR(-EINVAL);
> }
> +
> + /* if fan&&!of_property_read_u32 fail */
This comment only adds confusion and does not add any value.
Guenter
> + of_node_put(fan);
>
> /* allocate structures for channels and count instances of each type */
> device_for_each_child_node(dev, child) {
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re:Re: [PATCH] drivers: hwmon: Add missing of_node_put() in gsc-hwmon.c
2022-06-15 17:57 ` Guenter Roeck
@ 2022-06-16 2:37 ` 和亮
2022-06-16 3:05 ` Guenter Roeck
0 siblings, 1 reply; 4+ messages in thread
From: 和亮 @ 2022-06-16 2:37 UTC (permalink / raw)
To: Guenter Roeck; +Cc: tharvey, rjones, jdelvare, linux-hwmon, linux-kernel
At 2022-06-16 01:57:49, "Guenter Roeck" <linux@roeck-us.net> wrote:
>
>Please use proper subject lines. Here it should have been
>
>hwmon: (gsc-hwmon) Add missing of_node_put()
Thanks, I will change it in my new patch.
>>> drivers/hwmon/gsc-hwmon.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hwmon/gsc-hwmon.c b/drivers/hwmon/gsc-hwmon.c
>> index 1fe37418ff46..34c20d13627a 100644
>> --- a/drivers/hwmon/gsc-hwmon.c
>> +++ b/drivers/hwmon/gsc-hwmon.c
>> @@ -268,10 +268,14 @@ gsc_hwmon_get_devtree_pdata(struct device *dev)
>>
>> /* fan controller base address */
>> fan = of_find_compatible_node(dev->parent->of_node, NULL, "gw,gsc-fan");
>
>A single of_node_put(fan) here would have been be sufficient.
I think of_node_put after should come after its usage, right?
>>> - if (fan && of_property_read_u32(fan, "reg", &pdata->fan_base)) {
>> + if (fan && of_property_read_u32(fan, "reg", &pdata->fan_base)) {
>> + of_node_put(fan);
>> dev_err(dev, "fan node without base\n");
>> return ERR_PTR(-EINVAL);
>> }
>> +
>> + /* if fan&&!of_property_read_u32 fail */
>
>This comment only adds confusion and does not add any value.
Sorry, I just want to say, if *fan* is not NULL, but of_property_read_u32() returns 0.
In that case, we still need a of_node_put() to release fan, right?
>
>Guenter
>
>> + of_node_put(fan);
>>
>> /* allocate structures for channels and count instances of each type */
>> device_for_each_child_node(dev, child) {
Hi, Guenter, I am preparing my new patch and I want to discuss your suggestions as above.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drivers: hwmon: Add missing of_node_put() in gsc-hwmon.c
2022-06-16 2:37 ` 和亮
@ 2022-06-16 3:05 ` Guenter Roeck
0 siblings, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2022-06-16 3:05 UTC (permalink / raw)
To: 和亮; +Cc: tharvey, rjones, jdelvare, linux-hwmon, linux-kernel
On 6/15/22 19:37, 和亮 wrote:
>
>
>
> At 2022-06-16 01:57:49, "Guenter Roeck" <linux@roeck-us.net> wrote:
>>
>> Please use proper subject lines. Here it should have been
>>
>> hwmon: (gsc-hwmon) Add missing of_node_put()
>
>
>
> Thanks, I will change it in my new patch.
>
>
>>>> drivers/hwmon/gsc-hwmon.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwmon/gsc-hwmon.c b/drivers/hwmon/gsc-hwmon.c
>>> index 1fe37418ff46..34c20d13627a 100644
>>> --- a/drivers/hwmon/gsc-hwmon.c
>>> +++ b/drivers/hwmon/gsc-hwmon.c
>>> @@ -268,10 +268,14 @@ gsc_hwmon_get_devtree_pdata(struct device *dev)
>>>
>>> /* fan controller base address */
>>> fan = of_find_compatible_node(dev->parent->of_node, NULL, "gw,gsc-fan");
>>
>> A single of_node_put(fan) here would have been be sufficient.
>
>
>
> I think of_node_put after should come after its usage, right?
>
>
Yes, you are correct. Sorry for the noise.
>>>> - if (fan && of_property_read_u32(fan, "reg", &pdata->fan_base)) {
>>> + if (fan && of_property_read_u32(fan, "reg", &pdata->fan_base)) {
>>> + of_node_put(fan);
>>> dev_err(dev, "fan node without base\n");
>>> return ERR_PTR(-EINVAL);
>>> }
>>> +
>>> + /* if fan&&!of_property_read_u32 fail */
>>
>
>> This comment only adds confusion and does not add any value.
>
>
> Sorry, I just want to say, if *fan* is not NULL, but of_property_read_u32() returns 0.
> In that case, we still need a of_node_put() to release fan, right?
>
Yes, but that is obvious, and the comment is not needed.
Thanks,
Guenter
>>
>> Guenter
>>
>>> + of_node_put(fan);
>>>
>>> /* allocate structures for channels and count instances of each type */
>
>>> device_for_each_child_node(dev, child) {
>
>
> Hi, Guenter, I am preparing my new patch and I want to discuss your suggestions as above.
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-16 3:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-15 15:18 [PATCH] drivers: hwmon: Add missing of_node_put() in gsc-hwmon.c Liang He
2022-06-15 17:57 ` Guenter Roeck
2022-06-16 2:37 ` 和亮
2022-06-16 3:05 ` Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).