Skip to content

Commit

Permalink
Revert "ravb: Fixed to be able to unload modules"
Browse files Browse the repository at this point in the history
This reverts commit 1838d6c.

This commit moved the ravb_mdio_init() call (and thus the
of_mdiobus_register() call) from the ravb_probe() to the ravb_open()
call.  This causes a regression during system resume (s2idle/s2ram), as
new PHY devices cannot be bound while suspended.

During boot, the Micrel PHY is detected like this:

    Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Micrel KSZ9031 Gigabit PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=228)
    ravb e6800000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off

During system suspend, (A) defer_all_probes is set to true, and (B)
usermodehelper_disabled is set to UMH_DISABLED, to avoid drivers being
probed while suspended.

  A. If CONFIG_MODULES=n, phy_device_register() calling device_add()
     merely adds the device, but does not probe it yet, as
     really_probe() returns early due to defer_all_probes being set:

       dpm_resume+0x128/0x4f8
	 device_resume+0xcc/0x1b0
	   dpm_run_callback+0x74/0x340
	     ravb_resume+0x190/0x1b8
	       ravb_open+0x84/0x770
		 of_mdiobus_register+0x1e0/0x468
		   of_mdiobus_register_phy+0x1b8/0x250
		     of_mdiobus_phy_device_register+0x178/0x1e8
		       phy_device_register+0x114/0x1b8
			 device_add+0x3d4/0x798
			   bus_probe_device+0x98/0xa0
			     device_initial_probe+0x10/0x18
			       __device_attach+0xe4/0x140
				 bus_for_each_drv+0x64/0xc8
				   __device_attach_driver+0xb8/0xe0
				     driver_probe_device.part.11+0xc4/0xd8
				       really_probe+0x32c/0x3b8

     Later, phy_attach_direct() notices no PHY driver has been bound,
     and falls back to the Generic PHY, leading to degraded operation:

       Generic PHY e6800000.ethernet-ffffffff:00: attached PHY driver [Generic PHY] (mii_bus:phy_addr=e6800000.ethernet-ffffffff:00, irq=POLL)
       ravb e6800000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off

  B. If CONFIG_MODULES=y, request_module() returns early with -EBUSY due
     to UMH_DISABLED, and MDIO initialization fails completely:

       mdio_bus e6800000.ethernet-ffffffff:00: error -16 loading PHY driver module for ID 0x00221622
       ravb e6800000.ethernet eth0: failed to initialize MDIO
       PM: dpm_run_callback(): ravb_resume+0x0/0x1b8 returns -16
       PM: Device e6800000.ethernet failed to resume: error -16

     Ignoring -EBUSY in phy_request_driver_module(), like was done for
     -ENOENT in commit 21e1944 ("net: phy: fix issue with loading
     PHY driver w/o initramfs"), would makes it fall back to the Generic
     PHY, like in the CONFIG_MODULES=n case.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: stable@vger.kernel.org
Reviewed-by: Sergei Shtylyov <sergei.shtylyov@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Geert Uytterhoeven authored and David S. Miller committed Sep 24, 2020
1 parent ef59b19 commit 77972b5
Showing 1 changed file with 55 additions and 55 deletions.
110 changes: 55 additions & 55 deletions drivers/net/ethernet/renesas/ravb_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1342,51 +1342,6 @@ static inline int ravb_hook_irq(unsigned int irq, irq_handler_t handler,
return error;
}

/* MDIO bus init function */
static int ravb_mdio_init(struct ravb_private *priv)
{
struct platform_device *pdev = priv->pdev;
struct device *dev = &pdev->dev;
int error;

/* Bitbang init */
priv->mdiobb.ops = &bb_ops;

/* MII controller setting */
priv->mii_bus = alloc_mdio_bitbang(&priv->mdiobb);
if (!priv->mii_bus)
return -ENOMEM;

/* Hook up MII support for ethtool */
priv->mii_bus->name = "ravb_mii";
priv->mii_bus->parent = dev;
snprintf(priv->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
pdev->name, pdev->id);

/* Register MDIO bus */
error = of_mdiobus_register(priv->mii_bus, dev->of_node);
if (error)
goto out_free_bus;

return 0;

out_free_bus:
free_mdio_bitbang(priv->mii_bus);
return error;
}

/* MDIO bus release function */
static int ravb_mdio_release(struct ravb_private *priv)
{
/* Unregister mdio bus */
mdiobus_unregister(priv->mii_bus);

/* Free bitbang info */
free_mdio_bitbang(priv->mii_bus);

return 0;
}

/* Network device open function for Ethernet AVB */
static int ravb_open(struct net_device *ndev)
{
Expand All @@ -1395,13 +1350,6 @@ static int ravb_open(struct net_device *ndev)
struct device *dev = &pdev->dev;
int error;

/* MDIO bus init */
error = ravb_mdio_init(priv);
if (error) {
netdev_err(ndev, "failed to initialize MDIO\n");
return error;
}

napi_enable(&priv->napi[RAVB_BE]);
napi_enable(&priv->napi[RAVB_NC]);

Expand Down Expand Up @@ -1479,7 +1427,6 @@ static int ravb_open(struct net_device *ndev)
out_napi_off:
napi_disable(&priv->napi[RAVB_NC]);
napi_disable(&priv->napi[RAVB_BE]);
ravb_mdio_release(priv);
return error;
}

Expand Down Expand Up @@ -1789,8 +1736,6 @@ static int ravb_close(struct net_device *ndev)
ravb_ring_free(ndev, RAVB_BE);
ravb_ring_free(ndev, RAVB_NC);

ravb_mdio_release(priv);

return 0;
}

Expand Down Expand Up @@ -1942,6 +1887,51 @@ static const struct net_device_ops ravb_netdev_ops = {
.ndo_set_features = ravb_set_features,
};

/* MDIO bus init function */
static int ravb_mdio_init(struct ravb_private *priv)
{
struct platform_device *pdev = priv->pdev;
struct device *dev = &pdev->dev;
int error;

/* Bitbang init */
priv->mdiobb.ops = &bb_ops;

/* MII controller setting */
priv->mii_bus = alloc_mdio_bitbang(&priv->mdiobb);
if (!priv->mii_bus)
return -ENOMEM;

/* Hook up MII support for ethtool */
priv->mii_bus->name = "ravb_mii";
priv->mii_bus->parent = dev;
snprintf(priv->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
pdev->name, pdev->id);

/* Register MDIO bus */
error = of_mdiobus_register(priv->mii_bus, dev->of_node);
if (error)
goto out_free_bus;

return 0;

out_free_bus:
free_mdio_bitbang(priv->mii_bus);
return error;
}

/* MDIO bus release function */
static int ravb_mdio_release(struct ravb_private *priv)
{
/* Unregister mdio bus */
mdiobus_unregister(priv->mii_bus);

/* Free bitbang info */
free_mdio_bitbang(priv->mii_bus);

return 0;
}

static const struct of_device_id ravb_match_table[] = {
{ .compatible = "renesas,etheravb-r8a7790", .data = (void *)RCAR_GEN2 },
{ .compatible = "renesas,etheravb-r8a7794", .data = (void *)RCAR_GEN2 },
Expand Down Expand Up @@ -2184,6 +2174,13 @@ static int ravb_probe(struct platform_device *pdev)
eth_hw_addr_random(ndev);
}

/* MDIO bus init */
error = ravb_mdio_init(priv);
if (error) {
dev_err(&pdev->dev, "failed to initialize MDIO\n");
goto out_dma_free;
}

netif_napi_add(ndev, &priv->napi[RAVB_BE], ravb_poll, 64);
netif_napi_add(ndev, &priv->napi[RAVB_NC], ravb_poll, 64);

Expand All @@ -2205,6 +2202,8 @@ static int ravb_probe(struct platform_device *pdev)
out_napi_del:
netif_napi_del(&priv->napi[RAVB_NC]);
netif_napi_del(&priv->napi[RAVB_BE]);
ravb_mdio_release(priv);
out_dma_free:
dma_free_coherent(ndev->dev.parent, priv->desc_bat_size, priv->desc_bat,
priv->desc_bat_dma);

Expand Down Expand Up @@ -2236,6 +2235,7 @@ static int ravb_remove(struct platform_device *pdev)
unregister_netdev(ndev);
netif_napi_del(&priv->napi[RAVB_NC]);
netif_napi_del(&priv->napi[RAVB_BE]);
ravb_mdio_release(priv);
pm_runtime_disable(&pdev->dev);
free_netdev(ndev);
platform_set_drvdata(pdev, NULL);
Expand Down

0 comments on commit 77972b5

Please sign in to comment.