This is the mail archive of the ecos-discuss@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Bug (and fix) for FEC driver PowerPC FEC


Gary,

  Thanks for applying the patch and for your explaination of why we
don't need the FEC masking.  I concur that the ISR/DSR will handle it
and FEC masking is not needed.

  Fernando Flores

On 6/22/05, Gary Thomas <gary@mlbassoc.com> wrote:
> 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?
> 
> A very thoughtful workup - I'll apply these changes.  I disagree
> with the need for the interrupt masking - interrupts will be masked
> by the ISR/DSR process and these extra fiddles accomplish nothing
> and would just slow the code down.
> 
> As for the official process for submitting patches - generate a patch
> by running "cvs diff"(*) and then send the results (as an attachment to
> avoid mailer problems) to ecos-patches@ecos.sourceware.org  We'll
> review the changes and work with you to get them applied.  Depending
> on the magnitude of the changes, we may need a copyright assignment.
> Full details are on the web site (look for contributing...)
> 
> (*) Please set your CVS preferences to use "context diffs".  This can
> be done by adding the line:
>   diff -u5 -p -N
> to your ~/.cvsrc file.
> 
> --
> ------------------------------------------------------------
> Gary Thomas                 |  Consulting for the
> MLB Associates              |    Embedded world
> ------------------------------------------------------------
> 
>

--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]