Patchwork [1/1] via-rhine: Fix hanging with high CPU load on low-end broads.

login
register
mail settings
Submitter françois romieu
Date Dec. 29, 2011, 6:39 p.m.
Message ID <20111229183902.GC21946@electric-eye.fr.zoreil.com>
Download mbox | patch
Permalink /patch/133594/
State RFC
Delegated to: David Miller
Headers show

Comments

françois romieu - Dec. 29, 2011, 6:39 p.m.
Francois Romieu <romieu@fr.zoreil.com> :
[...]
> Feel free to try the extra "new year's eve week end is getting closer"
> patch below (against net-next).
> 
> You have been warned.

The one below could be a bit better.

Out for dinner.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjarke Istrup Pedersen - Dec. 29, 2011, 10 p.m.
2011/12/29 Francois Romieu <romieu@fr.zoreil.com>:
> Francois Romieu <romieu@fr.zoreil.com> :
> [...]
>> Feel free to try the extra "new year's eve week end is getting closer"
>> patch below (against net-next).
>>
>> You have been warned.
>
> The one below could be a bit better.
>
> Out for dinner.

Thanks, I'll try that one in a while, once I'm done bisecting 3.1.4 ->
3.1.5, since all kernels after 3.1.4 won't pass control onto init.

/Bjarke

> diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
> index 5c4983b..66e5c08 100644
> --- a/drivers/net/ethernet/via/via-rhine.c
> +++ b/drivers/net/ethernet/via/via-rhine.c
> @@ -42,7 +42,8 @@
>
>  #define DEBUG
>  static int debug = 1;  /* 1 normal messages, 0 quiet .. 7 verbose. */
> -static int max_interrupt_work = 20;
> +#define RHINE_MSG_DEFAULT \
> +        (NETIF_MSG_INTR)
>
>  /* Set the copy breakpoint for the copy-only-tiny-frames scheme.
>    Setting to > 1518 effectively disables this feature. */
> @@ -128,11 +129,9 @@ MODULE_AUTHOR("Donald Becker <becker@scyld.com>");
>  MODULE_DESCRIPTION("VIA Rhine PCI Fast Ethernet driver");
>  MODULE_LICENSE("GPL");
>
> -module_param(max_interrupt_work, int, 0);
>  module_param(debug, int, 0);
>  module_param(rx_copybreak, int, 0);
>  module_param(avoid_D3, bool, 0);
> -MODULE_PARM_DESC(max_interrupt_work, "VIA Rhine maximum events handled per interrupt");
>  MODULE_PARM_DESC(debug, "VIA Rhine debug level (0-7)");
>  MODULE_PARM_DESC(rx_copybreak, "VIA Rhine copy breakpoint for copy-only-tiny-frames");
>  MODULE_PARM_DESC(avoid_D3, "Avoid power state D3 (work-around for broken BIOSes)");
> @@ -351,16 +350,25 @@ static const int mmio_verify_registers[] = {
>
>  /* Bits in the interrupt status/mask registers. */
>  enum intr_status_bits {
> -       IntrRxDone=0x0001, IntrRxErr=0x0004, IntrRxEmpty=0x0020,
> -       IntrTxDone=0x0002, IntrTxError=0x0008, IntrTxUnderrun=0x0210,
> -       IntrPCIErr=0x0040,
> -       IntrStatsMax=0x0080, IntrRxEarly=0x0100,
> -       IntrRxOverflow=0x0400, IntrRxDropped=0x0800, IntrRxNoBuf=0x1000,
> -       IntrTxAborted=0x2000, IntrLinkChange=0x4000,
> -       IntrRxWakeUp=0x8000,
> -       IntrNormalSummary=0x0003, IntrAbnormalSummary=0xC260,
> -       IntrTxDescRace=0x080000,        /* mapped from IntrStatus2 */
> -       IntrTxErrSummary=0x082218,
> +       IntrRxDone      = 0x0001,
> +       IntrTxDone      = 0x0002,
> +       IntrRxErr       = 0x0004,
> +       IntrTxError     = 0x0008,
> +       IntrRxEmpty     = 0x0020,
> +       IntrPCIErr      = 0x0040,
> +       IntrStatsMax    = 0x0080,
> +       IntrRxEarly     = 0x0100,
> +       IntrTxUnderrun  = 0x0210,
> +       IntrRxOverflow  = 0x0400,
> +       IntrRxDropped   = 0x0800,
> +       IntrRxNoBuf     = 0x1000,
> +       IntrTxAborted   = 0x2000,
> +       IntrLinkChange  = 0x4000,
> +       IntrRxWakeUp    = 0x8000,
> +       IntrTxDescRace          = 0x080000,     /* mapped from IntrStatus2 */
> +       IntrNormalSummary       = IntrRxDone | IntrTxDone,
> +       IntrTxErrSummary        = IntrTxDescRace | IntrTxAborted | IntrTxError |
> +                                 IntrTxUnderrun,
>  };
>
>  /* Bits in WOLcrSet/WOLcrClr and PwrcsrSet/PwrcsrClr */
> @@ -439,8 +447,12 @@ struct rhine_private {
>        struct net_device *dev;
>        struct napi_struct napi;
>        spinlock_t lock;
> +       struct mutex task_lock;
> +       struct work_struct slow_event_task;
>        struct work_struct reset_task;
>
> +       u32 msg_enable;
> +
>        /* Frequently used values: keep some adjacent for cache effect. */
>        u32 quirks;
>        struct rx_desc *rx_head_desc;
> @@ -482,7 +494,6 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
>  static irqreturn_t rhine_interrupt(int irq, void *dev_instance);
>  static void rhine_tx(struct net_device *dev);
>  static int rhine_rx(struct net_device *dev, int limit);
> -static void rhine_error(struct net_device *dev, int intr_status);
>  static void rhine_set_rx_mode(struct net_device *dev);
>  static struct net_device_stats *rhine_get_stats(struct net_device *dev);
>  static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
> @@ -497,6 +508,8 @@ static void rhine_set_cam_mask(void __iomem *ioaddr, u32 mask);
>  static void rhine_set_vlan_cam_mask(void __iomem *ioaddr, u32 mask);
>  static void rhine_init_cam_filter(struct net_device *dev);
>  static void rhine_update_vcam(struct net_device *dev);
> +static void rhine_slow_event_task(struct work_struct *work);
> +static void rhine_restart_tx(struct net_device *dev);
>
>  #define RHINE_WAIT_FOR(condition)                              \
>  do {                                                           \
> @@ -508,7 +521,7 @@ do {                                                                \
>                        1024 - i, __func__, __LINE__);          \
>  } while (0)
>
> -static inline u32 get_intr_status(struct net_device *dev)
> +static u32 get_intr_status(struct net_device *dev)
>  {
>        struct rhine_private *rp = netdev_priv(dev);
>        void __iomem *ioaddr = rp->base;
> @@ -521,6 +534,16 @@ static inline u32 get_intr_status(struct net_device *dev)
>        return intr_status;
>  }
>
> +static void rhine_ack_event(struct rhine_private *rp, u32 mask)
> +{
> +       void __iomem *ioaddr = rp->base;
> +
> +       if (rp->quirks & rqStatusWBRace)
> +               iowrite8(mask >> 16, ioaddr + IntrStatus2);
> +       iowrite16(mask, ioaddr + IntrStatus);
> +       mmiowb();
> +}
> +
>  /*
>  * Get power related registers into sane state.
>  * Notify user about past WOL event.
> @@ -657,23 +680,127 @@ static void rhine_poll(struct net_device *dev)
>  }
>  #endif
>
> +static void rhine_kick_tx_threshold(struct rhine_private *rp)
> +{
> +       if (rp->tx_thresh < 0xe0) {
> +               void __iomem *ioaddr = rp->base;
> +
> +               rp->tx_thresh += 0x20;
> +               BYTE_REG_BITS_SET(rp->tx_thresh, 0x80, ioaddr + TxConfig);
> +       }
> +}
> +
> +static void rhine_tx_err(struct rhine_private *rp, u32 status)
> +{
> +       struct net_device *dev = rp->dev;
> +
> +       if (status & IntrTxAborted) {
> +               netif_info(rp, tx_err, dev,
> +                          "Abort %08x, frame dropped\n", status);
> +       }
> +
> +       if (status & IntrTxUnderrun) {
> +               rhine_kick_tx_threshold(rp);
> +               netif_info(rp, tx_err ,dev, "Transmitter underrun, "
> +                          "Tx threshold now %02x\n", rp->tx_thresh);
> +       }
> +
> +       if (status & IntrTxDescRace)
> +               netif_info(rp, tx_err, dev, "Tx descriptor write-back race\n");
> +
> +       if ((status & IntrTxError) &&
> +           (status & (IntrTxAborted | IntrTxUnderrun | IntrTxDescRace)) == 0) {
> +               rhine_kick_tx_threshold(rp);
> +               netif_info(rp, tx_err, dev, "Unspecified error. "
> +                          "Tx threshold now %02x\n", rp->tx_thresh);
> +       }
> +
> +       rhine_restart_tx(dev);
> +}
> +
> +static void rhine_update_rx_crc_and_missed_errord(struct rhine_private *rp)
> +{
> +       void __iomem *ioaddr = rp->base;
> +       struct net_device_stats *stats = &rp->dev->stats;
> +
> +       stats->rx_crc_errors    += ioread16(ioaddr + RxCRCErrs);
> +       stats->rx_missed_errors += ioread16(ioaddr + RxMissed);
> +
> +       /*
> +        * Clears the "tally counters" for CRC errors and missed frames(?).
> +        * It has been reported that some chips need a write of 0 to clear
> +        * these, for others the counters are set to 1 when written to and
> +        * instead cleared when read. So we clear them both ways ...
> +        */
> +       iowrite32(0, ioaddr + RxMissed);
> +       ioread16(ioaddr + RxCRCErrs);
> +       ioread16(ioaddr + RxMissed);
> +}
> +
> +#define RHINE_EVENT_NAPI_RX    (IntrRxDone | \
> +                                IntrRxErr | \
> +                                IntrRxEmpty | \
> +                                IntrRxOverflow | \
> +                                IntrRxDropped | \
> +                                IntrRxNoBuf | \
> +                                IntrRxWakeUp)
> +
> +#define RHINE_EVENT_NAPI_TX_ERR        (IntrTxError | \
> +                                IntrTxAborted | \
> +                                IntrTxUnderrun | \
> +                                IntrTxDescRace)
> +#define RHINE_EVENT_NAPI_TX    (IntrTxDone | RHINE_EVENT_NAPI_TX_ERR)
> +
> +#define RHINE_EVENT_NAPI       (RHINE_EVENT_NAPI_RX | \
> +                                RHINE_EVENT_NAPI_TX | \
> +                                IntrStatsMax)
> +#define RHINE_EVENT_SLOW       (IntrPCIErr | IntrLinkChange)
> +#define RHINE_EVENT            (RHINE_EVENT_NAPI | RHINE_EVENT_SLOW)
> +
>  static int rhine_napipoll(struct napi_struct *napi, int budget)
>  {
>        struct rhine_private *rp = container_of(napi, struct rhine_private, napi);
>        struct net_device *dev = rp->dev;
>        void __iomem *ioaddr = rp->base;
> -       int work_done;
> +       u16 enable_mask = RHINE_EVENT & 0xffff;
> +       int work_done = 0;
> +       u32 status;
> +
> +       status = get_intr_status(dev);
> +       rhine_ack_event(rp, status & ~RHINE_EVENT_SLOW);
> +
> +       if (status & RHINE_EVENT_NAPI_RX)
> +               work_done += rhine_rx(dev, budget);
> +
> +       if (status & RHINE_EVENT_NAPI_TX) {
> +               if (status & RHINE_EVENT_NAPI_TX_ERR) {
> +                       /* Avoid scavenging before Tx engine turned off */
> +                       RHINE_WAIT_FOR(!(ioread8(ioaddr + ChipCmd) & CmdTxOn));
> +                       if (ioread8(ioaddr + ChipCmd) & CmdTxOn)
> +                               netif_warn(rp, tx_err, dev, "Tx still on\n");
> +               }
>
> -       work_done = rhine_rx(dev, budget);
> +               rhine_tx(dev);
> +
> +               if (status & RHINE_EVENT_NAPI_TX_ERR)
> +                       rhine_tx_err(rp, status);
> +       }
> +
> +       if (status & IntrStatsMax) {
> +               spin_lock(&rp->lock);
> +               rhine_update_rx_crc_and_missed_errord(rp);
> +               spin_unlock(&rp->lock);
> +       }
> +
> +       if (status & RHINE_EVENT_SLOW) {
> +               enable_mask &= ~RHINE_EVENT_SLOW;
> +               schedule_work(&rp->slow_event_task);
> +       }
>
>        if (work_done < budget) {
>                napi_complete(napi);
> -
> -               iowrite16(IntrRxDone | IntrRxErr | IntrRxEmpty| IntrRxOverflow |
> -                         IntrRxDropped | IntrRxNoBuf | IntrTxAborted |
> -                         IntrTxDone | IntrTxError | IntrTxUnderrun |
> -                         IntrPCIErr | IntrStatsMax | IntrLinkChange,
> -                         ioaddr + IntrEnable);
> +               iowrite16(enable_mask, ioaddr + IntrEnable);
> +               mmiowb();
>        }
>        return work_done;
>  }
> @@ -797,6 +924,7 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
>        rp->quirks = quirks;
>        rp->pioaddr = pioaddr;
>        rp->pdev = pdev;
> +       rp->msg_enable = netif_msg_init(-1, RHINE_MSG_DEFAULT);
>
>        rc = pci_request_regions(pdev, DRV_NAME);
>        if (rc)
> @@ -856,7 +984,9 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
>        dev->irq = pdev->irq;
>
>        spin_lock_init(&rp->lock);
> +       mutex_init(&rp->task_lock);
>        INIT_WORK(&rp->reset_task, rhine_reset_task);
> +       INIT_WORK(&rp->slow_event_task, rhine_slow_event_task);
>
>        rp->mii_if.dev = dev;
>        rp->mii_if.mdio_read = mdio_read;
> @@ -1310,12 +1440,7 @@ static void init_registers(struct net_device *dev)
>
>        napi_enable(&rp->napi);
>
> -       /* Enable interrupts by setting the interrupt mask. */
> -       iowrite16(IntrRxDone | IntrRxErr | IntrRxEmpty| IntrRxOverflow |
> -              IntrRxDropped | IntrRxNoBuf | IntrTxAborted |
> -              IntrTxDone | IntrTxError | IntrTxUnderrun |
> -              IntrPCIErr | IntrStatsMax | IntrLinkChange,
> -              ioaddr + IntrEnable);
> +       iowrite16(RHINE_EVENT & 0xffff, ioaddr + IntrEnable);
>
>        iowrite16(CmdStart | CmdTxOn | CmdRxOn | (Cmd1NoTxPoll << 8),
>               ioaddr + ChipCmd);
> @@ -1434,13 +1559,10 @@ static void rhine_reset_task(struct work_struct *work)
>                                                reset_task);
>        struct net_device *dev = rp->dev;
>
> -       /* protect against concurrent rx interrupts */
> -       disable_irq(rp->pdev->irq);
> +       mutex_lock(&rp->task_lock);
>
>        napi_disable(&rp->napi);
>
> -       spin_lock_bh(&rp->lock);
> -
>        /* clear all descriptors */
>        free_tbufs(dev);
>        free_rbufs(dev);
> @@ -1451,12 +1573,11 @@ static void rhine_reset_task(struct work_struct *work)
>        rhine_chip_reset(dev);
>        init_registers(dev);
>
> -       spin_unlock_bh(&rp->lock);
> -       enable_irq(rp->pdev->irq);
> -
>        dev->trans_start = jiffies; /* prevent tx timeout */
>        dev->stats.tx_errors++;
>        netif_wake_queue(dev);
> +
> +       mutex_unlock(&rp->task_lock);
>  }
>
>  static void rhine_tx_timeout(struct net_device *dev)
> @@ -1477,7 +1598,6 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
>        struct rhine_private *rp = netdev_priv(dev);
>        void __iomem *ioaddr = rp->base;
>        unsigned entry;
> -       unsigned long flags;
>
>        /* Caution: the write order is important here, set the field
>           with the "ownership" bits last. */
> @@ -1529,7 +1649,6 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
>                rp->tx_ring[entry].tx_status = 0;
>
>        /* lock eth irq */
> -       spin_lock_irqsave(&rp->lock, flags);
>        wmb();
>        rp->tx_ring[entry].tx_status |= cpu_to_le32(DescOwn);
>        wmb();
> @@ -1543,15 +1662,12 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
>                BYTE_REG_BITS_ON(1 << 7, ioaddr + TQWake);
>
>        /* Wake the potentially-idle transmit channel */
> -       iowrite8(ioread8(ioaddr + ChipCmd1) | Cmd1TxDemand,
> -              ioaddr + ChipCmd1);
> +       iowrite8(ioread8(ioaddr + ChipCmd1) | Cmd1TxDemand, ioaddr + ChipCmd1);
>        IOSYNC;
>
>        if (rp->cur_tx == rp->dirty_tx + TX_QUEUE_LEN)
>                netif_stop_queue(dev);
>
> -       spin_unlock_irqrestore(&rp->lock, flags);
> -
>        if (debug > 4) {
>                netdev_dbg(dev, "Transmit frame #%d queued in slot %d\n",
>                           rp->cur_tx-1, entry);
> @@ -1565,63 +1681,26 @@ static irqreturn_t rhine_interrupt(int irq, void *dev_instance)
>  {
>        struct net_device *dev = dev_instance;
>        struct rhine_private *rp = netdev_priv(dev);
> -       void __iomem *ioaddr = rp->base;
> -       u32 intr_status;
> -       int boguscnt = max_interrupt_work;
> +       u32 status;
>        int handled = 0;
>
> -       while ((intr_status = get_intr_status(dev))) {
> -               handled = 1;
> -
> -               /* Acknowledge all of the current interrupt sources ASAP. */
> -               if (intr_status & IntrTxDescRace)
> -                       iowrite8(0x08, ioaddr + IntrStatus2);
> -               iowrite16(intr_status & 0xffff, ioaddr + IntrStatus);
> -               IOSYNC;
> -
> -               if (debug > 4)
> -                       netdev_dbg(dev, "Interrupt, status %08x\n",
> -                                  intr_status);
> -
> -               if (intr_status & (IntrRxDone | IntrRxErr | IntrRxDropped |
> -                                  IntrRxWakeUp | IntrRxEmpty | IntrRxNoBuf)) {
> -                       iowrite16(IntrTxAborted |
> -                                 IntrTxDone | IntrTxError | IntrTxUnderrun |
> -                                 IntrPCIErr | IntrStatsMax | IntrLinkChange,
> -                                 ioaddr + IntrEnable);
> +       status = get_intr_status(dev);
>
> -                       napi_schedule(&rp->napi);
> -               }
> +       netif_dbg(rp, intr, dev, "Interrupt, status %08x\n", status);
>
> -               if (intr_status & (IntrTxErrSummary | IntrTxDone)) {
> -                       if (intr_status & IntrTxErrSummary) {
> -                               /* Avoid scavenging before Tx engine turned off */
> -                               RHINE_WAIT_FOR(!(ioread8(ioaddr+ChipCmd) & CmdTxOn));
> -                               if (debug > 2 &&
> -                                   ioread8(ioaddr+ChipCmd) & CmdTxOn)
> -                                       netdev_warn(dev,
> -                                                   "%s: Tx engine still on\n",
> -                                                   __func__);
> -                       }
> -                       rhine_tx(dev);
> -               }
> +       if (status & RHINE_EVENT) {
> +               handled = 1;
>
> -               /* Abnormal error summary/uncommon events handlers. */
> -               if (intr_status & (IntrPCIErr | IntrLinkChange |
> -                                  IntrStatsMax | IntrTxError | IntrTxAborted |
> -                                  IntrTxUnderrun | IntrTxDescRace))
> -                       rhine_error(dev, intr_status);
> +               iowrite16(0x0000, rp->base + IntrEnable);
> +               mmiowb();
> +               napi_schedule(&rp->napi);
> +       }
>
> -               if (--boguscnt < 0) {
> -                       netdev_warn(dev, "Too much work at interrupt, status=%#08x\n",
> -                                   intr_status);
> -                       break;
> -               }
> +       if (status & ~(IntrLinkChange | IntrStatsMax | RHINE_EVENT_NAPI)) {
> +               netif_err(rp, intr, dev, "Something Wicked happened! %08x\n",
> +                         status);
>        }
>
> -       if (debug > 3)
> -               netdev_dbg(dev, "exiting interrupt, status=%08x\n",
> -                          ioread16(ioaddr + IntrStatus));
>        return IRQ_RETVAL(handled);
>  }
>
> @@ -1632,8 +1711,6 @@ static void rhine_tx(struct net_device *dev)
>        struct rhine_private *rp = netdev_priv(dev);
>        int txstatus = 0, entry = rp->dirty_tx % TX_RING_SIZE;
>
> -       spin_lock(&rp->lock);
> -
>        /* find and cleanup dirty tx descriptors */
>        while (rp->dirty_tx != rp->cur_tx) {
>                txstatus = le32_to_cpu(rp->tx_ring[entry].tx_status);
> @@ -1687,8 +1764,6 @@ static void rhine_tx(struct net_device *dev)
>        }
>        if ((rp->cur_tx - rp->dirty_tx) < TX_QUEUE_LEN - 4)
>                netif_wake_queue(dev);
> -
> -       spin_unlock(&rp->lock);
>  }
>
>  /**
> @@ -1839,20 +1914,8 @@ static int rhine_rx(struct net_device *dev, int limit)
>        return count;
>  }
>
> -/*
> - * Clears the "tally counters" for CRC errors and missed frames(?).
> - * It has been reported that some chips need a write of 0 to clear
> - * these, for others the counters are set to 1 when written to and
> - * instead cleared when read. So we clear them both ways ...
> - */
> -static inline void clear_tally_counters(void __iomem *ioaddr)
> +static void rhine_restart_tx(struct net_device *dev)
>  {
> -       iowrite32(0, ioaddr + RxMissed);
> -       ioread16(ioaddr + RxCRCErrs);
> -       ioread16(ioaddr + RxMissed);
> -}
> -
> -static void rhine_restart_tx(struct net_device *dev) {
>        struct rhine_private *rp = netdev_priv(dev);
>        void __iomem *ioaddr = rp->base;
>        int entry = rp->dirty_tx % TX_RING_SIZE;
> @@ -1880,8 +1943,7 @@ static void rhine_restart_tx(struct net_device *dev) {
>                iowrite8(ioread8(ioaddr + ChipCmd1) | Cmd1TxDemand,
>                       ioaddr + ChipCmd1);
>                IOSYNC;
> -       }
> -       else {
> +       } else {
>                /* This should never happen */
>                if (debug > 1)
>                        netdev_warn(dev, "%s() Another error occurred %08x\n",
> @@ -1890,72 +1952,46 @@ static void rhine_restart_tx(struct net_device *dev) {
>
>  }
>
> -static void rhine_error(struct net_device *dev, int intr_status)
> +static void rhine_slow_event_task(struct work_struct *work)
>  {
> -       struct rhine_private *rp = netdev_priv(dev);
> +       struct rhine_private *rp =
> +               container_of(work, struct rhine_private, slow_event_task);
> +       struct net_device *dev = rp->dev;
>        void __iomem *ioaddr = rp->base;
> +       u32 intr_status;
>
> -       spin_lock(&rp->lock);
> +       if (!netif_running(dev))
> +               return;
> +
> +       mutex_lock(&rp->task_lock);
> +
> +       intr_status = get_intr_status(dev);
> +       rhine_ack_event(rp, intr_status & RHINE_EVENT_SLOW);
>
>        if (intr_status & IntrLinkChange)
>                rhine_check_media(dev, 0);
> -       if (intr_status & IntrStatsMax) {
> -               dev->stats.rx_crc_errors += ioread16(ioaddr + RxCRCErrs);
> -               dev->stats.rx_missed_errors += ioread16(ioaddr + RxMissed);
> -               clear_tally_counters(ioaddr);
> -       }
> -       if (intr_status & IntrTxAborted) {
> -               if (debug > 1)
> -                       netdev_info(dev, "Abort %08x, frame dropped\n",
> -                                   intr_status);
> -       }
> -       if (intr_status & IntrTxUnderrun) {
> -               if (rp->tx_thresh < 0xE0)
> -                       BYTE_REG_BITS_SET((rp->tx_thresh += 0x20), 0x80, ioaddr + TxConfig);
> -               if (debug > 1)
> -                       netdev_info(dev, "Transmitter underrun, Tx threshold now %02x\n",
> -                                   rp->tx_thresh);
> -       }
> -       if (intr_status & IntrTxDescRace) {
> -               if (debug > 2)
> -                       netdev_info(dev, "Tx descriptor write-back race\n");
> -       }
> -       if ((intr_status & IntrTxError) &&
> -           (intr_status & (IntrTxAborted |
> -            IntrTxUnderrun | IntrTxDescRace)) == 0) {
> -               if (rp->tx_thresh < 0xE0) {
> -                       BYTE_REG_BITS_SET((rp->tx_thresh += 0x20), 0x80, ioaddr + TxConfig);
> -               }
> -               if (debug > 1)
> -                       netdev_info(dev, "Unspecified error. Tx threshold now %02x\n",
> -                                   rp->tx_thresh);
> -       }
> -       if (intr_status & (IntrTxAborted | IntrTxUnderrun | IntrTxDescRace |
> -                          IntrTxError))
> -               rhine_restart_tx(dev);
>
> -       if (intr_status & ~(IntrLinkChange | IntrStatsMax | IntrTxUnderrun |
> -                           IntrTxError | IntrTxAborted | IntrNormalSummary |
> -                           IntrTxDescRace)) {
> -               if (debug > 1)
> -                       netdev_err(dev, "Something Wicked happened! %08x\n",
> -                                  intr_status);
> -       }
> +       /* FIXME: ignore ? */
> +       if (intr_status & IntrPCIErr)
> +               netif_warn(rp, hw, dev, "PCI error\n");
>
> -       spin_unlock(&rp->lock);
> +       napi_disable(&rp->napi);
> +       iowrite16(0x0000, ioaddr + IntrEnable);
> +       mmiowb();
> +       /* Slow and safe. Consider __napi_schedule as a replacement ? */
> +       napi_enable(&rp->napi);
> +       napi_schedule(&rp->napi);
> +
> +       mutex_unlock(&rp->task_lock);
>  }
>
>  static struct net_device_stats *rhine_get_stats(struct net_device *dev)
>  {
>        struct rhine_private *rp = netdev_priv(dev);
> -       void __iomem *ioaddr = rp->base;
> -       unsigned long flags;
>
> -       spin_lock_irqsave(&rp->lock, flags);
> -       dev->stats.rx_crc_errors += ioread16(ioaddr + RxCRCErrs);
> -       dev->stats.rx_missed_errors += ioread16(ioaddr + RxMissed);
> -       clear_tally_counters(ioaddr);
> -       spin_unlock_irqrestore(&rp->lock, flags);
> +       spin_lock_bh(&rp->lock);
> +       rhine_update_rx_crc_and_missed_errord(rp);
> +       spin_unlock_bh(&rp->lock);
>
>        return &dev->stats;
>  }
> @@ -2022,9 +2058,9 @@ static int netdev_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
>        struct rhine_private *rp = netdev_priv(dev);
>        int rc;
>
> -       spin_lock_irq(&rp->lock);
> +       mutex_lock(&rp->task_lock);
>        rc = mii_ethtool_gset(&rp->mii_if, cmd);
> -       spin_unlock_irq(&rp->lock);
> +       mutex_unlock(&rp->task_lock);
>
>        return rc;
>  }
> @@ -2034,10 +2070,10 @@ static int netdev_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
>        struct rhine_private *rp = netdev_priv(dev);
>        int rc;
>
> -       spin_lock_irq(&rp->lock);
> +       mutex_lock(&rp->task_lock);
>        rc = mii_ethtool_sset(&rp->mii_if, cmd);
> -       spin_unlock_irq(&rp->lock);
>        rhine_set_carrier(&rp->mii_if);
> +       mutex_unlock(&rp->task_lock);
>
>        return rc;
>  }
> @@ -2073,6 +2109,7 @@ static void rhine_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
>        if (!(rp->quirks & rqWOL))
>                return;
>
> +       /* FIXME: huh ? */
>        spin_lock_irq(&rp->lock);
>        wol->supported = WAKE_PHY | WAKE_MAGIC |
>                         WAKE_UCAST | WAKE_MCAST | WAKE_BCAST;  /* Untested */
> @@ -2092,6 +2129,7 @@ static int rhine_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
>        if (wol->wolopts & ~support)
>                return -EINVAL;
>
> +       /* FIXME: huh (sic) ? */
>        spin_lock_irq(&rp->lock);
>        rp->wolopts = wol->wolopts;
>        spin_unlock_irq(&rp->lock);
> @@ -2119,10 +2157,10 @@ static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
>        if (!netif_running(dev))
>                return -EINVAL;
>
> -       spin_lock_irq(&rp->lock);
> +       mutex_lock(&rp->task_lock);
>        rc = generic_mii_ioctl(&rp->mii_if, if_mii(rq), cmd, NULL);
> -       spin_unlock_irq(&rp->lock);
>        rhine_set_carrier(&rp->mii_if);
> +       mutex_unlock(&rp->task_lock);
>
>        return rc;
>  }
> @@ -2133,11 +2171,10 @@ static int rhine_close(struct net_device *dev)
>        void __iomem *ioaddr = rp->base;
>
>        napi_disable(&rp->napi);
> +       cancel_work_sync(&rp->slow_event_task);
>        cancel_work_sync(&rp->reset_task);
>        netif_stop_queue(dev);
>
> -       spin_lock_irq(&rp->lock);
> -
>        if (debug > 1)
>                netdev_dbg(dev, "Shutting down ethercard, status was %04x\n",
>                           ioread16(ioaddr + ChipCmd));
> @@ -2151,8 +2188,6 @@ static int rhine_close(struct net_device *dev)
>        /* Stop the chip's Tx and Rx processes. */
>        iowrite16(CmdStop, ioaddr + ChipCmd);
>
> -       spin_unlock_irq(&rp->lock);
> -
>        free_irq(rp->pdev->irq, dev);
>        free_rbufs(dev);
>        free_tbufs(dev);
> @@ -2239,10 +2274,12 @@ static int rhine_suspend(struct pci_dev *pdev, pm_message_t state)
>        netif_device_detach(dev);
>        pci_save_state(pdev);
>
> +       /* FIXME: ? */
>        spin_lock_irqsave(&rp->lock, flags);
>        rhine_shutdown(pdev);
>        spin_unlock_irqrestore(&rp->lock, flags);
>
> +       /* FIXME: nuclear workaround ? Check the commit log. */
>        free_irq(dev->irq, dev);
>        return 0;
>  }
> @@ -2257,6 +2294,7 @@ static int rhine_resume(struct pci_dev *pdev)
>        if (!netif_running(dev))
>                return 0;
>
> +       /* FIXME: see above. */
>        if (request_irq(dev->irq, rhine_interrupt, IRQF_SHARED, dev->name, dev))
>                netdev_err(dev, "request_irq failed\n");
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Dec. 29, 2011, 10:05 p.m.
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Thu, 29 Dec 2011 19:39:02 +0100

> Francois Romieu <romieu@fr.zoreil.com> :
> [...]
>> Feel free to try the extra "new year's eve week end is getting closer"
>> patch below (against net-next).
>> 
>> You have been warned.
> 
> The one below could be a bit better.

Obviously you'll need to resolve the use of rp->lock as an IRQ safe lock
in {get,src}_wol() so that this lock is consistently used as a BH safe
lock.  Your "Huh?" comments make it clear you are aware of this TODO. :-)

But other than that this patch looks good to me, just some testing and
debugging are needed.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjarke Istrup Pedersen - Dec. 29, 2011, 11:19 p.m.
2011/12/29 David Miller <davem@davemloft.net>:
> From: Francois Romieu <romieu@fr.zoreil.com>
> Date: Thu, 29 Dec 2011 19:39:02 +0100
>
>> Francois Romieu <romieu@fr.zoreil.com> :
>> [...]
>>> Feel free to try the extra "new year's eve week end is getting closer"
>>> patch below (against net-next).
>>>
>>> You have been warned.
>>
>> The one below could be a bit better.
>
> Obviously you'll need to resolve the use of rp->lock as an IRQ safe lock
> in {get,src}_wol() so that this lock is consistently used as a BH safe
> lock.  Your "Huh?" comments make it clear you are aware of this TODO. :-)
>
> But other than that this patch looks good to me, just some testing and
> debugging are needed.

I got it to boot, had to revert another patch introduced in 3.1.5
that's causing alot of people problems, but thats another talk :)

It boots fine, untill it tries to bring up an interface, which causes
it to fails with a kernel panic.
Here is the output of the panic:
http://dev.gentoo.org/~gurligebis/kernel/panic.txt

/Bjarke

> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjarke Istrup Pedersen - Dec. 29, 2011, 11:37 p.m.
2011/12/30 Bjarke Istrup Pedersen <gurligebis@gentoo.org>:
> 2011/12/29 David Miller <davem@davemloft.net>:
>> From: Francois Romieu <romieu@fr.zoreil.com>
>> Date: Thu, 29 Dec 2011 19:39:02 +0100
>>
>>> Francois Romieu <romieu@fr.zoreil.com> :
>>> [...]
>>>> Feel free to try the extra "new year's eve week end is getting closer"
>>>> patch below (against net-next).
>>>>
>>>> You have been warned.
>>>
>>> The one below could be a bit better.
>>
>> Obviously you'll need to resolve the use of rp->lock as an IRQ safe lock
>> in {get,src}_wol() so that this lock is consistently used as a BH safe
>> lock.  Your "Huh?" comments make it clear you are aware of this TODO. :-)
>>
>> But other than that this patch looks good to me, just some testing and
>> debugging are needed.
>
> I got it to boot, had to revert another patch introduced in 3.1.5
> that's causing alot of people problems, but thats another talk :)
>
> It boots fine, untill it tries to bring up an interface, which causes
> it to fails with a kernel panic.
> Here is the output of the panic:
> http://dev.gentoo.org/~gurligebis/kernel/panic.txt
>
> /Bjarke

Hmm, it seems like this problem isn't related to the driver, but
related to the current HEAD of net-next :)

Would it be a problem if I try and use the patch on top of a clean
3.1.6 kernel ?

/Bjarke

>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
françois romieu - Dec. 29, 2011, 11:53 p.m.
Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
[...]
> Would it be a problem if I try and use the patch on top of a clean
> 3.1.6 kernel ?

You will have to "cd drivers/net && patch -p5 ..." as the paths are
different. Avoid suspend/resume/runtime PM.
Bjarke Istrup Pedersen - Dec. 30, 2011, 1:21 a.m.
2011/12/30 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Would it be a problem if I try and use the patch on top of a clean
>> 3.1.6 kernel ?
>
> You will have to "cd drivers/net && patch -p5 ..." as the paths are
> different. Avoid suspend/resume/runtime PM.

The driver seems to be working fine on 3.1.6 - tried VPN'ing into my
network using my phone, and tried copying some files, which seems to
be working.
Not sure if it will be a problem when there is more bandwidth, but so
far it looks alot better.

The log is flodded with this debug info, in case you need it:

Dec 30 01:19:27 woodpecker kernel: via-rhine 0000:00:08.0: lan0:
Interrupt, status 00000002
Dec 30 01:19:27 woodpecker kernel: via-rhine 0000:00:06.0: wan0:
Interrupt, status 00000001
Dec 30 01:19:27 woodpecker kernel: via-rhine 0000:00:06.0: wan0:
Interrupt, status 00000001
Dec 30 01:19:27 woodpecker kernel: via-rhine 0000:00:08.0: lan0:
Interrupt, status 00000002
Dec 30 01:19:27 woodpecker kernel: via-rhine 0000:00:08.0: lan0:
Interrupt, status 00000002
Dec 30 01:19:27 woodpecker kernel: via-rhine 0000:00:08.0: lan0:
Interrupt, status 00000001
Dec 30 01:19:27 woodpecker kernel: via-rhine 0000:00:06.0: wan0:
Interrupt, status 00000002

Looking forward to seeing this in mainline, without the log spamming :)

/Bjarke

> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjarke Istrup Pedersen - Dec. 30, 2011, 11:48 a.m.
2011/12/30 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Would it be a problem if I try and use the patch on top of a clean
>> 3.1.6 kernel ?
>
> You will have to "cd drivers/net && patch -p5 ..." as the paths are
> different. Avoid suspend/resume/runtime PM.

Sorry for asking a probably obvious question, but whats next to get it
fixed and ready for inclusion?

/Bjarke

> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
françois romieu - Dec. 30, 2011, 12:56 p.m.
Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
[...]
> Sorry for asking a probably obvious question, but whats next to get it
> fixed and ready for inclusion?

It should be there for testing and deeper review within a pair of hours.
I ended up vandalizing most of the locking. From a testing viewpoint it
means some amount of work (not mine :o) ) to get a decent coverage.

Once it is known tested and reviewed - it may need a few iterations - it
can be formally submitted for inclusion.
Bjarke Istrup Pedersen - Dec. 30, 2011, 1:51 p.m.
2011/12/30 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Sorry for asking a probably obvious question, but whats next to get it
>> fixed and ready for inclusion?
>
> It should be there for testing and deeper review within a pair of hours.
> I ended up vandalizing most of the locking. From a testing viewpoint it
> means some amount of work (not mine :o) ) to get a decent coverage.

Great, let me know when there is more to test :)

/Bjarke

> Once it is known tested and reviewed - it may need a few iterations - it
> can be formally submitted for inclusion.
>
> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
françois romieu - Dec. 30, 2011, 6:03 p.m.
Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
[...]
> Great, let me know when there is more to test :)

It's done. I have removed linux-kernel from the Cc: where patches are sent
since there is netdev.

If you don't crash trivially, feel free to :
1. loop ip link up / down + module removal / insertion under traffic
2. loop ethtool / vlan commands under traffic
3. do the same as above and perform suspend / resume
Bjarke Istrup Pedersen - Dec. 30, 2011, 6:45 p.m.
2011/12/30 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Great, let me know when there is more to test :)
>
> It's done. I have removed linux-kernel from the Cc: where patches are sent
> since there is netdev.
>
> If you don't crash trivially, feel free to :
> 1. loop ip link up / down + module removal / insertion under traffic
> 2. loop ethtool / vlan commands under traffic
> 3. do the same as above and perform suspend / resume

I can try and do some ip link up and down. - the kernel is built
without module support, to make it as compact as possible.
I can also try ethtool and see if that breaks anything.
It's a headless router, so it doesn't have support for suspend and resume :)

/Bjarke

> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjarke Istrup Pedersen - Dec. 30, 2011, 8:38 p.m.
2011/12/30 Francois Romieu <romieu@fr.zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@gentoo.org> :
> [...]
>> Great, let me know when there is more to test :)
>
> It's done. I have removed linux-kernel from the Cc: where patches are sent
> since there is netdev.
>
> If you don't crash trivially, feel free to :
> 1. loop ip link up / down + module removal / insertion under traffic
> 2. loop ethtool / vlan commands under traffic
> 3. do the same as above and perform suspend / resume

Hey,

Going to test it later tonight.
Just applied it to HEAD of net-next, and got the following compiler warning:
drivers/net/ethernet/via/via-rhine.c:2182:13: warning:
'rhine_task_enable' defined but not used

Just so you know that too :)

/Bjarke

> --
> Ueimor
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
index 5c4983b..66e5c08 100644
--- a/drivers/net/ethernet/via/via-rhine.c
+++ b/drivers/net/ethernet/via/via-rhine.c
@@ -42,7 +42,8 @@ 
 
 #define DEBUG
 static int debug = 1;	/* 1 normal messages, 0 quiet .. 7 verbose. */
-static int max_interrupt_work = 20;
+#define RHINE_MSG_DEFAULT \
+        (NETIF_MSG_INTR)
 
 /* Set the copy breakpoint for the copy-only-tiny-frames scheme.
    Setting to > 1518 effectively disables this feature. */
@@ -128,11 +129,9 @@  MODULE_AUTHOR("Donald Becker <becker@scyld.com>");
 MODULE_DESCRIPTION("VIA Rhine PCI Fast Ethernet driver");
 MODULE_LICENSE("GPL");
 
-module_param(max_interrupt_work, int, 0);
 module_param(debug, int, 0);
 module_param(rx_copybreak, int, 0);
 module_param(avoid_D3, bool, 0);
-MODULE_PARM_DESC(max_interrupt_work, "VIA Rhine maximum events handled per interrupt");
 MODULE_PARM_DESC(debug, "VIA Rhine debug level (0-7)");
 MODULE_PARM_DESC(rx_copybreak, "VIA Rhine copy breakpoint for copy-only-tiny-frames");
 MODULE_PARM_DESC(avoid_D3, "Avoid power state D3 (work-around for broken BIOSes)");
@@ -351,16 +350,25 @@  static const int mmio_verify_registers[] = {
 
 /* Bits in the interrupt status/mask registers. */
 enum intr_status_bits {
-	IntrRxDone=0x0001, IntrRxErr=0x0004, IntrRxEmpty=0x0020,
-	IntrTxDone=0x0002, IntrTxError=0x0008, IntrTxUnderrun=0x0210,
-	IntrPCIErr=0x0040,
-	IntrStatsMax=0x0080, IntrRxEarly=0x0100,
-	IntrRxOverflow=0x0400, IntrRxDropped=0x0800, IntrRxNoBuf=0x1000,
-	IntrTxAborted=0x2000, IntrLinkChange=0x4000,
-	IntrRxWakeUp=0x8000,
-	IntrNormalSummary=0x0003, IntrAbnormalSummary=0xC260,
-	IntrTxDescRace=0x080000,	/* mapped from IntrStatus2 */
-	IntrTxErrSummary=0x082218,
+	IntrRxDone	= 0x0001,
+	IntrTxDone	= 0x0002,
+	IntrRxErr	= 0x0004,
+	IntrTxError	= 0x0008,
+	IntrRxEmpty	= 0x0020,
+	IntrPCIErr	= 0x0040,
+	IntrStatsMax	= 0x0080,
+	IntrRxEarly	= 0x0100,
+	IntrTxUnderrun	= 0x0210,
+	IntrRxOverflow	= 0x0400,
+	IntrRxDropped	= 0x0800,
+	IntrRxNoBuf	= 0x1000,
+	IntrTxAborted	= 0x2000,
+	IntrLinkChange	= 0x4000,
+	IntrRxWakeUp	= 0x8000,
+	IntrTxDescRace		= 0x080000,	/* mapped from IntrStatus2 */
+	IntrNormalSummary	= IntrRxDone | IntrTxDone,
+	IntrTxErrSummary	= IntrTxDescRace | IntrTxAborted | IntrTxError |
+				  IntrTxUnderrun,
 };
 
 /* Bits in WOLcrSet/WOLcrClr and PwrcsrSet/PwrcsrClr */
@@ -439,8 +447,12 @@  struct rhine_private {
 	struct net_device *dev;
 	struct napi_struct napi;
 	spinlock_t lock;
+	struct mutex task_lock;
+	struct work_struct slow_event_task;
 	struct work_struct reset_task;
 
+	u32 msg_enable;
+
 	/* Frequently used values: keep some adjacent for cache effect. */
 	u32 quirks;
 	struct rx_desc *rx_head_desc;
@@ -482,7 +494,6 @@  static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
 static irqreturn_t rhine_interrupt(int irq, void *dev_instance);
 static void rhine_tx(struct net_device *dev);
 static int rhine_rx(struct net_device *dev, int limit);
-static void rhine_error(struct net_device *dev, int intr_status);
 static void rhine_set_rx_mode(struct net_device *dev);
 static struct net_device_stats *rhine_get_stats(struct net_device *dev);
 static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
@@ -497,6 +508,8 @@  static void rhine_set_cam_mask(void __iomem *ioaddr, u32 mask);
 static void rhine_set_vlan_cam_mask(void __iomem *ioaddr, u32 mask);
 static void rhine_init_cam_filter(struct net_device *dev);
 static void rhine_update_vcam(struct net_device *dev);
+static void rhine_slow_event_task(struct work_struct *work);
+static void rhine_restart_tx(struct net_device *dev);
 
 #define RHINE_WAIT_FOR(condition)				\
 do {								\
@@ -508,7 +521,7 @@  do {								\
 			1024 - i, __func__, __LINE__);		\
 } while (0)
 
-static inline u32 get_intr_status(struct net_device *dev)
+static u32 get_intr_status(struct net_device *dev)
 {
 	struct rhine_private *rp = netdev_priv(dev);
 	void __iomem *ioaddr = rp->base;
@@ -521,6 +534,16 @@  static inline u32 get_intr_status(struct net_device *dev)
 	return intr_status;
 }
 
+static void rhine_ack_event(struct rhine_private *rp, u32 mask)
+{
+	void __iomem *ioaddr = rp->base;
+
+	if (rp->quirks & rqStatusWBRace)
+		iowrite8(mask >> 16, ioaddr + IntrStatus2);
+	iowrite16(mask, ioaddr + IntrStatus);
+	mmiowb();
+}
+
 /*
  * Get power related registers into sane state.
  * Notify user about past WOL event.
@@ -657,23 +680,127 @@  static void rhine_poll(struct net_device *dev)
 }
 #endif
 
+static void rhine_kick_tx_threshold(struct rhine_private *rp)
+{
+	if (rp->tx_thresh < 0xe0) {
+		void __iomem *ioaddr = rp->base;
+
+		rp->tx_thresh += 0x20;
+		BYTE_REG_BITS_SET(rp->tx_thresh, 0x80, ioaddr + TxConfig);
+	}
+}
+
+static void rhine_tx_err(struct rhine_private *rp, u32 status)
+{
+	struct net_device *dev = rp->dev;
+
+	if (status & IntrTxAborted) {
+		netif_info(rp, tx_err, dev,
+			   "Abort %08x, frame dropped\n", status);
+	}
+
+	if (status & IntrTxUnderrun) {
+		rhine_kick_tx_threshold(rp);
+		netif_info(rp, tx_err ,dev, "Transmitter underrun, "
+			   "Tx threshold now %02x\n", rp->tx_thresh);
+	}
+
+	if (status & IntrTxDescRace)
+		netif_info(rp, tx_err, dev, "Tx descriptor write-back race\n");
+
+	if ((status & IntrTxError) &&
+	    (status & (IntrTxAborted | IntrTxUnderrun | IntrTxDescRace)) == 0) {
+		rhine_kick_tx_threshold(rp);
+		netif_info(rp, tx_err, dev, "Unspecified error. "
+			   "Tx threshold now %02x\n", rp->tx_thresh);
+	}
+
+	rhine_restart_tx(dev);
+}
+
+static void rhine_update_rx_crc_and_missed_errord(struct rhine_private *rp)
+{
+	void __iomem *ioaddr = rp->base;
+	struct net_device_stats *stats = &rp->dev->stats;
+
+	stats->rx_crc_errors    += ioread16(ioaddr + RxCRCErrs);
+	stats->rx_missed_errors += ioread16(ioaddr + RxMissed);
+
+	/*
+	 * Clears the "tally counters" for CRC errors and missed frames(?).
+	 * It has been reported that some chips need a write of 0 to clear
+	 * these, for others the counters are set to 1 when written to and
+	 * instead cleared when read. So we clear them both ways ...
+	 */
+	iowrite32(0, ioaddr + RxMissed);
+	ioread16(ioaddr + RxCRCErrs);
+	ioread16(ioaddr + RxMissed);
+}
+
+#define RHINE_EVENT_NAPI_RX	(IntrRxDone | \
+				 IntrRxErr | \
+				 IntrRxEmpty | \
+				 IntrRxOverflow	| \
+				 IntrRxDropped | \
+				 IntrRxNoBuf | \
+				 IntrRxWakeUp)
+
+#define RHINE_EVENT_NAPI_TX_ERR	(IntrTxError | \
+				 IntrTxAborted | \
+				 IntrTxUnderrun | \
+				 IntrTxDescRace)
+#define RHINE_EVENT_NAPI_TX	(IntrTxDone | RHINE_EVENT_NAPI_TX_ERR)
+
+#define RHINE_EVENT_NAPI	(RHINE_EVENT_NAPI_RX | \
+				 RHINE_EVENT_NAPI_TX | \
+				 IntrStatsMax)
+#define RHINE_EVENT_SLOW	(IntrPCIErr | IntrLinkChange)
+#define RHINE_EVENT		(RHINE_EVENT_NAPI | RHINE_EVENT_SLOW)
+
 static int rhine_napipoll(struct napi_struct *napi, int budget)
 {
 	struct rhine_private *rp = container_of(napi, struct rhine_private, napi);
 	struct net_device *dev = rp->dev;
 	void __iomem *ioaddr = rp->base;
-	int work_done;
+	u16 enable_mask = RHINE_EVENT & 0xffff;
+	int work_done = 0;
+	u32 status;
+
+	status = get_intr_status(dev);
+	rhine_ack_event(rp, status & ~RHINE_EVENT_SLOW);
+
+	if (status & RHINE_EVENT_NAPI_RX)
+		work_done += rhine_rx(dev, budget);
+
+	if (status & RHINE_EVENT_NAPI_TX) {
+		if (status & RHINE_EVENT_NAPI_TX_ERR) {
+			/* Avoid scavenging before Tx engine turned off */
+			RHINE_WAIT_FOR(!(ioread8(ioaddr + ChipCmd) & CmdTxOn));
+			if (ioread8(ioaddr + ChipCmd) & CmdTxOn)
+				netif_warn(rp, tx_err, dev, "Tx still on\n");
+		}
 
-	work_done = rhine_rx(dev, budget);
+		rhine_tx(dev);
+
+		if (status & RHINE_EVENT_NAPI_TX_ERR)
+			rhine_tx_err(rp, status);
+	}
+
+	if (status & IntrStatsMax) {
+		spin_lock(&rp->lock);
+		rhine_update_rx_crc_and_missed_errord(rp);
+		spin_unlock(&rp->lock);
+	}
+
+	if (status & RHINE_EVENT_SLOW) {
+		enable_mask &= ~RHINE_EVENT_SLOW;
+		schedule_work(&rp->slow_event_task);
+	}
 
 	if (work_done < budget) {
 		napi_complete(napi);
-
-		iowrite16(IntrRxDone | IntrRxErr | IntrRxEmpty| IntrRxOverflow |
-			  IntrRxDropped | IntrRxNoBuf | IntrTxAborted |
-			  IntrTxDone | IntrTxError | IntrTxUnderrun |
-			  IntrPCIErr | IntrStatsMax | IntrLinkChange,
-			  ioaddr + IntrEnable);
+		iowrite16(enable_mask, ioaddr + IntrEnable);
+		mmiowb();
 	}
 	return work_done;
 }
@@ -797,6 +924,7 @@  static int __devinit rhine_init_one(struct pci_dev *pdev,
 	rp->quirks = quirks;
 	rp->pioaddr = pioaddr;
 	rp->pdev = pdev;
+	rp->msg_enable = netif_msg_init(-1, RHINE_MSG_DEFAULT);
 
 	rc = pci_request_regions(pdev, DRV_NAME);
 	if (rc)
@@ -856,7 +984,9 @@  static int __devinit rhine_init_one(struct pci_dev *pdev,
 	dev->irq = pdev->irq;
 
 	spin_lock_init(&rp->lock);
+	mutex_init(&rp->task_lock);
 	INIT_WORK(&rp->reset_task, rhine_reset_task);
+	INIT_WORK(&rp->slow_event_task, rhine_slow_event_task);
 
 	rp->mii_if.dev = dev;
 	rp->mii_if.mdio_read = mdio_read;
@@ -1310,12 +1440,7 @@  static void init_registers(struct net_device *dev)
 
 	napi_enable(&rp->napi);
 
-	/* Enable interrupts by setting the interrupt mask. */
-	iowrite16(IntrRxDone | IntrRxErr | IntrRxEmpty| IntrRxOverflow |
-	       IntrRxDropped | IntrRxNoBuf | IntrTxAborted |
-	       IntrTxDone | IntrTxError | IntrTxUnderrun |
-	       IntrPCIErr | IntrStatsMax | IntrLinkChange,
-	       ioaddr + IntrEnable);
+	iowrite16(RHINE_EVENT & 0xffff, ioaddr + IntrEnable);
 
 	iowrite16(CmdStart | CmdTxOn | CmdRxOn | (Cmd1NoTxPoll << 8),
 	       ioaddr + ChipCmd);
@@ -1434,13 +1559,10 @@  static void rhine_reset_task(struct work_struct *work)
 						reset_task);
 	struct net_device *dev = rp->dev;
 
-	/* protect against concurrent rx interrupts */
-	disable_irq(rp->pdev->irq);
+	mutex_lock(&rp->task_lock);
 
 	napi_disable(&rp->napi);
 
-	spin_lock_bh(&rp->lock);
-
 	/* clear all descriptors */
 	free_tbufs(dev);
 	free_rbufs(dev);
@@ -1451,12 +1573,11 @@  static void rhine_reset_task(struct work_struct *work)
 	rhine_chip_reset(dev);
 	init_registers(dev);
 
-	spin_unlock_bh(&rp->lock);
-	enable_irq(rp->pdev->irq);
-
 	dev->trans_start = jiffies; /* prevent tx timeout */
 	dev->stats.tx_errors++;
 	netif_wake_queue(dev);
+
+	mutex_unlock(&rp->task_lock);
 }
 
 static void rhine_tx_timeout(struct net_device *dev)
@@ -1477,7 +1598,6 @@  static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
 	struct rhine_private *rp = netdev_priv(dev);
 	void __iomem *ioaddr = rp->base;
 	unsigned entry;
-	unsigned long flags;
 
 	/* Caution: the write order is important here, set the field
 	   with the "ownership" bits last. */
@@ -1529,7 +1649,6 @@  static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
 		rp->tx_ring[entry].tx_status = 0;
 
 	/* lock eth irq */
-	spin_lock_irqsave(&rp->lock, flags);
 	wmb();
 	rp->tx_ring[entry].tx_status |= cpu_to_le32(DescOwn);
 	wmb();
@@ -1543,15 +1662,12 @@  static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
 		BYTE_REG_BITS_ON(1 << 7, ioaddr + TQWake);
 
 	/* Wake the potentially-idle transmit channel */
-	iowrite8(ioread8(ioaddr + ChipCmd1) | Cmd1TxDemand,
-	       ioaddr + ChipCmd1);
+	iowrite8(ioread8(ioaddr + ChipCmd1) | Cmd1TxDemand, ioaddr + ChipCmd1);
 	IOSYNC;
 
 	if (rp->cur_tx == rp->dirty_tx + TX_QUEUE_LEN)
 		netif_stop_queue(dev);
 
-	spin_unlock_irqrestore(&rp->lock, flags);
-
 	if (debug > 4) {
 		netdev_dbg(dev, "Transmit frame #%d queued in slot %d\n",
 			   rp->cur_tx-1, entry);
@@ -1565,63 +1681,26 @@  static irqreturn_t rhine_interrupt(int irq, void *dev_instance)
 {
 	struct net_device *dev = dev_instance;
 	struct rhine_private *rp = netdev_priv(dev);
-	void __iomem *ioaddr = rp->base;
-	u32 intr_status;
-	int boguscnt = max_interrupt_work;
+	u32 status;
 	int handled = 0;
 
-	while ((intr_status = get_intr_status(dev))) {
-		handled = 1;
-
-		/* Acknowledge all of the current interrupt sources ASAP. */
-		if (intr_status & IntrTxDescRace)
-			iowrite8(0x08, ioaddr + IntrStatus2);
-		iowrite16(intr_status & 0xffff, ioaddr + IntrStatus);
-		IOSYNC;
-
-		if (debug > 4)
-			netdev_dbg(dev, "Interrupt, status %08x\n",
-				   intr_status);
-
-		if (intr_status & (IntrRxDone | IntrRxErr | IntrRxDropped |
-				   IntrRxWakeUp | IntrRxEmpty | IntrRxNoBuf)) {
-			iowrite16(IntrTxAborted |
-				  IntrTxDone | IntrTxError | IntrTxUnderrun |
-				  IntrPCIErr | IntrStatsMax | IntrLinkChange,
-				  ioaddr + IntrEnable);
+	status = get_intr_status(dev);
 
-			napi_schedule(&rp->napi);
-		}
+	netif_dbg(rp, intr, dev, "Interrupt, status %08x\n", status);
 
-		if (intr_status & (IntrTxErrSummary | IntrTxDone)) {
-			if (intr_status & IntrTxErrSummary) {
-				/* Avoid scavenging before Tx engine turned off */
-				RHINE_WAIT_FOR(!(ioread8(ioaddr+ChipCmd) & CmdTxOn));
-				if (debug > 2 &&
-				    ioread8(ioaddr+ChipCmd) & CmdTxOn)
-					netdev_warn(dev,
-						    "%s: Tx engine still on\n",
-						    __func__);
-			}
-			rhine_tx(dev);
-		}
+	if (status & RHINE_EVENT) {
+		handled = 1;
 
-		/* Abnormal error summary/uncommon events handlers. */
-		if (intr_status & (IntrPCIErr | IntrLinkChange |
-				   IntrStatsMax | IntrTxError | IntrTxAborted |
-				   IntrTxUnderrun | IntrTxDescRace))
-			rhine_error(dev, intr_status);
+		iowrite16(0x0000, rp->base + IntrEnable);
+		mmiowb();
+		napi_schedule(&rp->napi);
+	}
 
-		if (--boguscnt < 0) {
-			netdev_warn(dev, "Too much work at interrupt, status=%#08x\n",
-				    intr_status);
-			break;
-		}
+	if (status & ~(IntrLinkChange | IntrStatsMax | RHINE_EVENT_NAPI)) {
+		netif_err(rp, intr, dev, "Something Wicked happened! %08x\n",
+			  status);
 	}
 
-	if (debug > 3)
-		netdev_dbg(dev, "exiting interrupt, status=%08x\n",
-			   ioread16(ioaddr + IntrStatus));
 	return IRQ_RETVAL(handled);
 }
 
@@ -1632,8 +1711,6 @@  static void rhine_tx(struct net_device *dev)
 	struct rhine_private *rp = netdev_priv(dev);
 	int txstatus = 0, entry = rp->dirty_tx % TX_RING_SIZE;
 
-	spin_lock(&rp->lock);
-
 	/* find and cleanup dirty tx descriptors */
 	while (rp->dirty_tx != rp->cur_tx) {
 		txstatus = le32_to_cpu(rp->tx_ring[entry].tx_status);
@@ -1687,8 +1764,6 @@  static void rhine_tx(struct net_device *dev)
 	}
 	if ((rp->cur_tx - rp->dirty_tx) < TX_QUEUE_LEN - 4)
 		netif_wake_queue(dev);
-
-	spin_unlock(&rp->lock);
 }
 
 /**
@@ -1839,20 +1914,8 @@  static int rhine_rx(struct net_device *dev, int limit)
 	return count;
 }
 
-/*
- * Clears the "tally counters" for CRC errors and missed frames(?).
- * It has been reported that some chips need a write of 0 to clear
- * these, for others the counters are set to 1 when written to and
- * instead cleared when read. So we clear them both ways ...
- */
-static inline void clear_tally_counters(void __iomem *ioaddr)
+static void rhine_restart_tx(struct net_device *dev)
 {
-	iowrite32(0, ioaddr + RxMissed);
-	ioread16(ioaddr + RxCRCErrs);
-	ioread16(ioaddr + RxMissed);
-}
-
-static void rhine_restart_tx(struct net_device *dev) {
 	struct rhine_private *rp = netdev_priv(dev);
 	void __iomem *ioaddr = rp->base;
 	int entry = rp->dirty_tx % TX_RING_SIZE;
@@ -1880,8 +1943,7 @@  static void rhine_restart_tx(struct net_device *dev) {
 		iowrite8(ioread8(ioaddr + ChipCmd1) | Cmd1TxDemand,
 		       ioaddr + ChipCmd1);
 		IOSYNC;
-	}
-	else {
+	} else {
 		/* This should never happen */
 		if (debug > 1)
 			netdev_warn(dev, "%s() Another error occurred %08x\n",
@@ -1890,72 +1952,46 @@  static void rhine_restart_tx(struct net_device *dev) {
 
 }
 
-static void rhine_error(struct net_device *dev, int intr_status)
+static void rhine_slow_event_task(struct work_struct *work)
 {
-	struct rhine_private *rp = netdev_priv(dev);
+	struct rhine_private *rp =
+		container_of(work, struct rhine_private, slow_event_task);
+	struct net_device *dev = rp->dev;
 	void __iomem *ioaddr = rp->base;
+	u32 intr_status;
 
-	spin_lock(&rp->lock);
+	if (!netif_running(dev))
+		return;
+
+	mutex_lock(&rp->task_lock);
+
+	intr_status = get_intr_status(dev);
+	rhine_ack_event(rp, intr_status & RHINE_EVENT_SLOW);
 
 	if (intr_status & IntrLinkChange)
 		rhine_check_media(dev, 0);
-	if (intr_status & IntrStatsMax) {
-		dev->stats.rx_crc_errors += ioread16(ioaddr + RxCRCErrs);
-		dev->stats.rx_missed_errors += ioread16(ioaddr + RxMissed);
-		clear_tally_counters(ioaddr);
-	}
-	if (intr_status & IntrTxAborted) {
-		if (debug > 1)
-			netdev_info(dev, "Abort %08x, frame dropped\n",
-				    intr_status);
-	}
-	if (intr_status & IntrTxUnderrun) {
-		if (rp->tx_thresh < 0xE0)
-			BYTE_REG_BITS_SET((rp->tx_thresh += 0x20), 0x80, ioaddr + TxConfig);
-		if (debug > 1)
-			netdev_info(dev, "Transmitter underrun, Tx threshold now %02x\n",
-				    rp->tx_thresh);
-	}
-	if (intr_status & IntrTxDescRace) {
-		if (debug > 2)
-			netdev_info(dev, "Tx descriptor write-back race\n");
-	}
-	if ((intr_status & IntrTxError) &&
-	    (intr_status & (IntrTxAborted |
-	     IntrTxUnderrun | IntrTxDescRace)) == 0) {
-		if (rp->tx_thresh < 0xE0) {
-			BYTE_REG_BITS_SET((rp->tx_thresh += 0x20), 0x80, ioaddr + TxConfig);
-		}
-		if (debug > 1)
-			netdev_info(dev, "Unspecified error. Tx threshold now %02x\n",
-				    rp->tx_thresh);
-	}
-	if (intr_status & (IntrTxAborted | IntrTxUnderrun | IntrTxDescRace |
-			   IntrTxError))
-		rhine_restart_tx(dev);
 
-	if (intr_status & ~(IntrLinkChange | IntrStatsMax | IntrTxUnderrun |
-			    IntrTxError | IntrTxAborted | IntrNormalSummary |
-			    IntrTxDescRace)) {
-		if (debug > 1)
-			netdev_err(dev, "Something Wicked happened! %08x\n",
-				   intr_status);
-	}
+	/* FIXME: ignore ? */
+	if (intr_status & IntrPCIErr)
+		netif_warn(rp, hw, dev, "PCI error\n");
 
-	spin_unlock(&rp->lock);
+	napi_disable(&rp->napi);
+	iowrite16(0x0000, ioaddr + IntrEnable);
+	mmiowb();
+	/* Slow and safe. Consider __napi_schedule as a replacement ? */
+	napi_enable(&rp->napi);
+	napi_schedule(&rp->napi);
+
+	mutex_unlock(&rp->task_lock);
 }
 
 static struct net_device_stats *rhine_get_stats(struct net_device *dev)
 {
 	struct rhine_private *rp = netdev_priv(dev);
-	void __iomem *ioaddr = rp->base;
-	unsigned long flags;
 
-	spin_lock_irqsave(&rp->lock, flags);
-	dev->stats.rx_crc_errors += ioread16(ioaddr + RxCRCErrs);
-	dev->stats.rx_missed_errors += ioread16(ioaddr + RxMissed);
-	clear_tally_counters(ioaddr);
-	spin_unlock_irqrestore(&rp->lock, flags);
+	spin_lock_bh(&rp->lock);
+	rhine_update_rx_crc_and_missed_errord(rp);
+	spin_unlock_bh(&rp->lock);
 
 	return &dev->stats;
 }
@@ -2022,9 +2058,9 @@  static int netdev_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 	struct rhine_private *rp = netdev_priv(dev);
 	int rc;
 
-	spin_lock_irq(&rp->lock);
+	mutex_lock(&rp->task_lock);
 	rc = mii_ethtool_gset(&rp->mii_if, cmd);
-	spin_unlock_irq(&rp->lock);
+	mutex_unlock(&rp->task_lock);
 
 	return rc;
 }
@@ -2034,10 +2070,10 @@  static int netdev_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 	struct rhine_private *rp = netdev_priv(dev);
 	int rc;
 
-	spin_lock_irq(&rp->lock);
+	mutex_lock(&rp->task_lock);
 	rc = mii_ethtool_sset(&rp->mii_if, cmd);
-	spin_unlock_irq(&rp->lock);
 	rhine_set_carrier(&rp->mii_if);
+	mutex_unlock(&rp->task_lock);
 
 	return rc;
 }
@@ -2073,6 +2109,7 @@  static void rhine_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 	if (!(rp->quirks & rqWOL))
 		return;
 
+	/* FIXME: huh ? */
 	spin_lock_irq(&rp->lock);
 	wol->supported = WAKE_PHY | WAKE_MAGIC |
 			 WAKE_UCAST | WAKE_MCAST | WAKE_BCAST;	/* Untested */
@@ -2092,6 +2129,7 @@  static int rhine_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 	if (wol->wolopts & ~support)
 		return -EINVAL;
 
+	/* FIXME: huh (sic) ? */
 	spin_lock_irq(&rp->lock);
 	rp->wolopts = wol->wolopts;
 	spin_unlock_irq(&rp->lock);
@@ -2119,10 +2157,10 @@  static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	if (!netif_running(dev))
 		return -EINVAL;
 
-	spin_lock_irq(&rp->lock);
+	mutex_lock(&rp->task_lock);
 	rc = generic_mii_ioctl(&rp->mii_if, if_mii(rq), cmd, NULL);
-	spin_unlock_irq(&rp->lock);
 	rhine_set_carrier(&rp->mii_if);
+	mutex_unlock(&rp->task_lock);
 
 	return rc;
 }
@@ -2133,11 +2171,10 @@  static int rhine_close(struct net_device *dev)
 	void __iomem *ioaddr = rp->base;
 
 	napi_disable(&rp->napi);
+	cancel_work_sync(&rp->slow_event_task);
 	cancel_work_sync(&rp->reset_task);
 	netif_stop_queue(dev);
 
-	spin_lock_irq(&rp->lock);
-
 	if (debug > 1)
 		netdev_dbg(dev, "Shutting down ethercard, status was %04x\n",
 			   ioread16(ioaddr + ChipCmd));
@@ -2151,8 +2188,6 @@  static int rhine_close(struct net_device *dev)
 	/* Stop the chip's Tx and Rx processes. */
 	iowrite16(CmdStop, ioaddr + ChipCmd);
 
-	spin_unlock_irq(&rp->lock);
-
 	free_irq(rp->pdev->irq, dev);
 	free_rbufs(dev);
 	free_tbufs(dev);
@@ -2239,10 +2274,12 @@  static int rhine_suspend(struct pci_dev *pdev, pm_message_t state)
 	netif_device_detach(dev);
 	pci_save_state(pdev);
 
+	/* FIXME: ? */
 	spin_lock_irqsave(&rp->lock, flags);
 	rhine_shutdown(pdev);
 	spin_unlock_irqrestore(&rp->lock, flags);
 
+	/* FIXME: nuclear workaround ? Check the commit log. */
 	free_irq(dev->irq, dev);
 	return 0;
 }
@@ -2257,6 +2294,7 @@  static int rhine_resume(struct pci_dev *pdev)
 	if (!netif_running(dev))
 		return 0;
 
+	/* FIXME: see above. */
 	if (request_irq(dev->irq, rhine_interrupt, IRQF_SHARED, dev->name, dev))
 		netdev_err(dev, "request_irq failed\n");