[PATCH 1/9] myri10ge: ensure tx queues remain stopped

[PATCH 1/9] myri10ge: ensure tx queues remain stopped

[Jon Mason]

Ensure that our tx queues remain stopped when we stop them in
myri10ge_close().  Not doing so can potentially lead to traffic being
transmitted when the interface is removed, which can lead to NULL
pointer dereferences.

Signed-off-by: Jon Mason <mason@myri.com>
---
 drivers/net/myri10ge/myri10ge.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index 3ed5f35..a8ae641 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -1461,7 +1461,8 @@ myri10ge_tx_done(struct myri10ge_slice_state *ss, int mcp_index)
 
 	/* start the queue if we've stopped it */
 	if (netif_tx_queue_stopped(dev_queue) &&
-	    tx->req - tx->done < (tx->mask >> 1)) {
+	    tx->req - tx->done < (tx->mask >> 1) &&
+	    ss->mgp->running == MYRI10GE_ETH_RUNNING) {
 		tx->wake_queue++;
 		netif_tx_wake_queue(dev_queue);
 	}
-- 
1.7.5.4

[PATCH 2/9] myri10ge: Mask PCI Surprise Link Down Events

[Jon Mason]

A SRAM parity error can cause a surprise link down.  Since We can
recover from SRAM parity errors, mask PCI surprise down events.

Signed-off-by: Jon Mason <mason@myri.com>
---
 drivers/net/myri10ge/myri10ge.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index a8ae641..0f0f83d 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -3327,6 +3327,26 @@ static void myri10ge_select_firmware(struct myri10ge_priv *mgp)
 			 mgp->fw_name);
 }
 
+static void myri10ge_mask_surprise_down(struct pci_dev *pdev)
+{
+	struct pci_dev *bridge = pdev->bus->self;
+	int cap;
+	u32 mask;
+
+	if (bridge == NULL)
+		return;
+
+	cap = pci_find_ext_capability(bridge, PCI_EXT_CAP_ID_ERR);
+	if (cap) {
+		/* a sram parity error can cause a surprise link
+		 * down; since we expect and can recover from sram
+		 * parity errors, mask surprise link down events */
+		pci_read_config_dword(bridge, cap + PCI_ERR_UNCOR_MASK, &mask);
+		mask |= 0x20;
+		pci_write_config_dword(bridge, cap + PCI_ERR_UNCOR_MASK, mask);
+	}
+}
+
 #ifdef CONFIG_PM
 static int myri10ge_suspend(struct pci_dev *pdev, pm_message_t state)
 {
@@ -3845,6 +3865,7 @@ static int myri10ge_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto abort_with_enabled;
 	}
 
+	myri10ge_mask_surprise_down(pdev);
 	pci_set_master(pdev);
 	dac_enabled = 1;
 	status = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
-- 
1.7.5.4

Re: [PATCH 2/9] myri10ge: Mask PCI Surprise Link Down Events

[David Miller]

From: Jon Mason <mason@myri.com>
Date: Mon, 27 Jun 2011 10:05:01 -0500

> A SRAM parity error can cause a surprise link down.  Since We can
> recover from SRAM parity errors, mask PCI surprise down events.
> 
> Signed-off-by: Jon Mason <mason@myri.com>

Applied.

[PATCH 3/9] myri10ge: rework parity error check and cleanup

[Jon Mason]

Clean up watchdog reset code:
 - move code that checks for stuck slice to a common routine
 - unless there is a confirmed h/w fault, verify that a stuck
   slice is still stuck in the watchdog worker; if the slice is no
   longer stuck, abort the reset.
 - this removes an egregarious 2000ms pause in the watchdog worker that
   was a diagnostic aid (to look for spurious resets) the snuck into
   production code.

Signed-off-by: Jon Mason <mason@myri.com>
---
 drivers/net/myri10ge/myri10ge.c |  100 +++++++++++++++++++++++---------------
 1 files changed, 60 insertions(+), 40 deletions(-)

diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index 0f0f83d..f3df5eb 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -193,6 +193,7 @@ struct myri10ge_slice_state {
 	int watchdog_tx_done;
 	int watchdog_tx_req;
 	int watchdog_rx_done;
+	int stuck;
 #ifdef CONFIG_MYRI10GE_DCA
 	int cached_dca_tag;
 	int cpu;
@@ -3442,6 +3443,42 @@ static u32 myri10ge_read_reboot(struct myri10ge_priv *mgp)
 	return reboot;
 }
 
+static void
+myri10ge_check_slice(struct myri10ge_slice_state *ss, int *reset_needed,
+		     int *busy_slice_cnt, u32 rx_pause_cnt)
+{
+	struct myri10ge_priv *mgp = ss->mgp;
+	int slice = ss - mgp->ss;
+
+	if (ss->tx.req != ss->tx.done &&
+	    ss->tx.done == ss->watchdog_tx_done &&
+	    ss->watchdog_tx_req != ss->watchdog_tx_done) {
+		/* nic seems like it might be stuck.. */
+		if (rx_pause_cnt != mgp->watchdog_pause) {
+			if (net_ratelimit())
+				netdev_warn(mgp->dev, "slice %d: TX paused, "
+					    "check link partner\n", slice);
+		} else {
+			netdev_warn(mgp->dev,
+				    "slice %d: TX stuck %d %d %d %d %d %d\n",
+				    slice, ss->tx.queue_active, ss->tx.req,
+				    ss->tx.done, ss->tx.pkt_start,
+				    ss->tx.pkt_done,
+				    (int)ntohl(mgp->ss[slice].fw_stats->
+					       send_done_count));
+			*reset_needed = 1;
+			ss->stuck = 1;
+		}
+	}
+	if (ss->watchdog_tx_done != ss->tx.done ||
+	    ss->watchdog_rx_done != ss->rx_done.cnt) {
+		*busy_slice_cnt += 1;
+	}
+	ss->watchdog_tx_done = ss->tx.done;
+	ss->watchdog_tx_req = ss->tx.req;
+	ss->watchdog_rx_done = ss->rx_done.cnt;
+}
+
 /*
  * This watchdog is used to check whether the board has suffered
  * from a parity error and needs to be recovered.
@@ -3450,10 +3487,12 @@ static void myri10ge_watchdog(struct work_struct *work)
 {
 	struct myri10ge_priv *mgp =
 	    container_of(work, struct myri10ge_priv, watchdog_work);
-	struct myri10ge_tx_buf *tx;
-	u32 reboot;
+	struct myri10ge_slice_state *ss;
+	u32 reboot, rx_pause_cnt;
 	int status, rebooted;
 	int i;
+	int reset_needed = 0;
+	int busy_slice_cnt = 0;
 	u16 cmd, vendor;
 
 	mgp->watchdog_resets++;
@@ -3465,8 +3504,7 @@ static void myri10ge_watchdog(struct work_struct *work)
 		 * For now, just report it */
 		reboot = myri10ge_read_reboot(mgp);
 		netdev_err(mgp->dev, "NIC rebooted (0x%x),%s resetting\n",
-			   reboot,
-			   myri10ge_reset_recover ? "" : " not");
+			   reboot, myri10ge_reset_recover ? " " : " not");
 		if (myri10ge_reset_recover == 0)
 			return;
 		rtnl_lock();
@@ -3498,23 +3536,24 @@ static void myri10ge_watchdog(struct work_struct *work)
 				return;
 			}
 		}
-		/* Perhaps it is a software error.  Try to reset */
-
-		netdev_err(mgp->dev, "device timeout, resetting\n");
+		/* Perhaps it is a software error. See if stuck slice
+		 * has recovered, reset if not */
+		rx_pause_cnt = ntohl(mgp->ss[0].fw_stats->dropped_pause);
 		for (i = 0; i < mgp->num_slices; i++) {
-			tx = &mgp->ss[i].tx;
-			netdev_err(mgp->dev, "(%d): %d %d %d %d %d %d\n",
-				   i, tx->queue_active, tx->req,
-				   tx->done, tx->pkt_start, tx->pkt_done,
-				   (int)ntohl(mgp->ss[i].fw_stats->
-					      send_done_count));
-			msleep(2000);
-			netdev_info(mgp->dev, "(%d): %d %d %d %d %d %d\n",
-				    i, tx->queue_active, tx->req,
-				    tx->done, tx->pkt_start, tx->pkt_done,
-				    (int)ntohl(mgp->ss[i].fw_stats->
-					       send_done_count));
+			ss = mgp->ss;
+			if (ss->stuck) {
+				myri10ge_check_slice(ss, &reset_needed,
+						     &busy_slice_cnt,
+						     rx_pause_cnt);
+				ss->stuck = 0;
+			}
 		}
+		if (!reset_needed) {
+			netdev_err(mgp->dev, "not resetting\n");
+			return;
+		}
+
+		netdev_err(mgp->dev, "device timeout, resetting\n");
 	}
 
 	if (!rebooted) {
@@ -3567,27 +3606,8 @@ static void myri10ge_watchdog_timer(unsigned long arg)
 			    myri10ge_fill_thresh)
 				ss->rx_big.watchdog_needed = 0;
 		}
-
-		if (ss->tx.req != ss->tx.done &&
-		    ss->tx.done == ss->watchdog_tx_done &&
-		    ss->watchdog_tx_req != ss->watchdog_tx_done) {
-			/* nic seems like it might be stuck.. */
-			if (rx_pause_cnt != mgp->watchdog_pause) {
-				if (net_ratelimit())
-					netdev_err(mgp->dev, "slice %d: TX paused, check link partner\n",
-						   i);
-			} else {
-				netdev_warn(mgp->dev, "slice %d stuck:", i);
-				reset_needed = 1;
-			}
-		}
-		if (ss->watchdog_tx_done != ss->tx.done ||
-		    ss->watchdog_rx_done != ss->rx_done.cnt) {
-			busy_slice_cnt++;
-		}
-		ss->watchdog_tx_done = ss->tx.done;
-		ss->watchdog_tx_req = ss->tx.req;
-		ss->watchdog_rx_done = ss->rx_done.cnt;
+		myri10ge_check_slice(ss, &reset_needed, &busy_slice_cnt,
+				     rx_pause_cnt);
 	}
 	/* if we've sent or received no traffic, poll the NIC to
 	 * ensure it is still there.  Otherwise, we risk not noticing
-- 
1.7.5.4

Re: [PATCH 3/9] myri10ge: rework parity error check and cleanup

[Ben Hutchings]

On Mon, 2011-06-27 at 10:05 -0500, Jon Mason wrote:
> Clean up watchdog reset code:
>  - move code that checks for stuck slice to a common routine
>  - unless there is a confirmed h/w fault, verify that a stuck
>    slice is still stuck in the watchdog worker; if the slice is no
>    longer stuck, abort the reset.
>  - this removes an egregarious 2000ms pause in the watchdog worker that
[...]

Egregious & gregarious?  Or maybe just egregious?

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH 3/9] myri10ge: rework parity error check and cleanup

[Jon Mason]

On Mon, Jun 27, 2011 at 10:27 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
> On Mon, 2011-06-27 at 10:05 -0500, Jon Mason wrote:
>> Clean up watchdog reset code:
>>  - move code that checks for stuck slice to a common routine
>>  - unless there is a confirmed h/w fault, verify that a stuck
>>    slice is still stuck in the watchdog worker; if the slice is no
>>    longer stuck, abort the reset.
>>  - this removes an egregarious 2000ms pause in the watchdog worker that
> [...]
>
> Egregious & gregarious?  Or maybe just egregious?

It's my new errno, -EGREGARIOUS.  :)

Thanks, I'll roll this and Joe's comments in version #2.

>
> Ben.
>
> --
> Ben Hutchings, Senior Software Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
>
>

Re: [PATCH 3/9] myri10ge: rework parity error check and cleanup

[Joe Perches]

On Mon, 2011-06-27 at 10:05 -0500, Jon Mason wrote:
> Clean up watchdog reset code:
>  - unless there is a confirmed h/w fault, verify that a stuck
>    slice is still stuck in the watchdog worker; if the slice is no
>    longer stuck, abort the reset.
[]
> @@ -3498,23 +3536,24 @@ static void myri10ge_watchdog(struct work_struct *work)
[]
> +		if (!reset_needed) {
> +			netdev_err(mgp->dev, "not resetting\n");
> +			return;
> +		}

Probably the wrong level.  Maybe netdev_dbg() or netdev_info()

[PATCH 3/9 v2] myri10ge: rework parity error check and cleanup

[Jon Mason]

Clean up watchdog reset code:
 - move code that checks for stuck slice to a common routine
 - unless there is a confirmed h/w fault, verify that a stuck
   slice is still stuck in the watchdog worker; if the slice is no
   longer stuck, abort the reset.
 - this removes an egregious 2000ms pause in the watchdog worker that
   was a diagnostic aid (to look for spurious resets) the snuck into
   production code.

v2 includes corrections from Ben Hutchings and Joe Perches

Signed-off-by: Jon Mason <mason@myri.com>
---
 drivers/net/myri10ge/myri10ge.c |  100 +++++++++++++++++++++++---------------
 1 files changed, 60 insertions(+), 40 deletions(-)

diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index 0f0f83d..c2574c5 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -193,6 +193,7 @@ struct myri10ge_slice_state {
 	int watchdog_tx_done;
 	int watchdog_tx_req;
 	int watchdog_rx_done;
+	int stuck;
 #ifdef CONFIG_MYRI10GE_DCA
 	int cached_dca_tag;
 	int cpu;
@@ -3442,6 +3443,42 @@ static u32 myri10ge_read_reboot(struct myri10ge_priv *mgp)
 	return reboot;
 }
 
+static void
+myri10ge_check_slice(struct myri10ge_slice_state *ss, int *reset_needed,
+		     int *busy_slice_cnt, u32 rx_pause_cnt)
+{
+	struct myri10ge_priv *mgp = ss->mgp;
+	int slice = ss - mgp->ss;
+
+	if (ss->tx.req != ss->tx.done &&
+	    ss->tx.done == ss->watchdog_tx_done &&
+	    ss->watchdog_tx_req != ss->watchdog_tx_done) {
+		/* nic seems like it might be stuck.. */
+		if (rx_pause_cnt != mgp->watchdog_pause) {
+			if (net_ratelimit())
+				netdev_warn(mgp->dev, "slice %d: TX paused, "
+					    "check link partner\n", slice);
+		} else {
+			netdev_warn(mgp->dev,
+				    "slice %d: TX stuck %d %d %d %d %d %d\n",
+				    slice, ss->tx.queue_active, ss->tx.req,
+				    ss->tx.done, ss->tx.pkt_start,
+				    ss->tx.pkt_done,
+				    (int)ntohl(mgp->ss[slice].fw_stats->
+					       send_done_count));
+			*reset_needed = 1;
+			ss->stuck = 1;
+		}
+	}
+	if (ss->watchdog_tx_done != ss->tx.done ||
+	    ss->watchdog_rx_done != ss->rx_done.cnt) {
+		*busy_slice_cnt += 1;
+	}
+	ss->watchdog_tx_done = ss->tx.done;
+	ss->watchdog_tx_req = ss->tx.req;
+	ss->watchdog_rx_done = ss->rx_done.cnt;
+}
+
 /*
  * This watchdog is used to check whether the board has suffered
  * from a parity error and needs to be recovered.
@@ -3450,10 +3487,12 @@ static void myri10ge_watchdog(struct work_struct *work)
 {
 	struct myri10ge_priv *mgp =
 	    container_of(work, struct myri10ge_priv, watchdog_work);
-	struct myri10ge_tx_buf *tx;
-	u32 reboot;
+	struct myri10ge_slice_state *ss;
+	u32 reboot, rx_pause_cnt;
 	int status, rebooted;
 	int i;
+	int reset_needed = 0;
+	int busy_slice_cnt = 0;
 	u16 cmd, vendor;
 
 	mgp->watchdog_resets++;
@@ -3465,8 +3504,7 @@ static void myri10ge_watchdog(struct work_struct *work)
 		 * For now, just report it */
 		reboot = myri10ge_read_reboot(mgp);
 		netdev_err(mgp->dev, "NIC rebooted (0x%x),%s resetting\n",
-			   reboot,
-			   myri10ge_reset_recover ? "" : " not");
+			   reboot, myri10ge_reset_recover ? " " : " not");
 		if (myri10ge_reset_recover == 0)
 			return;
 		rtnl_lock();
@@ -3498,23 +3536,24 @@ static void myri10ge_watchdog(struct work_struct *work)
 				return;
 			}
 		}
-		/* Perhaps it is a software error.  Try to reset */
-
-		netdev_err(mgp->dev, "device timeout, resetting\n");
+		/* Perhaps it is a software error. See if stuck slice
+		 * has recovered, reset if not */
+		rx_pause_cnt = ntohl(mgp->ss[0].fw_stats->dropped_pause);
 		for (i = 0; i < mgp->num_slices; i++) {
-			tx = &mgp->ss[i].tx;
-			netdev_err(mgp->dev, "(%d): %d %d %d %d %d %d\n",
-				   i, tx->queue_active, tx->req,
-				   tx->done, tx->pkt_start, tx->pkt_done,
-				   (int)ntohl(mgp->ss[i].fw_stats->
-					      send_done_count));
-			msleep(2000);
-			netdev_info(mgp->dev, "(%d): %d %d %d %d %d %d\n",
-				    i, tx->queue_active, tx->req,
-				    tx->done, tx->pkt_start, tx->pkt_done,
-				    (int)ntohl(mgp->ss[i].fw_stats->
-					       send_done_count));
+			ss = mgp->ss;
+			if (ss->stuck) {
+				myri10ge_check_slice(ss, &reset_needed,
+						     &busy_slice_cnt,
+						     rx_pause_cnt);
+				ss->stuck = 0;
+			}
 		}
+		if (!reset_needed) {
+			netdev_dbg(mgp->dev, "not resetting\n");
+			return;
+		}
+
+		netdev_err(mgp->dev, "device timeout, resetting\n");
 	}
 
 	if (!rebooted) {
@@ -3567,27 +3606,8 @@ static void myri10ge_watchdog_timer(unsigned long arg)
 			    myri10ge_fill_thresh)
 				ss->rx_big.watchdog_needed = 0;
 		}
-
-		if (ss->tx.req != ss->tx.done &&
-		    ss->tx.done == ss->watchdog_tx_done &&
-		    ss->watchdog_tx_req != ss->watchdog_tx_done) {
-			/* nic seems like it might be stuck.. */
-			if (rx_pause_cnt != mgp->watchdog_pause) {
-				if (net_ratelimit())
-					netdev_err(mgp->dev, "slice %d: TX paused, check link partner\n",
-						   i);
-			} else {
-				netdev_warn(mgp->dev, "slice %d stuck:", i);
-				reset_needed = 1;
-			}
-		}
-		if (ss->watchdog_tx_done != ss->tx.done ||
-		    ss->watchdog_rx_done != ss->rx_done.cnt) {
-			busy_slice_cnt++;
-		}
-		ss->watchdog_tx_done = ss->tx.done;
-		ss->watchdog_tx_req = ss->tx.req;
-		ss->watchdog_rx_done = ss->rx_done.cnt;
+		myri10ge_check_slice(ss, &reset_needed, &busy_slice_cnt,
+				     rx_pause_cnt);
 	}
 	/* if we've sent or received no traffic, poll the NIC to
 	 * ensure it is still there.  Otherwise, we risk not noticing
-- 
1.7.5.4

Re: [PATCH 3/9 v2] myri10ge: rework parity error check and cleanup

[Joe Perches]

On Mon, 2011-06-27 at 15:54 -0500, Jon Mason wrote:
> Clean up watchdog reset code:
>  - move code that checks for stuck slice to a common routine
>  - unless there is a confirmed h/w fault, verify that a stuck
>    slice is still stuck in the watchdog worker; if the slice is no
>    longer stuck, abort the reset.
>  - this removes an egregious 2000ms pause in the watchdog worker that
>    was a diagnostic aid (to look for spurious resets) the snuck into
>    production code.
> v2 includes corrections from Ben Hutchings and Joe Perches

Here's some more trivia:

> diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
[]
> @@ -3442,6 +3443,42 @@ static u32 myri10ge_read_reboot(struct myri10ge_priv *mgp)
>  	return reboot;
>  }
>  
> +static void
> +myri10ge_check_slice(struct myri10ge_slice_state *ss, int *reset_needed,
> +		     int *busy_slice_cnt, u32 rx_pause_cnt)
> +{
[]
> +		/* nic seems like it might be stuck.. */
> +		if (rx_pause_cnt != mgp->watchdog_pause) {
> +			if (net_ratelimit())
> +				netdev_warn(mgp->dev, "slice %d: TX paused, "
> +					    "check link partner\n", slice);

I think this would be better if the format weren't split.

				netdev_warn(mgp->dev, "slice %d: TX paused, check link partner\n",
					    slice);
or
				netdev_warn(mgp->dev,
					    "slice %d: TX paused, check link partner\n",
					    slice);
or if you really must split it because exceeding 80 columns
makes you itchy:
				netdev_warn(mgp->dev, "slice %d: "
					    "TX paused, check link partner\n",
					    slice);

> @@ -3465,8 +3504,7 @@ static void myri10ge_watchdog(struct work_struct *work)
>  		 * For now, just report it */
>  		reboot = myri10ge_read_reboot(mgp);
>  		netdev_err(mgp->dev, "NIC rebooted (0x%x),%s resetting\n",
> -			   reboot,
> -			   myri10ge_reset_recover ? "" : " not");
> +			   reboot, myri10ge_reset_recover ? " " : " not");

I think this was correct before you changed it.

Maybe:
			   reboot, myri10ge_reset_recover ? "" : " not");

Re: [PATCH 3/9 v2] myri10ge: rework parity error check and cleanup

[Jon Mason]

On Mon, Jun 27, 2011 at 9:17 PM, Joe Perches <joe@perches.com> wrote:
> On Mon, 2011-06-27 at 15:54 -0500, Jon Mason wrote:
>> Clean up watchdog reset code:
>>  - move code that checks for stuck slice to a common routine
>>  - unless there is a confirmed h/w fault, verify that a stuck
>>    slice is still stuck in the watchdog worker; if the slice is no
>>    longer stuck, abort the reset.
>>  - this removes an egregious 2000ms pause in the watchdog worker that
>>    was a diagnostic aid (to look for spurious resets) the snuck into
>>    production code.
>> v2 includes corrections from Ben Hutchings and Joe Perches
>
> Here's some more trivia:
>
>> diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
> []
>> @@ -3442,6 +3443,42 @@ static u32 myri10ge_read_reboot(struct myri10ge_priv *mgp)
>>       return reboot;
>>  }
>>
>> +static void
>> +myri10ge_check_slice(struct myri10ge_slice_state *ss, int *reset_needed,
>> +                  int *busy_slice_cnt, u32 rx_pause_cnt)
>> +{
> []
>> +             /* nic seems like it might be stuck.. */
>> +             if (rx_pause_cnt != mgp->watchdog_pause) {
>> +                     if (net_ratelimit())
>> +                             netdev_warn(mgp->dev, "slice %d: TX paused, "
>> +                                         "check link partner\n", slice);
>
> I think this would be better if the format weren't split.
>
>                                netdev_warn(mgp->dev, "slice %d: TX paused, check link partner\n",
>                                            slice);
> or
>                                netdev_warn(mgp->dev,
>                                            "slice %d: TX paused, check link partner\n",
>                                            slice);
> or if you really must split it because exceeding 80 columns
> makes you itchy:
>                                netdev_warn(mgp->dev, "slice %d: "
>                                            "TX paused, check link partner\n",
>                                            slice);

Naa, I prefer it this way.

>> @@ -3465,8 +3504,7 @@ static void myri10ge_watchdog(struct work_struct *work)
>>                * For now, just report it */
>>               reboot = myri10ge_read_reboot(mgp);
>>               netdev_err(mgp->dev, "NIC rebooted (0x%x),%s resetting\n",
>> -                        reboot,
>> -                        myri10ge_reset_recover ? "" : " not");
>> +                        reboot, myri10ge_reset_recover ? " " : " not");
>
> I think this was correct before you changed it.
>
> Maybe:
>                           reboot, myri10ge_reset_recover ? "" : " not");

Yes, I believe this was the intent.

>
>
>

[PATCH 3/9 v3] myri10ge: rework parity error check and cleanup

[Jon Mason]

Clean up watchdog reset code:
 - move code that checks for stuck slice to a common routine
 - unless there is a confirmed h/w fault, verify that a stuck
   slice is still stuck in the watchdog worker; if the slice is no
   longer stuck, abort the reset.
 - this removes an egregious 2000ms pause in the watchdog worker that
   was a diagnostic aid (to look for spurious resets) the snuck into
   production code.

v3 includes corrections from Joe Perches

Signed-off-by: Jon Mason <mason@myri.com>
---
 drivers/net/myri10ge/myri10ge.c |  100 +++++++++++++++++++++++---------------
 1 files changed, 60 insertions(+), 40 deletions(-)

diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index 0f0f83d..ca03457 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -193,6 +193,7 @@ struct myri10ge_slice_state {
 	int watchdog_tx_done;
 	int watchdog_tx_req;
 	int watchdog_rx_done;
+	int stuck;
 #ifdef CONFIG_MYRI10GE_DCA
 	int cached_dca_tag;
 	int cpu;
@@ -3442,6 +3443,42 @@ static u32 myri10ge_read_reboot(struct myri10ge_priv *mgp)
 	return reboot;
 }
 
+static void
+myri10ge_check_slice(struct myri10ge_slice_state *ss, int *reset_needed,
+		     int *busy_slice_cnt, u32 rx_pause_cnt)
+{
+	struct myri10ge_priv *mgp = ss->mgp;
+	int slice = ss - mgp->ss;
+
+	if (ss->tx.req != ss->tx.done &&
+	    ss->tx.done == ss->watchdog_tx_done &&
+	    ss->watchdog_tx_req != ss->watchdog_tx_done) {
+		/* nic seems like it might be stuck.. */
+		if (rx_pause_cnt != mgp->watchdog_pause) {
+			if (net_ratelimit())
+				netdev_warn(mgp->dev, "slice %d: TX paused, "
+					    "check link partner\n", slice);
+		} else {
+			netdev_warn(mgp->dev,
+				    "slice %d: TX stuck %d %d %d %d %d %d\n",
+				    slice, ss->tx.queue_active, ss->tx.req,
+				    ss->tx.done, ss->tx.pkt_start,
+				    ss->tx.pkt_done,
+				    (int)ntohl(mgp->ss[slice].fw_stats->
+					       send_done_count));
+			*reset_needed = 1;
+			ss->stuck = 1;
+		}
+	}
+	if (ss->watchdog_tx_done != ss->tx.done ||
+	    ss->watchdog_rx_done != ss->rx_done.cnt) {
+		*busy_slice_cnt += 1;
+	}
+	ss->watchdog_tx_done = ss->tx.done;
+	ss->watchdog_tx_req = ss->tx.req;
+	ss->watchdog_rx_done = ss->rx_done.cnt;
+}
+
 /*
  * This watchdog is used to check whether the board has suffered
  * from a parity error and needs to be recovered.
@@ -3450,10 +3487,12 @@ static void myri10ge_watchdog(struct work_struct *work)
 {
 	struct myri10ge_priv *mgp =
 	    container_of(work, struct myri10ge_priv, watchdog_work);
-	struct myri10ge_tx_buf *tx;
-	u32 reboot;
+	struct myri10ge_slice_state *ss;
+	u32 reboot, rx_pause_cnt;
 	int status, rebooted;
 	int i;
+	int reset_needed = 0;
+	int busy_slice_cnt = 0;
 	u16 cmd, vendor;
 
 	mgp->watchdog_resets++;
@@ -3465,8 +3504,7 @@ static void myri10ge_watchdog(struct work_struct *work)
 		 * For now, just report it */
 		reboot = myri10ge_read_reboot(mgp);
 		netdev_err(mgp->dev, "NIC rebooted (0x%x),%s resetting\n",
-			   reboot,
-			   myri10ge_reset_recover ? "" : " not");
+			   reboot, myri10ge_reset_recover ? "" : " not");
 		if (myri10ge_reset_recover == 0)
 			return;
 		rtnl_lock();
@@ -3498,23 +3536,24 @@ static void myri10ge_watchdog(struct work_struct *work)
 				return;
 			}
 		}
-		/* Perhaps it is a software error.  Try to reset */
-
-		netdev_err(mgp->dev, "device timeout, resetting\n");
+		/* Perhaps it is a software error. See if stuck slice
+		 * has recovered, reset if not */
+		rx_pause_cnt = ntohl(mgp->ss[0].fw_stats->dropped_pause);
 		for (i = 0; i < mgp->num_slices; i++) {
-			tx = &mgp->ss[i].tx;
-			netdev_err(mgp->dev, "(%d): %d %d %d %d %d %d\n",
-				   i, tx->queue_active, tx->req,
-				   tx->done, tx->pkt_start, tx->pkt_done,
-				   (int)ntohl(mgp->ss[i].fw_stats->
-					      send_done_count));
-			msleep(2000);
-			netdev_info(mgp->dev, "(%d): %d %d %d %d %d %d\n",
-				    i, tx->queue_active, tx->req,
-				    tx->done, tx->pkt_start, tx->pkt_done,
-				    (int)ntohl(mgp->ss[i].fw_stats->
-					       send_done_count));
+			ss = mgp->ss;
+			if (ss->stuck) {
+				myri10ge_check_slice(ss, &reset_needed,
+						     &busy_slice_cnt,
+						     rx_pause_cnt);
+				ss->stuck = 0;
+			}
 		}
+		if (!reset_needed) {
+			netdev_dbg(mgp->dev, "not resetting\n");
+			return;
+		}
+
+		netdev_err(mgp->dev, "device timeout, resetting\n");
 	}
 
 	if (!rebooted) {
@@ -3567,27 +3606,8 @@ static void myri10ge_watchdog_timer(unsigned long arg)
 			    myri10ge_fill_thresh)
 				ss->rx_big.watchdog_needed = 0;
 		}
-
-		if (ss->tx.req != ss->tx.done &&
-		    ss->tx.done == ss->watchdog_tx_done &&
-		    ss->watchdog_tx_req != ss->watchdog_tx_done) {
-			/* nic seems like it might be stuck.. */
-			if (rx_pause_cnt != mgp->watchdog_pause) {
-				if (net_ratelimit())
-					netdev_err(mgp->dev, "slice %d: TX paused, check link partner\n",
-						   i);
-			} else {
-				netdev_warn(mgp->dev, "slice %d stuck:", i);
-				reset_needed = 1;
-			}
-		}
-		if (ss->watchdog_tx_done != ss->tx.done ||
-		    ss->watchdog_rx_done != ss->rx_done.cnt) {
-			busy_slice_cnt++;
-		}
-		ss->watchdog_tx_done = ss->tx.done;
-		ss->watchdog_tx_req = ss->tx.req;
-		ss->watchdog_rx_done = ss->rx_done.cnt;
+		myri10ge_check_slice(ss, &reset_needed, &busy_slice_cnt,
+				     rx_pause_cnt);
 	}
 	/* if we've sent or received no traffic, poll the NIC to
 	 * ensure it is still there.  Otherwise, we risk not noticing
-- 
1.7.5.4

Re: [PATCH 3/9 v3] myri10ge: rework parity error check and cleanup

[David Miller]

From: Jon Mason <mason@myri.com>
Date: Mon, 27 Jun 2011 22:57:28 -0500

> Clean up watchdog reset code:
>  - move code that checks for stuck slice to a common routine
>  - unless there is a confirmed h/w fault, verify that a stuck
>    slice is still stuck in the watchdog worker; if the slice is no
>    longer stuck, abort the reset.
>  - this removes an egregious 2000ms pause in the watchdog worker that
>    was a diagnostic aid (to look for spurious resets) the snuck into
>    production code.
> 
> v3 includes corrections from Joe Perches
> 
> Signed-off-by: Jon Mason <mason@myri.com>

Applied.

[PATCH 4/9] myri10ge: allow small_bytes = 0

[Jon Mason]

Allow page-based receive to work when small_bytes is set to 0.

Signed-off-by: Jon Mason <mason@myri.com>
---
 drivers/net/myri10ge/myri10ge.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index f3df5eb..71461e5 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -2004,8 +2004,12 @@ static int myri10ge_allocate_rings(struct myri10ge_slice_state *ss)
 	ss->rx_big.page_offset = MYRI10GE_ALLOC_SIZE;
 	ss->rx_small.watchdog_needed = 0;
 	ss->rx_big.watchdog_needed = 0;
-	myri10ge_alloc_rx_pages(mgp, &ss->rx_small,
-				mgp->small_bytes + MXGEFW_PAD, 0);
+	if (mgp->small_bytes == 0) {
+		ss->rx_small.fill_cnt = ss->rx_small.mask + 1;
+	} else {
+		myri10ge_alloc_rx_pages(mgp, &ss->rx_small,
+					mgp->small_bytes + MXGEFW_PAD, 0);
+	}
 
 	if (ss->rx_small.fill_cnt < ss->rx_small.mask + 1) {
 		netdev_err(dev, "slice-%d: alloced only %d small bufs\n",
@@ -2031,6 +2035,8 @@ abort_with_rx_big_ring:
 	}
 
 abort_with_rx_small_ring:
+	if (mgp->small_bytes == 0)
+		ss->rx_small.fill_cnt = ss->rx_small.cnt;
 	for (i = ss->rx_small.cnt; i < ss->rx_small.fill_cnt; i++) {
 		int idx = i & ss->rx_small.mask;
 		myri10ge_unmap_rx_page(mgp->pdev, &ss->rx_small.info[idx],
@@ -2081,6 +2087,8 @@ static void myri10ge_free_rings(struct myri10ge_slice_state *ss)
 		put_page(ss->rx_big.info[idx].page);
 	}
 
+	if (mgp->small_bytes == 0)
+		ss->rx_small.fill_cnt = ss->rx_small.cnt;
 	for (i = ss->rx_small.cnt; i < ss->rx_small.fill_cnt; i++) {
 		idx = i & ss->rx_small.mask;
 		if (i == ss->rx_small.fill_cnt - 1)
@@ -2418,7 +2426,7 @@ static int myri10ge_open(struct net_device *dev)
 		mgp->small_bytes = VLAN_ETH_FRAME_LEN;
 
 	/* Override the small buffer size? */
-	if (myri10ge_small_bytes > 0)
+	if (myri10ge_small_bytes >= 0)
 		mgp->small_bytes = myri10ge_small_bytes;
 
 	/* Firmware needs the big buff size as a power of 2.  Lie and
-- 
1.7.5.4

Re: [PATCH 4/9] myri10ge: allow small_bytes = 0

[David Miller]

From: Jon Mason <mason@myri.com>
Date: Mon, 27 Jun 2011 10:05:03 -0500

> Allow page-based receive to work when small_bytes is set to 0.
> 
> Signed-off-by: Jon Mason <mason@myri.com>

Applied.

[PATCH 5/9] myri10ge: add support for set_phys_id

[Jon Mason]

Add myri10ge driver support for the ethtool identify operation.
NOTE: Rather than blinking (which is the normal case), when identify is
used, the yellow LED turns solid.

Signed-off-by: Jon Mason <mason@myri.com>
---
 drivers/net/myri10ge/myri10ge.c                |   55 ++++++++++++++++++++++++
 drivers/net/myri10ge/myri10ge_mcp_gen_header.h |    2 +
 2 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index 71461e5..83cbf2c 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -1911,6 +1911,60 @@ static u32 myri10ge_get_msglevel(struct net_device *netdev)
 	return mgp->msg_enable;
 }
 
+/*
+ * Use a low-level command to change the LED behavior. Rather than
+ * blinking (which is the normal case), when identify is used, the
+ * yellow LED turns solid.
+ */
+static int myri10ge_led(struct myri10ge_priv *mgp, int on)
+{
+	struct mcp_gen_header *hdr;
+	struct device *dev = &mgp->pdev->dev;
+	size_t hdr_off, pattern_off, hdr_len;
+	u32 pattern = 0xfffffffe;
+
+	/* find running firmware header */
+	hdr_off = swab32(readl(mgp->sram + MCP_HEADER_PTR_OFFSET));
+	if ((hdr_off & 3) || hdr_off + sizeof(*hdr) > mgp->sram_size) {
+		dev_err(dev, "Running firmware has bad header offset (%d)\n",
+			(int)hdr_off);
+		return -EIO;
+	}
+	hdr_len = swab32(readl(mgp->sram + hdr_off +
+			       offsetof(struct mcp_gen_header, header_length)));
+	pattern_off = hdr_off + offsetof(struct mcp_gen_header, led_pattern);
+	if (pattern_off >= (hdr_len + hdr_off)) {
+		dev_info(dev, "Firmware does not support LED identification\n");
+		return -EINVAL;
+	}
+	if (!on)
+		pattern = swab32(readl(mgp->sram + pattern_off + 4));
+	writel(htonl(pattern), mgp->sram + pattern_off);
+	return 0;
+}
+
+static int
+myri10ge_phys_id(struct net_device *netdev, enum ethtool_phys_id_state state)
+{
+	struct myri10ge_priv *mgp = netdev_priv(netdev);
+	int rc;
+
+	switch (state) {
+	case ETHTOOL_ID_ACTIVE:
+		rc = myri10ge_led(mgp, 1);
+		break;
+
+	case ETHTOOL_ID_INACTIVE:
+		rc =  myri10ge_led(mgp, 0);
+		break;
+
+	default:
+		rc = -EINVAL;
+	}
+
+	return rc;
+}
+
 static const struct ethtool_ops myri10ge_ethtool_ops = {
 	.get_settings = myri10ge_get_settings,
 	.get_drvinfo = myri10ge_get_drvinfo,
@@ -1925,6 +1979,7 @@ static const struct ethtool_ops myri10ge_ethtool_ops = {
 	.get_ethtool_stats = myri10ge_get_ethtool_stats,
 	.set_msglevel = myri10ge_set_msglevel,
 	.get_msglevel = myri10ge_get_msglevel,
+	.set_phys_id = myri10ge_phys_id,
 };
 
 static int myri10ge_allocate_rings(struct myri10ge_slice_state *ss)
diff --git a/drivers/net/myri10ge/myri10ge_mcp_gen_header.h b/drivers/net/myri10ge/myri10ge_mcp_gen_header.h
index 62a1cba..7ec4b86 100644
--- a/drivers/net/myri10ge/myri10ge_mcp_gen_header.h
+++ b/drivers/net/myri10ge/myri10ge_mcp_gen_header.h
@@ -45,6 +45,8 @@ struct mcp_gen_header {
 	unsigned bss_addr;	/* start of bss */
 	unsigned features;
 	unsigned ee_hdr_addr;
+	unsigned led_pattern;
+	unsigned led_pattern_dflt;
 	/* 8 */
 };
 
-- 
1.7.5.4

Re: [PATCH 5/9] myri10ge: add support for set_phys_id

[David Miller]

From: Jon Mason <mason@myri.com>
Date: Mon, 27 Jun 2011 10:05:04 -0500

> Add myri10ge driver support for the ethtool identify operation.
> NOTE: Rather than blinking (which is the normal case), when identify is
> used, the yellow LED turns solid.
> 
> Signed-off-by: Jon Mason <mason@myri.com>

Applied.

[PATCH 6/9] myri10ge: remove unnecessary read of PCI_CAP_ID_EXP

[Jon Mason]

The PCIE capability offset is saved during PCI bus walking.  It will
remove an unnecessary search in the PCI configuration space if this
value is referenced instead of reacquiring it.

Signed-off-by: Jon Mason <mason@myri.com>
---
 drivers/net/myri10ge/myri10ge.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index 83cbf2c..851365c 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -1081,7 +1081,7 @@ static int myri10ge_toggle_relaxed(struct pci_dev *pdev, int on)
 	int ret, cap, err;
 	u16 ctl;
 
-	cap = pci_find_capability(pdev, PCI_CAP_ID_EXP);
+	cap = pci_pcie_cap(pdev);
 	if (!cap)
 		return 0;
 
@@ -3191,7 +3191,7 @@ static void myri10ge_enable_ecrc(struct myri10ge_priv *mgp)
 {
 	struct pci_dev *bridge = mgp->pdev->bus->self;
 	struct device *dev = &mgp->pdev->dev;
-	unsigned cap;
+	int cap;
 	unsigned err_cap;
 	u16 val;
 	u8 ext_type;
@@ -3201,7 +3201,7 @@ static void myri10ge_enable_ecrc(struct myri10ge_priv *mgp)
 		return;
 
 	/* check that the bridge is a root port */
-	cap = pci_find_capability(bridge, PCI_CAP_ID_EXP);
+	cap = pci_pcie_cap(bridge);
 	pci_read_config_word(bridge, cap + PCI_CAP_FLAGS, &val);
 	ext_type = (val & PCI_EXP_FLAGS_TYPE) >> 4;
 	if (ext_type != PCI_EXP_TYPE_ROOT_PORT) {
@@ -3219,8 +3219,7 @@ static void myri10ge_enable_ecrc(struct myri10ge_priv *mgp)
 						" to force ECRC\n");
 					return;
 				}
-				cap =
-				    pci_find_capability(bridge, PCI_CAP_ID_EXP);
+				cap = pci_pcie_cap(bridge);
 				pci_read_config_word(bridge,
 						     cap + PCI_CAP_FLAGS, &val);
 				ext_type = (val & PCI_EXP_FLAGS_TYPE) >> 4;
@@ -3341,7 +3340,7 @@ static void myri10ge_select_firmware(struct myri10ge_priv *mgp)
 		int link_width, exp_cap;
 		u16 lnk;
 
-		exp_cap = pci_find_capability(mgp->pdev, PCI_CAP_ID_EXP);
+		exp_cap = pci_pcie_cap(mgp->pdev);
 		pci_read_config_word(mgp->pdev, exp_cap + PCI_EXP_LNKSTA, &lnk);
 		link_width = (lnk >> 4) & 0x3f;
 
-- 
1.7.5.4

Re: [PATCH 6/9] myri10ge: remove unnecessary read of PCI_CAP_ID_EXP

[David Miller]

From: Jon Mason <mason@myri.com>
Date: Mon, 27 Jun 2011 10:05:05 -0500

> The PCIE capability offset is saved during PCI bus walking.  It will
> remove an unnecessary search in the PCI configuration space if this
> value is referenced instead of reacquiring it.
> 
> Signed-off-by: Jon Mason <mason@myri.com>

Applied.

[PATCH 7/9] myri10ge: misc style cleanups

[Jon Mason]

Miscellaneous white space, style, and other cleanups

Signed-off-by: Jon Mason <mason@myri.com>
---
 drivers/net/myri10ge/myri10ge.c |   29 ++++++++++++++---------------
 1 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index 851365c..1e1681b 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -1086,6 +1086,9 @@ static int myri10ge_toggle_relaxed(struct pci_dev *pdev, int on)
 		return 0;
 
 	err = pci_read_config_word(pdev, cap + PCI_EXP_DEVCTL, &ctl);
+	if (err)
+		return 0;
+
 	ret = (ctl & PCI_EXP_DEVCTL_RELAX_EN) >> 4;
 	if (ret != on) {
 		ctl &= ~PCI_EXP_DEVCTL_RELAX_EN;
@@ -1140,20 +1143,19 @@ static void myri10ge_setup_dca(struct myri10ge_priv *mgp)
 		mgp->ss[i].cpu = -1;
 		mgp->ss[i].cached_dca_tag = -1;
 		myri10ge_update_dca(&mgp->ss[i]);
-	 }
+	}
 }
 
 static void myri10ge_teardown_dca(struct myri10ge_priv *mgp)
 {
 	struct pci_dev *pdev = mgp->pdev;
-	int err;
 
 	if (!mgp->dca_enabled)
 		return;
 	mgp->dca_enabled = 0;
 	if (mgp->relaxed_order)
 		myri10ge_toggle_relaxed(pdev, 1);
-	err = dca_remove_requester(&pdev->dev);
+	dca_remove_requester(&pdev->dev);
 }
 
 static int myri10ge_notify_dca_device(struct device *dev, void *data)
@@ -1314,7 +1316,7 @@ myri10ge_unmap_rx_page(struct pci_dev *pdev,
 
 static inline int
 myri10ge_rx_done(struct myri10ge_slice_state *ss, int len, __wsum csum,
-		 int lro_enabled)
+		 bool lro_enabled)
 {
 	struct myri10ge_priv *mgp = ss->mgp;
 	struct sk_buff *skb;
@@ -1474,11 +1476,9 @@ myri10ge_clean_rx_done(struct myri10ge_slice_state *ss, int budget)
 {
 	struct myri10ge_rx_done *rx_done = &ss->rx_done;
 	struct myri10ge_priv *mgp = ss->mgp;
-
 	unsigned long rx_bytes = 0;
 	unsigned long rx_packets = 0;
 	unsigned long rx_ok;
-
 	int idx = rx_done->idx;
 	int cnt = rx_done->cnt;
 	int work_done = 0;
@@ -1538,9 +1538,10 @@ static inline void myri10ge_check_statblock(struct myri10ge_priv *mgp)
 			} else {
 				if (netif_msg_link(mgp))
 					netdev_info(mgp->dev, "link %s\n",
-					    link_up == MXGEFW_LINK_MYRINET ?
-					    "mismatch (Myrinet detected)" :
-					    "down");
+						    (link_up ==
+						     MXGEFW_LINK_MYRINET ?
+						     "mismatch (Myrinet detected)"
+						     : "down"));
 				netif_carrier_off(mgp->dev);
 				mgp->link_changes++;
 			}
@@ -1621,7 +1622,7 @@ static irqreturn_t myri10ge_intr(int irq, void *arg)
 		if (send_done_count != tx->pkt_done)
 			myri10ge_tx_done(ss, (int)send_done_count);
 		if (unlikely(i > myri10ge_max_irq_loops)) {
-			netdev_err(mgp->dev, "irq stuck?\n");
+			netdev_warn(mgp->dev, "irq stuck?\n");
 			stats->valid = 0;
 			schedule_work(&mgp->watchdog_work);
 		}
@@ -1785,9 +1786,8 @@ static const char myri10ge_gstrings_slice_stats[][ETH_GSTRING_LEN] = {
 	"----------- slice ---------",
 	"tx_pkt_start", "tx_pkt_done", "tx_req", "tx_done",
 	"rx_small_cnt", "rx_big_cnt",
-	"wake_queue", "stop_queue", "tx_linearized", "LRO aggregated",
-	    "LRO flushed",
-	"LRO avg aggr", "LRO no_desc"
+	"wake_queue", "stop_queue", "tx_linearized",
+	"LRO aggregated", "LRO flushed", "LRO avg aggr", "LRO no_desc",
 };
 
 #define MYRI10GE_NET_STATS_LEN      21
@@ -3329,7 +3329,6 @@ abort:
 	/* fall back to using the unaligned firmware */
 	mgp->tx_boundary = 2048;
 	set_fw_name(mgp, myri10ge_fw_unaligned, false);
-
 }
 
 static void myri10ge_select_firmware(struct myri10ge_priv *mgp)
@@ -3715,8 +3714,8 @@ static void myri10ge_free_slices(struct myri10ge_priv *mgp)
 			dma_free_coherent(&pdev->dev, bytes,
 					  ss->fw_stats, ss->fw_stats_bus);
 			ss->fw_stats = NULL;
-			netif_napi_del(&ss->napi);
 		}
+		netif_napi_del(&ss->napi);
 	}
 	kfree(mgp->ss);
 	mgp->ss = NULL;
-- 
1.7.5.4

Re: [PATCH 7/9] myri10ge: misc style cleanups

[Joe Perches]

On Mon, 2011-06-27 at 10:05 -0500, Jon Mason wrote:
> Miscellaneous white space, style, and other cleanups
[]
> diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
[]
> @@ -1538,9 +1538,10 @@ static inline void myri10ge_check_statblock(struct myri10ge_priv *mgp)
>  			} else {
>  				if (netif_msg_link(mgp))
>  					netdev_info(mgp->dev, "link %s\n",
> -					    link_up == MXGEFW_LINK_MYRINET ?
> -					    "mismatch (Myrinet detected)" :
> -					    "down");
> +						    (link_up ==
> +						     MXGEFW_LINK_MYRINET ?
> +						     "mismatch (Myrinet detected)"
> +						     : "down"));

This could be:

				netif_info(mgp, link, mgp->dev, "link %s\n"
					   link_up == MXGEFW_LINK_MYRINET ?
					   "mismatch (Myrinet detected)" :
					   "down");

[PATCH 7/9 v2] myri10ge: misc style cleanups

[Jon Mason]

Miscellaneous white space, style, and other cleanups

v2 includes corrections from Joe Perches

Signed-off-by: Jon Mason <mason@myri.com>
---
 drivers/net/myri10ge/myri10ge.c |   32 ++++++++++++++------------------
 1 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index b9b80c0..90c8330 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -1086,6 +1086,9 @@ static int myri10ge_toggle_relaxed(struct pci_dev *pdev, int on)
 		return 0;
 
 	err = pci_read_config_word(pdev, cap + PCI_EXP_DEVCTL, &ctl);
+	if (err)
+		return 0;
+
 	ret = (ctl & PCI_EXP_DEVCTL_RELAX_EN) >> 4;
 	if (ret != on) {
 		ctl &= ~PCI_EXP_DEVCTL_RELAX_EN;
@@ -1140,20 +1143,19 @@ static void myri10ge_setup_dca(struct myri10ge_priv *mgp)
 		mgp->ss[i].cpu = -1;
 		mgp->ss[i].cached_dca_tag = -1;
 		myri10ge_update_dca(&mgp->ss[i]);
-	 }
+	}
 }
 
 static void myri10ge_teardown_dca(struct myri10ge_priv *mgp)
 {
 	struct pci_dev *pdev = mgp->pdev;
-	int err;
 
 	if (!mgp->dca_enabled)
 		return;
 	mgp->dca_enabled = 0;
 	if (mgp->relaxed_order)
 		myri10ge_toggle_relaxed(pdev, 1);
-	err = dca_remove_requester(&pdev->dev);
+	dca_remove_requester(&pdev->dev);
 }
 
 static int myri10ge_notify_dca_device(struct device *dev, void *data)
@@ -1314,7 +1316,7 @@ myri10ge_unmap_rx_page(struct pci_dev *pdev,
 
 static inline int
 myri10ge_rx_done(struct myri10ge_slice_state *ss, int len, __wsum csum,
-		 int lro_enabled)
+		 bool lro_enabled)
 {
 	struct myri10ge_priv *mgp = ss->mgp;
 	struct sk_buff *skb;
@@ -1474,11 +1476,9 @@ myri10ge_clean_rx_done(struct myri10ge_slice_state *ss, int budget)
 {
 	struct myri10ge_rx_done *rx_done = &ss->rx_done;
 	struct myri10ge_priv *mgp = ss->mgp;
-
 	unsigned long rx_bytes = 0;
 	unsigned long rx_packets = 0;
 	unsigned long rx_ok;
-
 	int idx = rx_done->idx;
 	int cnt = rx_done->cnt;
 	int work_done = 0;
@@ -1531,16 +1531,14 @@ static inline void myri10ge_check_statblock(struct myri10ge_priv *mgp)
 			mgp->link_state = link_up;
 
 			if (mgp->link_state == MXGEFW_LINK_UP) {
-				if (netif_msg_link(mgp))
-					netdev_info(mgp->dev, "link up\n");
+				netif_info(mgp, link, mgp->dev, "link up\n");
 				netif_carrier_on(mgp->dev);
 				mgp->link_changes++;
 			} else {
-				if (netif_msg_link(mgp))
-					netdev_info(mgp->dev, "link %s\n",
-					    link_up == MXGEFW_LINK_MYRINET ?
+				netif_info(mgp, link, mgp->dev, "link %s\n",
+					   (link_up == MXGEFW_LINK_MYRINET ?
 					    "mismatch (Myrinet detected)" :
-					    "down");
+					    "down"));
 				netif_carrier_off(mgp->dev);
 				mgp->link_changes++;
 			}
@@ -1621,7 +1619,7 @@ static irqreturn_t myri10ge_intr(int irq, void *arg)
 		if (send_done_count != tx->pkt_done)
 			myri10ge_tx_done(ss, (int)send_done_count);
 		if (unlikely(i > myri10ge_max_irq_loops)) {
-			netdev_err(mgp->dev, "irq stuck?\n");
+			netdev_warn(mgp->dev, "irq stuck?\n");
 			stats->valid = 0;
 			schedule_work(&mgp->watchdog_work);
 		}
@@ -1785,9 +1783,8 @@ static const char myri10ge_gstrings_slice_stats[][ETH_GSTRING_LEN] = {
 	"----------- slice ---------",
 	"tx_pkt_start", "tx_pkt_done", "tx_req", "tx_done",
 	"rx_small_cnt", "rx_big_cnt",
-	"wake_queue", "stop_queue", "tx_linearized", "LRO aggregated",
-	    "LRO flushed",
-	"LRO avg aggr", "LRO no_desc"
+	"wake_queue", "stop_queue", "tx_linearized",
+	"LRO aggregated", "LRO flushed", "LRO avg aggr", "LRO no_desc",
 };
 
 #define MYRI10GE_NET_STATS_LEN      21
@@ -3329,7 +3326,6 @@ abort:
 	/* fall back to using the unaligned firmware */
 	mgp->tx_boundary = 2048;
 	set_fw_name(mgp, myri10ge_fw_unaligned, false);
-
 }
 
 static void myri10ge_select_firmware(struct myri10ge_priv *mgp)
@@ -3715,8 +3711,8 @@ static void myri10ge_free_slices(struct myri10ge_priv *mgp)
 			dma_free_coherent(&pdev->dev, bytes,
 					  ss->fw_stats, ss->fw_stats_bus);
 			ss->fw_stats = NULL;
-			netif_napi_del(&ss->napi);
 		}
+		netif_napi_del(&ss->napi);
 	}
 	kfree(mgp->ss);
 	mgp->ss = NULL;
-- 
1.7.5.4

Re: [PATCH 7/9 v2] myri10ge: misc style cleanups

[David Miller]

From: Jon Mason <mason@myri.com>
Date: Mon, 27 Jun 2011 15:56:41 -0500

> Miscellaneous white space, style, and other cleanups
> 
> v2 includes corrections from Joe Perches
> 
> Signed-off-by: Jon Mason <mason@myri.com>

Applied.

[PATCH 8/9] myri10ge: update version

[Jon Mason]

Update version and copyright

Signed-off-by: Jon Mason <mason@myri.com>
---
 drivers/net/myri10ge/myri10ge.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index 1e1681b..aae5de1 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -1,7 +1,7 @@
 /*************************************************************************
  * myri10ge.c: Myricom Myri-10G Ethernet driver.
  *
- * Copyright (C) 2005 - 2009 Myricom, Inc.
+ * Copyright (C) 2005 - 2011 Myricom, Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -79,7 +79,7 @@
 #include "myri10ge_mcp.h"
 #include "myri10ge_mcp_gen_header.h"
 
-#define MYRI10GE_VERSION_STR "1.5.2-1.459"
+#define MYRI10GE_VERSION_STR "1.5.3-1.534"
 
 MODULE_DESCRIPTION("Myricom 10G driver (10GbE)");
 MODULE_AUTHOR("Maintainer: help@myri.com");
-- 
1.7.5.4

Re: [PATCH 8/9] myri10ge: update version

[David Miller]

From: Jon Mason <mason@myri.com>
Date: Mon, 27 Jun 2011 10:05:07 -0500

> Update version and copyright
> 
> Signed-off-by: Jon Mason <mason@myri.com>

Applied.

[PATCH 9/9] myri10ge: Update MAINTAINERS

[Jon Mason]

Update MAINTAINERS to refelect new people working on myri10ge

Signed-off-by: Jon Mason <mason@myri.com>
---
 MAINTAINERS |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index f0358cd..f3b5b65 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4279,8 +4279,8 @@ S:	Maintained
 F:	drivers/usb/musb/
 
 MYRICOM MYRI-10G 10GbE DRIVER (MYRI10GE)
+M:	Jon Mason <mason@myri.com>
 M:	Andrew Gallatin <gallatin@myri.com>
-M:	Brice Goglin <brice@myri.com>
 L:	netdev@vger.kernel.org
 W:	http://www.myri.com/scs/download-Myri10GE.html
 S:	Supported
-- 
1.7.5.4

Re: [PATCH 9/9] myri10ge: Update MAINTAINERS

[David Miller]

From: Jon Mason <mason@myri.com>
Date: Mon, 27 Jun 2011 10:05:08 -0500

> Update MAINTAINERS to refelect new people working on myri10ge
> 
> Signed-off-by: Jon Mason <mason@myri.com>

Applied.

Re: [PATCH 1/9] myri10ge: ensure tx queues remain stopped

[David Miller]

From: Jon Mason <mason@myri.com>
Date: Mon, 27 Jun 2011 10:05:00 -0500

> Ensure that our tx queues remain stopped when we stop them in
> myri10ge_close().  Not doing so can potentially lead to traffic being
> transmitted when the interface is removed, which can lead to NULL
> pointer dereferences.
> 
> Signed-off-by: Jon Mason <mason@myri.com>

Applied.