[PATCH 0/4] ath6kl: Fix AP mode PS buffering and enable AP/P2P

[PATCH 0/4] ath6kl: Fix AP mode PS buffering and enable AP/P2P

[Jouni Malinen]

The first two patches in this set fix couple of issues in TIM updates
for AP mode PS buffering. With those fixed, the AP mode support is ready
to be enabled and since the initial P2P support was waiting for AP mode
to work properly, that can also be enabled now. Please note that the P2P
operations need a firmware update which has not yet been released.

Jouni Malinen (4):
  ath6kl: Fix WMI message structure for AP_SET_PVB
  ath6kl: Fix AP mode connect event parsing and TIM updates
  ath6kl: Allow AP mode to be configured
  ath6kl: Allow enabling of P2P support

 drivers/net/wireless/ath/ath6kl/cfg80211.c |   17 +++++-
 drivers/net/wireless/ath/ath6kl/core.h     |    6 ++
 drivers/net/wireless/ath/ath6kl/init.c     |   31 +++++++--
 drivers/net/wireless/ath/ath6kl/main.c     |   92 ++++++++++++----------------
 drivers/net/wireless/ath/ath6kl/target.h   |    6 ++
 drivers/net/wireless/ath/ath6kl/wmi.c      |   45 ++++++++++++--
 drivers/net/wireless/ath/ath6kl/wmi.h      |   30 ++++++++--
 7 files changed, 157 insertions(+), 70 deletions(-)

-- 
1.7.4.1

[PATCH 1/4] ath6kl: Fix WMI message structure for AP_SET_PVB

[Jouni Malinen]

There is a 2-octet reserved field between the flag and aid fields. Fix
that to make the target actually behave as requested.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/wmi.c |    1 +
 drivers/net/wireless/ath/ath6kl/wmi.h |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index c9ec630..b2c5c40 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -2740,6 +2740,7 @@ int ath6kl_wmi_set_pvb_cmd(struct wmi *wmi, u16 aid, bool flag)
 
 	cmd = (struct wmi_ap_set_pvb_cmd *) skb->data;
 	cmd->aid = cpu_to_le16(aid);
+	cmd->rsvd = cpu_to_le16(0);
 	cmd->flag = cpu_to_le32(flag);
 
 	ret = ath6kl_wmi_cmd_send(wmi, skb, WMI_AP_SET_PVB_CMDID,
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.h b/drivers/net/wireless/ath/ath6kl/wmi.h
index a78e21b..e86b81d 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.h
+++ b/drivers/net/wireless/ath/ath6kl/wmi.h
@@ -1922,6 +1922,7 @@ struct wmi_ap_set_mlme_cmd {
 
 struct wmi_ap_set_pvb_cmd {
 	__le32 flag;
+	__le16 rsvd;
 	__le16 aid;
 } __packed;
 
-- 
1.7.4.1

[PATCH 2/4] ath6kl: Fix AP mode connect event parsing and TIM updates

[Jouni Malinen]

This cleans up the connect event parsing by defining a union in
struct wmi_connect_event to match with the three possible sets of
fields that the target uses depending on which type of connect
event is being indicated. In addition, two AP cases are now
separated from ath6kl_connect_event() so that correct field names
can be used to make it actually possible to understand what the
code is doing.

The bug hiding in the previous mess was in parsing the AID incorrectly
when processing the new station connecting event in AP mode. The fix
here for that is also fixing TIM updates for PS buffering to use the
correct AID.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/core.h |    4 ++
 drivers/net/wireless/ath/ath6kl/main.c |   92 ++++++++++++++------------------
 drivers/net/wireless/ath/ath6kl/wmi.c  |   44 +++++++++++++--
 drivers/net/wireless/ath/ath6kl/wmi.h  |   29 ++++++++--
 4 files changed, 107 insertions(+), 62 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/core.h b/drivers/net/wireless/ath/ath6kl/core.h
index fdb796f..054da13 100644
--- a/drivers/net/wireless/ath/ath6kl/core.h
+++ b/drivers/net/wireless/ath/ath6kl/core.h
@@ -560,6 +560,10 @@ void ath6kl_connect_event(struct ath6kl *ar, u16 channel,
 			  u16 beacon_int, enum network_type net_type,
 			  u8 beacon_ie_len, u8 assoc_req_len,
 			  u8 assoc_resp_len, u8 *assoc_info);
+void ath6kl_connect_ap_mode_bss(struct ath6kl *ar, u16 channel);
+void ath6kl_connect_ap_mode_sta(struct ath6kl *ar, u16 aid, u8 *mac_addr,
+				u8 keymgmt, u8 ucipher, u8 auth,
+				u8 assoc_req_len, u8 *assoc_info);
 void ath6kl_disconnect_event(struct ath6kl *ar, u8 reason,
 			     u8 *bssid, u8 assoc_resp_len,
 			     u8 *assoc_info, u16 prot_reason_status);
diff --git a/drivers/net/wireless/ath/ath6kl/main.c b/drivers/net/wireless/ath/ath6kl/main.c
index 3cefca6..d510046 100644
--- a/drivers/net/wireless/ath/ath6kl/main.c
+++ b/drivers/net/wireless/ath/ath6kl/main.c
@@ -519,57 +519,55 @@ static void ath6kl_install_static_wep_keys(struct ath6kl *ar)
 	}
 }
 
-static void ath6kl_connect_ap_mode(struct ath6kl *ar, u16 channel, u8 *bssid,
-				   u16 listen_int, u16 beacon_int,
-				   u8 assoc_req_len, u8 *assoc_info)
+void ath6kl_connect_ap_mode_bss(struct ath6kl *ar, u16 channel)
 {
-	struct net_device *dev = ar->net_dev;
-	u8 *ies = NULL, *wpa_ie = NULL, *pos;
-	size_t ies_len = 0;
-	struct station_info sinfo;
 	struct ath6kl_req_key *ik;
 	int res;
 	u8 key_rsc[ATH6KL_KEY_SEQ_LEN];
 
-	if (memcmp(dev->dev_addr, bssid, ETH_ALEN) == 0) {
-		ik = &ar->ap_mode_bkey;
+	ik = &ar->ap_mode_bkey;
 
-		ath6kl_dbg(ATH6KL_DBG_WLAN_CFG, "AP mode started on %u MHz\n",
-			   channel);
+	ath6kl_dbg(ATH6KL_DBG_WLAN_CFG, "AP mode started on %u MHz\n", channel);
 
-		switch (ar->auth_mode) {
-		case NONE_AUTH:
-			if (ar->prwise_crypto == WEP_CRYPT)
-				ath6kl_install_static_wep_keys(ar);
+	switch (ar->auth_mode) {
+	case NONE_AUTH:
+		if (ar->prwise_crypto == WEP_CRYPT)
+			ath6kl_install_static_wep_keys(ar);
+		break;
+	case WPA_PSK_AUTH:
+	case WPA2_PSK_AUTH:
+	case (WPA_PSK_AUTH | WPA2_PSK_AUTH):
+		if (!ik->valid)
 			break;
-		case WPA_PSK_AUTH:
-		case WPA2_PSK_AUTH:
-		case (WPA_PSK_AUTH|WPA2_PSK_AUTH):
-			if (!ik->valid)
-				break;
 
-			ath6kl_dbg(ATH6KL_DBG_WLAN_CFG, "Delayed addkey for "
-				   "the initial group key for AP mode\n");
-			memset(key_rsc, 0, sizeof(key_rsc));
-			res = ath6kl_wmi_addkey_cmd(
-				ar->wmi, ik->key_index, ik->key_type,
-				GROUP_USAGE, ik->key_len, key_rsc, ik->key,
-				KEY_OP_INIT_VAL, NULL, SYNC_BOTH_WMIFLAG);
-			if (res) {
-				ath6kl_dbg(ATH6KL_DBG_WLAN_CFG, "Delayed "
-					   "addkey failed: %d\n", res);
-			}
-			break;
+		ath6kl_dbg(ATH6KL_DBG_WLAN_CFG, "Delayed addkey for "
+			   "the initial group key for AP mode\n");
+		memset(key_rsc, 0, sizeof(key_rsc));
+		res = ath6kl_wmi_addkey_cmd(
+			ar->wmi, ik->key_index, ik->key_type,
+			GROUP_USAGE, ik->key_len, key_rsc, ik->key,
+			KEY_OP_INIT_VAL, NULL, SYNC_BOTH_WMIFLAG);
+		if (res) {
+			ath6kl_dbg(ATH6KL_DBG_WLAN_CFG, "Delayed "
+				   "addkey failed: %d\n", res);
 		}
-
-		ath6kl_wmi_bssfilter_cmd(ar->wmi, NONE_BSS_FILTER, 0);
-		set_bit(CONNECTED, &ar->flag);
-		netif_carrier_on(ar->net_dev);
-		return;
+		break;
 	}
 
-	ath6kl_dbg(ATH6KL_DBG_TRC, "new station %pM aid=%d\n",
-		   bssid, channel);
+	ath6kl_wmi_bssfilter_cmd(ar->wmi, NONE_BSS_FILTER, 0);
+	set_bit(CONNECTED, &ar->flag);
+	netif_carrier_on(ar->net_dev);
+}
+
+void ath6kl_connect_ap_mode_sta(struct ath6kl *ar, u16 aid, u8 *mac_addr,
+				u8 keymgmt, u8 ucipher, u8 auth,
+				u8 assoc_req_len, u8 *assoc_info)
+{
+	u8 *ies = NULL, *wpa_ie = NULL, *pos;
+	size_t ies_len = 0;
+	struct station_info sinfo;
+
+	ath6kl_dbg(ATH6KL_DBG_TRC, "new station %pM aid=%d\n", mac_addr, aid);
 
 	if (assoc_req_len > sizeof(struct ieee80211_hdr_3addr)) {
 		struct ieee80211_mgmt *mgmt =
@@ -606,10 +604,9 @@ static void ath6kl_connect_ap_mode(struct ath6kl *ar, u16 channel, u8 *bssid,
 		pos += 2 + pos[1];
 	}
 
-	ath6kl_add_new_sta(ar, bssid, channel, wpa_ie,
+	ath6kl_add_new_sta(ar, mac_addr, aid, wpa_ie,
 			   wpa_ie ? 2 + wpa_ie[1] : 0,
-			   listen_int & 0xFF, beacon_int,
-			   (listen_int >> 8) & 0xFF);
+			   keymgmt, ucipher, auth);
 
 	/* send event to application */
 	memset(&sinfo, 0, sizeof(sinfo));
@@ -620,11 +617,9 @@ static void ath6kl_connect_ap_mode(struct ath6kl *ar, u16 channel, u8 *bssid,
 	sinfo.assoc_req_ies_len = ies_len;
 	sinfo.filled |= STATION_INFO_ASSOC_REQ_IES;
 
-	cfg80211_new_sta(ar->net_dev, bssid, &sinfo, GFP_KERNEL);
+	cfg80211_new_sta(ar->net_dev, mac_addr, &sinfo, GFP_KERNEL);
 
 	netif_wake_queue(ar->net_dev);
-
-	return;
 }
 
 /* Functions for Tx credit handling */
@@ -1030,13 +1025,6 @@ void ath6kl_connect_event(struct ath6kl *ar, u16 channel, u8 *bssid,
 {
 	unsigned long flags;
 
-	if (ar->nw_type == AP_NETWORK) {
-		ath6kl_connect_ap_mode(ar, channel, bssid, listen_int,
-				       beacon_int, assoc_req_len,
-				       assoc_info + beacon_ie_len);
-		return;
-	}
-
 	ath6kl_cfg80211_connect_event(ar, channel, bssid,
 				      listen_int, beacon_int,
 				      net_type, beacon_ie_len,
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index b2c5c40..b56830f 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -699,14 +699,47 @@ static int ath6kl_wmi_connect_event_rx(struct wmi *wmi, u8 *datap, int len)
 {
 	struct wmi_connect_event *ev;
 	u8 *pie, *peie;
+	struct ath6kl *ar = wmi->parent_dev;
 
 	if (len < sizeof(struct wmi_connect_event))
 		return -EINVAL;
 
 	ev = (struct wmi_connect_event *) datap;
 
+	if (ar->nw_type == AP_NETWORK) {
+		/* AP mode start/STA connected event */
+		struct net_device *dev = ar->net_dev;
+		if (memcmp(dev->dev_addr, ev->u.ap_bss.bssid, ETH_ALEN) == 0) {
+			ath6kl_dbg(ATH6KL_DBG_WMI, "%s: freq %d bssid %pM "
+				   "(AP started)\n",
+				   __func__, le16_to_cpu(ev->u.ap_bss.ch),
+				   ev->u.ap_bss.bssid);
+			ath6kl_connect_ap_mode_bss(
+				ar, le16_to_cpu(ev->u.ap_bss.ch));
+		} else {
+			ath6kl_dbg(ATH6KL_DBG_WMI, "%s: aid %u mac_addr %pM "
+				   "auth=%u keymgmt=%u cipher=%u apsd_info=%u "
+				   "(STA connected)\n",
+				   __func__, ev->u.ap_sta.aid,
+				   ev->u.ap_sta.mac_addr,
+				   ev->u.ap_sta.auth,
+				   ev->u.ap_sta.keymgmt,
+				   le16_to_cpu(ev->u.ap_sta.cipher),
+				   ev->u.ap_sta.apsd_info);
+			ath6kl_connect_ap_mode_sta(
+				ar, ev->u.ap_sta.aid, ev->u.ap_sta.mac_addr,
+				ev->u.ap_sta.keymgmt,
+				le16_to_cpu(ev->u.ap_sta.cipher),
+				ev->u.ap_sta.auth, ev->assoc_req_len,
+				ev->assoc_info + ev->beacon_ie_len);
+		}
+		return 0;
+	}
+
+	/* STA/IBSS mode connection event */
+
 	ath6kl_dbg(ATH6KL_DBG_WMI, "%s: freq %d bssid %pM\n",
-		   __func__, ev->ch, ev->bssid);
+		   __func__, le16_to_cpu(ev->u.sta.ch), ev->u.sta.bssid);
 
 	/* Start of assoc rsp IEs */
 	pie = ev->assoc_info + ev->beacon_ie_len +
@@ -735,10 +768,11 @@ static int ath6kl_wmi_connect_event_rx(struct wmi *wmi, u8 *datap, int len)
 		pie += pie[1] + 2;
 	}
 
-	ath6kl_connect_event(wmi->parent_dev, le16_to_cpu(ev->ch), ev->bssid,
-			     le16_to_cpu(ev->listen_intvl),
-			     le16_to_cpu(ev->beacon_intvl),
-			     le32_to_cpu(ev->nw_type),
+	ath6kl_connect_event(wmi->parent_dev, le16_to_cpu(ev->u.sta.ch),
+			     ev->u.sta.bssid,
+			     le16_to_cpu(ev->u.sta.listen_intvl),
+			     le16_to_cpu(ev->u.sta.beacon_intvl),
+			     le32_to_cpu(ev->u.sta.nw_type),
 			     ev->beacon_ie_len, ev->assoc_req_len,
 			     ev->assoc_resp_len, ev->assoc_info);
 
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.h b/drivers/net/wireless/ath/ath6kl/wmi.h
index e86b81d..5ca8c8e 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.h
+++ b/drivers/net/wireless/ath/ath6kl/wmi.h
@@ -1275,11 +1275,30 @@ struct wmi_ready_event_2 {
 
 /* Connect Event */
 struct wmi_connect_event {
-	__le16 ch;
-	u8 bssid[ETH_ALEN];
-	__le16 listen_intvl;
-	__le16 beacon_intvl;
-	__le32 nw_type;
+	union {
+		struct {
+			__le16 ch;
+			u8 bssid[ETH_ALEN];
+			__le16 listen_intvl;
+			__le16 beacon_intvl;
+			__le32 nw_type;
+		} sta;
+		struct {
+			u8 phymode;
+			u8 aid;
+			u8 mac_addr[ETH_ALEN];
+			u8 auth;
+			u8 keymgmt;
+			__le16 cipher;
+			u8 apsd_info;
+			u8 unused[3];
+		} ap_sta;
+		struct {
+			__le16 ch;
+			u8 bssid[ETH_ALEN];
+			u8 unused[8];
+		} ap_bss;
+	} u;
 	u8 beacon_ie_len;
 	u8 assoc_req_len;
 	u8 assoc_resp_len;
-- 
1.7.4.1

[PATCH 3/4] ath6kl: Allow AP mode to be configured

[Jouni Malinen]

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/cfg80211.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index 1fe55f6..6405696 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -1221,6 +1221,9 @@ static int ath6kl_cfg80211_change_iface(struct wiphy *wiphy,
 	case NL80211_IFTYPE_ADHOC:
 		ar->next_mode = ADHOC_NETWORK;
 		break;
+	case NL80211_IFTYPE_AP:
+		ar->next_mode = AP_NETWORK;
+		break;
 	case NL80211_IFTYPE_P2P_CLIENT:
 		ar->next_mode = INFRA_NETWORK;
 		break;
@@ -1956,7 +1959,7 @@ struct wireless_dev *ath6kl_cfg80211_init(struct device *dev)
 	set_wiphy_dev(wdev->wiphy, dev);
 
 	wdev->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
-	    BIT(NL80211_IFTYPE_ADHOC);
+		BIT(NL80211_IFTYPE_ADHOC) | BIT(NL80211_IFTYPE_AP);
 	/* max num of ssids that can be probed during scanning */
 	wdev->wiphy->max_scan_ssids = MAX_PROBED_SSID_INDEX;
 	wdev->wiphy->max_scan_ie_len = 1000; /* FIX: what is correct limit? */
-- 
1.7.4.1

[PATCH 4/4] ath6kl: Allow enabling of P2P support

[Jouni Malinen]

For now, use a module parameter (ath6kl_p2p) to allow P2P support
to be enabled. This is needed since there is no mechanism for
enabling the P2P mode more dynamically for a single netdev.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/cfg80211.c |   12 ++++++++++
 drivers/net/wireless/ath/ath6kl/core.h     |    2 +
 drivers/net/wireless/ath/ath6kl/init.c     |   31 +++++++++++++++++++++------
 drivers/net/wireless/ath/ath6kl/target.h   |    6 +++++
 4 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index 6405696..a889bf4 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -20,6 +20,10 @@
 #include "hif-ops.h"
 #include "testmode.h"
 
+static unsigned int ath6kl_p2p;
+
+module_param(ath6kl_p2p, uint, 0644);
+
 #define RATETAB_ENT(_rate, _rateid, _flags) {   \
 	.bitrate    = (_rate),                  \
 	.flags      = (_flags),                 \
@@ -1936,6 +1940,7 @@ struct wireless_dev *ath6kl_cfg80211_init(struct device *dev)
 {
 	int ret = 0;
 	struct wireless_dev *wdev;
+	struct ath6kl *ar;
 
 	wdev = kzalloc(sizeof(struct wireless_dev), GFP_KERNEL);
 	if (!wdev) {
@@ -1951,6 +1956,9 @@ struct wireless_dev *ath6kl_cfg80211_init(struct device *dev)
 		return NULL;
 	}
 
+	ar = wiphy_priv(wdev->wiphy);
+	ar->p2p = !!ath6kl_p2p;
+
 	wdev->wiphy->mgmt_stypes = ath6kl_mgmt_stypes;
 
 	wdev->wiphy->max_remain_on_channel_duration = 5000;
@@ -1960,6 +1968,10 @@ struct wireless_dev *ath6kl_cfg80211_init(struct device *dev)
 
 	wdev->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
 		BIT(NL80211_IFTYPE_ADHOC) | BIT(NL80211_IFTYPE_AP);
+	if (ar->p2p) {
+		wdev->wiphy->interface_modes |= BIT(NL80211_IFTYPE_P2P_GO) |
+			BIT(NL80211_IFTYPE_P2P_CLIENT);
+	}
 	/* max num of ssids that can be probed during scanning */
 	wdev->wiphy->max_scan_ssids = MAX_PROBED_SSID_INDEX;
 	wdev->wiphy->max_scan_ie_len = 1000; /* FIX: what is correct limit? */
diff --git a/drivers/net/wireless/ath/ath6kl/core.h b/drivers/net/wireless/ath/ath6kl/core.h
index 054da13..c6ed1fc 100644
--- a/drivers/net/wireless/ath/ath6kl/core.h
+++ b/drivers/net/wireless/ath/ath6kl/core.h
@@ -471,6 +471,8 @@ struct ath6kl {
 	bool probe_req_report;
 	u16 next_chan;
 
+	bool p2p;
+
 #ifdef CONFIG_ATH6KL_DEBUG
 	struct {
 		struct circ_buf fwlog_buf;
diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c
index 3d67025..eca34aa 100644
--- a/drivers/net/wireless/ath/ath6kl/init.c
+++ b/drivers/net/wireless/ath/ath6kl/init.c
@@ -448,13 +448,26 @@ static int ath6kl_target_config_wlan_params(struct ath6kl *ar)
 			status = -EIO;
 		}
 
-	ret = ath6kl_wmi_info_req_cmd(ar->wmi, P2P_FLAG_CAPABILITIES_REQ |
-				      P2P_FLAG_MACADDR_REQ |
-				      P2P_FLAG_HMODEL_REQ);
-	if (ret) {
-		ath6kl_dbg(ATH6KL_DBG_TRC, "failed to request P2P "
-			   "capabilities (%d) - assuming P2P not supported\n",
-			   ret);
+	if (ar->p2p) {
+		ret = ath6kl_wmi_info_req_cmd(ar->wmi,
+					      P2P_FLAG_CAPABILITIES_REQ |
+					      P2P_FLAG_MACADDR_REQ |
+					      P2P_FLAG_HMODEL_REQ);
+		if (ret) {
+			ath6kl_dbg(ATH6KL_DBG_TRC, "failed to request P2P "
+				   "capabilities (%d) - assuming P2P not "
+				   "supported\n", ret);
+			ar->p2p = 0;
+		}
+	}
+
+	if (ar->p2p) {
+		/* Enable Probe Request reporting for P2P */
+		ret = ath6kl_wmi_probe_report_req_cmd(ar->wmi, true);
+		if (ret) {
+			ath6kl_dbg(ATH6KL_DBG_TRC, "failed to enable Probe "
+				   "Request reporting (%d)\n", ret);
+		}
 	}
 
 	return status;
@@ -492,6 +505,10 @@ int ath6kl_configure_target(struct ath6kl *ar)
 
 	param |= (1 << HI_OPTION_NUM_DEV_SHIFT);
 	param |= (fw_iftype << HI_OPTION_FW_MODE_SHIFT);
+	if (ar->p2p && fw_iftype == HI_OPTION_FW_MODE_BSS_STA) {
+		param |= HI_OPTION_FW_SUBMODE_P2PDEV <<
+			HI_OPTION_FW_SUBMODE_SHIFT;
+	}
 	param |= (0 << HI_OPTION_MAC_ADDR_METHOD_SHIFT);
 	param |= (0 << HI_OPTION_FW_BRIDGE_SHIFT);
 
diff --git a/drivers/net/wireless/ath/ath6kl/target.h b/drivers/net/wireless/ath/ath6kl/target.h
index dd8b953..7db06a5 100644
--- a/drivers/net/wireless/ath/ath6kl/target.h
+++ b/drivers/net/wireless/ath/ath6kl/target.h
@@ -304,6 +304,11 @@ struct host_interest {
 #define HI_OPTION_FW_MODE_BSS_STA 0x1
 #define HI_OPTION_FW_MODE_AP      0x2
 
+#define HI_OPTION_FW_SUBMODE_NONE      0x0
+#define HI_OPTION_FW_SUBMODE_P2PDEV    0x1
+#define HI_OPTION_FW_SUBMODE_P2PCLIENT 0x2
+#define HI_OPTION_FW_SUBMODE_P2PGO     0x3
+
 #define HI_OPTION_NUM_DEV_SHIFT   0x9
 
 #define HI_OPTION_FW_BRIDGE_SHIFT 0x04
@@ -316,6 +321,7 @@ struct host_interest {
 |------------------------------------------------------------------------------|
 */
 #define HI_OPTION_FW_MODE_SHIFT        0xC
+#define HI_OPTION_FW_SUBMODE_SHIFT     0x14
 
 /* Convert a Target virtual address into a Target physical address */
 #define AR6003_VTOP(vaddr) ((vaddr) & 0x001fffff)
-- 
1.7.4.1

Re: [PATCH 4/4] ath6kl: Allow enabling of P2P support

[Kalle Valo]

On 09/05/2011 05:38 PM, Jouni Malinen wrote:
> For now, use a module parameter (ath6kl_p2p) to allow P2P support
> to be enabled. This is needed since there is no mechanism for
> enabling the P2P mode more dynamically for a single netdev.

[...]

> --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
> +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
> @@ -20,6 +20,10 @@
>  #include "hif-ops.h"
>  #include "testmode.h"
>  
> +static unsigned int ath6kl_p2p;
> +
> +module_param(ath6kl_p2p, uint, 0644);

Currently all module parameters are in init.c. It's not the best place
and most likely will move to core.c soon, but I still would prefer to
have them all in one place. I think moving this to init.c would make sense.

> --- a/drivers/net/wireless/ath/ath6kl/core.h
> +++ b/drivers/net/wireless/ath/ath6kl/core.h
> @@ -471,6 +471,8 @@ struct ath6kl {
>  	bool probe_req_report;
>  	u16 next_chan;
>  
> +	bool p2p;

We have struct ath6kl::conf_flags exactly for this purpose, see
ATH6KL_CONF_ENABLE_11N for an example. I think we should use it also
with P2P.

> +	if (ar->p2p) {
> +		ret = ath6kl_wmi_info_req_cmd(ar->wmi,
> +					      P2P_FLAG_CAPABILITIES_REQ |
> +					      P2P_FLAG_MACADDR_REQ |
> +					      P2P_FLAG_HMODEL_REQ);
> +		if (ret) {
> +			ath6kl_dbg(ATH6KL_DBG_TRC, "failed to request P2P "
> +				   "capabilities (%d) - assuming P2P not "
> +				   "supported\n", ret);
> +			ar->p2p = 0;
> +		}
> +	}
> +
> +	if (ar->p2p) {

Can we combine this if block with the first one?

If it's ok for you, I'll commit the first three patches and we can talk
more about this patch.

Kalle

Re: [PATCH 4/4] ath6kl: Allow enabling of P2P support

[Kalle Valo]

On 09/06/2011 09:58 AM, Kalle Valo wrote:
> On 09/05/2011 05:38 PM, Jouni Malinen wrote:
>> +	if (ar->p2p) {
>> +		ret = ath6kl_wmi_info_req_cmd(ar->wmi,
>> +					      P2P_FLAG_CAPABILITIES_REQ |
>> +					      P2P_FLAG_MACADDR_REQ |
>> +					      P2P_FLAG_HMODEL_REQ);
>> +		if (ret) {
>> +			ath6kl_dbg(ATH6KL_DBG_TRC, "failed to request P2P "
>> +				   "capabilities (%d) - assuming P2P not "
>> +				   "supported\n", ret);
>> +			ar->p2p = 0;
>> +		}
>> +	}
>> +
>> +	if (ar->p2p) {
> 
> Can we combine this if block with the first one?

Forget this comment. I was blind and didn't see 'ar->p2p = 0'.

Kalle

Re: [PATCH 4/4] ath6kl: Allow enabling of P2P support

[Jouni Malinen]

On Tue, Sep 06, 2011 at 09:58:35AM +0300, Kalle Valo wrote:
> On 09/05/2011 05:38 PM, Jouni Malinen wrote:
> > --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
> > +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
> > +static unsigned int ath6kl_p2p;
> > +
> > +module_param(ath6kl_p2p, uint, 0644);
> 
> Currently all module parameters are in init.c. It's not the best place
> and most likely will move to core.c soon, but I still would prefer to
> have them all in one place. I think moving this to init.c would make sense.

I started with this in init.c, but that causes checkpatch warnings when
the variable needs to be accessed as extern in cfg80211.c. Or well, I
guess I could hide that by defining the extern in a header file, but
this parameter should not really be used in any other file than
cfg80211.c to initialize the flag in struct ath6kl. As such, I would
prefer not to make its visibility any larger that necessary. Why would
all module parameters need to be defined in the same C file?

> > --- a/drivers/net/wireless/ath/ath6kl/core.h
> > +++ b/drivers/net/wireless/ath/ath6kl/core.h
> > @@ -471,6 +471,8 @@ struct ath6kl {
> > +	bool p2p;
> 
> We have struct ath6kl::conf_flags exactly for this purpose, see
> ATH6KL_CONF_ENABLE_11N for an example. I think we should use it also
> with P2P.

Sounds reasonable.

> > +	if (ar->p2p) {
> > +		ret = ath6kl_wmi_info_req_cmd(ar->wmi,
> > +					      P2P_FLAG_CAPABILITIES_REQ |
> > +					      P2P_FLAG_MACADDR_REQ |
> > +					      P2P_FLAG_HMODEL_REQ);
> > +		if (ret) {
> > +			ath6kl_dbg(ATH6KL_DBG_TRC, "failed to request P2P "
> > +				   "capabilities (%d) - assuming P2P not "
> > +				   "supported\n", ret);
> > +			ar->p2p = 0;
> > +		}
> > +	}
> > +
> > +	if (ar->p2p) {
> 
> Can we combine this if block with the first one?

Sure, but that would add extra indentation level for the following code,
so this separate check-if-P2P-is-supported followed by
if-P2P-is-supported looks cleaner to me.

> If it's ok for you, I'll commit the first three patches and we can talk
> more about this patch.

Yes, please do.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH 4/4] ath6kl: Allow enabling of P2P support

[Kalle Valo]

On 09/06/2011 10:21 AM, Jouni Malinen wrote:
> On Tue, Sep 06, 2011 at 09:58:35AM +0300, Kalle Valo wrote:
>> On 09/05/2011 05:38 PM, Jouni Malinen wrote:
>>> --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c +++
>>> b/drivers/net/wireless/ath/ath6kl/cfg80211.c +static unsigned int
>>> ath6kl_p2p; + +module_param(ath6kl_p2p, uint, 0644);
>> 
>> Currently all module parameters are in init.c. It's not the best
>> place and most likely will move to core.c soon, but I still would
>> prefer to have them all in one place. I think moving this to init.c
>> would make sense.
> 
> I started with this in init.c, but that causes checkpatch warnings
> when the variable needs to be accessed as extern in cfg80211.c. Or
> well, I guess I could hide that by defining the extern in a header
> file, but this parameter should not really be used in any other file
> than cfg80211.c to initialize the flag in struct ath6kl. As such, I
> would prefer not to make its visibility any larger that necessary.

I was more thinking that ath6kl_p2p would not be exposed outside init.c,
instead you would set the appropriate conf_flag in ath6kl_core_init() or
similar function. But leave the module_param as it is for now, I will
cleanup the module parameters anyway soon.

> Why would all module parameters need to be defined in the same C
> file?

Just for consistency so that people don't need to grep different locations.

>> Can we combine this if block with the first one?
> 
> Sure, but that would add extra indentation level for the following
> code, so this separate check-if-P2P-is-supported followed by 
> if-P2P-is-supported looks cleaner to me.

Yeah, you are right. My original comment was bogus.

>> If it's ok for you, I'll commit the first three patches and we can
>> talk more about this patch.
> 
> Yes, please do.

Ok, I have applied the first three now.

Kalle

Re: [PATCH 4/4] ath6kl: Allow enabling of P2P support

[Jouni Malinen]

On Tue, Sep 06, 2011 at 10:51:55AM +0300, Kalle Valo wrote:
> I was more thinking that ath6kl_p2p would not be exposed outside init.c,
> instead you would set the appropriate conf_flag in ath6kl_core_init() or
> similar function. But leave the module_param as it is for now, I will
> cleanup the module parameters anyway soon.

This and the change to using ar->conf_flags instead of ar->p2p turned
out to be more complex changes. The main problem behind this is in the
order that the driver is allocating and initializing the data
structures. In theory, this sounds great and should be the longer term
direction, but with the current initialization code path, changing the
struct ath6kl data within ath6kl_core_alloc() (including cfg80211 alloc
and registration) and ath6kl_core_init() can conflict pretty easily. For
example, conf_flags are initialized in ath6kl_init() which is called
from ath6kl_core_init(), but this is done only after ath6kl_core_alloc()
has already returned and the P2P flags are needed there before cfg80211
registration..

I think that the initialization steps need to be reordered in a way
that cfg80211 registration happens somewhere near the end of init()
rather than in alloc(). Once this is done, the conf_flags can be set
based on firmware and target information and those can then be used to
set up the cfg80211 information before calling wiphy_register(). Until
that gets done, it seems safest to apply this P2P enabling patch as-is
and do the proposed clean up separately after the wiphy_register() call
is moved.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH 4/4] ath6kl: Allow enabling of P2P support

[Kalle Valo]

On 09/06/2011 12:35 PM, Jouni Malinen wrote:
> On Tue, Sep 06, 2011 at 10:51:55AM +0300, Kalle Valo wrote:
>> I was more thinking that ath6kl_p2p would not be exposed outside init.c,
>> instead you would set the appropriate conf_flag in ath6kl_core_init() or
>> similar function. But leave the module_param as it is for now, I will
>> cleanup the module parameters anyway soon.
> 
> This and the change to using ar->conf_flags instead of ar->p2p turned
> out to be more complex changes. The main problem behind this is in the
> order that the driver is allocating and initializing the data
> structures. In theory, this sounds great and should be the longer term
> direction, but with the current initialization code path, changing the
> struct ath6kl data within ath6kl_core_alloc() (including cfg80211 alloc
> and registration) and ath6kl_core_init() can conflict pretty easily. For
> example, conf_flags are initialized in ath6kl_init() which is called
> from ath6kl_core_init(), but this is done only after ath6kl_core_alloc()
> has already returned and the P2P flags are needed there before cfg80211
> registration..
> 
> I think that the initialization steps need to be reordered in a way
> that cfg80211 registration happens somewhere near the end of init()
> rather than in alloc(). Once this is done, the conf_flags can be set
> based on firmware and target information and those can then be used to
> set up the cfg80211 information before calling wiphy_register(). Until
> that gets done, it seems safest to apply this P2P enabling patch as-is
> and do the proposed clean up separately after the wiphy_register() call
> is moved.

I agree. I didn't look at code closely enough when I suggested the
changes, but it's obvious that the way you implemented is the best for
now. I have applied your original patch 4 now. Sorry for causing you
extra work.

I need to work on firmware boot changes anyway and I will also try to
cleanup cfg80211 registration as well. After that I can change p2p
implementation as we planned.

Kalle