[PATCH v2] net: macb: add fixed-link node support

Subsystems: atmel macb ethernet driver, networking drivers, the rest

STALE3270d

13 messages, 4 authors, 2017-11-08 · open the first message on its own page

[PATCH v2] net: macb: add fixed-link node support

From: Michael Grzeschik <hidden>
Date: 2017-06-23 14:54:17 

In case the MACB is directly connected to a
non-mdio PHY/device, it should be possible to provide
a fixed link configuration in the DT.

Signed-off-by: Michael Grzeschik <redacted>
---
  v1 -> v2: - moved of_phy_connect case and phy_connect_direct into if else cases
            - moved of_pphy_register_fixed_link into if(np) case
	    - added of_node_put(bp->phy_node); into macb_remove
---
 drivers/net/ethernet/cadence/macb.c | 98 ++++++++++++++++++++++---------------
 drivers/net/ethernet/cadence/macb.h |  1 +
 2 files changed, 60 insertions(+), 39 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 91f7492623d3f..5781f1ede8c6e 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -425,32 +425,40 @@ static int macb_mii_probe(struct net_device *dev)
 	int phy_irq;
 	int ret;
 
-	phydev = phy_find_first(bp->mii_bus);
-	if (!phydev) {
-		netdev_err(dev, "no PHY found\n");
-		return -ENXIO;
-	}
+	if (bp->phy_node) {
+		phydev = of_phy_connect(dev, bp->phy_node,
+					&macb_handle_link_change, 0,
+					bp->phy_interface);
+		if (!phydev)
+			return -ENODEV;
+	} else {
+		phydev = phy_find_first(bp->mii_bus);
+		if (!phydev) {
+			netdev_err(dev, "no PHY found\n");
+			return -ENXIO;
+		}
 
-	pdata = dev_get_platdata(&bp->pdev->dev);
-	if (pdata) {
-		if (gpio_is_valid(pdata->phy_irq_pin)) {
-			ret = devm_gpio_request(&bp->pdev->dev,
-						pdata->phy_irq_pin, "phy int");
-			if (!ret) {
-				phy_irq = gpio_to_irq(pdata->phy_irq_pin);
-				phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
+		pdata = dev_get_platdata(&bp->pdev->dev);
+		if (pdata) {
+			if (gpio_is_valid(pdata->phy_irq_pin)) {
+				ret = devm_gpio_request(&bp->pdev->dev,
+							pdata->phy_irq_pin, "phy int");
+				if (!ret) {
+					phy_irq = gpio_to_irq(pdata->phy_irq_pin);
+					phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
+				}
+			} else {
+				phydev->irq = PHY_POLL;
 			}
-		} else {
-			phydev->irq = PHY_POLL;
 		}
-	}
 
-	/* attach the mac to the phy */
-	ret = phy_connect_direct(dev, phydev, &macb_handle_link_change,
-				 bp->phy_interface);
-	if (ret) {
-		netdev_err(dev, "Could not attach to PHY\n");
-		return ret;
+		/* attach the mac to the phy */
+		ret = phy_connect_direct(dev, phydev, &macb_handle_link_change,
+					 bp->phy_interface);
+		if (ret) {
+			netdev_err(dev, "Could not attach to PHY\n");
+			return ret;
+		}
 	}
 
 	/* mask with MAC supported features */
@@ -499,26 +507,37 @@ static int macb_mii_init(struct macb *bp)
 
 	np = bp->pdev->dev.of_node;
 	if (np) {
-		/* try dt phy registration */
-		err = of_mdiobus_register(bp->mii_bus, np);
+		if (of_phy_is_fixed_link(np)) {
+			if (of_phy_register_fixed_link(np) < 0) {
+				dev_err(&bp->pdev->dev,
+					"broken fixed-link specification\n");
+				goto err_out_unregister_bus;
+			}
+			bp->phy_node = of_node_get(np);
 
-		/* fallback to standard phy registration if no phy were
-		 * found during dt phy registration
-		 */
-		if (!err && !phy_find_first(bp->mii_bus)) {
-			for (i = 0; i < PHY_MAX_ADDR; i++) {
-				struct phy_device *phydev;
-
-				phydev = mdiobus_scan(bp->mii_bus, i);
-				if (IS_ERR(phydev) &&
-				    PTR_ERR(phydev) != -ENODEV) {
-					err = PTR_ERR(phydev);
-					break;
+			err = mdiobus_register(bp->mii_bus);
+		} else {
+			/* try dt phy registration */
+			err = of_mdiobus_register(bp->mii_bus, np);
+
+			/* fallback to standard phy registration if no phy were
+			 * found during dt phy registration
+			 */
+			if (!err && !phy_find_first(bp->mii_bus)) {
+				for (i = 0; i < PHY_MAX_ADDR; i++) {
+					struct phy_device *phydev;
+
+					phydev = mdiobus_scan(bp->mii_bus, i);
+					if (IS_ERR(phydev) &&
+					    PTR_ERR(phydev) != -ENODEV) {
+						err = PTR_ERR(phydev);
+						break;
+					}
 				}
-			}
 
-			if (err)
-				goto err_out_unregister_bus;
+				if (err)
+					goto err_out_unregister_bus;
+			}
 		}
 	} else {
 		for (i = 0; i < PHY_MAX_ADDR; i++)
@@ -3438,6 +3457,7 @@ static int macb_remove(struct platform_device *pdev)
 		clk_disable_unprepare(bp->hclk);
 		clk_disable_unprepare(bp->pclk);
 		clk_disable_unprepare(bp->rx_clk);
+		of_node_put(bp->phy_node);
 		free_netdev(dev);
 	}
 
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index ec037b0fa2a4d..2510661102bad 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -930,6 +930,7 @@ struct macb {
 	struct macb_or_gem_ops	macbgem_ops;
 
 	struct mii_bus		*mii_bus;
+	struct device_node	*phy_node;
 	int 			link;
 	int 			speed;
 	int 			duplex;
-- 
2.11.0

Re: [PATCH v2] net: macb: add fixed-link node support

From: David Miller <davem@davemloft.net>
Date: 2017-06-25 19:22:16 

From: Michael Grzeschik <redacted>
Date: Fri, 23 Jun 2017 16:54:10 +0200

In case the MACB is directly connected to a
non-mdio PHY/device, it should be possible to provide
a fixed link configuration in the DT.

Signed-off-by: Michael Grzeschik <redacted>
Applied, thanks.

Re: [PATCH v2] net: macb: add fixed-link node support

From: Nicolas Ferre <nicolas.ferre@microchip.com>
Date: 2017-11-06 09:26:53 

On 23/06/2017 at 16:54, Michael Grzeschik wrote:
In case the MACB is directly connected to a
non-mdio PHY/device, it should be possible to provide
a fixed link configuration in the DT.

Signed-off-by: Michael Grzeschik <redacted>
---
  v1 -> v2: - moved of_phy_connect case and phy_connect_direct into if else cases
            - moved of_pphy_register_fixed_link into if(np) case
	    - added of_node_put(bp->phy_node); into macb_remove
Hi Michael,

by re-scanning my email backlog, I found this remark that may apply to
your patch (already in Linus' tree actually)...
quoted hunk
---
 drivers/net/ethernet/cadence/macb.c | 98 ++++++++++++++++++++++---------------
 drivers/net/ethernet/cadence/macb.h |  1 +
 2 files changed, 60 insertions(+), 39 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 91f7492623d3f..5781f1ede8c6e 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -425,32 +425,40 @@ static int macb_mii_probe(struct net_device *dev)
 	int phy_irq;
 	int ret;
 
-	phydev = phy_find_first(bp->mii_bus);
-	if (!phydev) {
-		netdev_err(dev, "no PHY found\n");
-		return -ENXIO;
-	}
+	if (bp->phy_node) {
+		phydev = of_phy_connect(dev, bp->phy_node,
+					&macb_handle_link_change, 0,
+					bp->phy_interface);
+		if (!phydev)
+			return -ENODEV;
+	} else {
+		phydev = phy_find_first(bp->mii_bus);
+		if (!phydev) {
+			netdev_err(dev, "no PHY found\n");
+			return -ENXIO;
+		}
 
-	pdata = dev_get_platdata(&bp->pdev->dev);
-	if (pdata) {
-		if (gpio_is_valid(pdata->phy_irq_pin)) {
-			ret = devm_gpio_request(&bp->pdev->dev,
-						pdata->phy_irq_pin, "phy int");
-			if (!ret) {
-				phy_irq = gpio_to_irq(pdata->phy_irq_pin);
-				phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
+		pdata = dev_get_platdata(&bp->pdev->dev);
+		if (pdata) {
+			if (gpio_is_valid(pdata->phy_irq_pin)) {
+				ret = devm_gpio_request(&bp->pdev->dev,
+							pdata->phy_irq_pin, "phy int");
+				if (!ret) {
+					phy_irq = gpio_to_irq(pdata->phy_irq_pin);
+					phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
+				}
+			} else {
+				phydev->irq = PHY_POLL;
 			}
-		} else {
-			phydev->irq = PHY_POLL;
 		}
-	}
 
-	/* attach the mac to the phy */
-	ret = phy_connect_direct(dev, phydev, &macb_handle_link_change,
-				 bp->phy_interface);
-	if (ret) {
-		netdev_err(dev, "Could not attach to PHY\n");
-		return ret;
+		/* attach the mac to the phy */
+		ret = phy_connect_direct(dev, phydev, &macb_handle_link_change,
+					 bp->phy_interface);
+		if (ret) {
+			netdev_err(dev, "Could not attach to PHY\n");
+			return ret;
+		}
 	}
 
 	/* mask with MAC supported features */
@@ -499,26 +507,37 @@ static int macb_mii_init(struct macb *bp)
 
 	np = bp->pdev->dev.of_node;
 	if (np) {
-		/* try dt phy registration */
-		err = of_mdiobus_register(bp->mii_bus, np);
+		if (of_phy_is_fixed_link(np)) {
+			if (of_phy_register_fixed_link(np) < 0) {
remark done here by Andrew:
https://www.spinics.net/lists/netdev/msg421186.html

about missing of_phy_deregister_fixed_link().

Can you tell me if you would have time to have a look at it?


Thanks, best regards,
  Nicolas
quoted hunk
+				dev_err(&bp->pdev->dev,
+					"broken fixed-link specification\n");
+				goto err_out_unregister_bus;
+			}
+			bp->phy_node = of_node_get(np);
 
-		/* fallback to standard phy registration if no phy were
-		 * found during dt phy registration
-		 */
-		if (!err && !phy_find_first(bp->mii_bus)) {
-			for (i = 0; i < PHY_MAX_ADDR; i++) {
-				struct phy_device *phydev;
-
-				phydev = mdiobus_scan(bp->mii_bus, i);
-				if (IS_ERR(phydev) &&
-				    PTR_ERR(phydev) != -ENODEV) {
-					err = PTR_ERR(phydev);
-					break;
+			err = mdiobus_register(bp->mii_bus);
+		} else {
+			/* try dt phy registration */
+			err = of_mdiobus_register(bp->mii_bus, np);
+
+			/* fallback to standard phy registration if no phy were
+			 * found during dt phy registration
+			 */
+			if (!err && !phy_find_first(bp->mii_bus)) {
+				for (i = 0; i < PHY_MAX_ADDR; i++) {
+					struct phy_device *phydev;
+
+					phydev = mdiobus_scan(bp->mii_bus, i);
+					if (IS_ERR(phydev) &&
+					    PTR_ERR(phydev) != -ENODEV) {
+						err = PTR_ERR(phydev);
+						break;
+					}
 				}
-			}
 
-			if (err)
-				goto err_out_unregister_bus;
+				if (err)
+					goto err_out_unregister_bus;
+			}
 		}
 	} else {
 		for (i = 0; i < PHY_MAX_ADDR; i++)
@@ -3438,6 +3457,7 @@ static int macb_remove(struct platform_device *pdev)
 		clk_disable_unprepare(bp->hclk);
 		clk_disable_unprepare(bp->pclk);
 		clk_disable_unprepare(bp->rx_clk);
+		of_node_put(bp->phy_node);
 		free_netdev(dev);
 	}
 
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index ec037b0fa2a4d..2510661102bad 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -930,6 +930,7 @@ struct macb {
 	struct macb_or_gem_ops	macbgem_ops;
 
 	struct mii_bus		*mii_bus;
+	struct device_node	*phy_node;
 	int 			link;
 	int 			speed;
 	int 			duplex;

-- 
Nicolas Ferre

[PATCH 2/2] net: macb: add of_node_put to error paths

From: Michael Grzeschik <hidden>
Date: 2017-11-06 11:10:13 

We add the call of_node_put(bp->phy_node) to all associated error
paths for memory clean up.

Signed-off-by: Michael Grzeschik <redacted>
---
 drivers/net/ethernet/cadence/macb_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 2c2acd011329a..2698a1fde39c7 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -611,6 +611,7 @@ static int macb_mii_init(struct macb *bp)
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
 err_out_free_mdiobus:
+	of_node_put(bp->phy_node);
 	if ((np) && (of_phy_is_fixed_link(np)))
 		of_phy_deregister_fixed_link(np);
 	mdiobus_free(bp->mii_bus);
@@ -3554,6 +3555,7 @@ static int macb_probe(struct platform_device *pdev)
 err_out_unregister_mdio:
 	phy_disconnect(dev->phydev);
 	mdiobus_unregister(bp->mii_bus);
+	of_node_put(bp->phy_node);
 	if ((np) && (of_phy_is_fixed_link(np)))
 		of_phy_deregister_fixed_link(np);
 	mdiobus_free(bp->mii_bus);
@@ -3587,6 +3589,7 @@ static int macb_remove(struct platform_device *pdev)
 		if (dev->phydev)
 			phy_disconnect(dev->phydev);
 		mdiobus_unregister(bp->mii_bus);
+		of_node_put(bp->phy_node);
 		if ((np) && (of_phy_is_fixed_link(np)))
 			of_phy_deregister_fixed_link(np);
 		dev->phydev = NULL;
-- 
2.11.0

[PATCH 1/2] net: macb: add of_phy_deregister_fixed_link to error paths

From: Michael Grzeschik <hidden>
Date: 2017-11-06 11:10:13 

We add the call of_phy_deregister_fixed_link to all associated
error paths for memory clean up.

Signed-off-by: Michael Grzeschik <redacted>
---
 drivers/net/ethernet/cadence/macb_main.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 6df2cad61647a..2c2acd011329a 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -611,6 +611,8 @@ static int macb_mii_init(struct macb *bp)
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
 err_out_free_mdiobus:
+	if ((np) && (of_phy_is_fixed_link(np)))
+		of_phy_deregister_fixed_link(np);
 	mdiobus_free(bp->mii_bus);
 err_out:
 	return err;
@@ -3552,6 +3554,8 @@ static int macb_probe(struct platform_device *pdev)
 err_out_unregister_mdio:
 	phy_disconnect(dev->phydev);
 	mdiobus_unregister(bp->mii_bus);
+	if ((np) && (of_phy_is_fixed_link(np)))
+		of_phy_deregister_fixed_link(np);
 	mdiobus_free(bp->mii_bus);
 
 	/* Shutdown the PHY if there is a GPIO reset */
@@ -3574,6 +3578,7 @@ static int macb_remove(struct platform_device *pdev)
 {
 	struct net_device *dev;
 	struct macb *bp;
+	struct device_node *np = pdev->dev.of_node;
 
 	dev = platform_get_drvdata(pdev);
 
@@ -3582,6 +3587,8 @@ static int macb_remove(struct platform_device *pdev)
 		if (dev->phydev)
 			phy_disconnect(dev->phydev);
 		mdiobus_unregister(bp->mii_bus);
+		if ((np) && (of_phy_is_fixed_link(np)))
+			of_phy_deregister_fixed_link(np);
 		dev->phydev = NULL;
 		mdiobus_free(bp->mii_bus);
 
-- 
2.11.0

Re: [PATCH 1/2] net: macb: add of_phy_deregister_fixed_link to error paths

From: Nicolas Ferre <nicolas.ferre@microchip.com>
Date: 2017-11-06 14:37:25 

On 06/11/2017 at 12:10, Michael Grzeschik wrote:
We add the call of_phy_deregister_fixed_link to all associated
error paths for memory clean up.

Signed-off-by: Michael Grzeschik <redacted>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Thanks a lot for your quick answer!

Best regards,
  Nicolas
quoted hunk
---
 drivers/net/ethernet/cadence/macb_main.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 6df2cad61647a..2c2acd011329a 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -611,6 +611,8 @@ static int macb_mii_init(struct macb *bp)
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
 err_out_free_mdiobus:
+	if ((np) && (of_phy_is_fixed_link(np)))
+		of_phy_deregister_fixed_link(np);
 	mdiobus_free(bp->mii_bus);
 err_out:
 	return err;
@@ -3552,6 +3554,8 @@ static int macb_probe(struct platform_device *pdev)
 err_out_unregister_mdio:
 	phy_disconnect(dev->phydev);
 	mdiobus_unregister(bp->mii_bus);
+	if ((np) && (of_phy_is_fixed_link(np)))
+		of_phy_deregister_fixed_link(np);
 	mdiobus_free(bp->mii_bus);
 
 	/* Shutdown the PHY if there is a GPIO reset */
@@ -3574,6 +3578,7 @@ static int macb_remove(struct platform_device *pdev)
 {
 	struct net_device *dev;
 	struct macb *bp;
+	struct device_node *np = pdev->dev.of_node;
 
 	dev = platform_get_drvdata(pdev);
 
@@ -3582,6 +3587,8 @@ static int macb_remove(struct platform_device *pdev)
 		if (dev->phydev)
 			phy_disconnect(dev->phydev);
 		mdiobus_unregister(bp->mii_bus);
+		if ((np) && (of_phy_is_fixed_link(np)))
+			of_phy_deregister_fixed_link(np);
 		dev->phydev = NULL;
 		mdiobus_free(bp->mii_bus);
 

-- 
Nicolas Ferre

Re: [PATCH 2/2] net: macb: add of_node_put to error paths

From: Nicolas Ferre <nicolas.ferre@microchip.com>
Date: 2017-11-07 09:27:29 

On 06/11/2017 at 12:10, Michael Grzeschik wrote:
quoted hunk
We add the call of_node_put(bp->phy_node) to all associated error
paths for memory clean up.

Signed-off-by: Michael Grzeschik <redacted>
---
 drivers/net/ethernet/cadence/macb_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 2c2acd011329a..2698a1fde39c7 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -611,6 +611,7 @@ static int macb_mii_init(struct macb *bp)
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
 err_out_free_mdiobus:
+	of_node_put(bp->phy_node);
okay
quoted hunk
 	if ((np) && (of_phy_is_fixed_link(np)))
 		of_phy_deregister_fixed_link(np);
 	mdiobus_free(bp->mii_bus);
@@ -3554,6 +3555,7 @@ static int macb_probe(struct platform_device *pdev)
 err_out_unregister_mdio:
 	phy_disconnect(dev->phydev);
 	mdiobus_unregister(bp->mii_bus);
+	of_node_put(bp->phy_node);
yes
quoted hunk
 	if ((np) && (of_phy_is_fixed_link(np)))
 		of_phy_deregister_fixed_link(np);
 	mdiobus_free(bp->mii_bus);
@@ -3587,6 +3589,7 @@ static int macb_remove(struct platform_device *pdev)
 		if (dev->phydev)
 			phy_disconnect(dev->phydev);
 		mdiobus_unregister(bp->mii_bus);
+		of_node_put(bp->phy_node);
Isn't this call already done some lines below, just before
"free_netdev(dev);"?


 		if ((np) && (of_phy_is_fixed_link(np)))
 			of_phy_deregister_fixed_link(np);
 		dev->phydev = NULL;
Thanks for your patch.

Regards,
-- 
Nicolas Ferre

Re: [PATCH 2/2] net: macb: add of_node_put to error paths

From: Michael Grzeschik <hidden>
Date: 2017-11-07 10:00:47 

On Tue, Nov 07, 2017 at 10:27:20AM +0100, Nicolas Ferre wrote:
On 06/11/2017 at 12:10, Michael Grzeschik wrote:
quoted
We add the call of_node_put(bp->phy_node) to all associated error
paths for memory clean up.

Signed-off-by: Michael Grzeschik <redacted>
---
 drivers/net/ethernet/cadence/macb_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 2c2acd011329a..2698a1fde39c7 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -611,6 +611,7 @@ static int macb_mii_init(struct macb *bp)
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
 err_out_free_mdiobus:
+	of_node_put(bp->phy_node);
okay
quoted
 	if ((np) && (of_phy_is_fixed_link(np)))
 		of_phy_deregister_fixed_link(np);
 	mdiobus_free(bp->mii_bus);
@@ -3554,6 +3555,7 @@ static int macb_probe(struct platform_device *pdev)
 err_out_unregister_mdio:
 	phy_disconnect(dev->phydev);
 	mdiobus_unregister(bp->mii_bus);
+	of_node_put(bp->phy_node);
yes
quoted
 	if ((np) && (of_phy_is_fixed_link(np)))
 		of_phy_deregister_fixed_link(np);
 	mdiobus_free(bp->mii_bus);
@@ -3587,6 +3589,7 @@ static int macb_remove(struct platform_device *pdev)
 		if (dev->phydev)
 			phy_disconnect(dev->phydev);
 		mdiobus_unregister(bp->mii_bus);
+		of_node_put(bp->phy_node);
Isn't this call already done some lines below, just before
"free_netdev(dev);"?
You are absolutely right. Will send v2.
quoted
 		if ((np) && (of_phy_is_fixed_link(np)))
 			of_phy_deregister_fixed_link(np);
 		dev->phydev = NULL;
Thanks for your patch.
Thanks,
Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v2] net: macb: add of_node_put to error paths

From: Michael Grzeschik <hidden>
Date: 2017-11-07 10:00:50 

We add the call of_node_put(bp->phy_node) to all associated error
paths for memory clean up.

Signed-off-by: Michael Grzeschik <redacted>
---
v1 -> v2: removed extra of_node_put from macb_remove

 drivers/net/ethernet/cadence/macb_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 2c2acd011329a..c66df096e81a5 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -611,6 +611,7 @@ static int macb_mii_init(struct macb *bp)
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
 err_out_free_mdiobus:
+	of_node_put(bp->phy_node);
 	if ((np) && (of_phy_is_fixed_link(np)))
 		of_phy_deregister_fixed_link(np);
 	mdiobus_free(bp->mii_bus);
@@ -3554,6 +3555,7 @@ static int macb_probe(struct platform_device *pdev)
 err_out_unregister_mdio:
 	phy_disconnect(dev->phydev);
 	mdiobus_unregister(bp->mii_bus);
+	of_node_put(bp->phy_node);
 	if ((np) && (of_phy_is_fixed_link(np)))
 		of_phy_deregister_fixed_link(np);
 	mdiobus_free(bp->mii_bus);
-- 
2.11.0

Re: [PATCH v2] net: macb: add of_node_put to error paths

From: Nicolas Ferre <nicolas.ferre@microchip.com>
Date: 2017-11-07 11:12:29 

On 07/11/2017 at 10:59, Michael Grzeschik wrote:
We add the call of_node_put(bp->phy_node) to all associated error
paths for memory clean up.

Signed-off-by: Michael Grzeschik <redacted>
Thanks for your quick update:

Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Best regards,
  Nicolas
quoted hunk
---
v1 -> v2: removed extra of_node_put from macb_remove

 drivers/net/ethernet/cadence/macb_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 2c2acd011329a..c66df096e81a5 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -611,6 +611,7 @@ static int macb_mii_init(struct macb *bp)
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
 err_out_free_mdiobus:
+	of_node_put(bp->phy_node);
 	if ((np) && (of_phy_is_fixed_link(np)))
 		of_phy_deregister_fixed_link(np);
 	mdiobus_free(bp->mii_bus);
@@ -3554,6 +3555,7 @@ static int macb_probe(struct platform_device *pdev)
 err_out_unregister_mdio:
 	phy_disconnect(dev->phydev);
 	mdiobus_unregister(bp->mii_bus);
+	of_node_put(bp->phy_node);
 	if ((np) && (of_phy_is_fixed_link(np)))
 		of_phy_deregister_fixed_link(np);
 	mdiobus_free(bp->mii_bus);

-- 
Nicolas Ferre

Re: [PATCH 1/2] net: macb: add of_phy_deregister_fixed_link to error paths

From: David Miller <davem@davemloft.net>
Date: 2017-11-08 04:23:02 

From: Michael Grzeschik <redacted>
Date: Mon,  6 Nov 2017 12:10:04 +0100
quoted hunk
We add the call of_phy_deregister_fixed_link to all associated
error paths for memory clean up.

Signed-off-by: Michael Grzeschik <redacted>
---
 drivers/net/ethernet/cadence/macb_main.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 6df2cad61647a..2c2acd011329a 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -611,6 +611,8 @@ static int macb_mii_init(struct macb *bp)
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
 err_out_free_mdiobus:
+	if ((np) && (of_phy_is_fixed_link(np)))
Please don't use so many parenthesis in your conditionals:

	if (np && of_phy_is_fixed_link(np))

is more than sufficient.

Please fix this in your entire set of patches.

Thank you.

Re: [PATCH v2] net: macb: add of_node_put to error paths

From: David Miller <davem@davemloft.net>
Date: 2017-11-08 06:29:30 

From: Michael Grzeschik <redacted>
Date: Tue,  7 Nov 2017 10:59:49 +0100
quoted hunk
@@ -611,6 +611,7 @@ static int macb_mii_init(struct macb *bp)
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
 err_out_free_mdiobus:
+	of_node_put(bp->phy_node);
 	if ((np) && (of_phy_is_fixed_link(np)))
 		of_phy_deregister_fixed_link(np);
 	mdiobus_free(bp->mii_bus);
This is relative to another patch which has not been applied.

Please do not do this.

Re: [PATCH 1/2] net: macb: add of_phy_deregister_fixed_link to error paths

From: Michael Grzeschik <hidden>
Date: 2017-11-08 08:41:35 

On Wed, Nov 08, 2017 at 01:22:57PM +0900, David Miller wrote:
From: Michael Grzeschik <redacted>
Date: Mon,  6 Nov 2017 12:10:04 +0100
quoted
We add the call of_phy_deregister_fixed_link to all associated
error paths for memory clean up.

Signed-off-by: Michael Grzeschik <redacted>
---
 drivers/net/ethernet/cadence/macb_main.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 6df2cad61647a..2c2acd011329a 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -611,6 +611,8 @@ static int macb_mii_init(struct macb *bp)
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
 err_out_free_mdiobus:
+	if ((np) && (of_phy_is_fixed_link(np)))
Please don't use so many parenthesis in your conditionals:

	if (np && of_phy_is_fixed_link(np))

is more than sufficient.

Please fix this in your entire set of patches.
There are only two patches and not even in one series.
I will resend them both with this one fixed and create
a series. As the second one depends on this one.

Thanks,
Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help