[PATCH 0/5] sh_eth: Fix the race between open and MDIO bus registration

[PATCH 0/5] sh_eth: Fix the race between open and MDIO bus registration

[Laurent Pinchart]

Hello,

This patch series fixes the race condition that exists in the sh_eth driver
between network device open and MDIO bus registration. The actual fix is in
patch 4/5, with previous patches preparing the driver and patch 5/5 cleaning
up an unrelated issue.

I've based the idea on Sergei's attempt to fix the problem and can successfully
boot the Koelsch board over NFS with this series. I might have missed other
issues though, hence the RFC status.

The patches are based on top of the latest net-next master branch.

Laurent Pinchart (5):
  sh_eth: Use the platform device for memory allocation
  sh_eth: Use the platform device as the MDIO bus parent
  sh_eth: Simplify MDIO bus initialization and release
  sh_eth: Register MDIO bus before registering the network device
  sh_eth: Remove goto statements that jump straight to a return

 drivers/net/ethernet/renesas/sh_eth.c | 85 +++++++++++++----------------------
 1 file changed, 31 insertions(+), 54 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH 5/5] sh_eth: Remove goto statements that jump straight to a return

[Laurent Pinchart]

"goto" is well accepted for error paths in the kernel but should not be
used unnecessarily. Return the correct value directly instead of using a
goto when possible.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/net/ethernet/renesas/sh_eth.c | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index ace6da2..e4bff18 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -873,7 +873,7 @@ static int sh_eth_reset(struct net_device *ndev)
 
 		ret = sh_eth_check_reset(ndev);
 		if (ret)
-			goto out;
+			return ret;
 
 		/* Table Init */
 		sh_eth_write(ndev, 0x0, TDLAR);
@@ -900,7 +900,6 @@ static int sh_eth_reset(struct net_device *ndev)
 			     EDMR);
 	}
 
-out:
 	return ret;
 }
 
@@ -1264,7 +1263,7 @@ static int sh_eth_dev_init(struct net_device *ndev, bool start)
 	/* Soft Reset */
 	ret = sh_eth_reset(ndev);
 	if (ret)
-		goto out;
+		return ret;
 
 	if (mdp->cd->rmiimode)
 		sh_eth_write(ndev, 0x1, RMIIMODE);
@@ -1343,7 +1342,6 @@ static int sh_eth_dev_init(struct net_device *ndev, bool start)
 		netif_start_queue(ndev);
 	}
 
-out:
 	return ret;
 }
 
@@ -2605,10 +2603,8 @@ static int sh_mdio_init(struct sh_eth_private *mdp,
 
 	/* create bit control struct for PHY */
 	bitbang = devm_kzalloc(dev, sizeof(struct bb_info), GFP_KERNEL);
-	if (!bitbang) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!bitbang)
+		return -ENOMEM;
 
 	/* bitbang init */
 	bitbang->addr = mdp->addr + mdp->reg_offset[PIR];
@@ -2621,10 +2617,8 @@ static int sh_mdio_init(struct sh_eth_private *mdp,
 
 	/* MII controller setting */
 	mdp->mii_bus = alloc_mdio_bitbang(&bitbang->ctrl);
-	if (!mdp->mii_bus) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!mdp->mii_bus)
+		return -ENOMEM;
 
 	/* Hook up MII support for ethtool */
 	mdp->mii_bus->name = "sh_mii";
@@ -2659,8 +2653,6 @@ static int sh_mdio_init(struct sh_eth_private *mdp,
 
 out_free_bus:
 	free_mdio_bitbang(mdp->mii_bus);
-
-out:
 	return ret;
 }
 
@@ -2773,15 +2765,12 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (unlikely(res = NULL)) {
 		dev_err(&pdev->dev, "invalid resource\n");
-		ret = -EINVAL;
-		goto out;
+		return -EINVAL;
 	}
 
 	ndev = alloc_etherdev(sizeof(struct sh_eth_private));
-	if (!ndev) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!ndev)
+		return -ENOMEM;
 
 	/* The sh Ether-specific entries in the device structure. */
 	ndev->base_addr = res->start;
@@ -2922,7 +2911,6 @@ out_release:
 	if (ndev)
 		free_netdev(ndev);
 
-out:
 	return ret;
 }
 
-- 
1.8.3.2

[PATCH 4/5] sh_eth: Register MDIO bus before registering the network device

[Laurent Pinchart]

Network API functions that rely on the MDIO bus can be called as soon as
the driver calls register_netdev(). Register the MDIO bus before the
network device to avoid race conditions.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/net/ethernet/renesas/sh_eth.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index e9224f2..ace6da2 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2891,6 +2891,13 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* MDIO bus init */
+	ret = sh_mdio_init(mdp, pd);
+	if (ret) {
+		dev_err(&ndev->dev, "failed to initialise MDIO\n");
+		goto out_release;
+	}
+
 	netif_napi_add(ndev, &mdp->napi, sh_eth_poll, 64);
 
 	/* network device register */
@@ -2898,13 +2905,6 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
 	if (ret)
 		goto out_napi_del;
 
-	/* mdio bus init */
-	ret = sh_mdio_init(mdp, pd);
-	if (ret) {
-		dev_err(&ndev->dev, "failed to initialise MDIO\n");
-		goto out_unregister;
-	}
-
 	/* print device information */
 	netdev_info(ndev, "Base address at 0x%x, %pM, IRQ %d.\n",
 		    (u32)ndev->base_addr, ndev->dev_addr, ndev->irq);
@@ -2913,11 +2913,9 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
 
 	return ret;
 
-out_unregister:
-	unregister_netdev(ndev);
-
 out_napi_del:
 	netif_napi_del(&mdp->napi);
+	sh_mdio_release(mdp);
 
 out_release:
 	/* net_dev free */
@@ -2933,9 +2931,9 @@ static int sh_eth_drv_remove(struct platform_device *pdev)
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct sh_eth_private *mdp = netdev_priv(ndev);
 
-	sh_mdio_release(mdp);
 	unregister_netdev(ndev);
 	netif_napi_del(&mdp->napi);
+	sh_mdio_release(mdp);
 	pm_runtime_disable(&pdev->dev);
 	free_netdev(ndev);
 
-- 
1.8.3.2

[PATCH 1/5] sh_eth: Use the platform device for memory allocation

[Laurent Pinchart]

Memory allocated for the MDIO bus with the devm_kzalloc() API is
associated with the network device. While this will cause memory to be
freed at the right time, it doesn't allow allocating memory before the
network device is initialized.

Replace the network device with the parent platform device for memory
allocation to remove that dependency. This also improves consistency
with the other devm_* calls in the driver that all use the platform
device.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/net/ethernet/renesas/sh_eth.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index efaca6d..f669e2a 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2606,10 +2606,10 @@ static int sh_mdio_init(struct net_device *ndev, int id,
 	int ret, i;
 	struct bb_info *bitbang;
 	struct sh_eth_private *mdp = netdev_priv(ndev);
+	struct device *dev = &mdp->pdev->dev;
 
 	/* create bit control struct for PHY */
-	bitbang = devm_kzalloc(&ndev->dev, sizeof(struct bb_info),
-			       GFP_KERNEL);
+	bitbang = devm_kzalloc(dev, sizeof(struct bb_info), GFP_KERNEL);
 	if (!bitbang) {
 		ret = -ENOMEM;
 		goto out;
@@ -2638,8 +2638,7 @@ static int sh_mdio_init(struct net_device *ndev, int id,
 		 mdp->pdev->name, id);
 
 	/* PHY IRQ */
-	mdp->mii_bus->irq = devm_kzalloc(&ndev->dev,
-					 sizeof(int) * PHY_MAX_ADDR,
+	mdp->mii_bus->irq = devm_kzalloc(dev, sizeof(int) * PHY_MAX_ADDR,
 					 GFP_KERNEL);
 	if (!mdp->mii_bus->irq) {
 		ret = -ENOMEM;
-- 
1.8.3.2

[PATCH 2/5] sh_eth: Use the platform device as the MDIO bus parent

[Laurent Pinchart]

The MDIO bus parent is set to the network device. Beside not reflecting
the hardware topology, this prevents registering the MDIO bus before
initializing the network device. Fix it by setting the MDIO bus parent
to the platform device.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/net/ethernet/renesas/sh_eth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index f669e2a..443f14f 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2633,7 +2633,7 @@ static int sh_mdio_init(struct net_device *ndev, int id,
 
 	/* Hook up MII support for ethtool */
 	mdp->mii_bus->name = "sh_mii";
-	mdp->mii_bus->parent = &ndev->dev;
+	mdp->mii_bus->parent = dev;
 	snprintf(mdp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
 		 mdp->pdev->name, id);
 
-- 
1.8.3.2

[PATCH 3/5] sh_eth: Simplify MDIO bus initialization and release

[Laurent Pinchart]

The network device passed to the sh_mdio_init and sh_mdio_release
functions is only used to access the sh_eth_private instance. Pass it
directly to those functions.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/net/ethernet/renesas/sh_eth.c | 30 +++++++++++-------------------
 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 443f14f..e9224f2 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2583,29 +2583,24 @@ static void sh_eth_tsu_init(struct sh_eth_private *mdp)
 }
 
 /* MDIO bus release function */
-static int sh_mdio_release(struct net_device *ndev)
+static int sh_mdio_release(struct sh_eth_private *mdp)
 {
-	struct mii_bus *bus = dev_get_drvdata(&ndev->dev);
-
 	/* unregister mdio bus */
-	mdiobus_unregister(bus);
-
-	/* remove mdio bus info from net_device */
-	dev_set_drvdata(&ndev->dev, NULL);
+	mdiobus_unregister(mdp->mii_bus);
 
 	/* free bitbang info */
-	free_mdio_bitbang(bus);
+	free_mdio_bitbang(mdp->mii_bus);
 
 	return 0;
 }
 
 /* MDIO bus init function */
-static int sh_mdio_init(struct net_device *ndev, int id,
+static int sh_mdio_init(struct sh_eth_private *mdp,
 			struct sh_eth_plat_data *pd)
 {
 	int ret, i;
 	struct bb_info *bitbang;
-	struct sh_eth_private *mdp = netdev_priv(ndev);
+	struct platform_device *pdev = mdp->pdev;
 	struct device *dev = &mdp->pdev->dev;
 
 	/* create bit control struct for PHY */
@@ -2635,7 +2630,7 @@ static int sh_mdio_init(struct net_device *ndev, int id,
 	mdp->mii_bus->name = "sh_mii";
 	mdp->mii_bus->parent = dev;
 	snprintf(mdp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
-		 mdp->pdev->name, id);
+		 pdev->name, pdev->id);
 
 	/* PHY IRQ */
 	mdp->mii_bus->irq = devm_kzalloc(dev, sizeof(int) * PHY_MAX_ADDR,
@@ -2645,10 +2640,9 @@ static int sh_mdio_init(struct net_device *ndev, int id,
 		goto out_free_bus;
 	}
 
-	/* register mdio bus */
-	if (ndev->dev.parent->of_node) {
-		ret = of_mdiobus_register(mdp->mii_bus,
-					  ndev->dev.parent->of_node);
+	/* register MDIO bus */
+	if (dev->of_node) {
+		ret = of_mdiobus_register(mdp->mii_bus, dev->of_node);
 	} else {
 		for (i = 0; i < PHY_MAX_ADDR; i++)
 			mdp->mii_bus->irq[i] = PHY_POLL;
@@ -2661,8 +2655,6 @@ static int sh_mdio_init(struct net_device *ndev, int id,
 	if (ret)
 		goto out_free_bus;
 
-	dev_set_drvdata(&ndev->dev, mdp->mii_bus);
-
 	return 0;
 
 out_free_bus:
@@ -2907,7 +2899,7 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
 		goto out_napi_del;
 
 	/* mdio bus init */
-	ret = sh_mdio_init(ndev, pdev->id, pd);
+	ret = sh_mdio_init(mdp, pd);
 	if (ret) {
 		dev_err(&ndev->dev, "failed to initialise MDIO\n");
 		goto out_unregister;
@@ -2941,7 +2933,7 @@ static int sh_eth_drv_remove(struct platform_device *pdev)
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct sh_eth_private *mdp = netdev_priv(ndev);
 
-	sh_mdio_release(ndev);
+	sh_mdio_release(mdp);
 	unregister_netdev(ndev);
 	netif_napi_del(&mdp->napi);
 	pm_runtime_disable(&pdev->dev);
-- 
1.8.3.2

[PATCH 0/5] sh_eth: Fix the race between open and MDIO bus registration

[Laurent Pinchart]

Hello,

This patch series fixes the race condition that exists in the sh_eth driver
between network device open and MDIO bus registration. The actual fix is in
patch 4/5, with previous patches preparing the driver and patch 5/5 cleaning
up an unrelated issue.

I've based the idea on Sergei's attempt to fix the problem and can successfully
boot the Koelsch board over NFS with this series. I might have missed other
issues though, hence the RFC status.

The patches are based on top of the latest net-next master branch.

Laurent Pinchart (5):
  sh_eth: Use the platform device for memory allocation
  sh_eth: Use the platform device as the MDIO bus parent
  sh_eth: Simplify MDIO bus initialization and release
  sh_eth: Register MDIO bus before registering the network device
  sh_eth: Remove goto statements that jump straight to a return

 drivers/net/ethernet/renesas/sh_eth.c | 85 +++++++++++++----------------------
 1 file changed, 31 insertions(+), 54 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH 1/5] sh_eth: Use the platform device for memory allocation

[Laurent Pinchart]

Memory allocated for the MDIO bus with the devm_kzalloc() API is
associated with the network device. While this will cause memory to be
freed at the right time, it doesn't allow allocating memory before the
network device is initialized.

Replace the network device with the parent platform device for memory
allocation to remove that dependency. This also improves consistency
with the other devm_* calls in the driver that all use the platform
device.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/net/ethernet/renesas/sh_eth.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index efaca6d..f669e2a 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2606,10 +2606,10 @@ static int sh_mdio_init(struct net_device *ndev, int id,
 	int ret, i;
 	struct bb_info *bitbang;
 	struct sh_eth_private *mdp = netdev_priv(ndev);
+	struct device *dev = &mdp->pdev->dev;
 
 	/* create bit control struct for PHY */
-	bitbang = devm_kzalloc(&ndev->dev, sizeof(struct bb_info),
-			       GFP_KERNEL);
+	bitbang = devm_kzalloc(dev, sizeof(struct bb_info), GFP_KERNEL);
 	if (!bitbang) {
 		ret = -ENOMEM;
 		goto out;
@@ -2638,8 +2638,7 @@ static int sh_mdio_init(struct net_device *ndev, int id,
 		 mdp->pdev->name, id);
 
 	/* PHY IRQ */
-	mdp->mii_bus->irq = devm_kzalloc(&ndev->dev,
-					 sizeof(int) * PHY_MAX_ADDR,
+	mdp->mii_bus->irq = devm_kzalloc(dev, sizeof(int) * PHY_MAX_ADDR,
 					 GFP_KERNEL);
 	if (!mdp->mii_bus->irq) {
 		ret = -ENOMEM;
-- 
1.8.3.2

[PATCH 2/5] sh_eth: Use the platform device as the MDIO bus parent

[Laurent Pinchart]

The MDIO bus parent is set to the network device. Beside not reflecting
the hardware topology, this prevents registering the MDIO bus before
initializing the network device. Fix it by setting the MDIO bus parent
to the platform device.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/net/ethernet/renesas/sh_eth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index f669e2a..443f14f 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2633,7 +2633,7 @@ static int sh_mdio_init(struct net_device *ndev, int id,
 
 	/* Hook up MII support for ethtool */
 	mdp->mii_bus->name = "sh_mii";
-	mdp->mii_bus->parent = &ndev->dev;
+	mdp->mii_bus->parent = dev;
 	snprintf(mdp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
 		 mdp->pdev->name, id);
 
-- 
1.8.3.2

[PATCH 3/5] sh_eth: Simplify MDIO bus initialization and release

[Laurent Pinchart]

The network device passed to the sh_mdio_init and sh_mdio_release
functions is only used to access the sh_eth_private instance. Pass it
directly to those functions.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/net/ethernet/renesas/sh_eth.c | 30 +++++++++++-------------------
 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 443f14f..e9224f2 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2583,29 +2583,24 @@ static void sh_eth_tsu_init(struct sh_eth_private *mdp)
 }
 
 /* MDIO bus release function */
-static int sh_mdio_release(struct net_device *ndev)
+static int sh_mdio_release(struct sh_eth_private *mdp)
 {
-	struct mii_bus *bus = dev_get_drvdata(&ndev->dev);
-
 	/* unregister mdio bus */
-	mdiobus_unregister(bus);
-
-	/* remove mdio bus info from net_device */
-	dev_set_drvdata(&ndev->dev, NULL);
+	mdiobus_unregister(mdp->mii_bus);
 
 	/* free bitbang info */
-	free_mdio_bitbang(bus);
+	free_mdio_bitbang(mdp->mii_bus);
 
 	return 0;
 }
 
 /* MDIO bus init function */
-static int sh_mdio_init(struct net_device *ndev, int id,
+static int sh_mdio_init(struct sh_eth_private *mdp,
 			struct sh_eth_plat_data *pd)
 {
 	int ret, i;
 	struct bb_info *bitbang;
-	struct sh_eth_private *mdp = netdev_priv(ndev);
+	struct platform_device *pdev = mdp->pdev;
 	struct device *dev = &mdp->pdev->dev;
 
 	/* create bit control struct for PHY */
@@ -2635,7 +2630,7 @@ static int sh_mdio_init(struct net_device *ndev, int id,
 	mdp->mii_bus->name = "sh_mii";
 	mdp->mii_bus->parent = dev;
 	snprintf(mdp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
-		 mdp->pdev->name, id);
+		 pdev->name, pdev->id);
 
 	/* PHY IRQ */
 	mdp->mii_bus->irq = devm_kzalloc(dev, sizeof(int) * PHY_MAX_ADDR,
@@ -2645,10 +2640,9 @@ static int sh_mdio_init(struct net_device *ndev, int id,
 		goto out_free_bus;
 	}
 
-	/* register mdio bus */
-	if (ndev->dev.parent->of_node) {
-		ret = of_mdiobus_register(mdp->mii_bus,
-					  ndev->dev.parent->of_node);
+	/* register MDIO bus */
+	if (dev->of_node) {
+		ret = of_mdiobus_register(mdp->mii_bus, dev->of_node);
 	} else {
 		for (i = 0; i < PHY_MAX_ADDR; i++)
 			mdp->mii_bus->irq[i] = PHY_POLL;
@@ -2661,8 +2655,6 @@ static int sh_mdio_init(struct net_device *ndev, int id,
 	if (ret)
 		goto out_free_bus;
 
-	dev_set_drvdata(&ndev->dev, mdp->mii_bus);
-
 	return 0;
 
 out_free_bus:
@@ -2907,7 +2899,7 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
 		goto out_napi_del;
 
 	/* mdio bus init */
-	ret = sh_mdio_init(ndev, pdev->id, pd);
+	ret = sh_mdio_init(mdp, pd);
 	if (ret) {
 		dev_err(&ndev->dev, "failed to initialise MDIO\n");
 		goto out_unregister;
@@ -2941,7 +2933,7 @@ static int sh_eth_drv_remove(struct platform_device *pdev)
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct sh_eth_private *mdp = netdev_priv(ndev);
 
-	sh_mdio_release(ndev);
+	sh_mdio_release(mdp);
 	unregister_netdev(ndev);
 	netif_napi_del(&mdp->napi);
 	pm_runtime_disable(&pdev->dev);
-- 
1.8.3.2

[PATCH 4/5] sh_eth: Register MDIO bus before registering the network device

[Laurent Pinchart]

Network API functions that rely on the MDIO bus can be called as soon as
the driver calls register_netdev(). Register the MDIO bus before the
network device to avoid race conditions.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/net/ethernet/renesas/sh_eth.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index e9224f2..ace6da2 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2891,6 +2891,13 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* MDIO bus init */
+	ret = sh_mdio_init(mdp, pd);
+	if (ret) {
+		dev_err(&ndev->dev, "failed to initialise MDIO\n");
+		goto out_release;
+	}
+
 	netif_napi_add(ndev, &mdp->napi, sh_eth_poll, 64);
 
 	/* network device register */
@@ -2898,13 +2905,6 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
 	if (ret)
 		goto out_napi_del;
 
-	/* mdio bus init */
-	ret = sh_mdio_init(mdp, pd);
-	if (ret) {
-		dev_err(&ndev->dev, "failed to initialise MDIO\n");
-		goto out_unregister;
-	}
-
 	/* print device information */
 	netdev_info(ndev, "Base address at 0x%x, %pM, IRQ %d.\n",
 		    (u32)ndev->base_addr, ndev->dev_addr, ndev->irq);
@@ -2913,11 +2913,9 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
 
 	return ret;
 
-out_unregister:
-	unregister_netdev(ndev);
-
 out_napi_del:
 	netif_napi_del(&mdp->napi);
+	sh_mdio_release(mdp);
 
 out_release:
 	/* net_dev free */
@@ -2933,9 +2931,9 @@ static int sh_eth_drv_remove(struct platform_device *pdev)
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct sh_eth_private *mdp = netdev_priv(ndev);
 
-	sh_mdio_release(mdp);
 	unregister_netdev(ndev);
 	netif_napi_del(&mdp->napi);
+	sh_mdio_release(mdp);
 	pm_runtime_disable(&pdev->dev);
 	free_netdev(ndev);
 
-- 
1.8.3.2

[PATCH 5/5] sh_eth: Remove goto statements that jump straight to a return

[Laurent Pinchart]

"goto" is well accepted for error paths in the kernel but should not be
used unnecessarily. Return the correct value directly instead of using a
goto when possible.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/net/ethernet/renesas/sh_eth.c | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index ace6da2..e4bff18 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -873,7 +873,7 @@ static int sh_eth_reset(struct net_device *ndev)
 
 		ret = sh_eth_check_reset(ndev);
 		if (ret)
-			goto out;
+			return ret;
 
 		/* Table Init */
 		sh_eth_write(ndev, 0x0, TDLAR);
@@ -900,7 +900,6 @@ static int sh_eth_reset(struct net_device *ndev)
 			     EDMR);
 	}
 
-out:
 	return ret;
 }
 
@@ -1264,7 +1263,7 @@ static int sh_eth_dev_init(struct net_device *ndev, bool start)
 	/* Soft Reset */
 	ret = sh_eth_reset(ndev);
 	if (ret)
-		goto out;
+		return ret;
 
 	if (mdp->cd->rmiimode)
 		sh_eth_write(ndev, 0x1, RMIIMODE);
@@ -1343,7 +1342,6 @@ static int sh_eth_dev_init(struct net_device *ndev, bool start)
 		netif_start_queue(ndev);
 	}
 
-out:
 	return ret;
 }
 
@@ -2605,10 +2603,8 @@ static int sh_mdio_init(struct sh_eth_private *mdp,
 
 	/* create bit control struct for PHY */
 	bitbang = devm_kzalloc(dev, sizeof(struct bb_info), GFP_KERNEL);
-	if (!bitbang) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!bitbang)
+		return -ENOMEM;
 
 	/* bitbang init */
 	bitbang->addr = mdp->addr + mdp->reg_offset[PIR];
@@ -2621,10 +2617,8 @@ static int sh_mdio_init(struct sh_eth_private *mdp,
 
 	/* MII controller setting */
 	mdp->mii_bus = alloc_mdio_bitbang(&bitbang->ctrl);
-	if (!mdp->mii_bus) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!mdp->mii_bus)
+		return -ENOMEM;
 
 	/* Hook up MII support for ethtool */
 	mdp->mii_bus->name = "sh_mii";
@@ -2659,8 +2653,6 @@ static int sh_mdio_init(struct sh_eth_private *mdp,
 
 out_free_bus:
 	free_mdio_bitbang(mdp->mii_bus);
-
-out:
 	return ret;
 }
 
@@ -2773,15 +2765,12 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (unlikely(res == NULL)) {
 		dev_err(&pdev->dev, "invalid resource\n");
-		ret = -EINVAL;
-		goto out;
+		return -EINVAL;
 	}
 
 	ndev = alloc_etherdev(sizeof(struct sh_eth_private));
-	if (!ndev) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!ndev)
+		return -ENOMEM;
 
 	/* The sh Ether-specific entries in the device structure. */
 	ndev->base_addr = res->start;
@@ -2922,7 +2911,6 @@ out_release:
 	if (ndev)
 		free_netdev(ndev);
 
-out:
 	return ret;
 }
 
-- 
1.8.3.2

Re: [PATCH 3/5] sh_eth: Simplify MDIO bus initialization and release

[Sergei Shtylyov]

On 20-03-2014 18:00, Laurent Pinchart wrote:

> The network device passed to the sh_mdio_init and sh_mdio_release
> functions is only used to access the sh_eth_private instance. Pass it
> directly to those functions.

    The changelog is incomplete now. You added significant changes but forgot 
to document them.

> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>   drivers/net/ethernet/renesas/sh_eth.c | 30 +++++++++++-------------------
>   1 file changed, 11 insertions(+), 19 deletions(-)

> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 443f14f..e9224f2 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -2583,29 +2583,24 @@ static void sh_eth_tsu_init(struct sh_eth_private *mdp)
[...]
> -static int sh_mdio_init(struct net_device *ndev, int id,
> +static int sh_mdio_init(struct sh_eth_private *mdp,
>   			struct sh_eth_plat_data *pd)
>   {
>   	int ret, i;
>   	struct bb_info *bitbang;
> -	struct sh_eth_private *mdp = netdev_priv(ndev);
> +	struct platform_device *pdev = mdp->pdev;
>   	struct device *dev = &mdp->pdev->dev;

	struct device *dev = &pdev->dev;

> @@ -2635,7 +2630,7 @@ static int sh_mdio_init(struct net_device *ndev, int id,
>   	mdp->mii_bus->name = "sh_mii";
>   	mdp->mii_bus->parent = dev;
>   	snprintf(mdp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
> -		 mdp->pdev->name, id);
> +		 pdev->name, pdev->id);
>
>   	/* PHY IRQ */
>   	mdp->mii_bus->irq = devm_kzalloc(dev, sizeof(int) * PHY_MAX_ADDR,

WBR, Sergei

Re: [PATCH 3/5] sh_eth: Simplify MDIO bus initialization and release

[Sergei Shtylyov]

On 20-03-2014 18:00, Laurent Pinchart wrote:

> The network device passed to the sh_mdio_init and sh_mdio_release
> functions is only used to access the sh_eth_private instance. Pass it
> directly to those functions.

    The changelog is incomplete now. You added significant changes but forgot 
to document them.

> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>   drivers/net/ethernet/renesas/sh_eth.c | 30 +++++++++++-------------------
>   1 file changed, 11 insertions(+), 19 deletions(-)

> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 443f14f..e9224f2 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -2583,29 +2583,24 @@ static void sh_eth_tsu_init(struct sh_eth_private *mdp)
[...]
> -static int sh_mdio_init(struct net_device *ndev, int id,
> +static int sh_mdio_init(struct sh_eth_private *mdp,
>   			struct sh_eth_plat_data *pd)
>   {
>   	int ret, i;
>   	struct bb_info *bitbang;
> -	struct sh_eth_private *mdp = netdev_priv(ndev);
> +	struct platform_device *pdev = mdp->pdev;
>   	struct device *dev = &mdp->pdev->dev;

	struct device *dev = &pdev->dev;

> @@ -2635,7 +2630,7 @@ static int sh_mdio_init(struct net_device *ndev, int id,
>   	mdp->mii_bus->name = "sh_mii";
>   	mdp->mii_bus->parent = dev;
>   	snprintf(mdp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
> -		 mdp->pdev->name, id);
> +		 pdev->name, pdev->id);
>
>   	/* PHY IRQ */
>   	mdp->mii_bus->irq = devm_kzalloc(dev, sizeof(int) * PHY_MAX_ADDR,

WBR, Sergei

Re: [PATCH 0/5] sh_eth: Fix the race between open and MDIO bus registration

[David Miller]

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Date: Thu, 20 Mar 2014 15:00:30 +0100

> This patch series fixes the race condition that exists in the sh_eth driver
> between network device open and MDIO bus registration. The actual fix is in
> patch 4/5, with previous patches preparing the driver and patch 5/5 cleaning
> up an unrelated issue.
> 
> I've based the idea on Sergei's attempt to fix the problem and can successfully
> boot the Koelsch board over NFS with this series. I might have missed other
> issues though, hence the RFC status.
> 
> The patches are based on top of the latest net-next master branch.

Series applied to net-next, thanks.

Re: [PATCH 0/5] sh_eth: Fix the race between open and MDIO bus registration

[David Miller]

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Date: Thu, 20 Mar 2014 15:00:30 +0100

> This patch series fixes the race condition that exists in the sh_eth driver
> between network device open and MDIO bus registration. The actual fix is in
> patch 4/5, with previous patches preparing the driver and patch 5/5 cleaning
> up an unrelated issue.
> 
> I've based the idea on Sergei's attempt to fix the problem and can successfully
> boot the Koelsch board over NFS with this series. I might have missed other
> issues though, hence the RFC status.
> 
> The patches are based on top of the latest net-next master branch.

Series applied to net-next, thanks.