Re: [PATCH v3] lxt PHY: Support for the buggy LXT973 rev A2
From: Richard Cochran <richardcochran@gmail.com>
Date: 2012-09-22 17:32:39
Also in:
lkml
On Sat, Sep 22, 2012 at 06:16:49PM +0200, Christophe Leroy wrote:
This patch adds proper handling of the buggy revision A2 of LXT973 phy, adding precautions linked to ERRATA Item 4: Revision A2 of LXT973 chip randomly returns the contents of the previous even register when you read a odd register regularly
This patch does not apply to net-next. Also, I have just a few stylistic comments, below.
quoted hunk ↗ jump to hunk
Signed-off-by: Christophe Leroy <redacted> diff -u linux-3.5-vanilla/drivers/net/phy/lxt.c linux-3.5/drivers/net/phy/lxt.c--- linux-3.5-vanilla/drivers/net/phy/lxt.c 2012-07-21 22:58:29.000000000 +0200 +++ linux-3.5/drivers/net/phy/lxt.c 2012-09-15 19:20:34.000000000 +0200
...
+int lxt973a2_read_status(struct phy_device *phydev)
+{
+ int adv;
+ int err;
+ int lpa;
+ int lpagb = 0;
+
+ /* Update the link, but return if there was an error */
+ err = lxt973a2_update_link(phydev);
+ if (err)
+ return err;
+
+ if (AUTONEG_ENABLE == phydev->autoneg) {
+ int retry = 1;
+
+ adv = phy_read(phydev, MII_ADVERTISE);
+
+ if (adv < 0)
+ return adv;
+
+ do {
+ lpa = phy_read(phydev, MII_LPA);
+
+ if (lpa < 0)
+ return lpa;
+
+ /* If both registers are equal, it is suspect but not
+ * impossible, hence a new try
+ */
+ } while (lpa == adv && retry--);
+
+ lpa &= adv;
+
+ phydev->speed = SPEED_10;
+ phydev->duplex = DUPLEX_HALF;
+ phydev->pause = phydev->asym_pause = 0;
+
+ if (lpagb & (LPA_1000FULL | LPA_1000HALF)) {
+ phydev->speed = SPEED_1000;
+
+ if (lpagb & LPA_1000FULL)
+ phydev->duplex = DUPLEX_FULL;
+ } else if (lpa & (LPA_100FULL | LPA_100HALF)) {
+ phydev->speed = SPEED_100;
+
+ if (lpa & LPA_100FULL)
+ phydev->duplex = DUPLEX_FULL;
+ } else
+ if (lpa & LPA_10FULL)
+ phydev->duplex = DUPLEX_FULL;This last 'else' could use braces.
+
+ if (phydev->duplex == DUPLEX_FULL) {
+ phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0;
+ phydev->asym_pause = lpa & LPA_PAUSE_ASYM ? 1 : 0;
+ }
+ } else {
+ int bmcr = phy_read(phydev, MII_BMCR);
+
+ if (bmcr < 0)
+ return bmcr;
+
+ if (bmcr & BMCR_FULLDPLX)
+ phydev->duplex = DUPLEX_FULL;
+ else
+ phydev->duplex = DUPLEX_HALF;
+
+ if (bmcr & BMCR_SPEED1000)
+ phydev->speed = SPEED_1000;
+ else if (bmcr & BMCR_SPEED100)
+ phydev->speed = SPEED_100;
+ else
+ phydev->speed = SPEED_10;
+
+ phydev->pause = phydev->asym_pause = 0;
+ }
+
+ return 0;
+}
+
+static int lxt973_read_status(struct phy_device *phydev)
+{
+ return (int)phydev->priv&PHYDEV_PRIV_REVA2 ?
+ lxt973a2_read_status(phydev) :
+ genphy_read_status(phydev);Needs spacing, like this: return (int) phydev->priv & PHYDEV_PRIV_REVA2 ? lxt973a2_read_status(phydev) : genphy_read_status(phydev);
quoted hunk ↗ jump to hunk
+} + static int lxt973_probe(struct phy_device *phydev) { int val = phy_read(phydev, MII_LXT973_PCR); + int priv = 0; + + phydev->priv = NULL; + + if (val < 0) + return val; if (val & PCR_FIBER_SELECT) { /*@@ -136,17 +272,26 @@ val &= ~BMCR_ANENABLE; phy_write(phydev, MII_BMCR, val); /* Remember that the port is in fiber mode. */ - phydev->priv = lxt973_probe; - } else { - phydev->priv = NULL; + priv |= PHYDEV_PRIV_FIBER; + } + val = phy_read(phydev, MII_PHYSID2); + + if (val < 0) + return val; + + if ((val & 0xf) == 0) { /* rev A2 */ + dev_info(&phydev->dev, " LXT973 revision A2 has bugs\n"); + priv |= PHYDEV_PRIV_REVA2; } + phydev->priv = (void *)priv;
One space after cast, please: (void *) priv;
return 0;
}
static int lxt973_config_aneg(struct phy_device *phydev)
{
/* Do nothing if port is in fiber mode. */
- return phydev->priv ? 0 : genphy_config_aneg(phydev);
+ return (int)phydev->priv&PHYDEV_PRIV_FIBER ?
+ 0 : genphy_config_aneg(phydev);Same spacing issue again. Thanks, Richard