[U-Boot-Users] [PATCH 2/2 (resubmit)] NET: Add Ethernet 1000BASE-X support for PPC4xx

[U-Boot-Users] [PATCH 2/2 (resubmit)] NET: Add Ethernet 1000BASE-X support for PPC4xx

[Larry Johnson]

This patch adds support for 1000BASE-X to functions "miiphy_speed ()" and
"miiphy_duplex()".  It also adds function "miiphy_is_1000base_x ()", which
returns non-zero iff the PHY registers are configured for 1000BASE-X.  The
"mii info" command is modified to distinguish between 1000BASE-T and -X.

Signed-off-by: Larry Johnson <lrj@acm.org>
---

Ben, this patch attempts to address your recent comments.  How do you feel
about the new function?

 common/cmd_mii.c    |    8 ++-
 common/miiphyutil.c |  151 +++++++++++++++++++++++++++++++--------------------
 include/miiphy.h    |   22 ++++++++
 3 files changed, 121 insertions(+), 60 deletions(-)

diff --git a/common/cmd_mii.c b/common/cmd_mii.c
index 72e11d5..3b4dc8a 100644
--- a/common/cmd_mii.c
+++ b/common/cmd_mii.c
@@ -112,9 +112,11 @@ int do_mii (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 					"OUI = 0x%04X, "
 					"Model = 0x%02X, "
 					"Rev = 0x%02X, "
-					"%3dbaseT, %s\n",
+					"%3dbase%s, %s\n",
 					j, oui, model, rev,
 					miiphy_speed (devname, j),
+					miiphy_is_1000base_x (devname, j)
+						? "X" : "T",
 					(miiphy_duplex (devname, j) == FULL)
 						? "FDX" : "HDX");
 			}
@@ -496,9 +498,11 @@ int do_mii (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 					"OUI = 0x%04X, "
 					"Model = 0x%02X, "
 					"Rev = 0x%02X, "
-					"%3dbaseT, %s\n",
+					"%3dbase%s, %s\n",
 					j, oui, model, rev,
 					miiphy_speed (devname, j),
+					miiphy_is_1000base_x (devname, j)
+						? "X" : "T",
 					(miiphy_duplex (devname, j) == FULL)
 						? "FDX" : "HDX");
 			}
diff --git a/common/miiphyutil.c b/common/miiphyutil.c
index 58ebc5e..281f0b2 100644
--- a/common/miiphyutil.c
+++ b/common/miiphyutil.c
@@ -344,101 +344,136 @@ int miiphy_reset (char *devname, unsigned char addr)

 /*****************************************************************************
  *
- * Determine the ethernet speed (10/100).
+ * Determine the ethernet speed (10/100/1000).  Return 10 on error.
  */
 int miiphy_speed (char *devname, unsigned char addr)
 {
-	unsigned short reg;
+	u16 bmcr, anlpar;

 #if defined(CONFIG_PHY_GIGE)
-	if (miiphy_read (devname, addr, PHY_1000BTSR, &reg)) {
-		printf ("PHY 1000BT Status read failed\n");
-	} else {
-		if (reg != 0xFFFF) {
-			if ((reg & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD))
-			    != 0) {
-				return (_1000BASET);
-			}
-		}
+	u16 btsr;
+
+	/*
+	 * Check for 1000BASE-X.  If it is supported, then assume that the speed
+	 * is 1000.
+	 */
+	if (miiphy_is_1000base_x (devname, addr)) {
+		return _1000BASET;
+	}
+	/*
+	 * No 1000BASE-X, so assume 1000BASE-T/100BASE-TX/10BASE-T register set.
+	 */
+	/* Check for 1000BASE-T. */
+	if (miiphy_read (devname, addr, PHY_1000BTSR, &btsr)) {
+		printf ("PHY 1000BT status");
+		goto miiphy_read_failed;
+	}
+	if (btsr != 0xFFFF &&
+	    (btsr & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD))) {
+		return _1000BASET;
 	}
 #endif /* CONFIG_PHY_GIGE */

 	/* Check Basic Management Control Register first. */
-	if (miiphy_read (devname, addr, PHY_BMCR, &reg)) {
-		puts ("PHY speed read failed, assuming 10bT\n");
-		return (_10BASET);
+	if (miiphy_read (devname, addr, PHY_BMCR, &bmcr)) {
+		printf ("PHY speed");
+		goto miiphy_read_failed;
 	}
 	/* Check if auto-negotiation is on. */
-	if ((reg & PHY_BMCR_AUTON) != 0) {
+	if (bmcr & PHY_BMCR_AUTON) {
 		/* Get auto-negotiation results. */
-		if (miiphy_read (devname, addr, PHY_ANLPAR, &reg)) {
-			puts ("PHY AN speed read failed, assuming 10bT\n");
-			return (_10BASET);
-		}
-		if ((reg & PHY_ANLPAR_100) != 0) {
-			return (_100BASET);
-		} else {
-			return (_10BASET);
+		if (miiphy_read (devname, addr, PHY_ANLPAR, &anlpar)) {
+			printf ("PHY AN speed");
+			goto miiphy_read_failed;
 		}
+		return (anlpar & PHY_ANLPAR_100) ? _100BASET : _10BASET;
 	}
 	/* Get speed from basic control settings. */
-	else if (reg & PHY_BMCR_100MB) {
-		return (_100BASET);
-	} else {
-		return (_10BASET);
-	}
+	return (bmcr & PHY_BMCR_100MB) ? _100BASET : _10BASET;

+      miiphy_read_failed:
+	printf (" read failed, assuming 10BASE-T\n");
+	return _10BASET;
 }

 /*****************************************************************************
  *
- * Determine full/half duplex.
+ * Determine full/half duplex.  Return half on error.
  */
 int miiphy_duplex (char *devname, unsigned char addr)
 {
-	unsigned short reg;
+	u16 bmcr, anlpar;

 #if defined(CONFIG_PHY_GIGE)
-	if (miiphy_read (devname, addr, PHY_1000BTSR, &reg)) {
-		printf ("PHY 1000BT Status read failed\n");
-	} else {
-		if ((reg != 0xFFFF) &&
-		    (reg & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD))) {
-			if ((reg & PHY_1000BTSR_1000FD) != 0) {
-				return (FULL);
-			} else {
-				return (HALF);
-			}
+	u16 btsr;
+
+	/* Check for 1000BASE-X. */
+	if (miiphy_is_1000base_x (devname, addr)) {
+		/* 1000BASE-X */
+		if (miiphy_read (devname, addr, PHY_ANLPAR, &anlpar)) {
+			printf ("1000BASE-X PHY AN duplex");
+			goto miiphy_read_failed;
+		}
+	}
+	/*
+	 * No 1000BASE-X, so assume 1000BASE-T/100BASE-TX/10BASE-T register set.
+	 */
+	/* Check for 1000BASE-T. */
+	if (miiphy_read (devname, addr, PHY_1000BTSR, &btsr)) {
+		printf ("PHY 1000BT status");
+		goto miiphy_read_failed;
+	}
+	if (btsr != 0xFFFF) {
+		if (btsr & PHY_1000BTSR_1000FD) {
+			return FULL;
+		} else if (btsr & PHY_1000BTSR_1000HD) {
+			return HALF;
 		}
 	}
 #endif /* CONFIG_PHY_GIGE */

 	/* Check Basic Management Control Register first. */
-	if (miiphy_read (devname, addr, PHY_BMCR, &reg)) {
-		puts ("PHY duplex read failed, assuming half duplex\n");
-		return (HALF);
+	if (miiphy_read (devname, addr, PHY_BMCR, &bmcr)) {
+		puts ("PHY duplex");
+		goto miiphy_read_failed;
 	}
 	/* Check if auto-negotiation is on. */
-	if ((reg & PHY_BMCR_AUTON) != 0) {
+	if (bmcr & PHY_BMCR_AUTON) {
 		/* Get auto-negotiation results. */
-		if (miiphy_read (devname, addr, PHY_ANLPAR, &reg)) {
-			puts ("PHY AN duplex read failed, assuming half duplex\n");
-			return (HALF);
-		}
-
-		if ((reg & (PHY_ANLPAR_10FD | PHY_ANLPAR_TXFD)) != 0) {
-			return (FULL);
-		} else {
-			return (HALF);
+		if (miiphy_read (devname, addr, PHY_ANLPAR, &anlpar)) {
+			puts ("PHY AN duplex");
+			goto miiphy_read_failed;
 		}
+		return (anlpar & (PHY_ANLPAR_10FD | PHY_ANLPAR_TXFD)) ?
+		    FULL : HALF;
 	}
 	/* Get speed from basic control settings. */
-	else if (reg & PHY_BMCR_DPLX) {
-		return (FULL);
-	} else {
-		return (HALF);
-	}
+	return (bmcr & PHY_BMCR_DPLX) ? FULL : HALF;
+
+      miiphy_read_failed:
+	printf (" read failed, assuming half duplex\n");
+	return HALF;
+}

+/*****************************************************************************
+ *
+ * Return 1 if PHY supports 1000BASE-X, 0 if PHY supports 10BASE-T/100BASE-TX/
+ * 1000BASE-T, or on error.
+ */
+int miiphy_is_1000base_x (char *devname, unsigned char addr)
+{
+#if defined(CONFIG_PHY_GIGE)
+	u16 exsr;
+
+	if (miiphy_read (devname, addr, PHY_EXSR, &exsr)) {
+		printf ("PHY extended status read failed, assuming no "
+			"1000BASE-X\n");
+		return 0;
+	}
+	return 0 != (exsr & (PHY_EXSR_1000XF | PHY_EXSR_1000XH));
+#else
+	return 0;
+#endif
 }

 #ifdef CFG_FAULT_ECHO_LINK_DOWN
diff --git a/include/miiphy.h b/include/miiphy.h
index 42f2ad0..5518a0a 100644
--- a/include/miiphy.h
+++ b/include/miiphy.h
@@ -41,6 +41,7 @@ int miiphy_info (char *devname, unsigned char addr, unsigned int *oui,
 int miiphy_reset (char *devname, unsigned char addr);
 int miiphy_speed (char *devname, unsigned char addr);
 int miiphy_duplex (char *devname, unsigned char addr);
+int miiphy_is_1000base_x (char *devname, unsigned char addr);
 #ifdef CFG_FAULT_ECHO_LINK_DOWN
 int miiphy_link (char *devname, unsigned char addr);
 #endif
@@ -85,6 +86,7 @@ int bb_miiphy_write (char *devname, unsigned char addr,
 #define PHY_ANLPNP		0x08
 #define PHY_1000BTCR		0x09
 #define PHY_1000BTSR		0x0A
+#define PHY_EXSR		0x0F
 #define PHY_PHYSTS		0x10
 #define PHY_MIPSCR		0x11
 #define PHY_MIPGSR		0x12
@@ -118,6 +120,7 @@ int bb_miiphy_write (char *devname, unsigned char addr,
 #define PHY_BMSR_100TXH		0x2000
 #define PHY_BMSR_10TF		0x1000
 #define PHY_BMSR_10TH		0x0800
+#define PHY_BMSR_EXT_STAT	0x0100
 #define PHY_BMSR_PRE_SUP	0x0040
 #define PHY_BMSR_AUTN_COMP	0x0020
 #define PHY_BMSR_RF		0x0010
@@ -130,17 +133,30 @@ int bb_miiphy_write (char *devname, unsigned char addr,
 #define PHY_ANLPAR_NP		0x8000
 #define PHY_ANLPAR_ACK		0x4000
 #define PHY_ANLPAR_RF		0x2000
+#define PHY_ANLPAR_ASYMP	0x0800
+#define PHY_ANLPAR_PAUSE	0x0400
 #define PHY_ANLPAR_T4		0x0200
 #define PHY_ANLPAR_TXFD		0x0100
 #define PHY_ANLPAR_TX		0x0080
 #define PHY_ANLPAR_10FD		0x0040
 #define PHY_ANLPAR_10		0x0020
 #define PHY_ANLPAR_100		0x0380	/* we can run at 100 */
+/* phy ANLPAR 1000BASE-X */
+#define PHY_X_ANLPAR_NP		0x8000
+#define PHY_X_ANLPAR_ACK	0x4000
+#define PHY_X_ANLPAR_RF_MASK	0x3000
+#define PHY_X_ANLPAR_PAUSE_MASK	0x0180
+#define PHY_X_ANLPAR_HD		0x0040
+#define PHY_X_ANLPAR_FD		0x0020

 #define PHY_ANLPAR_PSB_MASK	0x001f
 #define PHY_ANLPAR_PSB_802_3	0x0001
 #define PHY_ANLPAR_PSB_802_9	0x0002

+/* phy 1000BTCR */
+#define PHY_1000BTCR_1000FD	0x0200
+#define PHY_1000BTCR_1000HD	0x0100
+
 /* phy 1000BTSR */
 #define PHY_1000BTSR_MSCF	0x8000
 #define PHY_1000BTSR_MSCR	0x4000
@@ -149,4 +165,10 @@ int bb_miiphy_write (char *devname, unsigned char addr,
 #define PHY_1000BTSR_1000FD	0x0800
 #define PHY_1000BTSR_1000HD	0x0400

+/* phy EXSR */
+#define PHY_EXSR_1000XF		0x8000
+#define PHY_EXSR_1000XH		0x4000
+#define PHY_EXSR_1000TF		0x2000
+#define PHY_EXSR_1000TH		0x1000
+
 #endif

[U-Boot-Users] [PATCH 2/2 (resubmit)] NET: Add Ethernet 1000BASE-X support for PPC4xx

[Larry Johnson]

This patch adds a new switch: "CONFIG_PHY_DYNAMIC_ANEG".  When this symbol
is defined, the PHY will advertise it's capabilities for autonegotiation
based on the capabilities shown in the PHY's status registers, including
1000BASE-X.  When "CONFIG_PHY_DYNAMIC_ANEG" is not defined, the PHY will
advertise hard-coded capabilities, as before.

Signed-off-by: Larry Johnson <lrj@acm.org>
---

 common/miiphyutil.c |  155 +++++++++++++++++++++++++++++++++------------------
 include/miiphy.h    |   21 +++++++
 2 files changed, 121 insertions(+), 55 deletions(-)

diff --git a/common/miiphyutil.c b/common/miiphyutil.c
index 58ebc5e..b2f62d0 100644
--- a/common/miiphyutil.c
+++ b/common/miiphyutil.c
@@ -344,101 +344,146 @@ int miiphy_reset (char *devname, unsigned char addr)

 /*****************************************************************************
  *
- * Determine the ethernet speed (10/100).
+ * Determine the ethernet speed (10/100/1000).  Return 10 on error.
  */
 int miiphy_speed (char *devname, unsigned char addr)
 {
-	unsigned short reg;
+	u16 bmcr;

 #if defined(CONFIG_PHY_GIGE)
-	if (miiphy_read (devname, addr, PHY_1000BTSR, &reg)) {
-		printf ("PHY 1000BT Status read failed\n");
-	} else {
-		if (reg != 0xFFFF) {
-			if ((reg & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD))
-			    != 0) {
-				return (_1000BASET);
-			}
+	u16 btsr;
+
+#if defined(CONFIG_PHY_DYNAMIC_ANEG)
+	u16 bmsr, exsr;
+
+	/* Check for 1000BASE-X. */
+	if (miiphy_read (devname, addr, PHY_BMSR, &bmsr)) {
+		printf ("PHY status");
+		goto miiphy_read_failed;
+	}
+	if (bmsr & PHY_BMSR_EXT_STAT) {
+
+		if (miiphy_read (devname, addr, PHY_EXSR, &exsr)) {
+			printf ("PHY extended status");
+			goto miiphy_read_failed;
+		}
+		if (exsr & (PHY_EXSR_1000XF | PHY_EXSR_1000XH)) {
+			/* 1000BASE-X */
+			return _1000BASET;
 		}
 	}
+#endif /* defined(CONFIG_PHY_DYNAMIC_ANEG) */
+
+	/* Check for 1000BASE-T. */
+	if (miiphy_read (devname, addr, PHY_1000BTSR, &btsr)) {
+		printf ("PHY 1000BT status");
+		goto miiphy_read_failed;
+	}
+	if (btsr != 0xFFFF &&
+	    (btsr & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD))) {
+		return _1000BASET;
+	}
 #endif /* CONFIG_PHY_GIGE */

 	/* Check Basic Management Control Register first. */
-	if (miiphy_read (devname, addr, PHY_BMCR, &reg)) {
-		puts ("PHY speed read failed, assuming 10bT\n");
-		return (_10BASET);
+	if (miiphy_read (devname, addr, PHY_BMCR, &bmcr)) {
+		printf ("PHY speed");
+		goto miiphy_read_failed;
 	}
 	/* Check if auto-negotiation is on. */
-	if ((reg & PHY_BMCR_AUTON) != 0) {
+	if (bmcr & PHY_BMCR_AUTON) {
 		/* Get auto-negotiation results. */
-		if (miiphy_read (devname, addr, PHY_ANLPAR, &reg)) {
-			puts ("PHY AN speed read failed, assuming 10bT\n");
-			return (_10BASET);
-		}
-		if ((reg & PHY_ANLPAR_100) != 0) {
-			return (_100BASET);
-		} else {
-			return (_10BASET);
+		u16 anlpar;
+
+		if (miiphy_read (devname, addr, PHY_ANLPAR, &anlpar)) {
+			printf ("PHY AN speed");
+			goto miiphy_read_failed;
 		}
+		return (anlpar & PHY_ANLPAR_100) ? _100BASET : _10BASET;
 	}
 	/* Get speed from basic control settings. */
-	else if (reg & PHY_BMCR_100MB) {
-		return (_100BASET);
-	} else {
-		return (_10BASET);
-	}
+	return (bmcr & PHY_BMCR_100MB) ? _100BASET : _10BASET;

+      miiphy_read_failed:
+	printf (" read failed, assuming 10BASE-T\n");
+	return _10BASET;
 }

 /*****************************************************************************
  *
- * Determine full/half duplex.
+ * Determine full/half duplex.  Return half on error.
  */
 int miiphy_duplex (char *devname, unsigned char addr)
 {
-	unsigned short reg;
+	u16 bmcr;

 #if defined(CONFIG_PHY_GIGE)
-	if (miiphy_read (devname, addr, PHY_1000BTSR, &reg)) {
-		printf ("PHY 1000BT Status read failed\n");
-	} else {
-		if ((reg != 0xFFFF) &&
-		    (reg & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD))) {
-			if ((reg & PHY_1000BTSR_1000FD) != 0) {
-				return (FULL);
-			} else {
-				return (HALF);
+	u16 btsr;
+
+#if defined(CONFIG_PHY_DYNAMIC_ANEG)
+	u16 bmsr, exsr;
+
+	/* Check for 1000BASE-X. */
+	if (miiphy_read (devname, addr, PHY_BMSR, &bmsr)) {
+		printf ("PHY status");
+		goto miiphy_read_failed;
+	}
+	if (bmsr & PHY_BMSR_EXT_STAT) {
+
+		if (miiphy_read (devname, addr, PHY_EXSR, &exsr)) {
+			printf ("PHY extended status");
+			goto miiphy_read_failed;
+		}
+		if (exsr & (PHY_EXSR_1000XF | PHY_EXSR_1000XH)) {
+			/* 1000BASE-X */
+			u16 anlpar;
+
+			if (miiphy_read (devname, addr, PHY_ANLPAR, &anlpar)) {
+				printf ("1000BASE-X PHY AN duplex");
+				goto miiphy_read_failed;
 			}
+			return (anlpar & PHY_X_ANLPAR_FD) ? FULL : HALF;
+		}
+	}
+#endif /* defined(CONFIG_PHY_DYNAMIC_ANEG) */
+
+	/* Check for 1000BASE-T. */
+	if (miiphy_read (devname, addr, PHY_1000BTSR, &btsr)) {
+		printf ("PHY 1000BT status");
+		goto miiphy_read_failed;
+	}
+	if (btsr != 0xFFFF) {
+		if (btsr & PHY_1000BTSR_1000FD) {
+			return FULL;
+		} else if (btsr & PHY_1000BTSR_1000HD) {
+			return HALF;
 		}
 	}
 #endif /* CONFIG_PHY_GIGE */

 	/* Check Basic Management Control Register first. */
-	if (miiphy_read (devname, addr, PHY_BMCR, &reg)) {
-		puts ("PHY duplex read failed, assuming half duplex\n");
-		return (HALF);
+	if (miiphy_read (devname, addr, PHY_BMCR, &bmcr)) {
+		puts ("PHY duplex");
+		goto miiphy_read_failed;
 	}
 	/* Check if auto-negotiation is on. */
-	if ((reg & PHY_BMCR_AUTON) != 0) {
+	if (bmcr & PHY_BMCR_AUTON) {
 		/* Get auto-negotiation results. */
-		if (miiphy_read (devname, addr, PHY_ANLPAR, &reg)) {
-			puts ("PHY AN duplex read failed, assuming half duplex\n");
-			return (HALF);
-		}
+		u16 anlpar;

-		if ((reg & (PHY_ANLPAR_10FD | PHY_ANLPAR_TXFD)) != 0) {
-			return (FULL);
-		} else {
-			return (HALF);
+		if (miiphy_read (devname, addr, PHY_ANLPAR, &anlpar)) {
+			puts ("PHY AN duplex");
+			goto miiphy_read_failed;
 		}
+		return (anlpar & (PHY_ANLPAR_10FD | PHY_ANLPAR_TXFD)) ?
+		    FULL : HALF;
 	}
 	/* Get speed from basic control settings. */
-	else if (reg & PHY_BMCR_DPLX) {
-		return (FULL);
-	} else {
-		return (HALF);
-	}
+	return (bmcr & PHY_BMCR_DPLX) ? FULL : HALF;

+      miiphy_read_failed:
+	printf (" read failed, assuming half duplex\n");
+	return HALF;
 }

 #ifdef CFG_FAULT_ECHO_LINK_DOWN
diff --git a/include/miiphy.h b/include/miiphy.h
index 42f2ad0..83234b9 100644
--- a/include/miiphy.h
+++ b/include/miiphy.h
@@ -85,6 +85,7 @@ int bb_miiphy_write (char *devname, unsigned char addr,
 #define PHY_ANLPNP		0x08
 #define PHY_1000BTCR		0x09
 #define PHY_1000BTSR		0x0A
+#define PHY_EXSR		0x0F
 #define PHY_PHYSTS		0x10
 #define PHY_MIPSCR		0x11
 #define PHY_MIPGSR		0x12
@@ -118,6 +119,7 @@ int bb_miiphy_write (char *devname, unsigned char addr,
 #define PHY_BMSR_100TXH		0x2000
 #define PHY_BMSR_10TF		0x1000
 #define PHY_BMSR_10TH		0x0800
+#define PHY_BMSR_EXT_STAT	0x0100
 #define PHY_BMSR_PRE_SUP	0x0040
 #define PHY_BMSR_AUTN_COMP	0x0020
 #define PHY_BMSR_RF		0x0010
@@ -130,17 +132,30 @@ int bb_miiphy_write (char *devname, unsigned char addr,
 #define PHY_ANLPAR_NP		0x8000
 #define PHY_ANLPAR_ACK		0x4000
 #define PHY_ANLPAR_RF		0x2000
+#define PHY_ANLPAR_ASYMP	0x0800
+#define PHY_ANLPAR_PAUSE	0x0400
 #define PHY_ANLPAR_T4		0x0200
 #define PHY_ANLPAR_TXFD		0x0100
 #define PHY_ANLPAR_TX		0x0080
 #define PHY_ANLPAR_10FD		0x0040
 #define PHY_ANLPAR_10		0x0020
 #define PHY_ANLPAR_100		0x0380	/* we can run at 100 */
+/* phy ANLPAR 1000BASE-X */
+#define PHY_X_ANLPAR_NP		0x8000
+#define PHY_X_ANLPAR_ACK	0x4000
+#define PHY_X_ANLPAR_RF_MASK	0x3000
+#define PHY_X_ANLPAR_PAUSE_MASK	0x0180
+#define PHY_X_ANLPAR_HD		0x0040
+#define PHY_X_ANLPAR_FD		0x0020

 #define PHY_ANLPAR_PSB_MASK	0x001f
 #define PHY_ANLPAR_PSB_802_3	0x0001
 #define PHY_ANLPAR_PSB_802_9	0x0002

+/* phy 1000BTCR */
+#define PHY_1000BTCR_1000FD	0x0200
+#define PHY_1000BTCR_1000HD	0x0100
+
 /* phy 1000BTSR */
 #define PHY_1000BTSR_MSCF	0x8000
 #define PHY_1000BTSR_MSCR	0x4000
@@ -149,4 +164,10 @@ int bb_miiphy_write (char *devname, unsigned char addr,
 #define PHY_1000BTSR_1000FD	0x0800
 #define PHY_1000BTSR_1000HD	0x0400

+/* phy EXSR */
+#define PHY_EXSR_1000XF		0x8000
+#define PHY_EXSR_1000XH		0x4000
+#define PHY_EXSR_1000TF		0x2000
+#define PHY_EXSR_1000TH		0x1000
+
 #endif

[U-Boot-Users] [PATCH 2/2 (resubmit)] NET: Add Ethernet 1000BASE-X support for PPC4xx

[Ben Warren]

Larry,

I think this can be simplified a bit. More later on...

Larry Johnson wrote:
> This patch adds a new switch: "CONFIG_PHY_DYNAMIC_ANEG".  When this symbol
> is defined, the PHY will advertise it's capabilities for autonegotiation
> based on the capabilities shown in the PHY's status registers, including
> 1000BASE-X.  When "CONFIG_PHY_DYNAMIC_ANEG" is not defined, the PHY will
> advertise hard-coded capabilities, as before.
>
> Signed-off-by: Larry Johnson <lrj@acm.org>
> ---
>
>  common/miiphyutil.c |  155 +++++++++++++++++++++++++++++++++------------------
>  include/miiphy.h    |   21 +++++++
>  2 files changed, 121 insertions(+), 55 deletions(-)
>
> diff --git a/common/miiphyutil.c b/common/miiphyutil.c
> index 58ebc5e..b2f62d0 100644
> --- a/common/miiphyutil.c
> +++ b/common/miiphyutil.c
> @@ -344,101 +344,146 @@ int miiphy_reset (char *devname, unsigned char addr)
>
>  /*****************************************************************************
>   *
> - * Determine the ethernet speed (10/100).
> + * Determine the ethernet speed (10/100/1000).  Return 10 on error.
>   */
>  int miiphy_speed (char *devname, unsigned char addr)
>  {
> -	unsigned short reg;
> +	u16 bmcr;
>
>  #if defined(CONFIG_PHY_GIGE)
> -	if (miiphy_read (devname, addr, PHY_1000BTSR, &reg)) {
> -		printf ("PHY 1000BT Status read failed\n");
> -	} else {
> -		if (reg != 0xFFFF) {
> -			if ((reg & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD))
> -			    != 0) {
> -				return (_1000BASET);
> -			}
> +	u16 btsr;
> +
> +#if defined(CONFIG_PHY_DYNAMIC_ANEG)
>   
I don't think you need this CONFIG. It doesn't really do anything.
> +	u16 bmsr, exsr;
> +
> +	/* Check for 1000BASE-X. */
> +	if (miiphy_read (devname, addr, PHY_BMSR, &bmsr)) {
> +		printf ("PHY status");
> +		goto miiphy_read_failed;
> +	}
> +	if (bmsr & PHY_BMSR_EXT_STAT) {
> +
> +		if (miiphy_read (devname, addr, PHY_EXSR, &exsr)) {
> +			printf ("PHY extended status");
> +			goto miiphy_read_failed;
> +		}
> +		if (exsr & (PHY_EXSR_1000XF | PHY_EXSR_1000XH)) {
> +			/* 1000BASE-X */
> +			return _1000BASET;
>   
Per IEEE 802.3-2005, All PHYs with capabilities > 100Mbps must have 
PHY_MBSR_EXT_STAT set, and EXSR contains capability info for 1000BX and 
1000BT, so you can check for all four here. Please correct me if I'm wrong.
>  		}
>  	}
> +#endif /* defined(CONFIG_PHY_DYNAMIC_ANEG) */
> +
> +	/* Check for 1000BASE-T. */
> +	if (miiphy_read (devname, addr, PHY_1000BTSR, &btsr)) {
> +		printf ("PHY 1000BT status");
> +		goto miiphy_read_failed;
> +	}
> +	if (btsr != 0xFFFF &&
> +	    (btsr & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD))) {
> +		return _1000BASET;
> +	}
>   
And then you don't need to check 1000BTSR
>  #endif /* CONFIG_PHY_GIGE */
>
>  	/* Check Basic Management Control Register first. */
> -	if (miiphy_read (devname, addr, PHY_BMCR, &reg)) {
> -		puts ("PHY speed read failed, assuming 10bT\n");
> -		return (_10BASET);
> +	if (miiphy_read (devname, addr, PHY_BMCR, &bmcr)) {
> +		printf ("PHY speed");
> +		goto miiphy_read_failed;
>  	}
>  	/* Check if auto-negotiation is on. */
> -	if ((reg & PHY_BMCR_AUTON) != 0) {
> +	if (bmcr & PHY_BMCR_AUTON) {
>  		/* Get auto-negotiation results. */
> -		if (miiphy_read (devname, addr, PHY_ANLPAR, &reg)) {
> -			puts ("PHY AN speed read failed, assuming 10bT\n");
> -			return (_10BASET);
> -		}
> -		if ((reg & PHY_ANLPAR_100) != 0) {
> -			return (_100BASET);
> -		} else {
> -			return (_10BASET);
> +		u16 anlpar;
>   
Please move the anlpar declaration to the top of the function
> +
> +		if (miiphy_read (devname, addr, PHY_ANLPAR, &anlpar)) {
> +			printf ("PHY AN speed");
> +			goto miiphy_read_failed;
>  		}
> +		return (anlpar & PHY_ANLPAR_100) ? _100BASET : _10BASET;
>  	}
>  	/* Get speed from basic control settings. */
> -	else if (reg & PHY_BMCR_100MB) {
> -		return (_100BASET);
> -	} else {
> -		return (_10BASET);
> -	}
> +	return (bmcr & PHY_BMCR_100MB) ? _100BASET : _10BASET;
>
> +      miiphy_read_failed:
> +	printf (" read failed, assuming 10BASE-T\n");
> +	return _10BASET;
>  }
>
>  /*****************************************************************************
>   *
> - * Determine full/half duplex.
> + * Determine full/half duplex.  Return half on error.
>   */
>  int miiphy_duplex (char *devname, unsigned char addr)
>  {
> -	unsigned short reg;
> +	u16 bmcr;
>
>  #if defined(CONFIG_PHY_GIGE)
> -	if (miiphy_read (devname, addr, PHY_1000BTSR, &reg)) {
> -		printf ("PHY 1000BT Status read failed\n");
> -	} else {
> -		if ((reg != 0xFFFF) &&
> -		    (reg & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD))) {
> -			if ((reg & PHY_1000BTSR_1000FD) != 0) {
> -				return (FULL);
> -			} else {
> -				return (HALF);
> +	u16 btsr;
> +
> +#if defined(CONFIG_PHY_DYNAMIC_ANEG)
>   
Same as above. Don't need it, I don't think.
> +	u16 bmsr, exsr;
> +
> +	/* Check for 1000BASE-X. */
> +	if (miiphy_read (devname, addr, PHY_BMSR, &bmsr)) {
> +		printf ("PHY status");
> +		goto miiphy_read_failed;
> +	}
> +	if (bmsr & PHY_BMSR_EXT_STAT) {
> +
> +		if (miiphy_read (devname, addr, PHY_EXSR, &exsr)) {
> +			printf ("PHY extended status");
> +			goto miiphy_read_failed;
> +		}
> +		if (exsr & (PHY_EXSR_1000XF | PHY_EXSR_1000XH)) {
> +			/* 1000BASE-X */
> +			u16 anlpar;
> +
> +			if (miiphy_read (devname, addr, PHY_ANLPAR, &anlpar)) {
> +				printf ("1000BASE-X PHY AN duplex");
> +				goto miiphy_read_failed;
>  			}
> +			return (anlpar & PHY_X_ANLPAR_FD) ? FULL : HALF;
> +		}
> +	}
> +#endif /* defined(CONFIG_PHY_DYNAMIC_ANEG) */
> +
> +	/* Check for 1000BASE-T. */
> +	if (miiphy_read (devname, addr, PHY_1000BTSR, &btsr)) {
> +		printf ("PHY 1000BT status");
> +		goto miiphy_read_failed;
> +	}
> +	if (btsr != 0xFFFF) {
> +		if (btsr & PHY_1000BTSR_1000FD) {
> +			return FULL;
> +		} else if (btsr & PHY_1000BTSR_1000HD) {
> +			return HALF;
>  		}
>   
Same as above. All GigE capabilities are listed in EXSR
>  	}
>  #endif /* CONFIG_PHY_GIGE */
>
>  	/* Check Basic Management Control Register first. */
> -	if (miiphy_read (devname, addr, PHY_BMCR, &reg)) {
> -		puts ("PHY duplex read failed, assuming half duplex\n");
> -		return (HALF);
> +	if (miiphy_read (devname, addr, PHY_BMCR, &bmcr)) {
> +		puts ("PHY duplex");
> +		goto miiphy_read_failed;
>  	}
>  	/* Check if auto-negotiation is on. */
> -	if ((reg & PHY_BMCR_AUTON) != 0) {
> +	if (bmcr & PHY_BMCR_AUTON) {
>  		/* Get auto-negotiation results. */
> -		if (miiphy_read (devname, addr, PHY_ANLPAR, &reg)) {
> -			puts ("PHY AN duplex read failed, assuming half duplex\n");
> -			return (HALF);
> -		}
> +		u16 anlpar;
>   
See comment above
> -		if ((reg & (PHY_ANLPAR_10FD | PHY_ANLPAR_TXFD)) != 0) {
> -			return (FULL);
> -		} else {
> -			return (HALF);
> +		if (miiphy_read (devname, addr, PHY_ANLPAR, &anlpar)) {
> +			puts ("PHY AN duplex");
> +			goto miiphy_read_failed;
>  		}
> +		return (anlpar & (PHY_ANLPAR_10FD | PHY_ANLPAR_TXFD)) ?
> +		    FULL : HALF;
>  	}
>  	/* Get speed from basic control settings. */
> -	else if (reg & PHY_BMCR_DPLX) {
> -		return (FULL);
> -	} else {
> -		return (HALF);
> -	}
> +	return (bmcr & PHY_BMCR_DPLX) ? FULL : HALF;
>
> +      miiphy_read_failed:
> +	printf (" read failed, assuming half duplex\n");
> +	return HALF;
>  }
>
>  #ifdef CFG_FAULT_ECHO_LINK_DOWN
> diff --git a/include/miiphy.h b/include/miiphy.h
> index 42f2ad0..83234b9 100644
> --- a/include/miiphy.h
> +++ b/include/miiphy.h
> @@ -85,6 +85,7 @@ int bb_miiphy_write (char *devname, unsigned char addr,
>  #define PHY_ANLPNP		0x08
>  #define PHY_1000BTCR		0x09
>  #define PHY_1000BTSR		0x0A
> +#define PHY_EXSR		0x0F
>  #define PHY_PHYSTS		0x10
>  #define PHY_MIPSCR		0x11
>  #define PHY_MIPGSR		0x12
> @@ -118,6 +119,7 @@ int bb_miiphy_write (char *devname, unsigned char addr,
>  #define PHY_BMSR_100TXH		0x2000
>  #define PHY_BMSR_10TF		0x1000
>  #define PHY_BMSR_10TH		0x0800
> +#define PHY_BMSR_EXT_STAT	0x0100
>  #define PHY_BMSR_PRE_SUP	0x0040
>  #define PHY_BMSR_AUTN_COMP	0x0020
>  #define PHY_BMSR_RF		0x0010
> @@ -130,17 +132,30 @@ int bb_miiphy_write (char *devname, unsigned char addr,
>  #define PHY_ANLPAR_NP		0x8000
>  #define PHY_ANLPAR_ACK		0x4000
>  #define PHY_ANLPAR_RF		0x2000
> +#define PHY_ANLPAR_ASYMP	0x0800
> +#define PHY_ANLPAR_PAUSE	0x0400
>  #define PHY_ANLPAR_T4		0x0200
>  #define PHY_ANLPAR_TXFD		0x0100
>  #define PHY_ANLPAR_TX		0x0080
>  #define PHY_ANLPAR_10FD		0x0040
>  #define PHY_ANLPAR_10		0x0020
>  #define PHY_ANLPAR_100		0x0380	/* we can run at 100 */
> +/* phy ANLPAR 1000BASE-X */
> +#define PHY_X_ANLPAR_NP		0x8000
> +#define PHY_X_ANLPAR_ACK	0x4000
> +#define PHY_X_ANLPAR_RF_MASK	0x3000
> +#define PHY_X_ANLPAR_PAUSE_MASK	0x0180
> +#define PHY_X_ANLPAR_HD		0x0040
> +#define PHY_X_ANLPAR_FD		0x0020
>
>  #define PHY_ANLPAR_PSB_MASK	0x001f
>  #define PHY_ANLPAR_PSB_802_3	0x0001
>  #define PHY_ANLPAR_PSB_802_9	0x0002
>
> +/* phy 1000BTCR */
> +#define PHY_1000BTCR_1000FD	0x0200
> +#define PHY_1000BTCR_1000HD	0x0100
> +
>  /* phy 1000BTSR */
>  #define PHY_1000BTSR_MSCF	0x8000
>  #define PHY_1000BTSR_MSCR	0x4000
> @@ -149,4 +164,10 @@ int bb_miiphy_write (char *devname, unsigned char addr,
>  #define PHY_1000BTSR_1000FD	0x0800
>  #define PHY_1000BTSR_1000HD	0x0400
>
> +/* phy EXSR */
> +#define PHY_EXSR_1000XF		0x8000
> +#define PHY_EXSR_1000XH		0x4000
> +#define PHY_EXSR_1000TF		0x2000
> +#define PHY_EXSR_1000TH		0x1000
> +
>  #endif
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems?  Stop.
> Now Search log events and configuration files using AJAX and a browser.
> Download your FREE copy of Splunk now >> http://get.splunk.com/
> _______________________________________________
> U-Boot-Users mailing list
> U-Boot-Users at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users
>
>   
Nice work.

regards,
Ben

[U-Boot-Users] [PATCH 2/2 (resubmit)] NET: Add Ethernet 1000BASE-X support for PPC4xx

[Larry Johnson]

Hi Ben,

Thank you for your comments.  See below...

Ben Warren wrote:
> Larry,
> 
> I think this can be simplified a bit. More later on...
> 
> Larry Johnson wrote:
>> This patch adds a new switch: "CONFIG_PHY_DYNAMIC_ANEG".  When this
>> symbol
>> is defined, the PHY will advertise it's capabilities for autonegotiation
>> based on the capabilities shown in the PHY's status registers, including
>> 1000BASE-X.  When "CONFIG_PHY_DYNAMIC_ANEG" is not defined, the PHY will
>> advertise hard-coded capabilities, as before.
>>
>> Signed-off-by: Larry Johnson <lrj@acm.org>
>> ---
>>
>>  common/miiphyutil.c |  155
>> +++++++++++++++++++++++++++++++++------------------
>>  include/miiphy.h    |   21 +++++++
>>  2 files changed, 121 insertions(+), 55 deletions(-)
>>
>> diff --git a/common/miiphyutil.c b/common/miiphyutil.c
>> index 58ebc5e..b2f62d0 100644
>> --- a/common/miiphyutil.c
>> +++ b/common/miiphyutil.c
>> @@ -344,101 +344,146 @@ int miiphy_reset (char *devname, unsigned char
>> addr)
>>
>>  /*****************************************************************************
>>
>>   *
>> - * Determine the ethernet speed (10/100).
>> + * Determine the ethernet speed (10/100/1000).  Return 10 on error.
>>   */
>>  int miiphy_speed (char *devname, unsigned char addr)
>>  {
>> -    unsigned short reg;
>> +    u16 bmcr;
>>
>>  #if defined(CONFIG_PHY_GIGE)
>> -    if (miiphy_read (devname, addr, PHY_1000BTSR, &reg)) {
>> -        printf ("PHY 1000BT Status read failed\n");
>> -    } else {
>> -        if (reg != 0xFFFF) {
>> -            if ((reg & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD))
>> -                != 0) {
>> -                return (_1000BASET);
>> -            }
>> +    u16 btsr;
>> +
>> +#if defined(CONFIG_PHY_DYNAMIC_ANEG)
>>   
> I don't think you need this CONFIG. It doesn't really do anything.

I only put it in to keep the footprint smaller for boards that aren't
using fiber, so I'll get rid if it.

>> +    u16 bmsr, exsr;
>> +
>> +    /* Check for 1000BASE-X. */
>> +    if (miiphy_read (devname, addr, PHY_BMSR, &bmsr)) {
>> +        printf ("PHY status");
>> +        goto miiphy_read_failed;
>> +    }
>> +    if (bmsr & PHY_BMSR_EXT_STAT) {
>> +
>> +        if (miiphy_read (devname, addr, PHY_EXSR, &exsr)) {
>> +            printf ("PHY extended status");
>> +            goto miiphy_read_failed;
>> +        }
>> +        if (exsr & (PHY_EXSR_1000XF | PHY_EXSR_1000XH)) {
>> +            /* 1000BASE-X */
>> +            return _1000BASET;
>>   
> Per IEEE 802.3-2005, All PHYs with capabilities > 100Mbps must have
> PHY_MBSR_EXT_STAT set, and EXSR contains capability info for 1000BX and
> 1000BT, so you can check for all four here. Please correct me if I'm wrong.

Checking PHY_BMSR_EXT_STAT when CONIF_PHY_GIGE is defined does seem a
bit paranoid, now that you mention it.

For the rest, let me explain my thinking.  The board I'm writing for,
named "Korat", has ethernet ports that are configurable for either
10BASE-T/100BASE-TX/1000BASE-T copper, or 1000BASE-X fiber.  IEEE
defines some registers differently depending on which is configured.  To
determine board-independently how the registers are to be interpreted, I
look in the extended status register.  If 1000BASE-X full or half duplex
is listed as a capability, I assume fiber, otherwise copper.  This works
on the PHY I'm using, and I can't see it working differently on another
PHY because some bits in some registers are used differently depending on
the medium.

At this particular point in the code, if the PHY is configured for 1000BASE-X,
then I report the speed as 1000 without checking for any autonegotiation
results.  If the PHY is configured for copper, then it is still necessary to
check 1000BTSR (as in the original code) to see what speed was negotiated.

>>          }
>>      }
>> +#endif /* defined(CONFIG_PHY_DYNAMIC_ANEG) */
>> +
>> +    /* Check for 1000BASE-T. */
>> +    if (miiphy_read (devname, addr, PHY_1000BTSR, &btsr)) {
>> +        printf ("PHY 1000BT status");
>> +        goto miiphy_read_failed;
>> +    }
>> +    if (btsr != 0xFFFF &&
>> +        (btsr & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD))) {
>> +        return _1000BASET;
>> +    }
>>   
> And then you don't need to check 1000BTSR

See above.

>>  #endif /* CONFIG_PHY_GIGE */
>>
>>      /* Check Basic Management Control Register first. */
>> -    if (miiphy_read (devname, addr, PHY_BMCR, &reg)) {
>> -        puts ("PHY speed read failed, assuming 10bT\n");
>> -        return (_10BASET);
>> +    if (miiphy_read (devname, addr, PHY_BMCR, &bmcr)) {
>> +        printf ("PHY speed");
>> +        goto miiphy_read_failed;
>>      }
>>      /* Check if auto-negotiation is on. */
>> -    if ((reg & PHY_BMCR_AUTON) != 0) {
>> +    if (bmcr & PHY_BMCR_AUTON) {
>>          /* Get auto-negotiation results. */
>> -        if (miiphy_read (devname, addr, PHY_ANLPAR, &reg)) {
>> -            puts ("PHY AN speed read failed, assuming 10bT\n");
>> -            return (_10BASET);
>> -        }
>> -        if ((reg & PHY_ANLPAR_100) != 0) {
>> -            return (_100BASET);
>> -        } else {
>> -            return (_10BASET);
>> +        u16 anlpar;
>>   
> Please move the anlpar declaration to the top of the function

OK.  (I missed the anlpar declarations. Sorry.)

>> +
>> +        if (miiphy_read (devname, addr, PHY_ANLPAR, &anlpar)) {
>> +            printf ("PHY AN speed");
>> +            goto miiphy_read_failed;
>>          }
>> +        return (anlpar & PHY_ANLPAR_100) ? _100BASET : _10BASET;
>>      }
>>      /* Get speed from basic control settings. */
>> -    else if (reg & PHY_BMCR_100MB) {
>> -        return (_100BASET);
>> -    } else {
>> -        return (_10BASET);
>> -    }
>> +    return (bmcr & PHY_BMCR_100MB) ? _100BASET : _10BASET;
>>
>> +      miiphy_read_failed:
>> +    printf (" read failed, assuming 10BASE-T\n");
>> +    return _10BASET;
>>  }
>>
>>  /*****************************************************************************
>>
>>   *
>> - * Determine full/half duplex.
>> + * Determine full/half duplex.  Return half on error.
>>   */
>>  int miiphy_duplex (char *devname, unsigned char addr)
>>  {
>> -    unsigned short reg;
>> +    u16 bmcr;
>>
>>  #if defined(CONFIG_PHY_GIGE)
>> -    if (miiphy_read (devname, addr, PHY_1000BTSR, &reg)) {
>> -        printf ("PHY 1000BT Status read failed\n");
>> -    } else {
>> -        if ((reg != 0xFFFF) &&
>> -            (reg & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD))) {
>> -            if ((reg & PHY_1000BTSR_1000FD) != 0) {
>> -                return (FULL);
>> -            } else {
>> -                return (HALF);
>> +    u16 btsr;
>> +
>> +#if defined(CONFIG_PHY_DYNAMIC_ANEG)
>>   
> Same as above. Don't need it, I don't think.

Good.

>> +    u16 bmsr, exsr;
>> +
>> +    /* Check for 1000BASE-X. */
>> +    if (miiphy_read (devname, addr, PHY_BMSR, &bmsr)) {
>> +        printf ("PHY status");
>> +        goto miiphy_read_failed;
>> +    }
>> +    if (bmsr & PHY_BMSR_EXT_STAT) {
>> +
>> +        if (miiphy_read (devname, addr, PHY_EXSR, &exsr)) {
>> +            printf ("PHY extended status");
>> +            goto miiphy_read_failed;
>> +        }
>> +        if (exsr & (PHY_EXSR_1000XF | PHY_EXSR_1000XH)) {
>> +            /* 1000BASE-X */
>> +            u16 anlpar;
>> +
>> +            if (miiphy_read (devname, addr, PHY_ANLPAR, &anlpar)) {
>> +                printf ("1000BASE-X PHY AN duplex");
>> +                goto miiphy_read_failed;
>>              }
>> +            return (anlpar & PHY_X_ANLPAR_FD) ? FULL : HALF;
>> +        }
>> +    }
>> +#endif /* defined(CONFIG_PHY_DYNAMIC_ANEG) */
>> +
>> +    /* Check for 1000BASE-T. */
>> +    if (miiphy_read (devname, addr, PHY_1000BTSR, &btsr)) {
>> +        printf ("PHY 1000BT status");
>> +        goto miiphy_read_failed;
>> +    }
>> +    if (btsr != 0xFFFF) {
>> +        if (btsr & PHY_1000BTSR_1000FD) {
>> +            return FULL;
>> +        } else if (btsr & PHY_1000BTSR_1000HD) {
>> +            return HALF;
>>          }
>>   
> Same as above. All GigE capabilities are listed in EXSR

See note above.  Again it's not the capabilities, but the autonegotiation
results that need to be checked.  In this case, I cannot make any assumption
based on being in 1000BASE-X mode, but must check the autonegotiation result,
which is in ANLPAR, but with different bit definitions than for 10/100 Mbs.

>>      }
>>  #endif /* CONFIG_PHY_GIGE */
>>
>>      /* Check Basic Management Control Register first. */
>> -    if (miiphy_read (devname, addr, PHY_BMCR, &reg)) {
>> -        puts ("PHY duplex read failed, assuming half duplex\n");
>> -        return (HALF);
>> +    if (miiphy_read (devname, addr, PHY_BMCR, &bmcr)) {
>> +        puts ("PHY duplex");
>> +        goto miiphy_read_failed;
>>      }
>>      /* Check if auto-negotiation is on. */
>> -    if ((reg & PHY_BMCR_AUTON) != 0) {
>> +    if (bmcr & PHY_BMCR_AUTON) {
>>          /* Get auto-negotiation results. */
>> -        if (miiphy_read (devname, addr, PHY_ANLPAR, &reg)) {
>> -            puts ("PHY AN duplex read failed, assuming half duplex\n");
>> -            return (HALF);
>> -        }
>> +        u16 anlpar;
>>   
> See comment above

Sorry, again.

[snip]

> Nice work.
> 
> regards,
> Ben

Thank you.  I'm thinking now that there should now be a function to report
whether the PHY is in 1000BASE-X mode, to go along with the speed and duplex
functions.  I'd like to try that to see if it makes the code clearer.  I'll
post the revised patch after I've had a chance to test it to get your and
others' opinions.

Best regards,
Larry

[U-Boot-Users] [PATCH 2/2 (resubmit)] NET: Add Ethernet 1000BASE-X support for PPC4xx

[Larry Johnson]

Ben Warren wrote:
> Larry Johnson wrote:
>> Hi Ben,
>>
>> Thank you for your comments.  See below...
>>
>> Ben Warren wrote:
>>  
>>> Larry,
>>>
>>> I think this can be simplified a bit. More later on...
>>>
>>> Larry Johnson wrote:
>>>    
>>>> This patch adds a new switch: "CONFIG_PHY_DYNAMIC_ANEG".  When this symbol
>>>> is defined, the PHY will advertise it's capabilities for autonegotiation
>>>> based on the capabilities shown in the PHY's status registers, including
>>>> 1000BASE-X.  When "CONFIG_PHY_DYNAMIC_ANEG" is not defined, the PHY will
>>>> advertise hard-coded capabilities, as before.
>>>>
>>>> Signed-off-by: Larry Johnson <lrj@acm.org>
>>>> ---
>>>>
>>>>  common/miiphyutil.c |  155
>>>> +++++++++++++++++++++++++++++++++------------------
>>>>  include/miiphy.h    |   21 +++++++
>>>>  2 files changed, 121 insertions(+), 55 deletions(-)
>>>>
>>>> diff --git a/common/miiphyutil.c b/common/miiphyutil.c
>>>> index 58ebc5e..b2f62d0 100644
>>>> --- a/common/miiphyutil.c
>>>> +++ b/common/miiphyutil.c
>>>> @@ -344,101 +344,146 @@ int miiphy_reset (char *devname, unsigned char
>>>> addr)
>>>>
>>>>  /*****************************************************************************
>>>>
>>>>
>>>>   *
>>>> - * Determine the ethernet speed (10/100).
>>>> + * Determine the ethernet speed (10/100/1000).  Return 10 on error.
>>>>   */
>>>>  int miiphy_speed (char *devname, unsigned char addr)
>>>>  {
>>>> -    unsigned short reg;
>>>> +    u16 bmcr;
>>>>
>>>>  #if defined(CONFIG_PHY_GIGE)
>>>> -    if (miiphy_read (devname, addr, PHY_1000BTSR, &reg)) {
>>>> -        printf ("PHY 1000BT Status read failed\n");
>>>> -    } else {
>>>> -        if (reg != 0xFFFF) {
>>>> -            if ((reg & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD))
>>>> -                != 0) {
>>>> -                return (_1000BASET);
>>>> -            }
>>>> +    u16 btsr;
>>>> +
>>>> +#if defined(CONFIG_PHY_DYNAMIC_ANEG)
>>>>         
>>> I don't think you need this CONFIG. It doesn't really do anything.
>>>     
>>
>> I only put it in to keep the footprint smaller for boards that aren't
>> using fiber, so I'll get rid if it.
>>
>>   
> Yeah, and I like that train of thought, but I think clarity wins out
> here, and I doubt there are that many GigE boards that can't handle a
> few extra bytes (Don't tell Wolfgang I said that)
> 
[snip]

Hi Ben and Stefan,

Thank you for incorporating my patch, Ben.

I have merged my changes into the PPC4xx "for-1.3.1" branch.  When I do
the MAKEALL there, the build for Ocotea board fails because the binary
is now 0xAC byte too large.  What are your opinions on the best way to
handle this?

Best regards,
Larry

[U-Boot-Users] [PATCH 2/2 (resubmit)] NET: Add Ethernet 1000BASE-X support for PPC4xx

[Stefan Roese]

On Wednesday 14 November 2007, Larry Johnson wrote:
> Thank you for incorporating my patch, Ben.
>
> I have merged my changes into the PPC4xx "for-1.3.1" branch.  When I do
> the MAKEALL there, the build for Ocotea board fails because the binary
> is now 0xAC byte too large.  What are your opinions on the best way to
> handle this?

Which gcc version did you use?

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] [PATCH 2/2 (resubmit)] NET: Add Ethernet 1000BASE-X support for PPC4xx

[Larry Johnson]

Stefan Roese wrote:
> On Wednesday 14 November 2007, Larry Johnson wrote:
>> Thank you for incorporating my patch, Ben.
>>
>> I have merged my changes into the PPC4xx "for-1.3.1" branch.  When I do
>> the MAKEALL there, the build for Ocotea board fails because the binary
>> is now 0xAC byte too large.  What are your opinions on the best way to
>> handle this?
> 
> Which gcc version did you use?
> 
> Best regards,
> Stefan
> 
> =====================================================================
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
> =====================================================================

ppc_4xxFP-gcc (GCC) 4.0.0 (DENX ELDK 4.1 4.0.0)

Best regards,
Larry

[U-Boot-Users] [PATCH 2/2 (resubmit)] NET: Add Ethernet 1000BASE-X support for PPC4xx

[Stefan Roese]

Hi Larry,

On Wednesday 14 November 2007, Larry Johnson wrote:
> >> I have merged my changes into the PPC4xx "for-1.3.1" branch.  When I do
> >> the MAKEALL there, the build for Ocotea board fails because the binary
> >> is now 0xAC byte too large.  What are your opinions on the best way to
> >> handle this?
> >
> > Which gcc version did you use?
>
> ppc_4xxFP-gcc (GCC) 4.0.0 (DENX ELDK 4.1 4.0.0)

Too bad. I had hoped it was 3.x.

OK, I will take care of it after the merge window opens.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] [PATCH 2/2 (resubmit)] NET: Add Ethernet 1000BASE-X support for PPC4xx

[Ben Warren]

Larry Johnson wrote:
> Ben Warren wrote:
>   
>> Larry Johnson wrote:
>>     
>>> Hi Ben,
>>>
>>> Thank you for your comments.  See below...
>>>
>>> Ben Warren wrote:
>>>  
>>>       
>>>> Larry,
>>>>
>>>> I think this can be simplified a bit. More later on...
>>>>
>>>> Larry Johnson wrote:
>>>>    
>>>>         
>>>>> This patch adds a new switch: "CONFIG_PHY_DYNAMIC_ANEG".  When this symbol
>>>>> is defined, the PHY will advertise it's capabilities for autonegotiation
>>>>> based on the capabilities shown in the PHY's status registers, including
>>>>> 1000BASE-X.  When "CONFIG_PHY_DYNAMIC_ANEG" is not defined, the PHY will
>>>>> advertise hard-coded capabilities, as before.
>>>>>
>>>>> Signed-off-by: Larry Johnson <lrj@acm.org>
>>>>> ---
>>>>>
>>>>>  common/miiphyutil.c |  155
>>>>> +++++++++++++++++++++++++++++++++------------------
>>>>>  include/miiphy.h    |   21 +++++++
>>>>>  2 files changed, 121 insertions(+), 55 deletions(-)
>>>>>
>>>>> diff --git a/common/miiphyutil.c b/common/miiphyutil.c
>>>>> index 58ebc5e..b2f62d0 100644
>>>>> --- a/common/miiphyutil.c
>>>>> +++ b/common/miiphyutil.c
>>>>> @@ -344,101 +344,146 @@ int miiphy_reset (char *devname, unsigned char
>>>>> addr)
>>>>>
>>>>>  /*****************************************************************************
>>>>>
>>>>>
>>>>>   *
>>>>> - * Determine the ethernet speed (10/100).
>>>>> + * Determine the ethernet speed (10/100/1000).  Return 10 on error.
>>>>>   */
>>>>>  int miiphy_speed (char *devname, unsigned char addr)
>>>>>  {
>>>>> -    unsigned short reg;
>>>>> +    u16 bmcr;
>>>>>
>>>>>  #if defined(CONFIG_PHY_GIGE)
>>>>> -    if (miiphy_read (devname, addr, PHY_1000BTSR, &reg)) {
>>>>> -        printf ("PHY 1000BT Status read failed\n");
>>>>> -    } else {
>>>>> -        if (reg != 0xFFFF) {
>>>>> -            if ((reg & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD))
>>>>> -                != 0) {
>>>>> -                return (_1000BASET);
>>>>> -            }
>>>>> +    u16 btsr;
>>>>> +
>>>>> +#if defined(CONFIG_PHY_DYNAMIC_ANEG)
>>>>>         
>>>>>           
>>>> I don't think you need this CONFIG. It doesn't really do anything.
>>>>     
>>>>         
>>> I only put it in to keep the footprint smaller for boards that aren't
>>> using fiber, so I'll get rid if it.
>>>
>>>   
>>>       
>> Yeah, and I like that train of thought, but I think clarity wins out
>> here, and I doubt there are that many GigE boards that can't handle a
>> few extra bytes (Don't tell Wolfgang I said that)
>>
>>     
> [snip]
>
> Hi Ben and Stefan,
>
> Thank you for incorporating my patch, Ben.
>
> I have merged my changes into the PPC4xx "for-1.3.1" branch.  When I do
> the MAKEALL there, the build for Ocotea board fails because the binary
> is now 0xAC byte too large.  What are your opinions on the best way to
> handle this?
>
> Best regards,
> Larry
>   
That sucks. I don't have this issue with my setup, but I also don't have 
anything to do with ppc4xx. Hopefully Stefan can shed some light.

regards,
ben