2010-03-01 15:30:41

by Andrew Blaich

[permalink] [raw]
Subject: [PATCH] ath5k: fixing retries in ath5k_hw_setup_4word_tx_desc

This Patch:

The rate control algorithm, default is Minstrel for ath5k, determines
the number of retries to use for each rate. However, there exists in
ath5k_hw_setup_4word_tx_desc (which is called for AR5212 like devices)
a set number of retries defined by AR5K_TUNE_HWTXTRIES. The set
number of tries is added to the tx_tries0 variable setup by the rate
control algorithm. This changes the number of retries the rate
control algorithm considers necessary. By removing the
AR5K_TUNE_HWTXTRIES from the retry calculation the rate control
algorithm is given control over the number of retries.


Signed-off-by:: Andrew Blaich <[email protected]>
---
diff --git a/drivers/net/wireless/ath/ath5k/desc.c
b/drivers/net/wireless/ath/ath5k/desc.c
index dc30a2b..c18d8d4 100644
--- a/drivers/net/wireless/ath/ath5k/desc.c
+++ b/drivers/net/wireless/ath/ath5k/desc.c
@@ -229,7 +229,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
AR5K_REG_SM(antenna_mode, AR5K_4W_TX_DESC_CTL0_ANT_MODE_XMIT);
tx_ctl->tx_control_1 |= AR5K_REG_SM(type,
AR5K_4W_TX_DESC_CTL1_FRAME_TYPE);
- tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0 + AR5K_TUNE_HWTXTRIES,
+ tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0,
AR5K_4W_TX_DESC_CTL2_XMIT_TRIES0);
tx_ctl->tx_control_3 = tx_rate0 & AR5K_4W_TX_DESC_CTL3_XMIT_RATE0;


2010-03-15 19:54:17

by Andrew Blaich

[permalink] [raw]
Subject: Re: [PATCH] ath5k: fixing retries in ath5k_hw_setup_4word_tx_desc

Ignore previous email, quoted text was included.

Removing AR5K_TUNE_HWTXTRIES from the Ath5k code as it adds in 4
retries on top of what is already in tx_tries0. By removing
AR5K_TUNE_HWTXTRIES, the rate control algorithm is given full control
in setting the number of retries.

Signed-off-by:: Andrew Blaich <[email protected]>
---
diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h
b/drivers/net/wireless/ath/ath5k/ath5k.h
index ac67f02..1c5834a 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -202,7 +202,6 @@
#define AR5K_TUNE_MAX_TXPOWER 63
#define AR5K_TUNE_DEFAULT_TXPOWER 25
#define AR5K_TUNE_TPC_TXPOWER false
-#define AR5K_TUNE_HWTXTRIES 4

#define AR5K_INIT_CARR_SENSE_EN 1

diff --git a/drivers/net/wireless/ath/ath5k/desc.c
b/drivers/net/wireless/ath/ath5k/desc.c
index dc30a2b..c18d8d4 100644
--- a/drivers/net/wireless/ath/ath5k/desc.c
+++ b/drivers/net/wireless/ath/ath5k/desc.c
@@ -229,7 +229,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
AR5K_REG_SM(antenna_mode, AR5K_4W_TX_DESC_CTL0_ANT_MODE_XMIT);
tx_ctl->tx_control_1 |= AR5K_REG_SM(type,
AR5K_4W_TX_DESC_CTL1_FRAME_TYPE);
- tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0 + AR5K_TUNE_HWTXTRIES,
+ tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0,
AR5K_4W_TX_DESC_CTL2_XMIT_TRIES0);
tx_ctl->tx_control_3 = tx_rate0 & AR5K_4W_TX_DESC_CTL3_XMIT_RATE0;

2010-03-05 13:15:16

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH] ath5k: fixing retries in ath5k_hw_setup_4word_tx_desc

Great, but:

On Thu, Mar 4, 2010 at 9:21 AM, Andrew Blaich <[email protected]> wrote:
> Updating the patch to include the removal of ?the define of
> AR5K_TUNE_HWTXTRIES per Benoit's suggestion as the patch makes it no
> longer necessary.

Please repost the patch as a follow-up to the thread, but
without the quoted discussion. This allows John to take the
email directly once it's ready, without editing the text.

You should always write the description as if it were going
into the commit log as-is. If you include changelog
information (and you should), put it after the '---' with
the diffstat. E.g.

ath5k: title here.

The original description of the brokenness of the tunable
here, written as if no one knew about the first patch.

Signed-off-by: here

---
Anything you want to write can go here, it will get
removed by git-am.

v2: remove AR5K_TUNE_HWTXTRIES per Benoit's comments

[diffstat]
---
[patch]

> Signed-off-by:: Andrew Blaich <[email protected]>

"Signed-off-by:" i.e. one ':'

Also you should avoid a lead-in like "This patch" to the
extent possible. See:

http://userweb.kernel.org/~akpm/stuff/tpp.txt

--
Bob Copeland %% http://www.bobcopeland.com

2010-03-15 19:50:04

by Andrew Blaich

[permalink] [raw]
Subject: Re: [PATCH] ath5k: fixing retries in ath5k_hw_setup_4word_tx_desc

Removing AR5K_TUNE_HWTXTRIES from the Ath5k code as it adds in 4
retries on top of what is already in tx_tries0. By removing
AR5K_TUNE_HWTXTRIES, the rate control algorithm is given full control
in setting the number of retries.

Signed-off-by:: Andrew Blaich <[email protected]>
---
diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h
b/drivers/net/wireless/ath/ath5k/ath5k.h
index ac67f02..1c5834a 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -202,7 +202,6 @@
#define AR5K_TUNE_MAX_TXPOWER 63
#define AR5K_TUNE_DEFAULT_TXPOWER 25
#define AR5K_TUNE_TPC_TXPOWER false
-#define AR5K_TUNE_HWTXTRIES 4

#define AR5K_INIT_CARR_SENSE_EN 1

diff --git a/drivers/net/wireless/ath/ath5k/desc.c
b/drivers/net/wireless/ath/ath5k/desc.c
index dc30a2b..c18d8d4 100644
--- a/drivers/net/wireless/ath/ath5k/desc.c
+++ b/drivers/net/wireless/ath/ath5k/desc.c
@@ -229,7 +229,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
AR5K_REG_SM(antenna_mode, AR5K_4W_TX_DESC_CTL0_ANT_MODE_XMIT);
tx_ctl->tx_control_1 |= AR5K_REG_SM(type,
AR5K_4W_TX_DESC_CTL1_FRAME_TYPE);
- tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0 + AR5K_TUNE_HWTXTRIES,
+ tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0,
AR5K_4W_TX_DESC_CTL2_XMIT_TRIES0);
tx_ctl->tx_control_3 = tx_rate0 & AR5K_4W_TX_DESC_CTL3_XMIT_RATE0;

2010-03-03 22:03:49

by Benoit Papillault

[permalink] [raw]
Subject: Re: [PATCH] ath5k: fixing retries in ath5k_hw_setup_4word_tx_desc

Andrew Blaich a ?crit :
> This Patch:
>
> The rate control algorithm, default is Minstrel for ath5k, determines
> the number of retries to use for each rate. However, there exists in
> ath5k_hw_setup_4word_tx_desc (which is called for AR5212 like devices)
> a set number of retries defined by AR5K_TUNE_HWTXTRIES. The set
> number of tries is added to the tx_tries0 variable setup by the rate
> control algorithm. This changes the number of retries the rate
> control algorithm considers necessary. By removing the
> AR5K_TUNE_HWTXTRIES from the retry calculation the rate control
> algorithm is given control over the number of retries.
>
>
> Signed-off-by:: Andrew Blaich <[email protected]>
> ---
> diff --git a/drivers/net/wireless/ath/ath5k/desc.c
> b/drivers/net/wireless/ath/ath5k/desc.c
> index dc30a2b..c18d8d4 100644
> --- a/drivers/net/wireless/ath/ath5k/desc.c
> +++ b/drivers/net/wireless/ath/ath5k/desc.c
> @@ -229,7 +229,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
> AR5K_REG_SM(antenna_mode, AR5K_4W_TX_DESC_CTL0_ANT_MODE_XMIT);
> tx_ctl->tx_control_1 |= AR5K_REG_SM(type,
> AR5K_4W_TX_DESC_CTL1_FRAME_TYPE);
> - tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0 + AR5K_TUNE_HWTXTRIES,
> + tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0,
> AR5K_4W_TX_DESC_CTL2_XMIT_TRIES0);
> tx_ctl->tx_control_3 = tx_rate0 & AR5K_4W_TX_DESC_CTL3_XMIT_RATE0;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
Good catch. If I were you, I would remove the #define in ath5.h as well
since it is no longer use.

Tested & Acked-by: Benoit Papillault <[email protected]>

Regards,
Benoit


2010-03-03 23:30:53

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH] ath5k: fixing retries in ath5k_hw_setup_4word_tx_desc

On Mon, Mar 1, 2010 at 10:30 AM, Andrew Blaich <[email protected]> wrote:
> This Patch:
>
> The rate control algorithm, default is Minstrel for ath5k, determines
> the number of retries to use for each rate. ?However, there exists in
> ath5k_hw_setup_4word_tx_desc (which is called for AR5212 like devices)
> a set number of retries defined by AR5K_TUNE_HWTXTRIES. ?The set
> number of tries is added to the tx_tries0 variable setup by the rate
> control algorithm. ?This changes the number of retries the rate
> control algorithm considers necessary. ?By removing the
> AR5K_TUNE_HWTXTRIES from the retry calculation the rate control
> algorithm is given control over the number of retries.
>
>
> Signed-off-by:: Andrew Blaich <[email protected]>
> ---
> diff --git a/drivers/net/wireless/ath/ath5k/desc.c
> b/drivers/net/wireless/ath/ath5k/desc.c
> index dc30a2b..c18d8d4 100644
> --- a/drivers/net/wireless/ath/ath5k/desc.c
> +++ b/drivers/net/wireless/ath/ath5k/desc.c
> @@ -229,7 +229,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
> ? ? ? ? ? ? ? ?AR5K_REG_SM(antenna_mode, AR5K_4W_TX_DESC_CTL0_ANT_MODE_XMIT);
> ? ? ? ?tx_ctl->tx_control_1 |= AR5K_REG_SM(type,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?AR5K_4W_TX_DESC_CTL1_FRAME_TYPE);
> - ? ? ? tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0 + AR5K_TUNE_HWTXTRIES,
> + ? ? ? tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0,

Thanks!

--
Bob Copeland %% http://www.bobcopeland.com

2010-03-04 14:21:48

by Andrew Blaich

[permalink] [raw]
Subject: Re: [PATCH] ath5k: fixing retries in ath5k_hw_setup_4word_tx_desc

Updating the patch to include the removal of the define of
AR5K_TUNE_HWTXTRIES per Benoit's suggestion as the patch makes it no
longer necessary.

Signed-off-by:: Andrew Blaich <[email protected]>
---
diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h
b/drivers/net/wireless/ath/ath5k/ath5k.h
index ac67f02..1c5834a 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -202,7 +202,6 @@
#define AR5K_TUNE_MAX_TXPOWER 63
#define AR5K_TUNE_DEFAULT_TXPOWER 25
#define AR5K_TUNE_TPC_TXPOWER false
-#define AR5K_TUNE_HWTXTRIES 4

#define AR5K_INIT_CARR_SENSE_EN 1

diff --git a/drivers/net/wireless/ath/ath5k/desc.c
b/drivers/net/wireless/ath/ath5k/desc.c
index dc30a2b..c18d8d4 100644
--- a/drivers/net/wireless/ath/ath5k/desc.c
+++ b/drivers/net/wireless/ath/ath5k/desc.c
@@ -229,7 +229,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
AR5K_REG_SM(antenna_mode, AR5K_4W_TX_DESC_CTL0_ANT_MODE_XMIT);
tx_ctl->tx_control_1 |= AR5K_REG_SM(type,
AR5K_4W_TX_DESC_CTL1_FRAME_TYPE);
- tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0 + AR5K_TUNE_HWTXTRIES,
+ tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0,
AR5K_4W_TX_DESC_CTL2_XMIT_TRIES0);
tx_ctl->tx_control_3 = tx_rate0 & AR5K_4W_TX_DESC_CTL3_XMIT_RATE0;


On Wed, Mar 3, 2010 at 6:30 PM, Bob Copeland <[email protected]> wrote:
> On Mon, Mar 1, 2010 at 10:30 AM, Andrew Blaich <[email protected]> wrote:
>> This Patch:
>>
>> The rate control algorithm, default is Minstrel for ath5k, determines
>> the number of retries to use for each rate. ?However, there exists in
>> ath5k_hw_setup_4word_tx_desc (which is called for AR5212 like devices)
>> a set number of retries defined by AR5K_TUNE_HWTXTRIES. ?The set
>> number of tries is added to the tx_tries0 variable setup by the rate
>> control algorithm. ?This changes the number of retries the rate
>> control algorithm considers necessary. ?By removing the
>> AR5K_TUNE_HWTXTRIES from the retry calculation the rate control
>> algorithm is given control over the number of retries.
>>
>>
>> Signed-off-by:: Andrew Blaich <[email protected]>
>> ---
>> diff --git a/drivers/net/wireless/ath/ath5k/desc.c
>> b/drivers/net/wireless/ath/ath5k/desc.c
>> index dc30a2b..c18d8d4 100644
>> --- a/drivers/net/wireless/ath/ath5k/desc.c
>> +++ b/drivers/net/wireless/ath/ath5k/desc.c
>> @@ -229,7 +229,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
>> ? ? ? ? ? ? ? ?AR5K_REG_SM(antenna_mode, AR5K_4W_TX_DESC_CTL0_ANT_MODE_XMIT);
>> ? ? ? ?tx_ctl->tx_control_1 |= AR5K_REG_SM(type,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?AR5K_4W_TX_DESC_CTL1_FRAME_TYPE);
>> - ? ? ? tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0 + AR5K_TUNE_HWTXTRIES,
>> + ? ? ? tx_ctl->tx_control_2 = AR5K_REG_SM(tx_tries0,
>
> Thanks!
>
> --
> Bob Copeland %% http://www.bobcopeland.com
>