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

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

[Sven Eckelmann]

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(-)

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;
-- 
2.11.0

[PATCH v2 2/3] ath10k: Configure rxnss_override for 10.4 firmware.

[Sven Eckelmann]

From: Ben Greear <greearb@candelatech.com>

QCA9984 hardware can do 4x4 at 80Mhz, but only 2x2 at 160Mhz.

First, report this to user-space by setting the max-tx-speed
and max-rx-speed vht capabilities.

Second, if the peer rx-speed is configured, and if we
are in 160 or 80+80 mode, and the peer rx-speed matches
the max speed for 2x2 or 1x1 at 160Mhz (long guard interval),
then use that info to set the peer_bw_rxnss_override appropriately.

Without this, a 9984 firmware will not use 2x2 ratesets when
transmitting to peer (it will be stuck at 1x1), because
the firmware would not have configured the rxnss_override.

This could use some testing....

Signed-off-by: Ben Greear <greearb@candelatech.com>
[sven.eckelmann@openmesh.com: rebase, cleanup, drop 160Mhz workaround cleanup]
Signed-off-by: Sven Eckelmann <sven.eckelmann@openmesh.com>
---
v2:
 - rebased patch
 - minor cleanups
 - removal of the 160 MHz workaround (see patch 1)

 drivers/net/wireless/ath/ath10k/mac.c | 31 +++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.c |  7 ++++++-
 drivers/net/wireless/ath/ath10k/wmi.h |  2 ++
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 8087b6be5484..0752cf351b4a 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2519,6 +2519,20 @@ static void ath10k_peer_assoc_h_vht(struct ath10k *ar,
 
 	ath10k_dbg(ar, ATH10K_DBG_MAC, "mac vht peer %pM max_mpdu %d flags 0x%x\n",
 		   sta->addr, arg->peer_max_mpdu, arg->peer_flags);
+
+	if (arg->peer_vht_rates.rx_max_rate &&
+	    (sta->vht_cap.cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK)) {
+		switch (arg->peer_vht_rates.rx_max_rate) {
+		case 1560:
+			/* Must be 2x2 at 160Mhz is all it can do. */
+			arg->peer_bw_rxnss_override = 2;
+			break;
+		case 780:
+			/* Can only do 1x1 at 160Mhz (Long Guard Interval) */
+			arg->peer_bw_rxnss_override = 1;
+			break;
+		}
+	}
 }
 
 static void ath10k_peer_assoc_h_qos(struct ath10k *ar,
@@ -4408,6 +4422,23 @@ static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k *ar)
 	vht_cap.vht_mcs.rx_mcs_map = cpu_to_le16(mcs_map);
 	vht_cap.vht_mcs.tx_mcs_map = cpu_to_le16(mcs_map);
 
+	/* If we are supporting 160Mhz or 80+80, then the NIC may be able to do
+	 * a restricted NSS for 160 or 80+80 vs what it can do for 80Mhz.  Give
+	 * user-space a clue if that is the case.
+	 */
+	if (vht_cap.cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK) {
+		switch (ar->dev_id) {
+		case QCA9984_1_0_DEVICE_ID:
+			/* Can do only 2x2 VHT160 or 80+80.
+			 * 1560Mbps is 4x4 80Mhz or 2x2 160Mhz,
+			 * long-guard-interval
+			 */
+			vht_cap.vht_mcs.rx_highest = 1560;
+			vht_cap.vht_mcs.tx_highest = 1560;
+			break;
+		}
+	}
+
 	return vht_cap;
 }
 
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 6afc8d27f0d5..2c3b0214ba5f 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -6757,7 +6757,12 @@ ath10k_wmi_peer_assoc_fill_10_4(struct ath10k *ar, void *buf,
 	struct wmi_10_4_peer_assoc_complete_cmd *cmd = buf;
 
 	ath10k_wmi_peer_assoc_fill_10_2(ar, buf, arg);
-	cmd->peer_bw_rxnss_override = 0;
+	if (arg->peer_bw_rxnss_override)
+		cmd->peer_bw_rxnss_override =
+			__cpu_to_le32((arg->peer_bw_rxnss_override - 1) |
+				      BIT(PEER_BW_RXNSS_OVERRIDE_OFFSET));
+	else
+		cmd->peer_bw_rxnss_override = 0;
 }
 
 static int
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 1b4865a55595..dd6cac150749 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -6028,6 +6028,7 @@ struct wmi_10_2_peer_assoc_complete_cmd {
 	__le32 info0; /* WMI_PEER_ASSOC_INFO0_ */
 } __packed;
 
+#define PEER_BW_RXNSS_OVERRIDE_OFFSET  31
 struct wmi_10_4_peer_assoc_complete_cmd {
 	struct wmi_10_2_peer_assoc_complete_cmd cmd;
 	__le32 peer_bw_rxnss_override;
@@ -6051,6 +6052,7 @@ struct wmi_peer_assoc_complete_arg {
 	u32 peer_vht_caps;
 	enum wmi_phy_mode peer_phymode;
 	struct wmi_vht_rate_set_arg peer_vht_rates;
+	u32 peer_bw_rxnss_override;
 };
 
 struct wmi_peer_add_wds_entry_cmd {
-- 
2.11.0

Re: [v2,2/3] ath10k: Configure rxnss_override for 10.4 firmware.

[Kalle Valo]

Sven Eckelmann <sven.eckelmann@openmesh.com> wrote:

> QCA9984 hardware can do 4x4 at 80Mhz, but only 2x2 at 160Mhz.
> 
> First, report this to user-space by setting the max-tx-speed
> and max-rx-speed vht capabilities.
> 
> Second, if the peer rx-speed is configured, and if we
> are in 160 or 80+80 mode, and the peer rx-speed matches
> the max speed for 2x2 or 1x1 at 160Mhz (long guard interval),
> then use that info to set the peer_bw_rxnss_override appropriately.
> 
> Without this, a 9984 firmware will not use 2x2 ratesets when
> transmitting to peer (it will be stuck at 1x1), because
> the firmware would not have configured the rxnss_override.
> 
> This could use some testing....
> 
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> [sven.eckelmann@openmesh.com: rebase, cleanup, drop 160Mhz workaround cleanup]
> Signed-off-by: Sven Eckelmann <sven.eckelmann@openmesh.com>
> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>

This one had warnings, fixed in pending branch:

drivers/net/wireless/ath/ath10k/mac.c:4436:52: warning: incorrect type in assignment (different base types)
drivers/net/wireless/ath/ath10k/mac.c:4436:52:    expected restricted __le16 [assigned] [usertype] rx_highest
drivers/net/wireless/ath/ath10k/mac.c:4436:52:    got int
drivers/net/wireless/ath/ath10k/mac.c:4437:52: warning: incorrect type in assignment (different base types)
drivers/net/wireless/ath/ath10k/mac.c:4437:52:    expected restricted __le16 [assigned] [usertype] tx_highest
drivers/net/wireless/ath/ath10k/mac.c:4437:52:    got int

-- 
https://patchwork.kernel.org/patch/9778087/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v2 2/3] ath10k: Configure rxnss_override for 10.4 firmware.

[Kalle Valo]

Sven Eckelmann <sven.eckelmann@openmesh.com> writes:

> From: Ben Greear <greearb@candelatech.com>
>
> QCA9984 hardware can do 4x4 at 80Mhz, but only 2x2 at 160Mhz.
>
> First, report this to user-space by setting the max-tx-speed
> and max-rx-speed vht capabilities.
>
> Second, if the peer rx-speed is configured, and if we
> are in 160 or 80+80 mode, and the peer rx-speed matches
> the max speed for 2x2 or 1x1 at 160Mhz (long guard interval),
> then use that info to set the peer_bw_rxnss_override appropriately.
>
> Without this, a 9984 firmware will not use 2x2 ratesets when
> transmitting to peer (it will be stuck at 1x1), because
> the firmware would not have configured the rxnss_override.
>
> This could use some testing....

Sven, I assume you have tested this so I removed the last sentence.

> @@ -4408,6 +4422,23 @@ static struct ieee80211_sta_vht_cap ath10k_create_=
vht_cap(struct ath10k *ar)
>  	vht_cap.vht_mcs.rx_mcs_map =3D cpu_to_le16(mcs_map);
>  	vht_cap.vht_mcs.tx_mcs_map =3D cpu_to_le16(mcs_map);
> =20
> +	/* If we are supporting 160Mhz or 80+80, then the NIC may be able to do
> +	 * a restricted NSS for 160 or 80+80 vs what it can do for 80Mhz.  Give
> +	 * user-space a clue if that is the case.
> +	 */
> +	if (vht_cap.cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK) {
> +		switch (ar->dev_id) {
> +		case QCA9984_1_0_DEVICE_ID:
> +			/* Can do only 2x2 VHT160 or 80+80.
> +			 * 1560Mbps is 4x4 80Mhz or 2x2 160Mhz,
> +			 * long-guard-interval
> +			 */
> +			vht_cap.vht_mcs.rx_highest =3D 1560;
> +			vht_cap.vht_mcs.tx_highest =3D 1560;
> +			break;
> +		}
> +	}

We have hw_params for stuff like this so I changed this and the
following patch to use that. Please review:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=3Dp=
ending&id=3D81f55f2a3e1837287a52de6577ca89a2c7393456

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=3Dp=
ending&id=3D09ce674529472c8deca41fbb28bad59e03330321

--=20
Kalle Valo=

Re: [PATCH v2 2/3] ath10k: Configure rxnss_override for 10.4 firmware.

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 237 bytes --]

On Freitag, 16. Juni 2017 08:50:13 CEST Kalle Valo wrote:
> We have hw_params for stuff like this so I changed this and the
> following patch to use that. Please review:

Looks good. Thanks for adjusting the patches.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 3/3] ath10k: Set rxnss_override for QCA9888

[Sven Eckelmann]

QCA9888 supports VHT80 with 2x2. But it only support 1x1 with VHT160 or
VHT80+80. Inform userspace and the the QCA firmware about that limitation
whenever VHT80+80 or VHT160 is configured.

Signed-off-by: Sven Eckelmann <sven.eckelmann@openmesh.com>
---
v2:
 - new patch

 drivers/net/wireless/ath/ath10k/mac.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 0752cf351b4a..75e90adc8fb4 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4436,6 +4436,14 @@ static struct ieee80211_sta_vht_cap ath10k_create_vht_cap(struct ath10k *ar)
 			vht_cap.vht_mcs.rx_highest = 1560;
 			vht_cap.vht_mcs.tx_highest = 1560;
 			break;
+		case QCA9888_2_0_DEVICE_ID:
+			/* Can do only 1x1 VHT160 or 80+80.
+			 * 780Mbps is 2x2 80Mhz or 1x1 160Mhz,
+			 * long-guard-interval
+			 */
+			vht_cap.vht_mcs.rx_highest = 780;
+			vht_cap.vht_mcs.tx_highest = 780;
+			break;
 		}
 	}
 
-- 
2.11.0

Re: [v2,3/3] ath10k: Set rxnss_override for QCA9888

[Kalle Valo]

Sven Eckelmann <sven.eckelmann@openmesh.com> wrote:

> QCA9888 supports VHT80 with 2x2. But it only support 1x1 with VHT160 or
> VHT80+80. Inform userspace and the the QCA firmware about that limitation
> whenever VHT80+80 or VHT160 is configured.
> 
> Signed-off-by: Sven Eckelmann <sven.eckelmann@openmesh.com>
> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>

This also had similar warnings, fixed in pending branch:

drivers/net/wireless/ath/ath10k/mac.c:4444:52: warning: incorrect type in assignment (different base types)
drivers/net/wireless/ath/ath10k/mac.c:4444:52:    expected restricted __le16 [assigned] [usertype] rx_highest
drivers/net/wireless/ath/ath10k/mac.c:4444:52:    got int
drivers/net/wireless/ath/ath10k/mac.c:4445:52: warning: incorrect type in assignment (different base types)
drivers/net/wireless/ath/ath10k/mac.c:4445:52:    expected restricted __le16 [assigned] [usertype] tx_highest
drivers/net/wireless/ath/ath10k/mac.c:4445:52:    got int


-- 
https://patchwork.kernel.org/patch/9778089/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

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

[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!

--Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

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

[Sebastian Gottschall]

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
>
>

-- 
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz:  Berliner Ring 101, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565

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

[Kalle Valo]

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

-- 
https://patchwork.kernel.org/patch/9778085/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches