1 From 4439d933884ee3c7e320b8d33bd2e268dd5b6fa5 Mon Sep 17 00:00:00 2001
2 From: Daniel Hellstrom <daniel@gaisler.com>
3 Date: Wed, 1 Dec 2010 11:40:19 +0100
4 Subject: [PATCH] GRETH: resolve SMP issues and other problems
7 1. POLL should not enable IRQ when work is not completed
8 2. No locking between TX descriptor cleaning and XMIT descriptor handling
9 3. No locking between RX POLL and XMIT modifying control register
10 4. Since TX cleaning (called from POLL) is running in parallel with XMIT
11 unnecessary locking is needed.
12 5. IRQ handler looks at RX frame status solely, this is wrong when IRQ is
13 temporarily disabled (in POLL), and when IRQ is shared.
14 6. IRQ handler clears IRQ status, which is unnecessary
15 7. TX queue was stopped in preventing cause when not MAX_SKB_FRAGS+1 descriptors
16 were available after a SKB been scheduled by XMIT. Instead the TX queue is
17 stopped first when not enough descriptors are available upon entering XMIT.
19 It was hard to split up this patch in smaller pieces since all are tied
22 Note the RX flag used in the interrupt handler does not signal that interrtupt
23 was asserted, but that a frame was received. Same goes for TX. Also, IRQ is not
24 asserted when the RX flag is set before enabling IRQ enable until a new frame is
25 received. So extra care must be taken to avoid enabling IRQ and all descriptors
26 are already used, hence dead lock will upon us. See new POLL implementation that
27 enableds IRQ then look at the RX flag to determine if one or more IRQs may have
28 been missed. TX/RX flags are cleared before handling previously enabled
29 descriptors, this ensures that the RX/TX flags are valid when determining if IRQ
30 should be turned on again.
32 By moving TX cleaning from POLL to XMIT in the standard case, removes some
33 locking trouble. Enabling TX cleaning from poll only when not enough TX
34 descriptors are available is safe because the TX queue is at the same time
35 stopped, thus XMIT will not be called. The TX queue is woken up again when
36 enough descriptrs are available.
38 TX Frames are always enabled with IRQ, however the TX IRQ Enable flag will not
39 be enabled until XMIT must wait for free descriptors.
41 Locking RX and XMIT parts of the driver from each other is needed because the
42 RX/TX enable bits share the same register.
44 Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
46 drivers/net/greth.c | 158 ++++++++++++++++++++++++++++++---------------------
47 1 files changed, 93 insertions(+), 65 deletions(-)
49 --- a/drivers/net/greth.c
50 +++ b/drivers/net/greth.c
53 * Aeroflex Gaisler GRETH 10/100/1G Ethernet MAC.
55 - * 2005-2009 (c) Aeroflex Gaisler AB
56 + * 2005-2010 (c) Aeroflex Gaisler AB
58 * This driver supports GRETH 10/100 and GRETH 10/100/1G Ethernet MACs
59 * available in the GRLIB VHDL IP core library.
60 @@ -401,12 +401,20 @@ greth_start_xmit(struct sk_buff *skb, st
61 struct greth_private *greth = netdev_priv(dev);
63 int err = NETDEV_TX_OK;
64 - u32 status, dma_addr;
65 + u32 status, dma_addr, ctrl;
66 + unsigned long flags;
68 - bdp = greth->tx_bd_base + greth->tx_next;
70 + greth_clean_tx(greth->netdev);
72 if (unlikely(greth->tx_free <= 0)) {
73 + spin_lock_irqsave(&greth->devlock, flags); /*save from poll/irq*/
74 + ctrl = GRETH_REGLOAD(greth->regs->control);
75 + /* Enable TX IRQ only if not already in poll() routine */
76 + if ( ctrl & GRETH_RXI )
77 + GRETH_REGSAVE(greth->regs->control, ctrl | GRETH_TXI);
78 netif_stop_queue(dev);
79 + spin_unlock_irqrestore(&greth->devlock, flags);
80 return NETDEV_TX_BUSY;
83 @@ -419,13 +427,14 @@ greth_start_xmit(struct sk_buff *skb, st
87 + bdp = greth->tx_bd_base + greth->tx_next;
88 dma_addr = greth_read_bd(&bdp->addr);
90 memcpy((unsigned char *) phys_to_virt(dma_addr), skb->data, skb->len);
92 dma_sync_single_for_device(greth->dev, dma_addr, skb->len, DMA_TO_DEVICE);
94 - status = GRETH_BD_EN | (skb->len & GRETH_BD_LEN);
95 + status = GRETH_BD_EN | GRETH_BD_IE | (skb->len & GRETH_BD_LEN);
97 /* Wrap around descriptor ring */
98 if (greth->tx_next == GRETH_TXBD_NUM_MASK) {
99 @@ -435,22 +444,11 @@ greth_start_xmit(struct sk_buff *skb, st
100 greth->tx_next = NEXT_TX(greth->tx_next);
103 - /* No more descriptors */
104 - if (unlikely(greth->tx_free == 0)) {
106 - /* Free transmitted descriptors */
107 - greth_clean_tx(dev);
109 - /* If nothing was cleaned, stop queue & wait for irq */
110 - if (unlikely(greth->tx_free == 0)) {
111 - status |= GRETH_BD_IE;
112 - netif_stop_queue(dev);
116 /* Write descriptor control word and enable transmission */
117 greth_write_bd(&bdp->stat, status);
118 + spin_lock_irqsave(&greth->devlock, flags); /*save from poll/irq*/
119 greth_enable_tx(greth);
120 + spin_unlock_irqrestore(&greth->devlock, flags);
124 @@ -463,13 +461,24 @@ greth_start_xmit_gbit(struct sk_buff *sk
126 struct greth_private *greth = netdev_priv(dev);
127 struct greth_bd *bdp;
128 - u32 status = 0, dma_addr;
129 + u32 status = 0, dma_addr, ctrl;
130 int curr_tx, nr_frags, i, err = NETDEV_TX_OK;
131 + unsigned long flags;
133 nr_frags = skb_shinfo(skb)->nr_frags;
135 + /* Clean TX Ring */
136 + greth_clean_tx_gbit(dev);
138 if (greth->tx_free < nr_frags + 1) {
139 + spin_lock_irqsave(&greth->devlock, flags); /*save from poll/irq*/
140 + ctrl = GRETH_REGLOAD(greth->regs->control);
141 + /* Enable TX IRQ only if not already in poll() routine */
142 + if ( ctrl & GRETH_RXI ) {
143 + GRETH_REGSAVE(greth->regs->control, ctrl | GRETH_TXI);
145 netif_stop_queue(dev);
146 + spin_unlock_irqrestore(&greth->devlock, flags);
147 err = NETDEV_TX_BUSY;
150 @@ -522,14 +531,8 @@ greth_start_xmit_gbit(struct sk_buff *sk
151 /* More fragments left */
152 if (i < nr_frags - 1)
153 status |= GRETH_TXBD_MORE;
155 - /* ... last fragment, check if out of descriptors */
156 - else if (greth->tx_free - nr_frags - 1 < (MAX_SKB_FRAGS + 1)) {
158 - /* Enable interrupts and stop queue */
159 - status |= GRETH_BD_IE;
160 - netif_stop_queue(dev);
163 + status |= GRETH_BD_IE; /* enable IRQ on last fragment */
165 greth_write_bd(&bdp->stat, status);
167 @@ -557,7 +560,9 @@ greth_start_xmit_gbit(struct sk_buff *sk
171 + spin_lock_irqsave(&greth->devlock, flags); /*save from poll/irq*/
172 greth_enable_tx(greth);
173 + spin_unlock_irqrestore(&greth->devlock, flags);
177 @@ -579,12 +584,11 @@ out:
182 static irqreturn_t greth_interrupt(int irq, void *dev_id)
184 struct net_device *dev = dev_id;
185 struct greth_private *greth;
188 irqreturn_t retval = IRQ_NONE;
190 greth = netdev_priv(dev);
191 @@ -594,13 +598,14 @@ static irqreturn_t greth_interrupt(int i
192 /* Get the interrupt events that caused us to be here. */
193 status = GRETH_REGLOAD(greth->regs->status);
195 - /* Handle rx and tx interrupts through poll */
196 - if (status & (GRETH_INT_RE | GRETH_INT_RX | GRETH_INT_TE | GRETH_INT_TX)) {
197 + /* Must see if interrupts are enabled also, INT_TX|INT_RX flags may be set regardless
198 + * of whether IRQ is enabled or not. Especially important when shared IRQ.
200 + ctrl = GRETH_REGLOAD(greth->regs->control);
202 - /* Clear interrupt status */
203 - GRETH_REGSAVE(greth->regs->status,
204 - status & (GRETH_INT_RE | GRETH_INT_RX |
205 - GRETH_INT_TE | GRETH_INT_TX));
206 + /* Handle rx and tx interrupts through poll */
207 + if (((status & (GRETH_INT_RE | GRETH_INT_RX)) && (ctrl & GRETH_RXI)) ||
208 + ((status & (GRETH_INT_TE | GRETH_INT_TX)) && (ctrl & GRETH_TXI))) {
210 retval = IRQ_HANDLED;
212 @@ -625,6 +630,8 @@ static void greth_clean_tx(struct net_de
215 bdp = greth->tx_bd_base + greth->tx_last;
216 + GRETH_REGSAVE(greth->regs->status, GRETH_INT_TE | GRETH_INT_TX);
218 stat = greth_read_bd(&bdp->stat);
220 if (unlikely(stat & GRETH_BD_EN))
221 @@ -685,7 +692,10 @@ static void greth_clean_tx_gbit(struct n
223 /* We only clean fully completed SKBs */
224 bdp_last_frag = greth->tx_bd_base + SKIP_TX(greth->tx_last, nr_frags);
225 - stat = bdp_last_frag->stat;
227 + GRETH_REGSAVE(greth->regs->status, GRETH_INT_TE | GRETH_INT_TX);
229 + stat = greth_read_bd(&bdp_last_frag->stat);
231 if (stat & GRETH_BD_EN)
233 @@ -717,23 +727,12 @@ static void greth_clean_tx_gbit(struct n
234 greth->tx_free += nr_frags+1;
237 - if (greth->tx_free > (MAX_SKB_FRAGS + 1)) {
239 + if (netif_queue_stopped(dev) && (greth->tx_free > (MAX_SKB_FRAGS + 1))) {
240 netif_wake_queue(dev);
244 -static int greth_pending_packets(struct greth_private *greth)
246 - struct greth_bd *bdp;
248 - bdp = greth->rx_bd_base + greth->rx_cur;
249 - status = greth_read_bd(&bdp->stat);
250 - if (status & GRETH_BD_EN)
256 static int greth_rx(struct net_device *dev, int limit)
258 struct greth_private *greth;
259 @@ -742,20 +741,24 @@ static int greth_rx(struct net_device *d
262 u32 status, dma_addr;
263 + unsigned long flags;
265 greth = netdev_priv(dev);
267 for (count = 0; count < limit; ++count) {
269 bdp = greth->rx_bd_base + greth->rx_cur;
270 + GRETH_REGSAVE(greth->regs->status, GRETH_INT_RE | GRETH_INT_RX);
272 status = greth_read_bd(&bdp->stat);
273 - dma_addr = greth_read_bd(&bdp->addr);
276 if (unlikely(status & GRETH_BD_EN)) {
280 + dma_addr = greth_read_bd(&bdp->addr);
283 /* Check status for errors. */
284 if (unlikely(status & GRETH_RXBD_STATUS)) {
285 if (status & GRETH_RXBD_ERR_FT) {
286 @@ -817,7 +820,9 @@ static int greth_rx(struct net_device *d
288 dma_sync_single_for_device(greth->dev, dma_addr, MAX_FRAME_SIZE, DMA_FROM_DEVICE);
290 + spin_lock_irqsave(&greth->devlock, flags); /* save from XMIT */
291 greth_enable_rx(greth);
292 + spin_unlock_irqrestore(&greth->devlock, flags);
294 greth->rx_cur = NEXT_RX(greth->rx_cur);
296 @@ -851,6 +856,7 @@ static int greth_rx_gbit(struct net_devi
299 u32 status, dma_addr;
300 + unsigned long flags;
302 greth = netdev_priv(dev);
304 @@ -858,6 +864,8 @@ static int greth_rx_gbit(struct net_devi
306 bdp = greth->rx_bd_base + greth->rx_cur;
307 skb = greth->rx_skbuff[greth->rx_cur];
308 + GRETH_REGSAVE(greth->regs->status, GRETH_INT_RE | GRETH_INT_RX);
310 status = greth_read_bd(&bdp->stat);
313 @@ -940,7 +948,9 @@ static int greth_rx_gbit(struct net_devi
316 greth_write_bd(&bdp->stat, status);
317 + spin_lock_irqsave(&greth->devlock, flags);
318 greth_enable_rx(greth);
319 + spin_unlock_irqrestore(&greth->devlock, flags);
320 greth->rx_cur = NEXT_RX(greth->rx_cur);
323 @@ -952,15 +962,19 @@ static int greth_poll(struct napi_struct
325 struct greth_private *greth;
327 + unsigned long flags;
329 greth = container_of(napi, struct greth_private, napi);
331 - if (greth->gbit_mac) {
332 - greth_clean_tx_gbit(greth->netdev);
334 - greth_clean_tx(greth->netdev);
336 + if ( netif_queue_stopped(greth->netdev) ) {
337 + if (greth->gbit_mac) {
338 + greth_clean_tx_gbit(greth->netdev);
340 + greth_clean_tx(greth->netdev);
345 if (greth->gbit_mac) {
346 work_done += greth_rx_gbit(greth->netdev, budget - work_done);
348 @@ -969,15 +983,29 @@ restart_poll:
350 if (work_done < budget) {
352 - napi_complete(napi);
353 + spin_lock_irqsave(&greth->devlock, flags);
355 - if (greth_pending_packets(greth)) {
356 - napi_reschedule(napi);
358 + ctrl = GRETH_REGLOAD(greth->regs->control);
359 + if (netif_queue_stopped(greth->netdev)) {
360 + GRETH_REGSAVE(greth->regs->control,
361 + ctrl | GRETH_TXI | GRETH_RXI);
362 + mask = GRETH_INT_RX | GRETH_INT_RE |
363 + GRETH_INT_TX | GRETH_INT_TE;
365 + GRETH_REGSAVE(greth->regs->control, ctrl | GRETH_RXI);
366 + mask = GRETH_INT_RX | GRETH_INT_RE;
369 + if (GRETH_REGLOAD(greth->regs->status) & mask) {
370 + GRETH_REGSAVE(greth->regs->control, ctrl);
371 + spin_unlock_irqrestore(&greth->devlock, flags);
372 + goto restart_txrx_poll;
374 + __napi_complete(napi);
375 + spin_unlock_irqrestore(&greth->devlock, flags);
379 - greth_enable_irqs(greth);
383 @@ -1172,11 +1200,11 @@ static const struct ethtool_ops greth_et
386 static struct net_device_ops greth_netdev_ops = {
387 - .ndo_open = greth_open,
388 - .ndo_stop = greth_close,
389 - .ndo_start_xmit = greth_start_xmit,
390 - .ndo_set_mac_address = greth_set_mac_add,
391 - .ndo_validate_addr = eth_validate_addr,
392 + .ndo_open = greth_open,
393 + .ndo_stop = greth_close,
394 + .ndo_start_xmit = greth_start_xmit,
395 + .ndo_set_mac_address = greth_set_mac_add,
396 + .ndo_validate_addr = eth_validate_addr,
399 static inline int wait_for_mdio(struct greth_private *greth)