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

2009-12-26 Thread Lukáš Turek 
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


signature.asc
Description: This is a digitally signed message part.
___
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel

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

2009-12-23 Thread 海藻敬之 
 text/html; charset=UTF-16BE: Unrecognized 
___
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel

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

2009-12-23 Thread Lukáš Turek 
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


signature.asc
Description: This is a digitally signed message part.
___
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel

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

2009-12-22 Thread Lukáš Turek 
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.0 +0900
 +++ reset.c 2009-12-22 09:51:16.0 +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 0x6400 constant. Is there anyone with access to Atheros 
documentation who could explain the algorithm at last?

Lukas Turek


signature.asc
Description: This is a digitally signed message part.
___
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel

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

2009-12-21 Thread 海藻敬之 
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 8...@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(0x, AR5K_TIME_OUT_ACK),
 - ah-ah_turbo) = timeout)
 + if (ath5k_hw_clocktoh(ah, AR5K_REG_MS(0x, 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,
 -

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

2009-12-21 Thread 海藻敬之 

Hi,

Let me confirm another point.

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.   am I missing something ?


static inline int ath5k_hw_write_ofdm_timings(struct ath5k_hw *ah,
   struct ieee80211_channel *channel)

   /* Get coefficient
* ALGO: coef = (5 * clock * carrier_freq) / 2)
* 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;
   coef_scaled = ((5 * (clock  24)) / 2) / channel-center_freq;

-

Takayuki Kaiso

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 8...@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

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

2009-12-21 Thread Lukáš Turek 
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 0x6400
  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


signature.asc
Description: This is a digitally signed message part.
___
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel

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

2009-12-21 Thread Bob Copeland 
2009/12/21 Lukáš Turek 8...@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
___
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel

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

2009-12-21 Thread 海藻敬之 
 text/html; charset=UTF-16BE: Unrecognized 
___
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel

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

2009-12-21 Thread 海藻敬之 
Hi, Lukas and Bob

my last post looks blank and corrupted, sorry for the junk mail..

Below shows my local patch.

Does original Atheros HAL calls the function just for 5GHz
or calls for both 2GHz and 5 GHz.?

In ath5k, ath5k_hw_write_ofdm_timings() is called when
AR_5212 and CHANNEL_OFDM are true, which means
for both 2GHz and 5GHz.

--- reset.c_org 2009-12-17 17:01:29.0 +0900
+++ reset.c 2009-12-22 09:51:16.0 +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 */
+ else
+ clock = 40;
+
+ if (channel-hw_value  CHANNEL_TURBO)
+ clock *= 2;
+
coef_scaled = ((5 * (clock  24)) / 2) / channel-center_freq;

/* Get exponent

Takayuki Kaiso
 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 0x6400
   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
   

___
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel

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

2009-12-21 Thread 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 m...@bobcopeland.com

to that patch.

-- 
Bob Copeland %% www.bobcopeland.com

___
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel