First patch to mrv8k driver?

PATCH John Daiker 4 participants 5 messages May 14, 2008 Networking drivers (wireless)

Share & tools

John Daiker

author

2008-05-14 00:59 UTC #0 lore raw

Hey All,

My first patch for the mrv8k driver. My machine locks up when I try to load a firmware, so I'm not sure if this patch helps anybody or not.

John Daiker

Some formatting fixes, as well as a fwe mrv_reg_read32 returnvalue changes

Signed-off-by John Daiker <[email protected]>

---
 drivers/net/wireless/mrv8k/mrv8k.c |   23 +++++++++++++++--------
 1 files changed, 15 insertions(+), 8 deletions(-)

drivers/net/wireless/mrv8k/mrv8k.c +15 -8

index a86ea9f..88497a1 100644
@@ -241,8 +241,9 @@ static int mrv_upload_fw(struct mrv_priv *priv)
 
 	_mrv_send_cmd(priv, MRV_HWMEM_CMD_BOOT);
 
-	/* the boot code is stupid enought that it does not report any status when it
-	 * loads correctly and become alive. Wait 1 second..*/
+	/* The boot code is stupid enough that it does not report any status
+	 * when it loads correctly and become alive. Wait 1 second...
+	 */
 	msleep(1000);
 
 	err = request_firmware(&img, FW_FIRMWARE_FILENAME, &priv->pdev->dev);

@@ -260,12 +261,15 @@ static int mrv_upload_fw(struct mrv_priv *priv)
 		memcpy(cmd_data, img->data + i, 256);
 		mrv_send_cmd(priv);
 		for (j = 0; j < 500; j++) {
-			if (mrv_reg_read32(priv, MRV_REG_STATUS) == 5)
-			  break;
+			if (mrv_reg_read32(priv, MRV_REG_STATUS) == 0x5)
+				break;
 			mdelay(1);
 		}
+		/* We waited for 500 msecs and the card never went to the
+		 * magical 'Loading firmware' state
+		 */
 		if (j == 500) {
-			printk(KERN_ERR"Boot FW has failed to transfer real FW\n");
+			printk(KERN_ERR"Boot Firmware has failed to transfer a real Firmware\n");
 			return -2;
 		}
 	}

@@ -278,12 +282,15 @@ static int mrv_upload_fw(struct mrv_priv *priv)
 		memcpy(cmd_data, img->data + i, img->size - i);
 		mrv_send_cmd(priv);
 		for (j = 0; j < 500; j++) {
-			if (mrv_reg_read32(priv, MRV_REG_STATUS) == 5)
-			  break;
+			if (mrv_reg_read32(priv, MRV_REG_STATUS) == 0x5)
+				break;
 			mdelay(1);
 		}
+		/* We waited for 500 msecs and the card never transfered the
+		 * last portion of the firmware... return an error
+		 */
 		if (j == 500) {
-			printk(KERN_ERR"Boot FW has failed to transfer last real FW chunk\n");
+			printk(KERN_ERR"Boot Firmware has failed to transfer the last real Firmware chunk\n");
 			return -2;
 		}
 	}

Pavel Roskin

2008-05-14 04:37 UTC #1 lore raw

Quoted 4 lines · John Daiker

Hey All,
My first patch for the mrv8k driver. My machine locks up when I try to load a firmware, so I'm not sure if this patch helps anybody or not.

I don't see any changes for mrv_reg_read32() claimed in the description. In fact, I don't see any changes to the code except comments and messages. I don't see how it can help against lockups, unless you missed some part of the patch.

You may want to spell check your patch, including the description, especially if the patch is fixing spelling mistakes made by others.

Signature · 2 lines

Regards,
Pavel Roskin

Markus Becker

2008-05-14 13:08 UTC #2 lore raw

Quoted 7 lines · Pavel Roskin

Quoted 4 lines · John Daiker
Hey All,
My first patch for the mrv8k driver. My machine locks up when I try to load a firmware, so I'm not sure if this patch helps anybody or not.
I don't see any changes for mrv_reg_read32() claimed in the description.

He is checking the return value to 0x5 instead of 5.

Quoted 3 lines

In fact, I don't see any changes to the code except comments and messages. I don't see how it can help against lockups, unless you missed some part of the patch.

It does not. But I pushed the comment and indent fixes to mrv8k nevertheless.

Quoted 11 lines

You may want to spell check your patch, including the description, especially if the patch is fixing spelling mistakes made by others.
Signature · 6 lines
Regards,
Pavel Roskin
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Johannes Berg

maintainer

2008-05-14 13:22 UTC #3 lore raw

Quoted 11 lines · Markus Becker

Quoted 7 lines · Pavel Roskin
Quoted 4 lines · John Daiker
Hey All,
My first patch for the mrv8k driver. My machine locks up when I try to load a firmware, so I'm not sure if this patch helps anybody or not.
I don't see any changes for mrv_reg_read32() claimed in the description.
He is checking the return value to 0x5 instead of 5.

You realise, of course, that those are the same number?

johannes

John Daiker

author

2008-05-14 17:56 UTC #4 lore raw

In reply to Markus Becker

Quoted 4 lines · Markus Becker

Quoted 1 line · Pavel Roskin
On Tue, 2008-05-13 at 17:59 -0700, John Daiker wrote:

Quoted 8 lines

Quoted 6 lines
Quoted 4 lines
Hey All,
My first patch for the mrv8k driver. My machine locks up when I try to load a firmware, so I'm not sure if this patch helps anybody or not.
I don't see any changes for mrv_reg_read32() claimed in the description.
He is checking the return value to 0x5 instead of 5.

Right... the aim here is for some sort of code consistency. Makes things easier to read, IMHO.

Quoted 6 lines

Quoted 4 lines
In fact, I don't see any changes to the code except comments and messages. I don't see how it can help against lockups, unless you missed some part of the patch.
It does not. But I pushed the comment and indent fixes to mrv8k nevertheless.

Thanks. I guess I mentioned that to solicit input from others on ideas for debugging.

Quoted 3 lines

Quoted 3 lines
You may want to spell check your patch, including the description, especially if the patch is fixing spelling mistakes made by others.

Duly noted. I will keep an keen eye out next time for mistakes. :)

Quoted 9 lines

Signature · 7 lines

-- 
Regards,
Pavel Roskin
-- 
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html