Skip to contentSkip to conversation

sky2: patches for 2.6.22.y

PATCH In review Stephen Hemminger 3 participants 8 messages Sep 28, 2007

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover sky2: patches for 2.6.22.y

Fixes for power regression, VLAN and resume problems. These are all in 2.6.23.

Signature · 1 line
Stephen Hemminger <[email protected]>
#1 lore raw

PATCH 1/3 sky2: reduce impact of watchdog timer

This is the 2.6.22 version of a regression fix that is already in 2.6.23. Change the watchdog timer form 10 per second all the time, to 1 per second and only if interface is up.

drivers/net/sky2.c +18 -27
@@ -96,10 +96,6 @@ static int disable_msi = 0;
 module_param(disable_msi, int, 0);
 MODULE_PARM_DESC(disable_msi, "Disable Message Signaled Interrupt (MSI)");
 
-static int idle_timeout = 100;
-module_param(idle_timeout, int, 0);
-MODULE_PARM_DESC(idle_timeout, "Watchdog timer for lost interrupts (ms)");
-
 static const struct pci_device_id sky2_id_table[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_SYSKONNECT, 0x9000) }, /* SK-9Sxx */
 	{ PCI_DEVICE(PCI_VENDOR_ID_SYSKONNECT, 0x9E00) }, /* SK-9Exx */
@@ -1693,6 +1689,8 @@ static void sky2_link_up(struct sky2_por
 
 	netif_carrier_on(sky2->netdev);
 
+	mod_timer(&hw->watchdog_timer, jiffies + 1);
+
 	/* Turn on link LED */
 	sky2_write8(hw, SK_REG(port, LNK_LED_REG),
 		    LINKLED_ON | LINKLED_BLINK_OFF | LINKLED_LINKSYNC_OFF);
@@ -2384,25 +2382,25 @@ static void sky2_le_error(struct sky2_hw
 	sky2_write32(hw, Q_ADDR(q, Q_CSR), BMU_CLR_IRQ_CHK);
 }
 
-/* If idle then force a fake soft NAPI poll once a second
- * to work around cases where sharing an edge triggered interrupt.
- */
-static inline void sky2_idle_start(struct sky2_hw *hw)
-{
-	if (idle_timeout > 0)
-		mod_timer(&hw->idle_timer,
-			  jiffies + msecs_to_jiffies(idle_timeout));
-}
-
-static void sky2_idle(unsigned long arg)
+/* Force a fake soft NAPI poll to handle lost IRQ's */
+static void sky2_watchdog(unsigned long arg)
 {
 	struct sky2_hw *hw = (struct sky2_hw *) arg;
 	struct net_device *dev = hw->dev[0];
+	int i, active = 0;
 
 	if (__netif_rx_schedule_prep(dev))
 		__netif_rx_schedule(dev);
 
-	mod_timer(&hw->idle_timer, jiffies + msecs_to_jiffies(idle_timeout));
+	for (i = 0; i < hw->ports; i++) {
+		dev = hw->dev[i];
+		if (!netif_running(dev))
+			continue;
+		++active;
+	}
+
+	if (active)
+		mod_timer(&hw->watchdog_timer, round_jiffies(jiffies + HZ));
 }
 
 /* Hardware/software error handling */
@@ -2692,8 +2690,6 @@ static void sky2_restart(struct work_str
 
 	dev_dbg(&hw->pdev->dev, "restarting\n");
 
-	del_timer_sync(&hw->idle_timer);
-
 	rtnl_lock();
 	sky2_write32(hw, B0_IMSK, 0);
 	sky2_read32(hw, B0_IMSK);
@@ -2722,8 +2718,6 @@ static void sky2_restart(struct work_str
 		}
 	}
 
-	sky2_idle_start(hw);
-
 	rtnl_unlock();
 }
 
@@ -3713,11 +3707,9 @@ static int __devinit sky2_probe(struct p
 			sky2_show_addr(dev1);
 	}
 
-	setup_timer(&hw->idle_timer, sky2_idle, (unsigned long) hw);
+	setup_timer(&hw->watchdog_timer, sky2_watchdog, (unsigned long) hw);
 	INIT_WORK(&hw->restart_work, sky2_restart);
 
-	sky2_idle_start(hw);
-
 	pci_set_drvdata(pdev, hw);
 
 	return 0;
@@ -3752,7 +3744,7 @@ static void __devexit sky2_remove(struct
 	if (!hw)
 		return;
 
-	del_timer_sync(&hw->idle_timer);
+	del_timer_sync(&hw->watchdog_timer);
 
 	flush_scheduled_work();
 
@@ -3796,7 +3788,7 @@ static int sky2_suspend(struct pci_dev *
 	if (!hw)
 		return 0;
 
-	del_timer_sync(&hw->idle_timer);
+	del_timer_sync(&hw->watchdog_timer);
 	netif_poll_disable(hw->dev[0]);
 
 	for (i = 0; i < hw->ports; i++) {
@@ -3862,7 +3854,7 @@ static int sky2_resume(struct pci_dev *p
 	}
 
 	netif_poll_enable(hw->dev[0]);
-	sky2_idle_start(hw);
+
 	return 0;
 out:
 	dev_err(&pdev->dev, "resume failed (%d)\n", err);
@@ -3879,7 +3871,6 @@ static void sky2_shutdown(struct pci_dev
 	if (!hw)
 		return;
 
-	del_timer_sync(&hw->idle_timer);
 	netif_poll_disable(hw->dev[0]);
 
 	for (i = 0; i < hw->ports; i++) {
drivers/net/sky2.h +1 -1
@@ -1921,7 +1921,7 @@ struct sky2_hw {
 	u32		     st_idx;
 	dma_addr_t   	     st_dma;
 
-	struct timer_list    idle_timer;
+	struct timer_list    watchdog_timer;
 	struct work_struct   restart_work;
 	int		     msi;
 	wait_queue_head_t    msi_wait;
Signature · 1 line
Stephen Hemminger <[email protected]>
#2 lore raw

PATCH 2/3 sky2: fix VLAN receive processing

In reply to Stephen Hemminger

The length check for truncated frames was not correctly handling the case where VLAN acceleration had already read the tag. Also, the Yukon EX has some features that use high bit of status as security tag.

drivers/net/sky2.c +8 -1
@@ -2049,6 +2049,7 @@ static struct sk_buff *sky2_receive(stru
  	struct sky2_port *sky2 = netdev_priv(dev);
 	struct rx_ring_info *re = sky2->rx_ring + sky2->rx_next;
 	struct sk_buff *skb = NULL;
+	u16 count;
 
 	if (unlikely(netif_msg_rx_status(sky2)))
 		printk(KERN_DEBUG PFX "%s: rx slot %u status 0x%x len %d\n",
@@ -2063,7 +2064,13 @@ static struct sk_buff *sky2_receive(stru
 	if (!(status & GMR_FS_RX_OK))
 		goto resubmit;
 
-	if (status >> 16 != length)
+	count = (status & GMR_FS_LEN) >> 16;
+#ifdef SKY2_VLAN_TAG_USED
+	/* Account for vlan tag */
+	if (sky2->vlgrp && (status & GMR_FS_VLAN))
+		count -= VLAN_HLEN;
+#endif
+	if (count != length)
 		goto len_mismatch;
 
 	if (length < copybreak)
Signature · 1 line
Stephen Hemminger <[email protected]>
#3 lore raw

Re: [PATCH 2/3] sky2: fix VLAN receive processing

Quoted 5 lines · Stephen Hemminger

The length check for truncated frames was not correctly handling the case where VLAN acceleration had already read the tag. Also, the Yukon EX has some features that use high bit of status as security tag.

Thank you.

Best regards

Krzysztof Oledzki

#4 lore raw

Re: [stable] [PATCH 2/3] sky2: fix VLAN receive processing

In reply to Stephen Hemminger
Quoted 4 lines · * Stephen Hemminger ([email protected]) wrote:

The length check for truncated frames was not correctly handling the case where VLAN acceleration had already read the tag. Also, the Yukon EX has some features that use high bit of status as security tag.

Did you leave out the GMR_FS_LEN change on purpose? AFAICT, w/out that you miss the Yukon EX high bit usage. The upstream patch applies, can we simply use that one (below rediffed for stable)?

thanks,
-chris
--

Quoted 1 line

From d6532232cd3de79c852685823a9c52f723816d0a Mon Sep 17 00:00:00 2001

From: Stephen Hemminger <[email protected]>
Date: Wed, 19 Sep 2007 15:36:42 -0700
Subject: sky2: fix VLAN receive processing (resend)

The length check for truncated frames was not correctly handling the case where VLAN acceleration had already read the tag. Also, the Yukon EX has some features that use high bit of status as security tag.

---
 drivers/net/sky2.c |   14 +++++++++++++-
 drivers/net/sky2.h |    2 +-
 2 files changed, 14 insertions(+), 2 deletions(-)
drivers/net/sky2.c +13 -1
@@ -2049,6 +2049,13 @@ static struct sk_buff *sky2_receive(stru
  	struct sky2_port *sky2 = netdev_priv(dev);
 	struct rx_ring_info *re = sky2->rx_ring + sky2->rx_next;
 	struct sk_buff *skb = NULL;
+	u16 count = (status & GMR_FS_LEN) >> 16;
+
+#ifdef SKY2_VLAN_TAG_USED
+	/* Account for vlan tag */
+	if (sky2->vlgrp && (status & GMR_FS_VLAN))
+		count -= VLAN_HLEN;
+#endif
 
 	if (unlikely(netif_msg_rx_status(sky2)))
 		printk(KERN_DEBUG PFX "%s: rx slot %u status 0x%x len %d\n",
@@ -2063,7 +2070,8 @@ static struct sk_buff *sky2_receive(stru
 	if (!(status & GMR_FS_RX_OK))
 		goto resubmit;
 
-	if (status >> 16 != length)
+	/* if length reported by DMA does not match PHY, packet was truncated */
+	if (length != count)
 		goto len_mismatch;
 
 	if (length < copybreak)
@@ -2079,6 +2087,10 @@ len_mismatch:
 	/* Truncation of overlength packets
 	   causes PHY length to not match MAC length */
 	++sky2->net_stats.rx_length_errors;
+	if (netif_msg_rx_err(sky2) && net_ratelimit())
+		pr_info(PFX "%s: rx length mismatch: length %d status %#x\n",
+			dev->name, length, status);
+	goto resubmit;
 
 error:
 	++sky2->net_stats.rx_errors;
drivers/net/sky2.h +1 -1
@@ -1579,7 +1579,7 @@ enum {
 
 /* Receive Frame Status Encoding */
 enum {
-	GMR_FS_LEN	= 0xffff<<16, /* Bit 31..16:	Rx Frame Length */
+	GMR_FS_LEN	= 0x7fff<<16, /* Bit 30..16:	Rx Frame Length */
 	GMR_FS_VLAN	= 1<<13, /* VLAN Packet */
 	GMR_FS_JABBER	= 1<<12, /* Jabber Packet */
 	GMR_FS_UN_SIZE	= 1<<11, /* Undersize Packet */
#5 lore raw

Re: [stable] [PATCH 2/3] sky2: fix VLAN receive processing

Quoted 6 lines · On Fri, 28 Sep 2007 12:20:44 -0700 Chris Wright <[email protected]> wrote:
Quoted 4 lines · * Stephen Hemminger ([email protected]) wrote:

The length check for truncated frames was not correctly handling the case where VLAN acceleration had already read the tag. Also, the Yukon EX has some features that use high bit of status as security tag.

Quoted 7 lines

Did you leave out the GMR_FS_LEN change on purpose? AFAICT, w/out that you miss the Yukon EX high bit usage. The upstream patch applies, can we simply use that one (below rediffed for stable)?

thanks,
-chris

I left it out on purpose because 2.6.22 doesn't support Yukon EX.

#6 lore raw

Re: [stable] [PATCH 2/3] sky2: fix VLAN receive processing

Quoted 1 line · * Stephen Hemminger ([email protected]) wrote:

I left it out on purpose because 2.6.22 doesn't support Yukon EX.

OK, thanks.

#7 lore raw

PATCH 3/3 sky2: fix transmit state on resume

In reply to Stephen Hemminger

After resume, driver has reset the chip so the current state of transmit checksum offload state machine and DMA state machine will be undefined.

The fix is to set the state so that first Tx will set MSS and offset values.

drivers/net/sky2.c +16 -1
@@ -831,6 +831,20 @@ static inline struct sky2_tx_le *get_tx_
 	return le;
 }
 
+static void tx_init(struct sky2_port *sky2)
+{
+	struct sky2_tx_le *le;
+
+	sky2->tx_prod = sky2->tx_cons = 0;
+	sky2->tx_tcpsum = 0;
+	sky2->tx_last_mss = 0;
+
+	le = get_tx_le(sky2);
+	le->addr = 0;
+	le->opcode = OP_ADDR64 | HW_OWNER;
+	sky2->tx_addr64 = 0;
+}
+
 static inline struct tx_ring_info *tx_le_re(struct sky2_port *sky2,
 					    struct sky2_tx_le *le)
 {
@@ -1244,7 +1258,8 @@ static int sky2_up(struct net_device *de
 				GFP_KERNEL);
 	if (!sky2->tx_ring)
 		goto err_out;
-	sky2->tx_prod = sky2->tx_cons = 0;
+
+	tx_init(sky2);
 
 	sky2->rx_le = pci_alloc_consistent(hw->pdev, RX_LE_BYTES,
 					   &sky2->rx_le_map);
Signature · 1 line
Stephen Hemminger <[email protected]>