Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[海藻敬之]

Thanks, Lukas
> According to my interpretation the calculation should depend 
> on channel width (20MHz in normal mode, 40MHz in turbo mode, same for 802.11g 
> and 802.11a), not on MAC chip clocks. 
thanks, then my guess was irrelevant...
I got some improvement (even not so significant), but I might have
mislead myself at this moment .

let me show my observation during last a few days.

at first
- chip is AR5414
- I pulled down codes via git a week before and your recent patches were
applied.

then what I have observed till now was

1. stable and excellent throughput for 5GHz (with 1hop and also 2hops cases)
16Mbps(1hop) , 8Mbps(2hops)
note: testing with IBSS ad-hoc mode, 1 hop, 2hops , 3 hops..

2. still not good and unstable throughput for 2.4Ghz
thanks to your patch[3/5], [4/5], throughput was improved pretty much
but still staying around 10~15Mbps(1hop), 1Mbps~6Mbps(2hops)

without my experimental patch, 2hops throughput was
worse than the case with that patch.
( 1~2Mbps vs 1~6Mbps --> I can't say this difference is significant ) .
debugfs(rc_stats) shows minstrel reported around 85%~95 success
rate with 48/54Mbps and selected low xmit rate.

(this was the result on limited number of tests. five ~ ten times )

in my these tests, node is sitting pretty close (1m) to each other,
therefore I did not enable RTS/CTS and just counted on physical
carrier sense.

Does it seem that there still exist vulnerable part around carrier
sensing for 2.4GHz.?

anyway,
I would like to get back to this issue, again later

thanks
Taka

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[Lukáš Turek]

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

On 23.12.2009 12:43 海藻敬之 wrote:
> 2. still not good and unstable throughput for 2.4Ghz
> thanks to your patch[3/5], [4/5], throughput was improved pretty much
> but still staying around 10~15Mbps(1hop), 1Mbps~6Mbps(2hops)
That's strange, those patches can't change anything... They modify a code that 
cannot be used until patch[5/5] is applied, and even then you probably don't 
use it, because you didn't say you set the coverage class.

I think there's something wrong with your test methodology, because I don't 
notice any difference, TCP throughput is always around 3MB/s. However I 
haven't tried ath5k on both ends yet.

Have you tried setting a fixed rate (via iwconfig, it's not in iw yet)?
Or setting a fixed antenna (there's no UI yet, you have to hardcode it, see 
function ath5k_hw_set_antenna_mode)?

Lukas Turek

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

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[Lukáš Turek]

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

On 24.12.2009 01:35 海藻敬之 wrote:
> ( sorry again for my last post which was corrupted. I do not why... some
> coding issue.. please be patient. )
For some reason your client tries to send the mail in UTF-16 instead of UTF-8, 
so mailserver cuts the mail at the zero byte in the first character of 
message body.

> My work did not include "coverage class". I just applied [3/5], [4/5].
> Before applying [4/5], ath5k_hw_clocktoh() takes care of 11a but 11g/b,
> and your new ath5k_he_get_clockrate() handles both 11a and 11b/g, correct ?
> Therefore, I thought this can make difference in throughput. am I wrong
> on this point ?
But ath5k_hw_get_clockrate() is just a dead code until [5/5] is applied, and 
even after applying [5/5] it's only called on set_coverage() path, so it 
couldn't make any difference in your case.

> 3Mbps/TCP is so low, isn't it ? how long is it between the two nodes ?
It's 3 megabytes per seconds, not megabits, as reported by wget. I haven't 
done any real throughput test with iperf yet, so your results might be more 
correct, but they shouldn't depend on my patches.

Lukas Turek

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

[PATCH 0/5] Setting coverage class (and ACK timeout and slot time), take two

[Lukáš Turek]

These patches implement a feature essential for long distance wireless links:
setting ACK timeout and slot time. The API is based on a parameter called
Coverage Class specified by the 802.11 standard.

The first two patches add support to the kernel 802.11 stack via a new nl80211
attribute and a mac80211 callback.

Following three patches implement the callback in ath5k driver, after some
requisite fixes. Implementation in ath9k driver is planned.

Please note the last patch outside the series is not a kernel patch, it adds
the necessary support to userspace program iw.

This is a second version of these patches, with these changes:

* The coverage class is calculated from roundtrip time instead of one-way
  delay asi specified by the standard

* Calculation of slot time and ACK timeouts is based on 802.11 standard and
  Madwifi 0.10.5.6, instead of athctrl program from Madwifi 0.9.4

* Full name "coverage_class" is used everywhere for consistency

Lukas Turek

[PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[Lukáš Turek]

The original code was correct in 802.11a mode only, 802.11b/g uses
different clock rates. The new code uses values taken from FreeBSD HAL
and should be correct for all modes including turbo modes.

The former rate calculation was used by slope coefficient calculation
function ath5k_hw_write_ofdm_timings. However, this function requires
the 802.11a values even in 802.11g mode. Thus the use of
ath5k_hw_htoclock was replaced by hardcoded values. Possibly the slope
coefficient calculation is not related to clock rate at all.

Signed-off-by: Lukas Turek <8an@praha12.net>
---
 drivers/net/wireless/ath/ath5k/ath5k.h |   22 ++---------
 drivers/net/wireless/ath/ath5k/pcu.c   |   64 +++++++++++++++++++++++++++-----
 drivers/net/wireless/ath/ath5k/qcu.c   |    4 +-
 drivers/net/wireless/ath/ath5k/reset.c |    3 +-
 4 files changed, 61 insertions(+), 32 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
index 6a2a967..ae311d2 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -1231,6 +1231,10 @@ extern int ath5k_hw_set_ack_timeout(struct ath5k_hw *ah, unsigned int timeout);
 extern unsigned int ath5k_hw_get_ack_timeout(struct ath5k_hw *ah);
 extern int ath5k_hw_set_cts_timeout(struct ath5k_hw *ah, unsigned int timeout);
 extern unsigned int ath5k_hw_get_cts_timeout(struct ath5k_hw *ah);
+/* Clock rate related functions */
+unsigned int ath5k_hw_htoclock(struct ath5k_hw *ah, unsigned int usec);
+unsigned int ath5k_hw_clocktoh(struct ath5k_hw *ah, unsigned int clock);
+unsigned int ath5k_hw_get_clockrate(struct ath5k_hw *ah);
 /* Key table (WEP) functions */
 extern int ath5k_hw_reset_key(struct ath5k_hw *ah, u16 entry);
 extern int ath5k_hw_is_key_valid(struct ath5k_hw *ah, u16 entry);
@@ -1310,24 +1314,6 @@ extern int ath5k_hw_set_txpower_limit(struct ath5k_hw *ah, u8 txpower);
  * Functions used internaly
  */
 
-/*
- * Translate usec to hw clock units
- * TODO: Half/quarter rate
- */
-static inline unsigned int ath5k_hw_htoclock(unsigned int usec, bool turbo)
-{
-	return turbo ? (usec * 80) : (usec * 40);
-}
-
-/*
- * Translate hw clock units to usec
- * TODO: Half/quarter rate
- */
-static inline unsigned int ath5k_hw_clocktoh(unsigned int clock, bool turbo)
-{
-	return turbo ? (clock / 80) : (clock / 40);
-}
-
 static inline struct ath_common *ath5k_hw_common(struct ath5k_hw *ah)
 {
         return &ah->common;
diff --git a/drivers/net/wireless/ath/ath5k/pcu.c b/drivers/net/wireless/ath/ath5k/pcu.c
index 64fc1eb..8c72845 100644
--- a/drivers/net/wireless/ath/ath5k/pcu.c
+++ b/drivers/net/wireless/ath/ath5k/pcu.c
@@ -187,8 +187,8 @@ unsigned int ath5k_hw_get_ack_timeout(struct ath5k_hw *ah)
 {
 	ATH5K_TRACE(ah->ah_sc);
 
-	return ath5k_hw_clocktoh(AR5K_REG_MS(ath5k_hw_reg_read(ah,
-			AR5K_TIME_OUT), AR5K_TIME_OUT_ACK), ah->ah_turbo);
+	return ath5k_hw_clocktoh(ah, AR5K_REG_MS(ath5k_hw_reg_read(ah,
+			AR5K_TIME_OUT), AR5K_TIME_OUT_ACK));
 }
 
 /**
@@ -200,12 +200,12 @@ unsigned int ath5k_hw_get_ack_timeout(struct ath5k_hw *ah)
 int ath5k_hw_set_ack_timeout(struct ath5k_hw *ah, unsigned int timeout)
 {
 	ATH5K_TRACE(ah->ah_sc);
-	if (ath5k_hw_clocktoh(AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_ACK),
-			ah->ah_turbo) <= timeout)
+	if (ath5k_hw_clocktoh(ah, AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_ACK))
+			<= timeout)
 		return -EINVAL;
 
 	AR5K_REG_WRITE_BITS(ah, AR5K_TIME_OUT, AR5K_TIME_OUT_ACK,
-		ath5k_hw_htoclock(timeout, ah->ah_turbo));
+		ath5k_hw_htoclock(ah, timeout));
 
 	return 0;
 }
@@ -218,8 +218,8 @@ int ath5k_hw_set_ack_timeout(struct ath5k_hw *ah, unsigned int timeout)
 unsigned int ath5k_hw_get_cts_timeout(struct ath5k_hw *ah)
 {
 	ATH5K_TRACE(ah->ah_sc);
-	return ath5k_hw_clocktoh(AR5K_REG_MS(ath5k_hw_reg_read(ah,
-			AR5K_TIME_OUT), AR5K_TIME_OUT_CTS), ah->ah_turbo);
+	return ath5k_hw_clocktoh(ah, AR5K_REG_MS(ath5k_hw_reg_read(ah,
+			AR5K_TIME_OUT), AR5K_TIME_OUT_CTS));
 }
 
 /**
@@ -231,17 +231,61 @@ unsigned int ath5k_hw_get_cts_timeout(struct ath5k_hw *ah)
 int ath5k_hw_set_cts_timeout(struct ath5k_hw *ah, unsigned int timeout)
 {
 	ATH5K_TRACE(ah->ah_sc);
-	if (ath5k_hw_clocktoh(AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_CTS),
-			ah->ah_turbo) <= timeout)
+	if (ath5k_hw_clocktoh(ah, AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_CTS))
+			<= timeout)
 		return -EINVAL;
 
 	AR5K_REG_WRITE_BITS(ah, AR5K_TIME_OUT, AR5K_TIME_OUT_CTS,
-			ath5k_hw_htoclock(timeout, ah->ah_turbo));
+			ath5k_hw_htoclock(ah, timeout));
 
 	return 0;
 }
 
 /**
+ * ath5k_hw_htoclock - Translate usec to hw clock units
+ *
+ * @ah: The &struct ath5k_hw
+ * @usec: value in microseconds
+ */
+unsigned int ath5k_hw_htoclock(struct ath5k_hw *ah, unsigned int usec)
+{
+	return usec * ath5k_hw_get_clockrate(ah);
+}
+
+/**
+ * ath5k_hw_clocktoh - Translate hw clock units to usec
+ * @clock: value in hw clock units
+ */
+unsigned int ath5k_hw_clocktoh(struct ath5k_hw *ah, unsigned int clock)
+{
+	return clock / ath5k_hw_get_clockrate(ah);
+}
+
+/**
+ * ath5k_hw_get_clockrate - Get the clock rate for current mode
+ *
+ * @ah: The &struct ath5k_hw
+ */
+unsigned int ath5k_hw_get_clockrate(struct ath5k_hw *ah)
+{
+	struct ieee80211_channel *channel = ah->ah_current_channel;
+	int clock;
+
+	if (channel->hw_value & CHANNEL_5GHZ)
+		clock = 40; /* 802.11a */
+	else if (channel->hw_value & CHANNEL_CCK)
+		clock = 22; /* 802.11b */
+	else
+		clock = 44; /* 802.11g */
+
+	/* Clock rate in turbo modes is twice the normal rate */
+	if (channel->hw_value & CHANNEL_TURBO)
+		clock *= 2;
+
+	return clock;
+}
+
+/**
  * ath5k_hw_set_lladdr - Set station id
  *
  * @ah: The &struct ath5k_hw
diff --git a/drivers/net/wireless/ath/ath5k/qcu.c b/drivers/net/wireless/ath/ath5k/qcu.c
index ed9021a..6af0ac8 100644
--- a/drivers/net/wireless/ath/ath5k/qcu.c
+++ b/drivers/net/wireless/ath/ath5k/qcu.c
@@ -529,7 +529,7 @@ unsigned int ath5k_hw_get_slot_time(struct ath5k_hw *ah)
 	else
 		slot_time_clock = ath5k_hw_reg_read(ah, AR5K_DCU_GBL_IFS_SLOT);
 	
-	return ath5k_hw_clocktoh(slot_time_clock & 0xffff, ah->ah_turbo);
+	return ath5k_hw_clocktoh(ah, slot_time_clock & 0xffff);
 }
 
 /*
@@ -537,7 +537,7 @@ unsigned int ath5k_hw_get_slot_time(struct ath5k_hw *ah)
  */
 int ath5k_hw_set_slot_time(struct ath5k_hw *ah, unsigned int slot_time)
 {
-	u32 slot_time_clock = ath5k_hw_htoclock(slot_time, ah->ah_turbo);
+	u32 slot_time_clock = ath5k_hw_htoclock(ah, slot_time);
 
 	ATH5K_TRACE(ah->ah_sc);
 
diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
index 62954fc..f1dc4c8 100644
--- a/drivers/net/wireless/ath/ath5k/reset.c
+++ b/drivers/net/wireless/ath/ath5k/reset.c
@@ -64,8 +64,7 @@ static inline int ath5k_hw_write_ofdm_timings(struct ath5k_hw *ah,
 	 * we scale coef by shifting clock value by 24 for
 	 * better precision since we use integers */
 	/* TODO: Half/quarter rate */
-	clock =  ath5k_hw_htoclock(1, channel->hw_value & CHANNEL_TURBO);
-
+	clock =  (channel->hw_value & CHANNEL_TURBO) ? 80 : 40;
 	coef_scaled = ((5 * (clock << 24)) / 2) / channel->center_freq;
 
 	/* Get exponent
-- 
1.6.4.4

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[海藻敬之]

Hi Lukas

 Didn't we have to handle CHANNEL_2GHZ case in 
ath5k_hw_write_ofdm_timings() shown below ?
 I think we should do. then I made my own patch to hadle it and it 
seemed  to improve the throughput of 2.4GHz. (even still not as good as 
5Ghz case )

> diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
> index 62954fc..f1dc4c8 100644
> --- a/drivers/net/wireless/ath/ath5k/reset.c
> +++ b/drivers/net/wireless/ath/ath5k/reset.c
> @@ -64,8 +64,7 @@ static inline int ath5k_hw_write_ofdm_timings(struct ath5k_hw *ah,
>  	 * we scale coef by shifting clock value by 24 for
>  	 * better precision since we use integers */
>  	/* TODO: Half/quarter rate */
> -	clock =  ath5k_hw_htoclock(1, channel->hw_value & CHANNEL_TURBO);
> -
> +	clock =  (channel->hw_value & CHANNEL_TURBO) ? 80 : 40;
>  	coef_scaled = ((5 * (clock << 24)) / 2) / channel->center_freq;
thanks
Takayuki Kaiso
Kobe/Japan

> The original code was correct in 802.11a mode only, 802.11b/g uses
> different clock rates. The new code uses values taken from FreeBSD HAL
> and should be correct for all modes including turbo modes.
>
> The former rate calculation was used by slope coefficient calculation
> function ath5k_hw_write_ofdm_timings. However, this function requires
> the 802.11a values even in 802.11g mode. Thus the use of
> ath5k_hw_htoclock was replaced by hardcoded values. Possibly the slope
> coefficient calculation is not related to clock rate at all.
>
> Signed-off-by: Lukas Turek <8an@praha12.net>
> ---
>  drivers/net/wireless/ath/ath5k/ath5k.h |   22 ++---------
>  drivers/net/wireless/ath/ath5k/pcu.c   |   64 +++++++++++++++++++++++++++-----
>  drivers/net/wireless/ath/ath5k/qcu.c   |    4 +-
>  drivers/net/wireless/ath/ath5k/reset.c |    3 +-
>  4 files changed, 61 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
> index 6a2a967..ae311d2 100644
> --- a/drivers/net/wireless/ath/ath5k/ath5k.h
> +++ b/drivers/net/wireless/ath/ath5k/ath5k.h
> @@ -1231,6 +1231,10 @@ extern int ath5k_hw_set_ack_timeout(struct ath5k_hw *ah, unsigned int timeout);
>  extern unsigned int ath5k_hw_get_ack_timeout(struct ath5k_hw *ah);
>  extern int ath5k_hw_set_cts_timeout(struct ath5k_hw *ah, unsigned int timeout);
>  extern unsigned int ath5k_hw_get_cts_timeout(struct ath5k_hw *ah);
> +/* Clock rate related functions */
> +unsigned int ath5k_hw_htoclock(struct ath5k_hw *ah, unsigned int usec);
> +unsigned int ath5k_hw_clocktoh(struct ath5k_hw *ah, unsigned int clock);
> +unsigned int ath5k_hw_get_clockrate(struct ath5k_hw *ah);
>  /* Key table (WEP) functions */
>  extern int ath5k_hw_reset_key(struct ath5k_hw *ah, u16 entry);
>  extern int ath5k_hw_is_key_valid(struct ath5k_hw *ah, u16 entry);
> @@ -1310,24 +1314,6 @@ extern int ath5k_hw_set_txpower_limit(struct ath5k_hw *ah, u8 txpower);
>   * Functions used internaly
>   */
>  
> -/*
> - * Translate usec to hw clock units
> - * TODO: Half/quarter rate
> - */
> -static inline unsigned int ath5k_hw_htoclock(unsigned int usec, bool turbo)
> -{
> -	return turbo ? (usec * 80) : (usec * 40);
> -}
> -
> -/*
> - * Translate hw clock units to usec
> - * TODO: Half/quarter rate
> - */
> -static inline unsigned int ath5k_hw_clocktoh(unsigned int clock, bool turbo)
> -{
> -	return turbo ? (clock / 80) : (clock / 40);
> -}
> -
>  static inline struct ath_common *ath5k_hw_common(struct ath5k_hw *ah)
>  {
>          return &ah->common;
> diff --git a/drivers/net/wireless/ath/ath5k/pcu.c b/drivers/net/wireless/ath/ath5k/pcu.c
> index 64fc1eb..8c72845 100644
> --- a/drivers/net/wireless/ath/ath5k/pcu.c
> +++ b/drivers/net/wireless/ath/ath5k/pcu.c
> @@ -187,8 +187,8 @@ unsigned int ath5k_hw_get_ack_timeout(struct ath5k_hw *ah)
>  {
>  	ATH5K_TRACE(ah->ah_sc);
>  
> -	return ath5k_hw_clocktoh(AR5K_REG_MS(ath5k_hw_reg_read(ah,
> -			AR5K_TIME_OUT), AR5K_TIME_OUT_ACK), ah->ah_turbo);
> +	return ath5k_hw_clocktoh(ah, AR5K_REG_MS(ath5k_hw_reg_read(ah,
> +			AR5K_TIME_OUT), AR5K_TIME_OUT_ACK));
>  }
>  
>  /**
> @@ -200,12 +200,12 @@ unsigned int ath5k_hw_get_ack_timeout(struct ath5k_hw *ah)
>  int ath5k_hw_set_ack_timeout(struct ath5k_hw *ah, unsigned int timeout)
>  {
>  	ATH5K_TRACE(ah->ah_sc);
> -	if (ath5k_hw_clocktoh(AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_ACK),
> -			ah->ah_turbo) <= timeout)
> +	if (ath5k_hw_clocktoh(ah, AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_ACK))
> +			<= timeout)
>  		return -EINVAL;
>  
>  	AR5K_REG_WRITE_BITS(ah, AR5K_TIME_OUT, AR5K_TIME_OUT_ACK,
> -		ath5k_hw_htoclock(timeout, ah->ah_turbo));
> +		ath5k_hw_htoclock(ah, timeout));
>  
>  	return 0;
>  }
> @@ -218,8 +218,8 @@ int ath5k_hw_set_ack_timeout(struct ath5k_hw *ah, unsigned int timeout)
>  unsigned int ath5k_hw_get_cts_timeout(struct ath5k_hw *ah)
>  {
>  	ATH5K_TRACE(ah->ah_sc);
> -	return ath5k_hw_clocktoh(AR5K_REG_MS(ath5k_hw_reg_read(ah,
> -			AR5K_TIME_OUT), AR5K_TIME_OUT_CTS), ah->ah_turbo);
> +	return ath5k_hw_clocktoh(ah, AR5K_REG_MS(ath5k_hw_reg_read(ah,
> +			AR5K_TIME_OUT), AR5K_TIME_OUT_CTS));
>  }
>  
>  /**
> @@ -231,17 +231,61 @@ unsigned int ath5k_hw_get_cts_timeout(struct ath5k_hw *ah)
>  int ath5k_hw_set_cts_timeout(struct ath5k_hw *ah, unsigned int timeout)
>  {
>  	ATH5K_TRACE(ah->ah_sc);
> -	if (ath5k_hw_clocktoh(AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_CTS),
> -			ah->ah_turbo) <= timeout)
> +	if (ath5k_hw_clocktoh(ah, AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_CTS))
> +			<= timeout)
>  		return -EINVAL;
>  
>  	AR5K_REG_WRITE_BITS(ah, AR5K_TIME_OUT, AR5K_TIME_OUT_CTS,
> -			ath5k_hw_htoclock(timeout, ah->ah_turbo));
> +			ath5k_hw_htoclock(ah, timeout));
>  
>  	return 0;
>  }
>  
>  /**
> + * ath5k_hw_htoclock - Translate usec to hw clock units
> + *
> + * @ah: The &struct ath5k_hw
> + * @usec: value in microseconds
> + */
> +unsigned int ath5k_hw_htoclock(struct ath5k_hw *ah, unsigned int usec)
> +{
> +	return usec * ath5k_hw_get_clockrate(ah);
> +}
> +
> +/**
> + * ath5k_hw_clocktoh - Translate hw clock units to usec
> + * @clock: value in hw clock units
> + */
> +unsigned int ath5k_hw_clocktoh(struct ath5k_hw *ah, unsigned int clock)
> +{
> +	return clock / ath5k_hw_get_clockrate(ah);
> +}
> +
> +/**
> + * ath5k_hw_get_clockrate - Get the clock rate for current mode
> + *
> + * @ah: The &struct ath5k_hw
> + */
> +unsigned int ath5k_hw_get_clockrate(struct ath5k_hw *ah)
> +{
> +	struct ieee80211_channel *channel = ah->ah_current_channel;
> +	int clock;
> +
> +	if (channel->hw_value & CHANNEL_5GHZ)
> +		clock = 40; /* 802.11a */
> +	else if (channel->hw_value & CHANNEL_CCK)
> +		clock = 22; /* 802.11b */
> +	else
> +		clock = 44; /* 802.11g */
> +
> +	/* Clock rate in turbo modes is twice the normal rate */
> +	if (channel->hw_value & CHANNEL_TURBO)
> +		clock *= 2;
> +
> +	return clock;
> +}
> +
> +/**
>   * ath5k_hw_set_lladdr - Set station id
>   *
>   * @ah: The &struct ath5k_hw
> diff --git a/drivers/net/wireless/ath/ath5k/qcu.c b/drivers/net/wireless/ath/ath5k/qcu.c
> index ed9021a..6af0ac8 100644
> --- a/drivers/net/wireless/ath/ath5k/qcu.c
> +++ b/drivers/net/wireless/ath/ath5k/qcu.c
> @@ -529,7 +529,7 @@ unsigned int ath5k_hw_get_slot_time(struct ath5k_hw *ah)
>  	else
>  		slot_time_clock = ath5k_hw_reg_read(ah, AR5K_DCU_GBL_IFS_SLOT);
>  	
> -	return ath5k_hw_clocktoh(slot_time_clock & 0xffff, ah->ah_turbo);
> +	return ath5k_hw_clocktoh(ah, slot_time_clock & 0xffff);
>  }
>  
>  /*
> @@ -537,7 +537,7 @@ unsigned int ath5k_hw_get_slot_time(struct ath5k_hw *ah)
>   */
>  int ath5k_hw_set_slot_time(struct ath5k_hw *ah, unsigned int slot_time)
>  {
> -	u32 slot_time_clock = ath5k_hw_htoclock(slot_time, ah->ah_turbo);
> +	u32 slot_time_clock = ath5k_hw_htoclock(ah, slot_time);
>  
>  	ATH5K_TRACE(ah->ah_sc);
>  
> diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
> index 62954fc..f1dc4c8 100644
> --- a/drivers/net/wireless/ath/ath5k/reset.c
> +++ b/drivers/net/wireless/ath/ath5k/reset.c
> @@ -64,8 +64,7 @@ static inline int ath5k_hw_write_ofdm_timings(struct ath5k_hw *ah,
>  	 * we scale coef by shifting clock value by 24 for
>  	 * better precision since we use integers */
>  	/* TODO: Half/quarter rate */
> -	clock =  ath5k_hw_htoclock(1, channel->hw_value & CHANNEL_TURBO);
> -
> +	clock =  (channel->hw_value & CHANNEL_TURBO) ? 80 : 40;
>  	coef_scaled = ((5 * (clock << 24)) / 2) / channel->center_freq;
>  
>  	/* Get exponent
>   


-- 
*****************************************
株式会社　シンクチューブ
海藻　敬之 tkaiso@thinktube.com
〒658-0032 神戸市東灘区向洋町中６－９　KFMビル　4E-10
Phone: 078-857-8390
Fax: 078-857-8389
www.thinktube.com

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[Lukáš Turek]

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

On 21.12.2009 11:26 海藻敬之 wrote:
>  Didn't we have to handle CHANNEL_2GHZ case in
> ath5k_hw_write_ofdm_timings() shown below ?
The ath5k gives exactly the same results as original Atheros HAL:

  #define INIT_CLOCKMHZSCALED     0x64000000
  unsigned long clockMhzScaled = INIT_CLOCKMHZSCALED;
  if (IS_CHAN_TURBO(chan)) clockMhzScaled *= 2;
  coef_scaled = clockMhzScaled / chan->channel;

I don't know how the calculations work, they might be explained in the 
referenced patent, but I didn't change their semantics at all.

>  I think we should do. then I made my own patch to hadle it and it
> seemed  to improve the throughput of 2.4GHz. (even still not as good as
> 5Ghz case )
Where's the patch?

Lukas Turek

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

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[Lukáš Turek]

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

On 22.12.2009 02:24 海藻敬之 wrote:
> Does original Atheros HAL calls the function just for 5GHz
> or calls for both 2GHz and 5 GHz.?
For both, too, the condition used is IS_CHAN_OFDM(chan).

> --- reset.c_org 2009-12-17 17:01:29.000000000 +0900
> +++ reset.c 2009-12-22 09:51:16.000000000 +0900
> @@ -64,7 +64,14 @@
> * we scale coef by shifting clock value by 24 for
> * better precision since we use integers */
> /* TODO: Half/quarter rate */
> - clock = (channel->hw_value & CHANNEL_TURBO) ? 80 : 40;
> + if (channel->hw_value & CHANNEL_2GHZ)
> + clock = 44; /* here, we do not have to worry about CCK */

If it really improved your throughput, there might be something on it, but I 
still don't think we should change it like that without really understanding 
the algorithm. According to my interpretation the calculation should depend 
on channel width (20MHz in normal mode, 40MHz in turbo mode, same for 802.11g 
and 802.11a), not on MAC chip clocks. At least some Atheros chipsets have 
separate radio chip with its own 40 MHz crystal.

I looked at ath9k source, and it uses the same calculation as FreeBSD HAL, 
including the 0x64000000 constant. Is there anyone with access to Atheros 
documentation who could explain the algorithm at last?

Lukas Turek

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

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[Lukáš Turek]

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

On 21.12.2009 11:41 海藻敬之 wrote:
>  In ath5k_hw_write_ofdm_timings(),  comment says
>       "ALGO: coef = (5 * clock * carrier_freq) / 2) ",
>  but current code is
>       "coef_scaled = ((5 * (clock << 24)) / 2) / channel->center_freq;"
>
>  Did they match each other ?
>  I am wondering the the comment is wrong, but I am not sure that either
> is wrong.
Good point, it seems the comment is wrong. The calculation would overflow 
32-bit integer if there was a multiplication instead of a division.

Lukas Turek

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

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[Bob Copeland]

2009/12/21 Lukáš Turek <8an@praha12.net>:
> On 21.12.2009 11:41 海藻敬之 wrote:
>>  In ath5k_hw_write_ofdm_timings(),  comment says
>>       "ALGO: coef = (5 * clock * carrier_freq) / 2) ",
>>  but current code is
>>       "coef_scaled = ((5 * (clock << 24)) / 2) / channel->center_freq;"
>>
>>  Did they match each other ?
>>  I am wondering the the comment is wrong, but I am not sure that either
>> is wrong.
> Good point, it seems the comment is wrong. The calculation would overflow
> 32-bit integer if there was a multiplication instead of a division.
>
> Lukas Turek

The original comment was:

/*
 * ALGO -> coef = 1e8/fcarrier*fclock/40;
 * scaled coef to provide precision for this floating calculation
 */
coef_scaled = clockMhzScaled / chan->channel;

So dividing by the carrier frequency sounds like the right thing,
I guess the comment is wrong.

I reviewed the patch, looks fine to me.  The ATH hal uses a lookup
table to keep it inline but I don't think we have a convenient
index available to do the same.

I'll see what I can find about the pilot tracking to see if that
makes sense here.

-- 
Bob Copeland %% www.bobcopeland.com

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[Lukáš Turek]

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

On 21.12.2009 16:08 Bob Copeland wrote:
> I reviewed the patch, looks fine to me.  The ATH hal uses a lookup
> table to keep it inline but I don't think we have a convenient
> index available to do the same.
Yes, HAL uses a lookup table for mode -> clocks coversion, but before that can 
be used it has to convert channel flags to mode, and that's a sequence of ifs 
too (see function ath_hal_chan2wmode).

If performance mattered, we could store the mode index somwhere. But it 
doesn't, the conversion is only needed when setting ACK timeout and slot 
time - which is, for most users, never done and defaults from initvals.c are 
used instead.

Lukas Turek

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

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[Bob Copeland]

On Mon, Dec 21, 2009 at 04:28:35PM +0100, Luk???? Turek wrote:
> On 21.12.2009 16:08 Bob Copeland wrote:
> If performance mattered, we could store the mode index somwhere. But it 
> doesn't, the conversion is only needed when setting ACK timeout and slot 
> time - which is, for most users, never done and defaults from initvals.c are 
> used instead.

Agreed.  Feel free to add my

Acked-by: Bob Copeland <me@bobcopeland.com>

to that patch.

-- 
Bob Copeland %% www.bobcopeland.com