[PATCH] Fix locking in gianfar
This patch fixes several bugs in the gianfar driver, including a major one
where spinlocks were horribly broken:
* Split gianfar locks into two types: TX and RX
* Made it so gfar_start() now clears RHALT
* Fixed a bug where calling gfar_start_xmit() with interrupts off would
corrupt the interrupt state
* Fixed a bug where a frame could potentially arrive, and never be handled
(if no more frames arrived
* Fixed a bug where the rx_work_limit would never be observed by the rx
completion code
* Fixed a bug where the interrupt handlers were not actually protected by
their spinlocks
Signed-off-by: Andy Fleming <afleming@freescale.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 771e25d..218d317 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -210,7 +210,8 @@
goto regs_fail;
}
- spin_lock_init(&priv->lock);
+ spin_lock_init(&priv->txlock);
+ spin_lock_init(&priv->rxlock);
platform_set_drvdata(pdev, dev);
@@ -515,11 +516,13 @@
phy_stop(priv->phydev);
/* Lock it down */
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock_irqsave(&priv->txlock, flags);
+ spin_lock(&priv->rxlock);
gfar_halt(dev);
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock(&priv->rxlock);
+ spin_unlock_irqrestore(&priv->txlock, flags);
/* Free the IRQs */
if (priv->einfo->device_flags & FSL_GIANFAR_DEV_HAS_MULTI_INTR) {
@@ -605,14 +608,15 @@
tempval |= DMACTRL_INIT_SETTINGS;
gfar_write(&priv->regs->dmactrl, tempval);
- /* Clear THLT, so that the DMA starts polling now */
- gfar_write(®s->tstat, TSTAT_CLEAR_THALT);
-
/* Make sure we aren't stopped */
tempval = gfar_read(&priv->regs->dmactrl);
tempval &= ~(DMACTRL_GRS | DMACTRL_GTS);
gfar_write(&priv->regs->dmactrl, tempval);
+ /* Clear THLT/RHLT, so that the DMA starts polling now */
+ gfar_write(®s->tstat, TSTAT_CLEAR_THALT);
+ gfar_write(®s->rstat, RSTAT_CLEAR_RHALT);
+
/* Unmask the interrupts we look for */
gfar_write(®s->imask, IMASK_DEFAULT);
}
@@ -928,12 +932,13 @@
struct txfcb *fcb = NULL;
struct txbd8 *txbdp;
u16 status;
+ unsigned long flags;
/* Update transmit stats */
priv->stats.tx_bytes += skb->len;
/* Lock priv now */
- spin_lock_irq(&priv->lock);
+ spin_lock_irqsave(&priv->txlock, flags);
/* Point at the first free tx descriptor */
txbdp = priv->cur_tx;
@@ -1004,7 +1009,7 @@
gfar_write(&priv->regs->tstat, TSTAT_CLEAR_THALT);
/* Unlock priv */
- spin_unlock_irq(&priv->lock);
+ spin_unlock_irqrestore(&priv->txlock, flags);
return 0;
}
@@ -1049,7 +1054,7 @@
unsigned long flags;
u32 tempval;
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock_irqsave(&priv->rxlock, flags);
priv->vlgrp = grp;
@@ -1076,7 +1081,7 @@
gfar_write(&priv->regs->rctrl, tempval);
}
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock_irqrestore(&priv->rxlock, flags);
}
@@ -1085,12 +1090,12 @@
struct gfar_private *priv = netdev_priv(dev);
unsigned long flags;
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock_irqsave(&priv->rxlock, flags);
if (priv->vlgrp)
priv->vlgrp->vlan_devices[vid] = NULL;
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock_irqrestore(&priv->rxlock, flags);
}
@@ -1179,7 +1184,7 @@
gfar_write(&priv->regs->ievent, IEVENT_TX_MASK);
/* Lock priv */
- spin_lock(&priv->lock);
+ spin_lock(&priv->txlock);
bdp = priv->dirty_tx;
while ((bdp->status & TXBD_READY) == 0) {
/* If dirty_tx and cur_tx are the same, then either the */
@@ -1224,7 +1229,7 @@
else
gfar_write(&priv->regs->txic, 0);
- spin_unlock(&priv->lock);
+ spin_unlock(&priv->txlock);
return IRQ_HANDLED;
}
@@ -1305,9 +1310,10 @@
{
struct net_device *dev = (struct net_device *) dev_id;
struct gfar_private *priv = netdev_priv(dev);
-
#ifdef CONFIG_GFAR_NAPI
u32 tempval;
+#else
+ unsigned long flags;
#endif
/* Clear IEVENT, so rx interrupt isn't called again
@@ -1330,7 +1336,7 @@
}
#else
- spin_lock(&priv->lock);
+ spin_lock_irqsave(&priv->rxlock, flags);
gfar_clean_rx_ring(dev, priv->rx_ring_size);
/* If we are coalescing interrupts, update the timer */
@@ -1341,7 +1347,7 @@
else
gfar_write(&priv->regs->rxic, 0);
- spin_unlock(&priv->lock);
+ spin_unlock_irqrestore(&priv->rxlock, flags);
#endif
return IRQ_HANDLED;
@@ -1490,13 +1496,6 @@
/* Update the current rxbd pointer to be the next one */
priv->cur_rx = bdp;
- /* If no packets have arrived since the
- * last one we processed, clear the IEVENT RX and
- * BSY bits so that another interrupt won't be
- * generated when we set IMASK */
- if (bdp->status & RXBD_EMPTY)
- gfar_write(&priv->regs->ievent, IEVENT_RX_MASK);
-
return howmany;
}
@@ -1516,7 +1515,7 @@
rx_work_limit -= howmany;
*budget -= howmany;
- if (rx_work_limit >= 0) {
+ if (rx_work_limit > 0) {
netif_rx_complete(dev);
/* Clear the halt bit in RSTAT */
@@ -1533,7 +1532,8 @@
gfar_write(&priv->regs->rxic, 0);
}
- return (rx_work_limit < 0) ? 1 : 0;
+ /* Return 1 if there's more work to do */
+ return (rx_work_limit > 0) ? 0 : 1;
}
#endif
@@ -1629,7 +1629,7 @@
struct phy_device *phydev = priv->phydev;
int new_state = 0;
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock_irqsave(&priv->txlock, flags);
if (phydev->link) {
u32 tempval = gfar_read(®s->maccfg2);
u32 ecntrl = gfar_read(®s->ecntrl);
@@ -1694,7 +1694,7 @@
if (new_state && netif_msg_link(priv))
phy_print_status(phydev);
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock_irqrestore(&priv->txlock, flags);
}
/* Update the hash table based on the current list of multicast
diff --git a/drivers/net/gianfar.h b/drivers/net/gianfar.h
index d37d540..127c98c 100644
--- a/drivers/net/gianfar.h
+++ b/drivers/net/gianfar.h
@@ -656,43 +656,62 @@
* the buffer descriptor determines the actual condition.
*/
struct gfar_private {
- /* pointers to arrays of skbuffs for tx and rx */
+ /* Fields controlled by TX lock */
+ spinlock_t txlock;
+
+ /* Pointer to the array of skbuffs */
struct sk_buff ** tx_skbuff;
- struct sk_buff ** rx_skbuff;
- /* indices pointing to the next free sbk in skb arrays */
+ /* next free skb in the array */
u16 skb_curtx;
- u16 skb_currx;
- /* index of the first skb which hasn't been transmitted
- * yet. */
+ /* First skb in line to be transmitted */
u16 skb_dirtytx;
/* Configuration info for the coalescing features */
unsigned char txcoalescing;
unsigned short txcount;
unsigned short txtime;
+
+ /* Buffer descriptor pointers */
+ struct txbd8 *tx_bd_base; /* First tx buffer descriptor */
+ struct txbd8 *cur_tx; /* Next free ring entry */
+ struct txbd8 *dirty_tx; /* First buffer in line
+ to be transmitted */
+ unsigned int tx_ring_size;
+
+ /* RX Locked fields */
+ spinlock_t rxlock;
+
+ /* skb array and index */
+ struct sk_buff ** rx_skbuff;
+ u16 skb_currx;
+
+ /* RX Coalescing values */
unsigned char rxcoalescing;
unsigned short rxcount;
unsigned short rxtime;
- /* GFAR addresses */
- struct rxbd8 *rx_bd_base; /* Base addresses of Rx and Tx Buffers */
- struct txbd8 *tx_bd_base;
+ struct rxbd8 *rx_bd_base; /* First Rx buffers */
struct rxbd8 *cur_rx; /* Next free rx ring entry */
- struct txbd8 *cur_tx; /* Next free ring entry */
- struct txbd8 *dirty_tx; /* The Ring entry to be freed. */
- struct gfar __iomem *regs; /* Pointer to the GFAR memory mapped Registers */
- u32 __iomem *hash_regs[16];
- int hash_width;
- struct net_device_stats stats; /* linux network statistics */
- struct gfar_extra_stats extra_stats;
- spinlock_t lock;
+
+ /* RX parameters */
+ unsigned int rx_ring_size;
unsigned int rx_buffer_size;
unsigned int rx_stash_size;
unsigned int rx_stash_index;
- unsigned int tx_ring_size;
- unsigned int rx_ring_size;
+
+ struct vlan_group *vlgrp;
+
+ /* Unprotected fields */
+ /* Pointer to the GFAR memory mapped Registers */
+ struct gfar __iomem *regs;
+
+ /* Hash registers and their width */
+ u32 __iomem *hash_regs[16];
+ int hash_width;
+
+ /* global parameters */
unsigned int fifo_threshold;
unsigned int fifo_starve;
unsigned int fifo_starve_off;
@@ -702,13 +721,15 @@
extended_hash:1,
bd_stash_en:1;
unsigned short padding;
- struct vlan_group *vlgrp;
- /* Info structure initialized by board setup code */
+
unsigned int interruptTransmit;
unsigned int interruptReceive;
unsigned int interruptError;
+
+ /* info structure initialized by platform code */
struct gianfar_platform_data *einfo;
+ /* PHY stuff */
struct phy_device *phydev;
struct mii_bus *mii_bus;
int oldspeed;
@@ -716,6 +737,10 @@
int oldlink;
uint32_t msg_enable;
+
+ /* Network Statistics */
+ struct net_device_stats stats;
+ struct gfar_extra_stats extra_stats;
};
static inline u32 gfar_read(volatile unsigned __iomem *addr)
diff --git a/drivers/net/gianfar_ethtool.c b/drivers/net/gianfar_ethtool.c
index 5de7b2e..d69698c 100644
--- a/drivers/net/gianfar_ethtool.c
+++ b/drivers/net/gianfar_ethtool.c
@@ -455,10 +455,14 @@
/* Halt TX and RX, and process the frames which
* have already been received */
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock_irqsave(&priv->txlock, flags);
+ spin_lock(&priv->rxlock);
+
gfar_halt(dev);
gfar_clean_rx_ring(dev, priv->rx_ring_size);
- spin_unlock_irqrestore(&priv->lock, flags);
+
+ spin_unlock(&priv->rxlock);
+ spin_unlock_irqrestore(&priv->txlock, flags);
/* Now we take down the rings to rebuild them */
stop_gfar(dev);
@@ -488,10 +492,14 @@
/* Halt TX and RX, and process the frames which
* have already been received */
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock_irqsave(&priv->txlock, flags);
+ spin_lock(&priv->rxlock);
+
gfar_halt(dev);
gfar_clean_rx_ring(dev, priv->rx_ring_size);
- spin_unlock_irqrestore(&priv->lock, flags);
+
+ spin_unlock(&priv->rxlock);
+ spin_unlock_irqrestore(&priv->txlock, flags);
/* Now we take down the rings to rebuild them */
stop_gfar(dev);
@@ -523,7 +531,7 @@
if (!(priv->einfo->device_flags & FSL_GIANFAR_DEV_HAS_CSUM))
return -EOPNOTSUPP;
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock_irqsave(&priv->txlock, flags);
gfar_halt(dev);
if (data)
@@ -532,7 +540,7 @@
dev->features &= ~NETIF_F_IP_CSUM;
gfar_start(dev);
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock_irqrestore(&priv->txlock, flags);
return 0;
}
diff --git a/drivers/net/gianfar_sysfs.c b/drivers/net/gianfar_sysfs.c
index 51ef181..a6d5c43 100644
--- a/drivers/net/gianfar_sysfs.c
+++ b/drivers/net/gianfar_sysfs.c
@@ -82,7 +82,7 @@
else
return count;
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock_irqsave(&priv->rxlock, flags);
/* Set the new stashing value */
priv->bd_stash_en = new_setting;
@@ -96,7 +96,7 @@
gfar_write(&priv->regs->attr, temp);
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock_irqrestore(&priv->rxlock, flags);
return count;
}
@@ -118,7 +118,7 @@
u32 temp;
unsigned long flags;
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock_irqsave(&priv->rxlock, flags);
if (length > priv->rx_buffer_size)
return count;
@@ -142,7 +142,7 @@
gfar_write(&priv->regs->attr, temp);
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock_irqrestore(&priv->rxlock, flags);
return count;
}
@@ -166,7 +166,7 @@
u32 temp;
unsigned long flags;
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock_irqsave(&priv->rxlock, flags);
if (index > priv->rx_stash_size)
return count;
@@ -180,7 +180,7 @@
temp |= ATTRELI_EI(index);
gfar_write(&priv->regs->attreli, flags);
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock_irqrestore(&priv->rxlock, flags);
return count;
}
@@ -205,7 +205,7 @@
if (length > GFAR_MAX_FIFO_THRESHOLD)
return count;
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock_irqsave(&priv->txlock, flags);
priv->fifo_threshold = length;
@@ -214,7 +214,7 @@
temp |= length;
gfar_write(&priv->regs->fifo_tx_thr, temp);
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock_irqrestore(&priv->txlock, flags);
return count;
}
@@ -240,7 +240,7 @@
if (num > GFAR_MAX_FIFO_STARVE)
return count;
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock_irqsave(&priv->txlock, flags);
priv->fifo_starve = num;
@@ -249,7 +249,7 @@
temp |= num;
gfar_write(&priv->regs->fifo_tx_starve, temp);
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock_irqrestore(&priv->txlock, flags);
return count;
}
@@ -274,7 +274,7 @@
if (num > GFAR_MAX_FIFO_STARVE_OFF)
return count;
- spin_lock_irqsave(&priv->lock, flags);
+ spin_lock_irqsave(&priv->txlock, flags);
priv->fifo_starve_off = num;
@@ -283,7 +283,7 @@
temp |= num;
gfar_write(&priv->regs->fifo_tx_starve_shutoff, temp);
- spin_unlock_irqrestore(&priv->lock, flags);
+ spin_unlock_irqrestore(&priv->txlock, flags);
return count;
}