This is the mail archive of the
ecos-discuss@sources.redhat.com
mailing list for the eCos project.
Re: Bug (and fix) for FEC driver PowerPC FEC
- From: Fernando Flores <nando67 at gmail dot com>
- To: Gary Thomas <gary at mlbassoc dot com>
- Cc: eCos Discussion <ecos-discuss at ecos dot sourceware dot org>
- Date: Wed, 22 Jun 2005 22:11:38 -0500
- Subject: Re: [ECOS] Bug (and fix) for FEC driver PowerPC FEC
- References: <b8cea958050622140447be2789@mail.gmail.com> <1119481692.25172.6.camel@hermes>
- Reply-to: Fernando Flores <nando67 at gmail dot com>
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