ar71xx: ag71xx: fix a race involving netdev registration

In particular, phy_connect before register_netdev. This is because
register_netdev runs the netdev notifiers, which can race with the rest of
the initialization in ag71xx_probe. In my case this manifested in two ways:

1) If ag71xx is compiled as a module and inserted after netifd has started,
   netifd is notified by register_netdev before the call to
   ag71xx_phy_connect. netifd tries to bring the interface up, which calls
   ag71xx_open, which in turn enters ag71xx_phy_start. This keys off
   ag->phy_dev (which is still NULL) and thinks this is a fixed-link board,
   and enters ag71xx_link_adjust. This looks at ag->speed which is not yet
   initialized and hits the BUG() in the switch (ag->speed) in
   ag71xx_link_adjust.

   This is the wrong code path for ag71xx_phy_start - my board has PHYs that
   need to be brought up with phy_start. Doing ag71xx_phy_connect before
   register_netdev ensures that ag->phy_dev is non-NULL before
   ag71xx_phy_start is ever called.

2) When ag71xx is built into the kernel, and netconsole is enabled, there
   is a gap in the initial burst of replayed printks right after the netdev
   comes up. My assumption is that netconsole is also triggered by a netdev
   notifier, and part of this printk burst happens before the call into
   ag71xx_phy_connect, so part of the burst is lost while the PHY comes up.
   This patch fixes the gap - all the printks before eth0 comes up are bursted
   in full when netconsole initializes.

ag71xx_phy_connect_xxx no longer runs with a registered netdev, so the
logging has been adjusted accordingly to avoid "unregistered net_device" or
"eth%d" messages in dmesg.

Signed-off-by: Catalin Patulea <cat@vv.carleton.ca>
Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
git-svn-id: svn://svn.openwrt.org/openwrt/trunk@38689 3c298f89-4303-0410-b956-a3cf2f4a3e73

diff --git a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
index fc6be0e..f4d6735 100644 (file)
--- a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
+++ b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
@@ -1178,16 +1178,6 @@ static int ag71xx_probe(struct platform_device *pdev)
 
        netif_napi_add(dev, &ag->napi, ag71xx_poll, AG71XX_NAPI_WEIGHT);
 
 
        netif_napi_add(dev, &ag->napi, ag71xx_poll, AG71XX_NAPI_WEIGHT);
 

-       err = register_netdev(dev);
-       if (err) {
-               dev_err(&pdev->dev, "unable to register net device\n");
-               goto err_free_desc;
-       }
-
-       pr_info("%s: Atheros AG71xx at 0x%08lx, irq %d, mode:%s\n",
-               dev->name, dev->base_addr, dev->irq,
-               ag71xx_get_phy_if_mode_name(pdata->phy_if_mode));
-

        ag71xx_dump_regs(ag);
 
        ag71xx_hw_init(ag);
        ag71xx_dump_regs(ag);
 
        ag71xx_hw_init(ag);


@@ -1196,7 +1186,7 @@ static int ag71xx_probe(struct platform_device *pdev)
 
        err = ag71xx_phy_connect(ag);
        if (err)
 
        err = ag71xx_phy_connect(ag);
        if (err)

-               goto err_unregister_netdev;
+               goto err_free_desc;

 
        err = ag71xx_debugfs_init(ag);
        if (err)
 
        err = ag71xx_debugfs_init(ag);
        if (err)


@@ -1204,12 +1194,20 @@ static int ag71xx_probe(struct platform_device *pdev)
 
        platform_set_drvdata(pdev, dev);
 
 
        platform_set_drvdata(pdev, dev);
 

+       err = register_netdev(dev);
+       if (err) {
+               dev_err(&pdev->dev, "unable to register net device\n");
+               goto err_phy_disconnect;
+       }
+
+       pr_info("%s: Atheros AG71xx at 0x%08lx, irq %d, mode:%s\n",
+               dev->name, dev->base_addr, dev->irq,
+               ag71xx_get_phy_if_mode_name(pdata->phy_if_mode));
+

        return 0;
 
 err_phy_disconnect:
        ag71xx_phy_disconnect(ag);
        return 0;
 
 err_phy_disconnect:
        ag71xx_phy_disconnect(ag);

-err_unregister_netdev:
-       unregister_netdev(dev);

 err_free_desc:
        dma_free_coherent(NULL, sizeof(struct ag71xx_desc), ag->stop_desc,
                          ag->stop_desc_dma);
 err_free_desc:
        dma_free_coherent(NULL, sizeof(struct ag71xx_desc), ag->stop_desc,
                          ag->stop_desc_dma);

diff --git a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_phy.c b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_phy.c
index f3791e2..9de77e9 100644 (file)
--- a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_phy.c
+++ b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_phy.c
@@ -76,7 +76,7 @@ void ag71xx_phy_stop(struct ag71xx *ag)
 
 static int ag71xx_phy_connect_fixed(struct ag71xx *ag)
 {
 
 static int ag71xx_phy_connect_fixed(struct ag71xx *ag)
 {

-       struct net_device *dev = ag->dev;
+       struct device *dev = &ag->pdev->dev;

        struct ag71xx_platform_data *pdata = ag71xx_get_pdata(ag);
        int ret = 0;
 
        struct ag71xx_platform_data *pdata = ag71xx_get_pdata(ag);
        int ret = 0;
 


@@ -87,12 +87,12 @@ static int ag71xx_phy_connect_fixed(struct ag71xx *ag)
        case SPEED_1000:
                break;
        default:
        case SPEED_1000:
                break;
        default:

-               netdev_err(dev, "invalid speed specified\n");
+               dev_err(dev, "invalid speed specified\n");

                ret = -EINVAL;
                break;
        }
 
                ret = -EINVAL;
                break;
        }
 

-       netdev_dbg(dev, "using fixed link parameters\n");
+       dev_dbg(dev, "using fixed link parameters\n");

 
        ag->duplex = pdata->duplex;
        ag->speed = pdata->speed;
 
        ag->duplex = pdata->duplex;
        ag->speed = pdata->speed;


@@ -102,7 +102,7 @@ static int ag71xx_phy_connect_fixed(struct ag71xx *ag)
 
 static int ag71xx_phy_connect_multi(struct ag71xx *ag)
 {
 
 static int ag71xx_phy_connect_multi(struct ag71xx *ag)
 {

-       struct net_device *dev = ag->dev;
+       struct device *dev = &ag->pdev->dev;

        struct ag71xx_platform_data *pdata = ag71xx_get_pdata(ag);
        struct phy_device *phydev = NULL;
        int phy_addr;
        struct ag71xx_platform_data *pdata = ag71xx_get_pdata(ag);
        struct phy_device *phydev = NULL;
        int phy_addr;


@@ -116,7 +116,7 @@ static int ag71xx_phy_connect_multi(struct ag71xx *ag)
                        continue;
 
                DBG("%s: PHY found at %s, uid=%08x\n",
                        continue;
 
                DBG("%s: PHY found at %s, uid=%08x\n",

-                       dev->name,
+                       dev_name(dev),

                        dev_name(&ag->mii_bus->phy_map[phy_addr]->dev),
                        ag->mii_bus->phy_map[phy_addr]->phy_id);
 
                        dev_name(&ag->mii_bus->phy_map[phy_addr]->dev),
                        ag->mii_bus->phy_map[phy_addr]->phy_id);
 


@@ -125,17 +125,17 @@ static int ag71xx_phy_connect_multi(struct ag71xx *ag)
        }
 
        if (!phydev) {
        }
 
        if (!phydev) {

-               netdev_err(dev, "no PHY found with phy_mask=%08x\n",
+               dev_err(dev, "no PHY found with phy_mask=%08x\n",

                           pdata->phy_mask);
                return -ENODEV;
        }
 
                           pdata->phy_mask);
                return -ENODEV;
        }
 

-       ag->phy_dev = phy_connect(dev, dev_name(&phydev->dev),
+       ag->phy_dev = phy_connect(ag->dev, dev_name(&phydev->dev),

                                  &ag71xx_phy_link_adjust,
                                  pdata->phy_if_mode);
 
        if (IS_ERR(ag->phy_dev)) {
                                  &ag71xx_phy_link_adjust,
                                  pdata->phy_if_mode);
 
        if (IS_ERR(ag->phy_dev)) {

-               netdev_err(dev, "could not connect to PHY at %s\n",
+               dev_err(dev, "could not connect to PHY at %s\n",

                           dev_name(&phydev->dev));
                return PTR_ERR(ag->phy_dev);
        }
                           dev_name(&phydev->dev));
                return PTR_ERR(ag->phy_dev);
        }


@@ -148,7 +148,7 @@ static int ag71xx_phy_connect_multi(struct ag71xx *ag)
 
        phydev->advertising = phydev->supported;
 
 
        phydev->advertising = phydev->supported;
 

-       netdev_info(dev, "connected to PHY at %s [uid=%08x, driver=%s]\n",
+       dev_info(dev, "connected to PHY at %s [uid=%08x, driver=%s]\n",

                    dev_name(&phydev->dev), phydev->phy_id, phydev->drv->name);
 
        ag->link = 0;
                    dev_name(&phydev->dev), phydev->phy_id, phydev->drv->name);
 
        ag->link = 0;


@@ -203,7 +203,7 @@ int ag71xx_phy_connect(struct ag71xx *ag)
 
        ag->mii_bus = dev_to_mii_bus(pdata->mii_bus_dev);
        if (ag->mii_bus == NULL) {
 
        ag->mii_bus = dev_to_mii_bus(pdata->mii_bus_dev);
        if (ag->mii_bus == NULL) {

-               netdev_err(ag->dev, "unable to find MII bus on device '%s'\n",
+               dev_err(&ag->pdev->dev, "unable to find MII bus on device '%s'\n",

                           dev_name(pdata->mii_bus_dev));
                return -ENODEV;
        }
                           dev_name(pdata->mii_bus_dev));
                return -ENODEV;
        }