[PATCH] airo: make debug-like messages printed by airo_print_dbg()

[PATCH] airo: make debug-like messages printed by airo_print_dbg()

[Robert Schulze]

I don't think that messages like

airo(eth1): cmd= 111

airo(eth1): status= 7f11

airo(eth1): Rsp0= 2

airo(eth1): Rsp1= 0

airo(eth1): Rsp2= 0

are interesting to normal users, so switch them from airo_print_err() to airo_print_dbg().
Furthermore, remove implicit linefeed in those messages, as they are handled by airo_print().

with kind regards,
Robert Schulze

--- linux-2.6.17.1/drivers/net/wireless/airo.c.orig	2006-07-08 14:02:12.000000000 +0200
+++ linux-2.6.17.1/drivers/net/wireless/airo.c	2006-07-08 17:36:08.000000000 +0200
@@ -3897,11 +3897,11 @@ static u16 issuecommand(struct airo_info
  	pRsp->rsp1 = IN4500(ai, RESP1);
  	pRsp->rsp2 = IN4500(ai, RESP2);
  	if ((pRsp->status & 0xff00)!=0 && pCmd->cmd != CMD_SOFTRESET) {
-		airo_print_err(ai->dev->name, "cmd= %x\n", pCmd->cmd);
-		airo_print_err(ai->dev->name, "status= %x\n", pRsp->status);
-		airo_print_err(ai->dev->name, "Rsp0= %x\n", pRsp->rsp0);
-		airo_print_err(ai->dev->name, "Rsp1= %x\n", pRsp->rsp1);
-		airo_print_err(ai->dev->name, "Rsp2= %x\n", pRsp->rsp2);
+		airo_print_dbg(ai->dev->name, "cmd= %x", pCmd->cmd);
+		airo_print_dbg(ai->dev->name, "status= %x", pRsp->status);
+		airo_print_dbg(ai->dev->name, "Rsp0= %x", pRsp->rsp0);
+		airo_print_dbg(ai->dev->name, "Rsp1= %x", pRsp->rsp1);
+		airo_print_dbg(ai->dev->name, "Rsp2= %x", pRsp->rsp2);
  	}

  	// clear stuck command busy if necessary

Re: [PATCH] airo: make debug-like messages printed by airo_print_dbg()

[Dan Williams]

On Sat, 2006-07-08 at 17:59 +0200, Robert Schulze wrote:
> I don't think that messages like
> 
> airo(eth1): cmd= 111
> 
> airo(eth1): status= 7f11
> 
> airo(eth1): Rsp0= 2
> 
> airo(eth1): Rsp1= 0
> 
> airo(eth1): Rsp2= 0
> 
> are interesting to normal users, so switch them from airo_print_err() to airo_print_dbg().
> Furthermore, remove implicit linefeed in those messages, as they are handled by airo_print().

+1.  If you wanted to get a bit more creative, maybe collapse all of
these onto a single line to print something like:

airo(eth1): Command didn't complete. cmd:0x111 status:0x7f11 (Rsp: 2, 0,
0)

This message will only happen if the card hangs up and stops responding
to commands anyway, so we don't necessarily care about making the
message decipherable to anyone other than developers.

Dan

> with kind regards,
> Robert Schulze
> 
> --- linux-2.6.17.1/drivers/net/wireless/airo.c.orig	2006-07-08 14:02:12.000000000 +0200
> +++ linux-2.6.17.1/drivers/net/wireless/airo.c	2006-07-08 17:36:08.000000000 +0200
> @@ -3897,11 +3897,11 @@ static u16 issuecommand(struct airo_info
>   	pRsp->rsp1 = IN4500(ai, RESP1);
>   	pRsp->rsp2 = IN4500(ai, RESP2);
>   	if ((pRsp->status & 0xff00)!=0 && pCmd->cmd != CMD_SOFTRESET) {
> -		airo_print_err(ai->dev->name, "cmd= %x\n", pCmd->cmd);
> -		airo_print_err(ai->dev->name, "status= %x\n", pRsp->status);
> -		airo_print_err(ai->dev->name, "Rsp0= %x\n", pRsp->rsp0);
> -		airo_print_err(ai->dev->name, "Rsp1= %x\n", pRsp->rsp1);
> -		airo_print_err(ai->dev->name, "Rsp2= %x\n", pRsp->rsp2);
> +		airo_print_dbg(ai->dev->name, "cmd= %x", pCmd->cmd);
> +		airo_print_dbg(ai->dev->name, "status= %x", pRsp->status);
> +		airo_print_dbg(ai->dev->name, "Rsp0= %x", pRsp->rsp0);
> +		airo_print_dbg(ai->dev->name, "Rsp1= %x", pRsp->rsp1);
> +		airo_print_dbg(ai->dev->name, "Rsp2= %x", pRsp->rsp2);
>   	}
> 
>   	// clear stuck command busy if necessary
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] airo: make debug-like messages printed by airo_print_dbg()

[Robert Schulze]

Hi,

Dan Williams schrieb:
 > This message will only happen if the card hangs up and stops responding
 > to commands anyway, so we don't necessarily care about making the
 > message decipherable to anyone other than developers.

Well, I get this message each time I insert my Cisco Aironet 350 PCMCIA card, which works obviously fine.
On card removal, I get:

airo(eth1): cmd= 21

airo(eth1): status= ffff

airo(eth1): Rsp0= ffff

airo(eth1): Rsp1= ffff

airo(eth1): Rsp2= ffff

four(!) times.

Besides, the messages can be read by issuing dmesg even after the patch, so no information gets lost.


with kind regards,

Robert Schulze

Re: [PATCH] airo: make debug-like messages printed by airo_print_dbg()

[Michael Tokarev]

Robert Schulze wrote:
> Hi,
> 
> Dan Williams schrieb:
>> This message will only happen if the card hangs up and stops responding
>> to commands anyway, so we don't necessarily care about making the
>> message decipherable to anyone other than developers.
> 
> Well, I get this message each time I insert my Cisco Aironet 350 PCMCIA
> card, which works obviously fine.
[]
> Besides, the messages can be read by issuing dmesg even after the patch,
> so no information gets lost.

The fact that you're getting that message indicates that something's wrong,
at least from the kernel's point of view.  So it better be understand and
fixed, instead of being hidden in debugging output.  If it's visible in
dmesg but isn't visible in syslog (default syslog configuration does not
capture any debugging messages), far less people will notice it.

I'd vote for making it one-line, but with current KERN_ERR priority.

/mjt (who don't even have the hardware in question)

Re: [PATCH] airo: make debug-like messages printed by airo_print_dbg()

[Robert Schulze]

Hi,

ok lets keep that message to be visible to anybody, but put it into one line.
Thanks for your comments.
Now, I will track down what causes the error on my machine, I think it has something todo with pcmcia...

with kind regards,

Robert Schulze

--- linux-2.6.17.1/drivers/net/wireless/airo.c.orig	2006-07-08 14:02:12.000000000 +0200
+++ linux-2.6.17.1/drivers/net/wireless/airo.c	2006-07-10 00:19:15.000000000 +0200
@@ -3897,11 +3897,10 @@ static u16 issuecommand(struct airo_info
  	pRsp->rsp1 = IN4500(ai, RESP1);
  	pRsp->rsp2 = IN4500(ai, RESP2);
  	if ((pRsp->status & 0xff00)!=0 && pCmd->cmd != CMD_SOFTRESET) {
-		airo_print_err(ai->dev->name, "cmd= %x\n", pCmd->cmd);
-		airo_print_err(ai->dev->name, "status= %x\n", pRsp->status);
-		airo_print_err(ai->dev->name, "Rsp0= %x\n", pRsp->rsp0);
-		airo_print_err(ai->dev->name, "Rsp1= %x\n", pRsp->rsp1);
-		airo_print_err(ai->dev->name, "Rsp2= %x\n", pRsp->rsp2);
+		airo_print_err(ai->dev->name,
+			"cmd:%x status:%x rsp0:%x rsp1:%x rsp2:%x"
+			pCmd->cmd, pRsp->status, pRsp->rsp0, pRsp->rsp1,
+			pRsp->rsp2);
  	}

  	// clear stuck command busy if necessary

Re: [PATCH] airo: make debug-like messages printed by airo_print_dbg()

[Dan Williams]

On Mon, 2006-07-10 at 00:23 +0200, Robert Schulze wrote:
> Hi,
> 
> ok lets keep that message to be visible to anybody, but put it into one line.
> Thanks for your comments.
> Now, I will track down what causes the error on my machine, I think it has something todo with pcmcia...

Looks good.  This will happen any time the driver tries to talk to the
card but can't.  So you might want to try looking at whether the card is
already pulled out or its resources have been deallocated before this
code runs.  Obviously we can talk to the card up until it's been pulled
out and the kernel notifies us that the card has been unplugged, but
perhaps there's a codepath that tries to mistakenly handle both
hot-unplug and module removal by writing something to that card (like
reset commands).  I've seen that before in other places.

Dan

> with kind regards,
> 
> Robert Schulze
> 
> --- linux-2.6.17.1/drivers/net/wireless/airo.c.orig	2006-07-08 14:02:12.000000000 +0200
> +++ linux-2.6.17.1/drivers/net/wireless/airo.c	2006-07-10 00:19:15.000000000 +0200
> @@ -3897,11 +3897,10 @@ static u16 issuecommand(struct airo_info
>   	pRsp->rsp1 = IN4500(ai, RESP1);
>   	pRsp->rsp2 = IN4500(ai, RESP2);
>   	if ((pRsp->status & 0xff00)!=0 && pCmd->cmd != CMD_SOFTRESET) {
> -		airo_print_err(ai->dev->name, "cmd= %x\n", pCmd->cmd);
> -		airo_print_err(ai->dev->name, "status= %x\n", pRsp->status);
> -		airo_print_err(ai->dev->name, "Rsp0= %x\n", pRsp->rsp0);
> -		airo_print_err(ai->dev->name, "Rsp1= %x\n", pRsp->rsp1);
> -		airo_print_err(ai->dev->name, "Rsp2= %x\n", pRsp->rsp2);
> +		airo_print_err(ai->dev->name,
> +			"cmd:%x status:%x rsp0:%x rsp1:%x rsp2:%x"
> +			pCmd->cmd, pRsp->status, pRsp->rsp0, pRsp->rsp1,
> +			pRsp->rsp2);
>   	}
> 
>   	// clear stuck command busy if necessary

Re: [PATCH] airo: make debug-like messages printed by airo_print_dbg()

[Robert Schulze]

Hi,

I regret making a mistake yesterday, I forgot a comma.
Here now is the working patch.

[PATCH] airo: collapse debugging-messages in issuecommand to one line

Robert Schulze


--- linux-2.6.17.1/drivers/net/wireless/airo.c.orig	2006-07-08 14:02:12.000000000 +0200
+++ linux-2.6.17.1/drivers/net/wireless/airo.c	2006-07-10 18:30:30.000000000 +0200
@@ -3897,11 +3897,10 @@ static u16 issuecommand(struct airo_info
  	pRsp->rsp1 = IN4500(ai, RESP1);
  	pRsp->rsp2 = IN4500(ai, RESP2);
  	if ((pRsp->status & 0xff00)!=0 && pCmd->cmd != CMD_SOFTRESET) {
-		airo_print_err(ai->dev->name, "cmd= %x\n", pCmd->cmd);
-		airo_print_err(ai->dev->name, "status= %x\n", pRsp->status);
-		airo_print_err(ai->dev->name, "Rsp0= %x\n", pRsp->rsp0);
-		airo_print_err(ai->dev->name, "Rsp1= %x\n", pRsp->rsp1);
-		airo_print_err(ai->dev->name, "Rsp2= %x\n", pRsp->rsp2);
+		airo_print_err(ai->dev->name,
+			"cmd:%x status:%x rsp0:%x rsp1:%x rsp2:%x",
+			pCmd->cmd, pRsp->status, pRsp->rsp0, pRsp->rsp1,
+			pRsp->rsp2);
  	}

  	// clear stuck command busy if necessary