diff mbox

[v3] lxt PHY: Support for the buggy LXT973 rev A2

Message ID 201209221616.q8MGGnRc019864@localhost.localdomain
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Christophe Leroy Sept. 22, 2012, 4:16 p.m. UTC
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>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Richard Cochran Sept. 22, 2012, 5:32 p.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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,},
 };