[PATCH 1/5] bnx2: Protect tx timeout reset with rtnl_lock().

[PATCH 1/5] bnx2: Protect tx timeout reset with rtnl_lock().

[Michael Chan]

To prevent race conditions with other reset events.

Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/bnx2.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 4cae2a8..03eb51f 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -6263,8 +6263,11 @@ bnx2_reset_task(struct work_struct *work)
 {
 	struct bnx2 *bp = container_of(work, struct bnx2, reset_task);
 
-	if (!netif_running(bp->dev))
+	rtnl_lock();
+	if (!netif_running(bp->dev)) {
+		rtnl_unlock();
 		return;
+	}
 
 	bnx2_netif_stop(bp);
 
@@ -6272,6 +6275,7 @@ bnx2_reset_task(struct work_struct *work)
 
 	atomic_set(&bp->intr_sem, 1);
 	bnx2_netif_start(bp);
+	rtnl_unlock();
 }
 
 static void
-- 
1.6.4.GIT

[PATCH 2/5] bnx2: Dump some state during tx timeout.

[Michael Chan]

To help debug the problem.

Signed-off-by: Michael Chan <mchan@broadcom.com>
Signed-off-by: Benjamin Li <benli@broadcom.com>
---
 drivers/net/bnx2.c |   23 +++++++++++++++++++++++
 drivers/net/bnx2.h |    2 ++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 03eb51f..665253a 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -6279,10 +6279,33 @@ bnx2_reset_task(struct work_struct *work)
 }
 
 static void
+bnx2_dump_state(struct bnx2 *bp)
+{
+	struct net_device *dev = bp->dev;
+
+	printk(KERN_ERR PFX "%s DEBUG: intr_sem[%x]\n", dev->name,
+		atomic_read(&bp->intr_sem));
+	printk(KERN_ERR PFX "%s DEBUG: EMAC_TX_STATUS[%08x] "
+			    "RPM_MGMT_PKT_CTRL[%08x]\n", dev->name,
+		REG_RD(bp, BNX2_EMAC_TX_STATUS),
+		REG_RD(bp, BNX2_RPM_MGMT_PKT_CTRL));
+	printk(KERN_ERR PFX "%s DEBUG: MCP_STATE_P0[%08x] MCP_STATE_P1[%08x]\n",
+		dev->name, bnx2_reg_rd_ind(bp, BNX2_MCP_STATE_P0),
+		bnx2_reg_rd_ind(bp, BNX2_MCP_STATE_P1));
+	printk(KERN_ERR PFX "%s DEBUG: HC_STATS_INTERRUPT_STATUS[%08x]\n",
+		dev->name, REG_RD(bp, BNX2_HC_STATS_INTERRUPT_STATUS));
+	if (bp->flags & BNX2_FLAG_USING_MSIX)
+		printk(KERN_ERR PFX "%s DEBUG: PBA[%08x]\n", dev->name,
+			REG_RD(bp, BNX2_PCI_GRC_WINDOW3_BASE));
+}
+
+static void
 bnx2_tx_timeout(struct net_device *dev)
 {
 	struct bnx2 *bp = netdev_priv(dev);
 
+	bnx2_dump_state(bp);
+
 	/* This allows the netif to be shutdown gracefully before resetting */
 	schedule_work(&bp->reset_task);
 }
diff --git a/drivers/net/bnx2.h b/drivers/net/bnx2.h
index 4908b9f..939dc44 100644
--- a/drivers/net/bnx2.h
+++ b/drivers/net/bnx2.h
@@ -6345,6 +6345,8 @@ struct l2_fhdr {
 
 #define BNX2_MCP_ROM					0x00150000
 #define BNX2_MCP_SCRATCH				0x00160000
+#define BNX2_MCP_STATE_P1				 0x0016f9c8
+#define BNX2_MCP_STATE_P0				 0x0016fdc8
 
 #define BNX2_SHM_HDR_SIGNATURE				BNX2_MCP_SCRATCH
 #define BNX2_SHM_HDR_SIGNATURE_SIG_MASK			 0xffff0000
-- 
1.6.4.GIT

Re: [PATCH 2/5] bnx2: Dump some state during tx timeout.

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Thu, 3 Dec 2009 11:46:32 -0800

> To help debug the problem.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> Signed-off-by: Benjamin Li <benli@broadcom.com>

Applied.

[PATCH 3/5] bnx2: Print warning when unable to allocate the full SKB/page ring.

[Michael Chan]

Signed-off-by: Michael Chan <mchan@broadcom.com>
Signed-off-by: Benjamin Li <benli@broadcom.com>
---
 drivers/net/bnx2.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 665253a..dba3840 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -5158,8 +5158,12 @@ bnx2_init_rx_ring(struct bnx2 *bp, int ring_num)
 
 	ring_prod = prod = rxr->rx_pg_prod;
 	for (i = 0; i < bp->rx_pg_ring_size; i++) {
-		if (bnx2_alloc_rx_page(bp, rxr, ring_prod) < 0)
+		if (bnx2_alloc_rx_page(bp, rxr, ring_prod) < 0) {
+			printk(KERN_WARNING PFX "%s: init'ed rx page ring %d "
+						"with %d/%d pages only\n",
+			       bp->dev->name, ring_num, i, bp->rx_pg_ring_size);
 			break;
+		}
 		prod = NEXT_RX_BD(prod);
 		ring_prod = RX_PG_RING_IDX(prod);
 	}
@@ -5167,8 +5171,12 @@ bnx2_init_rx_ring(struct bnx2 *bp, int ring_num)
 
 	ring_prod = prod = rxr->rx_prod;
 	for (i = 0; i < bp->rx_ring_size; i++) {
-		if (bnx2_alloc_rx_skb(bp, rxr, ring_prod) < 0)
+		if (bnx2_alloc_rx_skb(bp, rxr, ring_prod) < 0) {
+			printk(KERN_WARNING PFX "%s: init'ed rx ring %d with "
+						"%d/%d skbs only\n",
+			       bp->dev->name, ring_num, i, bp->rx_ring_size);
 			break;
+		}
 		prod = NEXT_RX_BD(prod);
 		ring_prod = RX_RING_IDX(prod);
 	}
-- 
1.6.4.GIT

Re: [PATCH 3/5] bnx2: Print warning when unable to allocate the full SKB/page ring.

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Thu, 3 Dec 2009 11:46:33 -0800

> Signed-off-by: Michael Chan <mchan@broadcom.com>
> Signed-off-by: Benjamin Li <benli@broadcom.com>

Applied, but I would rate limit this thing.

It could especially be problematic if this condition is
triggered when the device is being used for netconsole.

Re: [PATCH 3/5] bnx2: Print warning when unable to allocate the full SKB/page ring.

[Michael Chan]

On Thu, 2009-12-03 at 13:08 -0800, David Miller wrote:
> Applied, but I would rate limit this thing.
> 
> It could especially be problematic if this condition is
> triggered when the device is being used for netconsole.
> 

We only do the printk() during init. time if we cannot allocate buffers
for the whole ring.  printk() is not used at all during run-time.
Thanks.

Re: [PATCH 3/5] bnx2: Print warning when unable to allocate the full SKB/page ring.

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Thu, 3 Dec 2009 13:49:20 -0800

> 
> On Thu, 2009-12-03 at 13:08 -0800, David Miller wrote:
>> Applied, but I would rate limit this thing.
>> 
>> It could especially be problematic if this condition is
>> triggered when the device is being used for netconsole.
>> 
> 
> We only do the printk() during init. time if we cannot allocate buffers
> for the whole ring.  printk() is not used at all during run-time.

Thanks for clearing that up.  I mistakedly thought this was for
the buffers not the ring.

[PATCH 4/5] bnx2: Read firmware version from VPD.

[Michael Chan]

And display it through ethtool -i.

Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/bnx2.c |  102 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 99 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index dba3840..b7bc74b 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -7722,6 +7722,93 @@ bnx2_get_pci_speed(struct bnx2 *bp)
 
 }
 
+static void __devinit
+bnx2_read_vpd_fw_ver(struct bnx2 *bp)
+{
+	int rc, i, v0_len = 0;
+	u8 *data;
+	u8 *v0_str = NULL;
+	bool mn_match = false;
+
+#define BNX2_MAX_VER_SLEN	30
+
+	data = kmalloc(256, GFP_KERNEL);
+	if (!data)
+		return;
+
+	rc = bnx2_nvram_read(bp, 0x300, data + 128, 128);
+	if (rc)
+		goto vpd_done;
+
+	for (i = 0; i < 128; i += 4) {
+		data[i] = data[i + 131];
+		data[i + 1] = data[i + 130];
+		data[i + 2] = data[i + 129];
+		data[i + 3] = data[i + 128];
+	}
+
+	for (i = 0; i < 128; ) {
+		unsigned char val = data[i];
+		unsigned int block_end;
+
+		if (val == 0x82 || val == 0x91) {
+			i = (i + 3 + (data[i + 1] + (data[i + 2] << 8)));
+			continue;
+		}
+
+		if (val != 0x90)
+			goto vpd_done;
+
+		block_end = (i + 3 + (data[i + 1] + (data[i + 2] << 8)));
+		i += 3;
+
+		if (block_end > 128)
+			goto vpd_done;
+
+		while (i < (block_end - 2)) {
+			if (data[i] == 'M' && data[i + 1] == 'N') {
+				int mn_len = data[i + 2];
+
+				if (mn_len != 4)
+					goto vpd_done;
+
+				i += 3;
+				if (memcmp(&data[i], "1028", 4))
+					goto vpd_done;
+				mn_match = true;
+				i += 4;
+
+			} else if (data[i] == 'V' && data[i + 1] == '0') {
+				v0_len = data[i + 2];
+
+				i += 3;
+				if (v0_len > BNX2_MAX_VER_SLEN ||
+				    (v0_len + i) > 128)
+					goto vpd_done;
+
+				if (v0_len > BNX2_MAX_VER_SLEN)
+					v0_len = BNX2_MAX_VER_SLEN;
+
+				v0_str = &data[i];
+				i += data[i + 2];
+
+			} else {
+				i += 3 + data[i + 2];
+			}
+
+			if (mn_match && v0_str) {
+				memcpy(bp->fw_version, v0_str, v0_len);
+				bp->fw_version[v0_len] = ' ';
+				goto vpd_done;
+			}
+		}
+		goto vpd_done;
+	}
+
+vpd_done:
+	kfree(data);
+}
+
 static int __devinit
 bnx2_init_board(struct pci_dev *pdev, struct net_device *dev)
 {
@@ -7895,10 +7982,18 @@ bnx2_init_board(struct pci_dev *pdev, struct net_device *dev)
 		goto err_out_unmap;
 	}
 
+	bnx2_read_vpd_fw_ver(bp);
+
+	j = strlen(bp->fw_version);
 	reg = bnx2_shmem_rd(bp, BNX2_DEV_INFO_BC_REV);
-	for (i = 0, j = 0; i < 3; i++) {
+	for (i = 0; i < 3 && j < 24; i++) {
 		u8 num, k, skip0;
 
+		if (i == 0) {
+			bp->fw_version[j++] = 'b';
+			bp->fw_version[j++] = 'c';
+			bp->fw_version[j++] = ' ';
+		}
 		num = (u8) (reg >> (24 - (i * 8)));
 		for (k = 100, skip0 = 1; k >= 1; num %= k, k /= 10) {
 			if (num >= k || !skip0 || k == 1) {
@@ -7929,8 +8024,9 @@ bnx2_init_board(struct pci_dev *pdev, struct net_device *dev)
 	    reg != BNX2_CONDITION_MFW_RUN_NONE) {
 		u32 addr = bnx2_shmem_rd(bp, BNX2_MFW_VER_PTR);
 
-		bp->fw_version[j++] = ' ';
-		for (i = 0; i < 3; i++) {
+		if (j < 32)
+			bp->fw_version[j++] = ' ';
+		for (i = 0; i < 3 && j < 28; i++) {
 			reg = bnx2_reg_rd_ind(bp, addr + i * 4);
 			reg = swab32(reg);
 			memcpy(&bp->fw_version[j], &reg, 4);
-- 
1.6.4.GIT

Re: [PATCH 4/5] bnx2: Read firmware version from VPD.

[Ben Hutchings]

On Thu, 2009-12-03 at 11:46 -0800, Michael Chan wrote:
> And display it through ethtool -i.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> ---
>  drivers/net/bnx2.c |  102 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 99 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
> index dba3840..b7bc74b 100644
> --- a/drivers/net/bnx2.c
> +++ b/drivers/net/bnx2.c
> @@ -7722,6 +7722,93 @@ bnx2_get_pci_speed(struct bnx2 *bp)
>  
>  }
>  
> +static void __devinit
> +bnx2_read_vpd_fw_ver(struct bnx2 *bp)
> +{
> +	int rc, i, v0_len = 0;
> +	u8 *data;
> +	u8 *v0_str = NULL;
> +	bool mn_match = false;
> +
> +#define BNX2_MAX_VER_SLEN	30

And how about:

#define BNX2_VPD_LEN		128

and then using that instead of all the magic numbers below...

> +	data = kmalloc(256, GFP_KERNEL);
> +	if (!data)
> +		return;
> +
> +	rc = bnx2_nvram_read(bp, 0x300, data + 128, 128);
> +	if (rc)
> +		goto vpd_done;
> +
> +	for (i = 0; i < 128; i += 4) {
> +		data[i] = data[i + 131];
> +		data[i + 1] = data[i + 130];
> +		data[i + 2] = data[i + 129];
> +		data[i + 3] = data[i + 128];
> +	}

Is this correct for both big-endian and little-endian architectures?

> +	for (i = 0; i < 128; ) {

Allowing for the size of a long tag, that should be:

	for (i = 0; i <= BNX2_VPD_LEN - 3; ) {

> +		unsigned char val = data[i];
> +		unsigned int block_end;
> +
> +		if (val == 0x82 || val == 0x91) {
> +			i = (i + 3 + (data[i + 1] + (data[i + 2] << 8)));
> +			continue;
> +		}
> +
> +		if (val != 0x90)
> +			goto vpd_done;
> +
> +		block_end = (i + 3 + (data[i + 1] + (data[i + 2] << 8)));

Perhaps these VPD tag values should go in pci_regs.h.  And there's
really no need to repeat the length calculation.

> +		i += 3;
> +
> +		if (block_end > 128)
> +			goto vpd_done;
> +
> +		while (i < (block_end - 2)) {
> +			if (data[i] == 'M' && data[i + 1] == 'N') {
> +				int mn_len = data[i + 2];
> +
> +				if (mn_len != 4)
> +					goto vpd_done;
> +
> +				i += 3;
> +				if (memcmp(&data[i], "1028", 4))
> +					goto vpd_done;
> +				mn_match = true;
> +				i += 4;
> +
> +			} else if (data[i] == 'V' && data[i + 1] == '0') {
> +				v0_len = data[i + 2];
> +
> +				i += 3;
> +				if (v0_len > BNX2_MAX_VER_SLEN ||
> +				    (v0_len + i) > 128)
> +					goto vpd_done;
> +
> +				if (v0_len > BNX2_MAX_VER_SLEN)
> +					v0_len = BNX2_MAX_VER_SLEN;
> +
> +				v0_str = &data[i];
> +				i += data[i + 2];

This last statement is reading the length from the wrong byte since i
has already been updated.

> +			} else {
> +				i += 3 + data[i + 2];
> +			}
[...]

It would make more sense to read and validate length just once:

		while (i < (block_end - 2)) {
			int len = data[i + 2];

			if (i + 3 + len > block_end)
				goto vpd_done;

			if (data[i] == 'M' && data[i + 1] == 'N') {
				if (len != 4 ||
				    memcmp(&data[i + 3], "1028", 4))
					goto vpd_done;
				mn_match = true;
			} else if (data[i] == 'V' && data[i + 1] == '0') {
				if (len > BNX2_MAX_VER_SLEN)
					goto vpd_done;
				v0_str = &data[i + 3];
				v0_len = len;
			}

			i += 3 + len;
			...

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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 4/5] bnx2: Read firmware version from VPD.

[Michael Chan]

On Thu, 2009-12-03 at 12:17 -0800, Ben Hutchings wrote:
> > +     for (i = 0; i < 128; i += 4) {
> > +             data[i] = data[i + 131];
> > +             data[i + 1] = data[i + 130];
> > +             data[i + 2] = data[i + 129];
> > +             data[i + 3] = data[i + 128];
> > +     }
> 
> Is this correct for both big-endian and little-endian architectures?

Yes, the nvram function returns data in a byte-stream that is the same
in little or big endian.

> Perhaps these VPD tag values should go in pci_regs.h.

How about we do this later and convert other drivers at the same time?

I will send out a fixup patch shortly incorporating all your other
suggestions.  Thanks.

Re: [PATCH 4/5] bnx2: Read firmware version from VPD.

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Thu, 3 Dec 2009 11:46:34 -0800

> And display it through ethtool -i.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>

Applied, but please address Ben's feedback.

Thanks.

[PATCH 5/5] bnx2: Update version to 2.0.3.

[Michael Chan]

Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/bnx2.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index b7bc74b..cf4322d 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -59,8 +59,8 @@
 
 #define DRV_MODULE_NAME		"bnx2"
 #define PFX DRV_MODULE_NAME	": "
-#define DRV_MODULE_VERSION	"2.0.2"
-#define DRV_MODULE_RELDATE	"Aug 21, 2009"
+#define DRV_MODULE_VERSION	"2.0.3"
+#define DRV_MODULE_RELDATE	"Dec 03, 2009"
 #define FW_MIPS_FILE_06		"bnx2/bnx2-mips-06-5.0.0.j3.fw"
 #define FW_RV2P_FILE_06		"bnx2/bnx2-rv2p-06-5.0.0.j3.fw"
 #define FW_MIPS_FILE_09		"bnx2/bnx2-mips-09-5.0.0.j3.fw"
-- 
1.6.4.GIT

Re: [PATCH 5/5] bnx2: Update version to 2.0.3.

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Thu, 3 Dec 2009 11:46:35 -0800

> Signed-off-by: Michael Chan <mchan@broadcom.com>

Applied.

Re: [PATCH 1/5] bnx2: Protect tx timeout reset with rtnl_lock().

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Thu, 3 Dec 2009 11:46:31 -0800

> To prevent race conditions with other reset events.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>

Applied.