Re: was: FEC on MPC860T & race condition

[Steven Scholz <steven.scholz@imc-berlin.de>]

[-- Attachment #1: Type: text/plain, Size: 742 bytes --]

I wrote:

>>I just hab a look at arch/ppc/8xx_io/fec.c in the recent linuxppc_2_4 and
>>could find this problem fixed! The MII irq is still requested very early
>>without waiting for the PHY to discovered!
Damn it!!! Stupid fingers.
I meant to write "could NOT find this problem fixed" !

> So linuxppc_2_4 has a partial fix then is what you're saying?  Around
> what lines?  I'm interested to see why _devel doesn't have this.

So IHMO there is neither in recent linuxppc_2_4_devel nor in linuxppc_2_4 a fix
for the mentioned problem.

I'll attach my patch. It is not configurable since I think it won't break anything.

If you need or wish something like CONFIG_FEC_FIX_RACE or
CONFIG_FEC_LATE_ENABLE_PHY I could do that!

Regards,

Steven

[-- Attachment #2: fec8xx_fix_race.patch --]
[-- Type: text/plain, Size: 2447 bytes --]

Index: arch/ppc/8xx_io/fec.c
===================================================================
RCS file: /home/cvsroot/linuxppc_2_4_denx/arch/ppc/8xx_io/fec.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 fec.c
--- arch/ppc/8xx_io/fec.c	15 Dec 2002 05:06:41 -0000	1.1.1.1
+++ arch/ppc/8xx_io/fec.c	31 Jan 2003 14:55:18 -0000
@@ -33,6 +33,10 @@
  *
  * Added code for Multicast support, Frederic Goddeeris, Paul Geerinckx
  * Copyright (c) 2002 Siemens Atea
+ *
+ * Fix compiler warning: changed type of mem_addr
+ * Fix to avoid race condition: wait until PHY is discovered
+ *                              before requesting the MII irq
  */

 #include <linux/config.h>
@@ -2032,7 +2036,7 @@
 	struct fec_enet_private *fep;
 	int i, j, k;
 	unsigned char	*eap, *iap, *ba;
-	unsigned long	mem_addr;
+	dma_addr_t	mem_addr;
 	volatile	cbd_t	*bdp;
 	cbd_t		*cbd_base;
 	volatile	immap_t	*immap;
@@ -2172,17 +2176,6 @@
 	*((uint *) RPX_CSR_ADDR) &= ~BCSR2_FETHLEDMODE;
 #endif	/* CONFIG_RPXCLASSIC */

-#ifdef CONFIG_USE_MDIO
-# ifndef PHY_INTERRUPT
-#  error Want to use MII, but PHY_INTERRUPT not defined!
-# endif
-	((immap_t *)IMAP_ADDR)->im_siu_conf.sc_siel |=
-		(0x80000000 >> PHY_INTERRUPT);
-
-	if (request_8xxirq(PHY_INTERRUPT, mii_link_interrupt, 0, "mii", dev) != 0)
-		panic("Could not allocate MII IRQ!");
-#endif	/* CONFIG_USE_MDIO */
-
 	dev->base_addr = (unsigned long)fecp;
 	dev->priv = fep;

@@ -2285,6 +2278,39 @@
 	fep->old_status = 0;
 #endif	/* CONFIG_USE_MDIO */

+#ifdef CONFIG_USE_MDIO
+# ifndef PHY_INTERRUPT
+#  error Want to use MII, but PHY_INTERRUPT not defined!
+# endif
+	/* before requesting the irq, we should wait until PHY is discovered
+	 * to avoid race conditions
+	 */
+	while (!fep->phy_id_done) {
+		udelay(5);
+	}
+
+	/* I found the following lines in a post from yhkim@da-san.com>
+	 * (http://lists.linuxppc.org/linuxppc-embedded/200110/msg00206.html)
+	 * But I am not sure if and why this is necessary.
+	 * Enable it just to be sure, be sure ... ?
+	 * It works for me without it.
+	 */
+#if 0
+	mii_do_cmd(dev, fep->phy->ack_int);
+	mii_do_cmd(dev, fep->phy->startup);
+
+	{
+		int tmp;
+		for (tmp = 0; tmp < 50; tmp++)
+			udelay(5);
+	}
+#endif
+	if (request_8xxirq(PHY_INTERRUPT, mii_link_interrupt, 0, "mii", dev) != 0)
+		panic("Could not allocate MII IRQ!");
+
+	((immap_t *)IMAP_ADDR)->im_siu_conf.sc_siel |=
+		(0x80000000 >> PHY_INTERRUPT);
+#endif	/* CONFIG_USE_MDIO */
 	return 0;
 }