* [PATCH] ftgmac100: Hold reference returned by of_get_child_by_name()
@ 2022-07-04 15:18 Liang He
2022-07-06 1:48 ` Jakub Kicinski
0 siblings, 1 reply; 6+ messages in thread
From: Liang He @ 2022-07-04 15:18 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, windhl, netdev
In ftgmac100_probe(), we should hold the refernece returned by
of_get_child_by_name() and use it to call of_node_put() for
reference balance.
Signed-off-by: Liang He <windhl@126.com>
---
drivers/net/ethernet/faraday/ftgmac100.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 5231818943c6..e50bd7beb09b 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1770,7 +1770,7 @@ static int ftgmac100_probe(struct platform_device *pdev)
int irq;
struct net_device *netdev;
struct ftgmac100 *priv;
- struct device_node *np;
+ struct device_node *np, *child_np;
int err = 0;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1883,7 +1883,7 @@ static int ftgmac100_probe(struct platform_device *pdev)
/* Display what we found */
phy_attached_info(phy);
- } else if (np && !of_get_child_by_name(np, "mdio")) {
+ } else if (np && !(child_np = of_get_child_by_name(np, "mdio"))) {
/* Support legacy ASPEED devicetree descriptions that decribe a
* MAC with an embedded MDIO controller but have no "mdio"
* child node. Automatically scan the MDIO bus for available
@@ -1901,6 +1901,8 @@ static int ftgmac100_probe(struct platform_device *pdev)
}
}
+ if (child_np)
+ of_node_put(child_np);
if (priv->is_aspeed) {
err = ftgmac100_setup_clk(priv);
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ftgmac100: Hold reference returned by of_get_child_by_name()
2022-07-04 15:18 [PATCH] ftgmac100: Hold reference returned by of_get_child_by_name() Liang He
@ 2022-07-06 1:48 ` Jakub Kicinski
2022-07-06 1:58 ` Liang He
2022-07-06 8:55 ` Liang He
0 siblings, 2 replies; 6+ messages in thread
From: Jakub Kicinski @ 2022-07-06 1:48 UTC (permalink / raw)
To: Liang He; +Cc: davem, edumazet, pabeni, netdev
On Mon, 4 Jul 2022 23:18:19 +0800 Liang He wrote:
> In ftgmac100_probe(), we should hold the refernece returned by
> of_get_child_by_name() and use it to call of_node_put() for
> reference balance.
>
> Signed-off-by: Liang He <windhl@126.com>
> ---
> drivers/net/ethernet/faraday/ftgmac100.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 5231818943c6..e50bd7beb09b 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1770,7 +1770,7 @@ static int ftgmac100_probe(struct platform_device *pdev)
> int irq;
> struct net_device *netdev;
> struct ftgmac100 *priv;
> - struct device_node *np;
> + struct device_node *np, *child_np;
> int err = 0;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -1883,7 +1883,7 @@ static int ftgmac100_probe(struct platform_device *pdev)
>
> /* Display what we found */
> phy_attached_info(phy);
> - } else if (np && !of_get_child_by_name(np, "mdio")) {
> + } else if (np && !(child_np = of_get_child_by_name(np, "mdio"))) {
> /* Support legacy ASPEED devicetree descriptions that decribe a
> * MAC with an embedded MDIO controller but have no "mdio"
> * child node. Automatically scan the MDIO bus for available
> @@ -1901,6 +1901,8 @@ static int ftgmac100_probe(struct platform_device *pdev)
> }
>
> }
> + if (child_np)
> + of_node_put(child_np);
Since we don't care about the value of the node we should add a helper
which checks for presence of the node and releases the reference,
rather than have to do that in this large function.
Please also add a Fixes tag.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re:Re: [PATCH] ftgmac100: Hold reference returned by of_get_child_by_name()
2022-07-06 1:48 ` Jakub Kicinski
@ 2022-07-06 1:58 ` Liang He
2022-07-06 8:55 ` Liang He
1 sibling, 0 replies; 6+ messages in thread
From: Liang He @ 2022-07-06 1:58 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, edumazet, pabeni, netdev
At 2022-07-06 09:48:05, "Jakub Kicinski" <kuba@kernel.org> wrote:
>On Mon, 4 Jul 2022 23:18:19 +0800 Liang He wrote:
>> In ftgmac100_probe(), we should hold the refernece returned by
>> of_get_child_by_name() and use it to call of_node_put() for
>> reference balance.
>>
>> Signed-off-by: Liang He <windhl@126.com>
>> ---
>> drivers/net/ethernet/faraday/ftgmac100.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
>> index 5231818943c6..e50bd7beb09b 100644
>> --- a/drivers/net/ethernet/faraday/ftgmac100.c
>> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
>> @@ -1770,7 +1770,7 @@ static int ftgmac100_probe(struct platform_device *pdev)
>> int irq;
>> struct net_device *netdev;
>> struct ftgmac100 *priv;
>> - struct device_node *np;
>> + struct device_node *np, *child_np;
>> int err = 0;
>>
>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> @@ -1883,7 +1883,7 @@ static int ftgmac100_probe(struct platform_device *pdev)
>>
>> /* Display what we found */
>> phy_attached_info(phy);
>> - } else if (np && !of_get_child_by_name(np, "mdio")) {
>> + } else if (np && !(child_np = of_get_child_by_name(np, "mdio"))) {
>> /* Support legacy ASPEED devicetree descriptions that decribe a
>> * MAC with an embedded MDIO controller but have no "mdio"
>> * child node. Automatically scan the MDIO bus for available
>> @@ -1901,6 +1901,8 @@ static int ftgmac100_probe(struct platform_device *pdev)
>> }
>>
>> }
>> + if (child_np)
>> + of_node_put(child_np);
>
>Since we don't care about the value of the node we should add a helper
>which checks for presence of the node and releases the reference,
>rather than have to do that in this large function.
>
Thanks, I will try it.
>Please also add a Fixes tag.
Sorry, I miss the fix tag.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re:Re: [PATCH] ftgmac100: Hold reference returned by of_get_child_by_name()
2022-07-06 1:48 ` Jakub Kicinski
2022-07-06 1:58 ` Liang He
@ 2022-07-06 8:55 ` Liang He
2022-07-06 16:34 ` Jakub Kicinski
1 sibling, 1 reply; 6+ messages in thread
From: Liang He @ 2022-07-06 8:55 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, edumazet, pabeni, netdev
At 2022-07-06 09:48:05, "Jakub Kicinski" <kuba@kernel.org> wrote:
>On Mon, 4 Jul 2022 23:18:19 +0800 Liang He wrote:
>> In ftgmac100_probe(), we should hold the refernece returned by
>> of_get_child_by_name() and use it to call of_node_put() for
>> reference balance.
>>
>> Signed-off-by: Liang He <windhl@126.com>
>> ---
>> drivers/net/ethernet/faraday/ftgmac100.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
>> index 5231818943c6..e50bd7beb09b 100644
>> --- a/drivers/net/ethernet/faraday/ftgmac100.c
>> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
>> @@ -1770,7 +1770,7 @@ static int ftgmac100_probe(struct platform_device *pdev)
>> int irq;
>> struct net_device *netdev;
>> struct ftgmac100 *priv;
>> - struct device_node *np;
>> + struct device_node *np, *child_np;
>> int err = 0;
>>
>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> @@ -1883,7 +1883,7 @@ static int ftgmac100_probe(struct platform_device *pdev)
>>
>> /* Display what we found */
>> phy_attached_info(phy);
>> - } else if (np && !of_get_child_by_name(np, "mdio")) {
>> + } else if (np && !(child_np = of_get_child_by_name(np, "mdio"))) {
>> /* Support legacy ASPEED devicetree descriptions that decribe a
>> * MAC with an embedded MDIO controller but have no "mdio"
>> * child node. Automatically scan the MDIO bus for available
>> @@ -1901,6 +1901,8 @@ static int ftgmac100_probe(struct platform_device *pdev)
>> }
>>
>> }
>> + if (child_np)
>> + of_node_put(child_np);
>
>Since we don't care about the value of the node we should add a helper
>which checks for presence of the node and releases the reference,
>rather than have to do that in this large function.
>
>Please also add a Fixes tag.
Hi, Jakub,
Can you tell me where to add such helper?
you mean add a helper in of.h for common usasge or just add it in this file?
Thanks,
Liang
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ftgmac100: Hold reference returned by of_get_child_by_name()
2022-07-06 8:55 ` Liang He
@ 2022-07-06 16:34 ` Jakub Kicinski
2022-07-07 1:28 ` Liang He
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2022-07-06 16:34 UTC (permalink / raw)
To: Liang He; +Cc: davem, edumazet, pabeni, netdev
On Wed, 6 Jul 2022 16:55:37 +0800 (CST) Liang He wrote:
> >Since we don't care about the value of the node we should add a helper
> >which checks for presence of the node and releases the reference,
> >rather than have to do that in this large function.
> >
> >Please also add a Fixes tag.
>
> Hi, Jakub,
>
> Can you tell me where to add such helper?
>
> you mean add a helper in of.h for common usasge or just add it in this file?
I was wondering about that. Since this is a fix let's keep it simple
and add the helper directly in the same source file.
If you have more time to spend on this try searching around the tree to
see if there are more places where such helper would help. If there are
we can move the helper to of.h and convert the users in -next.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re:Re: [PATCH] ftgmac100: Hold reference returned by of_get_child_by_name()
2022-07-06 16:34 ` Jakub Kicinski
@ 2022-07-07 1:28 ` Liang He
0 siblings, 0 replies; 6+ messages in thread
From: Liang He @ 2022-07-07 1:28 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, edumazet, pabeni, netdev
At 2022-07-07 00:34:59, "Jakub Kicinski" <kuba@kernel.org> wrote:
>On Wed, 6 Jul 2022 16:55:37 +0800 (CST) Liang He wrote:
>> >Since we don't care about the value of the node we should add a helper
>> >which checks for presence of the node and releases the reference,
>> >rather than have to do that in this large function.
>> >
>> >Please also add a Fixes tag.
>>
>> Hi, Jakub,
>>
>> Can you tell me where to add such helper?
>>
>> you mean add a helper in of.h for common usasge or just add it in this file?
>
>I was wondering about that. Since this is a fix let's keep it simple
>and add the helper directly in the same source file.
>
Thanks, I will first make a quick patch for this bug.
>If you have more time to spend on this try searching around the tree to
>see if there are more places where such helper would help. If there are
>we can move the helper to of.h and convert the users in -next.
Yes, I have found many similar bugs and sent many similar patches, I would like
try to make a common helper in of.h.
Thanks,
Liang
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-07 1:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-04 15:18 [PATCH] ftgmac100: Hold reference returned by of_get_child_by_name() Liang He
2022-07-06 1:48 ` Jakub Kicinski
2022-07-06 1:58 ` Liang He
2022-07-06 8:55 ` Liang He
2022-07-06 16:34 ` Jakub Kicinski
2022-07-07 1:28 ` Liang He
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.