[PATCH] libertas: store rssi as an int

[PATCH] libertas: store rssi as an int

[Holger Schurig]

Don't store an long value in bss_descriptor, but
just an int.

Signed-off-by: Holger Schurig <hs4233@mail.mn-solutions.de>

---

This actually reduces the code size veeeeery slightly.

$ size libertas_old.ko libertas.ko
   text    data     bss     dec     hex filename
 116040    3724       8  119772   1d3dc libertas_old.ko
 116035    3724       8  119767   1d3d7 libertas.ko

Index: wireless-testing/drivers/net/wireless/libertas/scan.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/libertas/scan.h	
2008-03-18 14:00:34.000000000 +0100
+++ wireless-testing/drivers/net/wireless/libertas/scan.h	
2008-03-18 14:01:22.000000000 +0100
@@ -34,14 +34,9 @@ struct bss_descriptor {
 	u8 ssid_len;
 
 	u16 capability;
-
-	/* receive signal strength in dBm */
-	long rssi;
-
+	int rssi;
 	u32 channel;
-
 	u16 beaconperiod;
-
 	u32 atimwindow;
 
 	/* IW_MODE_AUTO, IW_MODE_ADHOC, IW_MODE_INFRA */
Index: wireless-testing/drivers/net/wireless/libertas/scan.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/libertas/scan.c	
2008-03-18 14:00:34.000000000 +0100
+++ wireless-testing/drivers/net/wireless/libertas/scan.c	
2008-03-18 14:01:22.000000000 +0100
@@ -602,7 +602,7 @@ static int lbs_scan_networks(struct lbs_
 	lbs_deb_scan("scan table:\n");
 	list_for_each_entry(iter, &priv->network_list, list)
 		lbs_deb_scan("%02d: BSSID %s, RSSI %d, SSID '%s'\n",
-			     i++, print_mac(mac, iter->bssid), (int)iter->rssi,
+			     i++, print_mac(mac, iter->bssid), iter->rssi,
 			     escape_essid(iter->ssid, iter->ssid_len));
 	mutex_unlock(&priv->lock);
 #endif
@@ -948,7 +948,7 @@ struct bss_descriptor *lbs_find_ssid_in_
 					     uint8_t *bssid, uint8_t mode,
 					     int channel)
 {
-	uint8_t bestrssi = 0;
+	int bestrssi = 0;
 	struct bss_descriptor * iter_bss = NULL;
 	struct bss_descriptor * found_bss = NULL;
 	struct bss_descriptor * tmp_oldest = NULL;
Index: wireless-testing/drivers/net/wireless/libertas/debugfs.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/libertas/debugfs.c	
2008-03-18 14:00:34.000000000 +0100
+++ wireless-testing/drivers/net/wireless/libertas/debugfs.c	
2008-03-18 14:01:22.000000000 +0100
@@ -78,7 +78,7 @@ static ssize_t lbs_getscantable(struct f
 		u16 spectrum_mgmt = (iter_bss->capability & 
WLAN_CAPABILITY_SPECTRUM_MGMT);
 
 		pos += snprintf(buf+pos, len-pos,
-			"%02u| %03d | %04ld | %s |",
+			"%02u| %03d | %04d | %s |",
 			numscansdone, iter_bss->channel, iter_bss->rssi,
 			print_mac(mac, iter_bss->bssid));
 		pos += snprintf(buf+pos, len-pos, " %04x-", 
iter_bss->capability);

Re: [PATCH] libertas: store rssi as an int

[Dan Williams]

On Tue, 2008-03-18 at 15:15 +0100, Holger Schurig wrote:
> Don't store an long value in bss_descriptor, but
> just an int.

RSSI as defined in 802.11 as a positive 8-bit value; the libertas
firmware also reports the RSSI in scan results as a u8.  So I'd suggest
a u8 instead of an int; an int isn't the right thing to do here.

Dan


> Signed-off-by: Holger Schurig <hs4233@mail.mn-solutions.de>
> 
> ---
> 
> This actually reduces the code size veeeeery slightly.
> 
> $ size libertas_old.ko libertas.ko
>    text    data     bss     dec     hex filename
>  116040    3724       8  119772   1d3dc libertas_old.ko
>  116035    3724       8  119767   1d3d7 libertas.ko
> 
> Index: wireless-testing/drivers/net/wireless/libertas/scan.h
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/libertas/scan.h	
> 2008-03-18 14:00:34.000000000 +0100
> +++ wireless-testing/drivers/net/wireless/libertas/scan.h	
> 2008-03-18 14:01:22.000000000 +0100
> @@ -34,14 +34,9 @@ struct bss_descriptor {
>  	u8 ssid_len;
>  
>  	u16 capability;
> -
> -	/* receive signal strength in dBm */
> -	long rssi;
> -
> +	int rssi;
>  	u32 channel;
> -
>  	u16 beaconperiod;
> -
>  	u32 atimwindow;
>  
>  	/* IW_MODE_AUTO, IW_MODE_ADHOC, IW_MODE_INFRA */
> Index: wireless-testing/drivers/net/wireless/libertas/scan.c
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/libertas/scan.c	
> 2008-03-18 14:00:34.000000000 +0100
> +++ wireless-testing/drivers/net/wireless/libertas/scan.c	
> 2008-03-18 14:01:22.000000000 +0100
> @@ -602,7 +602,7 @@ static int lbs_scan_networks(struct lbs_
>  	lbs_deb_scan("scan table:\n");
>  	list_for_each_entry(iter, &priv->network_list, list)
>  		lbs_deb_scan("%02d: BSSID %s, RSSI %d, SSID '%s'\n",
> -			     i++, print_mac(mac, iter->bssid), (int)iter->rssi,
> +			     i++, print_mac(mac, iter->bssid), iter->rssi,
>  			     escape_essid(iter->ssid, iter->ssid_len));
>  	mutex_unlock(&priv->lock);
>  #endif
> @@ -948,7 +948,7 @@ struct bss_descriptor *lbs_find_ssid_in_
>  					     uint8_t *bssid, uint8_t mode,
>  					     int channel)
>  {
> -	uint8_t bestrssi = 0;
> +	int bestrssi = 0;
>  	struct bss_descriptor * iter_bss = NULL;
>  	struct bss_descriptor * found_bss = NULL;
>  	struct bss_descriptor * tmp_oldest = NULL;
> Index: wireless-testing/drivers/net/wireless/libertas/debugfs.c
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/libertas/debugfs.c	
> 2008-03-18 14:00:34.000000000 +0100
> +++ wireless-testing/drivers/net/wireless/libertas/debugfs.c	
> 2008-03-18 14:01:22.000000000 +0100
> @@ -78,7 +78,7 @@ static ssize_t lbs_getscantable(struct f
>  		u16 spectrum_mgmt = (iter_bss->capability & 
> WLAN_CAPABILITY_SPECTRUM_MGMT);
>  
>  		pos += snprintf(buf+pos, len-pos,
> -			"%02u| %03d | %04ld | %s |",
> +			"%02u| %03d | %04d | %s |",
>  			numscansdone, iter_bss->channel, iter_bss->rssi,
>  			print_mac(mac, iter_bss->bssid));
>  		pos += snprintf(buf+pos, len-pos, " %04x-", 
> iter_bss->capability);

Re: [PATCH] libertas: store rssi as an int

[Holger Schurig]

> RSSI as defined in 802.11 as a positive 8-bit value; the
> libertas firmware also reports the RSSI in scan results as a
> u8.  So I'd suggest a u8 instead of an int; an int isn't the
> right thing to do here.

"struct bss_descriptor" has nothing to do with hardware, 
otherwise you would see __le32 there. Using an "u8" saves us 
nothing, except we move all u8's at the end of the struct. But 
this would give us unaligned u8's and some processors have 
problems with that.

Re: [PATCH] libertas: store rssi as an int

[Dan Williams]

On Wed, 2008-03-19 at 16:20 +0100, Holger Schurig wrote:
> > RSSI as defined in 802.11 as a positive 8-bit value; the
> > libertas firmware also reports the RSSI in scan results as a
> > u8.  So I'd suggest a u8 instead of an int; an int isn't the
> > right thing to do here.
> 
> "struct bss_descriptor" has nothing to do with hardware, 
> otherwise you would see __le32 there. Using an "u8" saves us 
> nothing, except we move all u8's at the end of the struct. But 
> this would give us unaligned u8's and some processors have 
> problems with that.

I still don't want an int used for a value that should never be
unsigned...  I don't particularly care what size it is as long as its
unsigned.  Otherwise, we should be calling it RSSI at all.  Either it's
unsigned+RSSI or signed+dBm, but in the signed+dBm case we'd have to
actually _convert_ it to dBm too.

Dan

Re: [PATCH] libertas: store rssi as an int

[Holger Schurig]

> I still don't want an int used for a value that should never
> be unsigned...

okay, "unsigned int" then :-)   I'm just glad that "long" was as
signedness as "int" ...

I'm just making a "take" for this.

Re: [PATCH, take 2] libertas: store rssi as an u32

[Holger Schurig]

[PATCH] store rssi as an int

Don't store an (hardware base) u8 value in bss_descriptor, but just an
unsigned int (RSSI is really unsigned). Compilers generate more efficent
code for ints than for bytes.

Signed-off-by: Holger Schurig <hs4233@mail.mn-solutions.de>

Index: wireless-testing/drivers/net/wireless/libertas/scan.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/libertas/scan.h	2008-03-19 09:04:43.000000000 +0100
+++ wireless-testing/drivers/net/wireless/libertas/scan.h	2008-03-19 09:05:53.000000000 +0100
@@ -34,14 +34,9 @@ struct bss_descriptor {
 	u8 ssid_len;
 
 	u16 capability;
-
-	/* receive signal strength in dBm */
-	long rssi;
-
+	u32 rssi;
 	u32 channel;
-
 	u16 beaconperiod;
-
 	u32 atimwindow;
 
 	/* IW_MODE_AUTO, IW_MODE_ADHOC, IW_MODE_INFRA */
Index: wireless-testing/drivers/net/wireless/libertas/scan.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/libertas/scan.c	2008-03-19 09:05:48.000000000 +0100
+++ wireless-testing/drivers/net/wireless/libertas/scan.c	2008-03-19 09:27:55.000000000 +0100
@@ -602,7 +602,7 @@ static int lbs_scan_networks(struct lbs_
 	lbs_deb_scan("scan table:\n");
 	list_for_each_entry(iter, &priv->network_list, list)
 		lbs_deb_scan("%02d: BSSID %s, RSSI %d, SSID '%s'\n",
-			     i++, print_mac(mac, iter->bssid), (int)iter->rssi,
+			     i++, print_mac(mac, iter->bssid), iter->rssi,
 			     escape_essid(iter->ssid, iter->ssid_len));
 	mutex_unlock(&priv->lock);
 #endif
@@ -948,7 +948,7 @@ struct bss_descriptor *lbs_find_ssid_in_
 					     uint8_t *bssid, uint8_t mode,
 					     int channel)
 {
-	uint8_t bestrssi = 0;
+	u32 bestrssi = 0;
 	struct bss_descriptor * iter_bss = NULL;
 	struct bss_descriptor * found_bss = NULL;
 	struct bss_descriptor * tmp_oldest = NULL;
Index: wireless-testing/drivers/net/wireless/libertas/debugfs.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/libertas/debugfs.c	2008-03-19 09:04:43.000000000 +0100
+++ wireless-testing/drivers/net/wireless/libertas/debugfs.c	2008-03-19 09:05:53.000000000 +0100
@@ -78,7 +78,7 @@ static ssize_t lbs_getscantable(struct f
 		u16 spectrum_mgmt = (iter_bss->capability & WLAN_CAPABILITY_SPECTRUM_MGMT);
 
 		pos += snprintf(buf+pos, len-pos,
-			"%02u| %03d | %04ld | %s |",
+			"%02u| %03d | %04d | %s |",
 			numscansdone, iter_bss->channel, iter_bss->rssi,
 			print_mac(mac, iter_bss->bssid));
 		pos += snprintf(buf+pos, len-pos, " %04x-", iter_bss->capability);

Re: [PATCH, take 2] libertas: store rssi as an u32

[Dan Williams]

On Wed, 2008-03-19 at 17:08 +0100, Holger Schurig wrote:
> [PATCH] store rssi as an int
> 
> Don't store an (hardware base) u8 value in bss_descriptor, but just an
> unsigned int (RSSI is really unsigned). Compilers generate more efficent
> code for ints than for bytes.
> 
> Signed-off-by: Holger Schurig <hs4233@mail.mn-solutions.de>

Acked-by: Dan Williams <dcbw@redhat.com>

> Index: wireless-testing/drivers/net/wireless/libertas/scan.h
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/libertas/scan.h	2008-03-19 09:04:43.000000000 +0100
> +++ wireless-testing/drivers/net/wireless/libertas/scan.h	2008-03-19 09:05:53.000000000 +0100
> @@ -34,14 +34,9 @@ struct bss_descriptor {
>  	u8 ssid_len;
>  
>  	u16 capability;
> -
> -	/* receive signal strength in dBm */
> -	long rssi;
> -
> +	u32 rssi;
>  	u32 channel;
> -
>  	u16 beaconperiod;
> -
>  	u32 atimwindow;
>  
>  	/* IW_MODE_AUTO, IW_MODE_ADHOC, IW_MODE_INFRA */
> Index: wireless-testing/drivers/net/wireless/libertas/scan.c
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/libertas/scan.c	2008-03-19 09:05:48.000000000 +0100
> +++ wireless-testing/drivers/net/wireless/libertas/scan.c	2008-03-19 09:27:55.000000000 +0100
> @@ -602,7 +602,7 @@ static int lbs_scan_networks(struct lbs_
>  	lbs_deb_scan("scan table:\n");
>  	list_for_each_entry(iter, &priv->network_list, list)
>  		lbs_deb_scan("%02d: BSSID %s, RSSI %d, SSID '%s'\n",
> -			     i++, print_mac(mac, iter->bssid), (int)iter->rssi,
> +			     i++, print_mac(mac, iter->bssid), iter->rssi,
>  			     escape_essid(iter->ssid, iter->ssid_len));
>  	mutex_unlock(&priv->lock);
>  #endif
> @@ -948,7 +948,7 @@ struct bss_descriptor *lbs_find_ssid_in_
>  					     uint8_t *bssid, uint8_t mode,
>  					     int channel)
>  {
> -	uint8_t bestrssi = 0;
> +	u32 bestrssi = 0;
>  	struct bss_descriptor * iter_bss = NULL;
>  	struct bss_descriptor * found_bss = NULL;
>  	struct bss_descriptor * tmp_oldest = NULL;
> Index: wireless-testing/drivers/net/wireless/libertas/debugfs.c
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/libertas/debugfs.c	2008-03-19 09:04:43.000000000 +0100
> +++ wireless-testing/drivers/net/wireless/libertas/debugfs.c	2008-03-19 09:05:53.000000000 +0100
> @@ -78,7 +78,7 @@ static ssize_t lbs_getscantable(struct f
>  		u16 spectrum_mgmt = (iter_bss->capability & WLAN_CAPABILITY_SPECTRUM_MGMT);
>  
>  		pos += snprintf(buf+pos, len-pos,
> -			"%02u| %03d | %04ld | %s |",
> +			"%02u| %03d | %04d | %s |",
>  			numscansdone, iter_bss->channel, iter_bss->rssi,
>  			print_mac(mac, iter_bss->bssid));
>  		pos += snprintf(buf+pos, len-pos, " %04x-", iter_bss->capability);