* Re: Regression in net-2.6.24?
From: David Miller @ 2007-10-12 1:25 UTC (permalink / raw)
To: shemminger; +Cc: takano, netdev, ilpo.jarvinen, mchan
In-Reply-To: <20071011182259.3715182d@freepuppy.rosehill>
From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Thu, 11 Oct 2007 18:22:59 -0700
> Please use the version I just sent, it saves another non-cached read.
No, rather, please send me fixed relative to that.
I'm trying to get the net-2.6.24 merge out the door.
^ permalink raw reply
* Re: [PATCH 1/6] sky2: status polling loop
From: David Miller @ 2007-10-12 1:23 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20071012012124.965282742@linux-foundation.org>
From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Thu, 11 Oct 2007 18:19:52 -0700
> Handle the corner case where budget is exhausted correctly.
>
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
Doesn't apply, I checked in my fixed version of your original patch
already.
You had to have some idea I would apply that fix, so this patch series
seems a bit premature since it's almost guarenteed to not apply any
more.
^ permalink raw reply
* Re: Regression in net-2.6.24?
From: Stephen Hemminger @ 2007-10-12 1:22 UTC (permalink / raw)
To: David Miller; +Cc: takano, netdev, ilpo.jarvinen, mchan
In-Reply-To: <20071011.181449.130846976.davem@davemloft.net>
On Thu, 11 Oct 2007 18:14:49 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
>
> Here is what I'm checking into net-2.6 for now:
>
> commit 6f535763165331bb91277d7519b507fed22034e5
> Author: David S. Miller <davem@sunset.davemloft.net>
> Date: Thu Oct 11 18:08:29 2007 -0700
>
> [NET]: Fix NAPI completion handling in some drivers.
>
> In order for the list handling in net_rx_action() to be
> correct, drivers must follow certain rules as stated by
> this comment in net_rx_action():
>
> /* Drivers must not modify the NAPI state if they
> * consume the entire weight. In such cases this code
> * still "owns" the NAPI instance and therefore can
> * move the instance around on the list at-will.
> */
>
> A few drivers do not do this because they mix the budget checks
> with reading hardware state, resulting in crashes like the one
> reported by takano@axe-inc.co.jp.
>
> BNX2 and TG3 are taken care of here, SKY2 fix is from Stephen
> Hemminger.
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
Please use the version I just sent, it saves another non-cached read.
^ permalink raw reply
* Re: Question on TSO maximum segment sizes.
From: John Heffner @ 2007-10-12 1:22 UTC (permalink / raw)
To: Ben Greear; +Cc: netdev
In-Reply-To: <470EBD2C.8020704@candelatech.com>
Ben Greear wrote:
> I just tried turning off my explicit SO_SNDBUF/SO_RCVBUG settings in my
> app,
> and the connection ran very poorly through a link with even a small
> bit of latency (~2-4ms I believe).
I often run at full gigabit or faster with latencies of 100+ ms. Can
you give a bit more detail?
-John
^ permalink raw reply
* [PATCH 2/6] sky2: ethtool register reserved area blackout
From: Stephen Hemminger @ 2007-10-12 1:19 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20071012011951.575936605@linux-foundation.org>
[-- Attachment #1: sky2-ethdump.patch --]
[-- Type: text/plain, Size: 2760 bytes --]
Make sure and not dump reserved areas of device space.
Touching some of these causes machine check exceptions on boards
like D-Link DGE-550SX.
Coding note, used a complex switch statement rather than bitmap
because it is easier to relate the block values to the documentation
rather than looking at a encoded bitmask.
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
--- a/drivers/net/sky2.c 2007-10-11 18:12:50.000000000 -0700
+++ b/drivers/net/sky2.c 2007-10-11 18:12:56.000000000 -0700
@@ -3569,20 +3569,64 @@ static void sky2_get_regs(struct net_dev
{
const struct sky2_port *sky2 = netdev_priv(dev);
const void __iomem *io = sky2->hw->regs;
+ unsigned int b;
regs->version = 1;
- memset(p, 0, regs->len);
- memcpy_fromio(p, io, B3_RAM_ADDR);
-
- /* skip diagnostic ram region */
- memcpy_fromio(p + B3_RI_WTO_R1, io + B3_RI_WTO_R1, 0x2000 - B3_RI_WTO_R1);
+ for (b = 0; b < 128; b++) {
+ /* This complicated switch statement is to make sure and
+ * only access regions that are unreserved.
+ * Some blocks are only valid on dual port cards.
+ * and block 3 has some special diagnostic registers that
+ * are poison.
+ */
+ switch (b) {
+ case 3:
+ /* skip diagnostic ram region */
+ memcpy_fromio(p + 0x10, io + 0x10, 128 - 0x10);
+ break;
- /* copy GMAC registers */
- memcpy_fromio(p + BASE_GMAC_1, io + BASE_GMAC_1, 0x1000);
- if (sky2->hw->ports > 1)
- memcpy_fromio(p + BASE_GMAC_2, io + BASE_GMAC_2, 0x1000);
+ /* dual port cards only */
+ case 5: /* Tx Arbiter 2 */
+ case 9: /* RX2 */
+ case 14 ... 15: /* TX2 */
+ case 17: case 19: /* Ram Buffer 2 */
+ case 22 ... 23: /* Tx Ram Buffer 2 */
+ case 25: /* Rx MAC Fifo 1 */
+ case 27: /* Tx MAC Fifo 2 */
+ case 31: /* GPHY 2 */
+ case 40 ... 47: /* Pattern Ram 2 */
+ case 52: case 54: /* TCP Segmentation 2 */
+ case 112 ... 116: /* GMAC 2 */
+ if (sky2->hw->ports == 1)
+ goto reserved;
+ /* fall through */
+ case 0: /* Control */
+ case 2: /* Mac address */
+ case 4: /* Tx Arbiter 1 */
+ case 7: /* PCI express reg */
+ case 8: /* RX1 */
+ case 12 ... 13: /* TX1 */
+ case 16: case 18:/* Rx Ram Buffer 1 */
+ case 20 ... 21: /* Tx Ram Buffer 1 */
+ case 24: /* Rx MAC Fifo 1 */
+ case 26: /* Tx MAC Fifo 1 */
+ case 28 ... 29: /* Descriptor and status unit */
+ case 30: /* GPHY 1*/
+ case 32 ... 39: /* Pattern Ram 1 */
+ case 48: case 50: /* TCP Segmentation 1 */
+ case 56 ... 60: /* PCI space */
+ case 80 ... 84: /* GMAC 1 */
+ memcpy_fromio(p, io, 128);
+ break;
+ default:
+reserved:
+ memset(p, 0, 128);
+ }
+ p += 128;
+ io += 128;
+ }
}
/* In order to do Jumbo packets on these chips, need to turn off the
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* [PATCH 5/6] sky2: use netdevice stats struct
From: Stephen Hemminger @ 2007-10-12 1:19 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20071012011951.575936605@linux-foundation.org>
[-- Attachment #1: sky2-net-stats.patch --]
[-- Type: text/plain, Size: 3607 bytes --]
Use builtin statistics structure from net device.
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
--- a/drivers/net/sky2.c 2007-10-11 18:13:01.000000000 -0700
+++ b/drivers/net/sky2.c 2007-10-11 18:13:02.000000000 -0700
@@ -1635,8 +1635,8 @@ static void sky2_tx_complete(struct sky2
printk(KERN_DEBUG "%s: tx done %u\n",
dev->name, idx);
- sky2->net_stats.tx_packets++;
- sky2->net_stats.tx_bytes += re->skb->len;
+ dev->stats.tx_packets++;
+ dev->stats.tx_bytes += re->skb->len;
dev_kfree_skb_any(re->skb);
sky2->tx_next = RING_NEXT(idx, TX_RING_SIZE);
@@ -2204,16 +2204,16 @@ resubmit:
len_error:
/* Truncation of overlength packets
causes PHY length to not match MAC length */
- ++sky2->net_stats.rx_length_errors;
+ ++dev->stats.rx_length_errors;
if (netif_msg_rx_err(sky2) && net_ratelimit())
pr_info(PFX "%s: rx length error: status %#x length %d\n",
dev->name, status, length);
goto resubmit;
error:
- ++sky2->net_stats.rx_errors;
+ ++dev->stats.rx_errors;
if (status & GMR_FS_RX_FF_OV) {
- sky2->net_stats.rx_over_errors++;
+ dev->stats.rx_over_errors++;
goto resubmit;
}
@@ -2222,11 +2222,11 @@ error:
dev->name, status, length);
if (status & (GMR_FS_LONG_ERR | GMR_FS_UN_SIZE))
- sky2->net_stats.rx_length_errors++;
+ dev->stats.rx_length_errors++;
if (status & GMR_FS_FRAGMENT)
- sky2->net_stats.rx_frame_errors++;
+ dev->stats.rx_frame_errors++;
if (status & GMR_FS_CRC_ERR)
- sky2->net_stats.rx_crc_errors++;
+ dev->stats.rx_crc_errors++;
goto resubmit;
}
@@ -2271,7 +2271,7 @@ static int sky2_status_intr(struct sky2_
++rx[port];
skb = sky2_receive(dev, length, status);
if (unlikely(!skb)) {
- sky2->net_stats.rx_dropped++;
+ dev->stats.rx_dropped++;
break;
}
@@ -2286,8 +2286,8 @@ static int sky2_status_intr(struct sky2_
}
skb->protocol = eth_type_trans(skb, dev);
- sky2->net_stats.rx_packets++;
- sky2->net_stats.rx_bytes += skb->len;
+ dev->stats.rx_packets++;
+ dev->stats.rx_bytes += skb->len;
dev->last_rx = jiffies;
#ifdef SKY2_VLAN_TAG_USED
@@ -2478,12 +2478,12 @@ static void sky2_mac_intr(struct sky2_hw
gma_read16(hw, port, GM_TX_IRQ_SRC);
if (status & GM_IS_RX_FF_OR) {
- ++sky2->net_stats.rx_fifo_errors;
+ ++dev->stats.rx_fifo_errors;
sky2_write8(hw, SK_REG(port, RX_GMF_CTRL_T), GMF_CLI_RX_FO);
}
if (status & GM_IS_TX_FF_UR) {
- ++sky2->net_stats.tx_fifo_errors;
+ ++dev->stats.tx_fifo_errors;
sky2_write8(hw, SK_REG(port, TX_GMF_CTRL_T), GMF_CLI_TX_FU);
}
}
@@ -3222,12 +3222,6 @@ static void sky2_get_strings(struct net_
}
}
-static struct net_device_stats *sky2_get_stats(struct net_device *dev)
-{
- struct sky2_port *sky2 = netdev_priv(dev);
- return &sky2->net_stats;
-}
-
static int sky2_set_mac_address(struct net_device *dev, void *p)
{
struct sky2_port *sky2 = netdev_priv(dev);
@@ -3977,7 +3971,6 @@ static __devinit struct net_device *sky2
dev->stop = sky2_down;
dev->do_ioctl = sky2_ioctl;
dev->hard_start_xmit = sky2_xmit_frame;
- dev->get_stats = sky2_get_stats;
dev->set_multicast_list = sky2_set_multicast;
dev->set_mac_address = sky2_set_mac_address;
dev->change_mtu = sky2_change_mtu;
--- a/drivers/net/sky2.h 2007-10-11 18:11:44.000000000 -0700
+++ b/drivers/net/sky2.h 2007-10-11 18:13:02.000000000 -0700
@@ -2031,8 +2031,6 @@ struct sky2_port {
#ifdef CONFIG_SKY2_DEBUG
struct dentry *debugfs;
#endif
- struct net_device_stats net_stats;
-
};
struct sky2_hw {
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* [PATCH 6/6] sky2: version 1.19
From: Stephen Hemminger @ 2007-10-12 1:19 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20071012011951.575936605@linux-foundation.org>
[-- Attachment #1: sky2-1.19 --]
[-- Type: text/plain, Size: 451 bytes --]
Update version to keep track of new changes.
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
--- a/drivers/net/sky2.c 2007-10-11 18:13:02.000000000 -0700
+++ b/drivers/net/sky2.c 2007-10-11 18:14:25.000000000 -0700
@@ -52,7 +52,7 @@
#include "sky2.h"
#define DRV_NAME "sky2"
-#define DRV_VERSION "1.18"
+#define DRV_VERSION "1.19"
#define PFX DRV_NAME " "
/*
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* [PATCH 0/6] sky2 patches for net-2.6
From: Stephen Hemminger @ 2007-10-12 1:19 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Includes:
* fix for new NAPI status loop
* use internal net_stats
* fixes for fiber PHY power
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* [PATCH 3/6] sky2: fix power settings on Yukon XL
From: Stephen Hemminger @ 2007-10-12 1:19 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20071012011951.575936605@linux-foundation.org>
[-- Attachment #1: sky2-phy-power.patch --]
[-- Type: text/plain, Size: 1178 bytes --]
Make sure PCI register for PHY power gets set correctly.
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
--- a/drivers/net/sky2.c 2007-10-11 18:12:56.000000000 -0700
+++ b/drivers/net/sky2.c 2007-10-11 18:13:00.000000000 -0700
@@ -606,20 +606,19 @@ static void sky2_phy_power(struct sky2_h
{
struct pci_dev *pdev = hw->pdev;
u32 reg1;
- static const u32 phy_power[]
- = { PCI_Y2_PHY1_POWD, PCI_Y2_PHY2_POWD };
-
- /* looks like this XL is back asswards .. */
- if (hw->chip_id == CHIP_ID_YUKON_XL && hw->chip_rev > 1)
- onoff = !onoff;
+ static const u32 phy_power[] = { PCI_Y2_PHY1_POWD, PCI_Y2_PHY2_POWD };
+ static const u32 coma_mode[] = { PCI_Y2_PHY1_COMA, PCI_Y2_PHY2_COMA };
pci_read_config_dword(pdev, PCI_DEV_REG1, ®1);
+ /* Turn on/off phy power saving */
if (onoff)
- /* Turn off phy power saving */
reg1 &= ~phy_power[port];
else
reg1 |= phy_power[port];
+ if (onoff && hw->chip_id == CHIP_ID_YUKON_XL && hw->chip_rev > 1)
+ reg1 |= coma_mode[port];
+
pci_write_config_dword(pdev, PCI_DEV_REG1, reg1);
pci_read_config_dword(pdev, PCI_DEV_REG1, ®1);
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* [PATCH 4/6] sky2: fiber advertise bits initialization (trivial)
From: Stephen Hemminger @ 2007-10-12 1:19 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20071012011951.575936605@linux-foundation.org>
[-- Attachment #1: fiber-mode-reorder.patch --]
[-- Type: text/plain, Size: 717 bytes --]
Put initialization in sequential order (same as other constants).
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
--- a/drivers/net/sky2.c 2007-10-11 18:13:00.000000000 -0700
+++ b/drivers/net/sky2.c 2007-10-11 18:13:01.000000000 -0700
@@ -296,10 +296,10 @@ static const u16 copper_fc_adv[] = {
/* flow control to advertise bits when using 1000BaseX */
static const u16 fiber_fc_adv[] = {
- [FC_BOTH] = PHY_M_P_BOTH_MD_X,
+ [FC_NONE] = PHY_M_P_NO_PAUSE_X,
[FC_TX] = PHY_M_P_ASYM_MD_X,
[FC_RX] = PHY_M_P_SYM_MD_X,
- [FC_NONE] = PHY_M_P_NO_PAUSE_X,
+ [FC_BOTH] = PHY_M_P_BOTH_MD_X,
};
/* flow control to GMA disable bits */
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* [PATCH 1/6] sky2: status polling loop
From: Stephen Hemminger @ 2007-10-12 1:19 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20071012011951.575936605@linux-foundation.org>
[-- Attachment #1: sky2-status-loop.patch --]
[-- Type: text/plain, Size: 2477 bytes --]
Handle the corner case where budget is exhausted correctly.
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
--- a/drivers/net/sky2.c 2007-10-11 18:11:44.000000000 -0700
+++ b/drivers/net/sky2.c 2007-10-11 18:12:50.000000000 -0700
@@ -2245,15 +2245,13 @@ static inline void sky2_tx_done(struct n
}
/* Process status response ring */
-static int sky2_status_intr(struct sky2_hw *hw, int to_do)
+static int sky2_status_intr(struct sky2_hw *hw, int to_do, u16 idx)
{
int work_done = 0;
unsigned rx[2] = { 0, 0 };
- u16 hwidx = sky2_read16(hw, STAT_PUT_IDX);
rmb();
-
- while (hw->st_idx != hwidx) {
+ do {
struct sky2_port *sky2;
struct sky2_status_le *le = hw->st_le + hw->st_idx;
unsigned port = le->css & CSS_LINK_BIT;
@@ -2364,7 +2362,7 @@ static int sky2_status_intr(struct sky2_
printk(KERN_WARNING PFX
"unknown status opcode 0x%x\n", le->opcode);
}
- }
+ } while (hw->st_idx != idx);
/* Fully processed status ring so clear irq */
sky2_write32(hw, STAT_CTRL, SC_STAT_CLR_IRQ);
@@ -2606,7 +2604,8 @@ static int sky2_poll(struct napi_struct
{
struct sky2_hw *hw = container_of(napi, struct sky2_hw, napi);
u32 status = sky2_read32(hw, B0_Y2_SP_EISR);
- int work_done;
+ int work_done = 0;
+ u16 idx;
if (unlikely(status & Y2_IS_ERROR))
sky2_err_intr(hw, status);
@@ -2617,21 +2616,24 @@ static int sky2_poll(struct napi_struct
if (status & Y2_IS_IRQ_PHY2)
sky2_phy_intr(hw, 1);
- work_done = sky2_status_intr(hw, work_limit);
+ while ((idx = sky2_read16(hw, STAT_PUT_IDX)) != hw->st_idx) {
+ work_done += sky2_status_intr(hw, work_limit - work_done, idx);
- /* More work? */
- if (hw->st_idx == sky2_read16(hw, STAT_PUT_IDX)) {
- /* Bug/Errata workaround?
- * Need to kick the TX irq moderation timer.
- */
- if (sky2_read8(hw, STAT_TX_TIMER_CTRL) == TIM_START) {
- sky2_write8(hw, STAT_TX_TIMER_CTRL, TIM_STOP);
- sky2_write8(hw, STAT_TX_TIMER_CTRL, TIM_START);
- }
+ if (work_done >= work_limit)
+ goto done;
+ }
- napi_complete(napi);
- sky2_read32(hw, B0_Y2_SP_LISR);
+ /* Bug/Errata workaround?
+ * Need to kick the TX irq moderation timer.
+ */
+ if (sky2_read8(hw, STAT_TX_TIMER_CTRL) == TIM_START) {
+ sky2_write8(hw, STAT_TX_TIMER_CTRL, TIM_STOP);
+ sky2_write8(hw, STAT_TX_TIMER_CTRL, TIM_START);
}
+ napi_complete(napi);
+ sky2_read32(hw, B0_Y2_SP_LISR);
+done:
+
return work_done;
}
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* Re: Regression in net-2.6.24?
From: David Miller @ 2007-10-12 1:14 UTC (permalink / raw)
To: shemminger; +Cc: takano, netdev, ilpo.jarvinen, mchan
In-Reply-To: <20071011.171743.118962219.davem@davemloft.net>
Here is what I'm checking into net-2.6 for now:
commit 6f535763165331bb91277d7519b507fed22034e5
Author: David S. Miller <davem@sunset.davemloft.net>
Date: Thu Oct 11 18:08:29 2007 -0700
[NET]: Fix NAPI completion handling in some drivers.
In order for the list handling in net_rx_action() to be
correct, drivers must follow certain rules as stated by
this comment in net_rx_action():
/* Drivers must not modify the NAPI state if they
* consume the entire weight. In such cases this code
* still "owns" the NAPI instance and therefore can
* move the instance around on the list at-will.
*/
A few drivers do not do this because they mix the budget checks
with reading hardware state, resulting in crashes like the one
reported by takano@axe-inc.co.jp.
BNX2 and TG3 are taken care of here, SKY2 fix is from Stephen
Hemminger.
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index bbfbdaf..d68acce 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -2633,15 +2633,11 @@ bnx2_has_work(struct bnx2 *bp)
return 0;
}
-static int
-bnx2_poll(struct napi_struct *napi, int budget)
+static int bnx2_poll_work(struct bnx2 *bp, int work_done, int budget)
{
- struct bnx2 *bp = container_of(napi, struct bnx2, napi);
- struct net_device *dev = bp->dev;
struct status_block *sblk = bp->status_blk;
u32 status_attn_bits = sblk->status_attn_bits;
u32 status_attn_bits_ack = sblk->status_attn_bits_ack;
- int work_done = 0;
if ((status_attn_bits & STATUS_ATTN_EVENTS) !=
(status_attn_bits_ack & STATUS_ATTN_EVENTS)) {
@@ -2660,27 +2656,43 @@ bnx2_poll(struct napi_struct *napi, int budget)
bnx2_tx_int(bp);
if (bp->status_blk->status_rx_quick_consumer_index0 != bp->hw_rx_cons)
- work_done = bnx2_rx_int(bp, budget);
+ work_done += bnx2_rx_int(bp, budget - work_done);
- bp->last_status_idx = bp->status_blk->status_idx;
- rmb();
+ return work_done;
+}
+
+static int bnx2_poll(struct napi_struct *napi, int budget)
+{
+ struct bnx2 *bp = container_of(napi, struct bnx2, napi);
+ int work_done = 0;
+
+ while (1) {
+ work_done = bnx2_poll_work(bp, work_done, budget);
- if (!bnx2_has_work(bp)) {
- netif_rx_complete(dev, napi);
- if (likely(bp->flags & USING_MSI_FLAG)) {
+ if (unlikely(work_done >= budget))
+ break;
+
+ if (likely(!bnx2_has_work(bp))) {
+ bp->last_status_idx = bp->status_blk->status_idx;
+ rmb();
+
+ netif_rx_complete(bp->dev, napi);
+ if (likely(bp->flags & USING_MSI_FLAG)) {
+ REG_WR(bp, BNX2_PCICFG_INT_ACK_CMD,
+ BNX2_PCICFG_INT_ACK_CMD_INDEX_VALID |
+ bp->last_status_idx);
+ return 0;
+ }
REG_WR(bp, BNX2_PCICFG_INT_ACK_CMD,
BNX2_PCICFG_INT_ACK_CMD_INDEX_VALID |
+ BNX2_PCICFG_INT_ACK_CMD_MASK_INT |
bp->last_status_idx);
- return 0;
- }
- REG_WR(bp, BNX2_PCICFG_INT_ACK_CMD,
- BNX2_PCICFG_INT_ACK_CMD_INDEX_VALID |
- BNX2_PCICFG_INT_ACK_CMD_MASK_INT |
- bp->last_status_idx);
- REG_WR(bp, BNX2_PCICFG_INT_ACK_CMD,
- BNX2_PCICFG_INT_ACK_CMD_INDEX_VALID |
- bp->last_status_idx);
+ REG_WR(bp, BNX2_PCICFG_INT_ACK_CMD,
+ BNX2_PCICFG_INT_ACK_CMD_INDEX_VALID |
+ bp->last_status_idx);
+ break;
+ }
}
return work_done;
diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index fe0e756..4e569fa 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -2606,7 +2606,7 @@ static int sky2_poll(struct napi_struct *napi, int work_limit)
{
struct sky2_hw *hw = container_of(napi, struct sky2_hw, napi);
u32 status = sky2_read32(hw, B0_Y2_SP_EISR);
- int work_done;
+ int work_done = 0;
if (unlikely(status & Y2_IS_ERROR))
sky2_err_intr(hw, status);
@@ -2617,10 +2617,16 @@ static int sky2_poll(struct napi_struct *napi, int work_limit)
if (status & Y2_IS_IRQ_PHY2)
sky2_phy_intr(hw, 1);
- work_done = sky2_status_intr(hw, work_limit);
+ for(;;) {
+ work_done += sky2_status_intr(hw, work_limit);
+
+ if (work_done >= work_limit)
+ break;
+
+ /* More work? */
+ if (hw->st_idx != sky2_read16(hw, STAT_PUT_IDX))
+ continue;
- /* More work? */
- if (hw->st_idx == sky2_read16(hw, STAT_PUT_IDX)) {
/* Bug/Errata workaround?
* Need to kick the TX irq moderation timer.
*/
@@ -2631,7 +2637,10 @@ static int sky2_poll(struct napi_struct *napi, int work_limit)
napi_complete(napi);
sky2_read32(hw, B0_Y2_SP_LISR);
+ break;
+
}
+
return work_done;
}
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index d2b30fb..a402b5c 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -3555,12 +3555,9 @@ next_pkt_nopost:
return received;
}
-static int tg3_poll(struct napi_struct *napi, int budget)
+static int tg3_poll_work(struct tg3 *tp, int work_done, int budget)
{
- struct tg3 *tp = container_of(napi, struct tg3, napi);
- struct net_device *netdev = tp->dev;
struct tg3_hw_status *sblk = tp->hw_status;
- int work_done = 0;
/* handle link change and other phy events */
if (!(tp->tg3_flags &
@@ -3578,11 +3575,8 @@ static int tg3_poll(struct napi_struct *napi, int budget)
/* run TX completion thread */
if (sblk->idx[0].tx_consumer != tp->tx_cons) {
tg3_tx(tp);
- if (unlikely(tp->tg3_flags & TG3_FLAG_TX_RECOVERY_PENDING)) {
- netif_rx_complete(netdev, napi);
- schedule_work(&tp->reset_task);
+ if (unlikely(tp->tg3_flags & TG3_FLAG_TX_RECOVERY_PENDING))
return 0;
- }
}
/* run RX thread, within the bounds set by NAPI.
@@ -3590,21 +3584,46 @@ static int tg3_poll(struct napi_struct *napi, int budget)
* code synchronizes with tg3->napi.poll()
*/
if (sblk->idx[0].rx_producer != tp->rx_rcb_ptr)
- work_done = tg3_rx(tp, budget);
+ work_done += tg3_rx(tp, budget - work_done);
- if (tp->tg3_flags & TG3_FLAG_TAGGED_STATUS) {
- tp->last_tag = sblk->status_tag;
- rmb();
- } else
- sblk->status &= ~SD_STATUS_UPDATED;
+ return work_done;
+}
- /* if no more work, tell net stack and NIC we're done */
- if (!tg3_has_work(tp)) {
- netif_rx_complete(netdev, napi);
- tg3_restart_ints(tp);
+static int tg3_poll(struct napi_struct *napi, int budget)
+{
+ struct tg3 *tp = container_of(napi, struct tg3, napi);
+ int work_done = 0;
+
+ while (1) {
+ work_done = tg3_poll_work(tp, work_done, budget);
+
+ if (unlikely(tp->tg3_flags & TG3_FLAG_TX_RECOVERY_PENDING))
+ goto tx_recovery;
+
+ if (unlikely(work_done >= budget))
+ break;
+
+ if (likely(!tg3_has_work(tp))) {
+ struct tg3_hw_status *sblk = tp->hw_status;
+
+ if (tp->tg3_flags & TG3_FLAG_TAGGED_STATUS) {
+ tp->last_tag = sblk->status_tag;
+ rmb();
+ } else
+ sblk->status &= ~SD_STATUS_UPDATED;
+
+ netif_rx_complete(tp->dev, napi);
+ tg3_restart_ints(tp);
+ break;
+ }
}
return work_done;
+
+tx_recovery:
+ netif_rx_complete(tp->dev, napi);
+ schedule_work(&tp->reset_task);
+ return 0;
}
static void tg3_irq_quiesce(struct tg3 *tp)
^ permalink raw reply related
* Re: Regression in net-2.6.24?
From: David Miller @ 2007-10-12 1:03 UTC (permalink / raw)
To: shemminger; +Cc: takano, netdev, ilpo.jarvinen, mchan
In-Reply-To: <20071011.180031.41631586.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Thu, 11 Oct 2007 18:00:31 -0700 (PDT)
> From: Stephen Hemminger <shemminger@linux-foundation.org>
> Date: Thu, 11 Oct 2007 17:50:59 -0700
>
> > On Thu, 11 Oct 2007 17:40:26 -0700 (PDT)
> > David Miller <davem@davemloft.net> wrote:
> >
> > > From: Stephen Hemminger <shemminger@linux-foundation.org>
> > > Date: Thu, 11 Oct 2007 17:31:49 -0700
> > >
> > > > You don't need to re-read the status register and process the PHY irq's inside loop.
> > > > Try this:
> > >
> > > Are you sure? What if a PHY interrupt comes in during the loop?
> >
> > The interrupt is level triggered, and will rearm.
>
> Fair enough.
Actually, your change isn't right for another reason.
You missed the necessary budget reducing logic that I used
in the original changes. You need to adjust work_limit
like this:
work_done += sky2_status_intr(hw, work_limit - work_done);
Otherwise if you just pass plain "work_limit", and we do loop, the
driver uses more quota than it really should.
And I've made that above correction to your patch in my tree.
^ permalink raw reply
* Re: Regression in net-2.6.24?
From: David Miller @ 2007-10-12 1:00 UTC (permalink / raw)
To: shemminger; +Cc: takano, netdev, ilpo.jarvinen, mchan
In-Reply-To: <20071011175059.7b35c9a1@freepuppy.rosehill>
From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Thu, 11 Oct 2007 17:50:59 -0700
> On Thu, 11 Oct 2007 17:40:26 -0700 (PDT)
> David Miller <davem@davemloft.net> wrote:
>
> > From: Stephen Hemminger <shemminger@linux-foundation.org>
> > Date: Thu, 11 Oct 2007 17:31:49 -0700
> >
> > > You don't need to re-read the status register and process the PHY irq's inside loop.
> > > Try this:
> >
> > Are you sure? What if a PHY interrupt comes in during the loop?
>
> The interrupt is level triggered, and will rearm.
Fair enough.
^ permalink raw reply
* APE changes cause tg3 regressions...
From: David Miller @ 2007-10-12 0:58 UTC (permalink / raw)
To: netdev; +Cc: mchan, mcarlson
On my onboard SunBlade1500 5703 chips, which lack firmware, the APE
changes introduce bus timeouts while bringing the chip up,
specifically I get crashes in tg3_write_sig_post_reset() such as:
[ 36.066603] eth1: Tigon3 [partno(BCM95703A30U) rev 1002 PHY(5703)] (PCI:33MHz:64-bit) 10/100/1000Base-T Ethernet 00:10:18:04:13:02
[ 36.076040] eth1: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] WireSpeed[1] TSOcap[1]
[ 36.085393] eth1: dma_rwctrl[763f0000] dma_mask[32-bit]
[ 36.095909] PCI: Enabling device: (0000:00:08.0), cmd 3
[ 37.541359] tg3: eth1: No firmware running.
[ 37.588662] ERROR(0): Cheetah error trap taken afsr[0000400000000000] afar[0000000000004210] TL1(0)
[ 37.598106] ERROR(0): TPC[1004a700] TNPC[1004a704] O7[42f488] TSTATE[80009602]
[ 37.607555] ERROR(0): TPC<tg3_write_sig_post_reset+0x68/0x78 [tg3]>
[ 37.617040] ERROR(0): M_SYND(0), E_SYND(0)
[ 37.626422] ERROR(0): Highest priority error (0000400000000000) "Error due to unsupported store"
[ 37.636046] ERROR(0): AFAR E-syndrome [???]
[ 37.645619] ERROR(0): D-cache idx[0] tag[0000000000000000] utag[0000000000000000] stag[0000000000000000]
[ 37.655394] ERROR(0): D-cache data0[0000000000000000] data1[0000000000000000] data2[0000000000000000] data3[0000000000000000]
[ 37.665254] ERROR(0): I-cache idx[0] tag[0000000000000000] utag[0000000000000000] stag[0000000000000000] u[0000000000000000] l[0000000000000000]
[ 37.675194] ERROR(0): I-cache INSN0[0000000000000000] INSN1[0000000000000000] INSN2[0000000000000000] INSN3[0000000000000000]
[ 37.685138] ERROR(0): I-cache INSN4[0000000000000000] INSN5[0000000000000000] INSN6[0000000000000000] INSN7[0000000000000000]
[ 37.695069] ERROR(0): E-cache idx[4200] tag[0000000002000fb1]
[ 37.704920] ERROR(0): E-cache data0[b0102000a6922000] data1[02800057a004c010] data2[9410001192100019] data3[a2042250b0100013]
[ 37.714978] /pci@1f,700000: Safari/JBUS interrupt, UNMAPPED error, interrogating IOMMUs.
My initial suspicion is that TG3_FLG3_ENABLE_APE isn't being set right
on these firmware-less cards, or some APE programming is not being
guarded properly with tests on that feature bit.
Please fix this, thanks!
^ permalink raw reply
* Re: Regression in net-2.6.24?
From: Stephen Hemminger @ 2007-10-12 0:50 UTC (permalink / raw)
To: David Miller; +Cc: takano, netdev, ilpo.jarvinen, mchan
In-Reply-To: <20071011.174026.26976971.davem@davemloft.net>
On Thu, 11 Oct 2007 17:40:26 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
> From: Stephen Hemminger <shemminger@linux-foundation.org>
> Date: Thu, 11 Oct 2007 17:31:49 -0700
>
> > You don't need to re-read the status register and process the PHY irq's inside loop.
> > Try this:
>
> Are you sure? What if a PHY interrupt comes in during the loop?
The interrupt is level triggered, and will rearm.
>
> I'm just preserving the semantics of the driver when ->poll()
> is invoked multiple times per interrupt.
>
> And I think preserving that makes sense while we're purely
> trying to fix this bug, so we don't add some new ones.
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* Re: Regression in net-2.6.24?
From: David Miller @ 2007-10-12 0:40 UTC (permalink / raw)
To: shemminger; +Cc: takano, netdev, ilpo.jarvinen, mchan
In-Reply-To: <20071011173149.5ec23a25@freepuppy.rosehill>
From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Thu, 11 Oct 2007 17:31:49 -0700
> You don't need to re-read the status register and process the PHY irq's inside loop.
> Try this:
Are you sure? What if a PHY interrupt comes in during the loop?
I'm just preserving the semantics of the driver when ->poll()
is invoked multiple times per interrupt.
And I think preserving that makes sense while we're purely
trying to fix this bug, so we don't add some new ones.
^ permalink raw reply
* Re: [PATCH net-2.6 0/7] TCP: small changes/fixes + lost_retrans brokeness fix
From: David Miller @ 2007-10-12 0:37 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: netdev, takano
In-Reply-To: <1192102867479-git-send-email-ilpo.jarvinen@helsinki.fi>
From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Thu, 11 Oct 2007 14:41:00 +0300
> Dave, please apply at will.
Done.
^ permalink raw reply
* Re: [PATCH 7/7] [TCP]: Limit processing lost_retrans loop to work-to-do cases
From: David Miller @ 2007-10-12 0:36 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: netdev, takano
In-Reply-To: <1192102867248-git-send-email-ilpo.jarvinen@helsinki.fi>
From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Thu, 11 Oct 2007 14:41:07 +0300
> This addition of lost_retrans_low to tcp_sock might be
> unnecessary, it's not clear how often lost_retrans worker is
> executed when there wasn't work to do.
>
> Cc: TAKANO Ryousei <takano@axe-inc.co.jp>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
Applied.
^ permalink raw reply
* Re: [PATCH 6/7] [TCP]: Fix lost_retrans loop vs fastpath problems
From: David Miller @ 2007-10-12 0:35 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: netdev, takano
In-Reply-To: <11921028671668-git-send-email-ilpo.jarvinen@helsinki.fi>
From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Thu, 11 Oct 2007 14:41:06 +0300
> Detection implemented with lost_retrans must work also when
> fastpath is taken, yet most of the queue is skipped including
> (very likely) those retransmitted skb's we're interested in.
> This problem appeared when the hints got added, which removed
> a need to always walk over the whole write queue head.
> Therefore decicion for the lost_retrans worker loop entry must
> be separated from the sacktag processing more than it was
> necessary before.
>
> It turns out to be problematic to optimize the worker loop
> very heavily because ack_seqs of skb may have a number of
> discontinuity points. Maybe similar approach as currently is
> implemented could be attempted but that's becoming more and
> more complex because the trend is towards less skb walking
> in sacktag marker. Trying a simple work until all rexmitted
> skbs heve been processed approach.
>
> Maybe after(highest_sack_end_seq, tp->high_seq) checking is not
> sufficiently accurate and causes entry too often in no-work-to-do
> cases. Since that's not known, I've separated solution to that
> from this patch.
>
> Noticed because of report against a related problem from TAKANO
> Ryousei <takano@axe-inc.co.jp>. He also provided a patch to
> that part of the problem. This patch includes solution to it
> (though this patch has to use somewhat different placement).
> TAKANO's description and patch is available here:
>
> http://marc.info/?l=linux-netdev&m=119149311913288&w=2
>
> ...In short, TAKANO's problem is that end_seq the loop is using
> not necessarily the largest SACK block's end_seq because the
> current ACK may still have higher SACK blocks which are later
> by the loop.
>
> Cc: TAKANO Ryousei <takano@axe-inc.co.jp>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
Applied.
^ permalink raw reply
* Re: [PATCH 5/7] [TCP]: No need to re-count fackets_out/sacked_out at RTO
From: David Miller @ 2007-10-12 0:35 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: netdev, takano
In-Reply-To: <11921028672853-git-send-email-ilpo.jarvinen@helsinki.fi>
From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Thu, 11 Oct 2007 14:41:05 +0300
> Both sacked_out and fackets_out are directly known from how
> parameter. Since fackets_out is accurate, there's no need for
> recounting (sacked_out was previously unnecessarily counted
> in the loop anyway).
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
Applied.
^ permalink raw reply
* Re: [PATCH 4/7] [TCP]: Extract tcp_match_queue_to_sack from sacktag code
From: David Miller @ 2007-10-12 0:34 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: netdev, takano
In-Reply-To: <11921028673030-git-send-email-ilpo.jarvinen@helsinki.fi>
From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Thu, 11 Oct 2007 14:41:04 +0300
> This is necessary for upcoming DSACK bugfix. Reduces sacktag
> length which is not very sad thing at all... :-)
>
> Notice that there's a need to handle out-of-mem at caller's
> place.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
Applied.
^ permalink raw reply
* Re: [PATCH 3/7] [TCP]: Kill almost unused variable pcount from sacktag
From: David Miller @ 2007-10-12 0:34 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: netdev, takano
In-Reply-To: <11921028673528-git-send-email-ilpo.jarvinen@helsinki.fi>
From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Thu, 11 Oct 2007 14:41:03 +0300
> It's on the way for future cutting of that function.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
Applied.
^ permalink raw reply
* Re: [PATCH 2/7] [TCP]: Fix mark_head_lost to ignore R-bit when trying to mark L
From: David Miller @ 2007-10-12 0:33 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: netdev, takano
In-Reply-To: <1192102867725-git-send-email-ilpo.jarvinen@helsinki.fi>
From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Thu, 11 Oct 2007 14:41:02 +0300
> This condition (plain R) can arise at least in recovery that
> is triggered after tcp_undo_loss. There isn't any reason why
> they should not be marked as lost, not marking makes in_flight
> estimator to return too large values.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
Nice observation, applied, thanks Ilpo.
^ permalink raw reply
* Re: Regression in net-2.6.24?
From: Stephen Hemminger @ 2007-10-12 0:31 UTC (permalink / raw)
To: David Miller; +Cc: takano, netdev, ilpo.jarvinen, mchan
In-Reply-To: <20071011.171743.118962219.davem@davemloft.net>
On Thu, 11 Oct 2007 17:17:43 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
> From: Stephen Hemminger <shemminger@linux-foundation.org>
> Date: Thu, 11 Oct 2007 16:55:48 -0700
>
> > On Thu, 11 Oct 2007 16:48:27 -0700 (PDT)
> > David Miller <davem@davemloft.net> wrote:
> >
> > > Alternatively we could loop in tg3_poll() until either budget
> > > is exhausted or tg3_has_work() returns false. Actually, this sounds
> > > like a cleaner scheme the more I think about it.
> > >
> > > BNX2 likely has a similar issue.
> >
> > sky2 as well.
>
> Thanks for the heads up Stephen.
>
> Here is a patch that implements the looping idea in tg3, bnx2, and
> sky2.
>
> diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
> index bbfbdaf..b015d52 100644
> --- a/drivers/net/bnx2.c
> +++ b/drivers/net/bnx2.c
> @@ -2633,15 +2633,11 @@ bnx2_has_work(struct bnx2 *bp)
> return 0;
> }
>
> -static int
> -bnx2_poll(struct napi_struct *napi, int budget)
> +static int bnx2_poll_work(struct bnx2 *bp, int work_done, int budget)
> {
> - struct bnx2 *bp = container_of(napi, struct bnx2, napi);
> - struct net_device *dev = bp->dev;
> struct status_block *sblk = bp->status_blk;
> u32 status_attn_bits = sblk->status_attn_bits;
> u32 status_attn_bits_ack = sblk->status_attn_bits_ack;
> - int work_done = 0;
>
> if ((status_attn_bits & STATUS_ATTN_EVENTS) !=
> (status_attn_bits_ack & STATUS_ATTN_EVENTS)) {
> @@ -2660,27 +2656,43 @@ bnx2_poll(struct napi_struct *napi, int budget)
> bnx2_tx_int(bp);
>
> if (bp->status_blk->status_rx_quick_consumer_index0 != bp->hw_rx_cons)
> - work_done = bnx2_rx_int(bp, budget);
> + work_done += bnx2_rx_int(bp, budget - work_done);
>
> - bp->last_status_idx = bp->status_blk->status_idx;
> - rmb();
> + return work_done;
> +}
> +
> +static int bnx2_poll(struct napi_struct *napi, int budget)
> +{
> + struct bnx2 *bp = container_of(napi, struct bnx2, napi);
> + int work_done = 0;
> +
> + while (1) {
> + work_done += bnx2_poll_work(bp, work_done, budget);
>
> - if (!bnx2_has_work(bp)) {
> - netif_rx_complete(dev, napi);
> - if (likely(bp->flags & USING_MSI_FLAG)) {
> + if (unlikely(work_done >= budget))
> + break;
> +
> + if (likely(!bnx2_has_work(bp))) {
> + bp->last_status_idx = bp->status_blk->status_idx;
> + rmb();
> +
> + netif_rx_complete(bp->dev, napi);
> + if (likely(bp->flags & USING_MSI_FLAG)) {
> + REG_WR(bp, BNX2_PCICFG_INT_ACK_CMD,
> + BNX2_PCICFG_INT_ACK_CMD_INDEX_VALID |
> + bp->last_status_idx);
> + return 0;
> + }
> REG_WR(bp, BNX2_PCICFG_INT_ACK_CMD,
> BNX2_PCICFG_INT_ACK_CMD_INDEX_VALID |
> + BNX2_PCICFG_INT_ACK_CMD_MASK_INT |
> bp->last_status_idx);
> - return 0;
> - }
> - REG_WR(bp, BNX2_PCICFG_INT_ACK_CMD,
> - BNX2_PCICFG_INT_ACK_CMD_INDEX_VALID |
> - BNX2_PCICFG_INT_ACK_CMD_MASK_INT |
> - bp->last_status_idx);
>
> - REG_WR(bp, BNX2_PCICFG_INT_ACK_CMD,
> - BNX2_PCICFG_INT_ACK_CMD_INDEX_VALID |
> - bp->last_status_idx);
> + REG_WR(bp, BNX2_PCICFG_INT_ACK_CMD,
> + BNX2_PCICFG_INT_ACK_CMD_INDEX_VALID |
> + bp->last_status_idx);
> + break;
> + }
> }
>
> return work_done;
> diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
> index fe0e756..25da238 100644
> --- a/drivers/net/sky2.c
> +++ b/drivers/net/sky2.c
> @@ -2605,33 +2605,41 @@ static void sky2_err_intr(struct sky2_hw *hw, u32 status)
> static int sky2_poll(struct napi_struct *napi, int work_limit)
> {
> struct sky2_hw *hw = container_of(napi, struct sky2_hw, napi);
> - u32 status = sky2_read32(hw, B0_Y2_SP_EISR);
> - int work_done;
> + int work_done = 0;
>
> - if (unlikely(status & Y2_IS_ERROR))
> - sky2_err_intr(hw, status);
> + while (1) {
> + u32 status = sky2_read32(hw, B0_Y2_SP_EISR);
>
> - if (status & Y2_IS_IRQ_PHY1)
> - sky2_phy_intr(hw, 0);
> + if (unlikely(status & Y2_IS_ERROR))
> + sky2_err_intr(hw, status);
>
> - if (status & Y2_IS_IRQ_PHY2)
> - sky2_phy_intr(hw, 1);
> + if (status & Y2_IS_IRQ_PHY1)
> + sky2_phy_intr(hw, 0);
>
> - work_done = sky2_status_intr(hw, work_limit);
> + if (status & Y2_IS_IRQ_PHY2)
> + sky2_phy_intr(hw, 1);
>
> - /* More work? */
> - if (hw->st_idx == sky2_read16(hw, STAT_PUT_IDX)) {
> - /* Bug/Errata workaround?
> - * Need to kick the TX irq moderation timer.
> - */
> - if (sky2_read8(hw, STAT_TX_TIMER_CTRL) == TIM_START) {
> - sky2_write8(hw, STAT_TX_TIMER_CTRL, TIM_STOP);
> - sky2_write8(hw, STAT_TX_TIMER_CTRL, TIM_START);
> - }
> + work_done += sky2_status_intr(hw, work_limit - work_done);
>
> - napi_complete(napi);
> - sky2_read32(hw, B0_Y2_SP_LISR);
> + if (unlikely(work_done >= work_limit))
> + break;
> +
> + /* More work? */
> + if (likely(hw->st_idx == sky2_read16(hw, STAT_PUT_IDX))) {
> + /* Bug/Errata workaround?
> + * Need to kick the TX irq moderation timer.
> + */
> + if (sky2_read8(hw, STAT_TX_TIMER_CTRL) == TIM_START) {
> + sky2_write8(hw, STAT_TX_TIMER_CTRL, TIM_STOP);
> + sky2_write8(hw, STAT_TX_TIMER_CTRL, TIM_START);
> + }
> +
> + napi_complete(napi);
> + sky2_read32(hw, B0_Y2_SP_LISR);
> + break;
> + }
> }
> +
> return work_done;
> }
You don't need to re-read the status register and process the PHY irq's inside loop.
Try this:
--- a/drivers/net/sky2.c 2007-10-11 13:16:15.000000000 -0700
+++ b/drivers/net/sky2.c 2007-10-11 17:30:29.000000000 -0700
@@ -2606,7 +2606,7 @@ static int sky2_poll(struct napi_struct
{
struct sky2_hw *hw = container_of(napi, struct sky2_hw, napi);
u32 status = sky2_read32(hw, B0_Y2_SP_EISR);
- int work_done;
+ int work_done = 0;
if (unlikely(status & Y2_IS_ERROR))
sky2_err_intr(hw, status);
@@ -2617,10 +2617,16 @@ static int sky2_poll(struct napi_struct
if (status & Y2_IS_IRQ_PHY2)
sky2_phy_intr(hw, 1);
- work_done = sky2_status_intr(hw, work_limit);
+ for(;;) {
+ work_done += sky2_status_intr(hw, work_limit);
+
+ if (work_done >= work_limit)
+ break;
+
+ /* More work? */
+ if (hw->st_idx != sky2_read16(hw, STAT_PUT_IDX))
+ continue;
- /* More work? */
- if (hw->st_idx == sky2_read16(hw, STAT_PUT_IDX)) {
/* Bug/Errata workaround?
* Need to kick the TX irq moderation timer.
*/
@@ -2628,10 +2634,13 @@ static int sky2_poll(struct napi_struct
sky2_write8(hw, STAT_TX_TIMER_CTRL, TIM_STOP);
sky2_write8(hw, STAT_TX_TIMER_CTRL, TIM_START);
}
-
+
napi_complete(napi);
sky2_read32(hw, B0_Y2_SP_LISR);
+ break;
+
}
+
return work_done;
}
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox