Return-path: Received: from mail-vw0-f46.google.com ([209.85.212.46]:38469 "EHLO mail-vw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755581Ab0KXAbU convert rfc822-to-8bit (ORCPT ); Tue, 23 Nov 2010 19:31:20 -0500 Received: by vws13 with SMTP id 13so4779652vws.19 for ; Tue, 23 Nov 2010 16:31:19 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20101123190911.GN4303@makis.mantri> References: <20101123190911.GN4303@makis.mantri> From: Jonathan Guerin Date: Wed, 24 Nov 2010 10:31:04 +1000 Message-ID: Subject: Re: [ath5k-devel] [PATCH 14/30] ath5k: Extend get_default_sifs/slot_time To: ath5k-devel@lists.ath5k.org, linux-wireless@vger.kernel.org, linville@tuxdriver.com, me@bobcopeland.com, mcgrof@gmail.com, jirislaby@gmail.com, nbd@openwrt.org, br1@einfach.org Content-Type: text/plain; charset=windows-1252 Sender: linux-wireless-owner@vger.kernel.org List-ID: On Wed, Nov 24, 2010 at 5:09 AM, Nick Kossifidis 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 > --- > ?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 > ath5k-devel@lists.ath5k.org > https://lists.ath5k.org/mailman/listinfo/ath5k-devel >