lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 15 Feb 2007 15:12:27 -0600 From: Andy Fleming <afleming@...escale.com> To: Sergei Shtylyov <sshtylyov@...mvista.com> Cc: jgarzik@...ox.com, netdev@...r.kernel.org, linuxppc-embedded@...abs.org Subject: Re: [PATCH v2, resend] gianfar: don't duplicate gfar_error() On Feb 15, 2007, at 07:56, Sergei Shtylyov wrote: > It was hardly necessary to repeat most of the code from gfar_error > () in > gfar_interrupt(), especially having some inconsistencies between > the two. > So, make the gfar_interrupt() just call gfar_error(), and not > acknowledge > the interrupts itself as gfar_{receive/transmit/error}() do it anyway. > While at it, also clarify/cleanup debug messages in gfar_error()... > > Signed-off-by: Sergei Shtylyov <sshtylyov@...mvista.com> Acked-by: Andy Fleming <afleming@...escale.com> > > --- > The patch survived netperf stressing on MPC8540ADS realtime > kernel. :-) > > Sorry, forgot to remove the obsolete regs argument from gfar_error > () call, > call, so the previous version wasn't even compilable -- I've tested > the patch > in the older kernel. Resending now with better placed comments > which you won't > have to edit out... :-< > > drivers/net/gianfar.c | 85 +++++++ > +------------------------------------------ > 1 files changed, 15 insertions(+), 70 deletions(-) > > Index: linux-2.6/drivers/net/gianfar.c > =================================================================== > --- linux-2.6.orig/drivers/net/gianfar.c > +++ linux-2.6/drivers/net/gianfar.c > @@ -10,6 +10,7 @@ > * Maintainer: Kumar Gala > * > * Copyright (c) 2002-2006 Freescale Semiconductor, Inc. > + * Copyright (c) 2007 MontaVista Software, Inc. > * > * This program is free software; you can redistribute it and/or > modify it > * under the terms of the GNU General Public License as > published by the > @@ -1613,71 +1614,17 @@ static irqreturn_t gfar_interrupt(int ir > /* Save ievent for future reference */ > u32 events = gfar_read(&priv->regs->ievent); > > - /* Clear IEVENT */ > - gfar_write(&priv->regs->ievent, events); > - > /* Check for reception */ > - if ((events & IEVENT_RXF0) || (events & IEVENT_RXB0)) > + if (events & IEVENT_RX_MASK) > gfar_receive(irq, dev_id); > > /* Check for transmit completion */ > - if ((events & IEVENT_TXF) || (events & IEVENT_TXB)) > + if (events & IEVENT_TX_MASK) > gfar_transmit(irq, dev_id); > > - /* Update error statistics */ > - if (events & IEVENT_TXE) { > - priv->stats.tx_errors++; > - > - if (events & IEVENT_LC) > - priv->stats.tx_window_errors++; > - if (events & IEVENT_CRL) > - priv->stats.tx_aborted_errors++; > - if (events & IEVENT_XFUN) { > - if (netif_msg_tx_err(priv)) > - printk(KERN_WARNING "%s: tx underrun. dropped packet\n", dev- > >name); > - priv->stats.tx_dropped++; > - priv->extra_stats.tx_underrun++; > - > - /* Reactivate the Tx Queues */ > - gfar_write(&priv->regs->tstat, TSTAT_CLEAR_THALT); > - } > - } > - if (events & IEVENT_BSY) { > - priv->stats.rx_errors++; > - priv->extra_stats.rx_bsy++; > - > - gfar_receive(irq, dev_id); > - > -#ifndef CONFIG_GFAR_NAPI > - /* Clear the halt bit in RSTAT */ > - gfar_write(&priv->regs->rstat, RSTAT_CLEAR_RHALT); > -#endif > - > - if (netif_msg_rx_err(priv)) > - printk(KERN_DEBUG "%s: busy error (rhalt: %x)\n", > - dev->name, > - gfar_read(&priv->regs->rstat)); > - } > - if (events & IEVENT_BABR) { > - priv->stats.rx_errors++; > - priv->extra_stats.rx_babr++; > - > - if (netif_msg_rx_err(priv)) > - printk(KERN_DEBUG "%s: babbling error\n", dev->name); > - } > - if (events & IEVENT_EBERR) { > - priv->extra_stats.eberr++; > - if (netif_msg_rx_err(priv)) > - printk(KERN_DEBUG "%s: EBERR\n", dev->name); > - } > - if ((events & IEVENT_RXC) && (netif_msg_rx_err(priv))) > - printk(KERN_DEBUG "%s: control frame\n", dev->name); > - > - if (events & IEVENT_BABT) { > - priv->extra_stats.tx_babt++; > - if (netif_msg_rx_err(priv)) > - printk(KERN_DEBUG "%s: babt error\n", dev->name); > - } > + /* Check for errors */ > + if (events & IEVENT_ERR_MASK) > + gfar_error(irq, dev_id); > > return IRQ_HANDLED; > } > @@ -1939,7 +1886,7 @@ static irqreturn_t gfar_error(int irq, v > /* Hmm... */ > if (netif_msg_rx_err(priv) || netif_msg_tx_err(priv)) > printk(KERN_DEBUG "%s: error interrupt (ievent=0x%08x imask=0x% > 08x)\n", > - dev->name, events, gfar_read(&priv->regs->imask)); > + dev->name, events, gfar_read(&priv->regs->imask)); > > /* Update the error counters */ > if (events & IEVENT_TXE) { > @@ -1951,8 +1898,8 @@ static irqreturn_t gfar_error(int irq, v > priv->stats.tx_aborted_errors++; > if (events & IEVENT_XFUN) { > if (netif_msg_tx_err(priv)) > - printk(KERN_DEBUG "%s: underrun. packet dropped.\n", > - dev->name); > + printk(KERN_DEBUG "%s: TX FIFO underrun, " > + "packet dropped.\n", dev->name); > priv->stats.tx_dropped++; > priv->extra_stats.tx_underrun++; > > @@ -1974,30 +1921,28 @@ static irqreturn_t gfar_error(int irq, v > #endif > > if (netif_msg_rx_err(priv)) > - printk(KERN_DEBUG "%s: busy error (rhalt: %x)\n", > - dev->name, > - gfar_read(&priv->regs->rstat)); > + printk(KERN_DEBUG "%s: busy error (rstat: %x)\n", > + dev->name, gfar_read(&priv->regs->rstat)); > } > if (events & IEVENT_BABR) { > priv->stats.rx_errors++; > priv->extra_stats.rx_babr++; > > if (netif_msg_rx_err(priv)) > - printk(KERN_DEBUG "%s: babbling error\n", dev->name); > + printk(KERN_DEBUG "%s: babbling RX error\n", dev->name); > } > if (events & IEVENT_EBERR) { > priv->extra_stats.eberr++; > if (netif_msg_rx_err(priv)) > - printk(KERN_DEBUG "%s: EBERR\n", dev->name); > + printk(KERN_DEBUG "%s: bus error\n", dev->name); > } > if ((events & IEVENT_RXC) && netif_msg_rx_status(priv)) > - if (netif_msg_rx_status(priv)) > - printk(KERN_DEBUG "%s: control frame\n", dev->name); > + printk(KERN_DEBUG "%s: control frame\n", dev->name); > > if (events & IEVENT_BABT) { > priv->extra_stats.tx_babt++; > if (netif_msg_tx_err(priv)) > - printk(KERN_DEBUG "%s: babt error\n", dev->name); > + printk(KERN_DEBUG "%s: babbling TX error\n", dev->name); > } > return IRQ_HANDLED; > } > > - > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@...r.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists