Skip to content

Commit

Permalink
[PATCH] Fix locking in gianfar
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
Andy Fleming authored and Jeff Garzik committed Apr 20, 2006
1 parent f18b95c commit fef6108
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 67 deletions.
56 changes: 28 additions & 28 deletions drivers/net/gianfar.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ static int gfar_probe(struct platform_device *pdev)
goto regs_fail;
}

spin_lock_init(&priv->lock);
spin_lock_init(&priv->txlock);
spin_lock_init(&priv->rxlock);

platform_set_drvdata(pdev, dev);

Expand Down Expand Up @@ -515,11 +516,13 @@ void stop_gfar(struct net_device *dev)
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) {
Expand Down Expand Up @@ -605,14 +608,15 @@ void gfar_start(struct net_device *dev)
tempval |= DMACTRL_INIT_SETTINGS;
gfar_write(&priv->regs->dmactrl, tempval);

/* Clear THLT, so that the DMA starts polling now */
gfar_write(&regs->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(&regs->tstat, TSTAT_CLEAR_THALT);
gfar_write(&regs->rstat, RSTAT_CLEAR_RHALT);

/* Unmask the interrupts we look for */
gfar_write(&regs->imask, IMASK_DEFAULT);
}
Expand Down Expand Up @@ -928,12 +932,13 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
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;
Expand Down Expand Up @@ -1004,7 +1009,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
gfar_write(&priv->regs->tstat, TSTAT_CLEAR_THALT);

/* Unlock priv */
spin_unlock_irq(&priv->lock);
spin_unlock_irqrestore(&priv->txlock, flags);

return 0;
}
Expand Down Expand Up @@ -1049,7 +1054,7 @@ static void gfar_vlan_rx_register(struct net_device *dev,
unsigned long flags;
u32 tempval;

spin_lock_irqsave(&priv->lock, flags);
spin_lock_irqsave(&priv->rxlock, flags);

priv->vlgrp = grp;

Expand All @@ -1076,7 +1081,7 @@ static void gfar_vlan_rx_register(struct net_device *dev,
gfar_write(&priv->regs->rctrl, tempval);
}

spin_unlock_irqrestore(&priv->lock, flags);
spin_unlock_irqrestore(&priv->rxlock, flags);
}


Expand All @@ -1085,12 +1090,12 @@ static void gfar_vlan_rx_kill_vid(struct net_device *dev, uint16_t vid)
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);
}


Expand Down Expand Up @@ -1179,7 +1184,7 @@ static irqreturn_t gfar_transmit(int irq, void *dev_id, struct pt_regs *regs)
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 */
Expand Down Expand Up @@ -1224,7 +1229,7 @@ static irqreturn_t gfar_transmit(int irq, void *dev_id, struct pt_regs *regs)
else
gfar_write(&priv->regs->txic, 0);

spin_unlock(&priv->lock);
spin_unlock(&priv->txlock);

return IRQ_HANDLED;
}
Expand Down Expand Up @@ -1305,9 +1310,10 @@ irqreturn_t gfar_receive(int irq, void *dev_id, struct pt_regs *regs)
{
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
Expand All @@ -1330,7 +1336,7 @@ irqreturn_t gfar_receive(int irq, void *dev_id, struct pt_regs *regs)
}
#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 */
Expand All @@ -1341,7 +1347,7 @@ irqreturn_t gfar_receive(int irq, void *dev_id, struct pt_regs *regs)
else
gfar_write(&priv->regs->rxic, 0);

spin_unlock(&priv->lock);
spin_unlock_irqrestore(&priv->rxlock, flags);
#endif

return IRQ_HANDLED;
Expand Down Expand Up @@ -1490,13 +1496,6 @@ int gfar_clean_rx_ring(struct net_device *dev, int rx_work_limit)
/* 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;
}

Expand All @@ -1516,7 +1515,7 @@ static int gfar_poll(struct net_device *dev, int *budget)
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 */
Expand All @@ -1533,7 +1532,8 @@ static int gfar_poll(struct net_device *dev, int *budget)
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

Expand Down Expand Up @@ -1629,7 +1629,7 @@ static void adjust_link(struct net_device *dev)
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(&regs->maccfg2);
u32 ecntrl = gfar_read(&regs->ecntrl);
Expand Down Expand Up @@ -1694,7 +1694,7 @@ static void adjust_link(struct net_device *dev)
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
Expand Down
67 changes: 46 additions & 21 deletions drivers/net/gianfar.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -656,43 +656,62 @@ struct gfar {
* 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;
Expand All @@ -702,20 +721,26 @@ struct gfar_private {
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;
int oldduplex;
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)
Expand Down
20 changes: 14 additions & 6 deletions drivers/net/gianfar_ethtool.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,10 +455,14 @@ static int gfar_sringparam(struct net_device *dev, struct ethtool_ringparam *rva

/* 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);
Expand Down Expand Up @@ -488,10 +492,14 @@ static int gfar_set_rx_csum(struct net_device *dev, uint32_t data)

/* 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);
Expand Down Expand Up @@ -523,7 +531,7 @@ static int gfar_set_tx_csum(struct net_device *dev, uint32_t data)
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)
Expand All @@ -532,7 +540,7 @@ static int gfar_set_tx_csum(struct net_device *dev, uint32_t data)
dev->features &= ~NETIF_F_IP_CSUM;

gfar_start(dev);
spin_unlock_irqrestore(&priv->lock, flags);
spin_unlock_irqrestore(&priv->txlock, flags);

return 0;
}
Expand Down
Loading

0 comments on commit fef6108

Please sign in to comment.