diff mbox

[v2,1/3] ath10k: Use complete VHT chan width for 160MHz workaround

Message ID 20170609110750.14950-1-sven.eckelmann@openmesh.com (mailing list archive)
State Accepted
Commit e509e59477083b09e808c8c20562983d34172bb9
Delegated to: Kalle Valo
Headers show

Commit Message

Sven Eckelmann June 9, 2017, 11:07 a.m. UTC 
From: Ben Greear <greearb@candelatech.com>

The ath10k firmware doesn't announce its VHT channel width capabilities in
the vht_cap information from the "service ready event" arguments. The
driver must therefore check whether the 160MHz short GI bit is set and
whether the driver still doesn't set the bits for the 160/80+80 MHz
capabilities.

The two bits for the channel width are a two bit integer and not two
separate bits which cannot be parsed without the knowledge of the other
bit. Using IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ (b10..) as a
mask for this task doesn't make any sense. The correct mask for the VHT
channel width should be used instead to make this check more readable.

Signed-off-by: Ben Greear <greearb@candelatech.com>
[sven.eckelmann@openmesh.com: separate 160Mhz workaround cleanup, add commit
 message]
Signed-off-by: Sven Eckelmann <sven.eckelmann@openmesh.com>
---
v2:
 - extracted this cleanup portion and converted it to a separate patch

 drivers/net/wireless/ath/ath10k/mac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ben Greear June 9, 2017, 1:50 p.m. UTC | #1 
On 06/09/2017 04:07 AM, Sven Eckelmann wrote:
> From: Ben Greear <greearb@candelatech.com>
>
> The ath10k firmware doesn't announce its VHT channel width capabilities in
> the vht_cap information from the "service ready event" arguments. The
> driver must therefore check whether the 160MHz short GI bit is set and
> whether the driver still doesn't set the bits for the 160/80+80 MHz
> capabilities.
>
> The two bits for the channel width are a two bit integer and not two
> separate bits which cannot be parsed without the knowledge of the other
> bit. Using IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ (b10..) as a
> mask for this task doesn't make any sense. The correct mask for the VHT
> channel width should be used instead to make this check more readable.

I didn't test them, but a quick review looks good.  Thanks for picking this up!

--Ben
Sebastian Gottschall June 9, 2017, 2:09 p.m. UTC | #2 
Am 09.06.2017 um 15:50 schrieb Ben Greear:
>
>
> On 06/09/2017 04:07 AM, Sven Eckelmann wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> The ath10k firmware doesn't announce its VHT channel width 
>> capabilities in
>> the vht_cap information from the "service ready event" arguments. The
>> driver must therefore check whether the 160MHz short GI bit is set and
>> whether the driver still doesn't set the bits for the 160/80+80 MHz
>> capabilities.
>>
>> The two bits for the channel width are a two bit integer and not two
>> separate bits which cannot be parsed without the knowledge of the other
>> bit. Using IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ (b10..) 
>> as a
>> mask for this task doesn't make any sense. The correct mask for the VHT
>> channel width should be used instead to make this check more readable.
>
> I didn't test them, but a quick review looks good.  Thanks for picking 
> this up!
mmh except that it wont change anything. this patch is a workaround for 
older firmwares. these older firmwares have this flag zeroed
newer firmwares have 80p80 set. thats all. for sure we can now check if 
the mask is 0 or not 0. but it wont change anything at the end
>
> --Ben
>
>
Kalle Valo June 21, 2017, 1:18 p.m. UTC | #3 
Sven Eckelmann <sven.eckelmann@openmesh.com> wrote:

> The ath10k firmware doesn't announce its VHT channel width capabilities in
> the vht_cap information from the "service ready event" arguments. The
> driver must therefore check whether the 160MHz short GI bit is set and
> whether the driver still doesn't set the bits for the 160/80+80 MHz
> capabilities.
> 
> The two bits for the channel width are a two bit integer and not two
> separate bits which cannot be parsed without the knowledge of the other
> bit. Using IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ (b10..) as a
> mask for this task doesn't make any sense. The correct mask for the VHT
> channel width should be used instead to make this check more readable.
> 
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> [sven.eckelmann@openmesh.com: separate 160Mhz workaround cleanup, add commit
>  message]
> Signed-off-by: Sven Eckelmann <sven.eckelmann@openmesh.com>
> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>

3 patches applied to ath-next branch of ath.git, thanks.

e509e5947708 ath10k: use complete VHT chan width for 160MHz workaround
cc914a55006e ath10k: configure rxnss_override for QCA9984
6824834946a6 ath10k: set rxnss_override for QCA9888
diff mbox

Patch

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 4674ff33d320..8087b6be5484 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4391,7 +4391,7 @@  static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k *ar)
 	 * mode until that's resolved.
 	 */
 	if ((ar->vht_cap_info & IEEE80211_VHT_CAP_SHORT_GI_160) &&
-	    !(ar->vht_cap_info & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ))
+	    (ar->vht_cap_info & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK) == 0)
 		vht_cap.cap |= IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ;
 
 	mcs_map = 0;