* was: FEC on MPC860T & race condition @ 2003-01-30 17:05 Steven Scholz 2003-01-30 20:22 ` Dan Malek 0 siblings, 1 reply; 13+ messages in thread From: Steven Scholz @ 2003-01-30 17:05 UTC (permalink / raw) To: Linuxppc-Embedded, laurent.pinchart, dan Hi there, in October 2001 (!) there were two threads, one was "FEC on MPC860T & race condition" and the other was "Problem using FEC on a860T" You, Laurent, told us about a race condition. You, Dan, first claimed, it was already fix but mentioned later that the patch was somehow lost. Now I have this problem as well and realised that in 2.4.20 the "mii_link_interrupt interrupt handler is still registered before the PHY chip is initialized". Are there any plans to fix that? Laurent, did you fix taht for your board properly or just as a work around? Thanks a million, Steven ** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: was: FEC on MPC860T & race condition 2003-01-30 17:05 was: FEC on MPC860T & race condition Steven Scholz @ 2003-01-30 20:22 ` Dan Malek 2003-01-31 9:16 ` Steven Scholz 0 siblings, 1 reply; 13+ messages in thread From: Dan Malek @ 2003-01-30 20:22 UTC (permalink / raw) To: Steven Scholz; +Cc: Linuxppc-Embedded, laurent.pinchart Steven Scholz wrote: > You, Laurent, told us about a race condition. > You, Dan, first claimed, it was already fix but mentioned later that the > patch > was somehow lost. IIRC, it was the usual problem of having it fixed in only one of the two 2.4 trees. I ensured both trees were made up to date. > Now I have this problem as well and realised that in 2.4.20 the > "mii_link_interrupt interrupt handler is still registered before the > PHY chip is initialized". The link interrupt is board design and PHY dependent. I don't think there is any one "right" way to initialize this interrupt handler. If you have something that isn't working to your liking, we may have to extend the MII logic in some way to accomodate this. Keep in mind that the existing driver is likely working with other boards, and you don't want to break those configurations. Thanks. -- Dan ** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: was: FEC on MPC860T & race condition 2003-01-30 20:22 ` Dan Malek @ 2003-01-31 9:16 ` Steven Scholz 2003-02-03 21:09 ` Tom Rini 0 siblings, 1 reply; 13+ messages in thread From: Steven Scholz @ 2003-01-31 9:16 UTC (permalink / raw) To: Dan Malek; +Cc: Linuxppc-Embedded, laurent.pinchart Dan Malek wrote: > Steven Scholz wrote: > >> You, Laurent, told us about a race condition. >> You, Dan, first claimed, it was already fix but mentioned later that >> the patch >> was somehow lost. > > > IIRC, it was the usual problem of having it fixed in only one of the > two 2.4 trees. I ensured both trees were made up to date. > There is still no fix in linuxppc_2_4_devel! >> Now I have this problem as well and realised that in 2.4.20 the >> "mii_link_interrupt interrupt handler is still registered before the >> PHY chip is initialized". > > The link interrupt is board design and PHY dependent. I don't think there > is any one "right" way to initialize this interrupt handler. If you have > something that isn't working to your liking, we may have to extend the > MII logic in some way to accomodate this. Keep in mind that the existing > driver is likely working with other boards, and you don't want to break > those configurations. There is a race condition! Lucky you if you never see it! :o) But fair enough. If more people had this problem, the fix would be in the trees by now. How about adding something like CONFIG_FEC_LATE_ENABLE_PHY or so. Of course with a help text in Documentation/Configure.help so that people having that problem could easily enable that. And for the rest (although this might be the majority) everythings stays as it it. Steven ** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: was: FEC on MPC860T & race condition 2003-01-31 9:16 ` Steven Scholz @ 2003-02-03 21:09 ` Tom Rini 2003-02-04 8:39 ` Steven Scholz 0 siblings, 1 reply; 13+ messages in thread From: Tom Rini @ 2003-02-03 21:09 UTC (permalink / raw) To: Steven Scholz; +Cc: Dan Malek, Linuxppc-Embedded, laurent.pinchart On Fri, Jan 31, 2003 at 10:16:25AM +0100, Steven Scholz wrote: > > Dan Malek wrote: > >Steven Scholz wrote: > > > >>You, Laurent, told us about a race condition. > >>You, Dan, first claimed, it was already fix but mentioned later that > >>the patch > >>was somehow lost. > > > > > >IIRC, it was the usual problem of having it fixed in only one of the > >two 2.4 trees. I ensured both trees were made up to date. > > There is still no fix in linuxppc_2_4_devel! Um, but there is in linuxppc_2_4 ? -- Tom Rini (TR1265) http://gate.crashing.org/~trini/ ** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: was: FEC on MPC860T & race condition 2003-02-03 21:09 ` Tom Rini @ 2003-02-04 8:39 ` Steven Scholz 2003-02-04 16:04 ` Tom Rini 0 siblings, 1 reply; 13+ messages in thread From: Steven Scholz @ 2003-02-04 8:39 UTC (permalink / raw) To: Tom Rini; +Cc: Linuxppc-Embedded Tom Rini schrieb: > On Fri, Jan 31, 2003 at 10:16:25AM +0100, Steven Scholz wrote: > >>Dan Malek wrote: >> >>>Steven Scholz wrote: >>> >>> >>>>You, Laurent, told us about a race condition. >>>>You, Dan, first claimed, it was already fix but mentioned later that >>>>the patch >>>>was somehow lost. >>> >>> >>>IIRC, it was the usual problem of having it fixed in only one of the >>>two 2.4 trees. I ensured both trees were made up to date. >> >>There is still no fix in linuxppc_2_4_devel! > > > Um, but there is in linuxppc_2_4 ? Is that a question or a statement? 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! Cheers, Steven ** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: was: FEC on MPC860T & race condition 2003-02-04 8:39 ` Steven Scholz @ 2003-02-04 16:04 ` Tom Rini 2003-02-04 16:14 ` Steven Scholz 0 siblings, 1 reply; 13+ messages in thread From: Tom Rini @ 2003-02-04 16:04 UTC (permalink / raw) To: Steven Scholz; +Cc: Linuxppc-Embedded On Tue, Feb 04, 2003 at 09:39:05AM +0100, Steven Scholz wrote: > Tom Rini schrieb: > > On Fri, Jan 31, 2003 at 10:16:25AM +0100, Steven Scholz wrote: > > > >>Dan Malek wrote: > >> > >>>Steven Scholz wrote: > >>> > >>> > >>>>You, Laurent, told us about a race condition. > >>>>You, Dan, first claimed, it was already fix but mentioned later that > >>>>the patch > >>>>was somehow lost. > >>> > >>> > >>>IIRC, it was the usual problem of having it fixed in only one of the > >>>two 2.4 trees. I ensured both trees were made up to date. > >> > >>There is still no fix in linuxppc_2_4_devel! > > > > > > Um, but there is in linuxppc_2_4 ? > Is that a question or a statement? A question. > 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! 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. -- Tom Rini (TR1265) http://gate.crashing.org/~trini/ ** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: was: FEC on MPC860T & race condition 2003-02-04 16:04 ` Tom Rini @ 2003-02-04 16:14 ` Steven Scholz 2003-02-04 19:32 ` Tom Rini 0 siblings, 1 reply; 13+ messages in thread From: Steven Scholz @ 2003-02-04 16:14 UTC (permalink / raw) To: Tom Rini, Linuxppc-Embedded [-- 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; } ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: was: FEC on MPC860T & race condition 2003-02-04 16:14 ` Steven Scholz @ 2003-02-04 19:32 ` Tom Rini 2003-02-04 19:55 ` Dan Malek 2003-02-07 7:37 ` Steven Scholz 0 siblings, 2 replies; 13+ messages in thread From: Tom Rini @ 2003-02-04 19:32 UTC (permalink / raw) To: Steven Scholz; +Cc: Linuxppc-Embedded On Tue, Feb 04, 2003 at 05:14:08PM +0100, Steven Scholz wrote: > I'll attach my patch. It is not configurable since I think it won't break > anything. Okay. Moving along the process, I have a comment: > @@ -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 believe this is wrong, in that trying to udelay() here is a bad idea. I don't have the time right now, but I suspect google, or #kernelnewbies might be able to suggest a more appropriate way of waiting here. Or perhaps classes have fried my mind, and this is correct. > + /* 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 I believe this is an attempt at waiting as well. Kick the PHY and do nothing until it's hopefully discovered. -- Tom Rini http://gate.crashing.org/~trini/ ** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: was: FEC on MPC860T & race condition 2003-02-04 19:32 ` Tom Rini @ 2003-02-04 19:55 ` Dan Malek 2003-02-07 7:35 ` Steven Scholz 2003-02-07 7:37 ` Steven Scholz 1 sibling, 1 reply; 13+ messages in thread From: Dan Malek @ 2003-02-04 19:55 UTC (permalink / raw) To: Tom Rini; +Cc: Steven Scholz, Linuxppc-Embedded Tom Rini wrote: > I believe this is wrong, in that trying to udelay() here is a bad idea. I agree. All of the MII communcation is interrupt driven. This is easy and the way everything should work on the 8xx. The link interrupt is more challenging because the interrupt from that depends upon the phy type and the board design. The link interrupts are either real interrupts or managed with a timed thread. If you need to wait before installing the link interrupt (which I still don't understand why), this should be done as part of the phy discovery interrupt. For example, add an indirect function pointer and if it isn't NULL, call it at that time to do anything that must wait until the phy is discovered and initialized. Thanks. -- Dan ** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: was: FEC on MPC860T & race condition 2003-02-04 19:55 ` Dan Malek @ 2003-02-07 7:35 ` Steven Scholz 0 siblings, 0 replies; 13+ messages in thread From: Steven Scholz @ 2003-02-07 7:35 UTC (permalink / raw) To: Dan Malek; +Cc: Linuxppc-Embedded Dan Malek schrieb: > Tom Rini wrote: > >> I believe this is wrong, in that trying to udelay() here is a bad idea. > > > I agree. All of the MII communcation is interrupt driven. This is easy > and the way everything should work on the 8xx. The link interrupt is > more challenging because the interrupt from that depends upon the phy > type and the board design. The link interrupts are either real interrupts > or managed with a timed thread. > > If you need to wait before installing the link interrupt (which I still > don't understand why), this should be done as part of the phy discovery > interrupt. For example, add an indirect function pointer and if it isn't > NULL, call it at that time to do anything that must wait until the phy > is discovered and initialized. I understand what you mean. And certanly you're right. I moved the request_irq into 3 but my board still hanges from time to time. At a different place in a different way though. I have to admit that I don't understand this mii_queue stuff. Please, Dan, would you please give me a small hint! I assume by "add an indirect function pointer" you mean "add something to mii_queue"... Is that right? Thanks! Steven ** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: was: FEC on MPC860T & race condition 2003-02-04 19:32 ` Tom Rini 2003-02-04 19:55 ` Dan Malek @ 2003-02-07 7:37 ` Steven Scholz 2003-02-07 17:12 ` Tom Rini 1 sibling, 1 reply; 13+ messages in thread From: Steven Scholz @ 2003-02-07 7:37 UTC (permalink / raw) To: Tom Rini; +Cc: Linuxppc-Embedded Tom Rini wrote: >... > Okay. Moving along the process, I have a comment: > > >>@@ -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 believe this is wrong, in that trying to udelay() here is a bad idea. > I don't have the time right now, but I suspect google, or #kernelnewbies > might be able to suggest a more appropriate way of waiting here. > > Or perhaps classes have fried my mind, and this is correct. No. I bet you're right. But can you make any suggestion on how to wait for some I/O to become ready? Steven ** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: was: FEC on MPC860T & race condition 2003-02-07 7:37 ` Steven Scholz @ 2003-02-07 17:12 ` Tom Rini 2003-02-07 17:15 ` Steven Scholz 0 siblings, 1 reply; 13+ messages in thread From: Tom Rini @ 2003-02-07 17:12 UTC (permalink / raw) To: Steven Scholz; +Cc: Linuxppc-Embedded On Fri, Feb 07, 2003 at 08:37:05AM +0100, Steven Scholz wrote: > Tom Rini wrote: > >... > >Okay. Moving along the process, I have a comment: > > > > > >>@@ -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 believe this is wrong, in that trying to udelay() here is a bad idea. > >I don't have the time right now, but I suspect google, or #kernelnewbies > >might be able to suggest a more appropriate way of waiting here. > > > >Or perhaps classes have fried my mind, and this is correct. > No. I bet you're right. > But can you make any suggestion on how to wait for some I/O to become ready? How about what Dan mentioned, with a function pointer and whatnot? -- Tom Rini http://gate.crashing.org/~trini/ ** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: was: FEC on MPC860T & race condition 2003-02-07 17:12 ` Tom Rini @ 2003-02-07 17:15 ` Steven Scholz 0 siblings, 0 replies; 13+ messages in thread From: Steven Scholz @ 2003-02-07 17:15 UTC (permalink / raw) To: Tom Rini; +Cc: Linuxppc-Embedded Tom Rini schrieb: > On Fri, Feb 07, 2003 at 08:37:05AM +0100, Steven Scholz wrote: > >>Tom Rini wrote: >> >>>... >>>Okay. Moving along the process, I have a comment: >>> >>> >>> >>>>@@ -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 believe this is wrong, in that trying to udelay() here is a bad idea. >>>I don't have the time right now, but I suspect google, or #kernelnewbies >>>might be able to suggest a more appropriate way of waiting here. >>> >>>Or perhaps classes have fried my mind, and this is correct. >> >>No. I bet you're right. >>But can you make any suggestion on how to wait for some I/O to become ready? > > > How about what Dan mentioned, with a function pointer and whatnot? As I admitted in my mail to Dan, I am not sure if I understand excactly what mean... Steven ** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/ ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2003-02-07 17:15 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2003-01-30 17:05 was: FEC on MPC860T & race condition Steven Scholz 2003-01-30 20:22 ` Dan Malek 2003-01-31 9:16 ` Steven Scholz 2003-02-03 21:09 ` Tom Rini 2003-02-04 8:39 ` Steven Scholz 2003-02-04 16:04 ` Tom Rini 2003-02-04 16:14 ` Steven Scholz 2003-02-04 19:32 ` Tom Rini 2003-02-04 19:55 ` Dan Malek 2003-02-07 7:35 ` Steven Scholz 2003-02-07 7:37 ` Steven Scholz 2003-02-07 17:12 ` Tom Rini 2003-02-07 17:15 ` Steven Scholz
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).