* [PATCH v3] lxt PHY: Support for the buggy LXT973 rev A2
@ 2012-09-22 16:16 Christophe Leroy
2012-09-22 17:32 ` Richard Cochran
0 siblings, 1 reply; 2+ messages in thread
From: Christophe Leroy @ 2012-09-22 16:16 UTC (permalink / raw)
To: David S Miller, Richard Cochran; +Cc: netdev, linux-kernel
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
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
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
@@ -54,8 +54,12 @@
#define MII_LXT971_ISR 19 /* Interrupt Status Register */
/* register definitions for the 973 */
-#define MII_LXT973_PCR 16 /* Port Configuration Register */
+#define MII_LXT973_PCR 16 /* Port Configuration Register */
#define PCR_FIBER_SELECT 1
+#define MII_LXT973_SFR 27 /* Special Function Register */
+
+#define PHYDEV_PRIV_FIBER 1
+#define PHYDEV_PRIV_REVA2 2
MODULE_DESCRIPTION("Intel LXT PHY driver");
MODULE_AUTHOR("Andy Fleming");
@@ -99,6 +103,9 @@
return err;
}
+/* register definitions for the 973 */
+#define MII_LXT973_PCR 16 /* Port Configuration Register */
+#define PCR_FIBER_SELECT 1
static int lxt971_ack_interrupt(struct phy_device *phydev)
{
@@ -122,9 +129,138 @@
return err;
}
+/*
+ * A2 version of LXT973 chip has an ERRATA: it randomly return the contents
+ * of the previous even register when you read a odd register regularly
+ */
+
+static int lxt973a2_update_link(struct phy_device *phydev)
+{
+ int status;
+ int control;
+ int retry = 8; /* we try 8 times */
+
+ /* Do a fake read */
+ status = phy_read(phydev, MII_BMSR);
+
+ if (status < 0)
+ return status;
+
+ control = phy_read(phydev, MII_BMCR);
+ if (control < 0)
+ return control;
+
+ do {
+ /* Read link and autonegotiation status */
+ status = phy_read(phydev, MII_BMSR);
+ } while (status >= 0 && retry-- && status == control);
+
+ if (status < 0)
+ return status;
+
+ if ((status & BMSR_LSTATUS) == 0)
+ phydev->link = 0;
+ else
+ phydev->link = 1;
+
+ return 0;
+}
+
+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;
+
+ 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);
+}
+
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;
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);
}
static struct phy_driver lxt970_driver = {
@@ -184,7 +329,10 @@
.flags = 0,
.probe = lxt973_probe,
.config_aneg = lxt973_config_aneg,
- .read_status = genphy_read_status,
+ .read_status = lxt973_read_status,
+ .suspend = genphy_suspend,
+ .resume = genphy_resume,
+ .isolate = genphy_isolate,
.driver = { .owner = THIS_MODULE,},
};
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v3] lxt PHY: Support for the buggy LXT973 rev A2
2012-09-22 16:16 [PATCH v3] lxt PHY: Support for the buggy LXT973 rev A2 Christophe Leroy
@ 2012-09-22 17:32 ` Richard Cochran
0 siblings, 0 replies; 2+ messages in thread
From: Richard Cochran @ 2012-09-22 17:32 UTC (permalink / raw)
To: Christophe Leroy; +Cc: David S Miller, netdev, linux-kernel
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.
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>
> 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);
> +}
> +
> 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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-09-22 17:32 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-22 16:16 [PATCH v3] lxt PHY: Support for the buggy LXT973 rev A2 Christophe Leroy
2012-09-22 17:32 ` Richard Cochran
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).