[PATCH 00/15] ethernet: add missing of_node_put after calling of_parse_phandle

[PATCH 00/15] ethernet: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

Hi David,

This patch set fixes missing of_node_put issue at ethernet driver.
of_node_put needs to be called when the device node which is got
from of_parse_phandle has finished using.

Thanks.

Peter Chen (15):
  ethernet: altera: add missing of_node_put
  ethernet: apm: xgene: add missing of_node_put after calling
    of_parse_phandle
  ethernet: arc: emac_main: add missing of_node_put after calling
    of_parse_phandle
  ethernet: aurora: nb8800: add missing of_node_put after calling
    of_parse_phandle
  ethernet: cavium: octeon: add missing of_node_put after calling
    of_parse_phandle
  ethernet: hisilicon: hns: hns_dsaf_mac: add missing of_node_put after
    calling of_parse_phandle
  ethernet: hisilicon: hns: hns_dsaf_main: add missing of_node_put after
    calling of_parse_phandle
  ethernet: marvell: mvneta: add missing of_node_put after calling
    of_parse_phandle
  ethernet: marvell: mvpp2: add missing of_node_put after calling
    of_parse_phandle
  ethernet: marvell: pxa168_eth: add missing of_node_put after calling
    of_parse_phandle
  ethernet: renesas: ravb_main: add missing of_node_put after calling
    of_parse_phandle
  ethernet: renesas: sh_eth: add missing of_node_put after calling
    of_parse_phandle
  ethernet: stmicro: stmmac: dwmac-socfpga: add missing of_node_put
    after calling of_parse_phandle
  ethernet: stmicro: stmmac: stmmac_platform: add missing of_node_put
    after calling of_parse_phandle
  ethernet: ti: davinci_emac: add missing of_node_put after calling
    of_parse_phandle

 drivers/net/ethernet/altera/altera_tse_main.c      |  1 +
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.c     |  1 +
 drivers/net/ethernet/arc/emac_main.c               | 14 ++++++---
 drivers/net/ethernet/aurora/nb8800.c               |  2 ++
 drivers/net/ethernet/cavium/octeon/octeon_mgmt.c   |  1 +
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c  |  9 ++++--
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c |  7 +++--
 drivers/net/ethernet/marvell/mvneta.c              |  1 +
 drivers/net/ethernet/marvell/mvpp2.c               |  2 ++
 drivers/net/ethernet/marvell/pxa168_eth.c          |  1 +
 drivers/net/ethernet/renesas/ravb_main.c           |  1 +
 drivers/net/ethernet/renesas/sh_eth.c              |  1 +
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    | 36 ++++++++++++++++------
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |  5 ++-
 drivers/net/ethernet/ti/davinci_emac.c             |  1 +
 15 files changed, 62 insertions(+), 21 deletions(-)

-- 
1.9.1

[PATCH 01/15] ethernet: altera: add missing of_node_put

[Peter Chen]

of_node_put needs to be called when the device node which is got
from of_parse_phandle (or of_node_get) has finished using.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/net/ethernet/altera/altera_tse_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index 49025e9..bda31f3 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -815,6 +815,7 @@ static int init_phy(struct net_device *dev)
 		phydev = of_phy_connect(dev, phynode,
 			&altera_tse_adjust_link, 0, priv->phy_iface);
 	}
+	of_node_put(phynode);
 
 	if (!phydev) {
 		netdev_err(dev, "Could not find the PHY\n");
-- 
1.9.1

[PATCH 02/15] ethernet: apm: xgene: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

of_node_put needs to be called when the device node which is got
from of_parse_phandle has finished using.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
index 2f5638f..fb23408 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
@@ -758,6 +758,7 @@ static int xgene_enet_phy_connect(struct net_device *ndev)
 
 		phy_dev = of_phy_connect(ndev, phy_np, &xgene_enet_adjust_link,
 					 0, pdata->phy_mode);
+		of_node_put(phy_np);
 		if (!phy_dev) {
 			netdev_err(ndev, "Could not connect to PHY\n");
 			return -ENODEV;
-- 
1.9.1

[PATCH 03/15] ethernet: arc: emac_main: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

of_node_put needs to be called when the device node which is got
from of_parse_phandle has finished using.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/net/ethernet/arc/emac_main.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/arc/emac_main.c b/drivers/net/ethernet/arc/emac_main.c
index 586beda..0e6331c 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -749,14 +749,16 @@ int arc_emac_probe(struct net_device *ndev, int interface)
 	err = of_address_to_resource(dev->of_node, 0, &res_regs);
 	if (err) {
 		dev_err(dev, "failed to retrieve registers base from device tree\n");
-		return -ENODEV;
+		err = -ENODEV;
+		goto out_put_node;
 	}
 
 	/* Get IRQ from device tree */
 	irq = irq_of_parse_and_map(dev->of_node, 0);
 	if (!irq) {
 		dev_err(dev, "failed to retrieve <irq> value from device tree\n");
-		return -ENODEV;
+		err = -ENODEV;
+		goto out_put_node;
 	}
 
 	ndev->netdev_ops = &arc_emac_netdev_ops;
@@ -778,7 +780,7 @@ int arc_emac_probe(struct net_device *ndev, int interface)
 		err = clk_prepare_enable(priv->clk);
 		if (err) {
 			dev_err(dev, "failed to enable clock\n");
-			return err;
+			goto out_put_node;
 		}
 
 		clock_frequency = clk_get_rate(priv->clk);
@@ -787,7 +789,8 @@ int arc_emac_probe(struct net_device *ndev, int interface)
 		if (of_property_read_u32(dev->of_node, "clock-frequency",
 					 &clock_frequency)) {
 			dev_err(dev, "failed to retrieve <clock-frequency> from device tree\n");
-			return -EINVAL;
+			err = -EINVAL;
+			goto out_put_node;
 		}
 	}
 
@@ -877,6 +880,9 @@ out_mdio:
 out_clken:
 	if (priv->clk)
 		clk_disable_unprepare(priv->clk);
+out_put_node:
+	of_node_put(phy_node);
+
 	return err;
 }
 EXPORT_SYMBOL_GPL(arc_emac_probe);
-- 
1.9.1

[PATCH 04/15] ethernet: aurora: nb8800: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

of_node_put needs to be called when the device node which is got
from of_parse_phandle has finished using.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/net/ethernet/aurora/nb8800.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
index 0d4ea92..d15d96b 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -1006,6 +1006,7 @@ static int nb8800_stop(struct net_device *dev)
 
 	netif_stop_queue(dev);
 	napi_disable(&priv->napi);
+	of_node_put(priv->phy_node);
 
 	nb8800_dma_stop(dev);
 	nb8800_mac_rx(dev, false);
@@ -1504,6 +1505,7 @@ static int nb8800_probe(struct platform_device *pdev)
 err_free_dma:
 	nb8800_dma_free(dev);
 err_free_bus:
+	of_node_put(priv->phy_node);
 	mdiobus_unregister(bus);
 err_disable_clk:
 	clk_disable_unprepare(priv->clk);
-- 
1.9.1

Re: [PATCH 04/15] ethernet: aurora: nb8800: add missing of_node_put after calling of_parse_phandle

[David Miller]

From: Peter Chen <peter.chen@nxp.com>
Date: Wed, 27 Jul 2016 10:20:37 +0800

> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
> index 0d4ea92..d15d96b 100644
> --- a/drivers/net/ethernet/aurora/nb8800.c
> +++ b/drivers/net/ethernet/aurora/nb8800.c
> @@ -1006,6 +1006,7 @@ static int nb8800_stop(struct net_device *dev)
>  
>  	netif_stop_queue(dev);
>  	napi_disable(&priv->napi);
> +	of_node_put(priv->phy_node);
>  
>  	nb8800_dma_stop(dev);
>  	nb8800_mac_rx(dev, false);

This is broken.  The priv->phy_node reference is taken in the probe function,
therefore you have to drop that reference in the "remove" function not the
"stop" function.

I'm just looking over basic details of this patch series, and along with some
other feedback you've received, it looks like you really didn't put a lot of
auditing into the changes you are making.

Please go over this series one more time and resubmit the entire thing after
everything is sorted out and double-checked.

Thank you.

RE: [PATCH 04/15] ethernet: aurora: nb8800: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

 
>> diff --git a/drivers/net/ethernet/aurora/nb8800.c
>> b/drivers/net/ethernet/aurora/nb8800.c
>> index 0d4ea92..d15d96b 100644
>> --- a/drivers/net/ethernet/aurora/nb8800.c
>> +++ b/drivers/net/ethernet/aurora/nb8800.c
>> @@ -1006,6 +1006,7 @@ static int nb8800_stop(struct net_device *dev)
>>
>>  	netif_stop_queue(dev);
>>  	napi_disable(&priv->napi);
>> +	of_node_put(priv->phy_node);
>>
>>  	nb8800_dma_stop(dev);
>>  	nb8800_mac_rx(dev, false);
>
>This is broken.  The priv->phy_node reference is taken in the probe function,
>therefore you have to drop that reference in the "remove" function not the "stop"
>function.
>
>I'm just looking over basic details of this patch series, and along with some other
>feedback you've received, it looks like you really didn't put a lot of auditing into the
>changes you are making.
>
>Please go over this series one more time and resubmit the entire thing after
>everything is sorted out and double-checked.
>

Thanks for your comments, David. I do not familiar with Ethernet driver, and thought
if the node is used up, it can be freed like other drivers. But for phy_node, it will be
used in whole driver's life cycle, it is my careless. I will review each patch in this series
again.

Best regards,
Peter

[PATCH 05/15] ethernet: cavium: octeon: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

of_node_put needs to be called when the device node which is got
from of_parse_phandle has finished using.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/net/ethernet/cavium/octeon/octeon_mgmt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
index e8bc15b..5eb9d8c 100644
--- a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
+++ b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
@@ -960,6 +960,7 @@ static int octeon_mgmt_init_phy(struct net_device *netdev)
 	phydev = of_phy_connect(netdev, p->phy_np,
 				octeon_mgmt_adjust_link, 0,
 				PHY_INTERFACE_MODE_MII);
+	of_node_put(p->phy_np);
 
 	if (!phydev)
 		return -ENODEV;
-- 
1.9.1

Re: [PATCH 05/15] ethernet: cavium: octeon: add missing of_node_put after calling of_parse_phandle

[David Daney]

On 07/26/2016 07:20 PM, Peter Chen wrote:
> of_node_put needs to be called when the device node which is got
> from of_parse_phandle has finished using.
>
> Signed-off-by: Peter Chen <peter.chen@nxp.com>

NAK.


> ---
>   drivers/net/ethernet/cavium/octeon/octeon_mgmt.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
> index e8bc15b..5eb9d8c 100644
> --- a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
> +++ b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
> @@ -960,6 +960,7 @@ static int octeon_mgmt_init_phy(struct net_device *netdev)
>   	phydev = of_phy_connect(netdev, p->phy_np,
>   				octeon_mgmt_adjust_link, 0,
>   				PHY_INTERFACE_MODE_MII);
> +	of_node_put(p->phy_np);

I don't think you can do this here.  octeon_mgmt_init_phy() may be 
called multiple times in the life of the driver, so p->phy_np must 
remain valid.

It may be appropriate to do the  of_node_put() in the 
octeon_mgmt_remove() function.


>
>   	if (!phydev)
>   		return -ENODEV;
>

RE: [PATCH 05/15] ethernet: cavium: octeon: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

 
>
>> ---
>>   drivers/net/ethernet/cavium/octeon/octeon_mgmt.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
>> b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
>> index e8bc15b..5eb9d8c 100644
>> --- a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
>> +++ b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
>> @@ -960,6 +960,7 @@ static int octeon_mgmt_init_phy(struct net_device *netdev)
>>   	phydev = of_phy_connect(netdev, p->phy_np,
>>   				octeon_mgmt_adjust_link, 0,
>>   				PHY_INTERFACE_MODE_MII);
>> +	of_node_put(p->phy_np);
>
>I don't think you can do this here.  octeon_mgmt_init_phy() may be called multiple
>times in the life of the driver, so p->phy_np must remain valid.
>
>It may be appropriate to do the  of_node_put() in the
>octeon_mgmt_remove() function.
 
Thanks, I will change it.

Peter

[PATCH 06/15] ethernet: hisilicon: hns: hns_dsaf_mac: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

of_node_put needs to be called when the device node which is got
from of_parse_phandle has finished using.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
index 3fb87e2..18d72ea 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
@@ -786,6 +786,7 @@ static int  hns_mac_get_info(struct hns_mac_cb *mac_cb)
 		np = of_parse_phandle(mac_cb->dev->of_node, "phy-handle",
 				      mac_cb->mac_id);
 		mac_cb->phy_dev = of_phy_find_device(np);
+		of_node_put(np);
 		if (mac_cb->phy_dev) {
 			/* refcount is held by of_phy_find_device()
 			 * if the phy_dev is found
@@ -804,6 +805,7 @@ static int  hns_mac_get_info(struct hns_mac_cb *mac_cb)
 		np = of_parse_phandle(to_of_node(mac_cb->fw_port),
 				      "phy-handle", 0);
 		mac_cb->phy_dev = of_phy_find_device(np);
+		of_node_put(np);
 		if (mac_cb->phy_dev) {
 			/* refcount is held by of_phy_find_device()
 			 * if the phy_dev is found
@@ -813,9 +815,10 @@ static int  hns_mac_get_info(struct hns_mac_cb *mac_cb)
 				mac_cb->mac_id, np->name);
 		}
 
-		syscon = syscon_node_to_regmap(
-				of_parse_phandle(to_of_node(mac_cb->fw_port),
-						 "serdes-syscon", 0));
+		np = of_parse_phandle(to_of_node(mac_cb->fw_port),
+					"serdes-syscon", 0);
+		syscon = syscon_node_to_regmap(np);
+		of_node_put(np);
 		if (IS_ERR_OR_NULL(syscon)) {
 			dev_err(mac_cb->dev, "serdes-syscon is needed!\n");
 			return -EINVAL;
-- 
1.9.1

Re: [PATCH 06/15] ethernet: hisilicon: hns: hns_dsaf_mac: add missing of_node_put after calling of_parse_phandle

[Yisen Zhuang]

在 2016/7/27 10:20, Peter Chen 写道:
> of_node_put needs to be called when the device node which is got
> from of_parse_phandle has finished using.
> 
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> ---
>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
> index 3fb87e2..18d72ea 100644
> --- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
> +++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
> @@ -786,6 +786,7 @@ static int  hns_mac_get_info(struct hns_mac_cb *mac_cb)
>  		np = of_parse_phandle(mac_cb->dev->of_node, "phy-handle",
>  				      mac_cb->mac_id);
>  		mac_cb->phy_dev = of_phy_find_device(np);
> +		of_node_put(np);
>  		if (mac_cb->phy_dev) {
>  			/* refcount is held by of_phy_find_device()
>  			 * if the phy_dev is found

np is accessed in case of of_phy_find_device() returns a no null value, so it has to be
moved after the dev_dbg log.

> @@ -804,6 +805,7 @@ static int  hns_mac_get_info(struct hns_mac_cb *mac_cb)
>  		np = of_parse_phandle(to_of_node(mac_cb->fw_port),
>  				      "phy-handle", 0);
>  		mac_cb->phy_dev = of_phy_find_device(np);
> +		of_node_put(np);
>  		if (mac_cb->phy_dev) {
>  			/* refcount is held by of_phy_find_device()
>  			 * if the phy_dev is found

np is accessed in case of of_phy_find_device() returns a no null value, so it has to be
moved after the dev_dbg log.

Thanks,

Yisen

> @@ -813,9 +815,10 @@ static int  hns_mac_get_info(struct hns_mac_cb *mac_cb)
>  				mac_cb->mac_id, np->name);
>  		}
>  
> -		syscon = syscon_node_to_regmap(
> -				of_parse_phandle(to_of_node(mac_cb->fw_port),
> -						 "serdes-syscon", 0));
> +		np = of_parse_phandle(to_of_node(mac_cb->fw_port),
> +					"serdes-syscon", 0);
> +		syscon = syscon_node_to_regmap(np);
> +		of_node_put(np);
>  		if (IS_ERR_OR_NULL(syscon)) {
>  			dev_err(mac_cb->dev, "serdes-syscon is needed!\n");
>  			return -EINVAL;
> 

RE: [PATCH 06/15] ethernet: hisilicon: hns: hns_dsaf_mac: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

 
>在 2016/7/27 10:20, Peter Chen 写道:
>> of_node_put needs to be called when the device node which is got from
>> of_parse_phandle has finished using.
>>
>> Signed-off-by: Peter Chen <peter.chen@nxp.com>
>> ---
>>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
>> b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
>> index 3fb87e2..18d72ea 100644
>> --- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
>> +++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
>> @@ -786,6 +786,7 @@ static int  hns_mac_get_info(struct hns_mac_cb *mac_cb)
>>  		np = of_parse_phandle(mac_cb->dev->of_node, "phy-handle",
>>  				      mac_cb->mac_id);
>>  		mac_cb->phy_dev = of_phy_find_device(np);
>> +		of_node_put(np);
>>  		if (mac_cb->phy_dev) {
>>  			/* refcount is held by of_phy_find_device()
>>  			 * if the phy_dev is found
>
>np is accessed in case of of_phy_find_device() returns a no null value, so it has to be
>moved after the dev_dbg log.
>
>> @@ -804,6 +805,7 @@ static int  hns_mac_get_info(struct hns_mac_cb *mac_cb)
>>  		np = of_parse_phandle(to_of_node(mac_cb->fw_port),
>>  				      "phy-handle", 0);
>>  		mac_cb->phy_dev = of_phy_find_device(np);
>> +		of_node_put(np);
>>  		if (mac_cb->phy_dev) {
>>  			/* refcount is held by of_phy_find_device()
>>  			 * if the phy_dev is found
>
>np is accessed in case of of_phy_find_device() returns a no null value, so it has to be
>moved after the dev_dbg log.
>

Thanks, I will change them.

Peter

[PATCH 07/15] ethernet: hisilicon: hns: hns_dsaf_main: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

of_node_put needs to be called when the device node which is got
from of_parse_phandle has finished using.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
index 2ef4277..afb5daa 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
@@ -51,7 +51,7 @@ int hns_dsaf_get_cfg(struct dsaf_device *dsaf_dev)
 	const char *mode_str;
 	struct regmap *syscon;
 	struct resource *res;
-	struct device_node *np = dsaf_dev->dev->of_node;
+	struct device_node *np = dsaf_dev->dev->of_node, *np_temp;
 	struct platform_device *pdev = to_platform_device(dsaf_dev->dev);
 
 	if (dev_of_node(dsaf_dev->dev)) {
@@ -102,8 +102,9 @@ int hns_dsaf_get_cfg(struct dsaf_device *dsaf_dev)
 		dsaf_dev->dsaf_tc_mode = HRD_DSAF_4TC_MODE;
 
 	if (dev_of_node(dsaf_dev->dev)) {
-		syscon = syscon_node_to_regmap(
-				of_parse_phandle(np, "subctrl-syscon", 0));
+		np_temp = of_parse_phandle(np, "subctrl-syscon", 0);
+		syscon = syscon_node_to_regmap(np_temp);
+		of_node_put(np_temp);
 		if (IS_ERR_OR_NULL(syscon)) {
 			res = platform_get_resource(pdev, IORESOURCE_MEM,
 						    res_idx++);
-- 
1.9.1

[PATCH 08/15] ethernet: marvell: mvneta: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

of_node_put needs to be called when the device node which is got
from of_parse_phandle has finished using.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/net/ethernet/marvell/mvneta.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index f92018b..16aca9d 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -4113,6 +4113,7 @@ static int mvneta_probe(struct platform_device *pdev)
 	if (bm_node && bm_node->data) {
 		pp->bm_priv = bm_node->data;
 		err = mvneta_bm_port_init(pdev, pp);
+		of_node_put(bm_node);
 		if (err < 0) {
 			dev_info(&pdev->dev, "use SW buffer management\n");
 			pp->bm_priv = NULL;
-- 
1.9.1

[PATCH 09/15] ethernet: marvell: mvpp2: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

of_node_put needs to be called when the device node which is got
from of_parse_phandle has finished using.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 0b04717..a86f22b 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -5531,6 +5531,7 @@ static int mvpp2_phy_connect(struct mvpp2_port *port)
 
 	phy_dev = of_phy_connect(port->dev, port->phy_node, mvpp2_link_event, 0,
 				 port->phy_interface);
+	of_node_put(port->phy_node);
 	if (!phy_dev) {
 		netdev_err(port->dev, "cannot connect to phy\n");
 		return -ENODEV;
@@ -6234,6 +6235,7 @@ err_free_stats:
 err_free_irq:
 	irq_dispose_mapping(port->irq);
 err_free_netdev:
+	of_node_put(phy_node);
 	free_netdev(dev);
 	return err;
 }
-- 
1.9.1

[PATCH 10/15] ethernet: marvell: pxa168_eth: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

of_node_put needs to be called when the device node which is got
from of_parse_phandle has finished using.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/net/ethernet/marvell/pxa168_eth.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index aeeb2e7..5d5000c 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -1506,6 +1506,7 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 		}
 		of_property_read_u32(np, "reg", &pep->phy_addr);
 		pep->phy_intf = of_get_phy_mode(pdev->dev.of_node);
+		of_node_put(np);
 	}
 
 	/* Hardware supports only 3 ports */
-- 
1.9.1

[PATCH 11/15] ethernet: renesas: ravb_main: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

of_node_put needs to be called when the device node which is got
from of_parse_phandle has finished using.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/net/ethernet/renesas/ravb_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 8377d02..1e1cc0f 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1005,6 +1005,7 @@ static int ravb_phy_init(struct net_device *ndev)
 	}
 	phydev = of_phy_connect(ndev, pn, ravb_adjust_link, 0,
 				priv->phy_interface);
+	of_node_put(pn);
 	if (!phydev) {
 		netdev_err(ndev, "failed to connect PHY\n");
 		return -ENOENT;
-- 
1.9.1

Re: [PATCH 11/15] ethernet: renesas: ravb_main: add missing of_node_put after calling of_parse_phandle

[Sergei Shtylyov]

On 7/27/2016 5:20 AM, Peter Chen wrote:

> of_node_put needs to be called when the device node which is got
> from of_parse_phandle has finished using.
>
> Signed-off-by: Peter Chen <peter.chen@nxp.com>

Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

MBR, Sergei

[PATCH 12/15] ethernet: renesas: sh_eth: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

of_node_put needs to be called when the device node which is got
from of_parse_phandle has finished using.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/net/ethernet/renesas/sh_eth.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 7bd910c..799d58d 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -1780,6 +1780,7 @@ static int sh_eth_phy_init(struct net_device *ndev)
 					sh_eth_adjust_link, 0,
 					mdp->phy_interface);
 
+		of_node_put(pn);
 		if (!phydev)
 			phydev = ERR_PTR(-ENOENT);
 	} else {
-- 
1.9.1

Re: [PATCH 12/15] ethernet: renesas: sh_eth: add missing of_node_put after calling of_parse_phandle

[Sergei Shtylyov]

On 7/27/2016 5:20 AM, Peter Chen wrote:

> of_node_put needs to be called when the device node which is got
> from of_parse_phandle has finished using.
>
> Signed-off-by: Peter Chen <peter.chen@nxp.com>

Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

MBR, Sergei

[PATCH 13/15] ethernet: stmicro: stmmac: dwmac-socfpga: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

of_node_put needs to be called when the device node which is got
from of_parse_phandle has finished using.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    | 36 ++++++++++++++++------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index edd20c3..bec6963 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -135,7 +135,9 @@ static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *
 
 	np_splitter = of_parse_phandle(np, "altr,emac-splitter", 0);
 	if (np_splitter) {
-		if (of_address_to_resource(np_splitter, 0, &res_splitter)) {
+		ret = of_address_to_resource(np_splitter, 0, &res_splitter);
+		of_node_put(np_splitter);
+		if (ret) {
 			dev_info(dev, "Missing emac splitter address\n");
 			return -EINVAL;
 		}
@@ -159,14 +161,17 @@ static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *
 				dev_err(dev,
 					"%s: ERROR: missing emac splitter address\n",
 					__func__);
-				return -EINVAL;
+				ret = -EINVAL;
+				goto err_node_put;
 			}
 
 			dwmac->splitter_base =
 			    devm_ioremap_resource(dev, &res_splitter);
 
-			if (IS_ERR(dwmac->splitter_base))
-				return PTR_ERR(dwmac->splitter_base);
+			if (IS_ERR(dwmac->splitter_base)) {
+				ret = PTR_ERR(dwmac->splitter_base);
+				goto err_node_put;
+			}
 		}
 
 		index = of_property_match_string(np_sgmii_adapter, "reg-names",
@@ -178,14 +183,17 @@ static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *
 				dev_err(dev,
 					"%s: ERROR: failed mapping adapter\n",
 					__func__);
-				return -EINVAL;
+				ret = -EINVAL;
+				goto err_node_put;
 			}
 
 			dwmac->pcs.sgmii_adapter_base =
 			    devm_ioremap_resource(dev, &res_sgmii_adapter);
 
-			if (IS_ERR(dwmac->pcs.sgmii_adapter_base))
-				return PTR_ERR(dwmac->pcs.sgmii_adapter_base);
+			if (IS_ERR(dwmac->pcs.sgmii_adapter_base)) {
+				ret = PTR_ERR(dwmac->pcs.sgmii_adapter_base);
+				goto err_node_put;
+			}
 		}
 
 		index = of_property_match_string(np_sgmii_adapter, "reg-names",
@@ -197,22 +205,30 @@ static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *
 				dev_err(dev,
 					"%s: ERROR: failed mapping tse control port\n",
 					__func__);
-				return -EINVAL;
+				ret = -EINVAL;
+				goto err_node_put;
 			}
 
 			dwmac->pcs.tse_pcs_base =
 			    devm_ioremap_resource(dev, &res_tse_pcs);
 
-			if (IS_ERR(dwmac->pcs.tse_pcs_base))
-				return PTR_ERR(dwmac->pcs.tse_pcs_base);
+			if (IS_ERR(dwmac->pcs.tse_pcs_base)) {
+				ret = PTR_ERR(dwmac->pcs.tse_pcs_base);
+				goto err_node_put;
+			}
 		}
 	}
 	dwmac->reg_offset = reg_offset;
 	dwmac->reg_shift = reg_shift;
 	dwmac->sys_mgr_base_addr = sys_mgr_base_addr;
 	dwmac->dev = dev;
+	of_node_put(np_sgmii_adapter);
 
 	return 0;
+
+err_node_put:
+	of_node_put(np_sgmii_adapter);
+	return ret;
 }
 
 static int socfpga_dwmac_set_phy_mode(struct socfpga_dwmac *dwmac)
-- 
1.9.1

[PATCH 14/15] ethernet: stmicro: stmmac: stmmac_platform: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

of_node_put needs to be called when the device node which is got
from of_parse_phandle has finished using.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index f7dfc0a..8d88782 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -113,8 +113,10 @@ static struct stmmac_axi *stmmac_axi_setup(struct platform_device *pdev)
 		return NULL;
 
 	axi = kzalloc(sizeof(*axi), GFP_KERNEL);
-	if (!axi)
+	if (!axi) {
+		of_node_put(np);
 		return ERR_PTR(-ENOMEM);
+	}
 
 	axi->axi_lpi_en = of_property_read_bool(np, "snps,lpi_en");
 	axi->axi_xit_frm = of_property_read_bool(np, "snps,xit_frm");
@@ -127,6 +129,7 @@ static struct stmmac_axi *stmmac_axi_setup(struct platform_device *pdev)
 	of_property_read_u32(np, "snps,wr_osr_lmt", &axi->axi_wr_osr_lmt);
 	of_property_read_u32(np, "snps,rd_osr_lmt", &axi->axi_rd_osr_lmt);
 	of_property_read_u32_array(np, "snps,blen", axi->axi_blen, AXI_BLEN);
+	of_node_put(np);
 
 	return axi;
 }
-- 
1.9.1

Re: [PATCH 14/15] ethernet: stmicro: stmmac: stmmac_platform: add missing of_node_put after calling of_parse_phandle

[Alexandre Torgue]

Hi,

On 07/27/2016 04:20 AM, Peter Chen wrote:
> of_node_put needs to be called when the device node which is got
> from of_parse_phandle has finished using.
>
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> ---
>   drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index f7dfc0a..8d88782 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -113,8 +113,10 @@ static struct stmmac_axi *stmmac_axi_setup(struct platform_device *pdev)
>   		return NULL;
>
>   	axi = kzalloc(sizeof(*axi), GFP_KERNEL);
> -	if (!axi)
> +	if (!axi) {
> +		of_node_put(np);
>   		return ERR_PTR(-ENOMEM);
> +	}
>
>   	axi->axi_lpi_en = of_property_read_bool(np, "snps,lpi_en");
>   	axi->axi_xit_frm = of_property_read_bool(np, "snps,xit_frm");
> @@ -127,6 +129,7 @@ static struct stmmac_axi *stmmac_axi_setup(struct platform_device *pdev)
>   	of_property_read_u32(np, "snps,wr_osr_lmt", &axi->axi_wr_osr_lmt);
>   	of_property_read_u32(np, "snps,rd_osr_lmt", &axi->axi_rd_osr_lmt);
>   	of_property_read_u32_array(np, "snps,blen", axi->axi_blen, AXI_BLEN);
> +	of_node_put(np);
>
>   	return axi;
>   }
>

I agree with the modification inside stmmac_axi. I just have a question 
about np = pdev->dev.of_node inside stmmac_probe_config_dt (same file). 
We could add a "of_node_put(np)" just before "return plat" ?

Regards

alex

RE: [PATCH 14/15] ethernet: stmicro: stmmac: stmmac_platform: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

 
>Hi,
>
>On 07/27/2016 04:20 AM, Peter Chen wrote:
>> of_node_put needs to be called when the device node which is got from
>> of_parse_phandle has finished using.
>>
>> Signed-off-by: Peter Chen <peter.chen@nxp.com>
>> ---
>>   drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> index f7dfc0a..8d88782 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -113,8 +113,10 @@ static struct stmmac_axi *stmmac_axi_setup(struct
>platform_device *pdev)
>>   		return NULL;
>>
>>   	axi = kzalloc(sizeof(*axi), GFP_KERNEL);
>> -	if (!axi)
>> +	if (!axi) {
>> +		of_node_put(np);
>>   		return ERR_PTR(-ENOMEM);
>> +	}
>>
>>   	axi->axi_lpi_en = of_property_read_bool(np, "snps,lpi_en");
>>   	axi->axi_xit_frm = of_property_read_bool(np, "snps,xit_frm"); @@
>> -127,6 +129,7 @@ static struct stmmac_axi *stmmac_axi_setup(struct
>platform_device *pdev)
>>   	of_property_read_u32(np, "snps,wr_osr_lmt", &axi->axi_wr_osr_lmt);
>>   	of_property_read_u32(np, "snps,rd_osr_lmt", &axi->axi_rd_osr_lmt);
>>   	of_property_read_u32_array(np, "snps,blen", axi->axi_blen,
>> AXI_BLEN);
>> +	of_node_put(np);
>>
>>   	return axi;
>>   }
>>
>
>I agree with the modification inside stmmac_axi. I just have a question about np =
>pdev->dev.of_node inside stmmac_probe_config_dt (same file).
>We could add a "of_node_put(np)" just before "return plat" ?
>

Yes, you remind me there is still one node need to be put (should be plat->phy_node),
except for changing node name at error path, how about calling of_node_put(plat->phy_node) at
stmmac_release after phy is disconnected?

Peter

RE: [PATCH 14/15] ethernet: stmicro: stmmac: stmmac_platform: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

 
>
>>Hi,
>>
>>On 07/27/2016 04:20 AM, Peter Chen wrote:
>>> of_node_put needs to be called when the device node which is got from
>>> of_parse_phandle has finished using.
>>>
>>> Signed-off-by: Peter Chen <peter.chen@nxp.com>
>>> ---
>>>   drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 5 ++++-
>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> index f7dfc0a..8d88782 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> @@ -113,8 +113,10 @@ static struct stmmac_axi
>>> *stmmac_axi_setup(struct
>>platform_device *pdev)
>>>   		return NULL;
>>>
>>>   	axi = kzalloc(sizeof(*axi), GFP_KERNEL);
>>> -	if (!axi)
>>> +	if (!axi) {
>>> +		of_node_put(np);
>>>   		return ERR_PTR(-ENOMEM);
>>> +	}
>>>
>>>   	axi->axi_lpi_en = of_property_read_bool(np, "snps,lpi_en");
>>>   	axi->axi_xit_frm = of_property_read_bool(np, "snps,xit_frm"); @@
>>> -127,6 +129,7 @@ static struct stmmac_axi *stmmac_axi_setup(struct
>>platform_device *pdev)
>>>   	of_property_read_u32(np, "snps,wr_osr_lmt", &axi->axi_wr_osr_lmt);
>>>   	of_property_read_u32(np, "snps,rd_osr_lmt", &axi->axi_rd_osr_lmt);
>>>   	of_property_read_u32_array(np, "snps,blen", axi->axi_blen,
>>> AXI_BLEN);
>>> +	of_node_put(np);
>>>
>>>   	return axi;
>>>   }
>>>
>>
>>I agree with the modification inside stmmac_axi. I just have a question
>>about np =
>>pdev->dev.of_node inside stmmac_probe_config_dt (same file).
>>We could add a "of_node_put(np)" just before "return plat" ?
>>
>
>Yes, you remind me there is still one node need to be put (should be plat->phy_node),
>except for changing node name at error path, how about calling of_node_put(plat-
>>phy_node) at stmmac_release after phy is disconnected?
>

Since the plat->phy_node will be used during the whole life cycle, I put it at
stmmac_dvr_remove at my v2 patch set.

Best regards,
Peter

[PATCH 15/15] ethernet: ti: davinci_emac: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

of_node_put needs to be called when the device node which is got
from of_parse_phandle has finished using.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/net/ethernet/ti/davinci_emac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index c6c5465..d8cb9d0 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -1571,6 +1571,7 @@ static int emac_dev_open(struct net_device *ndev)
 	if (priv->phy_node) {
 		phydev = of_phy_connect(ndev, priv->phy_node,
 					&emac_adjust_link, 0, 0);
+		of_node_put(priv->phy_node);
 		if (!phydev) {
 			dev_err(emac_dev, "could not connect to phy %s\n",
 				priv->phy_node->full_name);
-- 
1.9.1

Re: [PATCH 15/15] ethernet: ti: davinci_emac: add missing of_node_put after calling of_parse_phandle

[Mugunthan V N]

On Wednesday 27 July 2016 07:50 AM, Peter Chen wrote:
> of_node_put needs to be called when the device node which is got
> from of_parse_phandle has finished using.
> 
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> ---
>  drivers/net/ethernet/ti/davinci_emac.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
> index c6c5465..d8cb9d0 100644
> --- a/drivers/net/ethernet/ti/davinci_emac.c
> +++ b/drivers/net/ethernet/ti/davinci_emac.c
> @@ -1571,6 +1571,7 @@ static int emac_dev_open(struct net_device *ndev)
>  	if (priv->phy_node) {
>  		phydev = of_phy_connect(ndev, priv->phy_node,
>  					&emac_adjust_link, 0, 0);
> +		of_node_put(priv->phy_node);
>  		if (!phydev) {
>  			dev_err(emac_dev, "could not connect to phy %s\n",
>  				priv->phy_node->full_name);
> 

phy_node is accessed in case of of_phy_connect() returns error, so it
has to be moved after the dev_err log

Regards
Mugunthan V N

RE: [PATCH 15/15] ethernet: ti: davinci_emac: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

 
>On Wednesday 27 July 2016 07:50 AM, Peter Chen wrote:
>> of_node_put needs to be called when the device node which is got from
>> of_parse_phandle has finished using.
>>
>> Signed-off-by: Peter Chen <peter.chen@nxp.com>
>> ---
>>  drivers/net/ethernet/ti/davinci_emac.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/ethernet/ti/davinci_emac.c
>> b/drivers/net/ethernet/ti/davinci_emac.c
>> index c6c5465..d8cb9d0 100644
>> --- a/drivers/net/ethernet/ti/davinci_emac.c
>> +++ b/drivers/net/ethernet/ti/davinci_emac.c
>> @@ -1571,6 +1571,7 @@ static int emac_dev_open(struct net_device *ndev)
>>  	if (priv->phy_node) {
>>  		phydev = of_phy_connect(ndev, priv->phy_node,
>>  					&emac_adjust_link, 0, 0);
>> +		of_node_put(priv->phy_node);
>>  		if (!phydev) {
>>  			dev_err(emac_dev, "could not connect to phy %s\n",
>>  				priv->phy_node->full_name);
>>
>
>phy_node is accessed in case of of_phy_connect() returns error, so it has to be
>moved after the dev_err log
>

Yeah, you are right. I will change it, thanks.

Peter

Re: [PATCH 15/15] ethernet: ti: davinci_emac: add missing of_node_put after calling of_parse_phandle

[Mugunthan V N]

+ Linux Omap ML

On Wednesday 27 July 2016 01:13 PM, Peter Chen wrote:
>  
>> On Wednesday 27 July 2016 07:50 AM, Peter Chen wrote:
>>> of_node_put needs to be called when the device node which is got from
>>> of_parse_phandle has finished using.
>>>
>>> Signed-off-by: Peter Chen <peter.chen@nxp.com>
>>> ---
>>>  drivers/net/ethernet/ti/davinci_emac.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/net/ethernet/ti/davinci_emac.c
>>> b/drivers/net/ethernet/ti/davinci_emac.c
>>> index c6c5465..d8cb9d0 100644
>>> --- a/drivers/net/ethernet/ti/davinci_emac.c
>>> +++ b/drivers/net/ethernet/ti/davinci_emac.c
>>> @@ -1571,6 +1571,7 @@ static int emac_dev_open(struct net_device *ndev)
>>>  	if (priv->phy_node) {
>>>  		phydev = of_phy_connect(ndev, priv->phy_node,
>>>  					&emac_adjust_link, 0, 0);
>>> +		of_node_put(priv->phy_node);
>>>  		if (!phydev) {
>>>  			dev_err(emac_dev, "could not connect to phy %s\n",
>>>  				priv->phy_node->full_name);
>>>
>>
>> phy_node is accessed in case of of_phy_connect() returns error, so it has to be
>> moved after the dev_err log
>>
> 
> Yeah, you are right. I will change it, thanks.
> 

I see one more problem, when you stop and open the interface there is no
way to get the phy_node and interface will be unusable, so of_node_put()
should be moved to remove()

Regards
Mugunthan V N

Re: [PATCH 15/15] ethernet: ti: davinci_emac: add missing of_node_put after calling of_parse_phandle

[Mugunthan V N]

+ Linux Omap ML

On Wednesday 27 July 2016 01:13 PM, Peter Chen wrote:
>  
>> On Wednesday 27 July 2016 07:50 AM, Peter Chen wrote:
>>> of_node_put needs to be called when the device node which is got from
>>> of_parse_phandle has finished using.
>>>
>>> Signed-off-by: Peter Chen <peter.chen@nxp.com>
>>> ---
>>>  drivers/net/ethernet/ti/davinci_emac.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/net/ethernet/ti/davinci_emac.c
>>> b/drivers/net/ethernet/ti/davinci_emac.c
>>> index c6c5465..d8cb9d0 100644
>>> --- a/drivers/net/ethernet/ti/davinci_emac.c
>>> +++ b/drivers/net/ethernet/ti/davinci_emac.c
>>> @@ -1571,6 +1571,7 @@ static int emac_dev_open(struct net_device *ndev)
>>>  	if (priv->phy_node) {
>>>  		phydev = of_phy_connect(ndev, priv->phy_node,
>>>  					&emac_adjust_link, 0, 0);
>>> +		of_node_put(priv->phy_node);
>>>  		if (!phydev) {
>>>  			dev_err(emac_dev, "could not connect to phy %s\n",
>>>  				priv->phy_node->full_name);
>>>
>>
>> phy_node is accessed in case of of_phy_connect() returns error, so it has to be
>> moved after the dev_err log
>>
> 
> Yeah, you are right. I will change it, thanks.
> 

I see one more problem, when you stop and open the interface there is no
way to get the phy_node and interface will be unusable, so of_node_put()
should be moved to remove()

Regards
Mugunthan V N

RE: [PATCH 15/15] ethernet: ti: davinci_emac: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

 
>>
>>> On Wednesday 27 July 2016 07:50 AM, Peter Chen wrote:
>>>> of_node_put needs to be called when the device node which is got
>>>> from of_parse_phandle has finished using.
>>>>
>>>> Signed-off-by: Peter Chen <peter.chen@nxp.com>
>>>> ---
>>>>  drivers/net/ethernet/ti/davinci_emac.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/ti/davinci_emac.c
>>>> b/drivers/net/ethernet/ti/davinci_emac.c
>>>> index c6c5465..d8cb9d0 100644
>>>> --- a/drivers/net/ethernet/ti/davinci_emac.c
>>>> +++ b/drivers/net/ethernet/ti/davinci_emac.c
>>>> @@ -1571,6 +1571,7 @@ static int emac_dev_open(struct net_device *ndev)
>>>>  	if (priv->phy_node) {
>>>>  		phydev = of_phy_connect(ndev, priv->phy_node,
>>>>  					&emac_adjust_link, 0, 0);
>>>> +		of_node_put(priv->phy_node);
>>>>  		if (!phydev) {
>>>>  			dev_err(emac_dev, "could not connect to phy %s\n",
>>>>  				priv->phy_node->full_name);
>>>>
>>>
>>> phy_node is accessed in case of of_phy_connect() returns error, so it
>>> has to be moved after the dev_err log
>>>
>>
>> Yeah, you are right. I will change it, thanks.
>>
>
>I see one more problem, when you stop and open the interface there is no way to get
>the phy_node and interface will be unusable, so of_node_put() should be moved to
>remove()
>

It is my careless. Like I replied to David I did not know the phy_node would be used
in whole driver's life cycle. I will change it, thanks.

Peter

RE: [PATCH 15/15] ethernet: ti: davinci_emac: add missing of_node_put after calling of_parse_phandle

[Peter Chen]

 
>>
>>> On Wednesday 27 July 2016 07:50 AM, Peter Chen wrote:
>>>> of_node_put needs to be called when the device node which is got
>>>> from of_parse_phandle has finished using.
>>>>
>>>> Signed-off-by: Peter Chen <peter.chen@nxp.com>
>>>> ---
>>>>  drivers/net/ethernet/ti/davinci_emac.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/ti/davinci_emac.c
>>>> b/drivers/net/ethernet/ti/davinci_emac.c
>>>> index c6c5465..d8cb9d0 100644
>>>> --- a/drivers/net/ethernet/ti/davinci_emac.c
>>>> +++ b/drivers/net/ethernet/ti/davinci_emac.c
>>>> @@ -1571,6 +1571,7 @@ static int emac_dev_open(struct net_device *ndev)
>>>>  	if (priv->phy_node) {
>>>>  		phydev = of_phy_connect(ndev, priv->phy_node,
>>>>  					&emac_adjust_link, 0, 0);
>>>> +		of_node_put(priv->phy_node);
>>>>  		if (!phydev) {
>>>>  			dev_err(emac_dev, "could not connect to phy %s\n",
>>>>  				priv->phy_node->full_name);
>>>>
>>>
>>> phy_node is accessed in case of of_phy_connect() returns error, so it
>>> has to be moved after the dev_err log
>>>
>>
>> Yeah, you are right. I will change it, thanks.
>>
>
>I see one more problem, when you stop and open the interface there is no way to get
>the phy_node and interface will be unusable, so of_node_put() should be moved to
>remove()
>

It is my careless. Like I replied to David I did not know the phy_node would be used
in whole driver's life cycle. I will change it, thanks.

Peter