From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16165 invoked by alias); 23 Jun 2005 03:12:15 -0000 Mailing-List: contact ecos-discuss-help@ecos.sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: ecos-discuss-owner@ecos.sourceware.org Received: (qmail 16142 invoked by uid 22791); 23 Jun 2005 03:12:10 -0000 Received: from wproxy.gmail.com (HELO wproxy.gmail.com) (64.233.184.202) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Thu, 23 Jun 2005 03:12:10 +0000 Received: by wproxy.gmail.com with SMTP id 37so248890wra for ; Wed, 22 Jun 2005 20:12:09 -0700 (PDT) Received: by 10.54.129.19 with SMTP id b19mr863721wrd; Wed, 22 Jun 2005 20:12:08 -0700 (PDT) Received: by 10.54.39.72 with HTTP; Wed, 22 Jun 2005 20:11:38 -0700 (PDT) Message-ID: Date: Thu, 23 Jun 2005 03:12:00 -0000 From: Fernando Flores Reply-To: Fernando Flores To: Gary Thomas Cc: eCos Discussion In-Reply-To: <1119481692.25172.6.camel@hermes> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline References: <1119481692.25172.6.camel@hermes> Subject: Re: [ECOS] Bug (and fix) for FEC driver PowerPC FEC X-SW-Source: 2005-06/txt/msg00213.txt.bz2 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 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//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 =3D (struct fec_eth_info *)sc->driver_priva= te; > > unsigned long event; > > > > while ((event =3D qi->fec->iEvent) !=3D 0) { > > if ((event & iEvent_TFINT) !=3D 0) { > > fec_eth_TxEvent(sc); > > } > > if ((event & iEvent_RFINT) !=3D 0) { > > fec_eth_RxEvent(sc); > > } > > qi->fec->iEvent =3D 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 foll= owing: > > // > > // Interrupt processing > > // > > static void > > fec_eth_int(struct eth_drv_sc *sc) > > { > > struct fec_eth_info *qi =3D (struct fec_eth_info *)sc->driver_priva= te; > > unsigned long event; > > > > while ((event =3D qi->fec->iEvent) !=3D 0) { > > > > qi->fec->iEvent =3D event; // Reset the bits we will handle > > > > if ((event & iEvent_TFINT) !=3D 0) { > > fec_eth_TxEvent(sc); > > } > > if ((event & iEvent_RFINT) !=3D 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//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 =3D (struct fec_eth_info *)sc->driver_priva= te; > > unsigned long event; > > unsigned long mask =3D qi->fec->iMask; > > > > // Disable all interrupts > > qi->fec->iMask =3D 0x0000000; > > > > while ((event =3D qi->fec->iEvent) !=3D 0) { > > > > qi->fec->iEvent =3D event; // Reset the bits we will handled > > > > if ((event & iEvent_TFINT) !=3D 0) { > > fec_eth_TxEvent(sc); > > } > > if ((event & iEvent_RFINT) !=3D 0) { > > fec_eth_RxEvent(sc); > > } > > > > } > > // Re-enable Interrupts > > qi->fec->iMask =3D mask; // enable interrupts > > } > > > > Is there a formal process to submit these changes to the official eCos > > distribution? >=20 > 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. >=20 > 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...) >=20 > (*) 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. >=20 > -- > ------------------------------------------------------------ > Gary Thomas | Consulting for the > MLB Associates | Embedded world > ------------------------------------------------------------ >=20 > -- Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss