[PATCH] bcm43xx-d80211: measure the channel change time

[PATCH] bcm43xx-d80211: measure the channel change time

[Michael Buesch]

Measure the channel change time with the
bcm43xx tsf timer and remove the guesswork constant. ;)

Tests on my 4306 show that the time comes damn
close to reality.

Signed-off-by: Michael Buesch <mb@bu3sch.de>

Index: wireless-dev/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c	2006-05-03 18:12:27.000000000 +0200
+++ wireless-dev/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c	2006-05-03 20:51:24.000000000 +0200
@@ -354,6 +354,33 @@
 	bcm43xx_write32(bcm, BCM43xx_MMIO_STATUS_BITFIELD, status);
 }
 
+static void bcm43xx_measure_channel_change_time(struct bcm43xx_private *bcm)
+{
+	struct bcm43xx_radioinfo *radio;
+	u64 start, stop;
+	unsigned long flags;
+	u8 oldchan, testchan;
+
+	/* We (ab)use the bcm43xx TSF timer to measure the time needed
+	 * to switch channels. This information is handed over to
+	 * the ieee80211 subsystem.
+	 * Time is measured in microseconds.
+	 */
+
+	bcm43xx_lock_mmio(bcm, flags);
+	radio = bcm43xx_current_radio(bcm);
+	oldchan = radio->channel;
+	testchan = (oldchan == 6) ? 7 : 6;
+	bcm43xx_tsf_read(bcm, &start);
+	bcm43xx_radio_selectchannel(bcm, testchan, 0);
+	bcm43xx_tsf_read(bcm, &stop);
+	bcm43xx_radio_selectchannel(bcm, oldchan, 0);
+	bcm43xx_unlock_mmio(bcm, flags);
+
+	assert(stop > start);
+	bcm->ieee->channel_change_time = stop - start;
+}
+
 static
 void bcm43xx_macfilter_set(struct bcm43xx_private *bcm,
 			   u16 offset,
@@ -3706,6 +3733,7 @@
 	dprintk(KERN_INFO PFX "80211 cores initialized\n");
 	bcm43xx_setup_modes(bcm);
 	bcm43xx_security_init(bcm);
+	bcm43xx_measure_channel_change_time(bcm);
 	ieee80211_update_hw(bcm->net_dev, bcm->ieee);
 	ieee80211_netif_oper(bcm->net_dev, NETIF_ATTACH);
 	ieee80211_netif_oper(bcm->net_dev, NETIF_START);
@@ -4329,7 +4357,6 @@
 	ieee->host_gen_beacon = 1;
 	ieee->rx_includes_fcs = 1;
 	ieee->monitor_during_oper = 1;
-	ieee->channel_change_time = 20000;
 	ieee->tx = bcm43xx_net_hard_start_xmit;
 	ieee->open = bcm43xx_net_open;
 	ieee->stop = bcm43xx_net_stop;

-- 
Greetings Michael.

Re: [PATCH] bcm43xx-d80211: measure the channel change time

[Jiri Benc]

On Wed, 3 May 2006 21:05:53 +0200, Michael Buesch wrote:
> @@ -354,6 +354,33 @@
>  	bcm43xx_write32(bcm, BCM43xx_MMIO_STATUS_BITFIELD, status);
>  }
>  
> +static void bcm43xx_measure_channel_change_time(struct bcm43xx_private *bcm)
> +{
> +	struct bcm43xx_radioinfo *radio;
> +	u64 start, stop;
> +	unsigned long flags;
> +	u8 oldchan, testchan;
> +
> +	/* We (ab)use the bcm43xx TSF timer to measure the time needed
> +	 * to switch channels. This information is handed over to
> +	 * the ieee80211 subsystem.
> +	 * Time is measured in microseconds.
> +	 */
> +
> +	bcm43xx_lock_mmio(bcm, flags);
> +	radio = bcm43xx_current_radio(bcm);
> +	oldchan = radio->channel;
> +	testchan = (oldchan == 6) ? 7 : 6;
> +	bcm43xx_tsf_read(bcm, &start);
> +	bcm43xx_radio_selectchannel(bcm, testchan, 0);
> +	bcm43xx_tsf_read(bcm, &stop);
> +	bcm43xx_radio_selectchannel(bcm, oldchan, 0);
> +	bcm43xx_unlock_mmio(bcm, flags);
> +
> +	assert(stop > start);

I think some safety fallback would be better. Like
if (stop - start < SOME_MIN_VALUE) channel_change_time = SOME_MIN_VALUE
(of course, this won't work because stop and start are unsigned, but you
probably got the idea).

And couldn't TSF counter overflow during the measurement?

> +	bcm->ieee->channel_change_time = stop - start;
> +}
> +
>  static
>  void bcm43xx_macfilter_set(struct bcm43xx_private *bcm,
>  			   u16 offset,
> @@ -3706,6 +3733,7 @@

Please, could you use -p parameter when generating diffs for easier
review?

Thanks,

 Jiri

-- 
Jiri Benc
SUSE Labs

Re: [PATCH] bcm43xx-d80211: measure the channel change time

[Johannes Berg]

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

On Thu, 2006-05-04 at 11:11 +0200, Jiri Benc wrote:

> I think some safety fallback would be better. Like
> if (stop - start < SOME_MIN_VALUE) channel_change_time = SOME_MIN_VALUE
> (of course, this won't work because stop and start are unsigned, but you
> probably got the idea).

Probably. And yes, it should still work since stop>start at all times.

> And couldn't TSF counter overflow during the measurement?

Yes, if the card has been up for like 459148 years or has been reset to
an unbelievably high value. Remember, it's a 64 bit counter.

johannes

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

Re: [PATCH] bcm43xx-d80211: measure the channel change time

[Jiri Benc]

On Thu, 04 May 2006 14:22:20 +0200, Johannes Berg wrote:
> Probably. And yes, it should still work since stop>start at all times.

Are you sure you can guarantee that stop > start when these values come
from hardware you don't have specification for?

> Yes, if the card has been up for like 459148 years or has been reset to
> an unbelievably high value. Remember, it's a 64 bit counter.

TSF is set to a value sent by an AP. Is there anything that prevents
that AP to send you e.g. 0xfffffffffffff0?

 Jiri

-- 
Jiri Benc
SUSE Labs

Re: [PATCH] bcm43xx-d80211: measure the channel change time

[Johannes Berg]

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

On Thu, 2006-05-04 at 14:33 +0200, Jiri Benc wrote:

> TSF is set to a value sent by an AP. Is there anything that prevents
> that AP to send you e.g. 0xfffffffffffff0?

Ah, good point, I didn't think of stupid APs. Yeah, it should handle it
I guess.

johannes

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

Re: [PATCH] bcm43xx-d80211: measure the channel change time

[John W. Linville]

On Thu, May 04, 2006 at 02:36:56PM +0200, Johannes Berg wrote:
> On Thu, 2006-05-04 at 14:33 +0200, Jiri Benc wrote:
> 
> > TSF is set to a value sent by an AP. Is there anything that prevents
> > that AP to send you e.g. 0xfffffffffffff0?
> 
> Ah, good point, I didn't think of stupid APs. Yeah, it should handle it
> I guess.

I'm going to apply this patch as it stands.  Please follow-up with
a patch to handle the TSF overflow.

Thanks,

John
-- 
John W. Linville
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org