From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20716 invoked by alias); 22 Jun 2005 23:12:09 -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 17215 invoked by uid 22791); 22 Jun 2005 23:08:23 -0000 Received: from sta-206-168-96-204.rockynet.com (HELO hermes.chez-thomas.org) (206.168.96.204) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Wed, 22 Jun 2005 23:08:23 +0000 Received: by hermes.chez-thomas.org (Postfix, from userid 2000) id E23FE10053B; Wed, 22 Jun 2005 17:08:17 -0600 (MDT) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by hermes.chez-thomas.org (Postfix) with ESMTP id 8F4CD100485; Wed, 22 Jun 2005 17:08:12 -0600 (MDT) From: Gary Thomas To: Fernando Flores Cc: eCos Discussion In-Reply-To: References: Content-Type: text/plain Date: Wed, 22 Jun 2005 23:12:00 -0000 Message-Id: <1119481692.25172.6.camel@hermes> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [ECOS] Bug (and fix) for FEC driver PowerPC FEC X-SW-Source: 2005-06/txt/msg00211.txt.bz2 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 = (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//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