Skip to contentSkip to conversation

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

PATCH In review Robert Schulze 3 participants 7 messages Jul 8, 2006 – Jul 10, 2006

Subsystems (3)
Networking Network drivers Networking drivers (wireless)
lore
Share & tools
lore

Robert Schulze

author
#0 lore raw

PATCH 1/1 airo: make debug-like messages printed by airo_print_dbg()

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

drivers/net/wireless/airo.c renamed from drivers/net/wireless/airo.c.orig +5 -5
@@ -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
#1 lore raw
Quoted 6 lines · Robert Schulze

I don't think that messages like

airo(eth1): cmd= 111

airo(eth1): status= 7f11

Quoted 8 lines

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

Quoted 26 lines

with kind regards,
Robert Schulze

drivers/net/wireless/airo.c renamed from drivers/net/wireless/airo.c.orig +5 -6
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#2 lore raw

Hi,

Quoted 3 lines · Dan Williams

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

#3 lore raw
Quoted 9 lines · Robert Schulze

Hi,

Quoted 3 lines · Dan Williams

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.

[]

Quoted 2 lines

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)

author
#4 lore raw

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

drivers/net/wireless/airo.c renamed from drivers/net/wireless/airo.c.orig +4 -5
@@ -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
#5 lore raw
Quoted 5 lines · 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...

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

Quoted 22 lines

with kind regards,

Robert Schulze

drivers/net/wireless/airo.c renamed from drivers/net/wireless/airo.c.orig +4 -5
@@ -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
author
#6 lore raw
In reply to Michael Tokarev

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

drivers/net/wireless/airo.c renamed from drivers/net/wireless/airo.c.orig +4 -5
@@ -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