[PATCH] ath5k: fixing retries in ath5k_hw_setup_4word_tx_desc

[PATCH] ath5k: fixing retries in ath5k_hw_setup_4word_tx_desc

[Andrew Blaich]

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 <ablaich@gmail.com>
---
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;

Re: [PATCH] ath5k: fixing retries in ath5k_hw_setup_4word_tx_desc

[Benoit PAPILLAULT]

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 <ablaich@gmail.com>
> ---
> 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 majordomo@vger.kernel.org
> 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 <benoit.papillault@free.fr>

Regards,
Benoit

Re: [PATCH] ath5k: fixing retries in ath5k_hw_setup_4word_tx_desc

[Bob Copeland]

On Mon, Mar 1, 2010 at 10:30 AM, Andrew Blaich <ablaich@gmail.com> 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 <ablaich@gmail.com>
> ---
> 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 %% www.bobcopeland.com

Re: [PATCH] ath5k: fixing retries in ath5k_hw_setup_4word_tx_desc

[Andrew Blaich]

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 <ablaich@gmail.com>
---
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 <me@bobcopeland.com> wrote:
> On Mon, Mar 1, 2010 at 10:30 AM, Andrew Blaich <ablaich@gmail.com> 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 <ablaich@gmail.com>
>> ---
>> 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 %% www.bobcopeland.com
>

Re: [PATCH] ath5k: fixing retries in ath5k_hw_setup_4word_tx_desc

[Bob Copeland]

Great, but:

On Thu, Mar 4, 2010 at 9:21 AM, Andrew Blaich <ablaich@gmail.com> 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 <ablaich@gmail.com>

"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 %% www.bobcopeland.com

Re: [PATCH] ath5k: fixing retries in ath5k_hw_setup_4word_tx_desc

[Andrew Blaich]

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 <ablaich@gmail.com>
---
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;

Re: [PATCH] ath5k: fixing retries in ath5k_hw_setup_4word_tx_desc

[Andrew Blaich]

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 <ablaich@gmail.com>
---
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;