Add wireless statics to bcm43xx-d80211

PATCH In review Larry Finger 3 participants 5 messages Aug 14, 2006 – Aug 23, 2006

Subsystems (3)

Networking Network drivers Networking drivers (wireless)

lore

Share & tools

lore

Larry Finger

author

2006-08-14 13:29 UTC #0 lore raw

PATCH 1/1 Add wireless statics to bcm43xx-d80211

This patch implements wireless statistics for bcm43xx using the d80211 stack.It also sets a framework for the implementation in other drivers that use the d80211 code. The component parts have been circulated on the netdev mailing list, and all suggested changes have been incorporated. The specific changes are as follows:

1. Add new variables to struct ieee80211_rx_status to contain the latest values for signal, noise, and the maximum value of the received ssi. These could be u8's, but I left them as integers in case other drivers needed larger values.

2. Add new variables to struct ieee80211_local to contain the link_quality and noise.Again these could be u8's for bcm43xx.

3. Add last_signal, last_noise and max_rssi to struct sta_info.

4. Remove link_quality and add link_noise to struct bcm43xx_stats.

5. Remove the bogus calculation of link_quality and save the value of link_noise in the new variable bcm->stats.link_noise.

6. In bcm43xx_xmit.c, store the new variables in status for transmission to ieee80211.

7. In ieee80211_rx_h_sta_process, transfer the new status variables into struct sta_info.

8. Add the appropriate range parameters and routine ieee80211_get_wireless_stats to ieee80211_ioctl.c.

The patch is submitted as an attachment to preserve white space.

Signed-off-by Larry Finger <Larry.Finger.lwfinger.net>

drivers/net/wireless/d80211/bcm43xx/bcm43xx.h +3 -1

index a0faeca..75174f9 100644
@@ -27,6 +27,8 @@ #define BCM43xx_IRQWAIT_MAX_RETRIES	50
 
 #define BCM43xx_IO_SIZE			8192
 
+#define BCM43xx_RX_MAX_SSI		60
+
 /* Active Core PCI Configuration Register. */
 #define BCM43xx_PCICFG_ACTIVE_CORE	0x80
 /* SPROM control register. */

@@ -623,7 +625,7 @@ struct bcm43xx_noise_calculation {
 };
 
 struct bcm43xx_stats {
-	u8 link_quality;
+	u8 link_noise;
 	/* Store the last TX/RX times here for updating the leds. */
 	unsigned long last_tx;
 	unsigned long last_rx;

drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c +1 -9

index 6366020..7f0e0f6 100644
@@ -1506,15 +1506,7 @@ static void handle_irq_noise(struct bcm4
 		else
 			average -= 48;
 
-		if (average > -65)
-			bcm->stats.link_quality = 0;
-		else if (average > -75)
-			bcm->stats.link_quality = 1;
-		else if (average > -85)
-			bcm->stats.link_quality = 2;
-		else
-			bcm->stats.link_quality = 3;
-//		dprintk(KERN_INFO PFX "Link Quality: %u (avg was %d)\n", bcm->stats.link_quality, average);
+		bcm->stats.link_noise = average;
 drop_calculation:
 		bcm->noisecalc.calculation_running = 0;
 		return;

drivers/net/wireless/d80211/bcm43xx/bcm43xx_xmit.c +4 -1

index 3f86079..90776b2 100644
@@ -475,7 +475,10 @@ void bcm43xx_rx(struct bcm43xx_private *
 	 */
 
 	memset(&status, 0, sizeof(status));
-	status.ssi = bcm43xx_rssi_postprocess(bcm, rxhdr->rssi, is_ofdm,
+	status.noise = bcm->stats.link_noise;
+	status.ssi = rxhdr->rssi;
+	status.maxssi = BCM43xx_RX_MAX_SSI;
+	status.signal = bcm43xx_rssi_postprocess(bcm, rxhdr->rssi, is_ofdm,
 					      !!(rxflags1 & BCM43xx_RXHDR_FLAGS1_2053RSSIADJ),
 					      !!(rxflags3 & BCM43xx_RXHDR_FLAGS3_2050RSSIADJ));
 	if (is_ofdm)

include/net/d80211.h +3 -0

index ba5cb4c..8479b58 100644
@@ -205,6 +205,9 @@ struct ieee80211_rx_status {
         int channel;
         int phymode;
         int ssi;
+	int maxssi;
+	int signal;
+	int noise;
         int antenna;
         int rate;
         int flag;

net/d80211/ieee80211.c +3 -0

index 11804d5..0eef4a3 100644
@@ -3174,6 +3174,9 @@ ieee80211_rx_h_sta_process(struct ieee80
 	sta->rx_fragments++;
 	sta->rx_bytes += rx->skb->len;
 	sta->last_rssi = rx->u.rx.status->ssi;
+	sta->last_signal = rx->u.rx.status->signal;
+	sta->last_noise = rx->u.rx.status->noise;
+	sta->max_rssi = rx->u.rx.status->maxssi;
 
 	if (!(rx->fc & IEEE80211_FCTL_MOREFRAGS)) {
 		/* Change STA power saving mode only in the end of a frame

net/d80211/ieee80211_i.h +3 -0

index bdaaf5e..5e0ec45 100644
@@ -337,6 +337,9 @@ struct ieee80211_local {
 	struct net_device *apdev; /* wlan#ap - management frames (hostapd) */
 	int open_count;
 	int monitors;
+	int link_quality;
+	int noise;
+	struct iw_statistics wstats;
 	struct ieee80211_conf conf;
 
 	int dev_index;

net/d80211/ieee80211_ioctl.c +44 -0

62 lines not on this page · raw message

net/d80211/sta_info.h +3 -0

index 70334e5..4d4169e 100644
@@ -82,6 +82,9 @@ struct sta_info {
 	unsigned long rx_dropped; /* number of dropped MPDUs from this STA */
 
 	int last_rssi; /* RSSI of last received frame from this STA */
+	int last_signal; /* signal of last received frame from this STA */
+	int last_noise; /* noise of last received frame from this STA */
+	int max_rssi; /* maximum value of RSSI for this STA */
 	int last_ack_rssi[3]; /* RSSI of last received ACKs from this STA */
 	unsigned long last_ack;
         int channel_use;

Jiri Benc

2006-08-22 14:09 UTC #1 lore raw

Quoted 5 lines · Larry Finger

This patch implements wireless statistics for bcm43xx using the d80211 stack.It also sets a framework for the implementation in other drivers that use the d80211 code. The component parts have been circulated on the netdev mailing list, and all suggested changes have been incorporated. The specific changes are as follows:

Please, separate the d80211 part and the bcm43xx-d80211 part into two patches.

include/net/d80211.h struct ieee80211_rx_status { · line 208

Quoted 7 lines

include/net/d80211.h +1 -0

@@ -205,6 +205,9 @@ struct ieee80211_rx_status {
         int channel;
         int phymode;
         int ssi;
+	int maxssi;

Why is maxssi here? Can it really change between received frames?

net/d80211/ieee80211.c ieee80211_rx_h_sta_process(struct ieee80 · line 3179

Quoted 10 lines

[...]

net/d80211/ieee80211.c +3 -0

@@ -3174,6 +3174,9 @@ ieee80211_rx_h_sta_process(struct ieee80
 	sta->rx_fragments++;
 	sta->rx_bytes += rx->skb->len;
 	sta->last_rssi = rx->u.rx.status->ssi;
+	sta->last_signal = rx->u.rx.status->signal;
+	sta->last_noise = rx->u.rx.status->noise;
+	sta->max_rssi = rx->u.rx.status->maxssi;

Again, I see no reason why max_rssi should be in sta structure.

net/d80211/ieee80211_i.h struct ieee80211_local { · line 342

Quoted 10 lines

[...]

net/d80211/ieee80211_i.h +3 -0

@@ -337,6 +337,9 @@ struct ieee80211_local {
 	struct net_device *apdev; /* wlan#ap - management frames (hostapd) */
 	int open_count;
 	int monitors;
+	int link_quality;
+	int noise;
+	struct iw_statistics wstats;

Why are these three variables in ieee80211_local? They are not used anywhere.

net/d80211/ieee80211_ioctl.c static int ieee80211_ioctl_giwrange(stru · line 1584

Quoted 9 lines

[...]

net/d80211/ieee80211_ioctl.c +2 -0

@@ -1580,6 +1580,16 @@ static int ieee80211_ioctl_giwrange(stru
 	range->min_frag = 256;
 	range->max_frag = 2346;
 
+	range->max_qual.qual = 100;
+	range->max_qual.level = 152;  /* set floor at -104 dBm (152 - 256) */

I would suggest using -110 dBm as a floor (to be compatible with RCPI definition, see mail from Simon Barber describing it). Or is there any particular reason for -104 dBm?

net/d80211/sta_info.h struct sta_info { · line 86

Quoted 9 lines

[...]

net/d80211/sta_info.h +2 -0

@@ -82,6 +82,9 @@ struct sta_info {
 	unsigned long rx_dropped; /* number of dropped MPDUs from this STA */
 
 	int last_rssi; /* RSSI of last received frame from this STA */
+	int last_signal; /* signal of last received frame from this STA */
+	int last_noise; /* noise of last received frame from this STA */

Add these two variables also to sysfs, please.

Thanks,

Jiri

Signature · 2 lines

Jiri Benc
SUSE Labs

Larry Finger

author

2006-08-22 21:15 UTC #2 lore raw

Quoted 5 lines · Jiri Benc

Quoted 1 line · Larry Finger
This patch implements wireless statistics for bcm43xx using the d80211 stack.It also sets a framework for the implementation in other drivers that use the d80211 code. The component parts have been circulated on the netdev mailing list, and all suggested changes have been incorporated. The specific changes are as follows:
Please, separate the d80211 part and the bcm43xx-d80211 part into two patches.

OK. I split it into parts for the initial RFC submission, then combined them after I thought the comments were all in. Sorry.

Quoted 9 lines

include/net/d80211.h struct ieee80211_rx_status { · line 208
Quoted 7 lines
include/net/d80211.h +1 -0
@@ -205,6 +205,9 @@ struct ieee80211_rx_status {
         int channel;
         int phymode;
         int ssi;
+	int maxssi;
Why is maxssi here? Can it really change between received frames?

No it cannot change between frames; however, the max value can be very different for different drivers using d80211. On the bcm43xx, it is 60; whereas 100 seems to be a better value for the rt2x00 chips. Adding it here seemed like a good way to handle this situation. Do you suggest something else?

Quoted 4 lines

Quoted 4 lines
[...]
net/d80211/ieee80211.c +0 -0
@@ -3174,6 +3174,9 @@ ieee80211_rx_h_sta_process(struct ieee80

Quoted 8 lines

net/d80211/ieee80211.c ieee80211_rx_h_sta_process(struct ieee80 · line 3179
Quoted 6 lines
net/d80211/ieee80211.c +3 -0
 	sta->rx_fragments++;
 	sta->rx_bytes += rx->skb->len;
 	sta->last_rssi = rx->u.rx.status->ssi;
+	sta->last_signal = rx->u.rx.status->signal;
+	sta->last_noise = rx->u.rx.status->noise;
+	sta->max_rssi = rx->u.rx.status->maxssi;
Again, I see no reason why max_rssi should be in sta structure.

Again to pass the differing values for different drivers.

Quoted 5 lines

[...]

net/d80211/ieee80211_i.h +0 -0

@@ -337,6 +337,9 @@ struct ieee80211_local {
 	struct net_device *apdev; /* wlan#ap - management frames (hostapd) */

Quoted 8 lines

net/d80211/ieee80211_i.h struct ieee80211_local { · line 342
Quoted 5 lines
net/d80211/ieee80211_i.h +3 -0
 	int open_count;
 	int monitors;
+	int link_quality;
+	int noise;
+	struct iw_statistics wstats;
Why are these three variables in ieee80211_local? They are not used anywhere.

You are right about the first two; however, wstats is used in the new routine ieee80211_get_wireless_stats.

Quoted 7 lines

[...]

net/d80211/ieee80211_ioctl.c +0 -0

@@ -1580,6 +1580,16 @@ static int ieee80211_ioctl_giwrange(stru
 	range->min_frag = 256;
 	range->max_frag = 2346;

Quoted 6 lines

net/d80211/ieee80211_ioctl.c static int ieee80211_ioctl_giwrange(stru · line 1584
Quoted 2 lines
net/d80211/ieee80211_ioctl.c +2 -0
+	range->max_qual.qual = 100;
+	range->max_qual.level = 152;  /* set floor at -104 dBm (152 - 256) */
I would suggest using -110 dBm as a floor (to be compatible with RCPI definition, see mail from Simon Barber describing it). Or is there any particular reason for -104 dBm?

It is the value previously used in the softmac version of bcm43xx. A value of -110 would obviously be better.

Quoted 11 lines

net/d80211/sta_info.h struct sta_info { · line 86

Quoted 9 lines

[...]

net/d80211/sta_info.h +2 -0

@@ -82,6 +82,9 @@ struct sta_info {
 	unsigned long rx_dropped; /* number of dropped MPDUs from this STA */
 
 	int last_rssi; /* RSSI of last received frame from this STA */
+	int last_signal; /* signal of last received frame from this STA */
+	int last_noise; /* noise of last received frame from this STA */

Add these two variables also to sysfs, please.

Will be done.

Larry

Johannes Berg

maintainer

2006-08-23 07:53 UTC #3 lore raw

Quoted 8 lines · Larry Finger

Quoted 3 lines
include/net/d80211.h struct ieee80211_rx_status { · line 208
Quoted 1 line
include/net/d80211.h +1 -0
+	int maxssi;
Why is maxssi here? Can it really change between received frames?
No it cannot change between frames; however, the max value can be very different for different drivers using d80211. On the bcm43xx, it is 60; whereas 100 seems to be a better value for the rt2x00 chips. Adding it here seemed like a good way to handle this situation. Do you suggest something else?

I think the question was intended to be: "why is it in ieee80211_rx_status and not in struct ieee80211_hw?"

Quoted 1 line

Again to pass the differing values for different drivers.

Ditto here. Just stick it into struct ieee80211_hw instead.

Quoted 6 lines

Quoted 3 lines
I would suggest using -110 dBm as a floor (to be compatible with RCPI definition, see mail from Simon Barber describing it). Or is there any particular reason for -104 dBm?
It is the value previously used in the softmac version of bcm43xx. A value of -110 would obviously be better.

Who maintains the softmac version now? :P
I'd suggest to just change it there too for consistency.

johannes

Jiri Benc

2006-08-23 10:01 UTC #4 lore raw

In reply to Larry Finger

Quoted 4 lines · Larry Finger

No it cannot change between frames; however, the max value can be very different for different drivers using d80211. On the bcm43xx, it is 60; whereas 100 seems to be a better value for the rt2x00 chips. Adding it here seemed like a good way to handle this situation. Do you suggest something else?

As Johannes already said, it should be in ieee80211_hw.

Quoted 14 lines

Quoted 5 lines
Quoted 5 lines
[...]
net/d80211/ieee80211_i.h +0 -0
@@ -337,6 +337,9 @@ struct ieee80211_local {
 	struct net_device *apdev; /* wlan#ap - management frames (hostapd) */
Quoted 8 lines
net/d80211/ieee80211_i.h struct ieee80211_local { · line 342
Quoted 5 lines
net/d80211/ieee80211_i.h +3 -0
 	int open_count;
 	int monitors;
+	int link_quality;
+	int noise;
+	struct iw_statistics wstats;
Why are these three variables in ieee80211_local? They are not used anywhere.

Quoted 2 lines

You are right about the first two; however, wstats is used in the new routine ieee80211_get_wireless_stats.

Oh, get_wireless_stats returns struct iw_statistics, so it needs to be allocated all the time. Grr, that's stupid...

Okay, wstats really needs to be in ieee80211_local then.

Thanks,

Jiri

Signature · 2 lines

Jiri Benc
SUSE Labs