All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ath6kl: use WMI to set RSN capabilities
@ 2018-04-12 15:41 Alfonso Sánchez-Beato
  2018-04-13 10:48 ` Kalle Valo
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Alfonso Sánchez-Beato @ 2018-04-12 15:41 UTC (permalink / raw)
  To: kvalo; +Cc: ath6kl, linux-wireless, alfonso.sanchez-beato

This fixes AP mode when the ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE flag
is not present in the FW. The id of some WMI commands is also fixed
(there was an error in the enum order), and a function to set RSN caps
is added.

Signed-off-by: Alfonso Sánchez-Beato <alfonso.sanchez-beato@canonical.com>
---
 drivers/net/wireless/ath/ath6kl/cfg80211.c | 21 ++++++---------------
 drivers/net/wireless/ath/ath6kl/main.c     | 10 +++-------
 drivers/net/wireless/ath/ath6kl/wmi.c      | 23 +++++++++++++++++++++++
 drivers/net/wireless/ath/ath6kl/wmi.h      | 15 +++++++++++----
 4 files changed, 43 insertions(+), 26 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index 2ba8cf3f38af..1b89c53d47e7 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -2933,13 +2933,11 @@ static int ath6kl_start_ap(struct wiphy *wiphy, struct net_device *dev,
 	 * advertise the same in beacon/probe response. Send
 	 * the complete RSN IE capability field to firmware
 	 */
-	if (!ath6kl_get_rsn_capab(&info->beacon, (u8 *) &rsn_capab) &&
-	    test_bit(ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE,
-		     ar->fw_capabilities)) {
-		res = ath6kl_wmi_set_ie_cmd(ar->wmi, vif->fw_vif_idx,
-					    WLAN_EID_RSN, WMI_RSN_IE_CAPB,
-					    (const u8 *) &rsn_capab,
-					    sizeof(rsn_capab));
+	if (!ath6kl_get_rsn_capab(&info->beacon, (u8 *)&rsn_capab)) {
+		ath6kl_dbg(ATH6KL_DBG_WLAN_CFG, "RSN caps %d\n", rsn_capab);
+
+		res = ath6kl_wmi_set_rsn_cap(ar->wmi, vif->fw_vif_idx,
+					     rsn_capab);
 		vif->rsn_capab = rsn_capab;
 		if (res < 0)
 			return res;
@@ -3918,14 +3916,7 @@ int ath6kl_cfg80211_init(struct ath6kl *ar)
 		return -EINVAL;
 	}
 
-	/*
-	 * Even if the fw has HT support, advertise HT cap only when
-	 * the firmware has support to override RSN capability, otherwise
-	 * 4-way handshake would fail.
-	 */
-	if (!(ht &&
-	      test_bit(ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE,
-		       ar->fw_capabilities))) {
+	if (!ht) {
 		ath6kl_band_2ghz.ht_cap.cap = 0;
 		ath6kl_band_2ghz.ht_cap.ht_supported = false;
 		ath6kl_band_5ghz.ht_cap.cap = 0;
diff --git a/drivers/net/wireless/ath/ath6kl/main.c b/drivers/net/wireless/ath/ath6kl/main.c
index db95f85751e3..4e186f8b3950 100644
--- a/drivers/net/wireless/ath/ath6kl/main.c
+++ b/drivers/net/wireless/ath/ath6kl/main.c
@@ -579,13 +579,9 @@ static int ath6kl_commit_ch_switch(struct ath6kl_vif *vif, u16 channel)
 		 * reconfigure any saved RSN IE capabilites in the beacon /
 		 * probe response to stay in sync with the supplicant.
 		 */
-		if (vif->rsn_capab &&
-		    test_bit(ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE,
-			     ar->fw_capabilities))
-			ath6kl_wmi_set_ie_cmd(ar->wmi, vif->fw_vif_idx,
-					      WLAN_EID_RSN, WMI_RSN_IE_CAPB,
-					      (const u8 *) &vif->rsn_capab,
-					      sizeof(vif->rsn_capab));
+		if (vif->rsn_capab)
+			ath6kl_wmi_set_rsn_cap(ar->wmi, vif->fw_vif_idx,
+					       vif->rsn_capab);
 
 		return ath6kl_wmi_ap_profile_commit(ar->wmi, vif->fw_vif_idx,
 						    &vif->profile);
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index 777acc564ac9..d7de9224d755 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -4168,3 +4168,26 @@ void ath6kl_wmi_shutdown(struct wmi *wmi)
 	kfree(wmi->last_mgmt_tx_frame);
 	kfree(wmi);
 }
+
+int ath6kl_wmi_set_rsn_cap(struct wmi *wmi, u8 if_idx, u16 rsn_cap)
+{
+	struct sk_buff *skb;
+	struct wmi_rsn_cap_cmd *cmd;
+
+	skb = ath6kl_wmi_get_new_buf(sizeof(*cmd));
+	if (!skb)
+		return -ENOMEM;
+
+	ath6kl_dbg(ATH6KL_DBG_WMI, "set_rsn_cap: 0x%04x\n", rsn_cap);
+
+	cmd = (struct wmi_rsn_cap_cmd *)skb->data;
+	cmd->rsn_cap = cpu_to_le16(rsn_cap);
+
+	return ath6kl_wmi_cmd_send(wmi, if_idx, skb, WMI_SET_RSN_CAP_CMDID,
+				   NO_SYNC_WMIFLAG);
+}
+
+int ath6kl_wmi_get_rsn_cap(struct wmi *wmi, u8 if_idx)
+{
+	return ath6kl_wmi_simple_cmd(wmi, if_idx, WMI_GET_RSN_CAP_CMDID);
+}
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.h b/drivers/net/wireless/ath/ath6kl/wmi.h
index a60bb49fe920..011d4b6fb5ab 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.h
+++ b/drivers/net/wireless/ath/ath6kl/wmi.h
@@ -597,10 +597,6 @@ enum wmi_cmd_id {
 
 	WMI_GREENTX_PARAMS_CMDID,
 
-	WMI_RTT_MEASREQ_CMDID,
-	WMI_RTT_CAPREQ_CMDID,
-	WMI_RTT_STATUSREQ_CMDID,
-
 	/* WPS Commands */
 	WMI_WPS_START_CMDID,
 	WMI_GET_WPS_STATUS_CMDID,
@@ -621,6 +617,10 @@ enum wmi_cmd_id {
 	WMI_RX_FILTER_COALESCE_FILTER_OP_CMDID,
 	WMI_RX_FILTER_SET_FRAME_TEST_LIST_CMDID,
 
+	WMI_RTT_MEASREQ_CMDID,
+	WMI_RTT_CAPREQ_CMDID,
+	WMI_RTT_STATUSREQ_CMDID,
+
 	WMI_SEND_MGMT_CMDID,
 	WMI_BEGIN_SCAN_CMDID,
 
@@ -2533,6 +2533,10 @@ enum wmi_sync_flag {
 	END_WMIFLAG
 };
 
+struct wmi_rsn_cap_cmd {
+	__le16 rsn_cap;
+} __packed;
+
 enum htc_endpoint_id ath6kl_wmi_get_control_ep(struct wmi *wmi);
 void ath6kl_wmi_set_control_ep(struct wmi *wmi, enum htc_endpoint_id ep_id);
 int ath6kl_wmi_dix_2_dot3(struct wmi *wmi, struct sk_buff *skb);
@@ -2728,4 +2732,7 @@ void *ath6kl_wmi_init(struct ath6kl *devt);
 void ath6kl_wmi_shutdown(struct wmi *wmi);
 void ath6kl_wmi_reset(struct wmi *wmi);
 
+int ath6kl_wmi_set_rsn_cap(struct wmi *wmi, u8 if_idx, u16 rsn_cap);
+int ath6kl_wmi_get_rsn_cap(struct wmi *wmi, u8 if_idx);
+
 #endif /* WMI_H */
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] ath6kl: use WMI to set RSN capabilities
  2018-04-12 15:41 [PATCH] ath6kl: use WMI to set RSN capabilities Alfonso Sánchez-Beato
@ 2018-04-13 10:48 ` Kalle Valo
  2018-04-13 11:19   ` Alfonso Sanchez-Beato
  2018-04-27 11:23 ` Kalle Valo
  2018-04-27 15:47 ` [PATCH] " Steve deRosier
  2 siblings, 1 reply; 8+ messages in thread
From: Kalle Valo @ 2018-04-13 10:48 UTC (permalink / raw)
  To: Alfonso Sánchez-Beato; +Cc: ath6kl, linux-wireless

QWxmb25zbyBTw6FuY2hlei1CZWF0byA8YWxmb25zby5zYW5jaGV6LWJlYXRvQGNhbm9uaWNhbC5j
b20+IHdyaXRlczoNCg0KPiBUaGlzIGZpeGVzIEFQIG1vZGUgd2hlbiB0aGUgQVRINktMX0ZXX0NB
UEFCSUxJVFlfUlNOX0NBUF9PVkVSUklERSBmbGFnDQo+IGlzIG5vdCBwcmVzZW50IGluIHRoZSBG
Vy4gVGhlIGlkIG9mIHNvbWUgV01JIGNvbW1hbmRzIGlzIGFsc28gZml4ZWQNCj4gKHRoZXJlIHdh
cyBhbiBlcnJvciBpbiB0aGUgZW51bSBvcmRlciksIGFuZCBhIGZ1bmN0aW9uIHRvIHNldCBSU04g
Y2Fwcw0KPiBpcyBhZGRlZC4NCj4NCj4gU2lnbmVkLW9mZi1ieTogQWxmb25zbyBTw6FuY2hlei1C
ZWF0byA8YWxmb25zby5zYW5jaGV6LWJlYXRvQGNhbm9uaWNhbC5jb20+DQoNCldoYXQgaGFyZHdh
cmUgYW5kIGZpcm13YXJlIHZlcnNpb24/IERvIG5vdGUgdGhhdCBpZHMgY2FuIGNoYW5nZSBiZXR3
ZWVuDQphdGg2a2wgZmlybXdhcmUgYnJhbmNoZXMgc28gZXZlbiBpZiBpdCB3b3JrcyBmb3IgeW91
IGl0IG1pZ2h0IGJyZWFrIHNvbWUNCm90aGVyIGJyYW5jaC4gVGhhdCdzIHdoeSBpdCdzIGdvb2Qg
dG8gZG9jdW1lbnQgdGhlIHNldHVwIGluIHRoZSBjb21taXQNCmxvZy4NCg0KLS0gDQpLYWxsZSBW
YWxv

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ath6kl: use WMI to set RSN capabilities
  2018-04-13 10:48 ` Kalle Valo
@ 2018-04-13 11:19   ` Alfonso Sanchez-Beato
  2018-04-13 11:22     ` Kalle Valo
  0 siblings, 1 reply; 8+ messages in thread
From: Alfonso Sanchez-Beato @ 2018-04-13 11:19 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath6kl, linux-wireless

On Fri, Apr 13, 2018 at 12:48 PM, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>
> Alfonso Sánchez-Beato <alfonso.sanchez-beato@canonical.com> writes:
>
> > This fixes AP mode when the ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE flag
> > is not present in the FW. The id of some WMI commands is also fixed
> > (there was an error in the enum order), and a function to set RSN caps
> > is added.
> >
> > Signed-off-by: Alfonso Sánchez-Beato <alfonso.sanchez-beato@canonical.com>
>
> What hardware and firmware version? Do note that ids can change between
> ath6kl firmware branches so even if it works for you it might break some
> other branch. That's why it's good to document the setup in the commit
> log.

I have tested this in an Atheros QCA6234. kmsg shows this about the fw:

ath6kl: ar6004 hw 3.0 sdio fw 3.5.0.604 api 1
ath6kl: firmware supports: 64bit-rates,ap-inactivity-mins

The fw file is AR6004/hw3.0/fw.ram.bin

>
> --
> Kalle Valo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ath6kl: use WMI to set RSN capabilities
  2018-04-13 11:19   ` Alfonso Sanchez-Beato
@ 2018-04-13 11:22     ` Kalle Valo
  0 siblings, 0 replies; 8+ messages in thread
From: Kalle Valo @ 2018-04-13 11:22 UTC (permalink / raw)
  To: Alfonso Sanchez-Beato; +Cc: ath6kl, linux-wireless

QWxmb25zbyBTYW5jaGV6LUJlYXRvIDxhbGZvbnNvLnNhbmNoZXotYmVhdG9AY2Fub25pY2FsLmNv
bT4gd3JpdGVzOg0KDQo+IE9uIEZyaSwgQXByIDEzLCAyMDE4IGF0IDEyOjQ4IFBNLCBLYWxsZSBW
YWxvIDxrdmFsb0BxY2EucXVhbGNvbW0uY29tPiB3cm90ZToNCj4+DQo+PiBBbGZvbnNvIFPDoW5j
aGV6LUJlYXRvIDxhbGZvbnNvLnNhbmNoZXotYmVhdG9AY2Fub25pY2FsLmNvbT4gd3JpdGVzOg0K
Pj4NCj4+ID4gVGhpcyBmaXhlcyBBUCBtb2RlIHdoZW4gdGhlIEFUSDZLTF9GV19DQVBBQklMSVRZ
X1JTTl9DQVBfT1ZFUlJJREUgZmxhZw0KPj4gPiBpcyBub3QgcHJlc2VudCBpbiB0aGUgRlcuIFRo
ZSBpZCBvZiBzb21lIFdNSSBjb21tYW5kcyBpcyBhbHNvIGZpeGVkDQo+PiA+ICh0aGVyZSB3YXMg
YW4gZXJyb3IgaW4gdGhlIGVudW0gb3JkZXIpLCBhbmQgYSBmdW5jdGlvbiB0byBzZXQgUlNOIGNh
cHMNCj4+ID4gaXMgYWRkZWQuDQo+PiA+DQo+PiA+IFNpZ25lZC1vZmYtYnk6IEFsZm9uc28gU8Oh
bmNoZXotQmVhdG8gPGFsZm9uc28uc2FuY2hlei1iZWF0b0BjYW5vbmljYWwuY29tPg0KPj4NCj4+
IFdoYXQgaGFyZHdhcmUgYW5kIGZpcm13YXJlIHZlcnNpb24/IERvIG5vdGUgdGhhdCBpZHMgY2Fu
IGNoYW5nZSBiZXR3ZWVuDQo+PiBhdGg2a2wgZmlybXdhcmUgYnJhbmNoZXMgc28gZXZlbiBpZiBp
dCB3b3JrcyBmb3IgeW91IGl0IG1pZ2h0IGJyZWFrIHNvbWUNCj4+IG90aGVyIGJyYW5jaC4gVGhh
dCdzIHdoeSBpdCdzIGdvb2QgdG8gZG9jdW1lbnQgdGhlIHNldHVwIGluIHRoZSBjb21taXQNCj4+
IGxvZy4NCj4NCj4gSSBoYXZlIHRlc3RlZCB0aGlzIGluIGFuIEF0aGVyb3MgUUNBNjIzNC4ga21z
ZyBzaG93cyB0aGlzIGFib3V0IHRoZSBmdzoNCj4NCj4gYXRoNmtsOiBhcjYwMDQgaHcgMy4wIHNk
aW8gZncgMy41LjAuNjA0IGFwaSAxDQoNCk9rLCBJJ2xsIGFkZCB0aGlzIHRvIHRoZSBjb21taXQg
bG9nLg0KDQotLSANCkthbGxlIFZhbG8=

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ath6kl: use WMI to set RSN capabilities
  2018-04-12 15:41 [PATCH] ath6kl: use WMI to set RSN capabilities Alfonso Sánchez-Beato
  2018-04-13 10:48 ` Kalle Valo
@ 2018-04-27 11:23 ` Kalle Valo
  2018-04-27 15:47 ` [PATCH] " Steve deRosier
  2 siblings, 0 replies; 8+ messages in thread
From: Kalle Valo @ 2018-04-27 11:23 UTC (permalink / raw)
  To: Alfonso Sánchez-Beato
  Cc: kvalo, ath6kl, linux-wireless, alfonso.sanchez-beato, Steve deRosier

Alfonso Sánchez-Beato wrote:

> This fixes AP mode when the ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE flag
> is not present in the FW. The id of some WMI commands is also fixed
> (there was an error in the enum order), and a function to set RSN caps
> is added.
> 
> Signed-off-by: Alfonso Sánchez-Beato <alfonso.sanchez-beato@canonical.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Steve, what do you think of this?

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

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ath6kl: use WMI to set RSN capabilities
  2018-04-12 15:41 [PATCH] ath6kl: use WMI to set RSN capabilities Alfonso Sánchez-Beato
  2018-04-13 10:48 ` Kalle Valo
  2018-04-27 11:23 ` Kalle Valo
@ 2018-04-27 15:47 ` Steve deRosier
  2018-04-27 16:57   ` Alfonso Sanchez-Beato
  2 siblings, 1 reply; 8+ messages in thread
From: Steve deRosier @ 2018-04-27 15:47 UTC (permalink / raw)
  To: alfonso.sanchez-beato; +Cc: Kalle Valo, ath6kl, linux-wireless

Hi Alfonso,

On Thu, Apr 12, 2018 at 8:42 AM Alfonso S=C3=A1nchez-Beato <
alfonso.sanchez-beato@canonical.com> wrote:

> This fixes AP mode when the ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE flag
> is not present in the FW. The id of some WMI commands is also fixed
> (there was an error in the enum order), and a function to set RSN caps
> is added.

> Signed-off-by: Alfonso S=C3=A1nchez-Beato <alfonso.sanchez-beato@canonica=
l.com>
> ---
>   drivers/net/wireless/ath/ath6kl/cfg80211.c | 21 ++++++---------------
>   drivers/net/wireless/ath/ath6kl/main.c     | 10 +++-------
>   drivers/net/wireless/ath/ath6kl/wmi.c      | 23 +++++++++++++++++++++++
>   drivers/net/wireless/ath/ath6kl/wmi.h      | 15 +++++++++++----
>   4 files changed, 43 insertions(+), 26 deletions(-)

> diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c
b/drivers/net/wireless/ath/ath6kl/cfg80211.c
> index 2ba8cf3f38af..1b89c53d47e7 100644
> --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
> +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
> @@ -2933,13 +2933,11 @@ static int ath6kl_start_ap(struct wiphy *wiphy,
struct net_device *dev,
>           * advertise the same in beacon/probe response. Send
>           * the complete RSN IE capability field to firmware
>           */
> -       if (!ath6kl_get_rsn_capab(&info->beacon, (u8 *) &rsn_capab) &&
> -           test_bit(ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE,
> -                    ar->fw_capabilities)) {
> -               res =3D ath6kl_wmi_set_ie_cmd(ar->wmi, vif->fw_vif_idx,
> -                                           WLAN_EID_RSN, WMI_RSN_IE_CAPB=
,
> -                                           (const u8 *) &rsn_capab,
> -                                           sizeof(rsn_capab));
> +       if (!ath6kl_get_rsn_capab(&info->beacon, (u8 *)&rsn_capab)) {
> +               ath6kl_dbg(ATH6KL_DBG_WLAN_CFG, "RSN caps %d\n",
rsn_capab);
> +
> +               res =3D ath6kl_wmi_set_rsn_cap(ar->wmi, vif->fw_vif_idx,
> +                                            rsn_capab);
>                  vif->rsn_capab =3D rsn_capab;
>                  if (res < 0)
>                          return res;

So, let me see if I understand this correctly.  Original flow was:

1. get RSN capable from the beacon if one
2. if the firmware is capable of the override, set the IE  else don't do
anything

New flow:
1. get RSN capable from the beacon if one
2. unconditionally send the rsn_cap WMI down

So, what happens on a firmware that _does_ have the rsn-cap-override flag
set? I'm guessing nothing good.  Admittedly I haven't tried your patch on
my platform.

I think that simply ignoring the flag and using a WMI instead of setting
the IE on all devices AR6002..AR6004 is likely going to cause problems for
everyone else.


> @@ -3918,14 +3916,7 @@ int ath6kl_cfg80211_init(struct ath6kl *ar)
>                  return -EINVAL;
>          }

> -       /*
> -        * Even if the fw has HT support, advertise HT cap only when
> -        * the firmware has support to override RSN capability, otherwise
> -        * 4-way handshake would fail.
> -        */
> -       if (!(ht &&
> -             test_bit(ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE,
> -                      ar->fw_capabilities))) {
> +       if (!ht) {

Perhaps the comment isn't relevant if we're using the RSN WMI command on a
device with the flag, but I'm willing to bet you neither tested it nor that
the assumption is true.  I'm guessing this change just broke the 4-way
handshake for the majority of devices.

>                  ath6kl_band_2ghz.ht_cap.cap =3D 0;
>                  ath6kl_band_2ghz.ht_cap.ht_supported =3D false;
>                  ath6kl_band_5ghz.ht_cap.cap =3D 0;
> diff --git a/drivers/net/wireless/ath/ath6kl/main.c
b/drivers/net/wireless/ath/ath6kl/main.c
> index db95f85751e3..4e186f8b3950 100644
> --- a/drivers/net/wireless/ath/ath6kl/main.c
> +++ b/drivers/net/wireless/ath/ath6kl/main.c
> @@ -579,13 +579,9 @@ static int ath6kl_commit_ch_switch(struct ath6kl_vif
*vif, u16 channel)
>                   * reconfigure any saved RSN IE capabilites in the beaco=
n
/
>                   * probe response to stay in sync with the supplicant.
>                   */
> -               if (vif->rsn_capab &&
> -                   test_bit(ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE,
> -                            ar->fw_capabilities))
> -                       ath6kl_wmi_set_ie_cmd(ar->wmi, vif->fw_vif_idx,
> -                                             WLAN_EID_RSN,
WMI_RSN_IE_CAPB,
> -                                             (const u8 *)
&vif->rsn_capab,
> -                                             sizeof(vif->rsn_capab));
> +               if (vif->rsn_capab)
> +                       ath6kl_wmi_set_rsn_cap(ar->wmi, vif->fw_vif_idx,
> +                                              vif->rsn_capab);


So, basically same comment as the first one.

>                  return ath6kl_wmi_ap_profile_commit(ar->wmi,
vif->fw_vif_idx,
>                                                      &vif->profile);
> diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c
b/drivers/net/wireless/ath/ath6kl/wmi.c
> index 777acc564ac9..d7de9224d755 100644
> --- a/drivers/net/wireless/ath/ath6kl/wmi.c
> +++ b/drivers/net/wireless/ath/ath6kl/wmi.c
> @@ -4168,3 +4168,26 @@ void ath6kl_wmi_shutdown(struct wmi *wmi)
>          kfree(wmi->last_mgmt_tx_frame);
>          kfree(wmi);
>   }
> +
> +int ath6kl_wmi_set_rsn_cap(struct wmi *wmi, u8 if_idx, u16 rsn_cap)
> +{
> +       struct sk_buff *skb;
> +       struct wmi_rsn_cap_cmd *cmd;
> +
> +       skb =3D ath6kl_wmi_get_new_buf(sizeof(*cmd));
> +       if (!skb)
> +               return -ENOMEM;
> +
> +       ath6kl_dbg(ATH6KL_DBG_WMI, "set_rsn_cap: 0x%04x\n", rsn_cap);
> +
> +       cmd =3D (struct wmi_rsn_cap_cmd *)skb->data;
> +       cmd->rsn_cap =3D cpu_to_le16(rsn_cap);
> +
> +       return ath6kl_wmi_cmd_send(wmi, if_idx, skb,
WMI_SET_RSN_CAP_CMDID,
> +                                  NO_SYNC_WMIFLAG);
> +}
> +
> +int ath6kl_wmi_get_rsn_cap(struct wmi *wmi, u8 if_idx)
> +{
> +       return ath6kl_wmi_simple_cmd(wmi, if_idx, WMI_GET_RSN_CAP_CMDID);
> +}
> diff --git a/drivers/net/wireless/ath/ath6kl/wmi.h
b/drivers/net/wireless/ath/ath6kl/wmi.h
> index a60bb49fe920..011d4b6fb5ab 100644
> --- a/drivers/net/wireless/ath/ath6kl/wmi.h
> +++ b/drivers/net/wireless/ath/ath6kl/wmi.h
> @@ -597,10 +597,6 @@ enum wmi_cmd_id {

>          WMI_GREENTX_PARAMS_CMDID,

> -       WMI_RTT_MEASREQ_CMDID,
> -       WMI_RTT_CAPREQ_CMDID,
> -       WMI_RTT_STATUSREQ_CMDID,
> -
>          /* WPS Commands */
>          WMI_WPS_START_CMDID,
>          WMI_GET_WPS_STATUS_CMDID,
> @@ -621,6 +617,10 @@ enum wmi_cmd_id {
>          WMI_RX_FILTER_COALESCE_FILTER_OP_CMDID,
>          WMI_RX_FILTER_SET_FRAME_TEST_LIST_CMDID,

> +       WMI_RTT_MEASREQ_CMDID,
> +       WMI_RTT_CAPREQ_CMDID,
> +       WMI_RTT_STATUSREQ_CMDID,
> +
>          WMI_SEND_MGMT_CMDID,
>          WMI_BEGIN_SCAN_CMDID,

> @@ -2533,6 +2533,10 @@ enum wmi_sync_flag {
>          END_WMIFLAG
>   };


I can factually state that the above reordering of the commands is wrong
for a 6003. The order in the WMI file is consistent for a 6003. Your
reordering is consistent for a 6004. Now, the only commands affected by the
reordering aren't utilized by the driver, other than your added get/set
RSN_CAP_CMDIDs.  But on a 6003, your now used WMI_SET_RSN_CAP_CMDID will
trigger a WMI_GET_OPPPS_CMDID, which isn't what you want.

I've encountered this issue a before - wmi code mismatch between the
different firmwares and the driver. This is a limitation of the driver that
probably should be resolved in some meaningful way. To date, it's been
mitigated by the driver just not using the higher-numbered commands.  But
by "meaningful way" I don't include redefining command IDs in favor of one
particular person's firmware and causing problems on the other 99% of
devices out there.

> +struct wmi_rsn_cap_cmd {
> +       __le16 rsn_cap;
> +} __packed;
> +
>   enum htc_endpoint_id ath6kl_wmi_get_control_ep(struct wmi *wmi);
>   void ath6kl_wmi_set_control_ep(struct wmi *wmi, enum htc_endpoint_id
ep_id);
>   int ath6kl_wmi_dix_2_dot3(struct wmi *wmi, struct sk_buff *skb);
> @@ -2728,4 +2732,7 @@ void *ath6kl_wmi_init(struct ath6kl *devt);
>   void ath6kl_wmi_shutdown(struct wmi *wmi);
>   void ath6kl_wmi_reset(struct wmi *wmi);

> +int ath6kl_wmi_set_rsn_cap(struct wmi *wmi, u8 if_idx, u16 rsn_cap);
> +int ath6kl_wmi_get_rsn_cap(struct wmi *wmi, u8 if_idx);
> +
>   #endif /* WMI_H */
> --
> 2.14.1


 From your answer to Kalle re: what hardware

> I have tested this in an Atheros QCA6234. kmsg shows this about the fw:

> ath6kl: ar6004 hw 3.0 sdio fw 3.5.0.604 api 1
> ath6kl: firmware supports: 64bit-rates,ap-inactivity-mins

The firmware you're using is old. Mine for the QCA6234 is more advanced
than that and has the rsn-cap-override flag, but even the stock one in
linux-firmware is more up-to-date: 3.5.0.2356 api 3. I haven't run it
recently to see if it also has the rsn-cap-override flag, but it might.
Maybe you can try the current firmware to see if it solves your issue?

Thanks,
- Steve

--
Steve deRosier
Cal-Sierra Consulting
https://www.cal-sierra.com/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ath6kl: use WMI to set RSN capabilities
  2018-04-27 15:47 ` [PATCH] " Steve deRosier
@ 2018-04-27 16:57   ` Alfonso Sanchez-Beato
  2018-04-27 17:34     ` Steve deRosier
  0 siblings, 1 reply; 8+ messages in thread
From: Alfonso Sanchez-Beato @ 2018-04-27 16:57 UTC (permalink / raw)
  To: Steve deRosier; +Cc: Kalle Valo, ath6kl, linux-wireless

Hi Steve,

On Fri, Apr 27, 2018 at 5:47 PM, Steve deRosier <derosier@gmail.com> wrote:
> Hi Alfonso,
>
> On Thu, Apr 12, 2018 at 8:42 AM Alfonso Sánchez-Beato <
> alfonso.sanchez-beato@canonical.com> wrote:
>
>> This fixes AP mode when the ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE flag
>> is not present in the FW. The id of some WMI commands is also fixed
>> (there was an error in the enum order), and a function to set RSN caps
>> is added.
>
>> Signed-off-by: Alfonso Sánchez-Beato <alfonso.sanchez-beato@canonical.com>
>> ---
>>   drivers/net/wireless/ath/ath6kl/cfg80211.c | 21 ++++++---------------
>>   drivers/net/wireless/ath/ath6kl/main.c     | 10 +++-------
>>   drivers/net/wireless/ath/ath6kl/wmi.c      | 23 +++++++++++++++++++++++
>>   drivers/net/wireless/ath/ath6kl/wmi.h      | 15 +++++++++++----
>>   4 files changed, 43 insertions(+), 26 deletions(-)
>
>> diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c
> b/drivers/net/wireless/ath/ath6kl/cfg80211.c
>> index 2ba8cf3f38af..1b89c53d47e7 100644
>> --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
>> +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
>> @@ -2933,13 +2933,11 @@ static int ath6kl_start_ap(struct wiphy *wiphy,
> struct net_device *dev,
>>           * advertise the same in beacon/probe response. Send
>>           * the complete RSN IE capability field to firmware
>>           */
>> -       if (!ath6kl_get_rsn_capab(&info->beacon, (u8 *) &rsn_capab) &&
>> -           test_bit(ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE,
>> -                    ar->fw_capabilities)) {
>> -               res = ath6kl_wmi_set_ie_cmd(ar->wmi, vif->fw_vif_idx,
>> -                                           WLAN_EID_RSN, WMI_RSN_IE_CAPB,
>> -                                           (const u8 *) &rsn_capab,
>> -                                           sizeof(rsn_capab));
>> +       if (!ath6kl_get_rsn_capab(&info->beacon, (u8 *)&rsn_capab)) {
>> +               ath6kl_dbg(ATH6KL_DBG_WLAN_CFG, "RSN caps %d\n",
> rsn_capab);
>> +
>> +               res = ath6kl_wmi_set_rsn_cap(ar->wmi, vif->fw_vif_idx,
>> +                                            rsn_capab);
>>                  vif->rsn_capab = rsn_capab;
>>                  if (res < 0)
>>                          return res;
>
> So, let me see if I understand this correctly.  Original flow was:
>
> 1. get RSN capable from the beacon if one
> 2. if the firmware is capable of the override, set the IE  else don't do
> anything
>
> New flow:
> 1. get RSN capable from the beacon if one
> 2. unconditionally send the rsn_cap WMI down
>
> So, what happens on a firmware that _does_ have the rsn-cap-override flag
> set? I'm guessing nothing good.  Admittedly I haven't tried your patch on
> my platform.
>
> I think that simply ignoring the flag and using a WMI instead of setting
> the IE on all devices AR6002..AR6004 is likely going to cause problems for
> everyone else.

Admittedly, I have not a wide range of devices to test. This patch was
mostly an RFC to see if the issue is only for a particular fw revision
and to expose it for people that might find it useful. Note that I am
not the first person to hit this, see for instance

http://www.spinics.net/lists/linux-wireless/msg115085.html

>
>
>> @@ -3918,14 +3916,7 @@ int ath6kl_cfg80211_init(struct ath6kl *ar)
>>                  return -EINVAL;
>>          }
>
>> -       /*
>> -        * Even if the fw has HT support, advertise HT cap only when
>> -        * the firmware has support to override RSN capability, otherwise
>> -        * 4-way handshake would fail.
>> -        */
>> -       if (!(ht &&
>> -             test_bit(ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE,
>> -                      ar->fw_capabilities))) {
>> +       if (!ht) {
>
> Perhaps the comment isn't relevant if we're using the RSN WMI command on a
> device with the flag, but I'm willing to bet you neither tested it nor that
> the assumption is true.  I'm guessing this change just broke the 4-way
> handshake for the majority of devices.
>
>>                  ath6kl_band_2ghz.ht_cap.cap = 0;
>>                  ath6kl_band_2ghz.ht_cap.ht_supported = false;
>>                  ath6kl_band_5ghz.ht_cap.cap = 0;
>> diff --git a/drivers/net/wireless/ath/ath6kl/main.c
> b/drivers/net/wireless/ath/ath6kl/main.c
>> index db95f85751e3..4e186f8b3950 100644
>> --- a/drivers/net/wireless/ath/ath6kl/main.c
>> +++ b/drivers/net/wireless/ath/ath6kl/main.c
>> @@ -579,13 +579,9 @@ static int ath6kl_commit_ch_switch(struct ath6kl_vif
> *vif, u16 channel)
>>                   * reconfigure any saved RSN IE capabilites in the beacon
> /
>>                   * probe response to stay in sync with the supplicant.
>>                   */
>> -               if (vif->rsn_capab &&
>> -                   test_bit(ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE,
>> -                            ar->fw_capabilities))
>> -                       ath6kl_wmi_set_ie_cmd(ar->wmi, vif->fw_vif_idx,
>> -                                             WLAN_EID_RSN,
> WMI_RSN_IE_CAPB,
>> -                                             (const u8 *)
> &vif->rsn_capab,
>> -                                             sizeof(vif->rsn_capab));
>> +               if (vif->rsn_capab)
>> +                       ath6kl_wmi_set_rsn_cap(ar->wmi, vif->fw_vif_idx,
>> +                                              vif->rsn_capab);
>
>
> So, basically same comment as the first one.
>
>>                  return ath6kl_wmi_ap_profile_commit(ar->wmi,
> vif->fw_vif_idx,
>>                                                      &vif->profile);
>> diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c
> b/drivers/net/wireless/ath/ath6kl/wmi.c
>> index 777acc564ac9..d7de9224d755 100644
>> --- a/drivers/net/wireless/ath/ath6kl/wmi.c
>> +++ b/drivers/net/wireless/ath/ath6kl/wmi.c
>> @@ -4168,3 +4168,26 @@ void ath6kl_wmi_shutdown(struct wmi *wmi)
>>          kfree(wmi->last_mgmt_tx_frame);
>>          kfree(wmi);
>>   }
>> +
>> +int ath6kl_wmi_set_rsn_cap(struct wmi *wmi, u8 if_idx, u16 rsn_cap)
>> +{
>> +       struct sk_buff *skb;
>> +       struct wmi_rsn_cap_cmd *cmd;
>> +
>> +       skb = ath6kl_wmi_get_new_buf(sizeof(*cmd));
>> +       if (!skb)
>> +               return -ENOMEM;
>> +
>> +       ath6kl_dbg(ATH6KL_DBG_WMI, "set_rsn_cap: 0x%04x\n", rsn_cap);
>> +
>> +       cmd = (struct wmi_rsn_cap_cmd *)skb->data;
>> +       cmd->rsn_cap = cpu_to_le16(rsn_cap);
>> +
>> +       return ath6kl_wmi_cmd_send(wmi, if_idx, skb,
> WMI_SET_RSN_CAP_CMDID,
>> +                                  NO_SYNC_WMIFLAG);
>> +}
>> +
>> +int ath6kl_wmi_get_rsn_cap(struct wmi *wmi, u8 if_idx)
>> +{
>> +       return ath6kl_wmi_simple_cmd(wmi, if_idx, WMI_GET_RSN_CAP_CMDID);
>> +}
>> diff --git a/drivers/net/wireless/ath/ath6kl/wmi.h
> b/drivers/net/wireless/ath/ath6kl/wmi.h
>> index a60bb49fe920..011d4b6fb5ab 100644
>> --- a/drivers/net/wireless/ath/ath6kl/wmi.h
>> +++ b/drivers/net/wireless/ath/ath6kl/wmi.h
>> @@ -597,10 +597,6 @@ enum wmi_cmd_id {
>
>>          WMI_GREENTX_PARAMS_CMDID,
>
>> -       WMI_RTT_MEASREQ_CMDID,
>> -       WMI_RTT_CAPREQ_CMDID,
>> -       WMI_RTT_STATUSREQ_CMDID,
>> -
>>          /* WPS Commands */
>>          WMI_WPS_START_CMDID,
>>          WMI_GET_WPS_STATUS_CMDID,
>> @@ -621,6 +617,10 @@ enum wmi_cmd_id {
>>          WMI_RX_FILTER_COALESCE_FILTER_OP_CMDID,
>>          WMI_RX_FILTER_SET_FRAME_TEST_LIST_CMDID,
>
>> +       WMI_RTT_MEASREQ_CMDID,
>> +       WMI_RTT_CAPREQ_CMDID,
>> +       WMI_RTT_STATUSREQ_CMDID,
>> +
>>          WMI_SEND_MGMT_CMDID,
>>          WMI_BEGIN_SCAN_CMDID,
>
>> @@ -2533,6 +2533,10 @@ enum wmi_sync_flag {
>>          END_WMIFLAG
>>   };
>
>
> I can factually state that the above reordering of the commands is wrong
> for a 6003. The order in the WMI file is consistent for a 6003. Your
> reordering is consistent for a 6004. Now, the only commands affected by the
> reordering aren't utilized by the driver, other than your added get/set
> RSN_CAP_CMDIDs.  But on a 6003, your now used WMI_SET_RSN_CAP_CMDID will
> trigger a WMI_GET_OPPPS_CMDID, which isn't what you want.
>
> I've encountered this issue a before - wmi code mismatch between the
> different firmwares and the driver. This is a limitation of the driver that
> probably should be resolved in some meaningful way. To date, it's been
> mitigated by the driver just not using the higher-numbered commands.  But
> by "meaningful way" I don't include redefining command IDs in favor of one
> particular person's firmware and causing problems on the other 99% of
> devices out there.

Agreed. I do not have access to much information for the ath6k, so
this was more like a shot in the dark.

>
>> +struct wmi_rsn_cap_cmd {
>> +       __le16 rsn_cap;
>> +} __packed;
>> +
>>   enum htc_endpoint_id ath6kl_wmi_get_control_ep(struct wmi *wmi);
>>   void ath6kl_wmi_set_control_ep(struct wmi *wmi, enum htc_endpoint_id
> ep_id);
>>   int ath6kl_wmi_dix_2_dot3(struct wmi *wmi, struct sk_buff *skb);
>> @@ -2728,4 +2732,7 @@ void *ath6kl_wmi_init(struct ath6kl *devt);
>>   void ath6kl_wmi_shutdown(struct wmi *wmi);
>>   void ath6kl_wmi_reset(struct wmi *wmi);
>
>> +int ath6kl_wmi_set_rsn_cap(struct wmi *wmi, u8 if_idx, u16 rsn_cap);
>> +int ath6kl_wmi_get_rsn_cap(struct wmi *wmi, u8 if_idx);
>> +
>>   #endif /* WMI_H */
>> --
>> 2.14.1
>
>
>  From your answer to Kalle re: what hardware
>
>> I have tested this in an Atheros QCA6234. kmsg shows this about the fw:
>
>> ath6kl: ar6004 hw 3.0 sdio fw 3.5.0.604 api 1
>> ath6kl: firmware supports: 64bit-rates,ap-inactivity-mins
>
> The firmware you're using is old. Mine for the QCA6234 is more advanced
> than that and has the rsn-cap-override flag, but even the stock one in
> linux-firmware is more up-to-date: 3.5.0.2356 api 3. I haven't run it
> recently to see if it also has the rsn-cap-override flag, but it might.
> Maybe you can try the current firmware to see if it solves your issue?

This is hw 3.0, there is actually no ath6k/AR6004/hw3.0 folder in
linux-firmware. I would appreciate pointers for more modern fw for
this hardware version if you have them.

>
> Thanks,
> - Steve

Thanks,
Alfonso

>
> --
> Steve deRosier
> Cal-Sierra Consulting
> https://www.cal-sierra.com/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ath6kl: use WMI to set RSN capabilities
  2018-04-27 16:57   ` Alfonso Sanchez-Beato
@ 2018-04-27 17:34     ` Steve deRosier
  0 siblings, 0 replies; 8+ messages in thread
From: Steve deRosier @ 2018-04-27 17:34 UTC (permalink / raw)
  To: alfonso.sanchez-beato; +Cc: Kalle Valo, ath6kl, linux-wireless

On Fri, Apr 27, 2018 at 9:57 AM Alfonso Sanchez-Beato <
alfonso.sanchez-beato@canonical.com> wrote:

> Hi Steve,

> On Fri, Apr 27, 2018 at 5:47 PM, Steve deRosier <derosier@gmail.com>
wrote:
> > Hi Alfonso,
> >
> > On Thu, Apr 12, 2018 at 8:42 AM Alfonso S=C3=A1nchez-Beato <
> > alfonso.sanchez-beato@canonical.com> wrote:
> >
> >> This fixes AP mode when the ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE flag
> >> is not present in the FW. The id of some WMI commands is also fixed
> >> (there was an error in the enum order), and a function to set RSN caps
> >> is added.
> >
> >> Signed-off-by: Alfonso S=C3=A1nchez-Beato <
alfonso.sanchez-beato@canonical.com>
> >> ---
> >>   drivers/net/wireless/ath/ath6kl/cfg80211.c | 21 ++++++--------------=
-
> >>   drivers/net/wireless/ath/ath6kl/main.c     | 10 +++-------
> >>   drivers/net/wireless/ath/ath6kl/wmi.c      | 23
+++++++++++++++++++++++
> >>   drivers/net/wireless/ath/ath6kl/wmi.h      | 15 +++++++++++----
> >>   4 files changed, 43 insertions(+), 26 deletions(-)
> >
> >> diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c
> > b/drivers/net/wireless/ath/ath6kl/cfg80211.c
> >> index 2ba8cf3f38af..1b89c53d47e7 100644
> >> --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
> >> +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
> >> @@ -2933,13 +2933,11 @@ static int ath6kl_start_ap(struct wiphy *wiphy=
,
> > struct net_device *dev,
> >>           * advertise the same in beacon/probe response. Send
> >>           * the complete RSN IE capability field to firmware
> >>           */
> >> -       if (!ath6kl_get_rsn_capab(&info->beacon, (u8 *) &rsn_capab) &&
> >> -           test_bit(ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE,
> >> -                    ar->fw_capabilities)) {
> >> -               res =3D ath6kl_wmi_set_ie_cmd(ar->wmi, vif->fw_vif_idx=
,
> >> -                                           WLAN_EID_RSN,
WMI_RSN_IE_CAPB,
> >> -                                           (const u8 *) &rsn_capab,
> >> -                                           sizeof(rsn_capab));
> >> +       if (!ath6kl_get_rsn_capab(&info->beacon, (u8 *)&rsn_capab)) {
> >> +               ath6kl_dbg(ATH6KL_DBG_WLAN_CFG, "RSN caps %d\n",
> > rsn_capab);
> >> +
> >> +               res =3D ath6kl_wmi_set_rsn_cap(ar->wmi, vif->fw_vif_id=
x,
> >> +                                            rsn_capab);
> >>                  vif->rsn_capab =3D rsn_capab;
> >>                  if (res < 0)
> >>                          return res;
> >
> > So, let me see if I understand this correctly.  Original flow was:
> >
> > 1. get RSN capable from the beacon if one
> > 2. if the firmware is capable of the override, set the IE  else don't d=
o
> > anything
> >
> > New flow:
> > 1. get RSN capable from the beacon if one
> > 2. unconditionally send the rsn_cap WMI down
> >
> > So, what happens on a firmware that _does_ have the rsn-cap-override
flag
> > set? I'm guessing nothing good.  Admittedly I haven't tried your patch
on
> > my platform.
> >
> > I think that simply ignoring the flag and using a WMI instead of settin=
g
> > the IE on all devices AR6002..AR6004 is likely going to cause problems
for
> > everyone else.

> Admittedly, I have not a wide range of devices to test. This patch was
> mostly an RFC to see if the issue is only for a particular fw revision
> and to expose it for people that might find it useful. Note that I am
> not the first person to hit this, see for instance

> http://www.spinics.net/lists/linux-wireless/msg115085.html


Fair enough. However, breaking it for the rest of the devices out there
isn't good. I welcome the fixing of it, but it has to be done across the
spectrum of the supported chips.



> >
> >
> >> @@ -3918,14 +3916,7 @@ int ath6kl_cfg80211_init(struct ath6kl *ar)
> >>                  return -EINVAL;
> >>          }
> >
> >> -       /*
> >> -        * Even if the fw has HT support, advertise HT cap only when
> >> -        * the firmware has support to override RSN capability,
otherwise
> >> -        * 4-way handshake would fail.
> >> -        */
> >> -       if (!(ht &&
> >> -             test_bit(ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE,
> >> -                      ar->fw_capabilities))) {
> >> +       if (!ht) {
> >
> > Perhaps the comment isn't relevant if we're using the RSN WMI command
on a
> > device with the flag, but I'm willing to bet you neither tested it nor
that
> > the assumption is true.  I'm guessing this change just broke the 4-way
> > handshake for the majority of devices.
> >
> >>                  ath6kl_band_2ghz.ht_cap.cap =3D 0;
> >>                  ath6kl_band_2ghz.ht_cap.ht_supported =3D false;
> >>                  ath6kl_band_5ghz.ht_cap.cap =3D 0;
> >> diff --git a/drivers/net/wireless/ath/ath6kl/main.c
> > b/drivers/net/wireless/ath/ath6kl/main.c
> >> index db95f85751e3..4e186f8b3950 100644
> >> --- a/drivers/net/wireless/ath/ath6kl/main.c
> >> +++ b/drivers/net/wireless/ath/ath6kl/main.c
> >> @@ -579,13 +579,9 @@ static int ath6kl_commit_ch_switch(struct
ath6kl_vif
> > *vif, u16 channel)
> >>                   * reconfigure any saved RSN IE capabilites in the
beacon
> > /
> >>                   * probe response to stay in sync with the supplicant=
.
> >>                   */
> >> -               if (vif->rsn_capab &&
> >> -                   test_bit(ATH6KL_FW_CAPABILITY_RSN_CAP_OVERRIDE,
> >> -                            ar->fw_capabilities))
> >> -                       ath6kl_wmi_set_ie_cmd(ar->wmi, vif->fw_vif_idx=
,
> >> -                                             WLAN_EID_RSN,
> > WMI_RSN_IE_CAPB,
> >> -                                             (const u8 *)
> > &vif->rsn_capab,
> >> -                                             sizeof(vif->rsn_capab));
> >> +               if (vif->rsn_capab)
> >> +                       ath6kl_wmi_set_rsn_cap(ar->wmi,
vif->fw_vif_idx,
> >> +                                              vif->rsn_capab);
> >
> >
> > So, basically same comment as the first one.
> >
> >>                  return ath6kl_wmi_ap_profile_commit(ar->wmi,
> > vif->fw_vif_idx,
> >>                                                      &vif->profile);
> >> diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c
> > b/drivers/net/wireless/ath/ath6kl/wmi.c
> >> index 777acc564ac9..d7de9224d755 100644
> >> --- a/drivers/net/wireless/ath/ath6kl/wmi.c
> >> +++ b/drivers/net/wireless/ath/ath6kl/wmi.c
> >> @@ -4168,3 +4168,26 @@ void ath6kl_wmi_shutdown(struct wmi *wmi)
> >>          kfree(wmi->last_mgmt_tx_frame);
> >>          kfree(wmi);
> >>   }
> >> +
> >> +int ath6kl_wmi_set_rsn_cap(struct wmi *wmi, u8 if_idx, u16 rsn_cap)
> >> +{
> >> +       struct sk_buff *skb;
> >> +       struct wmi_rsn_cap_cmd *cmd;
> >> +
> >> +       skb =3D ath6kl_wmi_get_new_buf(sizeof(*cmd));
> >> +       if (!skb)
> >> +               return -ENOMEM;
> >> +
> >> +       ath6kl_dbg(ATH6KL_DBG_WMI, "set_rsn_cap: 0x%04x\n", rsn_cap);
> >> +
> >> +       cmd =3D (struct wmi_rsn_cap_cmd *)skb->data;
> >> +       cmd->rsn_cap =3D cpu_to_le16(rsn_cap);
> >> +
> >> +       return ath6kl_wmi_cmd_send(wmi, if_idx, skb,
> > WMI_SET_RSN_CAP_CMDID,
> >> +                                  NO_SYNC_WMIFLAG);
> >> +}
> >> +
> >> +int ath6kl_wmi_get_rsn_cap(struct wmi *wmi, u8 if_idx)
> >> +{
> >> +       return ath6kl_wmi_simple_cmd(wmi, if_idx,
WMI_GET_RSN_CAP_CMDID);
> >> +}
> >> diff --git a/drivers/net/wireless/ath/ath6kl/wmi.h
> > b/drivers/net/wireless/ath/ath6kl/wmi.h
> >> index a60bb49fe920..011d4b6fb5ab 100644
> >> --- a/drivers/net/wireless/ath/ath6kl/wmi.h
> >> +++ b/drivers/net/wireless/ath/ath6kl/wmi.h
> >> @@ -597,10 +597,6 @@ enum wmi_cmd_id {
> >
> >>          WMI_GREENTX_PARAMS_CMDID,
> >
> >> -       WMI_RTT_MEASREQ_CMDID,
> >> -       WMI_RTT_CAPREQ_CMDID,
> >> -       WMI_RTT_STATUSREQ_CMDID,
> >> -
> >>          /* WPS Commands */
> >>          WMI_WPS_START_CMDID,
> >>          WMI_GET_WPS_STATUS_CMDID,
> >> @@ -621,6 +617,10 @@ enum wmi_cmd_id {
> >>          WMI_RX_FILTER_COALESCE_FILTER_OP_CMDID,
> >>          WMI_RX_FILTER_SET_FRAME_TEST_LIST_CMDID,
> >
> >> +       WMI_RTT_MEASREQ_CMDID,
> >> +       WMI_RTT_CAPREQ_CMDID,
> >> +       WMI_RTT_STATUSREQ_CMDID,
> >> +
> >>          WMI_SEND_MGMT_CMDID,
> >>          WMI_BEGIN_SCAN_CMDID,
> >
> >> @@ -2533,6 +2533,10 @@ enum wmi_sync_flag {
> >>          END_WMIFLAG
> >>   };
> >
> >
> > I can factually state that the above reordering of the commands is wron=
g
> > for a 6003. The order in the WMI file is consistent for a 6003. Your
> > reordering is consistent for a 6004. Now, the only commands affected by
the
> > reordering aren't utilized by the driver, other than your added get/set
> > RSN_CAP_CMDIDs.  But on a 6003, your now used WMI_SET_RSN_CAP_CMDID wil=
l
> > trigger a WMI_GET_OPPPS_CMDID, which isn't what you want.
> >
> > I've encountered this issue a before - wmi code mismatch between the
> > different firmwares and the driver. This is a limitation of the driver
that
> > probably should be resolved in some meaningful way. To date, it's been
> > mitigated by the driver just not using the higher-numbered commands.
But
> > by "meaningful way" I don't include redefining command IDs in favor of
one
> > particular person's firmware and causing problems on the other 99% of
> > devices out there.

> Agreed. I do not have access to much information for the ath6k, so
> this was more like a shot in the dark.

> >
> >> +struct wmi_rsn_cap_cmd {
> >> +       __le16 rsn_cap;
> >> +} __packed;
> >> +
> >>   enum htc_endpoint_id ath6kl_wmi_get_control_ep(struct wmi *wmi);
> >>   void ath6kl_wmi_set_control_ep(struct wmi *wmi, enum htc_endpoint_id
> > ep_id);
> >>   int ath6kl_wmi_dix_2_dot3(struct wmi *wmi, struct sk_buff *skb);
> >> @@ -2728,4 +2732,7 @@ void *ath6kl_wmi_init(struct ath6kl *devt);
> >>   void ath6kl_wmi_shutdown(struct wmi *wmi);
> >>   void ath6kl_wmi_reset(struct wmi *wmi);
> >
> >> +int ath6kl_wmi_set_rsn_cap(struct wmi *wmi, u8 if_idx, u16 rsn_cap);
> >> +int ath6kl_wmi_get_rsn_cap(struct wmi *wmi, u8 if_idx);
> >> +
> >>   #endif /* WMI_H */
> >> --
> >> 2.14.1
> >
> >
> >  From your answer to Kalle re: what hardware
> >
> >> I have tested this in an Atheros QCA6234. kmsg shows this about the fw=
:
> >
> >> ath6kl: ar6004 hw 3.0 sdio fw 3.5.0.604 api 1
> >> ath6kl: firmware supports: 64bit-rates,ap-inactivity-mins
> >
> > The firmware you're using is old. Mine for the QCA6234 is more advanced
> > than that and has the rsn-cap-override flag, but even the stock one in
> > linux-firmware is more up-to-date: 3.5.0.2356 api 3. I haven't run it
> > recently to see if it also has the rsn-cap-override flag, but it might.
> > Maybe you can try the current firmware to see if it solves your issue?

> This is hw 3.0, there is actually no ath6k/AR6004/hw3.0 folder in
> linux-firmware. I would appreciate pointers for more modern fw for
> this hardware version if you have them.

Ah.  Silly me, I just looked at the highest number in linux-firmware.
You're right, there's no HW 3.0 in there. I know there's a publically
available firmware for HW 3.0 out there, because I have a note on it, but I
don't recall where I saw it as it's been years.

@Kalle - do you have a more recent official QCA drop for this chip? And if
so, can we get it into linux-firmware?

You never said who's module you're using. If it's a Laird SD/MSD/WB50NBT,
the firmware I use has the rsn-cap-override and is easy to get from them.
It should look like:
     ar6004 hw 3.0 sdio fw 3.5.0.10011 api 5
     firmware supports:
sched-scan,rsn-cap-override,sscan-match-list,rssi-scan-thold,tx-err-notify,=
sched-scan-v2,hb-poll,64bit-rates,no-ip-checksu

There possibly is a more current version than I have on-hand.

- Steve

--
Steve deRosier
Cal-Sierra Consulting
https://www.cal-sierra.com/

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-04-27 17:35 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-12 15:41 [PATCH] ath6kl: use WMI to set RSN capabilities Alfonso Sánchez-Beato
2018-04-13 10:48 ` Kalle Valo
2018-04-13 11:19   ` Alfonso Sanchez-Beato
2018-04-13 11:22     ` Kalle Valo
2018-04-27 11:23 ` Kalle Valo
2018-04-27 15:47 ` [PATCH] " Steve deRosier
2018-04-27 16:57   ` Alfonso Sanchez-Beato
2018-04-27 17:34     ` Steve deRosier

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.