2010-11-23 19:09:47

by Nick Kossifidis

[permalink] [raw]
Subject: [PATCH 14/30] ath5k: Extend get_default_sifs/slot_time

* Extend get_default_sifs/slot_time to include timings for turbo
half and quarter rate modes.

* AR5210 code for now uses timings already on core clock units
instead of usecs so rename them (we 'll clean it up later).

Signed-off-by: Nick Kossifidis <[email protected]>
---
drivers/net/wireless/ath/ath5k/ath5k.h | 24 ++++++++++++--
drivers/net/wireless/ath/ath5k/pcu.c | 52 ++++++++++++++++++++++++--------
drivers/net/wireless/ath/ath5k/qcu.c | 16 +++++----
3 files changed, 68 insertions(+), 24 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
index 005cad0..e11fc8f 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -226,16 +226,16 @@
#define AR5K_INIT_USEC 39
#define AR5K_INIT_USEC_TURBO 79
#define AR5K_INIT_USEC_32 31
-#define AR5K_INIT_SLOT_TIME 396
-#define AR5K_INIT_SLOT_TIME_TURBO 480
+#define AR5K_INIT_SLOT_TIME_CLOCK 396
+#define AR5K_INIT_SLOT_TIME_TURBO_CLOCK 480
#define AR5K_INIT_ACK_CTS_TIMEOUT 1024
#define AR5K_INIT_ACK_CTS_TIMEOUT_TURBO 0x08000800
#define AR5K_INIT_PROG_IFS 920
#define AR5K_INIT_PROG_IFS_TURBO 960
#define AR5K_INIT_EIFS 3440
#define AR5K_INIT_EIFS_TURBO 6880
-#define AR5K_INIT_SIFS 560
-#define AR5K_INIT_SIFS_TURBO 480
+#define AR5K_INIT_SIFS_CLOCK 560
+#define AR5K_INIT_SIFS_TURBO_CLOCK 480
#define AR5K_INIT_SH_RETRY 10
#define AR5K_INIT_LG_RETRY AR5K_INIT_SH_RETRY
#define AR5K_INIT_SSH_RETRY 32
@@ -251,6 +251,22 @@
(AR5K_INIT_PROG_IFS_TURBO) \
)

+/* Slot time */
+#define AR5K_INIT_SLOT_TIME_TURBO 6
+#define AR5K_INIT_SLOT_TIME_DEFAULT 9
+#define AR5K_INIT_SLOT_TIME_HALF_RATE 13
+#define AR5K_INIT_SLOT_TIME_QUARTER_RATE 21
+#define AR5K_INIT_SLOT_TIME_B 20
+#define AR5K_SLOT_TIME_MAX 0xffff
+
+/* SIFS */
+#define AR5K_INIT_SIFS_TURBO 6
+/* XXX: 8 from initvals 10 from standard */
+#define AR5K_INIT_SIFS_DEFAULT_BG 8
+#define AR5K_INIT_SIFS_DEFAULT_A 16
+#define AR5K_INIT_SIFS_HALF_RATE 32
+#define AR5K_INIT_SIFS_QUARTER_RATE 64
+
/* Rx latency for 5 and 10MHz operation (max ?) */
#define AR5K_INIT_RX_LAT_MAX 63
/* Tx latencies from initvals (5212 only but no problem
diff --git a/drivers/net/wireless/ath/ath5k/pcu.c b/drivers/net/wireless/ath/ath5k/pcu.c
index ae99e37..cb6e277 100644
--- a/drivers/net/wireless/ath/ath5k/pcu.c
+++ b/drivers/net/wireless/ath/ath5k/pcu.c
@@ -43,14 +43,27 @@
static unsigned int ath5k_hw_get_default_slottime(struct ath5k_hw *ah)
{
struct ieee80211_channel *channel = ah->ah_current_channel;
+ unsigned int slot_time;

- if (channel->hw_value & CHANNEL_TURBO)
- return 6; /* both turbo modes */
+ switch (ah->ah_bwmode) {
+ case AR5K_BWMODE_40MHZ:
+ slot_time = AR5K_INIT_SLOT_TIME_TURBO;
+ break;
+ case AR5K_BWMODE_10MHZ:
+ slot_time = AR5K_INIT_SLOT_TIME_HALF_RATE;
+ break;
+ case AR5K_BWMODE_5MHZ:
+ slot_time = AR5K_INIT_SLOT_TIME_QUARTER_RATE;
+ break;
+ case AR5K_BWMODE_DEFAULT:
+ slot_time = AR5K_INIT_SLOT_TIME_DEFAULT;
+ default:
+ if (channel->hw_value & CHANNEL_CCK)
+ slot_time = AR5K_INIT_SLOT_TIME_B;
+ break;
+ }

- if (channel->hw_value & CHANNEL_CCK)
- return 20; /* 802.11b */
-
- return 9; /* 802.11 a/g */
+ return slot_time;
}

/**
@@ -58,17 +71,30 @@ static unsigned int ath5k_hw_get_default_slottime(struct ath5k_hw *ah)
*
* @ah: The &struct ath5k_hw
*/
-static unsigned int ath5k_hw_get_default_sifs(struct ath5k_hw *ah)
+unsigned int ath5k_hw_get_default_sifs(struct ath5k_hw *ah)
{
struct ieee80211_channel *channel = ah->ah_current_channel;
+ unsigned int sifs;

- if (channel->hw_value & CHANNEL_TURBO)
- return 8; /* both turbo modes */
+ switch (ah->ah_bwmode) {
+ case AR5K_BWMODE_40MHZ:
+ sifs = AR5K_INIT_SIFS_TURBO;
+ break;
+ case AR5K_BWMODE_10MHZ:
+ sifs = AR5K_INIT_SIFS_HALF_RATE;
+ break;
+ case AR5K_BWMODE_5MHZ:
+ sifs = AR5K_INIT_SIFS_QUARTER_RATE;
+ break;
+ case AR5K_BWMODE_DEFAULT:
+ sifs = AR5K_INIT_SIFS_DEFAULT_BG;
+ default:
+ if (channel->hw_value & CHANNEL_5GHZ)
+ sifs = AR5K_INIT_SIFS_DEFAULT_A;
+ break;
+ }

- if (channel->hw_value & CHANNEL_5GHZ)
- return 16; /* 802.11a */
-
- return 10; /* 802.11 b/g */
+ return sifs;
}

/**
diff --git a/drivers/net/wireless/ath/ath5k/qcu.c b/drivers/net/wireless/ath/ath5k/qcu.c
index c422d5c..6eb6838 100644
--- a/drivers/net/wireless/ath/ath5k/qcu.c
+++ b/drivers/net/wireless/ath/ath5k/qcu.c
@@ -297,7 +297,8 @@ int ath5k_hw_reset_tx_queue(struct ath5k_hw *ah, unsigned int queue)

/* Set Slot time */
ath5k_hw_reg_write(ah, (ah->ah_bwmode == AR5K_BWMODE_40MHZ) ?
- AR5K_INIT_SLOT_TIME_TURBO : AR5K_INIT_SLOT_TIME,
+ AR5K_INIT_SLOT_TIME_TURBO_CLOCK :
+ AR5K_INIT_SLOT_TIME_CLOCK,
AR5K_SLOT_TIME);
/* Set ACK_CTS timeout */
ath5k_hw_reg_write(ah, (ah->ah_bwmode == AR5K_BWMODE_40MHZ) ?
@@ -306,15 +307,16 @@ int ath5k_hw_reset_tx_queue(struct ath5k_hw *ah, unsigned int queue)

/* Set IFS0 */
if (ah->ah_bwmode == AR5K_BWMODE_40MHZ) {
- ath5k_hw_reg_write(ah, ((AR5K_INIT_SIFS_TURBO +
- tq->tqi_aifs * AR5K_INIT_SLOT_TIME_TURBO) <<
- AR5K_IFS0_DIFS_S) | AR5K_INIT_SIFS_TURBO,
+ ath5k_hw_reg_write(ah, ((AR5K_INIT_SIFS_TURBO_CLOCK +
+ tq->tqi_aifs * AR5K_INIT_SLOT_TIME_TURBO_CLOCK)
+ << AR5K_IFS0_DIFS_S) |
+ AR5K_INIT_SIFS_TURBO_CLOCK,
AR5K_IFS0);
} else {
- ath5k_hw_reg_write(ah, ((AR5K_INIT_SIFS +
- tq->tqi_aifs * AR5K_INIT_SLOT_TIME) <<
+ ath5k_hw_reg_write(ah, ((AR5K_INIT_SIFS_CLOCK +
+ tq->tqi_aifs * AR5K_INIT_SLOT_TIME_CLOCK) <<
AR5K_IFS0_DIFS_S) |
- AR5K_INIT_SIFS, AR5K_IFS0);
+ AR5K_INIT_SIFS_CLOCK, AR5K_IFS0);
}

/* Set IFS1 */


2010-11-24 00:35:08

by Jonathan Guerin

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH 14/30] ath5k: Extend get_default_sifs/slot_time

On Wed, Nov 24, 2010 at 10:31 AM, Jonathan Guerin <[email protected]> wrote:
> On Wed, Nov 24, 2010 at 5:09 AM, Nick Kossifidis <[email protected]> wrote:
>> ?* Extend get_default_sifs/slot_time to include timings for turbo
>> ?half and quarter rate modes.
>>
>> ?* AR5210 code for now uses timings already on core clock units
>> ?instead of usecs so rename them (we 'll clean it up later).
>>
>> ?Signed-off-by: Nick Kossifidis <[email protected]>
>> ---
>> ?drivers/net/wireless/ath/ath5k/ath5k.h | ? 24 ++++++++++++--
>> ?drivers/net/wireless/ath/ath5k/pcu.c ? | ? 52 ++++++++++++++++++++++++--------
>> ?drivers/net/wireless/ath/ath5k/qcu.c ? | ? 16 +++++----
>> ?3 files changed, 68 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
>> index 005cad0..e11fc8f 100644
>> --- a/drivers/net/wireless/ath/ath5k/ath5k.h
>> +++ b/drivers/net/wireless/ath/ath5k/ath5k.h
>> @@ -226,16 +226,16 @@
>> ?#define AR5K_INIT_USEC ? ? ? ? ? ? ? ? ? ? ? ? 39
>> ?#define AR5K_INIT_USEC_TURBO ? ? ? ? ? ? ? ? ? 79
>> ?#define AR5K_INIT_USEC_32 ? ? ? ? ? ? ? ? ? ? ?31
>> -#define AR5K_INIT_SLOT_TIME ? ? ? ? ? ? ? ? ? ?396
>> -#define AR5K_INIT_SLOT_TIME_TURBO ? ? ? ? ? ? ?480
>> +#define AR5K_INIT_SLOT_TIME_CLOCK ? ? ? ? ? ? ?396
>
> This comes out as 9.9us (with a clock multiplier of 40), rather than 9
> - should be 360?
>
>> +#define AR5K_INIT_SLOT_TIME_TURBO_CLOCK ? ? ? ? ? ? ? ?480
>> ?#define AR5K_INIT_ACK_CTS_TIMEOUT ? ? ? ? ? ? ?1024
>
> From a previous post I'd made:
>
> According to the 802.11-2007 spec document, the ACKTimeout value is
> (Section 9.2.8 ACK procedure):
> ACKTimeout = aSIFSTime + aSlotTime + aPHY-RX-START-Delay
>
> From Table 17-15?OFDM PHY characteristics, the values are:
> aSIFSTime = 16
> aSlotTime = 9
> aPHY-RX-START-Delay = 25
>
> Therefore, ACKTimeout = 50
>
> With a value of 1024, the ACKTIMEOUT is actually 25.
>
>> ?#define AR5K_INIT_ACK_CTS_TIMEOUT_TURBO ? ? ? ? ? ? ? ?0x08000800
>> ?#define AR5K_INIT_PROG_IFS ? ? ? ? ? ? ? ? ? ? 920
>> ?#define AR5K_INIT_PROG_IFS_TURBO ? ? ? ? ? ? ? 960
>> ?#define AR5K_INIT_EIFS ? ? ? ? ? ? ? ? ? ? ? ? 3440
>
> From the 802.11-2007 Spec (9.2.10 DCF timing relations):
> The EIFS is derived from the SIFS and the DIFS and the length of time
> it takes to transmit an ACK Control frame at the lowest PHY mandatory
> rate by the following equation:
> EIFS = aSIFSTime + DIFS + ACKTxTime
> where ACKTxTime is the time expressed in microseconds required to
> transmit an ACK frame, including
> preamble, PLCP header and any additional PHY dependent information, at
> the lowest PHY mandatory rate.
>
> So, EIFS = 16 + 32 + 44 (ACKTX @ 24Mbps) = 92
> or 16 + 32 + 60 (ACKTX @ 6Mbps) = 108
>
> The current value is 86, which I guess is a nice midpoint, but is
> there any reasoning for this value? It should really be set to the
> longest value of 108, as an EIFS is only used when we have no
> information about a frame because it failed the checksum.
>
>> ?#define AR5K_INIT_EIFS_TURBO ? ? ? ? ? ? ? ? ? 6880
>> -#define AR5K_INIT_SIFS ? ? ? ? ? ? ? ? ? ? ? ? 560
>> -#define AR5K_INIT_SIFS_TURBO ? ? ? ? ? ? ? ? ? 480
>> +#define AR5K_INIT_SIFS_CLOCK ? ? ? ? ? ? ? ? ? 560
>
> This is wrong, when you divide by the clock rate, you get a SIFS of 14, not 16.
>
> Cheers,
>
> Jonathan

Just as an aside, I didn't mean to hijack Nick's patches for this, but
these are some things that I've been discussing with Bruno. I'm happy
to provide a separate patch to remedy these if required.

Cheers,

Jonathan

>
>> +#define AR5K_INIT_SIFS_TURBO_CLOCK ? ? ? ? ? ? 480
>> ?#define AR5K_INIT_SH_RETRY ? ? ? ? ? ? ? ? ? ? 10
>> ?#define AR5K_INIT_LG_RETRY ? ? ? ? ? ? ? ? ? ? AR5K_INIT_SH_RETRY
>> ?#define AR5K_INIT_SSH_RETRY ? ? ? ? ? ? ? ? ? ?32
>> @@ -251,6 +251,22 @@
>> ? ? ? ?(AR5K_INIT_PROG_IFS_TURBO) ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
>> ?)
>>
>> +/* Slot time */
>> +#define AR5K_INIT_SLOT_TIME_TURBO ? ? ? ? ? ? ?6
>> +#define AR5K_INIT_SLOT_TIME_DEFAULT ? ? ? ? ? ?9
>> +#define ? ? ? ?AR5K_INIT_SLOT_TIME_HALF_RATE ? ? ? ? ? 13
>> +#define ? ? ? ?AR5K_INIT_SLOT_TIME_QUARTER_RATE ? ? ? ?21
>> +#define ? ? ? ?AR5K_INIT_SLOT_TIME_B ? ? ? ? ? ? ? ? ? 20
>> +#define AR5K_SLOT_TIME_MAX ? ? ? ? ? ? ? ? ? ? 0xffff
>> +
>> +/* SIFS */
>> +#define ? ? ? ?AR5K_INIT_SIFS_TURBO ? ? ? ? ? ? ? ? ? ?6
>> +/* XXX: 8 from initvals 10 from standard */
>> +#define ? ? ? ?AR5K_INIT_SIFS_DEFAULT_BG ? ? ? ? ? ? ? 8
>> +#define ? ? ? ?AR5K_INIT_SIFS_DEFAULT_A ? ? ? ? ? ? ? ?16
>> +#define ? ? ? ?AR5K_INIT_SIFS_HALF_RATE ? ? ? ? ? ? ? ?32
>> +#define AR5K_INIT_SIFS_QUARTER_RATE ? ? ? ? ? ?64
>> +
>> ?/* Rx latency for 5 and 10MHz operation (max ?) */
>> ?#define AR5K_INIT_RX_LAT_MAX ? ? ? ? ? ? ? ? ? 63
>> ?/* Tx latencies from initvals (5212 only but no problem
>> diff --git a/drivers/net/wireless/ath/ath5k/pcu.c b/drivers/net/wireless/ath/ath5k/pcu.c
>> index ae99e37..cb6e277 100644
>> --- a/drivers/net/wireless/ath/ath5k/pcu.c
>> +++ b/drivers/net/wireless/ath/ath5k/pcu.c
>> @@ -43,14 +43,27 @@
>> ?static unsigned int ath5k_hw_get_default_slottime(struct ath5k_hw *ah)
>> ?{
>> ? ? ? ?struct ieee80211_channel *channel = ah->ah_current_channel;
>> + ? ? ? unsigned int slot_time;
>>
>> - ? ? ? if (channel->hw_value & CHANNEL_TURBO)
>> - ? ? ? ? ? ? ? return 6; /* both turbo modes */
>> + ? ? ? switch (ah->ah_bwmode) {
>> + ? ? ? case AR5K_BWMODE_40MHZ:
>> + ? ? ? ? ? ? ? slot_time = AR5K_INIT_SLOT_TIME_TURBO;
>> + ? ? ? ? ? ? ? break;
>> + ? ? ? case AR5K_BWMODE_10MHZ:
>> + ? ? ? ? ? ? ? slot_time = AR5K_INIT_SLOT_TIME_HALF_RATE;
>> + ? ? ? ? ? ? ? break;
>> + ? ? ? case AR5K_BWMODE_5MHZ:
>> + ? ? ? ? ? ? ? slot_time = AR5K_INIT_SLOT_TIME_QUARTER_RATE;
>> + ? ? ? ? ? ? ? break;
>> + ? ? ? case AR5K_BWMODE_DEFAULT:
>> + ? ? ? ? ? ? ? slot_time = AR5K_INIT_SLOT_TIME_DEFAULT;
>> + ? ? ? default:
>> + ? ? ? ? ? ? ? if (channel->hw_value & CHANNEL_CCK)
>> + ? ? ? ? ? ? ? ? ? ? ? slot_time = AR5K_INIT_SLOT_TIME_B;
>> + ? ? ? ? ? ? ? break;
>> + ? ? ? }
>>
>> - ? ? ? if (channel->hw_value & CHANNEL_CCK)
>> - ? ? ? ? ? ? ? return 20; /* 802.11b */
>> -
>> - ? ? ? return 9; /* 802.11 a/g */
>> + ? ? ? return slot_time;
>> ?}
>>
>> ?/**
>> @@ -58,17 +71,30 @@ static unsigned int ath5k_hw_get_default_slottime(struct ath5k_hw *ah)
>> ?*
>> ?* @ah: The &struct ath5k_hw
>> ?*/
>> -static unsigned int ath5k_hw_get_default_sifs(struct ath5k_hw *ah)
>> +unsigned int ath5k_hw_get_default_sifs(struct ath5k_hw *ah)
>> ?{
>> ? ? ? ?struct ieee80211_channel *channel = ah->ah_current_channel;
>> + ? ? ? unsigned int sifs;
>>
>> - ? ? ? if (channel->hw_value & CHANNEL_TURBO)
>> - ? ? ? ? ? ? ? return 8; /* both turbo modes */
>> + ? ? ? switch (ah->ah_bwmode) {
>> + ? ? ? case AR5K_BWMODE_40MHZ:
>> + ? ? ? ? ? ? ? sifs = AR5K_INIT_SIFS_TURBO;
>> + ? ? ? ? ? ? ? break;
>> + ? ? ? case AR5K_BWMODE_10MHZ:
>> + ? ? ? ? ? ? ? sifs = AR5K_INIT_SIFS_HALF_RATE;
>> + ? ? ? ? ? ? ? break;
>> + ? ? ? case AR5K_BWMODE_5MHZ:
>> + ? ? ? ? ? ? ? sifs = AR5K_INIT_SIFS_QUARTER_RATE;
>> + ? ? ? ? ? ? ? break;
>> + ? ? ? case AR5K_BWMODE_DEFAULT:
>> + ? ? ? ? ? ? ? sifs = AR5K_INIT_SIFS_DEFAULT_BG;
>> + ? ? ? default:
>> + ? ? ? ? ? ? ? if (channel->hw_value & CHANNEL_5GHZ)
>> + ? ? ? ? ? ? ? ? ? ? ? sifs = AR5K_INIT_SIFS_DEFAULT_A;
>> + ? ? ? ? ? ? ? break;
>> + ? ? ? }
>>
>> - ? ? ? if (channel->hw_value & CHANNEL_5GHZ)
>> - ? ? ? ? ? ? ? return 16; /* 802.11a */
>> -
>> - ? ? ? return 10; /* 802.11 b/g */
>> + ? ? ? return sifs;
>> ?}
>>
>> ?/**
>> diff --git a/drivers/net/wireless/ath/ath5k/qcu.c b/drivers/net/wireless/ath/ath5k/qcu.c
>> index c422d5c..6eb6838 100644
>> --- a/drivers/net/wireless/ath/ath5k/qcu.c
>> +++ b/drivers/net/wireless/ath/ath5k/qcu.c
>> @@ -297,7 +297,8 @@ int ath5k_hw_reset_tx_queue(struct ath5k_hw *ah, unsigned int queue)
>>
>> ? ? ? ? ? ? ? ?/* Set Slot time */
>> ? ? ? ? ? ? ? ?ath5k_hw_reg_write(ah, (ah->ah_bwmode == AR5K_BWMODE_40MHZ) ?
>> - ? ? ? ? ? ? ? ? ? ? ? AR5K_INIT_SLOT_TIME_TURBO : AR5K_INIT_SLOT_TIME,
>> + ? ? ? ? ? ? ? ? ? ? ? AR5K_INIT_SLOT_TIME_TURBO_CLOCK :
>> + ? ? ? ? ? ? ? ? ? ? ? AR5K_INIT_SLOT_TIME_CLOCK,
>> ? ? ? ? ? ? ? ? ? ? ? ?AR5K_SLOT_TIME);
>> ? ? ? ? ? ? ? ?/* Set ACK_CTS timeout */
>> ? ? ? ? ? ? ? ?ath5k_hw_reg_write(ah, (ah->ah_bwmode == AR5K_BWMODE_40MHZ) ?
>> @@ -306,15 +307,16 @@ int ath5k_hw_reset_tx_queue(struct ath5k_hw *ah, unsigned int queue)
>>
>> ? ? ? ? ? ? ? ?/* Set IFS0 */
>> ? ? ? ? ? ? ? ?if (ah->ah_bwmode == AR5K_BWMODE_40MHZ) {
>> - ? ? ? ? ? ? ? ? ? ? ? ath5k_hw_reg_write(ah, ((AR5K_INIT_SIFS_TURBO +
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? tq->tqi_aifs * AR5K_INIT_SLOT_TIME_TURBO) <<
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? AR5K_IFS0_DIFS_S) | AR5K_INIT_SIFS_TURBO,
>> + ? ? ? ? ? ? ? ? ? ? ? ath5k_hw_reg_write(ah, ((AR5K_INIT_SIFS_TURBO_CLOCK +
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? tq->tqi_aifs * AR5K_INIT_SLOT_TIME_TURBO_CLOCK)
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? << AR5K_IFS0_DIFS_S) |
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? AR5K_INIT_SIFS_TURBO_CLOCK,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?AR5K_IFS0);
>> ? ? ? ? ? ? ? ?} else {
>> - ? ? ? ? ? ? ? ? ? ? ? ath5k_hw_reg_write(ah, ((AR5K_INIT_SIFS +
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? tq->tqi_aifs * AR5K_INIT_SLOT_TIME) <<
>> + ? ? ? ? ? ? ? ? ? ? ? ath5k_hw_reg_write(ah, ((AR5K_INIT_SIFS_CLOCK +
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? tq->tqi_aifs * AR5K_INIT_SLOT_TIME_CLOCK) <<
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?AR5K_IFS0_DIFS_S) |
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? AR5K_INIT_SIFS, AR5K_IFS0);
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? AR5K_INIT_SIFS_CLOCK, AR5K_IFS0);
>> ? ? ? ? ? ? ? ?}
>>
>> ? ? ? ? ? ? ? ?/* Set IFS1 */
>> _______________________________________________
>> ath5k-devel mailing list
>> [email protected]
>> https://lists.ath5k.org/mailman/listinfo/ath5k-devel
>>
>

2010-11-24 00:31:20

by Jonathan Guerin

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH 14/30] ath5k: Extend get_default_sifs/slot_time

On Wed, Nov 24, 2010 at 5:09 AM, Nick Kossifidis <[email protected]> wrote:
> ?* Extend get_default_sifs/slot_time to include timings for turbo
> ?half and quarter rate modes.
>
> ?* AR5210 code for now uses timings already on core clock units
> ?instead of usecs so rename them (we 'll clean it up later).
>
> ?Signed-off-by: Nick Kossifidis <[email protected]>
> ---
> ?drivers/net/wireless/ath/ath5k/ath5k.h | ? 24 ++++++++++++--
> ?drivers/net/wireless/ath/ath5k/pcu.c ? | ? 52 ++++++++++++++++++++++++--------
> ?drivers/net/wireless/ath/ath5k/qcu.c ? | ? 16 +++++----
> ?3 files changed, 68 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
> index 005cad0..e11fc8f 100644
> --- a/drivers/net/wireless/ath/ath5k/ath5k.h
> +++ b/drivers/net/wireless/ath/ath5k/ath5k.h
> @@ -226,16 +226,16 @@
> ?#define AR5K_INIT_USEC ? ? ? ? ? ? ? ? ? ? ? ? 39
> ?#define AR5K_INIT_USEC_TURBO ? ? ? ? ? ? ? ? ? 79
> ?#define AR5K_INIT_USEC_32 ? ? ? ? ? ? ? ? ? ? ?31
> -#define AR5K_INIT_SLOT_TIME ? ? ? ? ? ? ? ? ? ?396
> -#define AR5K_INIT_SLOT_TIME_TURBO ? ? ? ? ? ? ?480
> +#define AR5K_INIT_SLOT_TIME_CLOCK ? ? ? ? ? ? ?396

This comes out as 9.9us (with a clock multiplier of 40), rather than 9
- should be 360?

> +#define AR5K_INIT_SLOT_TIME_TURBO_CLOCK ? ? ? ? ? ? ? ?480
> ?#define AR5K_INIT_ACK_CTS_TIMEOUT ? ? ? ? ? ? ?1024

>From a previous post I'd made:

According to the 802.11-2007 spec document, the ACKTimeout value is
(Section 9.2.8 ACK procedure):
ACKTimeout = aSIFSTime + aSlotTime + aPHY-RX-START-Delay

>From Table 17-15?OFDM PHY characteristics, the values are:
aSIFSTime = 16
aSlotTime = 9
aPHY-RX-START-Delay = 25

Therefore, ACKTimeout = 50

With a value of 1024, the ACKTIMEOUT is actually 25.

> ?#define AR5K_INIT_ACK_CTS_TIMEOUT_TURBO ? ? ? ? ? ? ? ?0x08000800
> ?#define AR5K_INIT_PROG_IFS ? ? ? ? ? ? ? ? ? ? 920
> ?#define AR5K_INIT_PROG_IFS_TURBO ? ? ? ? ? ? ? 960
> ?#define AR5K_INIT_EIFS ? ? ? ? ? ? ? ? ? ? ? ? 3440

>From the 802.11-2007 Spec (9.2.10 DCF timing relations):
The EIFS is derived from the SIFS and the DIFS and the length of time
it takes to transmit an ACK Control frame at the lowest PHY mandatory
rate by the following equation:
EIFS = aSIFSTime + DIFS + ACKTxTime
where ACKTxTime is the time expressed in microseconds required to
transmit an ACK frame, including
preamble, PLCP header and any additional PHY dependent information, at
the lowest PHY mandatory rate.

So, EIFS = 16 + 32 + 44 (ACKTX @ 24Mbps) = 92
or 16 + 32 + 60 (ACKTX @ 6Mbps) = 108

The current value is 86, which I guess is a nice midpoint, but is
there any reasoning for this value? It should really be set to the
longest value of 108, as an EIFS is only used when we have no
information about a frame because it failed the checksum.

> ?#define AR5K_INIT_EIFS_TURBO ? ? ? ? ? ? ? ? ? 6880
> -#define AR5K_INIT_SIFS ? ? ? ? ? ? ? ? ? ? ? ? 560
> -#define AR5K_INIT_SIFS_TURBO ? ? ? ? ? ? ? ? ? 480
> +#define AR5K_INIT_SIFS_CLOCK ? ? ? ? ? ? ? ? ? 560

This is wrong, when you divide by the clock rate, you get a SIFS of 14, not 16.

Cheers,

Jonathan

> +#define AR5K_INIT_SIFS_TURBO_CLOCK ? ? ? ? ? ? 480
> ?#define AR5K_INIT_SH_RETRY ? ? ? ? ? ? ? ? ? ? 10
> ?#define AR5K_INIT_LG_RETRY ? ? ? ? ? ? ? ? ? ? AR5K_INIT_SH_RETRY
> ?#define AR5K_INIT_SSH_RETRY ? ? ? ? ? ? ? ? ? ?32
> @@ -251,6 +251,22 @@
> ? ? ? ?(AR5K_INIT_PROG_IFS_TURBO) ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ?)
>
> +/* Slot time */
> +#define AR5K_INIT_SLOT_TIME_TURBO ? ? ? ? ? ? ?6
> +#define AR5K_INIT_SLOT_TIME_DEFAULT ? ? ? ? ? ?9
> +#define ? ? ? ?AR5K_INIT_SLOT_TIME_HALF_RATE ? ? ? ? ? 13
> +#define ? ? ? ?AR5K_INIT_SLOT_TIME_QUARTER_RATE ? ? ? ?21
> +#define ? ? ? ?AR5K_INIT_SLOT_TIME_B ? ? ? ? ? ? ? ? ? 20
> +#define AR5K_SLOT_TIME_MAX ? ? ? ? ? ? ? ? ? ? 0xffff
> +
> +/* SIFS */
> +#define ? ? ? ?AR5K_INIT_SIFS_TURBO ? ? ? ? ? ? ? ? ? ?6
> +/* XXX: 8 from initvals 10 from standard */
> +#define ? ? ? ?AR5K_INIT_SIFS_DEFAULT_BG ? ? ? ? ? ? ? 8
> +#define ? ? ? ?AR5K_INIT_SIFS_DEFAULT_A ? ? ? ? ? ? ? ?16
> +#define ? ? ? ?AR5K_INIT_SIFS_HALF_RATE ? ? ? ? ? ? ? ?32
> +#define AR5K_INIT_SIFS_QUARTER_RATE ? ? ? ? ? ?64
> +
> ?/* Rx latency for 5 and 10MHz operation (max ?) */
> ?#define AR5K_INIT_RX_LAT_MAX ? ? ? ? ? ? ? ? ? 63
> ?/* Tx latencies from initvals (5212 only but no problem
> diff --git a/drivers/net/wireless/ath/ath5k/pcu.c b/drivers/net/wireless/ath/ath5k/pcu.c
> index ae99e37..cb6e277 100644
> --- a/drivers/net/wireless/ath/ath5k/pcu.c
> +++ b/drivers/net/wireless/ath/ath5k/pcu.c
> @@ -43,14 +43,27 @@
> ?static unsigned int ath5k_hw_get_default_slottime(struct ath5k_hw *ah)
> ?{
> ? ? ? ?struct ieee80211_channel *channel = ah->ah_current_channel;
> + ? ? ? unsigned int slot_time;
>
> - ? ? ? if (channel->hw_value & CHANNEL_TURBO)
> - ? ? ? ? ? ? ? return 6; /* both turbo modes */
> + ? ? ? switch (ah->ah_bwmode) {
> + ? ? ? case AR5K_BWMODE_40MHZ:
> + ? ? ? ? ? ? ? slot_time = AR5K_INIT_SLOT_TIME_TURBO;
> + ? ? ? ? ? ? ? break;
> + ? ? ? case AR5K_BWMODE_10MHZ:
> + ? ? ? ? ? ? ? slot_time = AR5K_INIT_SLOT_TIME_HALF_RATE;
> + ? ? ? ? ? ? ? break;
> + ? ? ? case AR5K_BWMODE_5MHZ:
> + ? ? ? ? ? ? ? slot_time = AR5K_INIT_SLOT_TIME_QUARTER_RATE;
> + ? ? ? ? ? ? ? break;
> + ? ? ? case AR5K_BWMODE_DEFAULT:
> + ? ? ? ? ? ? ? slot_time = AR5K_INIT_SLOT_TIME_DEFAULT;
> + ? ? ? default:
> + ? ? ? ? ? ? ? if (channel->hw_value & CHANNEL_CCK)
> + ? ? ? ? ? ? ? ? ? ? ? slot_time = AR5K_INIT_SLOT_TIME_B;
> + ? ? ? ? ? ? ? break;
> + ? ? ? }
>
> - ? ? ? if (channel->hw_value & CHANNEL_CCK)
> - ? ? ? ? ? ? ? return 20; /* 802.11b */
> -
> - ? ? ? return 9; /* 802.11 a/g */
> + ? ? ? return slot_time;
> ?}
>
> ?/**
> @@ -58,17 +71,30 @@ static unsigned int ath5k_hw_get_default_slottime(struct ath5k_hw *ah)
> ?*
> ?* @ah: The &struct ath5k_hw
> ?*/
> -static unsigned int ath5k_hw_get_default_sifs(struct ath5k_hw *ah)
> +unsigned int ath5k_hw_get_default_sifs(struct ath5k_hw *ah)
> ?{
> ? ? ? ?struct ieee80211_channel *channel = ah->ah_current_channel;
> + ? ? ? unsigned int sifs;
>
> - ? ? ? if (channel->hw_value & CHANNEL_TURBO)
> - ? ? ? ? ? ? ? return 8; /* both turbo modes */
> + ? ? ? switch (ah->ah_bwmode) {
> + ? ? ? case AR5K_BWMODE_40MHZ:
> + ? ? ? ? ? ? ? sifs = AR5K_INIT_SIFS_TURBO;
> + ? ? ? ? ? ? ? break;
> + ? ? ? case AR5K_BWMODE_10MHZ:
> + ? ? ? ? ? ? ? sifs = AR5K_INIT_SIFS_HALF_RATE;
> + ? ? ? ? ? ? ? break;
> + ? ? ? case AR5K_BWMODE_5MHZ:
> + ? ? ? ? ? ? ? sifs = AR5K_INIT_SIFS_QUARTER_RATE;
> + ? ? ? ? ? ? ? break;
> + ? ? ? case AR5K_BWMODE_DEFAULT:
> + ? ? ? ? ? ? ? sifs = AR5K_INIT_SIFS_DEFAULT_BG;
> + ? ? ? default:
> + ? ? ? ? ? ? ? if (channel->hw_value & CHANNEL_5GHZ)
> + ? ? ? ? ? ? ? ? ? ? ? sifs = AR5K_INIT_SIFS_DEFAULT_A;
> + ? ? ? ? ? ? ? break;
> + ? ? ? }
>
> - ? ? ? if (channel->hw_value & CHANNEL_5GHZ)
> - ? ? ? ? ? ? ? return 16; /* 802.11a */
> -
> - ? ? ? return 10; /* 802.11 b/g */
> + ? ? ? return sifs;
> ?}
>
> ?/**
> diff --git a/drivers/net/wireless/ath/ath5k/qcu.c b/drivers/net/wireless/ath/ath5k/qcu.c
> index c422d5c..6eb6838 100644
> --- a/drivers/net/wireless/ath/ath5k/qcu.c
> +++ b/drivers/net/wireless/ath/ath5k/qcu.c
> @@ -297,7 +297,8 @@ int ath5k_hw_reset_tx_queue(struct ath5k_hw *ah, unsigned int queue)
>
> ? ? ? ? ? ? ? ?/* Set Slot time */
> ? ? ? ? ? ? ? ?ath5k_hw_reg_write(ah, (ah->ah_bwmode == AR5K_BWMODE_40MHZ) ?
> - ? ? ? ? ? ? ? ? ? ? ? AR5K_INIT_SLOT_TIME_TURBO : AR5K_INIT_SLOT_TIME,
> + ? ? ? ? ? ? ? ? ? ? ? AR5K_INIT_SLOT_TIME_TURBO_CLOCK :
> + ? ? ? ? ? ? ? ? ? ? ? AR5K_INIT_SLOT_TIME_CLOCK,
> ? ? ? ? ? ? ? ? ? ? ? ?AR5K_SLOT_TIME);
> ? ? ? ? ? ? ? ?/* Set ACK_CTS timeout */
> ? ? ? ? ? ? ? ?ath5k_hw_reg_write(ah, (ah->ah_bwmode == AR5K_BWMODE_40MHZ) ?
> @@ -306,15 +307,16 @@ int ath5k_hw_reset_tx_queue(struct ath5k_hw *ah, unsigned int queue)
>
> ? ? ? ? ? ? ? ?/* Set IFS0 */
> ? ? ? ? ? ? ? ?if (ah->ah_bwmode == AR5K_BWMODE_40MHZ) {
> - ? ? ? ? ? ? ? ? ? ? ? ath5k_hw_reg_write(ah, ((AR5K_INIT_SIFS_TURBO +
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? tq->tqi_aifs * AR5K_INIT_SLOT_TIME_TURBO) <<
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? AR5K_IFS0_DIFS_S) | AR5K_INIT_SIFS_TURBO,
> + ? ? ? ? ? ? ? ? ? ? ? ath5k_hw_reg_write(ah, ((AR5K_INIT_SIFS_TURBO_CLOCK +
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? tq->tqi_aifs * AR5K_INIT_SLOT_TIME_TURBO_CLOCK)
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? << AR5K_IFS0_DIFS_S) |
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? AR5K_INIT_SIFS_TURBO_CLOCK,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?AR5K_IFS0);
> ? ? ? ? ? ? ? ?} else {
> - ? ? ? ? ? ? ? ? ? ? ? ath5k_hw_reg_write(ah, ((AR5K_INIT_SIFS +
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? tq->tqi_aifs * AR5K_INIT_SLOT_TIME) <<
> + ? ? ? ? ? ? ? ? ? ? ? ath5k_hw_reg_write(ah, ((AR5K_INIT_SIFS_CLOCK +
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? tq->tqi_aifs * AR5K_INIT_SLOT_TIME_CLOCK) <<
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?AR5K_IFS0_DIFS_S) |
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? AR5K_INIT_SIFS, AR5K_IFS0);
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? AR5K_INIT_SIFS_CLOCK, AR5K_IFS0);
> ? ? ? ? ? ? ? ?}
>
> ? ? ? ? ? ? ? ?/* Set IFS1 */
> _______________________________________________
> ath5k-devel mailing list
> [email protected]
> https://lists.ath5k.org/mailman/listinfo/ath5k-devel
>

2010-11-24 01:04:50

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH 14/30] ath5k: Extend get_default_sifs/slot_time

2010/11/24 Nick Kossifidis <[email protected]>:
> 2010/11/24 Jonathan Guerin <[email protected]>:
>> On Wed, Nov 24, 2010 at 5:09 AM, Nick Kossifidis <[email protected]> wrote:
>>>  * Extend get_default_sifs/slot_time to include timings for turbo
>>>  half and quarter rate modes.
>>>
>>>  * AR5210 code for now uses timings already on core clock units
>>>  instead of usecs so rename them (we 'll clean it up later).
>>>
>>>  Signed-off-by: Nick Kossifidis <[email protected]>
>>> ---
>>>  drivers/net/wireless/ath/ath5k/ath5k.h |   24 ++++++++++++--
>>>  drivers/net/wireless/ath/ath5k/pcu.c   |   52 ++++++++++++++++++++++++--------
>>>  drivers/net/wireless/ath/ath5k/qcu.c   |   16 +++++----
>>>  3 files changed, 68 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
>>> index 005cad0..e11fc8f 100644
>>> --- a/drivers/net/wireless/ath/ath5k/ath5k.h
>>> +++ b/drivers/net/wireless/ath/ath5k/ath5k.h
>>> @@ -226,16 +226,16 @@
>>>  #define AR5K_INIT_USEC                         39
>>>  #define AR5K_INIT_USEC_TURBO                   79
>>>  #define AR5K_INIT_USEC_32                      31
>>> -#define AR5K_INIT_SLOT_TIME                    396
>>> -#define AR5K_INIT_SLOT_TIME_TURBO              480
>>> +#define AR5K_INIT_SLOT_TIME_CLOCK              396
>>
>> This comes out as 9.9us (with a clock multiplier of 40), rather than 9
>> - should be 360?
>>
>>> +#define AR5K_INIT_SLOT_TIME_TURBO_CLOCK                480
>>>  #define AR5K_INIT_ACK_CTS_TIMEOUT              1024
>>
>> From a previous post I'd made:
>>
>> According to the 802.11-2007 spec document, the ACKTimeout value is
>> (Section 9.2.8 ACK procedure):
>> ACKTimeout = aSIFSTime + aSlotTime + aPHY-RX-START-Delay
>>
>> From Table 17-15—OFDM PHY characteristics, the values are:
>> aSIFSTime = 16
>> aSlotTime = 9
>> aPHY-RX-START-Delay = 25
>>
>> Therefore, ACKTimeout = 50
>>
>> With a value of 1024, the ACKTIMEOUT is actually 25.
>>
>>>  #define AR5K_INIT_ACK_CTS_TIMEOUT_TURBO                0x08000800
>>>  #define AR5K_INIT_PROG_IFS                     920
>>>  #define AR5K_INIT_PROG_IFS_TURBO               960
>>>  #define AR5K_INIT_EIFS                         3440
>>
>> From the 802.11-2007 Spec (9.2.10 DCF timing relations):
>> The EIFS is derived from the SIFS and the DIFS and the length of time
>> it takes to transmit an ACK Control frame at the lowest PHY mandatory
>> rate by the following equation:
>> EIFS = aSIFSTime + DIFS + ACKTxTime
>> where ACKTxTime is the time expressed in microseconds required to
>> transmit an ACK frame, including
>> preamble, PLCP header and any additional PHY dependent information, at
>> the lowest PHY mandatory rate.
>>
>> So, EIFS = 16 + 32 + 44 (ACKTX @ 24Mbps) = 92
>> or 16 + 32 + 60 (ACKTX @ 6Mbps) = 108
>>
>> The current value is 86, which I guess is a nice midpoint, but is
>> there any reasoning for this value? It should really be set to the
>> longest value of 108, as an EIFS is only used when we have no
>> information about a frame because it failed the checksum.
>>
>>>  #define AR5K_INIT_EIFS_TURBO                   6880
>>> -#define AR5K_INIT_SIFS                         560
>>> -#define AR5K_INIT_SIFS_TURBO                   480
>>> +#define AR5K_INIT_SIFS_CLOCK                   560
>>
>> This is wrong, when you divide by the clock rate, you get a SIFS of 14, not 16.
>>
>> Cheers,
>>
>> Jonathan
>>
>
> All _CLOCK variables are left here temporarily until we clean up
> AR5210 stuff later, i know it's wrong ;-)
> Check out patch 16 that adds the new set_ifs_intervals, there we set
> them property, i just left them here so that code can still compile.
>

Also have in mind that clock on g is not 40 but 44 (for b
compatibility) so 396 is correct here (396/44 = 9).

Anyway we clean it up + these are only for AR5210 that we still don't
support, i just put a _CLOCK prefix so that they don't get mixed up
with the correct definitions. Check out patch 16, it should be fine
;-)


--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

2010-11-24 01:35:18

by Jonathan Guerin

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH 14/30] ath5k: Extend get_default_sifs/slot_time

On Wed, Nov 24, 2010 at 11:04 AM, Nick Kossifidis <[email protected]> wrote:
> 2010/11/24 Nick Kossifidis <[email protected]>:
>> 2010/11/24 Jonathan Guerin <[email protected]>:
>>> On Wed, Nov 24, 2010 at 5:09 AM, Nick Kossifidis <[email protected]> wrote:
>>>> ?* Extend get_default_sifs/slot_time to include timings for turbo
>>>> ?half and quarter rate modes.
>>>>
>>>> ?* AR5210 code for now uses timings already on core clock units
>>>> ?instead of usecs so rename them (we 'll clean it up later).
>>>>
>>>> ?Signed-off-by: Nick Kossifidis <[email protected]>
>>>> ---
>>>> ?drivers/net/wireless/ath/ath5k/ath5k.h | ? 24 ++++++++++++--
>>>> ?drivers/net/wireless/ath/ath5k/pcu.c ? | ? 52 ++++++++++++++++++++++++--------
>>>> ?drivers/net/wireless/ath/ath5k/qcu.c ? | ? 16 +++++----
>>>> ?3 files changed, 68 insertions(+), 24 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
>>>> index 005cad0..e11fc8f 100644
>>>> --- a/drivers/net/wireless/ath/ath5k/ath5k.h
>>>> +++ b/drivers/net/wireless/ath/ath5k/ath5k.h
>>>> @@ -226,16 +226,16 @@
>>>> ?#define AR5K_INIT_USEC ? ? ? ? ? ? ? ? ? ? ? ? 39
>>>> ?#define AR5K_INIT_USEC_TURBO ? ? ? ? ? ? ? ? ? 79
>>>> ?#define AR5K_INIT_USEC_32 ? ? ? ? ? ? ? ? ? ? ?31
>>>> -#define AR5K_INIT_SLOT_TIME ? ? ? ? ? ? ? ? ? ?396
>>>> -#define AR5K_INIT_SLOT_TIME_TURBO ? ? ? ? ? ? ?480
>>>> +#define AR5K_INIT_SLOT_TIME_CLOCK ? ? ? ? ? ? ?396
>>>
>>> This comes out as 9.9us (with a clock multiplier of 40), rather than 9
>>> - should be 360?
>>>
>>>> +#define AR5K_INIT_SLOT_TIME_TURBO_CLOCK ? ? ? ? ? ? ? ?480
>>>> ?#define AR5K_INIT_ACK_CTS_TIMEOUT ? ? ? ? ? ? ?1024
>>>
>>> From a previous post I'd made:
>>>
>>> According to the 802.11-2007 spec document, the ACKTimeout value is
>>> (Section 9.2.8 ACK procedure):
>>> ACKTimeout = aSIFSTime + aSlotTime + aPHY-RX-START-Delay
>>>
>>> From Table 17-15?OFDM PHY characteristics, the values are:
>>> aSIFSTime = 16
>>> aSlotTime = 9
>>> aPHY-RX-START-Delay = 25
>>>
>>> Therefore, ACKTimeout = 50
>>>
>>> With a value of 1024, the ACKTIMEOUT is actually 25.
>>>
>>>> ?#define AR5K_INIT_ACK_CTS_TIMEOUT_TURBO ? ? ? ? ? ? ? ?0x08000800
>>>> ?#define AR5K_INIT_PROG_IFS ? ? ? ? ? ? ? ? ? ? 920
>>>> ?#define AR5K_INIT_PROG_IFS_TURBO ? ? ? ? ? ? ? 960
>>>> ?#define AR5K_INIT_EIFS ? ? ? ? ? ? ? ? ? ? ? ? 3440
>>>
>>> From the 802.11-2007 Spec (9.2.10 DCF timing relations):
>>> The EIFS is derived from the SIFS and the DIFS and the length of time
>>> it takes to transmit an ACK Control frame at the lowest PHY mandatory
>>> rate by the following equation:
>>> EIFS = aSIFSTime + DIFS + ACKTxTime
>>> where ACKTxTime is the time expressed in microseconds required to
>>> transmit an ACK frame, including
>>> preamble, PLCP header and any additional PHY dependent information, at
>>> the lowest PHY mandatory rate.
>>>
>>> So, EIFS = 16 + 32 + 44 (ACKTX @ 24Mbps) = 92
>>> or 16 + 32 + 60 (ACKTX @ 6Mbps) = 108
>>>
>>> The current value is 86, which I guess is a nice midpoint, but is
>>> there any reasoning for this value? It should really be set to the
>>> longest value of 108, as an EIFS is only used when we have no
>>> information about a frame because it failed the checksum.
>>>
>>>> ?#define AR5K_INIT_EIFS_TURBO ? ? ? ? ? ? ? ? ? 6880
>>>> -#define AR5K_INIT_SIFS ? ? ? ? ? ? ? ? ? ? ? ? 560
>>>> -#define AR5K_INIT_SIFS_TURBO ? ? ? ? ? ? ? ? ? 480
>>>> +#define AR5K_INIT_SIFS_CLOCK ? ? ? ? ? ? ? ? ? 560
>>>
>>> This is wrong, when you divide by the clock rate, you get a SIFS of 14, not 16.
>>>
>>> Cheers,
>>>
>>> Jonathan
>>>
>>
>> All _CLOCK variables are left here temporarily until we clean up
>> AR5210 stuff later, i know it's wrong ;-)
>> Check out patch 16 that adds the new set_ifs_intervals, there we set
>> them property, i just left them here so that code can still compile.
>>
>
> Also have in mind that clock on g is not 40 but 44 (for b
> compatibility) so 396 is correct here (396/44 = 9).
>
> Anyway we clean it up + these are only for AR5210 that we still don't
> support, i just put a _CLOCK prefix so that they don't get mixed up
> with the correct definitions. Check out patch 16, it should be fine
> ;-)

Oops, my bad on the clock rate. So, for A mode, it's 40, and G mode,
44, or 44 for both?

If that's the case, then some of the values are still wrong (they're
also wrong in patch 16):

EIFS = 3440 / 44 = 78, which is way shorter than it should be! This
gives ath5k an advantage over other 802.11-2007-compliant stations.

ACK/CTS TIMEOUT = 1024/44 = 23, which is still much, much shorter than
the required 50us, once again giving ath5k an advantage for medium
contention.

SIFS = 560/44 = 12, which is shorter again than the required 16us!

Not saying your work is bad, by the way, but just that the values have
been wrong in ath5k for a while. :)

Cheers,

Jonathan
>
>
> --
> GPG ID: 0xD21DB2DB
> As you read this post global entropy rises. Have Fun ;-)
> Nick
>

2010-11-24 00:54:11

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH 14/30] ath5k: Extend get_default_sifs/slot_time

2010/11/24 Jonathan Guerin <[email protected]>:
> On Wed, Nov 24, 2010 at 5:09 AM, Nick Kossifidis <[email protected]> wrote:
>>  * Extend get_default_sifs/slot_time to include timings for turbo
>>  half and quarter rate modes.
>>
>>  * AR5210 code for now uses timings already on core clock units
>>  instead of usecs so rename them (we 'll clean it up later).
>>
>>  Signed-off-by: Nick Kossifidis <[email protected]>
>> ---
>>  drivers/net/wireless/ath/ath5k/ath5k.h |   24 ++++++++++++--
>>  drivers/net/wireless/ath/ath5k/pcu.c   |   52 ++++++++++++++++++++++++--------
>>  drivers/net/wireless/ath/ath5k/qcu.c   |   16 +++++----
>>  3 files changed, 68 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
>> index 005cad0..e11fc8f 100644
>> --- a/drivers/net/wireless/ath/ath5k/ath5k.h
>> +++ b/drivers/net/wireless/ath/ath5k/ath5k.h
>> @@ -226,16 +226,16 @@
>>  #define AR5K_INIT_USEC                         39
>>  #define AR5K_INIT_USEC_TURBO                   79
>>  #define AR5K_INIT_USEC_32                      31
>> -#define AR5K_INIT_SLOT_TIME                    396
>> -#define AR5K_INIT_SLOT_TIME_TURBO              480
>> +#define AR5K_INIT_SLOT_TIME_CLOCK              396
>
> This comes out as 9.9us (with a clock multiplier of 40), rather than 9
> - should be 360?
>
>> +#define AR5K_INIT_SLOT_TIME_TURBO_CLOCK                480
>>  #define AR5K_INIT_ACK_CTS_TIMEOUT              1024
>
> From a previous post I'd made:
>
> According to the 802.11-2007 spec document, the ACKTimeout value is
> (Section 9.2.8 ACK procedure):
> ACKTimeout = aSIFSTime + aSlotTime + aPHY-RX-START-Delay
>
> From Table 17-15—OFDM PHY characteristics, the values are:
> aSIFSTime = 16
> aSlotTime = 9
> aPHY-RX-START-Delay = 25
>
> Therefore, ACKTimeout = 50
>
> With a value of 1024, the ACKTIMEOUT is actually 25.
>
>>  #define AR5K_INIT_ACK_CTS_TIMEOUT_TURBO                0x08000800
>>  #define AR5K_INIT_PROG_IFS                     920
>>  #define AR5K_INIT_PROG_IFS_TURBO               960
>>  #define AR5K_INIT_EIFS                         3440
>
> From the 802.11-2007 Spec (9.2.10 DCF timing relations):
> The EIFS is derived from the SIFS and the DIFS and the length of time
> it takes to transmit an ACK Control frame at the lowest PHY mandatory
> rate by the following equation:
> EIFS = aSIFSTime + DIFS + ACKTxTime
> where ACKTxTime is the time expressed in microseconds required to
> transmit an ACK frame, including
> preamble, PLCP header and any additional PHY dependent information, at
> the lowest PHY mandatory rate.
>
> So, EIFS = 16 + 32 + 44 (ACKTX @ 24Mbps) = 92
> or 16 + 32 + 60 (ACKTX @ 6Mbps) = 108
>
> The current value is 86, which I guess is a nice midpoint, but is
> there any reasoning for this value? It should really be set to the
> longest value of 108, as an EIFS is only used when we have no
> information about a frame because it failed the checksum.
>
>>  #define AR5K_INIT_EIFS_TURBO                   6880
>> -#define AR5K_INIT_SIFS                         560
>> -#define AR5K_INIT_SIFS_TURBO                   480
>> +#define AR5K_INIT_SIFS_CLOCK                   560
>
> This is wrong, when you divide by the clock rate, you get a SIFS of 14, not 16.
>
> Cheers,
>
> Jonathan
>

All _CLOCK variables are left here temporarily until we clean up
AR5210 stuff later, i know it's wrong ;-)
Check out patch 16 that adds the new set_ifs_intervals, there we set
them property, i just left them here so that code can still compile.

--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

2010-11-24 01:41:45

by Jonathan Guerin

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH 14/30] ath5k: Extend get_default_sifs/slot_time

On Wed, Nov 24, 2010 at 11:35 AM, Jonathan Guerin <[email protected]> wrote:
> On Wed, Nov 24, 2010 at 11:04 AM, Nick Kossifidis <[email protected]> wrote:
>> 2010/11/24 Nick Kossifidis <[email protected]>:
>>> 2010/11/24 Jonathan Guerin <[email protected]>:
>>>> On Wed, Nov 24, 2010 at 5:09 AM, Nick Kossifidis <[email protected]> wrote:
>>>>> ?* Extend get_default_sifs/slot_time to include timings for turbo
>>>>> ?half and quarter rate modes.
>>>>>
>>>>> ?* AR5210 code for now uses timings already on core clock units
>>>>> ?instead of usecs so rename them (we 'll clean it up later).
>>>>>
>>>>> ?Signed-off-by: Nick Kossifidis <[email protected]>
>>>>> ---
>>>>> ?drivers/net/wireless/ath/ath5k/ath5k.h | ? 24 ++++++++++++--
>>>>> ?drivers/net/wireless/ath/ath5k/pcu.c ? | ? 52 ++++++++++++++++++++++++--------
>>>>> ?drivers/net/wireless/ath/ath5k/qcu.c ? | ? 16 +++++----
>>>>> ?3 files changed, 68 insertions(+), 24 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
>>>>> index 005cad0..e11fc8f 100644
>>>>> --- a/drivers/net/wireless/ath/ath5k/ath5k.h
>>>>> +++ b/drivers/net/wireless/ath/ath5k/ath5k.h
>>>>> @@ -226,16 +226,16 @@
>>>>> ?#define AR5K_INIT_USEC ? ? ? ? ? ? ? ? ? ? ? ? 39
>>>>> ?#define AR5K_INIT_USEC_TURBO ? ? ? ? ? ? ? ? ? 79
>>>>> ?#define AR5K_INIT_USEC_32 ? ? ? ? ? ? ? ? ? ? ?31
>>>>> -#define AR5K_INIT_SLOT_TIME ? ? ? ? ? ? ? ? ? ?396
>>>>> -#define AR5K_INIT_SLOT_TIME_TURBO ? ? ? ? ? ? ?480
>>>>> +#define AR5K_INIT_SLOT_TIME_CLOCK ? ? ? ? ? ? ?396
>>>>
>>>> This comes out as 9.9us (with a clock multiplier of 40), rather than 9
>>>> - should be 360?
>>>>
>>>>> +#define AR5K_INIT_SLOT_TIME_TURBO_CLOCK ? ? ? ? ? ? ? ?480
>>>>> ?#define AR5K_INIT_ACK_CTS_TIMEOUT ? ? ? ? ? ? ?1024
>>>>
>>>> From a previous post I'd made:
>>>>
>>>> According to the 802.11-2007 spec document, the ACKTimeout value is
>>>> (Section 9.2.8 ACK procedure):
>>>> ACKTimeout = aSIFSTime + aSlotTime + aPHY-RX-START-Delay
>>>>
>>>> From Table 17-15?OFDM PHY characteristics, the values are:
>>>> aSIFSTime = 16
>>>> aSlotTime = 9
>>>> aPHY-RX-START-Delay = 25
>>>>
>>>> Therefore, ACKTimeout = 50
>>>>
>>>> With a value of 1024, the ACKTIMEOUT is actually 25.
>>>>
>>>>> ?#define AR5K_INIT_ACK_CTS_TIMEOUT_TURBO ? ? ? ? ? ? ? ?0x08000800
>>>>> ?#define AR5K_INIT_PROG_IFS ? ? ? ? ? ? ? ? ? ? 920
>>>>> ?#define AR5K_INIT_PROG_IFS_TURBO ? ? ? ? ? ? ? 960
>>>>> ?#define AR5K_INIT_EIFS ? ? ? ? ? ? ? ? ? ? ? ? 3440
>>>>
>>>> From the 802.11-2007 Spec (9.2.10 DCF timing relations):
>>>> The EIFS is derived from the SIFS and the DIFS and the length of time
>>>> it takes to transmit an ACK Control frame at the lowest PHY mandatory
>>>> rate by the following equation:
>>>> EIFS = aSIFSTime + DIFS + ACKTxTime
>>>> where ACKTxTime is the time expressed in microseconds required to
>>>> transmit an ACK frame, including
>>>> preamble, PLCP header and any additional PHY dependent information, at
>>>> the lowest PHY mandatory rate.
>>>>
>>>> So, EIFS = 16 + 32 + 44 (ACKTX @ 24Mbps) = 92
>>>> or 16 + 32 + 60 (ACKTX @ 6Mbps) = 108
>>>>
>>>> The current value is 86, which I guess is a nice midpoint, but is
>>>> there any reasoning for this value? It should really be set to the
>>>> longest value of 108, as an EIFS is only used when we have no
>>>> information about a frame because it failed the checksum.
>>>>
>>>>> ?#define AR5K_INIT_EIFS_TURBO ? ? ? ? ? ? ? ? ? 6880
>>>>> -#define AR5K_INIT_SIFS ? ? ? ? ? ? ? ? ? ? ? ? 560
>>>>> -#define AR5K_INIT_SIFS_TURBO ? ? ? ? ? ? ? ? ? 480
>>>>> +#define AR5K_INIT_SIFS_CLOCK ? ? ? ? ? ? ? ? ? 560
>>>>
>>>> This is wrong, when you divide by the clock rate, you get a SIFS of 14, not 16.
>>>>
>>>> Cheers,
>>>>
>>>> Jonathan
>>>>
>>>
>>> All _CLOCK variables are left here temporarily until we clean up
>>> AR5210 stuff later, i know it's wrong ;-)
>>> Check out patch 16 that adds the new set_ifs_intervals, there we set
>>> them property, i just left them here so that code can still compile.
>>>
>>
>> Also have in mind that clock on g is not 40 but 44 (for b
>> compatibility) so 396 is correct here (396/44 = 9).
>>
>> Anyway we clean it up + these are only for AR5210 that we still don't
>> support, i just put a _CLOCK prefix so that they don't get mixed up
>> with the correct definitions. Check out patch 16, it should be fine
>> ;-)
>
> Oops, my bad on the clock rate. So, for A mode, it's 40, and G mode,
> 44, or 44 for both?
>
> If that's the case, then some of the values are still wrong (they're
> also wrong in patch 16):
>
> EIFS = 3440 / 44 = 78, which is way shorter than it should be! This
> gives ath5k an advantage over other 802.11-2007-compliant stations.
>
> ACK/CTS TIMEOUT = 1024/44 = 23, which is still much, much shorter than
> the required 50us, once again giving ath5k an advantage for medium
> contention.
>
> SIFS = 560/44 = 12, which is shorter again than the required 16us!
>
> Not saying your work is bad, by the way, but just that the values have
> been wrong in ath5k for a while. :)
>
> Cheers,
>
> Jonathan


Never mind, I didn't read Patch 16 properly... :(

Cheers,

Jonathan

>>
>>
>> --
>> GPG ID: 0xD21DB2DB
>> As you read this post global entropy rises. Have Fun ;-)
>> Nick
>>
>