[SUNGEM]: Make PM of PHYs more reliable (#2)

On my latest laptop, I've had occasional PHY dead on wakeup from
sleep... the PHY would be totally unresponsive even to toggling the hard
reset line until the machine is powered down... Looking closely at the
code, I found some possible issues in the way we setup the MDIO lines
during suspend along with slight divergences from what Darwin does when
resetting it that may explain the problem. That patch change these and
the problem appear to be gone for me at least... I also fixed an mdelay
-> msleep while I was at it to the pmac feature code that is called
when toggling the PHY reset line since sungem doesn't call it in an
atomic context anymore.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>b
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/arch/powerpc/platforms/powermac/feature.c b/arch/powerpc/platforms/powermac/feature.c
index 558dd06920921f04bde22e4084d0347ba73c8209..2296f3d46ca829ac979e92a7af0393bb422d05cf 100644 (file)
--- a/arch/powerpc/platforms/powermac/feature.c
+++ b/arch/powerpc/platforms/powermac/feature.c
@@ -910,16 +910,18 @@ core99_gmac_phy_reset(struct device_node *node, long param, long value)
            macio->type != macio_intrepid)
                return -ENODEV;
 
+       printk(KERN_DEBUG "Hard reset of PHY chip ...\n");
+
        LOCK(flags);
        MACIO_OUT8(KL_GPIO_ETH_PHY_RESET, KEYLARGO_GPIO_OUTPUT_ENABLE);
        (void)MACIO_IN8(KL_GPIO_ETH_PHY_RESET);
        UNLOCK(flags);
-       mdelay(10);
+       msleep(10);
        LOCK(flags);
        MACIO_OUT8(KL_GPIO_ETH_PHY_RESET, /*KEYLARGO_GPIO_OUTPUT_ENABLE | */
                KEYLARGO_GPIO_OUTOUT_DATA);
        UNLOCK(flags);
-       mdelay(10);
+       msleep(10);
 
        return 0;
 }

diff --git a/drivers/net/sungem.c b/drivers/net/sungem.c
index 28ce47a02408e9946c2e3941c491b5313ca56db2..55f3b856236e13262387bbcd0df3d05846b86c8d 100644 (file)
--- a/drivers/net/sungem.c
+++ b/drivers/net/sungem.c
@@ -1653,36 +1653,40 @@ static void gem_init_rings(struct gem *gp)
 /* Init PHY interface and start link poll state machine */
 static void gem_init_phy(struct gem *gp)
 {
-       u32 mifcfg;
+       u32 mif_cfg;
 
        /* Revert MIF CFG setting done on stop_phy */
-       mifcfg = readl(gp->regs + MIF_CFG);
-       mifcfg &= ~MIF_CFG_BBMODE;
-       writel(mifcfg, gp->regs + MIF_CFG);
+       mif_cfg = readl(gp->regs + MIF_CFG);
+       mif_cfg &= ~(MIF_CFG_PSELECT|MIF_CFG_POLL|MIF_CFG_BBMODE|MIF_CFG_MDI1);
+       mif_cfg |= MIF_CFG_MDI0;
+       writel(mif_cfg, gp->regs + MIF_CFG);
+       writel(PCS_DMODE_MGM, gp->regs + PCS_DMODE);
+       writel(MAC_XIFCFG_OE, gp->regs + MAC_XIFCFG);
        
        if (gp->pdev->vendor == PCI_VENDOR_ID_APPLE) {
                int i;
+               u16 ctrl;
 
-               /* Those delay sucks, the HW seem to love them though, I'll
-                * serisouly consider breaking some locks here to be able
-                * to schedule instead
-                */
-               for (i = 0; i < 3; i++) {
 #ifdef CONFIG_PPC_PMAC
-                       pmac_call_feature(PMAC_FTR_GMAC_PHY_RESET, gp->of_node, 0, 0);
-                       msleep(20);
+               pmac_call_feature(PMAC_FTR_GMAC_PHY_RESET, gp->of_node, 0, 0);
 #endif
-                       /* Some PHYs used by apple have problem getting back to us,
-                        * we do an additional reset here
-                        */
-                       phy_write(gp, MII_BMCR, BMCR_RESET);
-                       msleep(20);
-                       if (phy_read(gp, MII_BMCR) != 0xffff)
+
+               /* Some PHYs used by apple have problem getting back
+                * to us, we do an additional reset here
+                */
+               phy_write(gp, MII_BMCR, BMCR_RESET);
+               for (i = 0; i < 50; i++) {
+                       if ((phy_read(gp, MII_BMCR) & BMCR_RESET) == 0)
                                break;
-                       if (i == 2)
-                               printk(KERN_WARNING "%s: GMAC PHY not responding !\n",
-                                      gp->dev->name);
+                       msleep(10);
                }
+               if (i == 50)
+                       printk(KERN_WARNING "%s: GMAC PHY not responding !\n",
+                              gp->dev->name);
+               /* Make sure isolate is off */
+               ctrl = phy_read(gp, MII_BMCR);
+               if (ctrl & BMCR_ISOLATE)
+                       phy_write(gp, MII_BMCR, ctrl & ~BMCR_ISOLATE);
        }
 
        if (gp->pdev->vendor == PCI_VENDOR_ID_SUN &&
@@ -2119,7 +2123,7 @@ static void gem_reinit_chip(struct gem *gp)
 /* Must be invoked with no lock held. */
 static void gem_stop_phy(struct gem *gp, int wol)
 {
-       u32 mifcfg;
+       u32 mif_cfg;
        unsigned long flags;
 
        /* Let the chip settle down a bit, it seems that helps
@@ -2130,9 +2134,9 @@ static void gem_stop_phy(struct gem *gp, int wol)
        /* Make sure we aren't polling PHY status change. We
         * don't currently use that feature though
         */
-       mifcfg = readl(gp->regs + MIF_CFG);
-       mifcfg &= ~MIF_CFG_POLL;
-       writel(mifcfg, gp->regs + MIF_CFG);
+       mif_cfg = readl(gp->regs + MIF_CFG);
+       mif_cfg &= ~MIF_CFG_POLL;
+       writel(mif_cfg, gp->regs + MIF_CFG);
 
        if (wol && gp->has_wol) {
                unsigned char *e = &gp->dev->dev_addr[0];
@@ -2182,7 +2186,8 @@ static void gem_stop_phy(struct gem *gp, int wol)
                /* According to Apple, we must set the MDIO pins to this begnign
                 * state or we may 1) eat more current, 2) damage some PHYs
                 */
-               writel(mifcfg | MIF_CFG_BBMODE, gp->regs + MIF_CFG);
+               mif_cfg = 0;
+               writel(mif_cfg | MIF_CFG_BBMODE, gp->regs + MIF_CFG);
                writel(0, gp->regs + MIF_BBCLK);
                writel(0, gp->regs + MIF_BBDATA);
                writel(0, gp->regs + MIF_BBOENAB);