* [PATCH] ath9k: Fix rate table choice
@ 2010-02-04 21:06 Benoit Papillault
2010-02-05 0:14 ` Luis R. Rodriguez
2010-02-05 4:51 ` Sujith
0 siblings, 2 replies; 6+ messages in thread
From: Benoit Papillault @ 2010-02-04 21:06 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, Benoit Papillault
ath9k rate control choose the rate table based on the current channel
(if it is ht40 or not). However, it happens that at this stage, the
channel is still ht20 even if the peer sta capabilities would later
allow ht40. So let's use peer sta capabilities directly.
Signed-off-by: Benoit Papillault <benoit.papillault@free.fr>
---
drivers/net/wireless/ath/ath9k/rc.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 1196884..5ce9da5 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -1341,10 +1341,8 @@ static void ath_rate_update(void *priv, struct ieee80211_supported_band *sband,
if (sc->sc_ah->opmode != NL80211_IFTYPE_STATION)
return;
- if (sc->hw->conf.channel_type == NL80211_CHAN_HT40MINUS ||
- sc->hw->conf.channel_type == NL80211_CHAN_HT40PLUS)
- oper_cw40 = true;
-
+ oper_cw40 = (sta->ht_cap.cap &
+ IEEE80211_HT_CAP_SUP_WIDTH_20_40) ? true : false;
oper_sgi40 = (sta->ht_cap.cap & IEEE80211_HT_CAP_SGI_40) ?
true : false;
--
1.6.3.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ath9k: Fix rate table choice
2010-02-04 21:06 [PATCH] ath9k: Fix rate table choice Benoit Papillault
@ 2010-02-05 0:14 ` Luis R. Rodriguez
2010-02-05 0:36 ` Benoit PAPILLAULT
2010-02-05 4:51 ` Sujith
1 sibling, 1 reply; 6+ messages in thread
From: Luis R. Rodriguez @ 2010-02-05 0:14 UTC (permalink / raw)
To: Benoit Papillault; +Cc: johannes, linux-wireless
On Thu, Feb 4, 2010 at 1:06 PM, Benoit Papillault
<benoit.papillault@free.fr> wrote:
> ath9k rate control choose the rate table based on the current channel
> (if it is ht40 or not). However, it happens that at this stage, the
> channel is still ht20 even if the peer sta capabilities would later
> allow ht40. So let's use peer sta capabilities directly.
Can you clarify this commit log entry a little more, I don't think I
get it yet. Also, is this a stable fix? If so, please Cc:
stable@kernel.org on the commit log entry.
Luis
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ath9k: Fix rate table choice
2010-02-05 0:14 ` Luis R. Rodriguez
@ 2010-02-05 0:36 ` Benoit PAPILLAULT
0 siblings, 0 replies; 6+ messages in thread
From: Benoit PAPILLAULT @ 2010-02-05 0:36 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: johannes, linux-wireless
Luis R. Rodriguez a écrit :
> On Thu, Feb 4, 2010 at 1:06 PM, Benoit Papillault
> <benoit.papillault@free.fr> wrote:
>
>> ath9k rate control choose the rate table based on the current channel
>> (if it is ht40 or not). However, it happens that at this stage, the
>> channel is still ht20 even if the peer sta capabilities would later
>> allow ht40. So let's use peer sta capabilities directly.
>>
>
> Can you clarify this commit log entry a little more, I don't think I
> get it yet. Also, is this a stable fix? If so, please Cc:
> stable@kernel.org on the commit log entry.
>
> Luis
>
I think it could be Cc:stable@kernel.org, but I'll leave the decision up
to you.
In fact, I was using ath9k as a STA connected to an HT40 AP. However,
only HT20 was displayed in ath9k/phy0/rcstat.
After digging for a while, i found that the current channel
(sc->hw->conf.channel_type) was indeed an HT20 (or no-ht) channel!
In fact, association is handled as a queued work (from what I
understood). As such, it uses tmp_channel (see mac80211/work.c). In the
enable_ht() [mac80211/mlme.c], we update oper_channel_type before
calling ieee80211_hw_config to call the hw_config driver callback that
upates conf.channel_type. However, since we are using queued work,
ieee80211_hw_config will use tmp_channel instead. Then enable_ht calls
rate_control_rate_update that calls ath9k rate control callbacks ...
where the channel is ... not HT40! I guess that when the queued work is
done, the channel is switched again to ht40.
I would appreciate Johannes point of view here...
After applying this patch, ath9k/phy0/rcstat shows both ht20 and ht40 rates.
Regards,
Benoit
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] ath9k: Fix rate table choice
2010-02-04 21:06 [PATCH] ath9k: Fix rate table choice Benoit Papillault
2010-02-05 0:14 ` Luis R. Rodriguez
@ 2010-02-05 4:51 ` Sujith
2010-02-05 7:21 ` Benoit PAPILLAULT
1 sibling, 1 reply; 6+ messages in thread
From: Sujith @ 2010-02-05 4:51 UTC (permalink / raw)
To: Benoit Papillault
Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org
Benoit Papillault wrote:
> ath9k rate control choose the rate table based on the current channel
> (if it is ht40 or not). However, it happens that at this stage, the
> channel is still ht20 even if the peer sta capabilities would later
> allow ht40. So let's use peer sta capabilities directly.
>
> Signed-off-by: Benoit Papillault <benoit.papillault@free.fr>
> ---
> drivers/net/wireless/ath/ath9k/rc.c | 6 ++----
> 1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
> index 1196884..5ce9da5 100644
> --- a/drivers/net/wireless/ath/ath9k/rc.c
> +++ b/drivers/net/wireless/ath/ath9k/rc.c
> @@ -1341,10 +1341,8 @@ static void ath_rate_update(void *priv, struct ieee80211_supported_band *sband,
> if (sc->sc_ah->opmode != NL80211_IFTYPE_STATION)
> return;
>
> - if (sc->hw->conf.channel_type == NL80211_CHAN_HT40MINUS ||
> - sc->hw->conf.channel_type == NL80211_CHAN_HT40PLUS)
> - oper_cw40 = true;
> -
> + oper_cw40 = (sta->ht_cap.cap &
> + IEEE80211_HT_CAP_SUP_WIDTH_20_40) ? true : false;
> oper_sgi40 = (sta->ht_cap.cap & IEEE80211_HT_CAP_SGI_40) ?
> true : false;
>
This is wrong, the AP can certainly support HT40/HT20 and switch between them
dynamically - this patch changes how ath9k reacts to channel switching.
The AP can switch its operating channel to HT20 - but the HT40 supported bit
will _still_ be set. Can you capture the association sequence and check the
HT_INFO IE ( which is different from HT CAP) ?
Sujith
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ath9k: Fix rate table choice
2010-02-05 4:51 ` Sujith
@ 2010-02-05 7:21 ` Benoit PAPILLAULT
2010-02-05 7:57 ` Sujith
0 siblings, 1 reply; 6+ messages in thread
From: Benoit PAPILLAULT @ 2010-02-05 7:21 UTC (permalink / raw)
To: Sujith; +Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org
Sujith a écrit :
> Benoit Papillault wrote:
>
>> ath9k rate control choose the rate table based on the current channel
>> (if it is ht40 or not). However, it happens that at this stage, the
>> channel is still ht20 even if the peer sta capabilities would later
>> allow ht40. So let's use peer sta capabilities directly.
>>
>> Signed-off-by: Benoit Papillault <benoit.papillault@free.fr>
>> ---
>> drivers/net/wireless/ath/ath9k/rc.c | 6 ++----
>> 1 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
>> index 1196884..5ce9da5 100644
>> --- a/drivers/net/wireless/ath/ath9k/rc.c
>> +++ b/drivers/net/wireless/ath/ath9k/rc.c
>> @@ -1341,10 +1341,8 @@ static void ath_rate_update(void *priv, struct ieee80211_supported_band *sband,
>> if (sc->sc_ah->opmode != NL80211_IFTYPE_STATION)
>> return;
>>
>> - if (sc->hw->conf.channel_type == NL80211_CHAN_HT40MINUS ||
>> - sc->hw->conf.channel_type == NL80211_CHAN_HT40PLUS)
>> - oper_cw40 = true;
>> -
>> + oper_cw40 = (sta->ht_cap.cap &
>> + IEEE80211_HT_CAP_SUP_WIDTH_20_40) ? true : false;
>> oper_sgi40 = (sta->ht_cap.cap & IEEE80211_HT_CAP_SGI_40) ?
>> true : false;
>>
>>
>
> This is wrong, the AP can certainly support HT40/HT20 and switch between them
> dynamically - this patch changes how ath9k reacts to channel switching.
>
Sure, but when the channel is switched, ath9k rate control is not called
AFAIK. So nothing is updated.
ath_rate_update is called by rate_control_rate_update, which is only
called by enable_ht. enable_ht is called as association time and on RX
beacon. However, since the content of RX beacon is not changed,
rate_control_rate_update will not be called again. That's what I show in
my logs...
> The AP can switch its operating channel to HT20 - but the HT40 supported bit
> will _still_ be set. Can you capture the association sequence and check the
> HT_INFO IE ( which is different from HT CAP) ?
>
> Sujith
>
Will try to do it. I see your point :
- current code never enables ht40 rate table, but the rate table if
updated if the AP switch to an ht20 operating channel.
- my patch correctly enables ht40 rate table, but is not updated if the
AP switch to an ht20 operating channel.
Maybe local->oper_channel_type could be passed directly to
rate_control_rate_update?
Or as Johannes suggest, we could always select the ht40 rate table and
adapt in ath_get_rate according to hw.conf.channel_type dynamically?
Regards,
Benoit
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ath9k: Fix rate table choice
2010-02-05 7:21 ` Benoit PAPILLAULT
@ 2010-02-05 7:57 ` Sujith
0 siblings, 0 replies; 6+ messages in thread
From: Sujith @ 2010-02-05 7:57 UTC (permalink / raw)
To: Benoit PAPILLAULT
Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org
Benoit PAPILLAULT wrote:
> Sure, but when the channel is switched, ath9k rate control is not called
> AFAIK. So nothing is updated.
> ath_rate_update is called by rate_control_rate_update, which is only
> called by enable_ht. enable_ht is called as association time and on RX
> beacon. However, since the content of RX beacon is not changed,
> rate_control_rate_update will not be called again. That's what I show in
> my logs...
Can you post the ath9k logs with debug=0x200 ?
> Will try to do it. I see your point :
> - current code never enables ht40 rate table, but the rate table if
> updated if the AP switch to an ht20 operating channel.
Hm, if the channel type is HT40, then ath9k would enable HT40 in RC.
Sujith
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-02-05 7:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-04 21:06 [PATCH] ath9k: Fix rate table choice Benoit Papillault
2010-02-05 0:14 ` Luis R. Rodriguez
2010-02-05 0:36 ` Benoit PAPILLAULT
2010-02-05 4:51 ` Sujith
2010-02-05 7:21 ` Benoit PAPILLAULT
2010-02-05 7:57 ` Sujith
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.