Commit 5bb96e9f2434b49a5b8f135f2a384974aa73db51

Authored by Eugene Surovegin
Committed by Jeff Garzik
1 parent 0ec6d95053

ibm_emac: improved PHY support

Original patch is from Jeff Haran  <jharan@brocade.com> with my minor style
fixes. His comments follow:

The first problem was in the function that configures the PHY for
autonegotiation, genmii_setup_aneg(). The original code does a
read/modify/write of the autonegotiation advertizement register (reg 4),
followed by a read/modify/write of the control register (reg 0). While
the original code follows the proper procedure as per reading the IEEE
specs, what I found is that on at least one PHY model (National DP83843)
the read of the control register comes back with the soft reset bit set
(bit 15). Because of the read/modify/write operation, this causes the
write to write a 1 back to the reset bit, which initiates a software
reset of the PHY. This software reset causes the PHY to return to its
power up state which advertizes all modes of operation, thus negating
the write to the autoneg advertizement register. The modification is to
spin reading the control register until the soft reset bit is clear
before doing the modify/write.
The second problem was in the function that configures the PHY for
forced operation, genmii_setup_forced(). The original code initiates a
software reset operation via a write of a 1 to bit 15 of the control
register (reg 0), but then proceeds to do a second write to that same
register without waiting until that reset bit is cleared by the PHY
itself (which according to the IEEE specs indicates that the PHY reset
is complete). This is a violation of how one is supposed to use this
software reset feature of these PHYs and I believe was the cause of
mysterious, difficult to reproduce link failures that we've observed on
some of our systems that use this driver. The fix is to modify the
function so that it spins waiting for the reset bit to clear after doing
the soft reset and before doing the subsequent write.

Signed-off-by: Jeff Haran <jharan@brocade.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Eugene Surovegin <ebs@ebshome.net>
Signed-off-by: Jeff Garzik <jeff@garzik.org>

Showing 1 changed file with 45 additions and 15 deletions Side-by-side Diff

drivers/net/ibm_emac/ibm_emac_phy.c
... ... @@ -22,6 +22,7 @@
22 22  
23 23 #include <asm/ocp.h>
24 24  
  25 +#include "ibm_emac_core.h"
25 26 #include "ibm_emac_phy.h"
26 27  
27 28 static inline int phy_read(struct mii_phy *phy, int reg)
28 29  
... ... @@ -34,11 +35,39 @@
34 35 phy->mdio_write(phy->dev, phy->address, reg, val);
35 36 }
36 37  
37   -int mii_reset_phy(struct mii_phy *phy)
  38 +/*
  39 + * polls MII_BMCR until BMCR_RESET bit clears or operation times out.
  40 + *
  41 + * returns:
  42 + * >= 0 => success, value in BMCR returned to caller
  43 + * -EBUSY => failure, RESET bit never cleared
  44 + * otherwise => failure, lower level PHY read failed
  45 + */
  46 +static int mii_spin_reset_complete(struct mii_phy *phy)
38 47 {
39 48 int val;
40 49 int limit = 10000;
41 50  
  51 + while (limit--) {
  52 + val = phy_read(phy, MII_BMCR);
  53 + if (val >= 0 && !(val & BMCR_RESET))
  54 + return val; /* success */
  55 + udelay(10);
  56 + }
  57 + if (val & BMCR_RESET)
  58 + val = -EBUSY;
  59 +
  60 + if (net_ratelimit())
  61 + printk(KERN_ERR "emac%d: PHY reset timeout (%d)\n",
  62 + ((struct ocp_enet_private *)phy->dev->priv)->def->index,
  63 + val);
  64 + return val;
  65 +}
  66 +
  67 +int mii_reset_phy(struct mii_phy *phy)
  68 +{
  69 + int val;
  70 +
42 71 val = phy_read(phy, MII_BMCR);
43 72 val &= ~BMCR_ISOLATE;
44 73 val |= BMCR_RESET;
45 74  
... ... @@ -46,16 +75,11 @@
46 75  
47 76 udelay(300);
48 77  
49   - while (limit--) {
50   - val = phy_read(phy, MII_BMCR);
51   - if (val >= 0 && (val & BMCR_RESET) == 0)
52   - break;
53   - udelay(10);
54   - }
55   - if ((val & BMCR_ISOLATE) && limit > 0)
  78 + val = mii_spin_reset_complete(phy);
  79 + if (val >= 0 && (val & BMCR_ISOLATE))
56 80 phy_write(phy, MII_BMCR, val & ~BMCR_ISOLATE);
57 81  
58   - return limit <= 0;
  82 + return val < 0;
59 83 }
60 84  
61 85 static int genmii_setup_aneg(struct mii_phy *phy, u32 advertise)
... ... @@ -102,8 +126,14 @@
102 126 }
103 127  
104 128 /* Start/Restart aneg */
105   - ctl = phy_read(phy, MII_BMCR);
106   - ctl |= (BMCR_ANENABLE | BMCR_ANRESTART);
  129 + /* on some PHYs (e.g. National DP83843) a write to MII_ADVERTISE
  130 + * causes BMCR_RESET to be set on the next read of MII_BMCR, which
  131 + * if not checked for causes the PHY to be reset below */
  132 + ctl = mii_spin_reset_complete(phy);
  133 + if (ctl < 0)
  134 + return ctl;
  135 +
  136 + ctl |= BMCR_ANENABLE | BMCR_ANRESTART;
107 137 phy_write(phy, MII_BMCR, ctl);
108 138  
109 139 return 0;
110 140  
... ... @@ -118,13 +148,13 @@
118 148 phy->duplex = fd;
119 149 phy->pause = phy->asym_pause = 0;
120 150  
  151 + /* First reset the PHY */
  152 + mii_reset_phy(phy);
  153 +
121 154 ctl = phy_read(phy, MII_BMCR);
122 155 if (ctl < 0)
123 156 return ctl;
124   - ctl &= ~(BMCR_FULLDPLX | BMCR_SPEED100 | BMCR_ANENABLE);
125   -
126   - /* First reset the PHY */
127   - phy_write(phy, MII_BMCR, ctl | BMCR_RESET);
  157 + ctl &= ~(BMCR_FULLDPLX | BMCR_SPEED100 | BMCR_ANENABLE | BMCR_SPEED1000);
128 158  
129 159 /* Select speed & duplex */
130 160 switch (speed) {