Fix and optimize MII operations on FEC (MPC8xx) controllers

This patch fixes several issues at least on a MPC885 based system with two
FEC interfaces used in MII mode.

1. PHY discovery should first read PHY_PHYIDR2 register and only then
   PHY_PHYIDR1 like cpu/mpc8xx/fec.c::mii_discover_phy() does it,
   otherwise the values read are wrong. Also notice, that PHY discovery
   cannot work on MPC88x / MPC87x in setups with both FECs active at all
   in its present form, because for both interfaces the registers from FEC
   1 are used to communicate over MII.

2. Remove code duplication for resetting the FEC by isolating it into a
   separate function.

3. Initialize MII on FEC 1 when communicating over FEC 2 in fec_init().

4. Optimize mii_init() to only reset the FEC 1 controller once.

5. Fix a typo in mii_init() using index i instead of j thus potentially
   leading to unpredictable results.

Signed-off-by: Guennadi Liakhovetski <lg@denx.de>

1 files changed, 53 insertions, 54 deletions

diff --git a/cpu/mpc8xx/fec.c b/cpu/mpc8xx/fec.c
index 08a3715812..da473ca0b1 100644
--- a/cpu/mpc8xx/fec.c
+++ b/cpu/mpc8xx/fec.c
@@ -143,6 +143,7 @@ static int fec_send(struct eth_device* dev, volatile void *packet, int length);
 static int fec_recv(struct eth_device* dev);
 static int fec_init(struct eth_device* dev, bd_t * bd);
 static void fec_halt(struct eth_device* dev);
+static void __mii_init(void);
 
 int fec_initialize(bd_t *bis)
 {
@@ -539,6 +540,30 @@ static void fec_pin_init(int fecidx)
 	}
 }
 
+static int fec_reset(volatile fec_t *fecp)
+{
+	int i;
+
+	/* Whack a reset.
+	 * A delay is required between a reset of the FEC block and
+	 * initialization of other FEC registers because the reset takes
+	 * some time to complete. If you don't delay, subsequent writes
+	 * to FEC registers might get killed by the reset routine which is
+	 * still in progress.
+	 */
+
+	fecp->fec_ecntrl = FEC_ECNTRL_PINMUX | FEC_ECNTRL_RESET;
+	for (i = 0;
+	     (fecp->fec_ecntrl & FEC_ECNTRL_RESET) && (i < FEC_RESET_DELAY);
+	     ++i) {
+		udelay (1);
+	}
+	if (i == FEC_RESET_DELAY)
+		return -1;
+
+	return 0;
+}
+
 static int fec_init (struct eth_device *dev, bd_t * bd)
 {
 	struct ether_fcc_info_s *efis = dev->priv;
@@ -573,23 +598,17 @@ static int fec_init (struct eth_device *dev, bd_t * bd)
 #endif /* CONFIG_FADS */
 	}
 
-	/* Whack a reset.
-	 * A delay is required between a reset of the FEC block and
-	 * initialization of other FEC registers because the reset takes
-	 * some time to complete. If you don't delay, subsequent writes
-	 * to FEC registers might get killed by the reset routine which is
-	 * still in progress.
+#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII)
+	/* the MII interface is connected to FEC1
+	 * so for the miiphy_xxx function to work we must
+	 * call mii_init since fec_halt messes the thing up
 	 */
-	fecp->fec_ecntrl = FEC_ECNTRL_PINMUX | FEC_ECNTRL_RESET;
-	for (i = 0;
-	     (fecp->fec_ecntrl & FEC_ECNTRL_RESET) && (i < FEC_RESET_DELAY);
-	     ++i) {
-		udelay (1);
-	}
-	if (i == FEC_RESET_DELAY) {
+	if (efis->ether_index != 0)
+		__mii_init();
+#endif
+
+	if (fec_reset(fecp) < 0)
 		printf ("FEC_RESET_DELAY timeout\n");
-		return 0;
-	}
 
 	/* We use strictly polling mode only
 	 */
@@ -603,7 +622,7 @@ static int fec_init (struct eth_device *dev, bd_t * bd)
 
 	/* Set station address
 	 */
-#define ea eth_get_dev()->enetaddr
+#define ea dev->enetaddr
 	fecp->fec_addr_low = (ea[0] << 24) | (ea[1] << 16) | (ea[2] << 8) | (ea[3]);
 	fecp->fec_addr_high = (ea[4] << 8) | (ea[5]);
 #undef ea
@@ -716,15 +735,8 @@ static int fec_init (struct eth_device *dev, bd_t * bd)
 	} else {
 		efis->actual_phy_addr = efis->phy_addr;
 	}
-#if defined(CONFIG_MII) && defined(CONFIG_RMII)
-
-	/* the MII interface is connected to FEC1
-	 * so for the miiphy_xxx function to work we must
-	 * call mii_init since fec_halt messes the thing up
-	 */
-	if (efis->ether_index != 0)
-		mii_init();
 
+#if defined(CONFIG_MII) && defined(CONFIG_RMII)
 	/*
 	 * adapt the RMII speed to the speed of the phy
 	 */
@@ -874,15 +886,14 @@ static int mii_discover_phy(struct eth_device *dev)
 			udelay(10000);	/* wait 10ms */
 		}
 		for (phyno = 0; phyno < 32 && phyaddr < 0; ++phyno) {
-			phytype = mii_send(mk_mii_read(phyno, PHY_PHYIDR1));
+			phytype = mii_send(mk_mii_read(phyno, PHY_PHYIDR2));
 #ifdef ET_DEBUG
 			printf("PHY type 0x%x pass %d type ", phytype, pass);
 #endif
 			if (phytype != 0xffff) {
 				phyaddr = phyno;
-				phytype <<= 16;
 				phytype |= mii_send(mk_mii_read(phyno,
-								PHY_PHYIDR2));
+								PHY_PHYIDR1)) << 16;
 
 #ifdef ET_DEBUG
 				printf("PHY @ 0x%x pass %d type ",phyno,pass);
@@ -929,36 +940,17 @@ static int mii_discover_phy(struct eth_device *dev)
 #if (defined(CONFIG_MII) || defined(CONFIG_CMD_MII)) && !defined(CONFIG_BITBANGMII)
 
 /****************************************************************************
- * mii_init -- Initialize the MII for MII command without ethernet
+ * mii_init -- Initialize the MII via FEC 1 for MII command without ethernet
  * This function is a subset of eth_init
  ****************************************************************************
  */
-void mii_init (void)
+static void __mii_init(void)
 {
 	volatile immap_t *immr = (immap_t *) CFG_IMMR;
 	volatile fec_t *fecp = &(immr->im_cpm.cp_fec);
-	int i, j;
 
-	for (j = 0; j < sizeof(ether_fcc_info) / sizeof(ether_fcc_info[0]); j++) {
-
-	/* Whack a reset.
-	 * A delay is required between a reset of the FEC block and
-	 * initialization of other FEC registers because the reset takes
-	 * some time to complete. If you don't delay, subsequent writes
-	 * to FEC registers might get killed by the reset routine which is
-	 * still in progress.
-	 */
-
-	fecp->fec_ecntrl = FEC_ECNTRL_PINMUX | FEC_ECNTRL_RESET;
-	for (i = 0;
-	     (fecp->fec_ecntrl & FEC_ECNTRL_RESET) && (i < FEC_RESET_DELAY);
-	     ++i) {
-		udelay (1);
-	}
-	if (i == FEC_RESET_DELAY) {
+	if (fec_reset(fecp) < 0)
 		printf ("FEC_RESET_DELAY timeout\n");
-		return;
-	}
 
 	/* We use strictly polling mode only
 	 */
@@ -968,14 +960,21 @@ void mii_init (void)
 	 */
 	fecp->fec_ievent = 0xffc0;
 
-	/* Setup the pin configuration of the FEC(s)
-	*/
-		fec_pin_init(ether_fcc_info[i].ether_index);
-
 	/* Now enable the transmit and receive processing
 	 */
 	fecp->fec_ecntrl = FEC_ECNTRL_PINMUX | FEC_ECNTRL_ETHER_EN;
-	}
+}
+
+void mii_init (void)
+{
+	int i;
+
+	__mii_init();
+
+	/* Setup the pin configuration of the FEC(s)
+	*/
+	for (i = 0; i < sizeof(ether_fcc_info) / sizeof(ether_fcc_info[0]); i++)
+		fec_pin_init(ether_fcc_info[i].ether_index);
 }
 
 /*****************************************************************************