First patch to mrv8k driver?

First patch to mrv8k driver?

[John Daiker]

[-- Attachment #1: Type: text/plain, Size: 167 bytes --]

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



[-- Attachment #2: formatting.diff --]
[-- Type: text/x-diff, Size: 2194 bytes --]

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

Signed-off-by: John Daiker <daikerjohn@gmail.com>
---
 drivers/net/wireless/mrv8k/mrv8k.c |   23 +++++++++++++++--------
 1 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/mrv8k/mrv8k.c b/drivers/net/wireless/mrv8k/mrv8k.c
index a86ea9f..88497a1 100644
--- a/drivers/net/wireless/mrv8k/mrv8k.c
+++ b/drivers/net/wireless/mrv8k/mrv8k.c
@@ -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;
 		}
 	}

Re: First patch to mrv8k driver?

[Pavel Roskin]

On Tue, 2008-05-13 at 17:59 -0700, John Daiker wrote:
> 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.

-- 
Regards,
Pavel Roskin

Re: First patch to mrv8k driver?

[Markus Becker]

On Wed, 14 May 2008, Pavel Roskin wrote:

> On Tue, 2008-05-13 at 17:59 -0700, John Daiker wrote:
>> 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.

> 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.

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

Re: First patch to mrv8k driver?

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 552 bytes --]

On Wed, 2008-05-14 at 15:08 +0200, Markus Becker wrote:
> On Wed, 14 May 2008, Pavel Roskin wrote:
> 
> > On Tue, 2008-05-13 at 17:59 -0700, John Daiker wrote:
> >> 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: First patch to mrv8k driver?

[John Daiker]

Markus Becker wrote:
>
> On Wed, 14 May 2008, Pavel Roskin wrote:
>
>> On Tue, 2008-05-13 at 17:59 -0700, John Daiker wrote:
>>> 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.
>
>> 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.
>
>> 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. :)
>>
>> -- 
>> Regards,
>> Pavel Roskin
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe 
>> linux-wireless" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>