This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
Re: [ECOS] Bug (and fix) for FEC driver PowerPC FEC
- From: Gary Thomas <gary at mlbassoc dot com>
- To: Fernando Flores <nando67 at gmail dot com>
- Cc: eCos patches <ecos-patches at ecos dot sourceware dot org>
- Date: Wed, 22 Jun 2005 17:27:15 -0600
- Subject: Re: [ECOS] Bug (and fix) for FEC driver PowerPC FEC
- Organization: MLB Associates
- References: <b8cea958050622140447be2789@mail.gmail.com>
On Wed, 2005-06-22 at 16:04 -0500, Fernando Flores wrote:
> Hello,
>
> We have a Device with a Motorola PowerPC 855 w/ a FEC. We noticed
> that under heavy traffic loads, the network will stop receiving
> packets. Further investigation pointed to having all the RX buffer
> descriptors being full, but the processor stop sending interrupts.
> Upon examining the fec code under
> '/ecos/packages/devs/eth/powerpc/fec/<version>/src/if_fec.c, we
> noticed that the interrupt handler code was written as:
>
> //
> // Interrupt processing
> //
> static void
> fec_eth_int(struct eth_drv_sc *sc)
> {
> struct fec_eth_info *qi = (struct fec_eth_info *)sc->driver_private;
> unsigned long event;
>
> while ((event = qi->fec->iEvent) != 0) {
> if ((event & iEvent_TFINT) != 0) {
> fec_eth_TxEvent(sc);
> }
> if ((event & iEvent_RFINT) != 0) {
> fec_eth_RxEvent(sc);
> }
> qi->fec->iEvent = event; // Reset the bits we handled
> }
> }
>
>
> The problem here is that we first clear out the RX buffers, then
> handle the TX buffers. But the FEC may receive more packets and fill
> up all buffers before we reset the bits in the event register. We
> then clear the event register but never empty the buffers when we loop
> back around. The processor believes we have acknowledged the event
> for the newly arrived packets and never sets the interrupt.
>
> This can be easily fixed by changing the order of the reset to the following:
> //
> // Interrupt processing
> //
> static void
> fec_eth_int(struct eth_drv_sc *sc)
> {
> struct fec_eth_info *qi = (struct fec_eth_info *)sc->driver_private;
> unsigned long event;
>
> while ((event = qi->fec->iEvent) != 0) {
>
> qi->fec->iEvent = event; // Reset the bits we will handle
>
> if ((event & iEvent_TFINT) != 0) {
> fec_eth_TxEvent(sc);
> }
> if ((event & iEvent_RFINT) != 0) {
> fec_eth_RxEvent(sc);
> }
>
> }
> }
>
> We have tested this with and no longer have the network hang-up
> problems after flooding it with packets. The code is located in
> /ecos/packages/devs/eth/powerpc/fec/<version>/src/if_fec.c I hope
> this helps anyone having similar issues.
>
> To make this routine a bit better, it is a good idea to disable the
> FEC interrupts in the interrupt handler:
>
> //
> // Interrupt processing
> //
> static void
> fec_eth_int(struct eth_drv_sc *sc)
> {
> struct fec_eth_info *qi = (struct fec_eth_info *)sc->driver_private;
> unsigned long event;
> unsigned long mask = qi->fec->iMask;
>
> // Disable all interrupts
> qi->fec->iMask = 0x0000000;
>
> while ((event = qi->fec->iEvent) != 0) {
>
> qi->fec->iEvent = event; // Reset the bits we will handled
>
> if ((event & iEvent_TFINT) != 0) {
> fec_eth_TxEvent(sc);
> }
> if ((event & iEvent_RFINT) != 0) {
> fec_eth_RxEvent(sc);
> }
>
> }
> // Re-enable Interrupts
> qi->fec->iMask = mask; // enable interrupts
> }
>
> Is there a formal process to submit these changes to the official eCos
> distribution?
>
> Thanks,
>
> Fernando Flores
Attached patch applied, thanks.
--
------------------------------------------------------------
Gary Thomas | Consulting for the
MLB Associates | Embedded world
------------------------------------------------------------
Index: devs/eth/powerpc/fec/current/ChangeLog
===================================================================
RCS file: /misc/cvsfiles/ecos/packages/devs/eth/powerpc/fec/current/ChangeLog,v
retrieving revision 1.22
diff -u -5 -p -r1.22 ChangeLog
--- devs/eth/powerpc/fec/current/ChangeLog 13 Oct 2004 20:12:23 -0000 1.22
+++ devs/eth/powerpc/fec/current/ChangeLog 22 Jun 2005 23:23:40 -0000
@@ -1,5 +1,10 @@
+2005-06-22 Gary Thomas <gary@mlbassoc.com>
+
+ * src/if_fec.c (fec_eth_int): Avoid race when handling interrupt
+ events - reported by Fernando Flores
+
2004-10-11 Harald Kuethe <hkuethe@controlware.de>
* src/if_fec.c (fec_eth_RxEvent): remove crc from packet.
2003-12-21 Gary Thomas <gary@mlbassoc.com>
Index: devs/eth/powerpc/fec/current/src/if_fec.c
===================================================================
RCS file: /misc/cvsfiles/ecos/packages/devs/eth/powerpc/fec/current/src/if_fec.c,v
retrieving revision 1.19
diff -u -5 -p -r1.19 if_fec.c
--- devs/eth/powerpc/fec/current/src/if_fec.c 13 Oct 2004 20:12:24 -0000 1.19
+++ devs/eth/powerpc/fec/current/src/if_fec.c 22 Jun 2005 23:22:16 -0000
@@ -761,17 +761,17 @@ fec_eth_int(struct eth_drv_sc *sc)
{
struct fec_eth_info *qi = (struct fec_eth_info *)sc->driver_private;
unsigned long event;
while ((event = qi->fec->iEvent) != 0) {
+ qi->fec->iEvent = event; // Reset the bits we handled
if ((event & iEvent_TFINT) != 0) {
fec_eth_TxEvent(sc);
}
if ((event & iEvent_RFINT) != 0) {
fec_eth_RxEvent(sc);
}
- qi->fec->iEvent = event; // Reset the bits we handled
}
}
//
// Interrupt vector