[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <50956413-2C26-4DEC-89E0-F23351825EAB@freescale.com>
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